Skip to content

OCPBUGS-83619: Add version guard for OSStream rendering#6111

Merged
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
djoshy:fix-osstream-guard
Jun 2, 2026
Merged

OCPBUGS-83619: Add version guard for OSStream rendering#6111
openshift-merge-bot[bot] merged 1 commit into
openshift:mainfrom
djoshy:fix-osstream-guard

Conversation

@djoshy
Copy link
Copy Markdown
Contributor

@djoshy djoshy commented Jun 1, 2026

- What I did
Added a version guard so the render controller will ignore the OSImageStream generated by a new operator so a stale controller won't roll the MachineConfigs. Also added a couple units to verify this behavior.

- How to verify it
See bug for reproduction steps.

Summary by CodeRabbit

  • Bug Fixes
    • Added version compatibility validation for operating system image streams to prevent using images from incompatible operator versions.

@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 jira/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 1, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@djoshy: This pull request references Jira Issue OCPBUGS-83619, which is invalid:

  • expected the bug to target the "5.0.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

- What I did
Added a version guard so the render controller will ignore the OSImageStream generated by a new operator so a stale controller won't roll the MachineConfigs. Also added a couple units to verify this behavior.

- How to verify it
See bug for reproduction steps.

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.

@openshift-ci openshift-ci Bot requested review from RishabhSaini and yuqi-zhang June 1, 2026 14:40
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 1, 2026

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: e6b876fd-ed2f-4052-89c0-1e53eb233562

📥 Commits

Reviewing files that changed from the base of the PR and between d72b715 and 60ff74a.

📒 Files selected for processing (2)
  • pkg/controller/render/render_controller.go
  • pkg/controller/render/render_controller_test.go

Walkthrough

The PR adds a version guard to getOSImageStreamForPool that validates fetched OSImageStream objects contain a release version annotation matching the current operator version. A new test verifies the guard rejects mismatched or missing annotations and accepts matching ones.

Changes

OSImageStream Version Guard

Layer / File(s) Summary
Version guard implementation
pkg/controller/render/render_controller.go
getOSImageStreamForPool now checks that the fetched OSImageStream has a ReleaseImageVersionAnnotationKey annotation matching version.Hash, returning errors when the annotation is missing or does not match.
Version guard test coverage
pkg/controller/render/render_controller_test.go
New TestGetOSImageStreamVersionGuard test with updated imports validates the guard behavior across matching, mismatched, and missing version-annotation cases using an in-memory indexer.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • zaneb
  • pawanpinjarkar
🚥 Pre-merge checks | ✅ 13 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% 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 ❓ Inconclusive The custom check is designed for Ginkgo test code (It blocks, BeforeEach/AfterEach, Eventually/Consistently), but the PR adds a standard Go test using *testing.T with table-driven test patterns. The check applies to Ginkgo tests; clarify whether standard Go tests with testify assertions and table-driven patterns should be assessed differently.
✅ 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 describes the main change: adding a version guard mechanism for OSStream rendering in the render controller.
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 PR adds only standard Go tests (using testing.T), not Ginkgo tests. The custom check is specific to Ginkgo test names, so it is not applicable to this codebase.
Microshift Test Compatibility ✅ Passed The added test is a standard Go unit test (TestGetOSImageStreamVersionGuard), not a Ginkgo e2e test. Custom check only applies to Ginkgo e2e tests (It(), Describe(), etc.).
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR adds only standard Go unit tests (TestGetOSImageStreamVersionGuard), not Ginkgo e2e tests. SNO compatibility check applies only to new Ginkgo e2e tests.
Topology-Aware Scheduling Compatibility ✅ Passed This change adds version guard validation in controller logic, with no scheduling constraints, pod affinity rules, nodeSelectors, or workload deployments that would affect topology compatibility.
Ote Binary Stdout Contract ✅ Passed PR modifies render controller package (main operator code, not OTE). Changes confined to method implementations and unit tests, no process-level code or stdout writes.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The added test is a standard Go unit test, not a Ginkgo e2e test. The check applies only to Ginkgo e2e tests, which this PR does not contain.
No-Weak-Crypto ✅ Passed No weak crypto (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB) or custom implementations found. Version hash comparison is for version validation, not sensitive tokens.
Container-Privileges ✅ Passed PR modifies only Go source files adding OSImageStream version guard logic. No container/K8s manifests with privileged settings were modified.
No-Sensitive-Data-In-Logs ✅ Passed No sensitive data (passwords, tokens, API keys, PII, session IDs, hostnames, or customer data) is logged. Only pool names, annotation keys, and version hashes are logged.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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


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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2026
@djoshy
Copy link
Copy Markdown
Contributor Author

djoshy commented Jun 1, 2026

/cherry-pick release-4.22

/jira refresh

@openshift-cherrypick-robot
Copy link
Copy Markdown

@djoshy: once the present PR merges, I will cherry-pick it on top of release-4.22 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.22

/jira refresh

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.

@openshift-ci-robot openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 1, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@djoshy: This pull request references Jira Issue OCPBUGS-83619, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

/cherry-pick release-4.22

/jira refresh

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.

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@djoshy: This pull request references Jira Issue OCPBUGS-83619, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.0.0) matches configured target version for branch (5.0.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)
Details

In response to this:

- What I did
Added a version guard so the render controller will ignore the OSImageStream generated by a new operator so a stale controller won't roll the MachineConfigs. Also added a couple units to verify this behavior.

- How to verify it
See bug for reproduction steps.

Summary by CodeRabbit

  • Bug Fixes
  • Added version compatibility validation for operating system image streams to prevent using images from incompatible operator versions.

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.

Copy link
Copy Markdown
Member

@isabella-janssen isabella-janssen left a comment

Choose a reason for hiding this comment

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

/lgtm

Change makes sense and the new test cases for functionality are passing.

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 1, 2026
@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

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jun 1, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: djoshy, isabella-janssen

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 [djoshy,isabella-janssen]

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

@isabella-janssen
Copy link
Copy Markdown
Member

/retest-required

@pablintino
Copy link
Copy Markdown
Contributor

Just an ack for the future given that the PR has already the lgtm. The approach is fine /lgtm

@djoshy
Copy link
Copy Markdown
Contributor Author

djoshy commented Jun 2, 2026

/retest-required

@sergiordlr
Copy link
Copy Markdown
Contributor

sergiordlr commented Jun 2, 2026

Verified using IPI on AWS

Upgrade from 4.23 image + fix + techpreview to 4.23 image + fix + techpreview

In order the fix to work, the fix must be present in the initial version and in the target version because the process will be executed by the initla version's controller.

After upgrading we see that only one MC is rendered and it contains the changes in osImageURL, baseOSExtensionsContainerImage and all the other changes not related to those images. With the bug the images and the rest of the chages were rendered separately

We can see the "mcdiff diff ...." command output

spec.baseOSExtensionsContainerImage
  ± value change
    - registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:0e8ee79682a0d4d59a3422012d5f44011ddd051f1e1dfe374aae7c18596c7ce8
    + registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:e75c41af16a9414359a532a74f4fcd1dcb1f437818e2c02e3e08f8342f323572

spec.config.storage.files./etc/crio/crio.conf.d/00-default.contents.source
  ± value change in multiline text (one insert, one deletion)
    - pause_image = "registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:be6345d531eb919bd5f5ab483678bdc0f9eeb7c8c995f7420e3856e089fdd82b"
    + pause_image = "registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:069a8efefd42ae35269d4b3b59535fce6fed32736b2c157e83c949c27d0e8a84"
.....

spec.config.systemd.units.nodeip-configuration.service.contents
  ± value change in multiline text (one insert, one deletion)
    -   registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:604c8a3ad839e01dc5d6fbdaac0e8fcee9bffd7a95ac043a3f4779ad37d29178 \
    +   registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:8e0d853b8d54dec642cb535bfe070342665b060c66820e0a5b093b45f17c0417 \


spec.osImageURL
  ± value change
    - registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:a184e94da847db8d7b211052bc824829ff11f783cec9ba37cb7a9069c79fd251
    + registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:9878d2b83cfc9e397de8f34012509472ba0ff58d62b9d92b8b9541f50a0571bb

And we can check that only one MC was rendered


$ oc get mc
.....
rendered-master-0d63447d4a6d006bcfdd5f5a7fe0fb60   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             151m
rendered-master-3dc29aa281fe53e73490dd3897c932de   c24acffeefe23c52cc12778b13220b101fd3cacd   3.5.0             9m44s  <----- THIS ONE
rendered-master-68216b0005aaa413bda65b6115ead9ee   c942cab951619a7388c65ff92524a0c5cb10bcb8   3.5.0             144m
rendered-master-74176168378dacc23a79cb796b43280d   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h48m
rendered-master-d59c20035d6e231b9b14435a601f22bb   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h21m
rendered-worker-4a41e55e952dc2cff967ffcd3f93ec42   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h48m
rendered-worker-590c1d31cd3505fd64e38647396a81ce   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h21m
rendered-worker-88a2206ba7fbddd1ca63ff2c6d0f47de   c24acffeefe23c52cc12778b13220b101fd3cacd   3.5.0             9m44s  <----- THIS ONE
rendered-worker-ac2bc7ae5f1816e12ad3cc48a5f65246   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             151m
rendered-worker-e5c0ab756cd39f30ad88d29f50fe97e9   c942cab951619a7388c65ff92524a0c5cb10bcb8   3.5.0             144m

/verified by @sergiordlr

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 2, 2026
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@sergiordlr: This PR has been marked as verified by @sergiordlr.

Details

In response to this:

Verified using IPI on AWS

Upgrade from 4.23 image + fix + techpreview to 4.23 image + fix + techpreview

In order the fix to work, the fix must to be present in the initial version and in the target version because the process will be executed by the initla version's controller.

After upgrading we see that only one MC is rendered and it contains the changes in osImageURL, baseOSExtensionsContainerImage and all the other changes not related to those images. With the bug the images and the rest of the chages were rendered separately

We can see the "mcdiff diff ...." command output

spec.baseOSExtensionsContainerImage
 ± value change
   - registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:0e8ee79682a0d4d59a3422012d5f44011ddd051f1e1dfe374aae7c18596c7ce8
   + registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:e75c41af16a9414359a532a74f4fcd1dcb1f437818e2c02e3e08f8342f323572

spec.config.storage.files./etc/crio/crio.conf.d/00-default.contents.source
 ± value change in multiline text (one insert, one deletion)
   - pause_image = "registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:be6345d531eb919bd5f5ab483678bdc0f9eeb7c8c995f7420e3856e089fdd82b"
   + pause_image = "registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:069a8efefd42ae35269d4b3b59535fce6fed32736b2c157e83c949c27d0e8a84"
.....

spec.config.systemd.units.nodeip-configuration.service.contents
 ± value change in multiline text (one insert, one deletion)
   -   registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:604c8a3ad839e01dc5d6fbdaac0e8fcee9bffd7a95ac043a3f4779ad37d29178 \
   +   registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:8e0d853b8d54dec642cb535bfe070342665b060c66820e0a5b093b45f17c0417 \


spec.osImageURL
 ± value change
   - registry.build10.ci.openshift.org/ci-ln-23gqil2/stable@sha256:a184e94da847db8d7b211052bc824829ff11f783cec9ba37cb7a9069c79fd251
   + registry.build10.ci.openshift.org/ci-ln-h1bjlxb/stable@sha256:9878d2b83cfc9e397de8f34012509472ba0ff58d62b9d92b8b9541f50a0571bb

And we can check that only one MC was rendered


$ oc get mc
.....
rendered-master-0d63447d4a6d006bcfdd5f5a7fe0fb60   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             151m
rendered-master-3dc29aa281fe53e73490dd3897c932de   c24acffeefe23c52cc12778b13220b101fd3cacd   3.5.0             9m44s  <----- THIS ONE
rendered-master-68216b0005aaa413bda65b6115ead9ee   c942cab951619a7388c65ff92524a0c5cb10bcb8   3.5.0             144m
rendered-master-74176168378dacc23a79cb796b43280d   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h48m
rendered-master-d59c20035d6e231b9b14435a601f22bb   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h21m
rendered-worker-4a41e55e952dc2cff967ffcd3f93ec42   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h48m
rendered-worker-590c1d31cd3505fd64e38647396a81ce   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             3h21m
rendered-worker-88a2206ba7fbddd1ca63ff2c6d0f47de   c24acffeefe23c52cc12778b13220b101fd3cacd   3.5.0             9m44s  <----- THIS ONE
rendered-worker-ac2bc7ae5f1816e12ad3cc48a5f65246   d72b715f8f9e0fad5d27a45420ea074ea2628207   3.5.0             151m
rendered-worker-e5c0ab756cd39f30ad88d29f50fe97e9   c942cab951619a7388c65ff92524a0c5cb10bcb8   3.5.0             144m

/verified by @sergiordlr

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.

@djoshy
Copy link
Copy Markdown
Contributor Author

djoshy commented Jun 2, 2026

/override ci/prow/e2e-gcp-op-ocl-part1
/override ci/prow/e2e-gcp-op-ocl-part2

tests are currently broken(being fixed in openshift/release#79990)

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jun 2, 2026

@djoshy: Overrode contexts on behalf of djoshy: ci/prow/e2e-gcp-op-ocl-part1, ci/prow/e2e-gcp-op-ocl-part2

Details

In response to this:

/override ci/prow/e2e-gcp-op-ocl-part1
/override ci/prow/e2e-gcp-op-ocl-part2

tests are currently broken(being fixed in openshift/release#79990)

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.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Jun 2, 2026

@djoshy: all tests passed!

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.

@openshift-merge-bot openshift-merge-bot Bot merged commit 3ffecb5 into openshift:main Jun 2, 2026
17 checks passed
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@djoshy: Jira Issue Verification Checks: Jira Issue OCPBUGS-83619
✔️ This pull request was pre-merge verified.
✔️ All associated pull requests have merged.
✔️ All associated, merged pull requests were pre-merge verified.

Jira Issue OCPBUGS-83619 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓

Details

In response to this:

- What I did
Added a version guard so the render controller will ignore the OSImageStream generated by a new operator so a stale controller won't roll the MachineConfigs. Also added a couple units to verify this behavior.

- How to verify it
See bug for reproduction steps.

Summary by CodeRabbit

  • Bug Fixes
  • Added version compatibility validation for operating system image streams to prevent using images from incompatible operator versions.

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.

@openshift-cherrypick-robot
Copy link
Copy Markdown

@djoshy: new pull request created: #6123

Details

In response to this:

/cherry-pick release-4.22

/jira refresh

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.

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/severity-moderate Referenced Jira bug's severity is moderate for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants