OCPNODE-4518: Block runc on RHEL 10 via OSImageURL stream class inspection#6238
OCPNODE-4518: Block runc on RHEL 10 via OSImageURL stream class inspection#6238bitoku wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Skipping CI for Draft Pull Request. |
|
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 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. ChangesOS image stream-class enforcement
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bitoku The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
2338c29 to
dc5c6d7
Compare
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 `@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
📒 Files selected for processing (8)
cmd/machine-config-controller/start.gopkg/controller/bootstrap/bootstrap.gopkg/controller/common/constants.gopkg/controller/common/images.gopkg/controller/render/render_controller.gopkg/controller/render/render_controller_test.gopkg/osimagestream/inspector.gotest/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
dc5c6d7 to
f79c206
Compare
f79c206 to
1a01a98
Compare
|
@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. DetailsIn response to this:
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. |
a654f87 to
7b85ae2
Compare
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 `@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
📒 Files selected for processing (7)
cmd/machine-config-controller/start.gopkg/controller/bootstrap/bootstrap.gopkg/controller/common/constants.gopkg/controller/render/render_controller.gopkg/controller/render/render_controller_test.gopkg/osimagestream/inspector.gotest/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
|
/pipeline required |
|
Scheduling tests matching the |
Assisted-by: Claude Code <https://claude.com/claude-code>
7b85ae2 to
e9b2c17
Compare
|
@bitoku: 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. |
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
Bug Fixes
Tests