MCO-2142: Avoid OSImageStream defaultStream unset#2764
MCO-2142: Avoid OSImageStream defaultStream unset#2764pablintino wants to merge 1 commit intoopenshift:masterfrom
Conversation
This change prevents users from resetting the value of the OSImageStream.spec.defaultStream field once it's set. The main idea motivation behind this limitation is the design choice and agreement that makes all clusters have a default at install-time. Given that the field is not required (and it's not safe to make it required as the field already exist) this validation is the middle-ground we can provide without making the field required. Signed-off-by: Pablo Rodriguez Nava <git@amail.pablintino.eu>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@pablintino: This pull request references MCO-2142 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 "4.22.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. |
|
Hello @pablintino! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/M
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoPrevent removal of OSImageStream defaultStream once set
WalkthroughsDescription• Add validation rule preventing removal of spec.defaultStream once set • Ensures clusters maintain default stream at install-time • Update documentation and generated manifests with validation details • Add test cases for removal prevention and edge cases Diagramflowchart LR
A["OSImageStreamSpec Type"] -->|Add XValidation Rule| B["Validation Logic"]
B -->|Rule: !has oldSelf.defaultStream OR has self.defaultStream| C["Prevent Removal"]
C -->|Update Documentation| D["Comments & Swagger Docs"]
D -->|Generate CRD Manifests| E["CRD YAML Files"]
E -->|Add Test Cases| F["Test Suite"]
File Changes1. machineconfiguration/v1alpha1/types_osimagestream.go
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewⓘ The new review experience is currently in Beta. Learn more |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis change introduces validation to prevent removal of the spec.defaultStream field in OSImageStream resources once it has been set. The validation is implemented through a CEL-based XValidation rule applied to the OSImageStreamSpec type, which is subsequently reflected in the generated CRD manifest. The rule enforces that if defaultStream existed in the previous state, it must continue to exist in the updated state. Corresponding test cases are added to verify that removal attempts are rejected while updates without defaultStream remain permitted when the field was never set. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.11.3)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
@pablintino: all tests passed! 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. |
| kind: OSImageStream | ||
| metadata: | ||
| name: cluster | ||
| spec: {} |
There was a problem hiding this comment.
There isn't a sibling field apart from defaultStream right?
3 participants
This change prevents users from resetting the value of the OSImageStream.spec.defaultStream field once it's set. The main idea motivation behind this limitation is the design choice and agreement that makes all clusters have a default at install-time. Given that the field is not required (and it's not safe to make it required as the field already exist) this validation is the middle-ground we can provide without making the field required.