Validate task attributes with first finalized attrs after loop #80476
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
ansibot
added
WIP
s-hertel
force-pushed
the
fix-post-validating-tasks-with-loops
branch
from
71a25e0
to
2b46220
Compare
s-hertel
force-pushed
the
fix-post-validating-tasks-with-loops
branch
from
2b46220
to
f74f3bd
Compare
ansibot
removed
the
WIP
s-hertel
removed
the
needs_triage
once there's a way to post validate only certain fields, we can use self._task.post_validate() at the end instead of repopulating task fields from the iteration
bcoca
approved these changes
Apr 11, 2023
…e but needs to behave like ignore_errors rather than just using the last loop single out run_once from attrs to limit scope of the change
s-hertel
commented
Apr 12, 2023
| self._task.no_log = no_log | ||
| # NOTE: run_once cannot contain loop vars because it's templated earlier also | ||
| # This is saving the post-validated field from the last loop so the strategy can use the templated value post task execution | ||
| self._task.run_once = task_fields.get('run_once') |
There was a problem hiding this comment.
I was worried self._task.from_attrs(task_fields) might cause issues, so I tried to identify all the FieldAttributes potentially affected by this issue. I believe it's limited to the fields action, run_once, ignore_unreachable, ignore_errors, loop, and no_log since these may be used after the looped task is executed.
no_logis fine, because it's set just above this changeloopis fine because empty loops == skipped task and the codepath with the issue (template string is always truthy) doesn't occurignore_errorsis fine because the value for the Task is set to False if any of the individual items are False
So I fixed ignore_unreachable using the same pattern as ignore_errors, and limited this to just set run_once and action.
bcoca
approved these changes
Apr 12, 2023
s-hertel
added a commit
to s-hertel/ansible
that referenced
this pull request
Apr 13, 2023
…le#80476) * Validate task attributes `run_once` and `action` with finalized attrs after individual loop results * Validate task attribute `ignore_unreachable` using individual loop results Once there's a way to post validate only certain fields, we can use self._task.post_validate() instead This replaces the fix introduced in ansible#80051. (cherry picked from commit bd6feeb)
s-hertel
added a commit
to s-hertel/ansible
that referenced
this pull request
Apr 13, 2023
…le#80476) * Validate task attributes `run_once` and `action` with finalized attrs after individual loop results * Validate task attribute `ignore_unreachable` using individual loop results Once there's a way to post validate only certain fields, we can use self._task.post_validate() instead This replaces the fix introduced in ansible#80051. (cherry picked from commit bd6feeb)
This was referenced Apr 13, 2023
sivel
pushed a commit
that referenced
this pull request
Apr 17, 2023
… (#80517) * Validate task attributes `run_once` and `action` with finalized attrs after individual loop results * Validate task attribute `ignore_unreachable` using individual loop results Once there's a way to post validate only certain fields, we can use self._task.post_validate() instead This replaces the fix introduced in #80051. (cherry picked from commit bd6feeb)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
SUMMARY
Fixing post validating the task itself when getting loop results. Previously we validated a copy of the task per loop item, but never finalized the overall task the strategy uses later.
This is an alternative to the fix in #80051, without breaking inheritance. Omit still does not work as intended, as with many FA (it's a truthy string), but this is a more general problem we should address.
Supersedes #80382 as a stop-gap before #80394 to unbreak inheritance.
ISSUE TYPE
COMPONENT NAME
TE