Allow rules to request original raw task data #1834
Merged
Conversation
This file contains 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
If `needs_raw_task` (a class level attribute) is `True`, then the original task (before normalization) will be made available under `task["__raw_task__"]`.
cognifloyd
requested review from
relrod,
ganeshrn and
priyamsahoo
and removed request for
a team
cognifloyd
changed the title
cognifloyd
changed the title
AnsibleLintRule.needs_raw_task optional property
cognifloyd
changed the title
AnsibleLintRule.needs_raw_task optional propertyAnsibleLintRule.needs_raw_task optional property
cognifloyd
changed the title
AnsibleLintRule.needs_raw_task optional propertymatchtask() via task["__raw_task__"] (rules must opt-in to get it)
ssbarnea
approved these changes
Jan 27, 2022
ssbarnea
changed the title
matchtask() via task["__raw_task__"] (rules must opt-in to get it)
|
I like this approach more than the alternative but you need to also add a simple test for it, one that verifies that a rule that requests this does receive it. This will also address the problem of decreasing the code coverage. I think we already have a custom rule inside tests that you may reuse for testing this, or you can add a new one. |
We have not had prematch()/postmatch() in a long time. Now test_rule_matching and test_rule_postmatching are effectively duplicates of each other. So drop the postmatching rule test.
Also adjust test counts now that I added another rule under test/rules.
cognifloyd
commented
Jan 27, 2022
|
K. I added a test Rule that uses this and adjusted the counts for RulesCollection since that uses the new rule now too. codecov is happier now. |
|
The test errors are failures to upload codecov reports. |
|
I seen it and did a recheck. If it happens again let's make the uploading optional. Even if we fail to upload half of the files we will likely see any coverage drop do no problems. |
|
Tests are green :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This allows a rule to opt-in to receiving a "raw" copy of the task (as it is before normalization) via
task["__raw_task__"].To use it, rules set
needs_raw_task = Trueat the class level.For one of the rules I'm developing, I need to inspect the raw (not normalized via ansible) task to identify the target issue.
This rule flags tasks that use action shorthand.
normalize_task(), however, parses the shorthand and puts it intask['action'], so there's no good way to detect the action shorthand with the currentmatchtask()interface. There are a variety of awkward workarounds like parsing the playbooks myself, but I would really like to take advantage of all the workansible-lintalready does to find the relevant tasks and skip them as needed.https://github.com/cognifloyd/ansible-lint/blob/fmt/src/ansiblelint/rules/TaskNoActionShorthand.py
For more discussion about this no-action-shorthand rule, see: #1609
The TaskNoActionShorthand rule will be extracted from #1805.
This is an alternative implementation to #1833 which was extracted from #1805. Only one of the PRs should be merged.