-
Notifications
You must be signed in to change notification settings - Fork 365
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add option to mask potential secrents in env vars #1418
base: devel
Are you sure you want to change the base?
Conversation
since worker is used within AWX, awx credentials are sometimes passed as ENV vars to the ansible-runner. therefore they should not be printed on stdout since they could contain sensitive informations
since worker is used within AWX, awx credentials are sometimes passed as ENV vars to the ansible-runner. therefore they should not be printed on stdout since they could contain sensitive informations
Thanks for your PR. On changes like this, we have to ask someone from the Controller/AWX team to test the change to validate that it won't adversely affect that system (ping @AlanCoding or @john-westcott-iv). It might take some time for this change to be tested, so until that feedback can be given, I'd suggest investing some time in adding some unit/integration tests to this PR as we will not merge a PR that is not testing its own changes. Also, I'd like to point out that the AWX UI provides a way to use encrypted VMWare credentials (see https://ansible.readthedocs.io/projects/awx/en/24.6.1/userguide/credentials.html#vmware-vcenter), although I'm not personally able to guide you in that aspect. This is likely to be a more secure option for you than using unencrypted environment variables. |
Yes sure, we will provide some unit/integration tests.
That is exactly the way we are working. Unfortunately AWX passes the encrypted VMWare credentials ans env vars instead of ansible extra_vars to the play. |
This sounds like a security issue :) |
@Shrews thanks for starting the action workflow. |
Co-authored-by: Abhijeet Kasurde <[email protected]>
Random out loud thought... might be worth considering removing Also, unclear to me at this point that ever printing out env vars at the above point is going to be useful. |
Thanks for that not so random thought. Initially i had the same idea but there are components in AWX/tower which are requiring that ansible-runner prints at least one key-value pair in env since awx is iterating through the env vars at some point. Anyways, we discovered that a project sync in awx did not work anymore after we included this fix in our execution environment: |
Ansible-Runner is used within AWX to run ansible-playbooks in transmit and worker mode.
Since AWX passes the contents (user + password) of some credential types (e.g. VMware vcenter) to the playbook as environment variables, these environment variables are written to stdout in
streaming.py
:https://github.com/ansible/ansible-runner/blob/devel/src/ansible_runner/streaming.py#L228
self._output.write(json.dumps(printed_status_data).encode('utf-8'))
In environments such as Red Hat Openshift where global log forwarding to a log streaming server is enabled for all pods and their stdout, this immediately leads to the logging of sensitive data into log management.
This PR offers the possibility to suppress the logging of all environment variables by ansible-runner via CLI flag (
--suppress-env-print
) or environment variable (SUPPRESS_ENV_PRINT
).Due to the fact that awx uses a hard-coded CLI-call for the ansible-runner, it's important to allow log-suppressipon not only via CLI flag but also via environment variable. Otherwise, it would not be configurable:
https://github.com/ansible/awx/blob/devel/awx/main/tasks/receptor.py#L646
The suppress option can here be achieved via setting the environmet variable
SUPPRESS_ENV_PRINT=True
using the pod specification for the ansible-runner.