handle run_once with omit keyword on linear strategy

[tu1h]

SUMMARY

According to #80051 (comment). And after my verification, run_once template expression with omit keyword will cause failure indeed. Now fix it.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

linear strategy plugin

ADDITIONAL INFORMATION

[felixfontein]

I can confirm that this fixes the community.sops integration tests (ref #80051 (comment)).

[felixfontein]

/rebuild_failed

[s-hertel]

I don't think omit ever worked properly for run_once - before #80051 linear used the templated value ( the omit placeholder string, which is truthy) and the base strategy used the template string (also truthy), so it wouldn't have been able to inherit false.

It took me a few minutes looking at the initial patch to realize the key part was the setattr because while linear was already using the templated value, lib/ansible/plugins/strategy/__init__.py was not. free seems to have the same discrepancy: we use the templated value for run_once there but don't set it for lib/ansible/plugins/strategy/__init__.py to use the same value later. I think finalizing the value should be moved to common code that both strategy plugins can call.

[s-hertel]

free should also handle omit properly during templating.

[bcoca]

this is why we should always do post_validate as it handles omit correctly (among other things), otherwise we keep needing to recreate all functionality every time we do a partial lookup of the data.

[s-hertel]

@bcoca only at this point we don't have the information to template the whole task. Loops vars are undefined, so a module arg using a loop var would cause a traceback if we call task.post_validate(templar=templar) before accessing task.run_once. What do you think about a _post_validate_run_once method we call manually from the strategy plugins? Like s-hertel@59230c0.

[bcoca]

Currently get_validated_value is how we "mostly" post_validate a single field, this is still imperfect.

In the end just accessing the field should 'post validate it' and any other FAs it depends on (using the priority value), but that requires some redesign of post validation.

But before going through that .. we might just want to add an optional parameter to post_validate to stop at the FA specified vs looping over all of them (but still ensuring dependencies are validated first), this will probably solve many problems short term over a full redesign.

[bcoca]

related #80394, though I'm tempted to just accept this as is while we work on the permanent solution in that PR

[felixfontein]

Merging this soon (and backporting it to stable-2.15) would improve the situation for community.sops, where CI currently is red :) A permanent solution will likely take longer (and might not be backportable, depending on how it looks).

[s-hertel]

related #80394, though I'm tempted to just accept this as is...

This should not have been patched piecemeal for linear in the first place, imo. If we want to just patch linear for community.sops to "work" again, I'd be in favor of reinstating the previous behavior, i.e. a template string containing omit is truthy, rather than partially implementing this. My preference would be to revert the initial PR, I think.

[tu1h]

related #80394, though I'm tempted to just accept this as is...

This should not have been patched piecemeal for linear in the first place, imo. If we want to just patch linear for community.sops to "work" again, I'd be in favor of reinstating the previous behavior, i.e. a template string containing omit is truthy, rather than partially implementing this. My preference would be to revert the initial PR, I think.

On the previous behavior, a template string of run_once for linear is always truthy whether it contains "omit". A new design may not be completed quickly, but it is necessary that some known problems can be fixed before then, although it is imperfect. If the problems are indeed tolerated and accepted for us, I have no objection to that.

[s-hertel]

My objection is with the setattr specifically, because it breaks FA inheritance and was inconsistently applied. Either improving the consistency (like s-hertel@59230c0 - I could open that as a PR if time is an issue here), or moving the templating to occur after the getter would be preferable to me (there are several places though, so that gets a little messy).

Most places where we template FA values piecemeal do not modify the FA, so I think reverting might be the best thing to do here rather than play whack-a-mole with inheritance issues.

[s-hertel]

This should be resolved by #80476, backport for 2.15 is #80517.

omit still does not work as intended, but this should be a more general fix applied to all affected FieldAttributes.