Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion awx/main/models/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,9 @@ def get_job_kwargs(self):
# or labels, because they do not propogate WFJT-->node at all

# Combine WFJT prompts with node here, WFJT at higher level
node_prompts_data.update(wj_prompts_data)
# Empty string values on the workflow job (e.g. from IaC setting limit: "")
# should not override a node's explicit non-empty prompt value
node_prompts_data.update({k: v for k, v in wj_prompts_data.items() if v != ''})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The workflow job prompts actually do distinguish not-set versus "". Empty string indicates "target all hosts". And the UI should have this implemented correctly on its side.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlanCoding so the current behavior is actually the correct one?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably. This has also regressed in the past, clearly because it's so non-obvious.

accepted_fields, ignored_fields, errors = ujt_obj._accept_or_ignore_job_kwargs(**node_prompts_data)
if errors:
logger.info(
Expand Down
27 changes: 27 additions & 0 deletions awx/main/tests/functional/models/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,33 @@ def test_combine_prompts_WFJT_to_node(self, project, inventory, organization):
assert set(data['labels']) == set(node_labels) # as exception, WFJT labels not applied
assert data['limit'] == 'wj_limit'

def test_node_limit_not_overridden_by_empty_string_wj_limit(self, project, inventory):
"""
When the workflow job has an empty string limit (e.g., set via IaC with limit: ""),
the node-level limit should still be passed to the spawned job, not silently suppressed.
"""
jt = JobTemplate.objects.create(
project=project,
inventory=inventory,
ask_limit_on_launch=True,
)
# Simulate a workflow job whose WFJT was created via IaC with `limit: ""`
# (e.g. awx.awx.workflow_job_template: ... limit: "")
# This stores '' in char_prompts instead of treating it as None/"no limit".
wj = WorkflowJob.objects.create(name='test-wf-job')
wj.limit = '' # stores {'limit': ''} in char_prompts - the IaC bug scenario
wj.save()

node = WorkflowJobNode.objects.create(workflow_job=wj, unified_job_template=jt)
node.limit = 'web_servers'
node.save()

data = node.get_job_kwargs()
# The node-level limit should be applied; the WJ's empty string limit is not meaningful
assert data.get('limit') == 'web_servers', (
"Node-level limit 'web_servers' was not passed to the job. " "Likely caused by an empty string WJ limit overriding the node limit"
)


@pytest.mark.django_db
class TestWorkflowJobTemplate:
Expand Down
Loading