Remove no_log: True from openshift_version calls

[sdodson]

#6519 set no_log: True on several plays and tasks in order to prevent logging credentials that come over from the inventory. However that's led to openshift_version role being invoked in a manner that it omits required debugging information like the following. I think we need to be very careful not to apply no_log: True at the playbook level and instead only use it on specific tasks that are known to emit sensitive information.

I think it's also worth considering that the ansible logs simply need to be treated as sensitive data and handled appropriately rather than omitting potentially useful debugging data.

# ansible-playbook /usr/share/ansible/openshift-ansible/playbooks/byo/openshift-cluster/upgrades/v3_9/upgrade.yml

TASK [openshift_version : Get available atomic-openshift version] *************************************************************************************************************************************************
task path: /usr/share/ansible/openshift-ansible/roles/openshift_version/tasks/check_available_rpms.yml:2
Using module file /usr/share/ansible/openshift-ansible/roles/lib_utils/library/repoquery.py

TASK [openshift_version : fail] ***********************************************************************************************************************************************************************************
task path: /usr/share/ansible/openshift-ansible/roles/openshift_version/tasks/check_available_rpms.yml:8
fatal: [host-xxxx.redhat.com]: FAILED! => {
    "censored": "the output has been hidden due to the fact that 'no_log: true' was specified for this result", 
    "changed": false
}

[sdodson]

https://github.com/openshift/openshift-ansible/blob/master/playbooks/init/basic_facts.yml#L18 also adds no_log: True to a playbook and I'm concerned it may mask vital information.

[michaelgugino]

/lgtm

[dak1n1]

Hmm, this does present a dilemma. I don't want to obscure the debugging logs either. Unfortunately, the secrets are revealed at the play level, during Gathering Facts. This is from a previous test I did:

PLAY [Determine openshift_version to configure on first master] ****************

TASK [Gathering Facts] *********************************************************
Thursday 08 February 2018  00:26:08 +0000 (0:00:01.890)       0:00:22.929 ***** 

<snip>
 "storage": {"s3": {"secretkey": <REDACTED>
<snip>

I double-checked and thankfully only the S3 creds are revealed here. So it's probably an acceptable level of risk. I will run it by our security lead to be sure, but I'll give this PR a +1.

[dak1n1]

👍

[michaelgugino]

@sdodson @dak1n1
This looks like another gift from openshift_facts. That would be the only reason these values would be on disk and readable.

IMO, this is a potential security concern; we shouldn't be storing secrets in openshift_facts for exactly this reason.

We should prioritize the removal of these variables from openshift_facts

[dak1n1]

@michaelgugino I agree. Let's get this tracked somehow. Should I make a bz bug? Or github issue?

[michaelgugino]

@dak1n1 Let's put it on BZ, sometimes issues don't always get the attention they deserve and this should be a high priority.

[dak1n1]

Thanks, added. https://bugzilla.redhat.com/show_bug.cgi?id=1549313

[openshift-merge-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[openshift-merge-robot]

Automatic merge from submit-queue.

[openshift-ci-robot]

@sdodson: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/system-containers	0a5f4dc	link	/test system-containers
ci/openshift-jenkins/extended_conformance_install_crio	0a5f4dc	link	/test crio

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.