Skip to content

OCPNODE-4518: Block runc on RHEL 10 via OSImageURL stream class inspection#6238

Open
bitoku wants to merge 1 commit into
openshift:mainfrom
bitoku:atokubi/runc-block-osimageurl
Open

OCPNODE-4518: Block runc on RHEL 10 via OSImageURL stream class inspection#6238
bitoku wants to merge 1 commit into
openshift:mainfrom
bitoku:atokubi/runc-block-osimageurl

Conversation

@bitoku

@bitoku bitoku commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Assisted-by: Claude Code https://claude.com/claude-code

- What I did

When OSImageStream is not available, detect RHEL 10 by inspecting the container image's io.openshift.os.streamclass label from the OSImageURL. This complements the OSImageStream-based check (commit 50a5088) by covering the OSImageURL path.

- How to verify it

manual test and e2e test

- Description for the changelog

Added a block mechanism when OSImageURL is RHEL 10 based and runc is used.

Summary by CodeRabbit

  • New Features

    • Added OS image stream-class inspection from container image metadata (including mirror rules) to drive runtime enforcement decisions.
    • Improved bootstrap by falling back to inspecting the configured base OS container image when OS image stream data is unavailable.
  • Bug Fixes

    • Extended runc-blocked-on-RHEL10 enforcement to use the inspected stream class when OS image stream data is missing.
    • Added/updated caching of inspected stream-class results on generated machine configurations, with automatic invalidation when the inspected image URL changes.
  • Tests

    • Expanded unit and bootstrap test coverage for stream-class inspection, caching behavior, and enforcement outcomes.

@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

openshift-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 26, 2026
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds OS image stream-class inspection with mirror-rule support, caches the result on rendered MachineConfigs, and uses it to block runc when OSImageStream data is unavailable.

Changes

OS image stream-class enforcement

Layer / File(s) Summary
Controller wiring and cache sync
cmd/machine-config-controller/start.go, pkg/controller/render/render_controller.go, test/e2e-bootstrap/bootstrap_test.go
Render controller construction now accepts image and mirror-policy informers and waits for their caches before reconciliation starts.
Stream-class inspection helpers
pkg/controller/common/constants.go, pkg/osimagestream/inspector.go
Defines annotation keys for cached stream class and inspected image URL, and adds exported helpers for direct and mirror-aware stream-class inspection from image labels.
Runtime enforcement and bootstrap fallback
pkg/controller/bootstrap/bootstrap.go, pkg/controller/render/render_controller.go
Bootstrap now resolves a base stream class when needed, render reconciliation caches and reuses inspected stream classes, and both bootstrap and steady-state paths block runc on RHEL10 streams.
Controller and bootstrap tests
pkg/controller/render/render_controller_test.go
Updates controller test wiring, adds stream-class inspection and cache coverage, adjusts ignition expectations, and adds bootstrap stream-class enforcement coverage.

Sequence Diagram(s)

sequenceDiagram
  participant Bootstrap as Bootstrap.Run
  participant Render as render.RunBootstrap
  participant Inspector as defaultOSImageStreamClassInspector
  participant OSImageStream as osimagestream.InspectStreamClassWithMirrors

  Bootstrap->>Inspector: resolve baseStreamClass from BaseOSContainerImage
  Inspector->>OSImageStream: inspect image with mirrors
  OSImageStream-->>Inspector: stream class
  Bootstrap->>Render: RunBootstrap(..., baseStreamClass)
  Render->>Render: validateNoRuncFromOSImageURL / checkRuncBlockedOnStream
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

lgtm

Suggested reviewers

  • umohnani8
  • cheesesashimi
🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ⚠️ Warning newFakeMCLister still ignores indexer.Add errors, so malformed fixtures can be silently dropped and tests may pass on wrong state. Return or assert the Add error (e.g. require.NoError in newFakeMCLister) and thread *testing.T into the helper calls.
✅ Passed checks (13 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: blocking runc on RHEL 10 by inspecting OSImageURL stream class.
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 No dynamic or unstable test titles found; the added test names are static descriptive strings, and no Ginkgo titles use generated values.
Microshift Test Compatibility ✅ Passed PASS: The only e2e change is a helper wiring update in bootstrap_test.go; no new It/Describe/Context/When specs or MicroShift tags/skips were added.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the e2e-bootstrap change only wires extra informers into createControllers and has no multi-node/HA assumptions.
Topology-Aware Scheduling Compatibility ✅ Passed No pod specs, affinity, node selectors, replicas, PDBs, or topology-based scheduling logic were added; changes only wire informers and image-class inspection.
Ote Binary Stdout Contract ✅ Passed PASS: start.go sets logtostderr=true before logging, and the touched files have no fmt.Print/os.Stdout/TestMain/RunSpecs stdout writes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e specs were added; the touched e2e file only extends controller informer wiring and has no IPv4 or external connectivity assumptions.
No-Weak-Crypto ✅ Passed Changed files contain no weak crypto, custom crypto, or secret/token constant-time comparison issues; only null-parsing bytes.Equal and OSImageURL string caching checks.
Container-Privileges ✅ Passed Touched files only add image-inspection/render logic; no privileged, hostPID/Network/IPC, SYS_ADMIN, or allowPrivilegeEscalation settings were introduced.
No-Sensitive-Data-In-Logs ✅ Passed The changed logs are generic; the OSImageURL warning no longer prints the URL, and the new inspection paths don’t log secrets or PII.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci

openshift-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bitoku
Once this PR has been reviewed and has the lgtm label, please assign djoshy for approval. For more information see the Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@bitoku bitoku force-pushed the atokubi/runc-block-osimageurl branch from 2338c29 to dc5c6d7 Compare June 29, 2026 09:42

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@pkg/controller/bootstrap/bootstrap.go`:
- Around line 366-369: The base OS stream-class lookup in bootstrap is still
using the original pullSecret, which can miss the merged IRI credentials and
fail auth on no-registry/InternalReleaseImage installs. Update the call in Run
to pass the merged pull secret bytes used earlier after the IRI merge, and make
sure getBaseStreamClass (and any downstream inspection path it uses) receives
those newer credentials instead of the stale secret.
🪄 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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f54f4a98-8a95-4960-a70f-30f1425ce5b8

📥 Commits

Reviewing files that changed from the base of the PR and between 2338c29 and dc5c6d7.

📒 Files selected for processing (8)
  • cmd/machine-config-controller/start.go
  • pkg/controller/bootstrap/bootstrap.go
  • pkg/controller/common/constants.go
  • pkg/controller/common/images.go
  • pkg/controller/render/render_controller.go
  • pkg/controller/render/render_controller_test.go
  • pkg/osimagestream/inspector.go
  • test/e2e-bootstrap/bootstrap_test.go
✅ Files skipped from review due to trivial changes (2)
  • pkg/controller/common/images.go
  • pkg/controller/common/constants.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • cmd/machine-config-controller/start.go
  • test/e2e-bootstrap/bootstrap_test.go
  • pkg/osimagestream/inspector.go
  • pkg/controller/render/render_controller_test.go
  • pkg/controller/render/render_controller.go

Comment thread pkg/controller/bootstrap/bootstrap.go
@bitoku bitoku force-pushed the atokubi/runc-block-osimageurl branch from dc5c6d7 to f79c206 Compare June 29, 2026 10:56
@bitoku bitoku marked this pull request as ready for review June 29, 2026 10:57
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 29, 2026
@openshift-ci openshift-ci Bot requested review from djoshy and dkhater-redhat June 29, 2026 10:57
@bitoku bitoku force-pushed the atokubi/runc-block-osimageurl branch from f79c206 to 1a01a98 Compare June 29, 2026 11:01
@bitoku bitoku changed the title Block runc on RHEL 10 via OSImageURL stream class inspection OCPNODE-4518: Block runc on RHEL 10 via OSImageURL stream class inspection Jun 29, 2026
@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 29, 2026
@openshift-ci-robot

openshift-ci-robot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@bitoku: This pull request references OCPNODE-4518 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:

Assisted-by: Claude Code https://claude.com/claude-code

- What I did

When OSImageStream is not available, detect RHEL 10 by inspecting the container image's io.openshift.os.streamclass label from the OSImageURL. This complements the OSImageStream-based check (commit 50a5088) by covering the OSImageURL path.

- How to verify it

manual test and e2e test

- Description for the changelog

Added a block mechanism when OSImageURL is RHEL 10 based and runc is used.

Summary by CodeRabbit

  • New Features

  • Added support for detecting OS image stream class from container images, including mirrored registries, so the system can make smarter runtime decisions.

  • Improved bootstrap handling to account for the selected base OS image when startup data is unavailable.

  • Bug Fixes

  • Strengthened validation to block incompatible runtime settings on certain OS image streams.

  • Cached image inspection results more reliably and invalidates them when the referenced image changes.

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.

@bitoku bitoku force-pushed the atokubi/runc-block-osimageurl branch 2 times, most recently from a654f87 to 7b85ae2 Compare June 29, 2026 12:13

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 `@pkg/controller/render/render_controller_test.go`:
- Around line 1481-1485: The fake lister helper newFakeMCLister currently
ignores the error from indexer.Add, which can silently skip malformed
MachineConfig fixtures and hide bad test state. Update the helper to handle the
Add failure explicitly by either returning an error from newFakeMCLister or
switching it to take testing.T and failing the test immediately, so cache-based
tests cannot proceed with incomplete fixtures.
🪄 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: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 670ef7f7-6325-4548-9912-9fd6bd766880

📥 Commits

Reviewing files that changed from the base of the PR and between a654f87 and 7b85ae2.

📒 Files selected for processing (7)
  • cmd/machine-config-controller/start.go
  • pkg/controller/bootstrap/bootstrap.go
  • pkg/controller/common/constants.go
  • pkg/controller/render/render_controller.go
  • pkg/controller/render/render_controller_test.go
  • pkg/osimagestream/inspector.go
  • test/e2e-bootstrap/bootstrap_test.go
🚧 Files skipped from review as they are similar to previous changes (6)
  • pkg/controller/common/constants.go
  • cmd/machine-config-controller/start.go
  • test/e2e-bootstrap/bootstrap_test.go
  • pkg/osimagestream/inspector.go
  • pkg/controller/bootstrap/bootstrap.go
  • pkg/controller/render/render_controller.go

Comment thread pkg/controller/render/render_controller_test.go Outdated
@bitoku

bitoku commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

/pipeline required

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Scheduling tests matching the pipeline_run_if_changed or not excluded by pipeline_skip_if_only_changed parameters:
/test e2e-aws-ovn
/test e2e-aws-ovn-upgrade
/test e2e-gcp-op-ocl-part1
/test e2e-gcp-op-ocl-part2
/test e2e-gcp-op-part1
/test e2e-gcp-op-part2
/test e2e-gcp-op-single-node
/test e2e-hypershift

@bitoku bitoku force-pushed the atokubi/runc-block-osimageurl branch from 7b85ae2 to e9b2c17 Compare June 29, 2026 15:00
@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

@bitoku: The following test 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/e2e-aws-ovn-upgrade 7b85ae2 link true /test e2e-aws-ovn-upgrade

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

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