MCO-2294: promote OSStreams to GA in self-managed clusters#2849
MCO-2294: promote OSStreams to GA in self-managed clusters#2849cheesesashimi wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Hello @cheesesashimi! Some important instructions when contributing to openshift/api: |
openshift-ci
Bot
added
the
size/S
📝 WalkthroughWalkthroughThis PR enables the OSStreams feature for SelfManagedHA (Default and OKD) and for Hypershift preview channels by splitting and updating the feature-gate logic in features/features.go, updates the features.md matrix to mark SelfManagedHA/OKD as Enabled, moves OSStreams from disabled to enabled in the SelfManagedHA Default and OKD featureGate manifests, removes include.release.openshift.io/self-managed-high-availability annotations from several Hypershift CRD variants, and adds SelfManagedHA CRDs for MachineConfigPool and OSImageStream with full schemas. Suggested reviewers
🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented 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. Comment |
|
[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 |
|
/test ci/prow/verify-feature-promotion |
|
/test verify-feature-promotion |
cheesesashimi
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@cheesesashimi: This pull request references MCO-2294 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. |
|
/jira-refresh |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
cheesesashimi
force-pushed
the
zzlotnik/promote-osstream-to-ga
branch
from
e00a624 to
2cbf3c8
Compare
openshift-ci
Bot
added
size/XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-SelfManagedHA.crd.yaml`:
- Around line 350-360: The pinnedImageSets[].name schema and pattern allow
uppercase letters but must enforce RFC1123 lowercase; update the description to
state the name MUST be lowercase RFC1123 (a–z, 0–9, -) and replace all
[a-zA-Z0-9] occurrences in the pattern with [a-z0-9] so each segment and the
overall pattern only permit lowercase alphanumerics and hyphens (keep the
existing segment length rules and anchors intact); ensure the schema's pattern
string for name is the only change so validation rejects uppercase names that
cannot match Kubernetes object names.
In
`@payload-manifests/crds/0000_80_machine-config_01_osimagestreams-SelfManagedHA.crd.yaml`:
- Around line 20-29: The CRD currently exposes OSImageStream only as v1alpha1
with "Compatibility level 4", which hides the GA surface (notably
spec.defaultStream); promote this resource to a stable API by switching the CRD
version from v1alpha1 to a GA/stable version (e.g., v1) and update the
compatibility metadata (remove/replace "Compatibility level 4" with the
appropriate stable level) so spec.defaultStream is published with a stable
contract; ensure the OpenAPI schema under openAPIV3Schema for OSImageStream
remains intact and adjust any apiVersion/version fields and validation fields to
match the promoted CRD (target symbols: OSImageStream, spec.defaultStream,
v1alpha1).
- Around line 126-158: The repository/name part of the OCI pull-spec regex
allows an empty repository (e.g. "quay.io/@sha256:..."); update the two
x-kubernetes-validations rules (the rule for the OCI Image name under both the
earlier field and osImage) to require at least one character for the final
repository segment by changing the terminal quantifier from '*' to '+' (i.e.
ensure the final class [a-zA-Z0-9-_.] uses + so repository name is non-empty),
leaving the rest of the host[:port][/namespace]/name pattern intact and keeping
both validation locations in sync.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7e2d5896-aa38-472f-a2e3-aecca24e341c
⛔ Files ignored due to path filters (8)
machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-Hypershift-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-Hypershift-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-Hypershift-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-Hypershift-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-Hypershift-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-SelfManagedHA.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_osimagestreams-Hypershift.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_osimagestreams-SelfManagedHA.crd.yamlis excluded by!**/zz_generated.crd-manifests/*
📒 Files selected for processing (12)
features.mdfeatures/features.gopayload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-Default.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-OKD.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_machineconfigpools-SelfManagedHA.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_osimagestreams-Hypershift.crd.yamlpayload-manifests/crds/0000_80_machine-config_01_osimagestreams-SelfManagedHA.crd.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
💤 Files with no reviewable changes (6)
- payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-OKD.crd.yaml
- payload-manifests/crds/0000_80_machine-config_01_osimagestreams-Hypershift.crd.yaml
- payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-CustomNoUpgrade.crd.yaml
- payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-Hypershift-Default.crd.yaml
| name: | ||
| description: |- | ||
| name is a reference to the name of a PinnedImageSet. Must adhere to | ||
| RFC-1123 (https://tools.ietf.org/html/rfc1123). | ||
| Made up of one of more period-separated (.) segments, where each segment | ||
| consists of alphanumeric characters and hyphens (-), must begin and end | ||
| with an alphanumeric character, and is at most 63 characters in length. | ||
| The total length of the name must not exceed 253 characters. | ||
| maxLength: 253 | ||
| minLength: 1 | ||
| pattern: ^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])(\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9]))*$ |
There was a problem hiding this comment.
Make pinnedImageSets[].name actually enforce RFC1123 lowercase.
Line 360 currently allows uppercase characters even though the field is documented as RFC1123. That lets users submit PinnedImageSet refs that can never match a valid Kubernetes object name.
Suggested fix
- pattern: ^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])(\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9]))*$
+ pattern: ^([a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9])(\.([a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9]))*$📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: | |
| description: |- | |
| name is a reference to the name of a PinnedImageSet. Must adhere to | |
| RFC-1123 (https://tools.ietf.org/html/rfc1123). | |
| Made up of one of more period-separated (.) segments, where each segment | |
| consists of alphanumeric characters and hyphens (-), must begin and end | |
| with an alphanumeric character, and is at most 63 characters in length. | |
| The total length of the name must not exceed 253 characters. | |
| maxLength: 253 | |
| minLength: 1 | |
| pattern: ^([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])(\.([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]{0,61}[a-zA-Z0-9]))*$ | |
| name: | |
| description: |- | |
| name is a reference to the name of a PinnedImageSet. Must adhere to | |
| RFC-1123 (https://tools.ietf.org/html/rfc1123). | |
| Made up of one of more period-separated (.) segments, where each segment | |
| consists of alphanumeric characters and hyphens (-), must begin and end | |
| with an alphanumeric character, and is at most 63 characters in length. | |
| The total length of the name must not exceed 253 characters. | |
| maxLength: 253 | |
| minLength: 1 | |
| pattern: ^([a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9])(\.([a-z0-9]|[a-z0-9][a-z0-9\-]{0,61}[a-z0-9]))*$ |
🤖 Prompt for 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.
In
`@payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-SelfManagedHA.crd.yaml`
around lines 350 - 360, The pinnedImageSets[].name schema and pattern allow
uppercase letters but must enforce RFC1123 lowercase; update the description to
state the name MUST be lowercase RFC1123 (a–z, 0–9, -) and replace all
[a-zA-Z0-9] occurrences in the pattern with [a-z0-9] so each segment and the
overall pattern only permit lowercase alphanumerics and hyphens (keep the
existing segment length rules and anchors intact); ensure the schema's pattern
string for name is the only change so validation rejects uppercase names that
cannot match Kubernetes object names.
| - name: v1alpha1 | ||
| schema: | ||
| openAPIV3Schema: | ||
| description: |- | ||
| OSImageStream describes a set of streams and associated images available | ||
| for the MachineConfigPools to be used as base OS images. | ||
|
|
||
| The resource is a singleton named "cluster". | ||
|
|
||
| Compatibility level 4: No compatibility is provided, the API can change at any point for any reason. These capabilities should not be used by applications needing long term support. |
There was a problem hiding this comment.
Don’t ship the GA path behind a v1alpha1/compat-level-4 CRD.
This PR moves OSStreams into Default/OKD for self-managed clusters, but the only published OSImageStream API here is still v1alpha1 with “Compatibility level 4”. That leaves the user-editable spec.defaultStream surface without a stable contract while the feature is effectively GA.
🤖 Prompt for 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.
In
`@payload-manifests/crds/0000_80_machine-config_01_osimagestreams-SelfManagedHA.crd.yaml`
around lines 20 - 29, The CRD currently exposes OSImageStream only as v1alpha1
with "Compatibility level 4", which hides the GA surface (notably
spec.defaultStream); promote this resource to a stable API by switching the CRD
version from v1alpha1 to a GA/stable version (e.g., v1) and update the
compatibility metadata (remove/replace "Compatibility level 4" with the
appropriate stable level) so spec.defaultStream is published with a stable
contract; ensure the OpenAPI schema under openAPIV3Schema for OSImageStream
remains intact and adjust any apiVersion/version fields and validation fields to
match the promoted CRD (target symbols: OSImageStream, spec.defaultStream,
v1alpha1).
| The format of the image pull spec is: host[:port][/namespace]/name@sha256:<digest>, | ||
| where the digest must be 64 characters long, and consist only of lowercase hexadecimal characters, a-f and 0-9. | ||
| The length of the whole spec must be between 1 to 447 characters. | ||
| maxLength: 447 | ||
| minLength: 1 | ||
| type: string | ||
| x-kubernetes-validations: | ||
| - message: the OCI Image reference must end with a valid '@sha256:<digest>' | ||
| suffix, where '<digest>' is 64 characters long | ||
| rule: (self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')) | ||
| - message: the OCI Image name should follow the host[:port][/namespace]/name | ||
| format, resembling a valid URL without the scheme | ||
| rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$')) | ||
| osImage: | ||
| description: |- | ||
| osImage is a required OS Image referenced by digest. | ||
|
|
||
| osImage contains the immutable, fundamental operating system components, including the kernel | ||
| and base utilities, that define the core environment for the node's host operating system. | ||
|
|
||
| The format of the image pull spec is: host[:port][/namespace]/name@sha256:<digest>, | ||
| where the digest must be 64 characters long, and consist only of lowercase hexadecimal characters, a-f and 0-9. | ||
| The length of the whole spec must be between 1 to 447 characters. | ||
| maxLength: 447 | ||
| minLength: 1 | ||
| type: string | ||
| x-kubernetes-validations: | ||
| - message: the OCI Image reference must end with a valid '@sha256:<digest>' | ||
| suffix, where '<digest>' is 64 characters long | ||
| rule: (self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')) | ||
| - message: the OCI Image name should follow the host[:port][/namespace]/name | ||
| format, resembling a valid URL without the scheme | ||
| rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$')) |
There was a problem hiding this comment.
Tighten the pull-spec regex so empty repository names stop validating.
The current rules accept invalid refs like quay.io/@sha256:... because the namespace part can be empty and the final repository component is *. That weakens validation for the image refs this CRD publishes.
Suggested fix
- rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$'))
+ rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{1,61}/)?[a-zA-Z0-9-_.]+$'))
...
- rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$'))
+ rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{1,61}/)?[a-zA-Z0-9-_.]+$'))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The format of the image pull spec is: host[:port][/namespace]/name@sha256:<digest>, | |
| where the digest must be 64 characters long, and consist only of lowercase hexadecimal characters, a-f and 0-9. | |
| The length of the whole spec must be between 1 to 447 characters. | |
| maxLength: 447 | |
| minLength: 1 | |
| type: string | |
| x-kubernetes-validations: | |
| - message: the OCI Image reference must end with a valid '@sha256:<digest>' | |
| suffix, where '<digest>' is 64 characters long | |
| rule: (self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')) | |
| - message: the OCI Image name should follow the host[:port][/namespace]/name | |
| format, resembling a valid URL without the scheme | |
| rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$')) | |
| osImage: | |
| description: |- | |
| osImage is a required OS Image referenced by digest. | |
| osImage contains the immutable, fundamental operating system components, including the kernel | |
| and base utilities, that define the core environment for the node's host operating system. | |
| The format of the image pull spec is: host[:port][/namespace]/name@sha256:<digest>, | |
| where the digest must be 64 characters long, and consist only of lowercase hexadecimal characters, a-f and 0-9. | |
| The length of the whole spec must be between 1 to 447 characters. | |
| maxLength: 447 | |
| minLength: 1 | |
| type: string | |
| x-kubernetes-validations: | |
| - message: the OCI Image reference must end with a valid '@sha256:<digest>' | |
| suffix, where '<digest>' is 64 characters long | |
| rule: (self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')) | |
| - message: the OCI Image name should follow the host[:port][/namespace]/name | |
| format, resembling a valid URL without the scheme | |
| rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{0,61}/)?[a-zA-Z0-9-_.]*?$')) | |
| The format of the image pull spec is: host[:port][/namespace]/name@sha256:<digest>, | |
| where the digest must be 64 characters long, and consist only of lowercase hexadecimal characters, a-f and 0-9. | |
| The length of the whole spec must be between 1 to 447 characters. | |
| maxLength: 447 | |
| minLength: 1 | |
| type: string | |
| x-kubernetes-validations: | |
| - message: the OCI Image reference must end with a valid '`@sha256`:<digest>' | |
| suffix, where '<digest>' is 64 characters long | |
| rule: (self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')) | |
| - message: the OCI Image name should follow the host[:port][/namespace]/name | |
| format, resembling a valid URL without the scheme | |
| rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{1,61}/)?[a-zA-Z0-9-_.]+$')) | |
| osImage: | |
| description: |- | |
| osImage is a required OS Image referenced by digest. | |
| osImage contains the immutable, fundamental operating system components, including the kernel | |
| and base utilities, that define the core environment for the node's host operating system. | |
| The format of the image pull spec is: host[:port][/namespace]/name@sha256:<digest>, | |
| where the digest must be 64 characters long, and consist only of lowercase hexadecimal characters, a-f and 0-9. | |
| The length of the whole spec must be between 1 to 447 characters. | |
| maxLength: 447 | |
| minLength: 1 | |
| type: string | |
| x-kubernetes-validations: | |
| - message: the OCI Image reference must end with a valid '`@sha256`:<digest>' | |
| suffix, where '<digest>' is 64 characters long | |
| rule: (self.split('@').size() == 2 && self.split('@')[1].matches('^sha256:[a-f0-9]{64}$')) | |
| - message: the OCI Image name should follow the host[:port][/namespace]/name | |
| format, resembling a valid URL without the scheme | |
| rule: (self.split('@')[0].matches('^([a-zA-Z0-9-]+\\.)+[a-zA-Z0-9-]+(:[0-9]{2,5})?/([a-zA-Z0-9-_]{1,61}/)?[a-zA-Z0-9-_.]+$')) |
🤖 Prompt for 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.
In
`@payload-manifests/crds/0000_80_machine-config_01_osimagestreams-SelfManagedHA.crd.yaml`
around lines 126 - 158, The repository/name part of the OCI pull-spec regex
allows an empty repository (e.g. "quay.io/@sha256:..."); update the two
x-kubernetes-validations rules (the rule for the OCI Image name under both the
earlier field and osImage) to require at least one character for the final
repository segment by changing the terminal quantifier from '*' to '+' (i.e.
ensure the final class [a-zA-Z0-9-_.] uses + so repository name is non-empty),
leaving the rest of the host[:port][/namespace]/name pattern intact and keeping
both validation locations in sync.
|
/test verify-feature-promotion |
|
@cheesesashimi: 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. |
3 participants
This promotes the OSStream feature to GA for self-managed clusters. The intent behind this is that we will be able to begin testing OCP on RHEL10 since OSStreams helps make that possible. This PR purposely avoids default enablement in Hypershift as there are some unknowns and must-have conversations around that.