USHIFT-6973 Add CIS Level2 Smoke Testing#7041
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: copejon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds CIS Level 2 hardening assets and documentation for RHEL 9 and 10, periodic VM scenarios, and Robot Framework validation covering OpenSCAP results, MicroShift readiness, firewall configuration, and route-based smoke testing. ChangesCIS Level 2 hardening
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PeriodicScenario
participant RobotSuite
participant AnsibleCISRole
participant OpenSCAP
participant MicroShift
PeriodicScenario->>RobotSuite: run CIS suite
RobotSuite->>AnsibleCISRole: apply hardening
RobotSuite->>OpenSCAP: run baseline and post scans
RobotSuite->>MicroShift: install and verify workloads
RobotSuite->>OpenSCAP: compare failing rules
RobotSuite->>MicroShift: run route smoke test
Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/suites/cis/validate-cis-lvl2.robot`:
- Around line 83-89: Update the grep command in Get CIS Failure Count to run
with sudo=True so the root-owned OSCAP_RESULTS_FILE is readable, and replace “||
echo 0” with “|| true” so zero matches preserve grep’s single 0 output without
causing conversion errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8a8b0b74-4e7f-4bf5-8ca5-67d5fd5b23cf
📒 Files selected for processing (5)
test/assets/cis/cis-harden.ymltest/assets/cis/cis-requirements.ymltest/image-blueprints/layer2-presubmit/group1/rhel98-source.image-installertest/scenarios/periodics/el98-src@cis-lvl2.shtest/suites/cis/validate-cis-lvl2.robot
| run_command_on_vm host1 "sudo reboot" || true | ||
| sleep 10 | ||
| local -r ip=$(get_vm_property host1 ip) | ||
| wait_for_ssh "${ip}" |
There was a problem hiding this comment.
This is not reliable because we do not guarantee the host / ssh would go down in 10s.
Consider doing reboot in RF code where we have reliable sequences checking boot id.
Alternatively, check for some other scenarios for reboots.
pmtk
left a comment
There was a problem hiding this comment.
Description seems outdated
The .image-installer marker enables build_images.sh to produce
rhel-9.8-microshift-source.iso, which is required for the liveimg
kickstart pattern
I don't see .image-installer anywhere
|
/retest |
6690cdc to
dcd0ed9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/scenarios/periodics/el98-src`@cis-lvl2.sh:
- Line 28: Update the timeout branch in the polling function around attempts so
it outputs the relevant log before returning failure. Preserve the existing
timeout threshold and return status, ensuring the timeout message or captured
playbook log is emitted before return 1.
- Around line 38-40: Update the reboot flow around run_command_on_vm, sleep, and
wait_for_ssh so it cannot proceed against the pre-reboot instance. Implement a
boot-ID verification loop that records the pre-reboot boot ID and waits for SSH
plus a changed boot ID, or move the sequence to the existing Robot Framework
boot-ID checking flow.
- Around line 1-2: Update the shell script header to use the required
/usr/bin/bash shebang, then add set -euo pipefail immediately afterward so the
script runs with the mandated strict shell options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 30d7b809-1b9c-4930-84ff-aedc66431254
📒 Files selected for processing (4)
test/assets/cis/cis-harden.ymltest/assets/cis/cis-requirements.ymltest/scenarios/periodics/el98-src@cis-lvl2.shtest/suites/cis/validate-cis-lvl2.robot
🚧 Files skipped from review as they are similar to previous changes (3)
- test/assets/cis/cis-requirements.yml
- test/assets/cis/cis-harden.yml
- test/suites/cis/validate-cis-lvl2.robot
dcd0ed9 to
d58b3ff
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/scenarios-bootc/el10/periodics/el102-src`@cis-lvl2.sh:
- Around line 1-11: Update both periodic scripts,
test/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.sh lines 1-11 and
test/scenarios-bootc/el9/periodics/el98-src@cis-lvl2.sh lines 1-11, to use the
/usr/bin/bash shebang and add set -euo pipefail immediately after it.
In `@test/suites/cis/validate-cis-lvl2.robot`:
- Around line 133-139: Refactor the long shell command in the “Regenerate
Privileged Command Audit Rules” test into a multi-line shell block or helper
script, preserving its find, awk, output-file, and quoting behavior. Keep the
subsequent augenrules --load step unchanged and ensure the Robot file satisfies
Robocop line-length limits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: df2eb413-1cb8-47e5-a385-57c082d9e3a9
📒 Files selected for processing (11)
test/assets/cis/cis-harden-el10.ymltest/assets/cis/cis-harden.ymltest/assets/cis/cis-requirements-el10.ymltest/assets/cis/cis-requirements.ymltest/assets/cis/cis-tailoring-el10.xmltest/assets/cis/cis-tailoring-el9.xmltest/kickstart-templates/includes/main-prologue-cis.cfgtest/kickstart-templates/kickstart-liveimg-cis.ks.templatetest/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.shtest/scenarios-bootc/el9/periodics/el98-src@cis-lvl2.shtest/suites/cis/validate-cis-lvl2.robot
🚧 Files skipped from review as they are similar to previous changes (2)
- test/assets/cis/cis-harden.yml
- test/assets/cis/cis-requirements.yml
d58b3ff to
6f04552
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
test/suites/cis/validate-cis-lvl2.robot (1)
92-93: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
sudo=Trueto ensure root-owned scan artifacts can be chmodded.The OpenSCAP result files are created by a command running with
sudo=True(line 165) on a CIS-hardened system with a restrictive umask. They will be owned by root and unreadable by the standard SSH user. IfCommand Should Workdoes not inherently execute with elevated privileges, thischmodcommand will fail, causing the subsequentSSHLibrary.Get Filesteps to fail and preventing artifact collection.Consider using
Execute Commandwithsudo=Trueto be explicit and guarantee success.🛠️ Proposed change
Run Keyword And Ignore Error - ... Command Should Work chmod 644 ${OSCAP_BASELINE_FILE} ${OSCAP_POST_FILE} ${OSCAP_REPORT_FILE} + ... Execute Command chmod 644 ${OSCAP_BASELINE_FILE} ${OSCAP_POST_FILE} ${OSCAP_REPORT_FILE} sudo=True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/suites/cis/validate-cis-lvl2.robot` around lines 92 - 93, Update the chmod step in the CIS validation flow to execute with elevated privileges, explicitly using the SSH command keyword’s sudo=True option rather than relying on Command Should Work. Preserve the existing chmod targets and ignored-error behavior so root-owned OpenSCAP artifacts become readable before the subsequent SSHLibrary.Get File steps.test/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.sh (1)
13-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared "write repo file to VM" helper.
Both new periodic scripts repeat the same mktemp →
teeini content →copy_file_to_vm→ remotecp→rmsequence 3-4 times each. A single helper taking a destination path and content would remove this duplication.
test/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.sh#L13-L138: extract awrite_repo_file(dest_path, content)helper and use it inconfigure_microshift_mirror,configure_cdn_repo, the beta branch ofconfigure_rhocp_repo, and the inlinemicroshift-localblock.test/scenarios-bootc/el9/periodics/el98-src@cis-lvl2.sh#L13-L99: extract the same helper and use it inconfigure_microshift_mirror, the beta branch ofconfigure_rhocp_repo, and the inlinemicroshift-localblock.♻️ Proposed helper
+write_repo_file() { + local -r dest_path=$1 + local -r content=$2 + local -r tmp_file=$(mktemp) + printf '%s\n' "${content}" | tee "${tmp_file}" >/dev/null + copy_file_to_vm host1 "${tmp_file}" "${tmp_file}" + run_command_on_vm host1 "sudo cp ${tmp_file} ${dest_path}" + rm -f "${tmp_file}" +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/scenarios-bootc/el10/periodics/el102-src`@cis-lvl2.sh around lines 13 - 138, Extract a shared write_repo_file helper in both test/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.sh lines 13-138 and test/scenarios-bootc/el9/periodics/el98-src@cis-lvl2.sh lines 13-99, accepting the destination path and repository content while handling mktemp, transfer, remote cp, and cleanup. Replace the duplicated sequences in configure_microshift_mirror, the beta branch of configure_rhocp_repo, and the inline microshift-local block in both files; also update configure_cdn_repo in the EL10 file. Preserve each repository’s existing content and destination path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user/howto_cis_hardening.md`:
- Line 107: Update the firewall command in the CIS hardening guide to avoid
exposing port 6443 across the entire public zone; use a management-only zone or
a source-restricted firewall rule while preserving API access for authorized
management sources.
- Around line 101-106: Update the firewall guidance in the CIS hardening section
to state that only the pod network and host-endpoint sources require trust,
matching the commands shown; do not describe the service networks as requiring
firewall rules, and preserve the existing source values.
- Around line 52-55: Update the privileged-file audit rule instructions around
the find/augenrules commands to account for split-/usr systems, ensuring
/usr/bin is scanned so microshift and microshift-etcd are included; either
document the required mount-layout limitation explicitly or scan both relevant
mount points while preserving the existing rule-generation behavior.
In `@test/scenarios-bootc/el10/periodics/el102-src`@cis-lvl2.sh:
- Around line 67-81: Update configure_rhocp so the default arch is resolved
inside the target VM rather than via the runner’s uname -m; preserve the
explicit fourth-argument override and use the VM architecture when constructing
the configure_cdn_repo URL.
In `@test/suites/cis/validate-cis-lvl2.robot`:
- Around line 177-182: Refactor the inline Python passed to Execute Command in
the results-parsing block into a multi-line script assembled with Catenate,
keeping the existing rule extraction and sorting behavior. After execution,
conditionally set @{rules} to an empty list when ${stdout.strip()} is empty;
otherwise split the output as before, so zero failures produce no placeholder
entry.
---
Nitpick comments:
In `@test/scenarios-bootc/el10/periodics/el102-src`@cis-lvl2.sh:
- Around line 13-138: Extract a shared write_repo_file helper in both
test/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.sh lines 13-138 and
test/scenarios-bootc/el9/periodics/el98-src@cis-lvl2.sh lines 13-99, accepting
the destination path and repository content while handling mktemp, transfer,
remote cp, and cleanup. Replace the duplicated sequences in
configure_microshift_mirror, the beta branch of configure_rhocp_repo, and the
inline microshift-local block in both files; also update configure_cdn_repo in
the EL10 file. Preserve each repository’s existing content and destination path.
In `@test/suites/cis/validate-cis-lvl2.robot`:
- Around line 92-93: Update the chmod step in the CIS validation flow to execute
with elevated privileges, explicitly using the SSH command keyword’s sudo=True
option rather than relying on Command Should Work. Preserve the existing chmod
targets and ignored-error behavior so root-owned OpenSCAP artifacts become
readable before the subsequent SSHLibrary.Get File steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9ed96bc6-6a0b-47da-8bb0-42a022d2521d
📒 Files selected for processing (9)
docs/user/README.mddocs/user/howto_cis_hardening.mdtest/assets/cis/cis-harden-el10.ymltest/assets/cis/cis-harden.ymltest/assets/cis/cis-requirements-el10.ymltest/assets/cis/cis-requirements.ymltest/scenarios-bootc/el10/periodics/el102-src@cis-lvl2.shtest/scenarios-bootc/el9/periodics/el98-src@cis-lvl2.shtest/suites/cis/validate-cis-lvl2.robot
🚧 Files skipped from review as they are similar to previous changes (4)
- test/assets/cis/cis-requirements.yml
- test/assets/cis/cis-harden.yml
- test/assets/cis/cis-harden-el10.yml
- test/assets/cis/cis-requirements-el10.yml
|
/lgtm cancel |
|
Addressed CodeRabbit review-body nitpick findings:
|
- Fix firewall doc wording: commands trust pod network and host-endpoint sources, not service networks (10.43.0.0/16 needs no firewall rules) - Resolve VM architecture on the VM, not the CI runner, for CDN repo URLs Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6f04552 to
d378ffc
Compare
|
@copejon: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
637e223 to
8549a83
Compare
Ansible playbooks that apply CIS Level 2 hardening via RedHatOfficial CIS roles for RHEL 9 and RHEL 10. The playbooks override IP forwarding variables to prevent the role from disabling forwarding, which MicroShift OVN-Kubernetes requires for pod networking. The sudo NOPASSWD override preserves passwordless sudo needed by the CI test framework. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Test that MicroShift only introduces expected CIS rule changes on a hardened system. The Robot suite runs an OpenSCAP scan before and after installing MicroShift, computes the set difference, and asserts every new failure belongs to a known list of MicroShift-caused rules (IP forwarding, container file ownership, kubelet volume permissions, audit rules for new setuid binaries). This focuses the test on MicroShift's CIS impact rather than overall system compliance. Includes scenario scripts for RHEL 9.8 and RHEL 10.2 that provision a VM, configure repositories, and delegate hardening and scanning to the Robot suite. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
User-facing documentation describing which CIS rules MicroShift affects and why: IP forwarding for OVN-Kubernetes, container file ownership from non-host UIDs/GIDs, kubelet volume permissions, and audit rules for new setuid binaries. Includes the required ansible playbook overrides and firewall configuration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix firewall doc wording: commands trust pod network and host-endpoint sources, not service networks (10.43.0.0/16 needs no firewall rules) - Resolve VM architecture on the VM, not the CI runner, for CDN repo URLs Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- LEN03: Extract Harden And Scan Baseline keyword to reduce Setup length - VAR11: Rename report_file arg to oscap_report to avoid reserved name - DEPR06: Use VAR instead of Create List for empty list init - MISC09: Replace multi-line IF with inline IF Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reformat CIS rule tables with per-OS columns, replace duplicated firewall commands with a reference to howto_firewall.md, and rename cis-harden.yml / cis-requirements.yml with -el9 suffix for consistency with the el10 variants. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
8549a83 to
b943da2
Compare
Introduce a periodic CI scenario that installs MicroShift on a
CIS Level 2 hardened system and validates it functions correctly.
The test provisions a VM from the source image-installer ISO,
registers it with subscription-manager, installs OpenSCAP and
Ansible, applies CIS Level 2 hardening via ansible-role-rhel9-cis,
then runs four validation checks: OpenSCAP scan produces a report,
CIS failure count stays within threshold, all pods are running,
and a smoke test route is accessible.
New files:
The .image-installer marker enables build_images.sh to produce
rhel-9.8-microshift-source.iso, which is required for the liveimg
kickstart pattern (mutable RPM-based system needed for post-boot
CIS.
Summary by CodeRabbit
New Features
Documentation