Merge pull request #931 from peternied/metadata-extra-args-fixed

Fix extra args command parsing used by Metadata migration

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 is None:
+            return
+
+        def isCommand(arg: str) -> bool:
+            if arg is None:
+                return False
+            return arg.startswith('--') or arg.startswith('-')
+
+        def isValue(arg: str) -> bool:
+            if arg is 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"],

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)