Validate job_cluster_key inside for_each_task#5518
Open
simonfaltum wants to merge 3 commits into
Open
Conversation
The job_cluster_key validation only checked top-level tasks, so an undefined key referenced by a task nested under for_each_task passed "bundle validate" silently and failed at run time. Extend the check to tasks[*].for_each_task.task, mirroring the path set used by the job paths visitor. Co-authored-by: Isaac
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
Collaborator
Integration test reportCommit: 16c7494
29 interesting tests: 15 SKIP, 7 KNOWN, 4 flaky, 3 FAIL
Top 50 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Found during a full-repo review of the CLI. The
job_cluster_keyvalidation only checks top-level tasks. A typo'd or missing key referenced by the task nested underfor_each_taskpassesbundle validatesilently and only fails when the job runs.Changes
Before,
bundle validateonly warned about an undefinedjob_cluster_keyontasks[*]; now it also warns fortasks[*].for_each_task.task, the same path set the job paths visitor covers. The diagnostic construction inbundle/config/validate/job_cluster_key_defined.gomoved into a smallcheckJobClusterKeyhelper that is called for both the task and its nested for-each task. One level of unwrapping is enough because the Jobs API rejects nestedfor_each_task. The warning text, severity, and locations are unchanged.Test plan
job_cluster_keyinsidefor_each_task, including the reported diagnostic pathgo test ./bundle/config/validatepassesgo test ./acceptance -run TestAccept/bundle/paths/nominalpasses (the only existing fixture withjob_cluster_keyunderfor_each_task), confirming no output changes./task fmt-q,./task lint-q, and./task checkspassThis pull request and its description were written by Isaac.