Skip to content

Commit

Permalink
Change --host argument to --host-nqn in host add and del commands.
Browse files Browse the repository at this point in the history 
Fixes #735

Signed-off-by: Gil Bregman <gbregman@il.ibm.com>
  • Loading branch information
@gbregman
gbregman committed Aug 21, 2024
1 parent e3bf616 commit 6eb43ae
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 35 deletions.
30 changes: 13 additions & 17 deletions control/cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1034,27 +1034,25 @@ def host_add(self, args):
"""Add a host to a subsystem."""

out_func, err_func = self.get_output_functions(args)
if not args.host:
self.cli.parser.error("--host argument is mandatory for add command")
if args.host == "*" and args.psk:
if args.host_nqn == "*" and args.psk:
self.cli.parser.error("PSK is only allowed for specific hosts")

req = pb2.add_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host, psk=args.psk)
req = pb2.add_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host_nqn, psk=args.psk)
try:
ret = self.stub.add_host(req)
except Exception as ex:
if args.host == "*":
if args.host_nqn == "*":
errmsg = f"Failure allowing open host access to {args.subsystem}"
else:
errmsg = f"Failure adding host {args.host} to {args.subsystem}"
errmsg = f"Failure adding host {args.host_nqn} to {args.subsystem}"
ret = pb2.req_status(status = errno.EINVAL, error_message = f"{errmsg}:\n{ex}")

if args.format == "text" or args.format == "plain":
if ret.status == 0:
if args.host == "*":
if args.host_nqn == "*":
out_func(f"Allowing open host access to {args.subsystem}: Successful")
else:
out_func(f"Adding host {args.host} to {args.subsystem}: Successful")
out_func(f"Adding host {args.host_nqn} to {args.subsystem}: Successful")
else:
err_func(f"{ret.error_message}")
elif args.format == "json" or args.format == "yaml":
Expand All @@ -1079,25 +1077,23 @@ def host_del(self, args):
"""Delete a host from a subsystem."""

out_func, err_func = self.get_output_functions(args)
if not args.host:
self.cli.parser.error("--host argument is mandatory for del command")
req = pb2.remove_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host)
req = pb2.remove_host_req(subsystem_nqn=args.subsystem, host_nqn=args.host_nqn)

try:
ret = self.stub.remove_host(req)
except Exception as ex:
if args.host == "*":
if args.host_nqn == "*":
errmsg = f"Failure disabling open host access to {args.subsystem}"
else:
errmsg = f"Failure removing host {args.host} access to {args.subsystem}"
errmsg = f"Failure removing host {args.host_nqn} access to {args.subsystem}"
ret = pb2.req_status(status = errno.EINVAL, error_message = f"{errmsg}:\n{ex}")

if args.format == "text" or args.format == "plain":
if ret.status == 0:
if args.host == "*":
if args.host_nqn == "*":
out_func(f"Disabling open host access to {args.subsystem}: Successful")
else:
out_func(f"Removing host {args.host} access from {args.subsystem}: Successful")
out_func(f"Removing host {args.host_nqn} access from {args.subsystem}: Successful")
else:
err_func(f"{ret.error_message}")
elif args.format == "json" or args.format == "yaml":
Expand Down Expand Up @@ -1172,11 +1168,11 @@ def host_list(self, args):
argument("--subsystem", "-n", help="Subsystem NQN", required=True),
]
host_add_args = host_common_args + [
argument("--host", "-t", help="Host NQN", required=True),
argument("--host-nqn", "-t", help="Host NQN", required=True),
argument("--psk", help="Host's PSK key", required=False),
]
host_del_args = host_common_args + [
argument("--host", "-t", help="Host NQN", required=True),
argument("--host-nqn", "-t", help="Host NQN", required=True),
]
host_list_args = host_common_args + [
]
Expand Down
16 changes: 8 additions & 8 deletions tests/test_cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -636,28 +636,28 @@ def test_add_host(self, caplog, host):
except SystemExit as sysex:
rc = int(str(sysex))
pass
assert "error: the following arguments are required: --host/-t" in caplog.text
assert "error: the following arguments are required: --host-nqn/-t" in caplog.text
assert rc == 2
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", host])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", host])
if host == "*":
assert f"Allowing open host access to {subsystem}: Successful" in caplog.text
else:
assert f"Adding host {host} to {subsystem}: Successful" in caplog.text

def test_add_host_invalid_nqn(self, caplog):
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", "nqn.2016"])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "nqn.2016"])
assert f'NQN "nqn.2016" is too short, minimal length is 11' in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", "nqn.2X16-06.io.spdk:host1"])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "nqn.2X16-06.io.spdk:host1"])
assert f"invalid date code" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", "nqn.2016-06.io.spdk:host1_X"])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "nqn.2016-06.io.spdk:host1_X"])
assert f"Invalid host NQN" in caplog.text
assert f"contains invalid characters" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", f"{subsystem}_X", "--host", "nqn.2016-06.io.spdk:host2"])
cli(["host", "add", "--subsystem", f"{subsystem}_X", "--host-nqn", "nqn.2016-06.io.spdk:host2"])
assert f"Invalid subsystem NQN" in caplog.text
assert f"contains invalid characters" in caplog.text

Expand Down Expand Up @@ -754,10 +754,10 @@ def test_remove_host(self, caplog, host, gateway):
except SystemExit as sysex:
rc = int(str(sysex))
pass
assert "error: the following arguments are required: --host/-t" in caplog.text
assert "error: the following arguments are required: --host-nqn/-t" in caplog.text
assert rc == 2
caplog.clear()
cli(["host", "del", "--subsystem", subsystem, "--host", host])
cli(["host", "del", "--subsystem", subsystem, "--host-nqn", host])
if host == "*":
assert f"Disabling open host access to {subsystem}: Successful" in caplog.text
else:
Expand Down
20 changes: 10 additions & 10 deletions tests/test_psk.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def test_setup(caplog, gateway):

def test_allow_any_host(caplog, gateway):
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", "*"])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "*"])
assert f"Allowing open host access to {subsystem}: Successful" in caplog.text

def test_create_secure_with_any_host(caplog, gateway):
Expand All @@ -82,44 +82,44 @@ def test_create_secure_with_any_host(caplog, gateway):

def test_remove_any_host_access(caplog, gateway):
caplog.clear()
cli(["host", "del", "--subsystem", subsystem, "--host", "*"])
cli(["host", "del", "--subsystem", subsystem, "--host-nqn", "*"])
assert f"Disabling open host access to {subsystem}: Successful" in caplog.text

def test_create_secure(caplog, gateway):
caplog.clear()
cli(["listener", "add", "--subsystem", subsystem, "--host-name", host_name, "-a", addr, "-s", "5001", "--secure"])
assert f"Adding {subsystem} listener at {addr}:5001: Successful" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn, "--psk", hostpsk])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn, "--psk", hostpsk])
assert f"Adding host {hostnqn} to {subsystem}: Successful" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn2, "--psk", hostpsk2])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn2, "--psk", hostpsk2])
assert f"Adding host {hostnqn2} to {subsystem}: Successful" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn4, "--psk", hostpsk4])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn4, "--psk", hostpsk4])
assert f"Adding host {hostnqn4} to {subsystem}: Successful" in caplog.text

def test_create_not_secure(caplog, gateway):
caplog.clear()
cli(["listener", "add", "--subsystem", subsystem, "--host-name", host_name, "-a", addr, "-s", "5002"])
assert f"Adding {subsystem} listener at {addr}:5002: Successful" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn6])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn6])
assert f"Adding host {hostnqn6} to {subsystem}: Successful" in caplog.text
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn7])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn7])
assert f"Adding host {hostnqn7} to {subsystem}: Successful" in caplog.text

def test_create_secure_junk_key(caplog, gateway):
caplog.clear()
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn3, "--psk", hostpsk3])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn3, "--psk", hostpsk3])
assert f"Failure adding host {hostnqn3} to {subsystem}" in caplog.text

def test_create_secure_no_key(caplog, gateway):
caplog.clear()
rc = 0
try:
cli(["host", "add", "--subsystem", subsystem, "--host", hostnqn5, "--psk"])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", hostnqn5, "--psk"])
except SystemExit as sysex:
rc = int(str(sysex))
pass
Expand Down Expand Up @@ -157,7 +157,7 @@ def test_allow_any_host_with_psk(caplog, gateway):
caplog.clear()
rc = 0
try:
cli(["host", "add", "--subsystem", subsystem, "--host", "*", "--psk", hostpsk])
cli(["host", "add", "--subsystem", subsystem, "--host-nqn", "*", "--psk", hostpsk])
except SystemExit as sysex:
rc = int(str(sysex))
pass
Expand Down

0 comments on commit 6eb43ae

Please sign in to comment.