Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request juju#13748 from rwcarlsen/action-key-guard
juju#13748 When charms set action results/keys via action-set, it is possible that the charm sets "special" action keys (i.e. stdout, stderr) via the key=val args. This caused a race condition between juju collecting stdout/stderr from running the action hook (i.e. worker/uniter/runner runner.updateActionResults) and the hook's call to "action-set [key=val]..." resulting in one of the action-set key+val or the hook stdout winning and clobbering the other in the controller. It also caused other unexpected behavior if the charm set the stdout result key to the empty string (the "stdout" key would be omitted from results entirely) or showing the content of the "stdout" result key under the capitalized "Stdout" key in the final client results printout, etc. To resolve all these issues this PR adds a check to ensure that none of the keys being set via action-set are in a reserved set that juju uses for special purposes under the hood and errors out accordingly. This was originally reported and investigated via canonical/operator#689. ## QA steps I added a unit test: ``` cd worker/uniter/runner/jujuc go test ``` canonical/operator#689 details how the problem was initially encountered - basically calling the hook tool like this `action-set stdout=42` was enough to trigger unexpected behavior, but should now just cause an error to be emitted with this PR. More specifically, an ops charm with an action that has this code will cause clobbering - and we'd prefer an error (i.e. this PR/patch): ``` def _on_foo_action(self, event) event.set_results({'stdout': 'forty-two'}) print(42) ``` It is a race between: * the runner recording `42` from the hook's stdout and * the action-set called by the ops framework using event.set_results content - `forty-two`. This patch causes an error to be emitted when running such an action instead of the race/clobbering. ## Documentation changes Included.
- Loading branch information