From 09bead08be81f016e84408aead589fd31d2a6f01 Mon Sep 17 00:00:00 2001
From: Mark Wiebe <399551+mwiebe@users.noreply.github.com>
Date: Tue, 14 May 2024 10:18:52 -0700
Subject: [PATCH] fix: bundle submit parameter processing splits name/value at
 right-most = (#331)

* Also incorporated a regex for the Open Job Description identifier
  constraints, to fail parameter validation when parameter names do not
  conform.
* Added two tests, one for the '=' splitting and one for the parameter name
  validation.

Signed-off-by: Mark Wiebe <399551+mwiebe@users.noreply.github.com>
---
 .../client/cli/_groups/bundle_group.py        | 15 +++-
 .../deadline_client/cli/test_cli_bundle.py    | 84 ++++++++++++++++++-
 2 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/src/deadline/client/cli/_groups/bundle_group.py b/src/deadline/client/cli/_groups/bundle_group.py
index 1047c6cf..13648bec 100644
--- a/src/deadline/client/cli/_groups/bundle_group.py
+++ b/src/deadline/client/cli/_groups/bundle_group.py
@@ -41,18 +41,27 @@ def cli_bundle():
     """
 
 
+# Latin alphanumeric, starting with a letter
+_openjd_identifier_regex = r"(?-m:^[A-Za-z_][A-Za-z0-9_]*\Z)"
+
+
 def validate_parameters(ctx, param, value):
     """
-    Validate provided --parameter values, ensuring that they are in the format "Key=Value", and convert them to a dict with the
+    Validate provided --parameter values, ensuring that they are in the format "ParamName=Value", and convert them to a dict with the
     following format:
         [{"name": "<name>", "value": "<value>"}, ...]
     """
     parameters_split = []
     for parameter in value:
-        regex_match = re.match("(.+)=(.*)", parameter)
+        regex_match = re.match("([^=]+)=(.*)", parameter)
         if not regex_match:
             raise click.BadParameter(
-                f'Parameters must be provided in the format "Key=Value". Invalid Parameter: {parameter}'
+                f'Parameters must be provided in the format "ParamName=Value". Invalid parameter: {parameter}'
+            )
+
+        if not re.match(_openjd_identifier_regex, regex_match[1]):
+            raise click.BadParameter(
+                f"Parameter names must be alphanumeric Open Job Description identifiers. Invalid parameter name: {regex_match[1]}"
             )
 
         parameters_split.append({"name": regex_match[1], "value": regex_match[2]})
diff --git a/test/unit/deadline_client/cli/test_cli_bundle.py b/test/unit/deadline_client/cli/test_cli_bundle.py
index 487843ae..970c4455 100644
--- a/test/unit/deadline_client/cli/test_cli_bundle.py
+++ b/test/unit/deadline_client/cli/test_cli_bundle.py
@@ -487,6 +487,51 @@ def test_cli_bundle_empty_job_parameter_from_cli(fresh_deadline_config):
         assert result.exit_code == 0
 
 
+def test_cli_bundle_job_parameter_with_equals_from_cli(fresh_deadline_config):
+    """
+    Verify that a job parameter value with an '=' in it is passed correctly to the CreateJob call
+    """
+    # Use a temporary directory for the job bundle
+    with tempfile.TemporaryDirectory() as tmpdir, patch.object(boto3, "Session") as session_mock:
+        session_mock().client("deadline").create_job.return_value = MOCK_CREATE_JOB_RESPONSE
+        session_mock().client("deadline").get_job.return_value = MOCK_GET_JOB_RESPONSE
+        session_mock.reset_mock()
+
+        # Write a JSON template
+        with open(os.path.join(tmpdir, "template.json"), "w", encoding="utf8") as f:
+            f.write(MOCK_JOB_TEMPLATE_CASES["MINIMAL_JSON"][1])
+
+        runner = CliRunner()
+        result = runner.invoke(
+            main,
+            [
+                "bundle",
+                "submit",
+                tmpdir,
+                "--farm-id",
+                MOCK_FARM_ID,
+                "--queue-id",
+                MOCK_QUEUE_ID,
+                "--parameter",
+                "sceneFile=this=is=a=test",
+            ],
+        )
+
+        print(result.output)
+        session_mock().client().create_job.assert_called_once_with(
+            farmId=MOCK_FARM_ID,
+            queueId=MOCK_QUEUE_ID,
+            template=ANY,
+            templateType="JSON",
+            parameters={
+                "sceneFile": {"string": "this=is=a=test"},
+            },
+            priority=50,
+        )
+
+        assert result.exit_code == 0
+
+
 def test_cli_bundle_invalid_job_paramter(fresh_deadline_config):
     """
     Verify that a badly formatted parameter value (without "Key=Value") throws an error
@@ -517,7 +562,44 @@ def test_cli_bundle_invalid_job_paramter(fresh_deadline_config):
             ],
         )
 
-        assert 'Parameters must be provided in the format "Key=Value"' in result.output
+        assert 'Parameters must be provided in the format "ParamName=Value"' in result.output
+        assert result.exit_code == 2
+
+
+def test_cli_bundle_invalid_job_paramter_name(fresh_deadline_config):
+    """
+    Verify that a non-identifier parameter name raises an error.
+    """
+    # Use a temporary directory for the job bundle
+    with tempfile.TemporaryDirectory() as tmpdir, patch.object(boto3, "Session") as session_mock:
+        session_mock().client("deadline").create_job.return_value = MOCK_CREATE_JOB_RESPONSE
+        session_mock().client("deadline").get_job.return_value = MOCK_GET_JOB_RESPONSE
+        session_mock.reset_mock()
+
+        # Write a JSON template
+        with open(os.path.join(tmpdir, "template.json"), "w", encoding="utf8") as f:
+            f.write(MOCK_JOB_TEMPLATE_CASES["MINIMAL_JSON"][1])
+
+        runner = CliRunner()
+        result = runner.invoke(
+            main,
+            [
+                "bundle",
+                "submit",
+                tmpdir,
+                "--farm-id",
+                MOCK_FARM_ID,
+                "--queue-id",
+                MOCK_QUEUE_ID,
+                "--parameter",
+                "Param*Name=Value",
+            ],
+        )
+
+        assert (
+            "Parameter names must be alphanumeric Open Job Description identifiers."
+            in result.output
+        )
         assert result.exit_code == 2