Skip to content

Conversation

@jatinsu
Copy link
Contributor

@jatinsu jatinsu commented Aug 12, 2025

This PR was largely bult on the work done by @Prashanth684

  • This first pass only includes all featuregates which are default
  • For OKD clusters, this would be the "default" featureset
  • Allows for upgrades

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 12, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 12, 2025

@jatinsu: This pull request references OKD-259 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 epic to target the "4.20.0" version, but no target version was set.

Details

In response to this:

This PR, built largely on the work done by @Prashanth684, intends to create a OKD featureset that will be enabled by default on all OKD clusters. This will offload testing for TechPreviewNoUpgrade features to OKD and allow customers to try out new features on clusters which then can be upgraded unlike the other featuresets.

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 added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 12, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 12, 2025

Hello @jatinsu! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

@openshift-ci openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 12, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 12, 2025

@jatinsu: This pull request references OKD-259 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 epic to target the "4.20.0" version, but no target version was set.

Details

In response to this:

This PR was largely bult on the work done by @Prashanth684

  • This first pass only includes all featuregates which are default
  • For OKD clusters, this would be the "default" featureset
  • Allows for upgrades

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-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 13, 2025
@jatinsu jatinsu force-pushed the OKD-featuregates branch 2 times, most recently from d58e1ad to fb290e4 Compare October 14, 2025 20:55
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2025
@jatinsu
Copy link
Contributor Author

jatinsu commented Oct 17, 2025

/test e2e-aws-ovn-techpreview

@jatinsu
Copy link
Contributor Author

jatinsu commented Oct 17, 2025

/test okd-scos-e2e-aws-ovn

@Prashanth684
Copy link
Contributor

/test e2e-aws-ovn-techpreview

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 20, 2025
@jatinsu
Copy link
Contributor Author

jatinsu commented Oct 21, 2025

/test ci/prow/images ci/prow/minor-e2e-upgrade-minor

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 21, 2025

@jatinsu: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test e2e-aws-ovn
/test e2e-aws-ovn-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change
/test images
/test integration
/test lint
/test minor-e2e-upgrade-minor
/test minor-images
/test okd-scos-images
/test unit
/test verify
/test verify-client-go
/test verify-crd-schema
/test verify-crdify
/test verify-deps
/test verify-feature-promotion

The following commands are available to trigger optional jobs:

/test okd-scos-e2e-aws-ovn

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-api-master-build
pull-ci-openshift-api-master-images
pull-ci-openshift-api-master-integration
pull-ci-openshift-api-master-lint
pull-ci-openshift-api-master-minor-e2e-upgrade-minor
pull-ci-openshift-api-master-minor-images
pull-ci-openshift-api-master-okd-scos-e2e-aws-ovn
pull-ci-openshift-api-master-okd-scos-images
pull-ci-openshift-api-master-unit
pull-ci-openshift-api-master-verify
pull-ci-openshift-api-master-verify-client-go
pull-ci-openshift-api-master-verify-crd-schema
pull-ci-openshift-api-master-verify-crdify
pull-ci-openshift-api-master-verify-deps
pull-ci-openshift-api-master-verify-feature-promotion
Details

In response to this:

/test ci/prow/images ci/prow/minor-e2e-upgrade-minor

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.

@jatinsu
Copy link
Contributor Author

jatinsu commented Oct 21, 2025

/test images minor-e2e-upgrade-minor

@jatinsu jatinsu changed the title WIP: OKD-259: Support an "OKD" featureset to be enabled by default on OKD clusters OKD-259: Support an "OKD" featureset to be enabled by default on OKD clusters Oct 21, 2025
@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 Oct 21, 2025
Copy link
Contributor

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Please add a test somewhere that ensures that a feature is not enabled in Default without being enabled in OKD as well

Please also link the work that will prevent the OKD featureset being enabled in OCP clusters

// +kubebuilder:validation:XValidation:rule="oldSelf == 'CustomNoUpgrade' ? self == 'CustomNoUpgrade' : true",message="CustomNoUpgrade may not be changed"
// +kubebuilder:validation:XValidation:rule="oldSelf == 'TechPreviewNoUpgrade' ? self == 'TechPreviewNoUpgrade' : true",message="TechPreviewNoUpgrade may not be changed"
// +kubebuilder:validation:XValidation:rule="oldSelf == 'DevPreviewNoUpgrade' ? self == 'DevPreviewNoUpgrade' : true",message="DevPreviewNoUpgrade may not be changed"
// +kubebuilder:validation:XValidation:rule="oldSelf == 'OKD' ? self == 'OKD' : true",message="OKD may not be changed"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm expecting the the OKD featureset is a subset of TechPreviewNoUpgrade which is also a subset of DevPreviewNoUpgrade. Have you considered allowing a transition from OKD to either TP or DP?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please add a test somewhere that ensures that a feature is not enabled in Default without being enabled in OKD as well

Sure thing! I'll get on that

Please also link the work that will prevent the OKD featureset being enabled in OCP clusters

Here's what we added for the validation: openshift/kubernetes@87b2e70

Do you want this featureset to be opt-in? Or on by default in all OKD clusters?

It won't be enabled by default, the feature will be opt in through the installer

I'm expecting the the OKD featureset is a subset of TechPreviewNoUpgrade which is also a subset of DevPreviewNoUpgrade. Have you considered allowing a transition from OKD to either TP or DP?

I haven't, but @Prashanth684 would this be something that we would be interested in?

Copy link
Contributor

Choose a reason for hiding this comment

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

It won't be enabled by default, the feature will be opt in through the installer

This contradicts something @Prashanth684 mentioned on another thread https://github.com/openshift/api/pull/2451/files#r2461166939

I haven't, but @Prashanth684 would this be something that we would be interested in?

Prasanth also suggested the same, so lets make this change

Copy link
Contributor

Choose a reason for hiding this comment

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

This contradicts something @Prashanth684 mentioned on another thread https://github.com/openshift/api/pull/2451/files#r2461166939

The more i think about this - i feel CVO should be the one dictating what featureset gets installed for OKD, not the installer. CVO should also migrate OKD clusters with default featureset to OKD featureset. I tried an implementation with some CVO changes on installs and upgrades and it worked fine. https://gist.github.com/Prashanth684/c650c14b8c2b02d820e67fe75153c669 captures the major changes needed to have OKD featureset by default on install and upgrades.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do folks need a way to opt out here? Are we certain forcing every OKD cluster onto this OKD featureset is the right thing to do?

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (9)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_nodes-OKD.crd.yaml (1)

56-58: Fix typo and grammatical error in description (duplicate of prior feedback).

The workerLatencyProfile description contains a typo and grammatical issue that was flagged in a previous review and should be corrected.

Apply this diff to fix the typo and improve clarity:

              workerLatencyProfile:
                description: |-
-                  workerLatencyProfile determins the how fast the kubelet is updating
-                  the status and corresponding reaction of the cluster
+                  workerLatencyProfile determines how fast the kubelet is updating
+                  the status and corresponding reaction of the cluster
operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml (2)

104-111: Missing controlplanemachinesets in enum (spec section).

The resource field description documents both machinesets and controlplanemachinesets as valid values, but the enum only lists machinesets. This causes the CRD validation to reject legitimate controlplanemachinesets configurations. Add the missing enum value and ensure the status section (lines 742–744) is kept synchronized.

Apply this diff:

                        resource:
                          description: |-
                            resource is the machine management resource's type.
                            Valid values are machinesets and controlplanemachinesets.
                            machinesets means that the machine manager will only register resources of the kind MachineSet.
                            controlplanemachinesets means that the machine manager will only register resources of the kind ControlPlaneMachineSet.
                          enum:
                          - machinesets
+                         - controlplanemachinesets
                          type: string

737-744: Missing controlplanemachinesets in enum (status section).

Mirror the same fix from the spec section: the status copy of the resource field enum also lists only machinesets but should include controlplanemachinesets. Keep both enums synchronized.

Apply this diff:

                        resource:
                          description: |-
                            resource is the machine management resource's type.
                            Valid values are machinesets and controlplanemachinesets.
                            machinesets means that the machine manager will only register resources of the kind MachineSet.
                            controlplanemachinesets means that the machine manager will only register resources of the kind ControlPlaneMachineSet.
                          enum:
                          - machinesets
+                         - controlplanemachinesets
                          type: string
payload-manifests/crds/0000_10_config-operator_01_nodes-OKD.crd.yaml (1)

55-58: Fix typo in workerLatencyProfile description.

Line 57 contains a typo and grammar error: "determins the how fast" should be "determines how fast".

              workerLatencyProfile:
                description: |-
-                  workerLatencyProfile determins the how fast the kubelet is updating
+                  workerLatencyProfile determines how fast the kubelet is updating
                  the status and corresponding reaction of the cluster
payload-manifests/crds/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (1)

950-954: Critical: Scale subresource references a non-existent status field.

Line 951 specifies labelSelectorPath: .status.labelSelector, but the status schema (lines 839–945) does not define a labelSelector field. This will cause the scale subresource to fail at runtime.

Add the missing field to status:

          status:
            description: ControlPlaneMachineSetStatus represents the status of the
              ControlPlaneMachineSet CRD.
            properties:
+              labelSelector:
+                description: label selector for Machines that match this ControlPlaneMachineSet.
+                type: string
              conditions:

Alternatively, remove the labelSelectorPath from the scale subresource if it is not needed.

payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml (1)

384-390: Fix inconsistent enum value casing for status condition.

Line 388 has Unknown without quotes, while lines 386–387 have quoted values "True" and "False". This inconsistency can cause YAML type parsing issues. Ensure consistency:

- enum:
-   - "True"
-   - "False"
-   - Unknown
+ enum:
+   - "True"
+   - "False"
+   - "Unknown"
payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml (1)

481-485: Minor: Inconsistent enum value casing for status condition.

Lines 482–484 have mixed quoting: "True" and "False" are quoted, but Unknown is unquoted. Ensure consistency:

- enum:
-   - "True"
-   - "False"
-   - Unknown
+ enum:
+   - "True"
+   - "False"
+   - "Unknown"
payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml (1)

384-390: Fix inconsistent enum value casing for status condition.

Line 389 has Unknown without quotes, while lines 387–388 have quoted values "True" and "False". Ensure consistent quoting:

- enum:
-   - "True"
-   - "False"
-   - Unknown
+ enum:
+   - "True"
+   - "False"
+   - "Unknown"
example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-OKD.crd.yaml (1)

5-5: Replace placeholder URL with actual PR reference or clear example placeholder.

The api-approved annotation contains a placeholder URL (pull/xxx). Even for example CRDs, use either a real PR reference or an unambiguous placeholder like https://github.com/openshift/api/pull/EXAMPLE.

-    api-approved.openshift.io: https://github.com/openshift/api/pull/xxx
+    api-approved.openshift.io: https://github.com/openshift/api/pull/EXAMPLE
🧹 Nitpick comments (1)
features/okd_featureset_parity_test.go (1)

16-52: Test implementation is correct and comprehensive.

The test properly verifies the containment relationship between Default and OKD feature sets. The error message provides actionable context with the cluster profile and sorted list of missing gates.

Minor observation: missingInOKD.List() already returns a sorted slice, so the sort.Strings(missingList) on line 44 is redundant but harmless.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between ec6894d and 35fdece.

📒 Files selected for processing (50)
  • config/v1/tests/featuregates.config.openshift.io/AAA_ungated.yaml (2 hunks)
  • config/v1/types_feature.go (2 hunks)
  • config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_featuregates.crd.yaml (2 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_nodes-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.featuregated-crd-manifests/featuregates.config.openshift.io/AAA_ungated.yaml (2 hunks)
  • example/v1/types_stable.go (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-Default.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-DevPreviewNoUpgrade.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-OKD.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-TechPreviewNoUpgrade.crd.yaml (1 hunks)
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/AAA_ungated.yaml (1 hunks)
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example+Example2.yaml (1 hunks)
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example.yaml (1 hunks)
  • features.md (1 hunks)
  • features/features.go (22 hunks)
  • features/okd_featureset_parity_test.go (1 hunks)
  • machine/v1/zz_generated.crd-manifests/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (1 hunks)
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-OKD.crd.yaml (1 hunks)
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machinesets-OKD.crd.yaml (1 hunks)
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml (1 hunks)
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml (1 hunks)
  • openapi/generated_openapi/zz_generated.openapi.go (1 hunks)
  • openapi/openapi.json (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_apiservers-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-CustomNoUpgrade.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-Default.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-DevPreviewNoUpgrade.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-TechPreviewNoUpgrade.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_featuregates.crd.yaml (2 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-CustomNoUpgrade.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-Default.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-DevPreviewNoUpgrade.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_nodes-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_12_etcd_01_etcds-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
  • config/v1/tests/featuregates.config.openshift.io/AAA_ungated.yaml
  • payload-manifests/crds/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example+Example2.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-DevPreviewNoUpgrade.crd.yaml
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml
  • openapi/generated_openapi/zz_generated.openapi.go
  • config/v1/zz_generated.featuregated-crd-manifests/featuregates.config.openshift.io/AAA_ungated.yaml
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machinesets-OKD.crd.yaml
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/AAA_ungated.yaml
  • payload-manifests/crds/0000_12_etcd_01_etcds-OKD.crd.yaml
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_featuregates.crd.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • config/v1/types_feature.go
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example.yaml
  • features.md
  • example/v1/types_stable.go
  • features/features.go
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-Default.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml
  • features/okd_featureset_parity_test.go
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-CustomNoUpgrade.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml
  • payload-manifests/crds/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-DevPreviewNoUpgrade.crd.yaml
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-OKD.crd.yaml
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-Default.crd.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-CustomNoUpgrade.crd.yaml
  • machine/v1/zz_generated.crd-manifests/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_nodes-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-DevPreviewNoUpgrade.crd.yaml
  • openapi/openapi.json
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_nodes-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-Default.crd.yaml
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-TechPreviewNoUpgrade.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-TechPreviewNoUpgrade.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_apiservers-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_featuregates.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml
  • payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml
  • config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-OKD.crd.yaml
🧬 Code graph analysis (1)
features/okd_featureset_parity_test.go (2)
features/features.go (1)
  • AllFeatureSets (21-34)
config/v1/types_feature.go (3)
  • Default (41-41)
  • OKD (58-58)
  • FeatureGateAttributes (136-144)
🔇 Additional comments (39)
example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml (1)

333-333: Good consistency improvement on array list-type annotation.

Adding x-kubernetes-list-type: atomic to excludeSubnets aligns it with the sibling subnets field (line 351), ensuring both array fields in subnetsWithExclusions are treated consistently. This is a safe schema metadata change with no impact on validation or functionality.

config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-OKD.crd.yaml (4)

200-206: CEL validation rules are well-formed.

The desiredUpdate field constraints (lines 200-206) properly enforce the documented mutual exclusivity and interdependency rules via x-kubernetes-validations. The rules correctly prevent conflicting field combinations.


246-251: List type annotations are consistent and complete.

All array fields with map-style semantics properly declare x-kubernetes-list-type: map with corresponding x-kubernetes-list-map-keys (e.g., overrides, conditions in conditionalUpdates, conditionalUpdates.risks). Array fields with atomic or set semantics properly declare their list types. This ensures correct Kubernetes merge behavior and API consistency.

Also applies to: 456-458, 589-591, 639-641


780-790: Cross-field validation rule correctly enforces capability dependencies.

The marketplace-to-OperatorLifecycleManager capability constraint is properly expressed via CEL rule (lines 780-790) and covers both explicit enablement and implicit status tracking, preventing invalid capability configurations.


1-10: OKD ClusterVersion CRD structure is correct and consistent with the Default variant.

Verification confirms this is a properly generated CRD manifest for the OKD feature-set. The file differs from the Default variant only in the release.openshift.io/feature-set annotation value (line 8), which is the expected behavior. All schema definitions, validation rules, required fields, and list type annotations are identical across variants.

operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1)

1-335: CRD manifest is correctly structured and properly annotated for OKD featureset.

The CustomResourceDefinition for KubeAPIServer has a valid OpenAPI v3 schema with comprehensive field definitions, proper list-type annotations on all array fields (conditions, generations, nodeStatuses, lastFailedRevisionErrors, serviceAccountIssuers), and appropriate validation rules. The release.openshift.io/feature-set: OKD annotation correctly identifies this as OKD-specific, consistent with other featureset variants in the repository.

machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml (3)

4-9: OKD feature-set annotation properly configured.

The CRD metadata includes appropriate annotations for OKD deployment, API approval reference, and merge-by-featuregates configuration. This aligns with the PR objectives to add OKD-specific manifests.


355-367: Well-structured cross-field validations for generation constraints.

The pinnedImageSets cross-field validations properly enforce the ordering requirement (desiredGeneration ≥ currentGeneration and ≥ lastFailedGeneration) and ensure error field presence on failure. These constraints are clear and maintainable.


121-125: The RFC 1123 validation rules use documented Kubernetes CEL functions with correct syntax.

The CEL validation rules !format.dns1123Subdomain().validate(self).hasValue() are properly documented in Kubernetes CEL validation libraries and implement the correct logic: format.dns1123Subdomain().validate(self) returns optional.none when valid (no errors), and .hasValue() checks for the presence of error values. The negation (!) correctly evaluates to true when validation succeeds. This pattern is intentionally implemented across multiple fields and is confirmed in the Go source definitions.

machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml (3)

4-9: OKD feature-set annotation properly configured with multi-environment support.

The CRD metadata includes appropriate annotations for OKD deployment, including support for both ibm-cloud-managed and self-managed-high-availability environments. API approval references a prior PR review (1453) and proper feature-set labeling.


24-62: Comprehensive printer columns provide excellent observability.

The MachineConfigPool printer columns surface critical state information: current config, update/degraded status, and machine counts (total, ready, updated, degraded). This provides operators with immediate visibility into pool health and progression without requiring detailed spec/status inspection.


577-588: Comprehensive cross-field validations enforce consistent machine count invariants.

The poolSynchronizersStatus validations properly enforce logical constraints: totalCount ≥ subcounts (updated, available, unavailable, ready), and availableCount ≥ readyCount. These constraints prevent invalid state representations and support reliable status reporting for PinnedImageSets synchronizers.

payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-Default.crd.yaml (1)

9-9: Feature-set annotation correctly applied.

The release.openshift.io/feature-set: Default annotation properly tags this CRD variant for the Default feature set, enabling the merge-by-featuregates system to apply the correct manifest based on cluster configuration. This is consistent with the PR's introduction of OKD as a new feature-set variant.

example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-Default.crd.yaml (1)

328-328: Atomic list type appropriately applied for consistency and validation safety.

Adding x-kubernetes-list-type: atomic to excludeSubnets mirrors the existing annotation on the subnets field and is semantically correct: both fields are tightly coupled by validation rules (line 358 enforces that excludeSubnets must be subnetworks of subnets). Atomic semantics prevent partial updates that could violate this invariant. This change aligns with Kubernetes best practices and maintains consistency across all feature-set variants.

openapi/openapi.json (1)

14292-14297: Addition of atomic list type annotation is correctly structured.

The x-kubernetes-list-type: "atomic" annotation on line 14296 is properly placed at the field level (sibling to items) and appropriately marks the resolvedAddresses list as atomic. This is semantically correct for a DNS address list where order matters, duplicates are meaningless, and atomic replacement semantics align with expected behavior.

features.md (1)

1-115: Documentation table appropriately reflects OKD featureset additions.

The features.md table correctly presents OKD as a new cluster profile option with corresponding feature gate enablement across Hypershift and SelfManagedHA variants. The table structure and column ordering are consistent with existing patterns, and the enabled/disabled status reflects the intended OKD feature coverage.

operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml (1)

1-331: CRD structure is comprehensive and well-validated.

The Etcd CRD manifest includes appropriate operator pattern fields (managementState, logLevel, observedConfig), proper enum constraints, and sophisticated validation rules including CEL expressions for cross-field constraints (e.g., ensuring at most one node has a nonzero targetRevision). The x-kubernetes-list-type and x-kubernetes-validations annotations are correctly applied throughout.

operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml (1)

1-488: CSI driver configuration validation is comprehensive and platform-aware.

The ClusterCSIDriver CRD includes appropriate enumeration of supported CSI drivers in the metadata.name field and implements sophisticated platform-specific configuration validation. The CEL rule enforcing the ibmcloud configuration requirement (lines 302–306) correctly prevents misconfiguration. The use of x-kubernetes-list-type: atomic for topologyCategories is appropriate.

example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-TechPreviewNoUpgrade.crd.yaml (1)

318-333: Atomic list type consistency applied correctly.

Adding x-kubernetes-list-type: atomic to the excludeSubnets array (line 333) aligns with the atomic semantics already applied to the subnets list (line 351). This ensures consistent handling of both CIDR lists within the subnetsWithExclusions structure.

payload-manifests/crds/0000_10_config-operator_01_imagepolicies-Default.crd.yaml (1)

1-511: Image policy validation is thorough and correctly enforces configuration requirements.

The ImagePolicy CRD includes comprehensive validation for signature verification policies. The cross-field CEL validations (lines 271–283 and 388–396) correctly enforce that policyType/matchPolicy values must be paired with their corresponding configuration sections. The scopes field properly validates image reference format per Docker Registry HTTP API V2 conventions, and the use of x-kubernetes-list-type: set is appropriate for a collection of unique values.

example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example.yaml (1)

318-333: Atomic list type annotation ensures consistent list handling across feature sets.

The addition of x-kubernetes-list-type: atomic to excludeSubnets (line 333) maintains consistency with the subnets array (line 351) and aligns with the same change applied across related CRD manifests in this PR.

example/v1/types_stable.go (1)

207-208: LGTM!

Adding +listType=atomic to ExcludeSubnets aligns with the existing Subnets field pattern and ensures consistent CRD generation for list semantics.

config/v1/types_feature.go (2)

56-61: LGTM!

The OKD FeatureSet is properly defined with clear documentation about its upgrade behavior and cluster type restrictions. Adding it to AllFixedFeatureSets ensures it's included in feature gate processing.


74-78: Validation rules are correctly implemented.

The XValidation rule oldSelf == 'OKD' ? self != '' : true correctly prevents transitioning from OKD back to Default (empty string) while allowing transitions to TechPreviewNoUpgrade or DevPreviewNoUpgrade, which aligns with the design discussed in the PR comments.

payload-manifests/crds/0000_10_config-operator_01_imagepolicies-CustomNoUpgrade.crd.yaml (1)

1-511: LGTM!

The ImagePolicy CRD manifest for CustomNoUpgrade is well-structured with comprehensive OpenAPI v3 schema validation for image signature verification scenarios (PublicKey, FulcioCAWithRekor, PKI). The CEL validation rules for PEM formats, URLs, and email addresses are appropriately defined.

payload-manifests/crds/0000_10_config-operator_01_featuregates.crd.yaml (1)

82-96: LGTM!

The OKD enum value and validation rule are correctly implemented. The rule oldSelf == 'OKD' ? self != '' : true properly prevents transitioning from OKD back to Default (represented as empty string) while permitting transitions to other feature sets like TechPreviewNoUpgrade or DevPreviewNoUpgrade.

payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1)

1-122: Well-formed CRD manifest with appropriate OKD feature-set marking.

The Scheduler CRD is consistent with other config.openshift.io cluster-scoped resources in this PR. Schema is complete with clear field descriptions and proper status subresource configuration.

config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yaml (1)

1-437: Well-formed APIServer CRD with comprehensive TLS and authentication configuration.

The APIServer CRD for OKD is complete with proper x-kubernetes-list-type annotations, comprehensive audit and TLS security profile options, and clear documentation. Status subresource is properly configured.

payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml (1)

130-216: Well-structured PKI root-of-trust configuration with comprehensive validations.

The pki field definition is complete with all required components (caRootsData, caIntermediatesData, pkiCertificateSubject) and proper validation rules. This structure should be mirrored in the ClusterImagePolicy CRD (File 5), which currently lacks the pki field definition.

payload-manifests/crds/0000_10_config-operator_01_apiservers-OKD.crd.yaml (1)

1-437: Duplicate CRD definition with File 4.

This APIServer CRD (File 8, payload-manifests/crds/) is identical to File 4 (config/v1/zz_generated.crd-manifests/). Both declare the same CRD apiservers.config.openshift.io with identical schemas and annotations.

Verify whether this duplication is intentional (e.g., for release manifest ordering) or if one should be removed to avoid maintenance issues and potential deployment conflicts.

payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-TechPreviewNoUpgrade.crd.yaml (1)

1-510: ClusterImagePolicy TechPreview CRD schema looks consistent and correct

The schema, required fields, and CEL/x-kubernetes-validations align with the other ClusterImagePolicy variants (DevPreview, CustomNoUpgrade) and enforce the expected invariants on rootOfTrust, signedIdentity, and scopes. I don’t see correctness or security issues here.

payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-DevPreviewNoUpgrade.crd.yaml (1)

1-510: DevPreview ClusterImagePolicy CRD mirrors the other variants correctly

This CRD matches the TechPreview/Custom versions (apart from the feature-set annotation) and reuses the same validated schema and CEL rules. No functional or validation issues stand out.

config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1)

1-122: OKD Scheduler CRD variant is consistent with existing Scheduler API

The OKD-scoped Scheduler CRD carries over the known schema (defaultNodeSelector, mastersSchedulable, deprecated policy, profile enum) with a required spec and status subresource. I don’t see any validation or structural problems here.

features/features.go (1)

39-939: OKD inclusion in feature gates is consistent with the intended Default⊆OKD relationship

The additions of configv1.OKD to enableIn(...) and enableForClusterProfile(...) calls are consistent: any feature enabled for Default here is also enabled for OKD, while DevPreview/TechPreview- or Hypershift-only gates remain unchanged. Given the separate parity test enforcing Default→OKD promotion, this looks correct and maintainable.

payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml (1)

1-488: ClusterCSIDriver OKD CRD wiring and validations look sound

The OKD variant reuses the existing ClusterCSIDriver schema, with correct driverType discrimination (notably the IBMCloud ibmcloud presence check), sensible numeric bounds, and standard operator status/conditions structure. No correctness or security concerns identified.

payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-CustomNoUpgrade.crd.yaml (1)

1-510: CustomNoUpgrade ClusterImagePolicy CRD matches other variants without issues

This CustomNoUpgrade CRD is structurally identical to the TechPreview/DevPreview versions aside from the feature-set annotation, so the same well-formed schema and CEL validations apply here. No additional issues detected.

machine/v1/zz_generated.crd-manifests/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (2)

78-100: machineNamePrefix CEL validation is correctly using !format.dns1123Subdomain().validate(self).hasValue()

The description requires machineNamePrefix to be a valid lowercase RFC 1123 subdomain. The CEL rule negates format.dns1123Subdomain().validate(self).hasValue(), which is the standard pattern: validate returns an error value when the string is invalid, so !…hasValue() is true only for valid names. This matches other usages of dns1123Subdomain in the repo and does not need to be inverted.


101-949: ControlPlaneMachineSet OKD CRD aligns with existing API and validations

Beyond machineNamePrefix, the CRD correctly enforces:

  • replicas limited to 3 or 5 and immutable,
  • immutable selector,
  • one-of platform-specific failureDomain blocks via CEL,
  • required template metadata labels for control-plane machines,
  • standard status conditions and scale subresource wiring.

This matches the established ControlPlaneMachineSet schema; no functional issues found.

machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-OKD.crd.yaml (1)

1-544: OKD Machines CRD cleanly reuses the standard Machine v1beta1 schema

The OKD-scoped Machines CRD carries over the established Machine API (spec, status, lifecycleHooks, taints, provider* fields, status conditions) without altering behavior. I don’t see any schema or validation issues here.

Comment on lines 130 to 283
policyType:
description: |-
policyType is a required field specifies the type of the policy for verification. This field must correspond to how the policy was generated.
Allowed values are "PublicKey", "FulcioCAWithRekor", and "PKI".
When set to "PublicKey", the policy relies on a sigstore publicKey and may optionally use a Rekor verification.
When set to "FulcioCAWithRekor", the policy is based on the Fulcio certification and incorporates a Rekor verification.
When set to "PKI", the policy is based on the certificates from Bring Your Own Public Key Infrastructure (BYOPKI).
type: string
publicKey:
description: |-
publicKey defines the root of trust configuration based on a sigstore public key. Optionally include a Rekor public key for Rekor verification.
publicKey is required when policyType is PublicKey, and forbidden otherwise.
properties:
keyData:
description: |-
keyData is a required field contains inline base64-encoded data for the PEM format public key.
keyData must be at most 8192 characters.
format: byte
maxLength: 8192
minLength: 68
type: string
x-kubernetes-validations:
- message: the keyData must start with base64 encoding
of '-----BEGIN PUBLIC KEY-----'.
rule: string(self).startsWith('-----BEGIN PUBLIC KEY-----')
- message: the keyData must end with base64 encoding of
'-----END PUBLIC KEY-----'.
rule: string(self).endsWith('-----END PUBLIC KEY-----\n')
|| string(self).endsWith('-----END PUBLIC KEY-----')
rekorKeyData:
description: |-
rekorKeyData is an optional field contains inline base64-encoded data for the PEM format from the Rekor public key.
rekorKeyData must be at most 8192 characters.
format: byte
maxLength: 8192
type: string
x-kubernetes-validations:
- message: the rekorKeyData must start with base64 encoding
of '-----BEGIN PUBLIC KEY-----'.
rule: string(self).startsWith('-----BEGIN PUBLIC KEY-----')
- message: the rekorKeyData must end with base64 encoding
of '-----END PUBLIC KEY-----'.
rule: string(self).endsWith('-----END PUBLIC KEY-----\n')
|| string(self).endsWith('-----END PUBLIC KEY-----')
required:
- keyData
type: object
required:
- policyType
type: object
x-kubernetes-validations:
- message: publicKey is required when policyType is PublicKey,
and forbidden otherwise
rule: 'has(self.policyType) && self.policyType == ''PublicKey''
? has(self.publicKey) : !has(self.publicKey)'
- message: fulcioCAWithRekor is required when policyType is FulcioCAWithRekor,
and forbidden otherwise
rule: 'has(self.policyType) && self.policyType == ''FulcioCAWithRekor''
? has(self.fulcioCAWithRekor) : !has(self.fulcioCAWithRekor)'
signedIdentity:
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, locate the file and verify its existence
find . -name "*clusterimagepolicies*OKD*" -type f

Repository: openshift/api

Length of output: 142


🏁 Script executed:

#!/bin/bash
# Read the file to check the structure and verify the pki field
cat -n ./payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml | sed -n '130,200p'

Repository: openshift/api

Length of output: 5981


🏁 Script executed:

#!/bin/bash
# Also search for the pki field definition anywhere in the file
grep -n "pki:" ./payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml

Repository: openshift/api

Length of output: 39


🏁 Script executed:

#!/bin/bash
# Find similar CRD files for ImagePolicy to compare structure
find . -name "*imagepolic*" -type f | head -20

Repository: openshift/api

Length of output: 1361


🏁 Script executed:

#!/bin/bash
# Search for pki field in the TechPreviewNoUpgrade variant to see expected structure
grep -n "pki:" ./payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-TechPreviewNoUpgrade.crd.yaml | head -5

Repository: openshift/api

Length of output: 87


🏁 Script executed:

#!/bin/bash
# Read the rootOfTrust section from TechPreviewNoUpgrade to understand the structure
cat -n ./payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-TechPreviewNoUpgrade.crd.yaml | sed -n '130,250p'

Repository: openshift/api

Length of output: 9885


🏁 Script executed:

#!/bin/bash
# Verify the validation rules section in OKD variant to confirm missing PKI validation
cat -n ./payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml | sed -n '177,195p'

Repository: openshift/api

Length of output: 1755


Add missing pki field to rootOfTrust schema.

Line 133 documents "PKI" as a valid policyType value, but the schema lacks the corresponding pki object definition and validation rule. The rootOfTrust properties must include a pki field (after line 129) with caRootsData, caIntermediatesData, and pkiCertificateSubject, and x-kubernetes-validations must enforce that pki is required when policyType is "PKI" and forbidden otherwise. Compare with the TechPreviewNoUpgrade variant (lines 130–216), which includes the complete pki definition.

🤖 Prompt for AI Agents
In
payload-manifests/crds/0000_10_config-operator_01_clusterimagepolicies-OKD.crd.yaml
around lines 130 to 189, add the missing rootOfTrust.pki object (insert after
line 129 inside the publicKey/fulcioCAWithRekor/pki group) mirroring the
TechPreviewNoUpgrade variant: define pki as an object with properties
caRootsData (format: byte, maxLength: 8192, type: string,
x-kubernetes-validations to check begins/ends with PEM markers and minLength
similar to keyData), caIntermediatesData (format: byte, maxLength: 8192, type:
string, with same PEM validations), and pkiCertificateSubject (type: string,
description), mark any required subfields (caRootsData at minimum), and then add
an x-kubernetes-validations rule at the same level as the existing policyType
rules: 'has(self.policyType) && self.policyType == ''PKI'' ? has(self.pki) :
!has(self.pki)'. Ensure formatting and indentation match surrounding schema and
validation patterns.

Comment on lines 130 to 180
policyType:
description: |-
policyType is a required field specifies the type of the policy for verification. This field must correspond to how the policy was generated.
Allowed values are "PublicKey", "FulcioCAWithRekor", and "PKI".
When set to "PublicKey", the policy relies on a sigstore publicKey and may optionally use a Rekor verification.
When set to "FulcioCAWithRekor", the policy is based on the Fulcio certification and incorporates a Rekor verification.
When set to "PKI", the policy is based on the certificates from Bring Your Own Public Key Infrastructure (BYOPKI).
type: string
publicKey:
description: |-
publicKey defines the root of trust configuration based on a sigstore public key. Optionally include a Rekor public key for Rekor verification.
publicKey is required when policyType is PublicKey, and forbidden otherwise.
properties:
keyData:
description: |-
keyData is a required field contains inline base64-encoded data for the PEM format public key.
keyData must be at most 8192 characters.
format: byte
maxLength: 8192
minLength: 68
type: string
x-kubernetes-validations:
- message: the keyData must start with base64 encoding
of '-----BEGIN PUBLIC KEY-----'.
rule: string(self).startsWith('-----BEGIN PUBLIC KEY-----')
- message: the keyData must end with base64 encoding of
'-----END PUBLIC KEY-----'.
rule: string(self).endsWith('-----END PUBLIC KEY-----\n')
|| string(self).endsWith('-----END PUBLIC KEY-----')
rekorKeyData:
description: |-
rekorKeyData is an optional field contains inline base64-encoded data for the PEM format from the Rekor public key.
rekorKeyData must be at most 8192 characters.
format: byte
maxLength: 8192
type: string
x-kubernetes-validations:
- message: the rekorKeyData must start with base64 encoding
of '-----BEGIN PUBLIC KEY-----'.
rule: string(self).startsWith('-----BEGIN PUBLIC KEY-----')
- message: the rekorKeyData must end with base64 encoding
of '-----END PUBLIC KEY-----'.
rule: string(self).endsWith('-----END PUBLIC KEY-----\n')
|| string(self).endsWith('-----END PUBLIC KEY-----')
required:
- keyData
type: object
required:
- policyType
type: object
x-kubernetes-validations:
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's find the relevant CRD files
find . -name "*imagepolicies*" -type f | head -20

Repository: openshift/api

Length of output: 1361


🏁 Script executed:

# Check the OKD file size and read around the policyType enum area
wc -l ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

Repository: openshift/api

Length of output: 139


🏁 Script executed:

# Check policyType enum area in OKD file (around line 227 mentioned in review)
sed -n '220,240p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

echo "---"

# Search for pki field in OKD file
grep -n "pki:" ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

Repository: openshift/api

Length of output: 2157


🏁 Script executed:

# Read the rootOfTrust and policyType sections in OKD file
sed -n '100,180p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

Repository: openshift/api

Length of output: 5345


🏁 Script executed:

# Continue reading to see rest of rootOfTrust structure
sed -n '180,250p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

Repository: openshift/api

Length of output: 7021


🏁 Script executed:

# Read lines around where rootOfTrust should be defined and end
sed -n '70,195p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml | head -100

Repository: openshift/api

Length of output: 7052


🏁 Script executed:

# Check the TechPreviewNoUpgrade variant for comparison
wc -l ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

echo "---"

# Check if pki field exists in TechPreviewNoUpgrade
grep -n "^\s*pki:" ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 191


🏁 Script executed:

# Check the pki field in TechPreviewNoUpgrade variant
sed -n '130,250p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 9038


🏁 Script executed:

# Check if OKD has validation rules for pki field
sed -n '180,200p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

echo "---"

# Check TechPreviewNoUpgrade validation rules for pki
sed -n '216,240p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 3763


🏁 Script executed:

# Get full validation rules from TechPreviewNoUpgrade for pki
sed -n '290,330p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 3962


🏁 Script executed:

# Find the validation rules for pki field in TechPreviewNoUpgrade
grep -n "pki is required when policyType is PKI" ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 240


🏁 Script executed:

# Get the pki validation rule from TechPreviewNoUpgrade
sed -n '270,280p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 778


🏁 Script executed:

# Final verification: confirm OKD has the enum value but not the field
echo "OKD policyType enum:"
sed -n '130,143p' ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

echo -e "\n---\nOKD has pki field:"
grep -c "^\s*pki:" ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml

echo -e "\nTechPreviewNoUpgrade has pki field:"
grep -c "^\s*pki:" ./payload-manifests/crds/0000_10_config-operator_01_imagepolicies-TechPreviewNoUpgrade.crd.yaml

Repository: openshift/api

Length of output: 1353


Add missing pki field to rootOfTrust schema.

The OKD ImagePolicy variant declares support for policyType: PKI in the description but lacks the corresponding pki field definition. The schema must include the pki object with caRootsData, caIntermediatesData, and pkiCertificateSubject properties (see TechPreviewNoUpgrade variant for reference), along with the validation rule requiring pki when policyType is PKI.

🤖 Prompt for AI Agents
In payload-manifests/crds/0000_10_config-operator_01_imagepolicies-OKD.crd.yaml
around lines 130-180, the rootOfTrust schema documents support for policyType
"PKI" but lacks the corresponding `pki` property and validation; add a `pki`
object to the rootOfTrust sibling to `publicKey` that matches the
TechPreviewNoUpgrade variant (include `caRootsData` (format: byte, maxLength
8192, type: string, required), optional `caIntermediatesData` (format: byte,
maxLength 8192, type: string), and a `pkiCertificateSubject` object with the
same fields and validations used in the TechPreviewNoUpgrade file), and add an
x-kubernetes-validation rule requiring `pki` when `.policyType == "PKI"` (and
ensure `pki` is forbidden for other policyType values if the other variants
enforce that).

@JoelSpeed
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2025
@openshift-ci-robot
Copy link

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-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change

@jatinsu
Copy link
Contributor Author

jatinsu commented Dec 12, 2025

/test e2e-aws-ovn-hypershift

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2025
ran make update

adding files to test

updated and vendored api

updated all generated files to reflect new feature gates

Re-add hack/update-protobuf

Add kube-api-server crds

adding listType=atomic to excludeSubnets
Fix error in integration tests
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2025
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (12)
example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-OKD.crd.yaml (1)

4-6: Replace placeholder api-approved.openshift.io URL.
This still points at pull/xxx, which is ambiguous.

 metadata:
   annotations:
-    api-approved.openshift.io: https://github.com/openshift/api/pull/xxx
+    api-approved.openshift.io: https://github.com/openshift/api/pull/EXAMPLE
     api.openshift.io/merged-by-featuregates: "true"
operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml (1)

103-111: Enum contradicts description: include controlplanemachinesets in both copies.
The description allows machinesets and controlplanemachinesets, but the enum only allows machinesets (in both spec and status).

                         resource:
                           description: |-
                             resource is the machine management resource's type.
                             Valid values are machinesets and controlplanemachinesets.
@@
                           enum:
                           - machinesets
+                          - controlplanemachinesets
                           type: string
@@
                         resource:
                           description: |-
                             resource is the machine management resource's type.
                             Valid values are machinesets and controlplanemachinesets.
@@
                           enum:
                           - machinesets
+                          - controlplanemachinesets
                           type: string

Also applies to: 737-744

payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml (3)

33-34: Fix double-negative / grammar in Updating printer column description (still present).

The description reads incorrectly (“one of machine is not either not…”).

-    - description: When at least one of machine is not either not updated or is in
-        the process of updating to the desired machine config.
+    - description: When at least one machine is not updated or is in the process of
+        updating to the desired machine config.

347-353: Fix ceryExpiry typo in schema description.

-                items:
-                  description: ceryExpiry contains the bundle name and the expiry
-                    date
+                items:
+                  description: certExpiry contains the bundle name and the expiry
+                    date

502-505: Remove double period in degradedMachineCount description.

-                  A node is marked degraded if applying a configuration failed..
+                  A node is marked degraded if applying a configuration failed.
machine/v1/zz_generated.crd-manifests/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (1)

94-100: Inverted CEL validation logic for machineNamePrefix.

The rule appears to enforce an invalid DNS1123 subdomain. Fix by removing the negation:

-                  rule: '!format.dns1123Subdomain().validate(self).hasValue()'
+                  rule: 'format.dns1123Subdomain().validate(self).hasValue()'
payload-manifests/crds/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (2)

94-100: Inverted CEL validation logic for machineNamePrefix.

-                  rule: '!format.dns1123Subdomain().validate(self).hasValue()'
+                  rule: 'format.dns1123Subdomain().validate(self).hasValue()'

950-954: Scale subresource references missing .status.labelSelector.

labelSelectorPath: .status.labelSelector is set, but .status.labelSelector isn’t defined in the schema. Add it under status.properties or remove labelSelectorPath.

           status:
             description: ControlPlaneMachineSetStatus represents the status of the
               ControlPlaneMachineSet CRD.
             properties:
+              labelSelector:
+                description: Label selector for Machines that match this ControlPlaneMachineSet.
+                type: string
               conditions:

Also applies to: 836-845

config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_nodes-OKD.crd.yaml (1)

55-63: Fix typo/grammar in workerLatencyProfile description.
This is still "determins the how fast" (Line 57).

               workerLatencyProfile:
                 description: |-
-                  workerLatencyProfile determins the how fast the kubelet is updating
-                  the status and corresponding reaction of the cluster
+                  workerLatencyProfile determines how quickly the kubelet updates
+                  status and how the cluster reacts.
                 enum:
                 - Default
                 - MediumUpdateAverageReaction
                 - LowUpdateSlowReaction
payload-manifests/crds/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1)

78-82: Revisit managementState validation: Force is likely non-standard.
Pattern currently allows only Managed|Force (Line 81).

operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1)

78-82: Same managementState validation issue here (Force). Fix at the source so generated CRDs agree.
Pattern is ^(Managed|Force)$ (Line 81).

payload-manifests/crds/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml (1)

103-111: Enum contradicts description: add controlplanemachinesets to resource.
Description says both machinesets and controlplanemachinesets are valid, but enum only permits machinesets (Line 109; also Line 742).

                         resource:
                           description: |-
                             resource is the machine management resource's type.
                             Valid values are machinesets and controlplanemachinesets.
                             machinesets means that the machine manager will only register resources of the kind MachineSet.
                             controlplanemachinesets means that the machine manager will only register resources of the kind ControlPlaneMachineSet.
                           enum:
                           - machinesets
+                          - controlplanemachinesets
                           type: string

Also applies to: 737-744

🧹 Nitpick comments (1)
features/features.go (1)

39-932: OKD enablement additions look correct; consider normalizing FeatureSet argument ordering for maintainability.
Right now the enableIn(...) / enableForClusterProfile(...) args are in mixed orders (e.g., Line 43 vs Line 123), which tends to create noisy diffs over time.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between c68e866 and d1bbe4d.

📒 Files selected for processing (46)
  • config/v1/tests/featuregates.config.openshift.io/AAA_ungated.yaml (2 hunks)
  • config/v1/types_feature.go (2 hunks)
  • config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_featuregates.crd.yaml (2 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_nodes-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • config/v1/zz_generated.featuregated-crd-manifests/featuregates.config.openshift.io/AAA_ungated.yaml (2 hunks)
  • example/v1/types_stable.go (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-Default.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-DevPreviewNoUpgrade.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-OKD.crd.yaml (1 hunks)
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-TechPreviewNoUpgrade.crd.yaml (1 hunks)
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/AAA_ungated.yaml (1 hunks)
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example+Example2.yaml (1 hunks)
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example.yaml (1 hunks)
  • features.md (1 hunks)
  • features/features.go (21 hunks)
  • features/okd_featureset_parity_test.go (1 hunks)
  • machine/v1/zz_generated.crd-manifests/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (1 hunks)
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-OKD.crd.yaml (1 hunks)
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machinesets-OKD.crd.yaml (1 hunks)
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml (1 hunks)
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml (1 hunks)
  • openapi/generated_openapi/zz_generated.openapi.go (1 hunks)
  • openapi/openapi.json (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml (1 hunks)
  • operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_apiservers-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_featuregates.crd.yaml (2 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_nodes-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_12_etcd_01_etcds-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml (1 hunks)
  • payload-manifests/crds/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml (1 hunks)
  • payload-manifests/featuregates/featureGate-Hypershift-OKD.yaml (1 hunks)
  • payload-manifests/featuregates/featureGate-SelfManagedHA-OKD.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
  • config/v1/tests/featuregates.config.openshift.io/AAA_ungated.yaml
  • features/okd_featureset_parity_test.go
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-DevPreviewNoUpgrade.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_nodes-OKD.crd.yaml
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_apiservers-OKD.crd.yaml
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-Default.crd.yaml
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machinesets-OKD.crd.yaml
  • openapi/generated_openapi/zz_generated.openapi.go
  • config/v1/zz_generated.featuregated-crd-manifests/featuregates.config.openshift.io/AAA_ungated.yaml
  • features.md
  • payload-manifests/crds/0000_12_etcd_01_etcds-OKD.crd.yaml
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_featuregates.crd.yaml
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example+Example2.yaml
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_featuregates.crd.yaml
  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/Example.yaml
  • payload-manifests/featuregates/featureGate-SelfManagedHA-OKD.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

  • example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/AAA_ungated.yaml
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_apiservers-OKD.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-OKD.crd.yaml
  • machine/v1/zz_generated.crd-manifests/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml
  • machineconfiguration/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml
  • payload-manifests/crds/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml
  • openapi/openapi.json
  • config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_nodes-OKD.crd.yaml
  • payload-manifests/crds/0000_10_control-plane-machine-set_01_controlplanemachinesets-OKD.crd.yaml
  • operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml
  • features/features.go
  • example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-TechPreviewNoUpgrade.crd.yaml
  • payload-manifests/featuregates/featureGate-Hypershift-OKD.yaml
  • payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml
  • payload-manifests/crds/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml
  • payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml
  • config/v1/zz_generated.crd-manifests/0000_00_cluster-version-operator_01_clusterversions-OKD.crd.yaml
  • payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml
  • payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-SelfManagedHA-OKD.crd.yaml
  • config/v1/types_feature.go
  • payload-manifests/crds/0000_80_machine-config_01_machineconfigpools-OKD.crd.yaml
  • machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-OKD.crd.yaml
  • example/v1/types_stable.go
🧬 Code graph analysis (1)
features/features.go (2)
config/v1/types_feature.go (4)
  • Default (41-41)
  • OKD (58-58)
  • DevPreviewNoUpgrade (49-49)
  • TechPreviewNoUpgrade (45-45)
features/util.go (2)
  • SelfManaged (36-36)
  • Hypershift (35-35)
🔇 Additional comments (11)
example/v1/types_stable.go (1)

203-208: +listType=atomic addition looks correct and consistent with other list fields.

machine/v1beta1/zz_generated.crd-manifests/0000_10_machine-api_01_machines-OKD.crd.yaml (1)

1-544: Well-structured OKD-specific Machine CRD with proper Kubernetes API conventions.

This CRD manifest properly implements the OKD-specific Machine resource. The schema is comprehensive with:

  • Correct feature-set annotation marking it as OKD (line 10)
  • Proper list-type semantics with atomic and mapped list markers throughout (lines 122–124, 158–160, 342, 365, 410–413)
  • Atomic map types for nested object references (lines 275, 525)
  • Extensibility via x-kubernetes-preserve-unknown-fields for provider-specific fields (lines 305, 538)
  • Complete OpenAPI v3 schema with detailed descriptions aligned with Kubernetes conventions
  • Status subresource properly declared (lines 541–544)

All lifecycle hooks, metadata ownership, conditions, and provider references are correctly marked for proper list and map semantics in etcd storage and API responses.

example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-TechPreviewNoUpgrade.crd.yaml (1)

318-334: Confirm excludeSubnets should be atomic (SSA/merge semantics change).

Adding x-kubernetes-list-type: atomic means server-side apply and strategic merge treat excludeSubnets as “replace the whole list” rather than merge item-wise. Please verify this is the intended UX for concurrent writers/field managers updating exclusions (and that it matches the corresponding OKD/Default CRDs for the same schema).

openapi/openapi.json (1)

14292-14298: Atomic list-type extension aligns with Kubernetes best practices.

The addition of "x-kubernetes-list-type": "atomic" to the excludeSubnets field marks the list for atomic merge semantics, which is a standard Kubernetes CRD best practice. This prevents item-by-item merging and instead replaces the entire list, reducing merge conflicts and improving predictability.

Please verify:

  1. Whether openapi/openapi.json is auto-generated from source files (e.g., Go types in config/v1, CRD manifests) or hand-maintained. If auto-generated, this change should originate from the underlying source files.
  2. That this atomic-list pattern is applied consistently across all similar list fields in the OpenAPI schema and related CRD manifests introduced in this PR.
example/v1/zz_generated.featuregated-crd-manifests/stableconfigtypes.example.openshift.io/AAA_ungated.yaml (1)

326-329: excludeSubnets atomic list annotation looks appropriate.

example/v1/zz_generated.crd-manifests/0000_50_my-operator_01_stableconfigtypes-CustomNoUpgrade.crd.yaml (1)

331-334: excludeSubnets atomic list annotation looks appropriate.

payload-manifests/crds/0000_80_machine-config_01_machineconfignodes-Hypershift-OKD.crd.yaml (1)

121-126: The DNS1123 validation pattern is correct as-is; no changes needed.

The CEL validation rule !format.dns1123Subdomain().validate(self).hasValue() is the standard and correct pattern in Kubernetes. The format.dns1123Subdomain().validate(self) returns an optional that is empty (none) when the input is valid and contains error messages when invalid. The !hasValue() negation therefore correctly validates that the string is a valid DNS1123 subdomain. This pattern is consistent across the entire codebase, including hand-written Go source files, and is documented in the Kubernetes CEL reference. Removing the negation would actually break validation by accepting invalid names.

config/v1/types_feature.go (1)

56-62: OKD FeatureSet addition and "no transition back to Default" validation are correct.

The XValidation rule preventing OKD → Default transitions is properly enforced at the API level. Regarding OCP/OKD separation: enforcement occurs at the CVO/payload level through ClusterVersion reconciliation and FeatureGate status validation, not through type-level restrictions. This is the intended design—the type enum allows "OKD", but CVO validates FeatureSet compatibility against the release payload and reports unsupported combinations as status conditions. No additional gating needed in this file.

payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-OKD.crd.yaml (1)

1-122: Scheduler OKD CRD addition looks consistent with existing config.openshift.io patterns.

features/features.go (1)

9-34: FeatureSets()/AllFeatureSets() logic looks fine.

operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml (1)

89-93: No changes needed. The managementState pattern ^(Managed|Unmanaged|Force|Removed)$ correctly reflects the canonical ManagementState type defined in operator/v1/types.go. All four values are intentionally supported across OpenShift operators, with Force explicitly documented as a valid state that allows the operator to manage resources while tolerating unmet upgrade prerequisites.

@Prashanth684
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 12, 2025
@openshift-ci-robot
Copy link

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-hypershift
/test e2e-aws-ovn-hypershift-conformance
/test e2e-aws-ovn-techpreview
/test e2e-aws-serial-1of2
/test e2e-aws-serial-2of2
/test e2e-aws-serial-techpreview-1of2
/test e2e-aws-serial-techpreview-2of2
/test e2e-azure
/test e2e-gcp
/test e2e-upgrade
/test e2e-upgrade-out-of-change

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 12, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, Prashanth684

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:

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

@Prashanth684
Copy link
Contributor

/retest-required

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 13, 2025

@jatinsu: 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.

@Prashanth684
Copy link
Contributor

/verified by @jatinsu

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Dec 14, 2025
@openshift-ci-robot
Copy link

@Prashanth684: This PR has been marked as verified by @jatinsu.

Details

In response to this:

/verified by @jatinsu

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.

@Prashanth684 Prashanth684 added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Dec 14, 2025
@openshift-merge-bot openshift-merge-bot bot merged commit bfa868a into openshift:master Dec 14, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. 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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants