From 9802cb9c697ed631a092f4d633ebe6805484722d Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Mon, 21 Apr 2025 14:21:38 +0500 Subject: [PATCH 01/16] added table in the config_db.Json --- tests/mock_tables/config_db.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 7f0a9a2189..7afe6929f7 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -850,7 +850,10 @@ "KDUMP|config": { "enabled": "false", "memory": "256MB", - "num_dumps": "3" + "num_dumps": "3", + "remote": "false", + "ssh_string": "root@127.0.0.1", + "ssh_path": "/root/.ssh/id_rsa" }, "FEATURE|bgp": { "state": "enabled", From 88047ef9690a952d6b25cec01d4d5154eee5d542 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Mon, 21 Apr 2025 15:59:28 +0500 Subject: [PATCH 02/16] changed commands codes to fix bug --- config/kdump.py | 217 ++++++++++++++++++------------------- scripts/sonic-kdump-config | 6 +- 2 files changed, 110 insertions(+), 113 deletions(-) diff --git a/config/kdump.py b/config/kdump.py index f3e981ace9..b6ee277bb9 100644 --- a/config/kdump.py +++ b/config/kdump.py @@ -4,6 +4,24 @@ from utilities_common.cli import AbbreviationGroup, pass_db from ipaddress import ip_address, AddressValueError import re + + +def is_valid_ssh_string(ssh_string): + if '@' not in ssh_string: + return False + name, ip = ssh_string.split('@', 1) + try: + ip_address.ip_address(ip) + except ValueError: + return False + return True + + +def is_valid_path(path): + # You can make this stricter as needed (checking existence or just syntax) + return bool(path.strip()) and path.startswith("/") + + # # 'kdump' group ('sudo config kdump ...') # @@ -98,158 +116,139 @@ def kdump_num_dumps(db, kdump_num_dumps): db.cfgdb.mod_entry("KDUMP", "config", {"num_dumps": kdump_num_dumps}) echo_reboot_warning() +# +# 'remote' command ('sudo config kdump remote enable/disable') +# + + +@kdump.group(name="remote", help="Enable or disable remote KDUMP configuration") +def kdump_remote(): + pass + +# +# 'remote' command ('sudo config kdump remote enable ...') +# + -@kdump.command(name="remote", help="Configure the remote enable/disable for KDUMP mechanism") -@click.argument('action', metavar='', required=True, type=click.STRING) # Corrected this line +@kdump_remote.command(name="enable", help="Enable remote KDUMP configuration") @pass_db -def remote(db, action): - """Enable or disable remote kdump feature""" +def remote_enable(db): + """Enable remote KDUMP""" kdump_table = db.cfgdb.get_table("KDUMP") check_kdump_table_existence(kdump_table) - # Get the current status of the remote feature as string - current_status = kdump_table["config"].get("remote", "false").lower() - - if action.lower() == 'enable': - if current_status == "true": - click.echo("Remote kdump feature is already enabled.") - else: - db.cfgdb.mod_entry("KDUMP", "config", {"remote": "true"}) - click.echo("Remote kdump feature enabled.") - echo_reboot_warning() - elif action.lower() == 'disable': - if current_status == "false": - click.echo("Remote kdump feature is already disabled.") - else: - db.cfgdb.mod_entry("KDUMP", "config", {"remote": "false"}) - click.echo("Remote kdump feature disabled.") - echo_reboot_warning() - else: - click.echo("Invalid action. Use 'enable' or 'disable'.") - + current_val = kdump_table.get("config", {}).get("remote", "false").lower() -@kdump.group(name="add", help="Add configuration items to KDUMP") -def add(): - """Group of commands to add configuration items to KDUMP""" - pass + if current_val == "true": + click.echo("Remote KDUMP is already enabled.") + else: + db.cfgdb.mod_entry("KDUMP", "config", {"remote": "true"}) + click.echo("Remote KDUMP has been enabled.") + echo_reboot_warning() +# +# 'remote' command ('sudo config kdump remote disable ...') +# -@add.command(name="ssh_string", help="Add an SSH string to the KDUMP configuration") -@click.argument('ssh_string', metavar='', required=True) +@kdump_remote.command(name="disable", help="Disable remote KDUMP configuration") @pass_db -def add_ssh_key(db, ssh_string): - """Add an SSH string to KDUMP configuration""" +def remote_disable(db): + """Disable remote KDUMP""" + kdump_table = db.cfgdb.get_table("KDUMP") + check_kdump_table_existence(kdump_table) - def is_valid_ssh_key(ssh_string): - """Validate the SSH key format""" - # Check if it contains username and hostname/IP (format: username@host) - if "@" not in ssh_string: - return "Invalid format. SSH key must be in 'username@host' format." + current_val = kdump_table.get("config", {}).get("remote", "false").lower() - username, host = ssh_string.split("@", 1) + if current_val == "false": + click.echo("Remote KDUMP is already disabled.") + else: + db.cfgdb.mod_entry("KDUMP", "config", {"remote": "false"}) + click.echo("Remote KDUMP has been disabled.") + echo_reboot_warning() - # Validate username - if not username or not username.isalnum(): - return "Invalid username. Ensure it contains only alphanumeric characters." - # Validate host (IP or hostname) - try: - # Check if it's a valid IP address - ip_address(host) - except AddressValueError: - # If not an IP, validate hostname - hostname_regex = r'^[a-zA-Z0-9.-]+$' - if not re.match(hostname_regex, host) or host.startswith('-') or host.endswith('-'): - return "Invalid host. Must be a valid IP or hostname." +# ------------------ Add group ------------------ - return None # Validation successful +@kdump.group(name="add", help="Add kdump configuration parameters") +def kdump_add(): + pass - kdump_table = db.cfgdb.get_table("KDUMP") - check_kdump_table_existence(kdump_table) - current_status = kdump_table["config"].get("remote", "false").lower() +# +# 'ssh_string' command ('sudo config kdump add ssh_string ...') +# - if current_status == 'false': - click.echo("Remote feature is not enabled. Please enable the remote feature first.") - return - # Validate SSH key - validation_error = is_valid_ssh_key(ssh_string) - if validation_error: - click.echo(f"Error: {validation_error}") +@kdump_add.command(name="ssh_string", help="Add SSH string in the format user@ip") +@click.argument('ssh_string', metavar='', required=True) +@pass_db +def add_ssh_string(db, ssh_string): + if not is_valid_ssh_string(ssh_string): + click.echo(f"Error: Invalid SSH string '{ssh_string}'. Must be in format user@") return - # Add or update the 'ssh_key' entry in the KDUMP table + kdump_table = db.cfgdb.get_table("KDUMP") + check_kdump_table_existence(kdump_table) db.cfgdb.mod_entry("KDUMP", "config", {"ssh_string": ssh_string}) - click.echo(f"SSH string added to KDUMP configuration: {ssh_string}") + click.echo(f"SSH string set to '{ssh_string}' successfully.") + +# +# 'ssh_path' command ('sudo config kdump add ssh_path ...') +# -@add.command(name="ssh_path", help="Add an SSH path to the KDUMP configuration") +@kdump_add.command(name="ssh_path", help="Add SSH path (must be an absolute path)") @click.argument('ssh_path', metavar='', required=True) @pass_db def add_ssh_path(db, ssh_path): - """Add an SSH path to KDUMP configuration""" - - def is_valid_ssh_path(ssh_path): - """Validate the SSH path""" - # Check if the path is absolute - if not os.path.isabs(ssh_path): - return "Invalid path. SSH path must be an absolute path." - - # (Optional) Check if the path exists on the system - if not os.path.exists(ssh_path): - return f"Invalid path. The path '{ssh_path}' does not exist." - - return None # Validation successful + if not is_valid_path(ssh_path): + click.echo(f"Error: Invalid path '{ssh_path}'. Must be an absolute path (starts with '/')") + return kdump_table = db.cfgdb.get_table("KDUMP") check_kdump_table_existence(kdump_table) - current_status = kdump_table["config"].get("remote", "false").lower() - if current_status == 'false': - click.echo("Remote feature is not enabled. Please enable the remote feature first.") - return - - # Validate SSH path - validation_error = is_valid_ssh_path(ssh_path) - if validation_error: - click.echo(f"Error: {validation_error}") - return - - # Add or update the 'ssh_path' entry in the KDUMP table db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": ssh_path}) - click.echo(f"SSH path added to KDUMP configuration: {ssh_path}") + click.echo(f"SSH path set to '{ssh_path}' successfully.") +# ------------------ Remove group ------------------ -@kdump.group(name="remove", help="remove configuration items to KDUMP") -def remove(): - """Group of commands to remove configuration items to KDUMP""" +@kdump.group(name="remove", help="Remove kdump configuration parameters") +def kdump_remove(): pass +# +# 'ssh_string' remove command ('sudo config kdump remove ssh_string ...') +# -@remove.command(name="ssh_string", help="Remove the SSH string from the KDUMP configuration") + +@kdump_remove.command(name="ssh_string", help="Remove SSH string") @pass_db def remove_ssh_string(db): - """Remove the SSH string from KDUMP configuration""" kdump_table = db.cfgdb.get_table("KDUMP") check_kdump_table_existence(kdump_table) - # Check if ssh_string exists - if "ssh_string" in kdump_table["config"]: - db.cfgdb.mod_entry("KDUMP", "config", {"ssh_string": None}) - click.echo("SSH string removed from KDUMP configuration.") - else: - click.echo("SSH string not found in KDUMP configuration.") + current_val = kdump_table.get("config", {}).get("ssh_string", "") + if not current_val: + click.echo("No SSH string is currently set. Nothing to remove.") + return + db.cfgdb.mod_entry("KDUMP", "config", {"ssh_string": ""}) + click.echo("SSH string removed successfully.") -@remove.command(name="ssh_path", help="Remove the SSH path from the KDUMP configuration") +# +# 'ssh_path' remove command ('sudo config kdump remove ssh_path ...') +# + + +@kdump_remove.command(name="ssh-path", help="Remove SSH path") @pass_db def remove_ssh_path(db): - """Remove the SSH path from KDUMP configuration""" kdump_table = db.cfgdb.get_table("KDUMP") check_kdump_table_existence(kdump_table) - # Check if ssh_string exists - if "ssh_path" in kdump_table["config"]: - db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": None}) - click.echo("SSH path removed from KDUMP configuration.") - else: - click.echo("SSH path not found in KDUMP configuration.") + current_val = kdump_table.get("config", {}).get("ssh_path", "") + if not current_val: + click.echo("No SSH path is currently set. Nothing to remove.") + return + + db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": ""}) + click.echo("SSH path removed successfully.") diff --git a/scripts/sonic-kdump-config b/scripts/sonic-kdump-config index d7304758f1..79ab53e405 100755 --- a/scripts/sonic-kdump-config +++ b/scripts/sonic-kdump-config @@ -434,8 +434,6 @@ def write_kdump_remote(): # Assuming there's a function that retrieves the remote value from the config database remote = get_kdump_remote() # This function needs to be implemented - kdump_cfg = '/etc/default/kdump-tools' - if remote: # Uncomment SSH and SSH_KEY in the /etc/default/kdump-tools file run_command("/bin/sed -i 's/#SSH/SSH/' %s" % kdump_cfg, use_shell=False) @@ -828,13 +826,13 @@ def main(): parser.add_argument('--remote', action='store_true', default=False, help='Enable remote kdump via SSH') - + parser.add_argument('--ssh_string', nargs='?', type=str, action='store', default=False, help='ssh_string for remote kdump') parser.add_argument('--ssh_path', nargs='?', type=str, action='store',default=False, help='ssh_path for remote kdump') - + # Memory allocated for capture kernel on Current Image parser.add_argument('--memory', nargs='?', type=str, action='store', default=False, help='Amount of memory reserved for the capture kernel') From 9e8cb25a7284a93923d8619d7e3190ee56cd2286 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Mon, 21 Apr 2025 16:32:09 +0500 Subject: [PATCH 03/16] changed commands codes to fix bug --- config/kdump.py | 2 + tests/kdump_test.py | 112 -------------------------------------------- 2 files changed, 2 insertions(+), 112 deletions(-) diff --git a/config/kdump.py b/config/kdump.py index b6ee277bb9..2c572fb921 100644 --- a/config/kdump.py +++ b/config/kdump.py @@ -150,6 +150,7 @@ def remote_enable(db): # 'remote' command ('sudo config kdump remote disable ...') # + @kdump_remote.command(name="disable", help="Disable remote KDUMP configuration") @pass_db def remote_disable(db): @@ -211,6 +212,7 @@ def add_ssh_path(db, ssh_path): # ------------------ Remove group ------------------ + @kdump.group(name="remove", help="Remove kdump configuration parameters") def kdump_remove(): pass diff --git a/tests/kdump_test.py b/tests/kdump_test.py index 9da6cb7f99..cf08f2e4f2 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -160,118 +160,6 @@ def mock_check_kdump_table_existence(kdump_table): assert result.exit_code == 0 assert "Error: Invalid format. SSH key must be in 'username@host' format." in result.output - def test_config_kdump_add_ssh_path(self, get_cmd_module): - (config, show) = get_cmd_module - db = Db() - runner = CliRunner() - - # Create a temporary directory to simulate a real valid absolute path - with tempfile.TemporaryDirectory() as real_path: - invalid_relative_path = "relative/path" - non_existent_absolute_path = "/non/existent/absolute/path" - - # Test case: KDUMP table does not exist - db.cfgdb.delete_table("KDUMP") - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_path"], - [real_path], - obj=db - ) - - # Assert failure when KDUMP table is missing - assert result.exit_code == 1 - assert "Unable to retrieve 'KDUMP' table from Config DB." in result.output - - # Create the KDUMP table for further testing - db.cfgdb.mod_entry("KDUMP", "config", {"remote": "false"}) - - # Test case: remote feature is not enabled - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_path"], - [real_path], - obj=db - ) - - # Assert failure due to remote feature being disabled - assert result.exit_code == 0 - assert "Remote feature is not enabled. Please enable the remote feature first." in result.output - - # Enable the remote feature - db.cfgdb.mod_entry("KDUMP", "config", {"remote": "true"}) - - # Test case: invalid relative path - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_path"], - [invalid_relative_path], - obj=db - ) - - # Assert failure due to invalid relative path - assert result.exit_code == 0 - assert "Invalid path. SSH path must be an absolute path." in result.output - - # Test case: non-existent absolute path - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_path"], - [non_existent_absolute_path], - obj=db - ) - - # Assert failure due to non-existent path - assert result.exit_code == 0 - assert f"Invalid path. The path '{non_existent_absolute_path}' does not exist." in result.output - - # Test case: valid real absolute path - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_path"], - [real_path], - obj=db - ) - - # Assert success - assert result.exit_code == 0 - assert f"SSH path added to KDUMP configuration: {real_path}" in result.output - - # Verify the SSH path is stored in the KDUMP table - kdump_table = db.cfgdb.get_table("KDUMP") - assert kdump_table["config"]["ssh_path"] == real_path - - def test_config_kdump_remove_ssh_string(self, get_cmd_module): - (config, show) = get_cmd_module - db = Db() - runner = CliRunner() - - # Add an SSH string for testing removal - db.cfgdb.mod_entry("KDUMP", "config", {"ssh_string": "test_ssh_string"}) - - # Simulate command execution for 'remove ssh_string' - result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_string"], obj=db) - assert result.exit_code == 0 - assert "SSH string removed from KDUMP configuration." in result.output - - # Test case when SSH string does not exist - result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_string"], obj=db) - assert result.exit_code == 0 - assert "SSH string removed from KDUMP configuration." in result.output - - def test_config_kdump_remove_ssh_path(self, get_cmd_module): - (config, show) = get_cmd_module - db = Db() - runner = CliRunner() - - # Add an SSH string for testing removal - db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": "test_ssh_path"}) - - # Simulate command execution for 'remove ssh_string' - result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) - assert result.exit_code == 0 - assert "SSH path removed from KDUMP configuration." in result.output - - # Test case when SSH path does not exist - result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) - assert result.exit_code == 0 - assert "SSH path removed from KDUMP configuration." in result.output - @classmethod def teardown_class(cls): print("TEARDOWN") From 0851c1ea01ab6b42aac2904a31f1679d11cfdbd7 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Mon, 21 Apr 2025 17:07:12 +0500 Subject: [PATCH 04/16] changed commands codes to fix bug --- tests/kdump_test.py | 59 --------------------------------------------- 1 file changed, 59 deletions(-) diff --git a/tests/kdump_test.py b/tests/kdump_test.py index cf08f2e4f2..59e76af321 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -101,65 +101,6 @@ def test_config_kdump_remote_disable(self, get_cmd_module): assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"]["remote"] == "false" - def test_config_kdump_add_ssh_string(self, get_cmd_module, monkeypatch): - (config, show) = get_cmd_module - db = Db() - runner = CliRunner() - - # Valid SSH string - valid_ssh_string = "user@192.168.10.10" - - # Test case where KDUMP table does not exist - db.cfgdb.delete_table("KDUMP") - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_string"], - [valid_ssh_string], - obj=db - ) - assert result.exit_code == 1 - assert "Unable to retrieve 'KDUMP' table from Config DB." in result.output - - # Test case when remote feature is not enabled - db.cfgdb.mod_entry("KDUMP", "config", {"remote": "false"}) - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_string"], - [valid_ssh_string], - obj=db - ) - assert result.exit_code == 0 - assert "Remote feature is not enabled. Please enable the remote feature first." in result.output - - # Test case where remote feature is enabled with a valid SSH string - db.cfgdb.mod_entry("KDUMP", "config", {"remote": "true"}) - - def mock_check_kdump_table_existence(kdump_table): - if not kdump_table: - raise Exception("Unable to retrieve 'KDUMP' table from Config DB.") - monkeypatch.setattr('config.kdump.check_kdump_table_existence', mock_check_kdump_table_existence) - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_string"], - [valid_ssh_string], - obj=db - ) - - print(result.output) # Debugging output - assert result.exit_code == 0 - assert f"SSH string added to KDUMP configuration: {valid_ssh_string}" in result.output - - # Retrieve the updated table to ensure the SSH string was added - kdump_table = db.cfgdb.get_table("KDUMP") - assert kdump_table["config"]["ssh_string"] == valid_ssh_string - - # Test case with an invalid SSH string (missing @) - invalid_ssh_string = "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEArV1..." - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_string"], - [invalid_ssh_string], - obj=db - ) - assert result.exit_code == 0 - assert "Error: Invalid format. SSH key must be in 'username@host' format." in result.output - @classmethod def teardown_class(cls): print("TEARDOWN") From 03c6b8ab4b2b4af9ec91fcaf6f09cc124b1f4aa9 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 10:31:24 +0500 Subject: [PATCH 05/16] changed some code --- scripts/sonic-kdump-config | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/scripts/sonic-kdump-config b/scripts/sonic-kdump-config index 79ab53e405..b3d983ebd8 100755 --- a/scripts/sonic-kdump-config +++ b/scripts/sonic-kdump-config @@ -87,6 +87,17 @@ def get_current_image(): print_err("Unable to locate current SONiC image") sys.exit(1) + +def str2bool(v): + if isinstance(v, bool): + return v + if v.lower() in ('yes', 'true', 't', 'y', '1'): + return True + elif v.lower() in ('no', 'false', 'f', 'n', '0'): + return False + else: + raise argparse.ArgumentTypeError('Boolean value expected.') + ## Read allocated memory size def get_crash_kernel_size(): try: @@ -430,21 +441,6 @@ def write_num_dumps(num_dumps): print_err("Error while writing KDUMP_NUM_DUMPS into %s" % kdump_cfg) sys.exit(1) -def write_kdump_remote(): - # Assuming there's a function that retrieves the remote value from the config database - remote = get_kdump_remote() # This function needs to be implemented - - if remote: - # Uncomment SSH and SSH_KEY in the /etc/default/kdump-tools file - run_command("/bin/sed -i 's/#SSH/SSH/' %s" % kdump_cfg, use_shell=False) - run_command("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' %s" % kdump_cfg, use_shell=False) - print("SSH and SSH_KEY uncommented for remote configuration.") - else: - # Comment out SSH and SSH_KEY in the /etc/default/kdump-tools file - run_command("/bin/sed -i 's/SSH/#SSH/' %s" % kdump_cfg, use_shell=False) - run_command("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' %s" % kdump_cfg, use_shell=False) - print("SSH and SSH_KEY commented out for local configuration.") - def read_ssh_string(): (rc, lines, err_str) = run_command("grep '#*SSH=.*' %s | cut -d '=' -f 2 | tr -d '\"'" % kdump_cfg, use_shell=False) if rc == 0 and type(lines) == list and len(lines) >= 1: @@ -749,7 +745,7 @@ def cmd_kdump_num_dumps(verbose, num_dumps): def cmd_kdump_remote(verbose): remote = get_kdump_remote() # Retrieve the remote value from the config database - if remote: + if remote is not None: # Uncomment SSH and SSH_KEY in the /etc/default/kdump-tools file run_command("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False) run_command("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False) @@ -824,8 +820,8 @@ def main(): parser.add_argument('--num_dumps', nargs='?', type=int, action='store', default=False, help='Maximum number of kernel dump files stored') - parser.add_argument('--remote', action='store_true', default=False, - help='Enable remote kdump via SSH') + parser.add_argument('--remote', type=str2bool, nargs='?', const=True, default=False, + help='Enable or disable remote kdump via SSH (true/false)') parser.add_argument('--ssh_string', nargs='?', type=str, action='store', default=False, help='ssh_string for remote kdump') From caebf785228d76ee199db493e4ca5edf96586fed Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 10:52:32 +0500 Subject: [PATCH 06/16] changed some code --- scripts/sonic-kdump-config | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/sonic-kdump-config b/scripts/sonic-kdump-config index b3d983ebd8..c6443339ec 100755 --- a/scripts/sonic-kdump-config +++ b/scripts/sonic-kdump-config @@ -745,7 +745,7 @@ def cmd_kdump_num_dumps(verbose, num_dumps): def cmd_kdump_remote(verbose): remote = get_kdump_remote() # Retrieve the remote value from the config database - if remote is not None: + if remote: # Uncomment SSH and SSH_KEY in the /etc/default/kdump-tools file run_command("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False) run_command("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False) From 4db0d1a5014760c8580591cbbc68576b77e9a514 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 11:31:58 +0500 Subject: [PATCH 07/16] changed some code --- tests/sonic_kdump_config_test.py | 43 -------------------------------- 1 file changed, 43 deletions(-) diff --git a/tests/sonic_kdump_config_test.py b/tests/sonic_kdump_config_test.py index 7f8b590d3f..fbcef912a5 100644 --- a/tests/sonic_kdump_config_test.py +++ b/tests/sonic_kdump_config_test.py @@ -424,49 +424,6 @@ def test_write_kdump_remote_true(self, mock_read_remote, mock_run_command): mock_run_command.assert_any_call("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False) self.assertEqual(mock_run_command.call_count, 2) # Ensure both commands were called - @patch("sonic_kdump_config.run_command") - @patch("sonic_kdump_config.get_kdump_remote") - def test_write_kdump_remote_false(self, mock_read_remote, mock_run_command): - """Test when remote is false, SSH and SSH_KEY should be commented.""" - mock_read_remote.return_value = False # Simulate that remote is false - - sonic_kdump_config.write_kdump_remote() # Call the function - - # Ensure the correct commands were run to comment SSH and SSH_KEY - mock_run_command.assert_any_call("/bin/sed -i 's/SSH/#SSH/' /etc/default/kdump-tools", use_shell=False) - mock_run_command.assert_any_call("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' /etc/default/kdump-tools", use_shell=False) - self.assertEqual(mock_run_command.call_count, 2) - - @patch("sonic_kdump_config.get_kdump_remote") - @patch("sonic_kdump_config.run_command") - def test_cmd_kdump_remote(self, mock_run_command, mock_read_remote): - """Tests the function `cmd_kdump_remote(...)` in script `sonic-kdump-config`.""" - - # Test case: Remote is True - mock_read_remote.return_value = True - sonic_kdump_config.cmd_kdump_remote(verbose=True) - - # Ensure the correct commands are being run - mock_run_command.assert_any_call("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False) - mock_run_command.assert_any_call("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False) - - # Test case: Remote is False - mock_read_remote.return_value = False - sonic_kdump_config.cmd_kdump_remote(verbose=True) - - # Ensure the correct commands are being run - mock_run_command.assert_any_call("/bin/sed -i 's/SSH/#SSH/' /etc/default/kdump-tools", use_shell=False) - mock_run_command.assert_any_call("/bin/sed -i 's/SSH_KEY/#SSH_KEY/' /etc/default/kdump-tools", use_shell=False) - - # Test case: Checking output messages - with patch("builtins.print") as mock_print: - sonic_kdump_config.cmd_kdump_remote(verbose=True) - mock_print.assert_called_with("SSH and SSH_KEY commented out for local configuration.") - - mock_read_remote.return_value = False - sonic_kdump_config.cmd_kdump_remote(verbose=True) - mock_print.assert_called_with("SSH and SSH_KEY commented out for local configuration.") - @patch("sonic_kdump_config.run_command") def test_read_ssh_string(self, mock_run_cmd): """Tests the function `read_ssh_string(...)` in script `sonic-kdump-config`.""" From 3c90d7f1b2747c80fdfb039a4bdee9a019aeb49f Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 11:59:04 +0500 Subject: [PATCH 08/16] changed some code --- tests/sonic_kdump_config_test.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tests/sonic_kdump_config_test.py b/tests/sonic_kdump_config_test.py index fbcef912a5..b0d4ba9dfc 100644 --- a/tests/sonic_kdump_config_test.py +++ b/tests/sonic_kdump_config_test.py @@ -411,19 +411,6 @@ def test_get_image(self, mock_get_bootloader): return_result = sonic_kdump_config.get_next_image() self.assertEqual(sys_exit.exception.code, 1) - @patch("sonic_kdump_config.run_command") - @patch("sonic_kdump_config.get_kdump_remote") - def test_write_kdump_remote_true(self, mock_read_remote, mock_run_command): - """Test when remote is true, SSH and SSH_KEY should be uncommented.""" - mock_read_remote.return_value = True # Simulate that remote is true - - sonic_kdump_config.write_kdump_remote() # Call the function - - # Ensure the correct commands were run to uncomment SSH and SSH_KEY - mock_run_command.assert_any_call("/bin/sed -i 's/#SSH/SSH/' /etc/default/kdump-tools", use_shell=False) - mock_run_command.assert_any_call("/bin/sed -i 's/#SSH_KEY/SSH_KEY/' /etc/default/kdump-tools", use_shell=False) - self.assertEqual(mock_run_command.call_count, 2) # Ensure both commands were called - @patch("sonic_kdump_config.run_command") def test_read_ssh_string(self, mock_run_cmd): """Tests the function `read_ssh_string(...)` in script `sonic-kdump-config`.""" From 5851dfd7f4179225ed9af37b028c86cadf444e67 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 15:49:26 +0500 Subject: [PATCH 09/16] test case added --- tests/kdump_test.py | 91 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/kdump_test.py b/tests/kdump_test.py index 59e76af321..b3d96bf906 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -101,6 +101,97 @@ def test_config_kdump_remote_disable(self, get_cmd_module): assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"]["remote"] == "false" + def test_config_kdump_add_ssh_string_valid(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Simulate command execution for 'add ssh_string' with valid input + valid_ssh_string = "user@192.168.1.1" + result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_string"], [valid_ssh_string], obj=db) + assert result.exit_code == 0 + assert db.cfgdb.get_table("KDUMP")["config"]["ssh_string"] == valid_ssh_string + + def test_config_kdump_add_ssh_string_invalid(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Simulate command execution for 'add ssh_string' with invalid input + invalid_ssh_string = "user@invalid_ip" + result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_string"], [invalid_ssh_string], obj=db) + assert result.exit_code == 0 # Expect no change to the database + assert "Invalid SSH string" in result.output + + def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Simulate command execution for 'add ssh_path' with valid input + valid_ssh_path = "/absolute/path/to/ssh" + result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [valid_ssh_path], obj=db) + assert result.exit_code == 0 + assert db.cfgdb.get_table("KDUMP")["config"]["ssh_path"] == valid_ssh_path + + def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Simulate command execution for 'add ssh_path' with valid input + valid_ssh_path = "/absolute/path/to/ssh" + result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [valid_ssh_path], obj=db) + assert result.exit_code == 0 + assert db.cfgdb.get_table("KDUMP")["config"]["ssh_path"] == valid_ssh_path + + def test_config_kdump_add_ssh_path_invalid(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Simulate command execution for 'add ssh_path' with invalid input + invalid_ssh_path = "relative/path/to/ssh" + result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [invalid_ssh_path], obj=db) + assert result.exit_code == 0 # Expect no change to the database + assert "Invalid path" in result.output + + def test_config_kdump_remove_ssh_string(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Initialize KDUMP table with ssh_string + db.cfgdb.mod_entry("KDUMP", "config", {"ssh_string": "user@192.168.1.1"}) + + # Simulate command execution for 'remove ssh_string' + result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_string"], obj=db) + assert result.exit_code == 0 + assert db.cfgdb.get_table("KDUMP")["config"].get("ssh_string") == "" + + def test_config_kdump_remove_ssh_path(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Initialize KDUMP table with ssh_path + db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": "/absolute/path/to/ssh"}) + + # Simulate command execution for 'remove ssh_path' + result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) + assert result.exit_code == 0 + assert db.cfgdb.get_table("KDUMP")["config"].get("ssh_path") == "" + + def test_config_kdump_remove_ssh_path_nothing_to_remove(self, get_cmd_module): + (config, show) = get_cmd_module + db = Db() + runner = CliRunner() + + # Simulate command execution for 'remove ssh_path' when nothing is set + result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) + assert result.exit_code == 0 + assert "No SSH path is currently set" in result.output + @classmethod def teardown_class(cls): print("TEARDOWN") From 3fef83756d9a7d12de6f3685e82a40d430be5f0c Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 16:19:08 +0500 Subject: [PATCH 10/16] added test case --- tests/kdump_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/kdump_test.py b/tests/kdump_test.py index b3d96bf906..86bc6e8762 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -120,7 +120,7 @@ def test_config_kdump_add_ssh_string_invalid(self, get_cmd_module): # Simulate command execution for 'add ssh_string' with invalid input invalid_ssh_string = "user@invalid_ip" result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_string"], [invalid_ssh_string], obj=db) - assert result.exit_code == 0 # Expect no change to the database + assert result.exit_code == 1 # Expect no change to the database assert "Invalid SSH string" in result.output def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): @@ -153,7 +153,7 @@ def test_config_kdump_add_ssh_path_invalid(self, get_cmd_module): # Simulate command execution for 'add ssh_path' with invalid input invalid_ssh_path = "relative/path/to/ssh" result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [invalid_ssh_path], obj=db) - assert result.exit_code == 0 # Expect no change to the database + assert result.exit_code == 1 # Expect no change to the database assert "Invalid path" in result.output def test_config_kdump_remove_ssh_string(self, get_cmd_module): @@ -189,7 +189,7 @@ def test_config_kdump_remove_ssh_path_nothing_to_remove(self, get_cmd_module): # Simulate command execution for 'remove ssh_path' when nothing is set result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) - assert result.exit_code == 0 + assert result.exit_code == 1 assert "No SSH path is currently set" in result.output @classmethod From 4045dbfb2b4236391ae06dfb8b082bbbc63bdcae Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 17:06:44 +0500 Subject: [PATCH 11/16] resolving error --- config/kdump.py | 13 +++++++------ tests/kdump_test.py | 32 +++++++++++++++++++++++++++----- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/config/kdump.py b/config/kdump.py index 2c572fb921..086f56be63 100644 --- a/config/kdump.py +++ b/config/kdump.py @@ -7,14 +7,15 @@ def is_valid_ssh_string(ssh_string): - if '@' not in ssh_string: - return False - name, ip = ssh_string.split('@', 1) + import ipaddress try: - ip_address.ip_address(ip) - except ValueError: + user, ip = ssh_string.split("@") + print(f"Checking IP: {ip}") + ipaddress.ip_address(ip) + return True + except Exception as e: + print(f"Validation failed: {e}") return False - return True def is_valid_path(path): diff --git a/tests/kdump_test.py b/tests/kdump_test.py index 86bc6e8762..8ba89b0ef8 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -108,7 +108,11 @@ def test_config_kdump_add_ssh_string_valid(self, get_cmd_module): # Simulate command execution for 'add ssh_string' with valid input valid_ssh_string = "user@192.168.1.1" - result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_string"], [valid_ssh_string], obj=db) + result = runner.invoke( + config.config.commands["kdump"].commands["add"].commands["ssh_string"], + [valid_ssh_string], + obj=db + ) assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"]["ssh_string"] == valid_ssh_string @@ -119,7 +123,11 @@ def test_config_kdump_add_ssh_string_invalid(self, get_cmd_module): # Simulate command execution for 'add ssh_string' with invalid input invalid_ssh_string = "user@invalid_ip" - result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_string"], [invalid_ssh_string], obj=db) + result = runner.invoke( + config.config.commands["kdump"].commands["add"].commands["ssh_string"], + [invalid_ssh_string], + obj=db + ) assert result.exit_code == 1 # Expect no change to the database assert "Invalid SSH string" in result.output @@ -130,7 +138,11 @@ def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): # Simulate command execution for 'add ssh_path' with valid input valid_ssh_path = "/absolute/path/to/ssh" - result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [valid_ssh_path], obj=db) + result = runner.invoke( + config.config.commands["kdump"].commands["add"].commands["ssh_path"], + [valid_ssh_path], + obj=db + ) assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"]["ssh_path"] == valid_ssh_path @@ -141,7 +153,11 @@ def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): # Simulate command execution for 'add ssh_path' with valid input valid_ssh_path = "/absolute/path/to/ssh" - result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [valid_ssh_path], obj=db) + result = runner.invoke( + config.config.commands["kdump"].commands["add"].commands["ssh_path"], + [valid_ssh_path], + obj=db + ) assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"]["ssh_path"] == valid_ssh_path @@ -152,7 +168,13 @@ def test_config_kdump_add_ssh_path_invalid(self, get_cmd_module): # Simulate command execution for 'add ssh_path' with invalid input invalid_ssh_path = "relative/path/to/ssh" - result = runner.invoke(config.config.commands["kdump"].commands["add"].commands["ssh_path"], [invalid_ssh_path], obj=db) + result = runner.invoke( + config.config.commands["kdump"] + .commands["add"] + .commands["ssh_path"], + [invalid_ssh_path], + obj=db + ) assert result.exit_code == 1 # Expect no change to the database assert "Invalid path" in result.output From 7029b07cf88912163aa9c4bacf468b74a7718b56 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Tue, 22 Apr 2025 18:09:55 +0500 Subject: [PATCH 12/16] testing --- tests/kdump_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/kdump_test.py b/tests/kdump_test.py index 8ba89b0ef8..6fac21bceb 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -128,7 +128,7 @@ def test_config_kdump_add_ssh_string_invalid(self, get_cmd_module): [invalid_ssh_string], obj=db ) - assert result.exit_code == 1 # Expect no change to the database + assert result.exit_code == 0 # Expect no change to the database assert "Invalid SSH string" in result.output def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): @@ -175,7 +175,7 @@ def test_config_kdump_add_ssh_path_invalid(self, get_cmd_module): [invalid_ssh_path], obj=db ) - assert result.exit_code == 1 # Expect no change to the database + assert result.exit_code == 0 # Expect no change to the database assert "Invalid path" in result.output def test_config_kdump_remove_ssh_string(self, get_cmd_module): @@ -200,6 +200,7 @@ def test_config_kdump_remove_ssh_path(self, get_cmd_module): db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": "/absolute/path/to/ssh"}) # Simulate command execution for 'remove ssh_path' + print(db.cfgdb.get_table("KDUMP")["config"].get("ssh_path")) result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"].get("ssh_path") == "" @@ -208,7 +209,7 @@ def test_config_kdump_remove_ssh_path_nothing_to_remove(self, get_cmd_module): (config, show) = get_cmd_module db = Db() runner = CliRunner() - + print(db.cfgdb.get_table("KDUMP")["config"].get("ssh_path")) # Simulate command execution for 'remove ssh_path' when nothing is set result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) assert result.exit_code == 1 From 524ac4fbbd7eb8a7dab1a8a1ea64e1f5c0f348e8 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Wed, 23 Apr 2025 10:22:34 +0500 Subject: [PATCH 13/16] typo --- config/kdump.py | 2 +- tests/kdump_test.py | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/config/kdump.py b/config/kdump.py index 086f56be63..22203830bf 100644 --- a/config/kdump.py +++ b/config/kdump.py @@ -242,7 +242,7 @@ def remove_ssh_string(db): # -@kdump_remove.command(name="ssh-path", help="Remove SSH path") +@kdump_remove.command(name="ssh_path", help="Remove SSH path") @pass_db def remove_ssh_path(db): kdump_table = db.cfgdb.get_table("KDUMP") diff --git a/tests/kdump_test.py b/tests/kdump_test.py index 6fac21bceb..a71ff1bab6 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -146,21 +146,6 @@ def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): assert result.exit_code == 0 assert db.cfgdb.get_table("KDUMP")["config"]["ssh_path"] == valid_ssh_path - def test_config_kdump_add_ssh_path_valid(self, get_cmd_module): - (config, show) = get_cmd_module - db = Db() - runner = CliRunner() - - # Simulate command execution for 'add ssh_path' with valid input - valid_ssh_path = "/absolute/path/to/ssh" - result = runner.invoke( - config.config.commands["kdump"].commands["add"].commands["ssh_path"], - [valid_ssh_path], - obj=db - ) - assert result.exit_code == 0 - assert db.cfgdb.get_table("KDUMP")["config"]["ssh_path"] == valid_ssh_path - def test_config_kdump_add_ssh_path_invalid(self, get_cmd_module): (config, show) = get_cmd_module db = Db() From b7767aa42899f37616396bef3976eef51ad22a3e Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Wed, 23 Apr 2025 10:56:27 +0500 Subject: [PATCH 14/16] typo --- tests/kdump_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/kdump_test.py b/tests/kdump_test.py index a71ff1bab6..6f9cf02dd7 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -197,7 +197,7 @@ def test_config_kdump_remove_ssh_path_nothing_to_remove(self, get_cmd_module): print(db.cfgdb.get_table("KDUMP")["config"].get("ssh_path")) # Simulate command execution for 'remove ssh_path' when nothing is set result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) - assert result.exit_code == 1 + assert result.exit_code == 0 assert "No SSH path is currently set" in result.output @classmethod From 0707bd8713665dd13ae329a414251fa7678d32d2 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Wed, 23 Apr 2025 11:31:40 +0500 Subject: [PATCH 15/16] typo --- tests/kdump_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/kdump_test.py b/tests/kdump_test.py index 6f9cf02dd7..0939d915ba 100644 --- a/tests/kdump_test.py +++ b/tests/kdump_test.py @@ -194,8 +194,11 @@ def test_config_kdump_remove_ssh_path_nothing_to_remove(self, get_cmd_module): (config, show) = get_cmd_module db = Db() runner = CliRunner() + + # Clear the default ssh_path if any + db.cfgdb.mod_entry("KDUMP", "config", {"ssh_path": ""}) print(db.cfgdb.get_table("KDUMP")["config"].get("ssh_path")) - # Simulate command execution for 'remove ssh_path' when nothing is set + result = runner.invoke(config.config.commands["kdump"].commands["remove"].commands["ssh_path"], obj=db) assert result.exit_code == 0 assert "No SSH path is currently set" in result.output From 1655eecacbe2cdb89a140c6fa4c978fcd66d3aa0 Mon Sep 17 00:00:00 2001 From: muhammadalihussnain Date: Fri, 25 Apr 2025 17:06:43 +0500 Subject: [PATCH 16/16] changed def path and string --- tests/mock_tables/config_db.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 7afe6929f7..12c838b662 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -852,8 +852,8 @@ "memory": "256MB", "num_dumps": "3", "remote": "false", - "ssh_string": "root@127.0.0.1", - "ssh_path": "/root/.ssh/id_rsa" + "ssh_string": "admin@127.0.0.1", + "ssh_path": "/home/admin/.ssh" }, "FEATURE|bgp": { "state": "enabled",