From f4160a5e9fc0045973a64ccb6864e01be1b9fdc9 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 6 Sep 2024 21:52:09 +0000 Subject: [PATCH 1/2] Fix extra args command parsing Signed-off-by: Peter Nied --- .../console_link/models/metadata.py | 41 ++++++++++++++----- .../lib/console_link/tests/test_metadata.py | 39 ++++++++++++++++++ 2 files changed, 70 insertions(+), 10 deletions(-) diff --git a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py index 3f0ebf7fe..aa206e1c5 100644 --- a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py +++ b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py @@ -9,6 +9,7 @@ from console_link.models.schema_tools import list_schema from console_link.models.cluster import AuthMethod, Cluster from console_link.models.snapshot import S3Snapshot, Snapshot, FileSystemSnapshot +from typing import Any, Dict, List logger = logging.getLogger(__name__) @@ -133,6 +134,35 @@ def _init_from_fs_snapshot(self, snapshot: FileSystemSnapshot) -> None: self._snapshot_location = "fs" self._repo_path = snapshot.repo_path + def _appendArgs(self, commands: Dict[str, Any], args_to_add: List[str]) -> None: + if args_to_add == None: + return + + def isCommand(arg: str) -> bool: + if arg == None: + return False + return arg.startswith('--') or arg.startswith('-') + + def isValue(arg: str) -> bool: + if arg == None: + return False + return not isCommand(arg) + + i = 0 + while i < len(args_to_add): + arg = args_to_add[i] + nextArg = args_to_add[i + 1] if (i + 1 < len(args_to_add)) else None + + if isCommand(arg) and isValue(nextArg): + commands[arg] = nextArg + i += 2 # Move past the command and value + elif isCommand(arg): + commands[arg] = None + i += 1 # Move past the command, its a flag + else: + logger.warning(f"Ignoring extra value {arg}, there was no command name before it") + i += 1 + def migrate(self, detached_log=None, extra_args=None) -> CommandResult: logger.info("Starting metadata migration") command_base = "/root/metadataMigration/bin/MetadataMigration" @@ -185,16 +215,7 @@ def migrate(self, detached_log=None, extra_args=None) -> CommandResult: command_args.update({"--otel-collector-endpoint": self._otel_endpoint}) # Extra args might not be represented with dictionary, so convert args to list and append commands - cmd_args = [] - for key, value in command_args.items(): - cmd_args.append(key) - if value is not None: - cmd_args.append(value) - - if extra_args: - it = iter(extra_args) - for arg in it: - cmd_args.append(arg) + self._appendArgs(command_args, extra_args) command_runner = CommandRunner(command_base, command_args, sensitive_fields=["--target-password"], diff --git a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py index b70508bd9..9e78d8b50 100644 --- a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py +++ b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py @@ -345,3 +345,42 @@ def test_metadata_with_target_sigv4_makes_correct_subprocess_call(mocker): "--target-insecure", ], stdout=None, stderr=None, text=True, check=True ) + +def test_metadata_init_with_minimal_config_and_extra_args(mocker): + config = { + "from_snapshot": { + "snapshot_name": "reindex_from_snapshot", + "s3": { + "repo_uri": "s3://my-bucket", + "aws_region": "us-east-1" + }, + } + } + metadata = Metadata(config, create_valid_cluster(), None) + + mock = mocker.patch("subprocess.run") + metadata.migrate(extra_args=[ + "--foo", "bar", # Pair of command and value + "--flag", # Flag with no value afterward + "--bar", "baz", # Another pair of command and value + "bazzy" # Lone value, will be ignored + ]) + + print(mock.call_args_list) + + + mock.assert_called_once_with([ + '/root/metadataMigration/bin/MetadataMigration', + "--snapshot-name", config["from_snapshot"]["snapshot_name"], + '--target-host', 'https://opensearchtarget:9200', + '--min-replicas', '0', + "--s3-local-dir", mocker.ANY, + "--s3-repo-uri", config["from_snapshot"]["s3"]["repo_uri"], + "--s3-region", config["from_snapshot"]["s3"]["aws_region"], + '--target-username', 'admin', + '--target-password', 'myStrongPassword123!', + '--target-insecure', + '--foo', 'bar', + '--flag', + '--bar', 'baz' + ], stdout=None, stderr=None, text=True, check=True) \ No newline at end of file From 84cbe46416e3382ffc51800e6a71b72f8ee9d1c5 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Mon, 9 Sep 2024 08:25:48 -0500 Subject: [PATCH 2/2] Fix flake8 violations Signed-off-by: Peter Nied --- .../lib/console_link/console_link/models/metadata.py | 6 +++--- .../lib/console_link/tests/test_metadata.py | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py index aa206e1c5..22faef9f3 100644 --- a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py +++ b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/console_link/models/metadata.py @@ -135,16 +135,16 @@ def _init_from_fs_snapshot(self, snapshot: FileSystemSnapshot) -> None: self._repo_path = snapshot.repo_path def _appendArgs(self, commands: Dict[str, Any], args_to_add: List[str]) -> None: - if args_to_add == None: + if args_to_add is None: return def isCommand(arg: str) -> bool: - if arg == None: + if arg is None: return False return arg.startswith('--') or arg.startswith('-') def isValue(arg: str) -> bool: - if arg == None: + if arg is None: return False return not isCommand(arg) diff --git a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py index 9e78d8b50..fe8db19af 100644 --- a/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py +++ b/TrafficCapture/dockerSolution/src/main/docker/migrationConsole/lib/console_link/tests/test_metadata.py @@ -346,6 +346,7 @@ def test_metadata_with_target_sigv4_makes_correct_subprocess_call(mocker): ], stdout=None, stderr=None, text=True, check=True ) + def test_metadata_init_with_minimal_config_and_extra_args(mocker): config = { "from_snapshot": { @@ -360,15 +361,14 @@ def test_metadata_init_with_minimal_config_and_extra_args(mocker): mock = mocker.patch("subprocess.run") metadata.migrate(extra_args=[ - "--foo", "bar", # Pair of command and value - "--flag", # Flag with no value afterward - "--bar", "baz", # Another pair of command and value - "bazzy" # Lone value, will be ignored + "--foo", "bar", # Pair of command and value + "--flag", # Flag with no value afterward + "--bar", "baz", # Another pair of command and value + "bazzy" # Lone value, will be ignored ]) print(mock.call_args_list) - mock.assert_called_once_with([ '/root/metadataMigration/bin/MetadataMigration', "--snapshot-name", config["from_snapshot"]["snapshot_name"], @@ -383,4 +383,4 @@ def test_metadata_init_with_minimal_config_and_extra_args(mocker): '--foo', 'bar', '--flag', '--bar', 'baz' - ], stdout=None, stderr=None, text=True, check=True) \ No newline at end of file + ], stdout=None, stderr=None, text=True, check=True)