Skip to content

Commit

Permalink
fix: bundle submit parameter processing splits name/value at right-mo…
Browse files Browse the repository at this point in the history
…st = (aws-deadline#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>
  • Loading branch information
mwiebe authored May 14, 2024
1 parent 3e369f9 commit 09bead0
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 4 deletions.
15 changes: 12 additions & 3 deletions src/deadline/client/cli/_groups/bundle_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]})
Expand Down
84 changes: 83 additions & 1 deletion test/unit/deadline_client/cli/test_cli_bundle.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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


Expand Down

0 comments on commit 09bead0

Please sign in to comment.