Update ansible remediation CCE-85972-8 to support idempotency

[namoyer10]

Description:

• Add idempotency to the users_nopasswd compliance check for CCE-85972-8.

Rationale:

• Ansible should be able to run over and over again on systems and return 0 changes if nothing was actually modified on the target system. The current behavior will always return a changed value.

• Fixes: Added "changed_when" logic to only report a change if the returned stdout_lines is populated with results. If no results are returned, it will report as "ok".

Review Hints:

Result JSON from a run with this change:

{"changed": false, "cmd": ["awk", "-F:", "!$2 {print $1}", "/etc/shadow"], "delta": "0:00:00.006307", "end": "2024-07-12 08:36:16.120933", "msg": "", "rc": 0, "start": "2024-07-12 08:36:16.114626", "stderr": "", "stderr_lines": [], "stdout": "", "stdout_lines": []}

[openshift-ci]

Hi @namoyer10. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[github-actions]

Start a new ephemeral environment with changes proposed in this pull request:

rhel8 (from CTF) Environment (using Fedora as testing environment)

Fedora Testing Environment

Oracle Linux 8 Environment

[github-actions]

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_no_empty_passwords_etc_shadow' differs.
--- xccdf_org.ssgproject.content_rule_no_empty_passwords_etc_shadow
+++ xccdf_org.ssgproject.content_rule_no_empty_passwords_etc_shadow
@@ -2,6 +2,7 @@
   command: |
     awk -F: '!$2 {print $1}' /etc/shadow
   register: users_nopasswd
+  changed_when: false
   when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
   tags:
   - CCE-85953-8

[github-actions]

🤖 A k8s content image for this PR is available at:
ghcr.io/complianceascode/k8scontent:12152
This image was built from commit: 97b87d5

Click here to see how to deploy it

If you alread have Compliance Operator deployed:
utils/build_ds_container.py -i ghcr.io/complianceascode/k8scontent:12152

Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and:
CONTENT_IMAGE=ghcr.io/complianceascode/k8scontent:12152 make deploy-local

[marcusburghardt]

Actually this doesn't affect the task idempotency, but is good to not report "changed" in a task only intended to collect information. In this case, it would be even better to have changed_when: false instead.

[namoyer10]

Actually this doesn't affect the task idempotency, but is good to not report "changed" in a task only intended to collect information. In this case, it would be even better to have changed_when: false instead.

Good morning,

My initial thought on this was to still have the information gathering check show "changed" when changes are needed. This will allow for easier review of failed or changed tasks by downstream projects where the remediations are combined into a single, mono-playbook for hardening purposes such as the RHEL CUI repos. From my understanding, those playbooks use the ComplianceAsCode Framework for their hardening profiles and playbooks.

If you would still like the mentioned change, let me know. Just wanted to throw out my thoughts before making changes.

[marcusburghardt]

Actually this doesn't affect the task idempotency, but is good to not report "changed" in a task only intended to collect information. In this case, it would be even better to have changed_when: false instead.

Good morning,

My initial thought on this was to still have the information gathering check show "changed" when changes are needed. This will allow for easier review of failed or changed tasks by downstream projects where the remediations are combined into a single, mono-playbook for hardening purposes such as the RHEL CUI repos. From my understanding, those playbooks use the ComplianceAsCode Framework for their hardening profiles and playbooks.

If you would still like the mentioned change, let me know. Just wanted to throw out my thoughts before making changes.

Yes, since the command is not actually changing anything, but just collecting information, changed_when: false is better for this task and also used many times in similar cases within the project. The subsequente task, actually changes something based on the information collected from the previous task, so it will still report the status properly (skipped or changed).

[codeclimate]

Code Climate has analyzed commit 97b87d5 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 59.4% (0.0% change).

View more on Code Climate.

[marcusburghardt]

Thanks for the contribution @namoyer10

[marcusburghardt]

The testing-farm:centos-stream-9-x86_64:/static-checks is failure was already fixed by
#12174 and is not related to this PR.

Considering this PR is simple, it is safe to waive this test and merge it without rebasing it.