Skip to content

MCO-2393: Use /proc/sys/crypto/fips_enabled for FIPS check in tests#6233

Open
HarshwardhanPatil07 wants to merge 2 commits into
openshift:mainfrom
HarshwardhanPatil07:fix-fips-check-rhel10
Open

MCO-2393: Use /proc/sys/crypto/fips_enabled for FIPS check in tests#6233
HarshwardhanPatil07 wants to merge 2 commits into
openshift:mainfrom
HarshwardhanPatil07:fix-fips-check-rhel10

Conversation

@HarshwardhanPatil07

@HarshwardhanPatil07 HarshwardhanPatil07 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

- What I did

  • Updated IsFIPSEnabled() in test/extended-priv/node.go to read /proc/sys/crypto/fips_enabled instead of running fips-mode-setup --check. The fips-mode-setup binary (from crypto-policies-scripts) is not available on RHEL 10, which causes the MCO kernel test PolarionID:53668 to fail with chroot: failed to run command 'fips-mode-setup': No such file or directory. The /proc/sys/crypto/fips_enabled interface is available on all RHEL versions and is already used by the MCO daemon itself in pkg/daemon/update.go.

- How to verify it

  • Run the MCO kernel test PolarionID:53668 ([sig-mco] MCO kernel when FIPS and realtime kernel are both enabled node should NOT be degraded) on an OCP 5.0 FIPS-enabled cluster.

- Description for the changelog

  • Use /proc/sys/crypto/fips_enabled for FIPS check in tests to fix RHEL 10 compatibility.

Summary by CodeRabbit

  • Bug Fixes
    • Improved FIPS mode detection by reading the system’s FIPS status directly and interpreting the result more reliably, instead of relying on the previous command output.

fips-mode-setup does not exist on RHEL 10, causing PolarionID:53668
to fail with "No such file or directory". Use /proc/sys/crypto/fips_enabled
instead, consistent with how the MCO daemon checks FIPS in pkg/daemon/update.go.
@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 25, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@HarshwardhanPatil07: This pull request references MCO-2393 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set.

Details

In response to this:

- What I did

  • Updated IsFIPSEnabled() in test/extended-priv/node.go to read /proc/sys/crypto/fips_enabled instead of running fips-mode-setup --check. The fips-mode-setup binary (from crypto-policies-scripts) is not available on RHEL 10, which causes the MCO kernel test PolarionID:53668 to fail with chroot: failed to run command 'fips-mode-setup': No such file or directory. The /proc/sys/crypto/fips_enabled interface is available on all RHEL versions and is already used by the MCO daemon itself in pkg/daemon/update.go.

- How to verify it

  • Run the MCO kernel test PolarionID:53668 ([sig-mco] MCO kernel when FIPS and realtime kernel are both enabled node should NOT be degraded) on an OCP 5.0 FIPS-enabled cluster.

- Description for the changelog

  • Use /proc/sys/crypto/fips_enabled for FIPS check in tests to fix RHEL 10 compatibility.

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 07408b0f-25bf-4466-bdd3-06034a9e7623

📥 Commits

Reviewing files that changed from the base of the PR and between 4603002 and 1ccbd74.

📒 Files selected for processing (1)
  • test/extended-priv/node.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/extended-priv/node.go

Walkthrough

IsFIPSEnabled now reads /proc/sys/crypto/fips_enabled, trims the content, and treats 1 as enabled. It still logs read errors and returns the computed boolean with the error.

Changes

FIPS detection path

Layer / File(s) Summary
Read kernel FIPS flag
test/extended-priv/node.go
IsFIPSEnabled reads /proc/sys/crypto/fips_enabled instead of running fips-mode-setup --check, trims the file contents, and compares the result to 1 while keeping the existing error handling shape.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested labels

jira/valid-bug

Suggested reviewers

  • proietfb
  • ptalgulk01
🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: switching the test FIPS check to /proc/sys/crypto/fips_enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed The only changed file is a helper; it adds no Ginkgo test titles or dynamic test names.
Test Structure And Quality ✅ Passed Helper-only change; the related Ginkgo test keeps explicit cleanup, no indefinite waits, and standard assertion messaging.
Microshift Test Compatibility ✅ Passed PASS: The PR only changes the IsFIPSEnabled helper; it adds no new Ginkgo tests, tags, or MicroShift-unsupported APIs/resources.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The PR only changes the IsFIPSEnabled helper in test/extended-priv/node.go; no Ginkgo test blocks or SNO assumptions were added.
Topology-Aware Scheduling Compatibility ✅ Passed Only a test helper in test/extended-priv/node.go changed; no manifests, controllers, or scheduling constraints were introduced.
Ote Binary Stdout Contract ✅ Passed The PR only changes a helper method in node.go; no stdout writes were found in process-level code or suite setup files.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Only test/extended-priv/node.go changed; it updates a FIPS helper and adds no new Ginkgo tests or network/internet assumptions.
No-Weak-Crypto ✅ Passed Changed code only reads /proc/sys/crypto/fips_enabled and trims stdout; no weak ciphers, custom crypto, or secret comparisons were added.
Container-Privileges ✅ Passed PR only changes test/extended-priv/node.go FIPS helper; no container/K8s manifests or securityContext fields (privileged/hostPID/etc.) are modified.
No-Sensitive-Data-In-Logs ✅ Passed The PR only swaps the FIPS probe command; it doesn’t add any logs of secrets/PII, and the new error log only records a generic error.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

Command failed


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HarshwardhanPatil07

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [HarshwardhanPatil07]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 25, 2026
@HarshwardhanPatil07

Copy link
Copy Markdown
Contributor Author

/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-fips-proxy-longduration-1of3

@openshift-ci

openshift-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

@HarshwardhanPatil07: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-fips-proxy-longduration-1of3

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/ec607e20-7074-11f1-81c3-7fd87333d00f-0

@HarshwardhanPatil07 HarshwardhanPatil07 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verification Steps on Cluster Locally
 
```
harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc get cm cluster-config-v1 -n kube-system -o jsonpath='{.data.install-config}'|grep fips
fips: true

harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ cat > /tmp/fips.yaml << 'EOF'
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: master
name: 50-fips-bz-poc
spec:
config:
ignition:
version: 3.2.0
kernelArguments:

  • trigger-sno-fips-issue=1
    EOF

harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc create -f /tmp/fips.yaml
machineconfig.machineconfiguration.openshift.io/50-fips-bz-poc created

harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc get mcp
NAME CONFIG UPDATED UPDATING DEGRADED MACHINECOUNT READYMACHINECOUNT UPDATEDMACHINECOUNT DEGRADEDMACHINECOUNT AGE
master rendered-master-6ded900152cc64012955c5788254db10 False True False 3 2 2 0 175m
worker rendered-worker-023bec9aece038cf11b074c60087158b True False False 3 3 3 0 175m

harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ cat > /tmp/kernel.yaml << 'EOF'
apiVersion: machineconfiguration.openshift.io/v1
kind: MachineConfig
metadata:
labels:
machineconfiguration.openshift.io/role: master
name: 50-realtime-kernel
spec:
config:
ignition:
version: 3.2.0
kernelType: "realtime"
EOF
oc create -f /tmp/kernel.yaml
machineconfig.machineconfiguration.openshift.io/50-realtime-kernel created

harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc get mcp
NAME CONFIG UPDATED UPDATING DEGRADED MACHINECOUNT READYMACHINECOUNT UPDATEDMACHINECOUNT DEGRADEDMACHINECOUNT AGE
master rendered-master-35bb86e8f2513414d4375b379f0171ae True False False 3 3 3 0 5h49m
worker rendered-worker-023bec9aece038cf11b074c60087158b True False False 3 3 3 0 5h49m

harshpat@harshpat-thinkpadp1gen4i:/Downloads/repos/openshift-tests-private$ node=$(oc get nodes -l node-role.kubernetes.io/master -o jsonpath='{.items[0].metadata.name}')
echo "Target Node: $node"
Target Node: ip-10-0-21-23.us-east-2.compute.internal
harshpat@harshpat-thinkpadp1gen4i:
/Downloads/repos/openshift-tests-private$ oc debug node/$node -- chroot /host cat /proc/sys/crypto/fips_enabled 2>&1 | grep -v Warning
Starting pod/ip-10-0-21-23us-east-2computeinternal-debug-5lq5w ...
To use host binaries, run chroot /host. Instead, if you need to access host namespaces, run nsenter -a -t 1.
1

Removing debug pod ...
harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc debug node/$node -- chroot /host fips-mode-setup --check 2>&1 | grep -v Warning
Starting pod/ip-10-0-21-23us-east-2computeinternal-debug-mcn4h ...
To use host binaries, run chroot /host. Instead, if you need to access host namespaces, run nsenter -a -t 1.
FIPS mode is enabled.

Removing debug pod ...
harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc debug node/$node -- chroot /host cat /proc/cmdline 2>&1 | grep -v Warning
Starting pod/ip-10-0-21-23us-east-2computeinternal-debug-4rtt5 ...
To use host binaries, run chroot /host. Instead, if you need to access host namespaces, run nsenter -a -t 1.
BOOT_IMAGE=(hd0,gpt3)/boot/ostree/rhcos-45e8e6b723ebef5595d8c8f3e2a6cf8079fffd5838d60b9e3d6abce559e01cbe/vmlinuz-5.14.0-687.17.1.el9_8.x86_64+rt rw ostree=/ostree/boot.0/rhcos/45e8e6b723ebef5595d8c8f3e2a6cf8079fffd5838d60b9e3d6abce559e01cbe/0 ignition.platform.id=aws console=tty0 console=ttyS0,115200n8 fips=1 boot=LABEL=boot root=UUID=b56bd452-a78f-4697-885a-a677ef7d9998 rw rootflags=prjquota trigger-sno-fips-issue=1 trigger-fips-issue=1 systemd.unified_cgroup_hierarchy=1 cgroup_no_v1=all

Removing debug pod ...
harshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/openshift-tests-private$ oc debug node/$node -- uname -a 2>&1 | grep -v Warning
Starting pod/ip-10-0-21-23us-east-2computeinternal-debug-zn5lj ...
To use host binaries, run chroot /host. Instead, if you need to access host namespaces, run nsenter -a -t 1.
Linux ip-10-0-21-23 5.14.0-687.17.1.el9_8.x86_64+rt #1 SMP PREEMPT_RT Thu Jun 18 13:38:12 EDT 2026 x86_64 x86_64 x86_64 GNU/Linux

Removing debug pod ...

</pre>
</details>

…t from stderr

DebugNodeWithChroot concatenates stdout and stderr, so checking
the output of `cat /proc/sys/crypto/fips_enabled` with an exact
match `== "1"` fails because stderr contains debug pod lifecycle
messages. Use DebugNodeWithChrootStd which returns stdout separately.

Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
@openshift-ci

openshift-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

@HarshwardhanPatil07: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/verify-deps 1ccbd74 link true /test verify-deps
ci/prow/unit 1ccbd74 link true /test unit

Full PR test history. Your PR dashboard.

Details

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants