https://github.com/ansible/ansible/pull/77503 suggested that an ActionModule could legitimately want to access both become_ and modules_ variables right before encoding, and doing so via moving a small code block about become_kwargs into a distinct method, thus allowing an ActionModule to (easily) override it.
Context:
_configure_module is a critical but very monolithic method.
An ActionModule could find a use-case for altering this method what implies a bunch of boilerplate code copy/pasting. This method is also responsible of calling modify_module after which module’s arguments are definitely encoded.
The newly introduced method takes as its arguments multiple variables not available to a child-class otherwise. The default behavior is totally preserved and such a new method gives a chance to an ActionModule to modify them.
This trivial change greatly opens up extensibility of ActionModule in the most needed code-path.
Bonus point: It may make that part of the code easier to unit-test.
Replying to s-hertel concerns:
- Security: An ActionModule can already alter argument processing in pretty much any way… and it’s fine. (User must install modules deemed secure).
Having a code difficult to override keeps good developers from improving and extending it rather than keeping evil developers from writing evil modules.
- Examples: The use-case of a module which needs two things: 1) Passing become_* arguments down to the remote execution context. 2) Alter the modules’ arguments before AnsiballZ encoding (which may be cleaner and more generic than overriding multiple individual modules) : It’s useful for sandboxed runs, for logging purpose, granular command-line processing, playbook simulations and more generally to any mod that need altering processing/arguments for several modules at once.
I hope that, contrary to the GitHub PR, this group is a room for discussion.
Regards