OCPNODE-3863: Add CRIOCredentialProviderConfig API

[QiWang19]

• enhancement: OCPNODE-3753: CRIOCredentialProviderConfig for Namespace-Scoped Mirror Authentication enhancements#1861

• Add CRIOCredentialProviderConfig API setting configs https://github.com/cri-o/crio-credential-provider for retrieving mirror pull secrets

• Add feature gate for CRIOCredentialProviderConfig feature gate.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[openshift-ci]

Hello @QiWang19! 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.

[QiWang19]

/retest-required

[QiWang19]

/test integration

[QiWang19]

PR has been reviewed by claude code /api-review. It helped run make lint and correct the failures.

[openshift-ci-robot]

@QiWang19: This pull request references OCPNODE-3863 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

• Add CRIOCredentialProviderConfig API setting configs https://github.com/cri-o/crio-credential-provider for retrieving mirror pull secrets

• Add feature gate for CRIOCredentialProviderConfig feature gate.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci-robot]

@QiWang19: This pull request references OCPNODE-3863 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.21.0" version, but no target version was set.

Details

In response to this:

• enhancement: OCPNODE-3753: CRIOCredentialProviderConfig for Namespace-Scoped Mirror Authentication enhancements#1861

• Add CRIOCredentialProviderConfig API setting configs https://github.com/cri-o/crio-credential-provider for retrieving mirror pull secrets

• Add feature gate for CRIOCredentialProviderConfig feature gate.

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.

[saschagrunert]

Two nits, otherwise LGTM

[QiWang19]

/assign @everettraven

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (2)

openapi/openapi.json (2)

11792-11800: Missing array bounds validation on matchImages (duplicate of previous review).

The matchImages array lacks minItems: 1 and maxItems: 50 constraints despite the PR spec explicitly requiring "1–50 unique strings". These bounds should be present in the OpenAPI schema alongside the existing x-kubernetes-list-type: "set".

Add cardinality constraints:

         "matchImages": {
           "description": "...",
           "type": "array",
+          "minItems": 1,
+          "maxItems": 50,
           "items": {
             "type": "string",
             "default": ""
           },
           "x-kubernetes-list-type": "set"
         }

Also verify that the corresponding Go struct in config/v1alpha1/types_crio_credential_provider_config.go includes the +kubebuilder:validation:MinItems=1 and +kubebuilder:validation:MaxItems=50 markers on the MatchImages field so that codegen produces these OpenAPI constraints.

---

11807-11818: Missing maxItems constraint on conditions array (duplicate of previous review).

The conditions array lacks the maxItems: 4 constraint specified in the PR requirements. Add this bound to enforce the cardinality limit:

         "conditions": {
           "description": "conditions represent the latest available observations of the configuration state",
           "type": "array",
+          "maxItems": 4,
           "items": {
             "default": {},
             "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.Condition"
           },
           "x-kubernetes-list-map-keys": [
             "type"
           ],
           "x-kubernetes-list-type": "map"
         }

Also verify that the corresponding Go struct includes the +kubebuilder:validation:MaxItems=4 marker on the Conditions field.

📜 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 33614f2 and c34aa43.

📒 Files selected for processing (24)

• config/v1alpha1/register.go (1 hunks)
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/types_crio_credential_provider_config.go (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.deepcopy.go (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/zz_generated.swagger_doc_generated.go (1 hunks)
• features.md (1 hunks)
• features/features.go (1 hunks)
• hack/update-payload-crds.sh (1 hunks)
• openapi/generated_openapi/zz_generated.openapi.go (2 hunks)
• openapi/openapi.json (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (7)

• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
• features/features.go
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• features.md
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.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/v1alpha1/register.go
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• config/v1alpha1/types_crio_credential_provider_config.go
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.swagger_doc_generated.go
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
• openapi/generated_openapi/zz_generated.openapi.go
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• config/v1alpha1/zz_generated.deepcopy.go
• hack/update-payload-crds.sh
• openapi/openapi.json

🧬 Code graph analysis (4)

config/v1alpha1/register.go (2)

config/v1alpha1/types_crio_credential_provider_config.go (2)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)

config/v1alpha1/zz_generated.swagger_doc_generated.go (2)

• CRIOCredentialProviderConfig (236-238)
• CRIOCredentialProviderConfigList (245-247)

config/v1alpha1/types_crio_credential_provider_config.go (1)

config/v1alpha1/zz_generated.swagger_doc_generated.go (3)

• CRIOCredentialProviderConfigSpec (254-256)
• CRIOCredentialProviderConfigStatus (263-265)
• CRIOCredentialProviderConfig (236-238)

config/v1alpha1/zz_generated.swagger_doc_generated.go (1)

config/v1alpha1/types_crio_credential_provider_config.go (4)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)
• CRIOCredentialProviderConfigSpec (46-94)
• CRIOCredentialProviderConfigStatus (117-131)

config/v1alpha1/zz_generated.deepcopy.go (1)

config/v1alpha1/types_crio_credential_provider_config.go (5)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)
• CRIOCredentialProviderConfigSpec (46-94)
• MatchImage (112-112)
• CRIOCredentialProviderConfigStatus (117-131)

🔇 Additional comments (15)

hack/update-payload-crds.sh (1)

29-29: LGTM!

The CRD glob pattern addition correctly follows the established convention and will properly match all CRIOCredentialProviderConfig manifest variants.

payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)

103-105: LGTM!

The CRIOCredentialProviderConfig feature gate is correctly added to the enabled list in alphabetical order, consistent with the DevPreviewNoUpgrade feature set.

payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1)

118-120: LGTM!

The CRIOCredentialProviderConfig feature gate is correctly added to the enabled list in alphabetical order, consistent with the TechPreviewNoUpgrade feature set.

config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml (1)

24-46: LGTM!

The CRIOCredentialProviderConfig CRD metadata entry is complete and correctly structured, with appropriate singleton cluster scope and status subresource configuration.

config/v1alpha1/register.go (1)

43-44: LGTM!

The registration of CRIOCredentialProviderConfig and CRIOCredentialProviderConfigList types follows the standard Kubernetes API registration pattern and is correctly placed alphabetically.

config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1)

1-92: LGTM!

The test suite provides comprehensive coverage with seven test cases validating both valid configurations and various error scenarios (invalid characters, uppercase, wildcard placement, global wildcard). The structure is correct and aligns with the CRD validation rules.

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1)

1-214: LGTM!

The CRD manifest is well-structured with appropriate validations:

• Singleton enforcement via CEL validation
• Comprehensive matchImages validation (lowercase enforcement, wildcard rules, character constraints)
• Status is correctly optional while spec.matchImages is required
• Set semantics for matchImages prevent duplicates

config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1)

1-214: LGTM!

The feature-gated CRD manifest is correctly structured and consistent with the variant CRD manifests. The validations are appropriate, status is correctly optional, and the singleton enforcement is in place.

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1)

55-124: CRD schema and validations look consistent with the Go type and design

matchImages constraints (min/max items, set semantics, per-item max length, CEL regex, and explicit self != '*' guard) and the singleton name validation on .metadata.name == 'cluster' line up with the documented behavior. Status shape and condition map semantics are also wired correctly.

Also applies to: 127-210

config/v1alpha1/zz_generated.swagger_doc_generated.go (1)

229-265: Swagger documentation wiring for CRIOCredentialProviderConfig is coherent

The added SwaggerDoc maps for CRIOCredentialProviderConfig, its List, Spec, and Status accurately reflect the Go doc comments and clearly describe singleton semantics and matchImages behavior.

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

55-124: CustomNoUpgrade CRD matches generated schema and enforces intended invariants

The CustomNoUpgrade CRD mirrors the generated CRD: same matchImages validation (including self != '*' and regex) and singleton .metadata.name == 'cluster' rule, so behavior should be consistent across feature sets.

Also applies to: 127-210

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1)

55-124: Generated CustomNoUpgrade CRD aligns with the type definition and other variants

The CustomNoUpgrade CRD under config/v1alpha1/zz_generated.crd-manifests correctly reflects the Go type: matchImages constraints, the self != '*' guard, and the singleton .metadata.name == 'cluster' constraint all match the other variants.

Also applies to: 127-210

config/v1alpha1/zz_generated.deepcopy.go (1)

195-298: Deepcopy implementations for CRIOCredentialProviderConfig types are correct

The generated DeepCopy/DeepCopyInto/DeepCopyObject methods for CRIOCredentialProviderConfig, its List, Spec, and Status all correctly deep-copy slices and metadata, matching the patterns used for existing types in this package.

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

55-124: TechPreviewNoUpgrade payload CRD is consistent with the generated CRD and types

This payload CRD faithfully mirrors the generated TechPreview schema: matchImages validation (including the global '*' rejection and regex), singleton .metadata.name == 'cluster' validation, and the status/conditions structure all line up with the Go API.

Also applies to: 127-210

openapi/generated_openapi/zz_generated.openapi.go (1)

431-434: LGTM: Schema registration is correct.

The four new CRIOCredentialProviderConfig type registrations follow the established pattern and correctly reference their schema generator functions.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (2)

openapi/generated_openapi/zz_generated.openapi.go (1)

21636-21668: Validation constraints still missing after previous fixes.

Previous reviews flagged missing array validation constraints that remain absent:

1. matchImages (lines 21650-21661): Description documents "between 1 and 50 entries" but MinItems: 1 and MaxItems: 50 are not present in the schema.
2. conditions (lines 21687-21697): Description states "maximum number of conditions is 16" but MaxItems: 16 is not present in the schema.

These constraints are essential to prevent empty arrays (matchImages) and unbounded growth (conditions).

Since these issues were previously marked as addressed, verify that the source types have the correct kubebuilder validation markers and regenerate:

make update-codegen-crds

If regeneration still omits the constraints, the OpenAPI generator may not translate kubebuilder markers for these fields, requiring manual schema updates or generator enhancement.

Also applies to: 21677-21697

openapi/openapi.json (1)

11792-11800: Missing array bounds validation (duplicate of earlier comment; still unfixed).

The matchImages array (lines 11792–11800) lacks minItems: 1 and maxItems: 50, and the conditions array (lines 11807–11818) lacks maxItems: 4, despite PR requirements explicitly stating these bounds. This validation gap was flagged in the previous review and remains unaddressed.

The earlier comment provided the exact fix; please apply it or confirm the bounds should not be enforced at the schema level.

Also applies to: 11807-11818

📜 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 c34aa43 and 07c6dc5.

📒 Files selected for processing (24)

• config/v1alpha1/register.go (1 hunks)
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/types_crio_credential_provider_config.go (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.deepcopy.go (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/zz_generated.swagger_doc_generated.go (1 hunks)
• features.md (1 hunks)
• features/features.go (1 hunks)
• hack/update-payload-crds.sh (1 hunks)
• openapi/generated_openapi/zz_generated.openapi.go (2 hunks)
• openapi/openapi.json (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (10)

• features/features.go
• features.md
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
• config/v1alpha1/zz_generated.deepcopy.go
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.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:

• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• hack/update-payload-crds.sh
• config/v1alpha1/register.go
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• config/v1alpha1/types_crio_credential_provider_config.go
• config/v1alpha1/zz_generated.swagger_doc_generated.go
• openapi/generated_openapi/zz_generated.openapi.go
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• openapi/openapi.json

🧬 Code graph analysis (2)

config/v1alpha1/register.go (2)

config/v1alpha1/types_crio_credential_provider_config.go (2)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)

config/v1alpha1/zz_generated.swagger_doc_generated.go (2)

• CRIOCredentialProviderConfig (236-238)
• CRIOCredentialProviderConfigList (245-247)

config/v1alpha1/zz_generated.swagger_doc_generated.go (1)

config/v1alpha1/types_crio_credential_provider_config.go (4)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)
• CRIOCredentialProviderConfigSpec (46-94)
• CRIOCredentialProviderConfigStatus (117-131)

🔇 Additional comments (14)

hack/update-payload-crds.sh (1)

5-34: New CRD glob inclusion looks correct

Adding the criocredentialproviderconfigs glob under config/v1alpha1/zz_generated.crd-manifests should properly pull the generated CRD variants into payload-manifests/crds/.

payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)

55-62: Feature gate wired into Hypershift Default disabled set

The new CRIOCredentialProviderConfig entry is correctly added to the disabled list for the Hypershift Default payload.

config/v1alpha1/register.go (1)

30-47: Scheme registration for new v1alpha1 types is correct

Adding &CRIOCredentialProviderConfig{} and &CRIOCredentialProviderConfigList{} to scheme.AddKnownTypes(...) is the right wiring for API recognition/serialization.

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (2)

55-126: Validation + singleton enforcement look consistent with documented contract

Good to see:

• explicit rejection of global "*" via self != '*'
• set semantics on matchImages
• singleton enforcement via self.metadata.name == 'cluster'

Also applies to: 207-214

---

4-10: This file is auto-generated and should not be manually edited. The api-approved.openshift.io annotation is sourced from the openshift:api-approved.openshift.io marker in config/v1alpha1/types_crio_credential_provider_config.go. The reference to PR #1929 is correct and consistent across all CRD variants and generated manifests. Any annotation updates must be made to the Go source marker, not to the generated YAML files.

Likely an incorrect or invalid review comment.

config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (2)

6-10: Feature-gate annotation on the CRD is correctly applied

feature-gate.release.openshift.io/CRIOCredentialProviderConfig: "true" is the right mechanism for the featuregated manifest.

---

4-10: No action required. The api-approved.openshift.io annotation is correctly set to PR #1929 and is auto-generated from the source code annotation in config/v1alpha1/types_crio_credential_provider_config.go. This generated file should not be manually edited; if the PR reference needs to change, it must be updated in the source Go file's annotation comment.

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1)

4-10: No action needed. All criocredentialproviderconfigs CRD variants (CustomNoUpgrade, DevPreviewNoUpgrade, and TechPreviewNoUpgrade) correctly reference PR #1929 as defined in the source file types_crio_credential_provider_config.go. The variants are properly aligned and the generated manifests correctly reflect the canonical API approval annotation.

Likely an incorrect or invalid review comment.

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

4-10: Verify whether PR #1929 is the correct API approval reference for CRIOCredentialProviderConfig. Web search could not confirm PR #1929 exists or relates to this API. All CRD variants in the repository consistently reference this PR, but the approval link should be validated against the actual PR used when this API was approved.

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

1-214: LGTM! CRD manifest properly configured.

The CustomNoUpgrade CRD manifest is well-structured with:

• Proper singleton enforcement via CEL validation (lines 207-210)
• Correct wildcard rejection rule self != '*' (lines 115-116) addressing the previous review concern
• Consistent validation schema matching the Go type definitions
• Appropriate status subresource configuration

Previous review concerns about wildcard validation have been properly addressed.

config/v1alpha1/types_crio_credential_provider_config.go (1)

1-184: LGTM! Well-structured API type definitions.

The Go type definitions are comprehensive and properly configured:

• Singleton enforcement via CEL validation (line 24)
• Correct use of project-specific json tags (omitzero/omitempty) following kubeapilinter conventions
• Dual validation rules for MatchImage preventing global wildcard and enforcing pattern (lines 110-111)
• Clear documentation throughout, including detailed condition constants (lines 149-184)
• Status semantics properly aligned with validation constraints

All previous review concerns have been addressed, and the implementation follows OpenShift API conventions.

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1)

1-214: LGTM! Generated CRD manifest consistent with API types.

The generated CustomNoUpgrade CRD manifest correctly reflects the Go type definitions with:

• Proper validation rules including wildcard rejection (lines 115-116)
• Singleton enforcement (lines 207-210)
• Consistent schema across all generated variants

The generation process has correctly translated the Go type annotations into OpenAPI v3 schema.

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

1-214: LGTM! TechPreviewNoUpgrade variant properly configured.

The TechPreviewNoUpgrade CRD manifest is correctly configured with:

• Appropriate feature-set annotation (line 9)
• Consistent validation schema matching other variants
• Proper singleton and wildcard validation rules

The manifest maintains consistency across feature-set variants while correctly applying the TechPreviewNoUpgrade feature gate.

openapi/generated_openapi/zz_generated.openapi.go (1)

431-434: LGTM!

The registry additions correctly wire the new CRIOCredentialProviderConfig schemas into the OpenAPI definitions map. The entries follow the established naming convention and are properly ordered.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Swagger docs addition is fine; List doc likely missing items

map_CRIOCredentialProviderConfigList should likely include an "items" entry like other *List docs in this file.

 var map_CRIOCredentialProviderConfigList = map[string]string{
 	"":         "CRIOCredentialProviderConfigList contains a list of CRIOCredentialProviderConfig resources\n\nCompatibility 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.",
 	"metadata": "metadata is the standard list's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
+	"items":    "items is a list of CRIOCredentialProviderConfig",
 }

📝 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.

Suggested change

	var map_CRIOCredentialProviderConfig = map[string]string{
	"": "CRIOCredentialProviderConfig holds cluster-wide singleton resource configurations for CRI-O credential provider, the name of this instance is \"cluster\". CRI-O credential provider is a binary shipped with CRI-O that provides a way to obtain container image pull credentials from external sources. For example, it can be used to fetch mirror registry credentials from secrets resources in the cluster within the same namespace the pod will be running in. CRIOCredentialProviderConfig configuration specifies the pod image sources registries that should trigger the CRI-O credential provider execution, which will resolve the CRI-O mirror configurations and obtain the necessary credentials for pod creation. Note: Configuration changes will only take effect after the kubelet restarts, which is automatically managed by the cluster during rollout.\n\nThe resource is a singleton named \"cluster\".\n\nCompatibility 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.",
	"metadata": "metadata is the standard object's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
	"spec": "spec defines the desired configuration of the CRI-O Credential Provider. This field is required and must be provided when creating the resource.",
	"status": "status represents the current state of the CRIOCredentialProviderConfig. When omitted or nil, it indicates that the status has not yet been set by the controller. The controller will populate this field with validation conditions and operational state.",
	}
	func (CRIOCredentialProviderConfig) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfig
	}
	var map_CRIOCredentialProviderConfigList = map[string]string{
	"": "CRIOCredentialProviderConfigList contains a list of CRIOCredentialProviderConfig resources\n\nCompatibility 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.",
	"metadata": "metadata is the standard list's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
	}
	func (CRIOCredentialProviderConfigList) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfigList
	}
	var map_CRIOCredentialProviderConfigSpec = map[string]string{
	"": "CRIOCredentialProviderConfigSpec defines the desired configuration of the CRI-O Credential Provider.",
	"matchImages": "matchImages is a required list of string patterns used to determine whether the CRI-O credential provider should be invoked for a given image. This list is passed to the kubelet CredentialProviderConfig, and if any pattern matches the requested image, CRI-O credential provider will be invoked to obtain credentials for pulling that image or its mirrors. Depending on the platform, the CRI-O credential provider may be installed alongside an existing platform specific provider. Conflicts between the existing platform specific provider image match configuration and this list will be handled by the following precedence rule: credentials from built-in kubelet providers (e.g., ECR, GCR, ACR) take precedence over those from the CRIOCredentialProviderConfig when both match the same image. To avoid uncertainty, it is recommended to avoid configuring your private image patterns to overlap with existing platform specific provider config(e.g., the entries from https://github.com/openshift/machine-config-operator/blob/main/templates/common/aws/files/etc-kubernetes-credential-providers-ecr-credential-provider.yaml). You can check the resource's Status conditions to see if any entries were ignored due to exact matches with known built-in provider patterns.\n\nThis field is required and must contain between 1 and 50 entries. The list is treated as a set, so duplicate entries are not allowed.\n\nFor more details, see: https://kubernetes.io/docs/tasks/administer-cluster/kubelet-credential-provider/ https://github.com/cri-o/crio-credential-provider#architecture\n\nEach entry in matchImages is a pattern which can optionally contain a port and a path. Each entry must be no longer than 512 characters. Wildcards ('*') are supported for full subdomain labels, such as '*.k8s.io' or 'k8s.*.io', and for top-level domains, such as 'k8s.*' (which matches 'k8s.io' or 'k8s.net'). A global wildcard '*' (matching any domain) is not allowed. Wildcards may replace an entire hostname label (e.g., *.example.com), but they cannot appear within a label (e.g., f*oo.example.com) and are not allowed in the port or path. For example, 'example.*.com' is valid, but 'exa*mple.*.com' is not. Each wildcard matches only a single domain label, so '*.io' does **not** match '*.k8s.io'.\n\nA match exists between an image and a matchImage when all of the below are true: Both contain the same number of domain parts and each part matches. The URL path of an matchImages must be a prefix of the target image URL path. If the matchImages contains a port, then the port must match in the image as well.\n\nExample values of matchImages: - 123456789.dkr.ecr.us-east-1.amazonaws.com - *.azurecr.io - gcr.io - *.*.registry.io - registry.io:8080/path",
	}
	func (CRIOCredentialProviderConfigSpec) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfigSpec
	}
	var map_CRIOCredentialProviderConfigStatus = map[string]string{
	"": "CRIOCredentialProviderConfigStatus defines the observed state of CRIOCredentialProviderConfig",
	"conditions": "conditions represent the latest available observations of the configuration state. When omitted, it indicates that no conditions have been reported yet. The maximum number of conditions is 16. Conditions are stored as a map keyed by condition type, ensuring uniqueness.\n\nExpected condition types include: \"Validated\": indicates whether the matchImages configuration is valid",
	}
	func (CRIOCredentialProviderConfigStatus) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfigStatus
	}
	var map_CRIOCredentialProviderConfigList = map[string]string{
	"": "CRIOCredentialProviderConfigList contains a list of CRIOCredentialProviderConfig resources\n\nCompatibility 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.",
	"metadata": "metadata is the standard list's metadata. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata",
	"items": "items is a list of CRIOCredentialProviderConfig",
	}
	func (CRIOCredentialProviderConfigList) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfigList
	}
	var map_CRIOCredentialProviderConfigSpec = map[string]string{
	"": "CRIOCredentialProviderConfigSpec defines the desired configuration of the CRI-O Credential Provider.",
	"matchImages": "matchImages is a required list of string patterns used to determine whether the CRI-O credential provider should be invoked for a given image. This list is passed to the kubelet CredentialProviderConfig, and if any pattern matches the requested image, CRI-O credential provider will be invoked to obtain credentials for pulling that image or its mirrors. Depending on the platform, the CRI-O credential provider may be installed alongside an existing platform specific provider. Conflicts between the existing platform specific provider image match configuration and this list will be handled by the following precedence rule: credentials from built-in kubelet providers (e.g., ECR, GCR, ACR) take precedence over those from the CRIOCredentialProviderConfig when both match the same image. To avoid uncertainty, it is recommended to avoid configuring your private image patterns to overlap with existing platform specific provider config(e.g., the entries from https://github.com/openshift/machine-config-operator/blob/main/templates/common/aws/files/etc-kubernetes-credential-providers-ecr-credential-provider.yaml). You can check the resource's Status conditions to see if any entries were ignored due to exact matches with known built-in provider patterns.\n\nThis field is required and must contain between 1 and 50 entries. The list is treated as a set, so duplicate entries are not allowed.\n\nFor more details, see: https://kubernetes.io/docs/tasks/administer-cluster/kubelet-credential-provider/ https://github.com/cri-o/crio-credential-provider#architecture\n\nEach entry in matchImages is a pattern which can optionally contain a port and a path. Each entry must be no longer than 512 characters. Wildcards ('*') are supported for full subdomain labels, such as '*.k8s.io' or 'k8s.*.io', and for top-level domains, such as 'k8s.*' (which matches 'k8s.io' or 'k8s.net'). A global wildcard '*' (matching any domain) is not allowed. Wildcards may replace an entire hostname label (e.g., *.example.com), but they cannot appear within a label (e.g., f*oo.example.com) and are not allowed in the port or path. For example, 'example.*.com' is valid, but 'exa*mple.*.com' is not. Each wildcard matches only a single domain label, so '*.io' does **not** match '*.k8s.io'.\n\nA match exists between an image and a matchImage when all of the below are true: Both contain the same number of domain parts and each part matches. The URL path of an matchImages must be a prefix of the target image URL path. If the matchImages contains a port, then the port must match in the image as well.\n\nExample values of matchImages: - 123456789.dkr.ecr.us-east-1.amazonaws.com - *.azurecr.io - gcr.io - *.*.registry.io - registry.io:8080/path",
	}
	func (CRIOCredentialProviderConfigSpec) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfigSpec
	}
	var map_CRIOCredentialProviderConfigStatus = map[string]string{
	"": "CRIOCredentialProviderConfigStatus defines the observed state of CRIOCredentialProviderConfig",
	"conditions": "conditions represent the latest available observations of the configuration state. When omitted, it indicates that no conditions have been reported yet. The maximum number of conditions is 16. Conditions are stored as a map keyed by condition type, ensuring uniqueness.\n\nExpected condition types include: \"Validated\": indicates whether the matchImages configuration is valid",
	}
	func (CRIOCredentialProviderConfigStatus) SwaggerDoc() map[string]string {
	return map_CRIOCredentialProviderConfigStatus
	}

🤖 Prompt for AI Agents

In config/v1alpha1/zz_generated.swagger_doc_generated.go around lines 229 to
265, the CRIOCredentialProviderConfigList swagger map is missing the "items"
entry; add an "items" key to map_CRIOCredentialProviderConfigList with a short
description like: "items is the list of CRIOCredentialProviderConfig objects"
(matching the style of other *List docs), so the generated Swagger docs include
the list contents description.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (3)

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1)

114-120: Relax matchImages path regex to allow . in repository paths
Current regex forbids dots in the optional path (/[-a-z0-9_/]*), which can block valid/common repo names. (This was raised previously.)

-                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9_/]*)?$')
+                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9._/]*)?$')

openapi/generated_openapi/zz_generated.openapi.go (1)

21535-21705: Reintroduced: matchImages/conditions array limits described but not enforced in schema. The generated schema lacks MinItems/MaxItems for matchImages (1..50) and MaxItems for status.conditions (16), even though the descriptions require them—this is likely a source-marker or generator/regeneration issue (and was previously addressed).

Please fix via the source Go types + regeneration (don’t patch this generated file). To verify quickly in-repo:

#!/bin/bash
set -euo pipefail

# Expect to see MinItems/MaxItems near matchImages and MaxItems near conditions.
rg -n "matchImages|conditions" openapi/generated_openapi/zz_generated.openapi.go -n -C2

echo "---- matchImages constraints ----"
rg -n "matchImages" openapi/generated_openapi/zz_generated.openapi.go -n -A40 | rg -n "MinItems|MaxItems|x-kubernetes-list-type" || true

echo "---- conditions constraints ----"
rg -n "\"conditions\"" openapi/generated_openapi/zz_generated.openapi.go -n -A40 | rg -n "MaxItems|x-kubernetes-list-type" || true

openapi/openapi.json (1)

11785-11820: Add missing array bounds in generated schema (spec.matchImages, status.conditions)
This hunk still lacks minItems/maxItems on matchImages and maxItems on status.conditions, so empty/unbounded lists remain valid at the OpenAPI layer. Please ensure codegen emits minItems: 1, maxItems: 50 for matchImages and maxItems: 4 for conditions (likely via kubebuilder markers on the Go types).

📜 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 07c6dc5 and 267555a.

📒 Files selected for processing (24)

• config/v1alpha1/register.go (1 hunks)
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/types_crio_credential_provider_config.go (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.deepcopy.go (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/zz_generated.swagger_doc_generated.go (1 hunks)
• features.md (1 hunks)
• features/features.go (1 hunks)
• hack/update-payload-crds.sh (1 hunks)
• openapi/generated_openapi/zz_generated.openapi.go (2 hunks)
• openapi/openapi.json (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (11)

• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
• hack/update-payload-crds.sh
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
• features.md
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• config/v1alpha1/types_crio_credential_provider_config.go

🧰 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:

• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• config/v1alpha1/register.go
• openapi/generated_openapi/zz_generated.openapi.go
• features/features.go
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.swagger_doc_generated.go
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• openapi/openapi.json
• config/v1alpha1/zz_generated.deepcopy.go

🧬 Code graph analysis (4)

config/v1alpha1/register.go (2)

config/v1alpha1/types_crio_credential_provider_config.go (2)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)

config/v1alpha1/zz_generated.swagger_doc_generated.go (2)

• CRIOCredentialProviderConfig (236-238)
• CRIOCredentialProviderConfigList (245-247)

features/features.go (1)

config/v1/types_feature.go (2)

• DevPreviewNoUpgrade (49-49)
• TechPreviewNoUpgrade (45-45)

config/v1alpha1/zz_generated.swagger_doc_generated.go (1)

config/v1alpha1/types_crio_credential_provider_config.go (4)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)
• CRIOCredentialProviderConfigSpec (46-94)
• CRIOCredentialProviderConfigStatus (117-131)

config/v1alpha1/zz_generated.deepcopy.go (2)

config/v1alpha1/types_crio_credential_provider_config.go (5)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)
• CRIOCredentialProviderConfigSpec (46-94)
• MatchImage (112-112)
• CRIOCredentialProviderConfigStatus (117-131)

config/v1alpha1/zz_generated.swagger_doc_generated.go (4)

• CRIOCredentialProviderConfig (236-238)
• CRIOCredentialProviderConfigList (245-247)
• CRIOCredentialProviderConfigSpec (254-256)
• CRIOCredentialProviderConfigStatus (263-265)

🔇 Additional comments (11)

config/v1alpha1/zz_generated.deepcopy.go (1)

195-299: Generated DeepCopy implementations look correct for the new CRIOCredentialProviderConfig types

payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)

101-108: FeatureGate enablement update is consistent

payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)

52-65: Default Hypershift gating (disabled) matches expected rollout posture

config/v1alpha1/register.go (1)

30-47: Scheme registration for CRIOCredentialProviderConfig types looks correct

features/features.go (1)

134-140: String "CRIOCredentialProviderConfig" is consistent across code and payload manifests

All references to the feature gate string maintain consistent casing and spelling across type definitions, CRD manifests, feature gate configurations, and generated code. No typos or case drift detected.

config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1)

1-214: LGTM! CRD validation design is sound.

The CRD manifest is well-structured and addresses previous review concerns:

• The singleton enforcement via CEL validation (lines 208-210) correctly restricts the resource name to "cluster"
• The matchImages regex validation (line 119) has been verified through tests to correctly accept valid domain patterns with wildcards, ports, and paths
• The status constraints are semantically appropriate:
  • status is optional and can be omitted for new resources
  • minProperties: 1 (line 132) ensures that if status is provided, it contains at least the conditions field
  • minItems: 1 (line 199) ensures that if conditions are provided, at least one condition is present
  • This prevents empty status objects while allowing controllers to properly initialize status with meaningful conditions

The validation rules align with the documented behavior that status can be omitted initially and will be populated by controllers with validation conditions.

config/v1alpha1/zz_generated.swagger_doc_generated.go (1)

229-265: LGTM! Swagger documentation is complete and follows established patterns.

The generated Swagger documentation for CRIOCredentialProviderConfig and its related types is comprehensive and accurate:

• All four types (CRIOCredentialProviderConfig, CRIOCredentialProviderConfigList, CRIOCredentialProviderConfigSpec, CRIOCredentialProviderConfigStatus) have proper documentation maps
• Descriptions are detailed and match the CRD schema definitions
• The documentation clearly explains the singleton nature, matchImages validation requirements, and status conditions

The code follows the established pattern in this generated file. While the map_CRIOCredentialProviderConfigList could include an "items" entry for additional completeness (similar to map_ClusterMonitoringList at line 169), this is consistent with most other *List types in this file which also omit that entry, and was noted in previous reviews.

openapi/generated_openapi/zz_generated.openapi.go (1)

428-434: Registry wiring for new OpenAPI definitions looks consistent.

payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (3)

127-203: Clarify status constraints and conditions array handling.

The status schema combines minProperties: 1 (line 132) with minItems: 1 on the conditions array (line 198), which means status can only exist in one of two valid states: absent, or present with at least one condition. However, the description at lines 136–137 states "When omitted, it indicates that no conditions have been reported yet," which could be misunderstood as allowing an empty conditions array or empty status object {}.

For clarity and to align with the intended controller behavior, consider:

1. If this constraint is intentional (status must always contain conditions): update the description to explicitly state that status should only be present when there is at least one condition to report, or use a CEL validation rule to document this pattern.
2. If empty status should be allowed: either remove minProperties: 1 or set minItems: 0 on the conditions array, and update descriptions accordingly.

The current schema is technically consistent, but the wording could mislead controller implementations into attempting invalid configurations.

---

50-126: matchImages validation and constraints are well-designed.

The spec validation rules are comprehensive and appropriate:

• Regex pattern (line 119) correctly enforces lowercase FQDN format with optional wildcards, ports, and paths
• Wildcard constraints (line 116) prevent unsafe global matches
• Array bounds (minItems: 1, maxItems: 50) and string length limits (maxLength: 512) are reasonable
• x-kubernetes-list-type: set (line 123) prevents duplicates

---

207-210: Singleton constraint enforcement is correct.

The CEL validation rule properly enforces the singleton pattern by requiring metadata.name == 'cluster', which aligns with the resource description and prevents accidental creation of multiple instances.

[coderabbitai]

Actionable comments posted: 3

♻️ Duplicate comments (9)

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1)

114-119: Fix matchImages path validation to allow dots in repository paths.

The regex pattern on line 119 restricts the path component to [-a-z0-9_/]*, excluding dots. Repository paths commonly include dots (e.g., .tar, .tgz). This restriction is unnecessarily strict.

Update the path character class to include dots, matching the fix needed in the DevPreviewNoUpgrade variant:

-                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9_/]*)?$')
+                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9._/]*)?$')

Ensure both DevPreviewNoUpgrade and TechPreviewNoUpgrade CRD variants receive this fix for consistency.

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

114-123: Allow dots in matchImages path component for repository paths.

The regex pattern at line 119 disallows dots in the optional path segment: (/[-a-z0-9_/]*) does not include \.. This rejects legitimate image references like registry.io/my.package/image, which commonly appear in container registries (e.g., Python packages, npm scopes with dots).

Update the path character class to include dots:

-                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9_/]*)?$')
+                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9._/]*)?$')

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

114-123: Allow dots in matchImages path component for repository paths.

The regex pattern at line 119 disallows dots in the optional path segment: (/[-a-z0-9_/]*) does not include \.. This rejects legitimate image references like registry.io/my.package/image, which commonly appear in container registries (e.g., Python packages, npm scopes with dots).

Update the path character class to include dots:

-                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9_/]*)?$')
+                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9._/]*)?$')

openapi/generated_openapi/zz_generated.openapi.go (2)

21636-21668: matchImages schema missing MinItems/MaxItems vs documented 1..50 contract. This allows empty/unbounded arrays despite the description. Since this is generated output, fix via kubebuilder markers on the source type and regenerate; if generation still omits these, the openapi generation tooling/version likely isn’t honoring the markers.

 					"matchImages": {
 						VendorExtensible: spec.VendorExtensible{
 							Extensions: spec.Extensions{
 								"x-kubernetes-list-type": "set",
 							},
 						},
 						SchemaProps: spec.SchemaProps{
 							Description: "matchImages is a required list ... This field is required and must contain between 1 and 50 entries. ...",
 							Type:        []string{"array"},
+							MinItems:    1,
+							MaxItems:    50,
 							Items: &spec.SchemaOrArray{
 								Schema: &spec.Schema{
 									SchemaProps: spec.SchemaProps{
 										Default: "",
 										Type:    []string{"string"},
 										Format:  "",
 									},
 								},
 							},
 						},
 					},

---

21670-21705: conditions schema missing MaxItems=16 vs documented limit. This makes the status array effectively unbounded. Fix at source + regenerate; if generation still omits it, treat as a tooling/generator gap.

 					"conditions": {
 						VendorExtensible: spec.VendorExtensible{
 							Extensions: spec.Extensions{
 								"x-kubernetes-list-map-keys": []interface{}{
 									"type",
 								},
 								"x-kubernetes-list-type": "map",
 							},
 						},
 						SchemaProps: spec.SchemaProps{
 							Description: "conditions represent ... The maximum number of conditions is 16. ...",
 							Type:        []string{"array"},
+							MaxItems:    16,
 							Items: &spec.SchemaOrArray{
 								Schema: &spec.Schema{
 									SchemaProps: spec.SchemaProps{
 										Default: map[string]interface{}{},
 										Ref:     ref("k8s.io/apimachinery/pkg/apis/meta/v1.Condition"),
 									},
 								},
 							},
 						},
 					},

config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1)

114-119: Verify if path segment restriction is intentional.

The matchImages validation regex at line 119 uses the character class [-a-z0-9_/] for the path portion, which does not include . (dot). This means patterns like registry.io/project.v2/subpath would be rejected.

A previous review flagged this same concern and was marked as "Addressed in commit a31eec1", but the regex still excludes dots from path segments. Please confirm whether:

1. This restriction is intentional (paths with dots are not supported by design), or
2. The fix was incomplete and the regex should be updated to /[-a-z0-9._/]* to allow dots in paths

Container image repository paths commonly include dots (per OCI naming conventions), so this limitation may surprise users and block valid configurations unless it's a deliberate design choice.

Based on past review comments.

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

114-119: Verify if path segment restriction is intentional.

The matchImages validation regex at line 119 uses the character class [-a-z0-9_/] for the path portion, which does not include . (dot). This means patterns like registry.io/project.v2/subpath would be rejected.

This is the same regex pattern as in the CustomNoUpgrade variant. Please confirm whether:

1. This restriction is intentional (paths with dots are not supported by design), or
2. The regex should be updated to /[-a-z0-9._/]* to allow dots in paths

Container image repository paths commonly include dots (per OCI naming conventions), so this limitation may surprise users and block valid configurations unless it's a deliberate design choice.

Based on past review comments.

openapi/openapi.json (2)

11792-11800: [Duplicate] Missing array bounds validation on matchImages.

This issue was already flagged in a previous review. The matchImages array schema lacks minItems: 1 and maxItems: 50 constraints despite PR requirements specifying "1–50 unique strings". The array definition currently only includes x-kubernetes-list-type: "set" for uniqueness.

Add the missing cardinality constraints:

         "matchImages": {
           "description": "...",
           "type": "array",
+          "minItems": 1,
+          "maxItems": 50,
           "items": {
             "type": "string",
             "default": ""
           },
           "x-kubernetes-list-type": "set"
         }

Ensure the corresponding Go struct in config/v1alpha1/types_crio_credential_provider_config.go includes +kubebuilder:validation:MinItems=1 and +kubebuilder:validation:MaxItems=50 markers so codegen produces these OpenAPI constraints.

---

11807-11818: [Duplicate] Missing maxItems constraint on conditions array.

This issue was already flagged in a previous review. The conditions array schema in the status lacks the maxItems: 4 constraint despite PR requirements specifying "up to 4" conditions. The array definition currently only includes list map configuration for type-based keying.

Add the missing cardinality constraint:

         "conditions": {
           "description": "conditions represent the latest available observations of the configuration state",
           "type": "array",
+          "maxItems": 4,
           "items": {
             "default": {},
             "$ref": "#/definitions/io.k8s.apimachinery.pkg.apis.meta.v1.Condition"
           },
           "x-kubernetes-list-map-keys": [
             "type"
           ],
           "x-kubernetes-list-type": "map"
         }

Ensure the corresponding Go struct includes +kubebuilder:validation:MaxItems=4 marker so codegen produces this constraint in the OpenAPI schema.

📜 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 267555a and da5c4a2.

📒 Files selected for processing (26)

• config/v1alpha1/register.go (1 hunks)
• config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/types_crio_credential_provider_config.go (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• config/v1alpha1/zz_generated.deepcopy.go (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml (1 hunks)
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1 hunks)
• config/v1alpha1/zz_generated.swagger_doc_generated.go (1 hunks)
• features.md (1 hunks)
• features/features.go (1 hunks)
• hack/update-payload-crds.sh (1 hunks)
• openapi/generated_openapi/zz_generated.openapi.go (2 hunks)
• openapi/openapi.json (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-OKD.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-OKD.yaml (1 hunks)
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (10)

• config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
• features/features.go
• hack/update-payload-crds.sh
• config/v1alpha1/register.go
• payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
• config/v1alpha1/types_crio_credential_provider_config.go
• features.md
• config/v1alpha1/zz_generated.swagger_doc_generated.go
• payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.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/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• openapi/generated_openapi/zz_generated.openapi.go
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• payload-manifests/featuregates/featureGate-Hypershift-OKD.yaml
• config/v1alpha1/zz_generated.crd-manifests/0000_10_config-operator_01_criocredentialproviderconfigs-DevPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml
• payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
• payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-TechPreviewNoUpgrade.crd.yaml
• config/v1alpha1/zz_generated.deepcopy.go
• payload-manifests/crds/0000_10_config-operator_01_criocredentialproviderconfigs-CustomNoUpgrade.crd.yaml
• openapi/openapi.json
• payload-manifests/featuregates/featureGate-SelfManagedHA-OKD.yaml

🧬 Code graph analysis (1)

config/v1alpha1/zz_generated.deepcopy.go (2)

config/v1alpha1/types_crio_credential_provider_config.go (5)

• CRIOCredentialProviderConfig (25-43)
• CRIOCredentialProviderConfigList (139-147)
• CRIOCredentialProviderConfigSpec (46-94)
• MatchImage (112-112)
• CRIOCredentialProviderConfigStatus (117-131)

config/v1alpha1/zz_generated.swagger_doc_generated.go (4)

• CRIOCredentialProviderConfig (236-238)
• CRIOCredentialProviderConfigList (245-247)
• CRIOCredentialProviderConfigSpec (254-256)
• CRIOCredentialProviderConfigStatus (263-265)

🔇 Additional comments (11)

payload-manifests/featuregates/featureGate-Hypershift-OKD.yaml (1)

59-61: Feature gate entry correctly positioned in alphabetical order.

Insertion into the disabled list is properly placed between CRDCompatibilityRequirementOperator and ClientsAllowCBOR, maintaining alphabetical order.

payload-manifests/featuregates/featureGate-SelfManagedHA-OKD.yaml (1)

59-61: Feature gate entry correctly positioned.

Consistent with other OKD feature gate files. Entry is properly placed in the disabled list in alphabetical order.

payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)

103-105: Feature gate correctly enabled for DevPreviewNoUpgrade.

Entry is properly placed in the enabled list in alphabetical order, consistent with the DevPreviewNoUpgrade feature set semantics.

payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1)

133-135: Feature gate correctly enabled for TechPreviewNoUpgrade.

Entry properly positioned in the enabled list in alphabetical order, consistent with TechPreviewNoUpgrade feature gating.

payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1)

118-120: Feature gate entry correctly positioned for DevPreviewNoUpgrade.

Properly placed in alphabetical order within the enabled list for Hypershift DevPreviewNoUpgrade.

config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1)

1-92: Test cases properly structured with comprehensive validation coverage.

The YAML structure is correct (no nesting issues as were fixed in commit 514adaf), and the seven test cases comprehensively cover the matchImages validation rules: valid registries, invalid characters, uppercase enforcement, wildcard constraints, and global wildcard rejection. Test case error messages align with the CRD validation rules.

config/v1alpha1/zz_generated.featuregated-crd-manifests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml (1)

127-203: LGTM: Status structure is properly optional and well-defined.

The status field is correctly optional (not in the required array), with minProperties: 1 ensuring that when present it's not empty. The conditions array has minItems: 1, preventing empty arrays, and the description correctly states "When omitted, it indicates that no conditions have been reported yet."—this aligns with the schema constraints.

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

127-203: LGTM: Status structure is properly optional and well-defined.

The status field is correctly optional (not in the required array), with minProperties: 1 ensuring that when present it's not empty. The conditions array has minItems: 1, preventing empty arrays, and the description correctly states "When omitted, it indicates that no conditions have been reported yet."—this aligns with the schema constraints.

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

127-203: LGTM: Status structure is properly optional and well-defined.

The status field is correctly optional (not in the required array), with minProperties: 1 ensuring that when present it's not empty. The conditions array has minItems: 1, preventing empty arrays, and the description correctly states "When omitted, it indicates that no conditions have been reported yet."—this aligns with the schema constraints.

openapi/generated_openapi/zz_generated.openapi.go (1)

428-434: OpenAPI registry wiring for new CRIOCredentialProviderConfig types looks correct.

config/v1alpha1/zz_generated.deepcopy.go (1)

195-298: LGTM! Generated deepcopy implementations are correct.

The auto-generated deepcopy methods for the new CRIOCredentialProviderConfig types follow the standard patterns and include proper nil checks, correct field copying, and appropriate runtime.Object implementations. All slice fields (MatchImages, Conditions, Items) are handled correctly with allocation and element-by-element deep copying.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix matchImages path validation to allow dots in repository paths.

The regex pattern on line 119 restricts the path component to [-a-z0-9_/]*, which excludes dots (.). This is overly restrictive—repository paths commonly include dots (e.g., /my.repo/image.tar).

Update the path character class to include dots:

-                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9_/]*)?$')
+                    rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9._/]*)?$')

This aligns with the documented intent to support arbitrary repository paths. Based on learnings, this is a known issue flagged in previous reviews that should be corrected.

📝 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.

Suggested change

	x-kubernetes-validations:
	- message: global wildcard '*' is not allowed
	rule: self != '*'
	- message: invalid matchImages value, must be a valid fully qualified
	domain name in lowercase with optional wildcard, port, and path
	rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9_/]*)?$')
	x-kubernetes-validations:
	- message: global wildcard '*' is not allowed
	rule: self != '*'
	- message: invalid matchImages value, must be a valid fully qualified
	domain name in lowercase with optional wildcard, port, and path
	rule: self.matches('^((\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?)(\\.(\\*|[a-z0-9]([a-z0-9-]*[a-z0-9])?))*)(:[0-9]+)?(/[-a-z0-9._/]*)?$')

[QiWang19]

@JoelSpeed Is it mandatory to ship a default singleton resource? If we have to ship a default singleton manifest, it will be an empty spec since we don't have meaningful default values needed for default.
I think if we have to ship this default resource, spec.matchImages should be optional to pass the validation?

[JoelSpeed]

No, I think it can be off by default and have it created only when the user wants something to actually happen

[QiWang19]

@JoelSpeed The openshift has implementaiton does not allow oc delete cluster resources. And the API cel validation does not allow "" spec.matchImages.

As a result, a user cannot delete the entire configuration they added via the CR.
Would it makes sense to let users clear their configuration, even if the CR itself is cluster and cannot be deleted? Probably we can allow spec.matchImages to be empty.

Although from the underlying implementation, once the secret obejct(s) of the matchImages entry is deleted, the effective result is equivalent to removing item(s) from spec.matchImages.

$ oc delete criocredentialproviderconfig/cluster
Error from server (Forbidden): criocredentialproviderconfigs.config.openshift.io "cluster" is forbidden: deleting required criocredentialproviderconfigs.config.openshift.io resource, named cluster, is not allowed

[JoelSpeed]

One nit, otherwise LGTM, thanks @QiWang19

Hows the EP for this feature going? Has it been merged?

[JoelSpeed]

Needs omitzero here please

Suggested change

	metav1.ObjectMeta `json:"metadata"`
	metav1.ObjectMeta `json:"metadata,omitzero"`

[QiWang19]

/retest-required

[QiWang19]

Hows the EP for this feature going? Has it been merged?

Hi @JoelSpeed, the enhancement PR got merged. openshift/enhancements#1861. we will need to followup on the enhancement to address the CRIOCredentialProvider resource a singleton.

[JoelSpeed]

/lgtm

[openshift-ci-robot]

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]

@QiWang19: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/okd-scos-e2e-aws-ovn	cc9c041	link	false	/test okd-scos-e2e-aws-ovn
ci/prow/e2e-aws-ovn	e504b5e	link	true	/test e2e-aws-ovn
ci/prow/okd-scos-images	e504b5e	link	true	/test okd-scos-images
ci/prow/e2e-aws-ovn-hypershift-conformance	e504b5e	link	true	/test e2e-aws-ovn-hypershift-conformance

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.

[QiWang19]

@JoelSpeed WDYT? The openshift has implementaiton does not allow oc delete cluster resources. And the API cel validation does not allow "" spec.matchImages.

As a result, a user cannot delete the entire configuration they added via the CR.
Would it makes sense to let users clear their configuration, even if the CR itself is cluster and cannot be deleted? Probably we can allow spec.matchImages to be empty.

Although from the underlying implementation, once the secret obejct(s) of the matchImages entry is deleted, the effective result is equivalent to removing item(s) from spec.matchImages.

$ oc delete criocredentialproviderconfig/cluster
Error from server (Forbidden): criocredentialproviderconfigs.config.openshift.io "cluster" is forbidden: deleting required criocredentialproviderconfigs.config.openshift.io resource, named cluster, is not allowed

[JoelSpeed]

@QiWang19 So this goes against some of our previous conversation, but good find! Do you have any idea where this is implemented? I wonder if there's a way to add an exception since we expect this cluster resource to not always exist.

Else yes, we will need to allow the spec to be removed for this particular resource

[QiWang19]

@JoelSpeed That sounds good, we can add an exception at the code path
https://github.com/openshift/kubernetes/blob/76cb1db555ed7a57f12025018d54622ef2e9458c/openshift-kube-apiserver/admission/customresourcevalidation/config/deny_delete_cluster_config_resource.go#L31

[QiWang19]

Verified correct code path we can add exception to allow delete criocredentialproviderconfig.config.openshift.io/cluster openshift/kubernetes#2542

on cluster 4.22.0-0.ci-2025-12-18-101944, openshift/kubernetes#2542, openshift/api#2557:

qiwan@fedora:~$ oc create -f /home/qiwan/Documents/test-crds/criocredentialprovider/criocp.yaml
criocredentialproviderconfig.config.openshift.io/cluster created


qiwan@fedora:~$ oc describe criocredentialproviderconfig.config.openshift.io/cluster
Name:         cluster
Namespace:    
Labels:       <none>
Annotations:  <none>
API Version:  config.openshift.io/v1alpha1
Kind:         CRIOCredentialProviderConfig
Metadata:
  Creation Timestamp:  2025-12-18T19:50:35Z
  Generation:          1
  Resource Version:    38292
  UID:                 4ef0e05d-d875-46c9-b467-804513093041
Spec:
  Match Images:
    123456789.dkr.ecr.us-east-1.amazonaws.com
    *.azurecr.io
    *.*.registry.io
    registry.io:8080/path
Events:  <none>


qiwan@fedora:~$ oc delete criocredentialproviderconfig.config.openshift.io/cluster
criocredentialproviderconfig.config.openshift.io "cluster" deleted

[JoelSpeed]

/lgtm cancel
/approve cancel

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: saschagrunert
Once this PR has been reviewed and has the lgtm label, please ask for approval from joelspeed. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[QiWang19]

/verified by tests api/config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml

[openshift-ci-robot]

@QiWang19: This PR has been marked as verified by tests api/config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml``.

Details

In response to this:

/verified by tests api/config/v1alpha1/tests/criocredentialproviderconfigs.config.openshift.io/CRIOCredentialProviderConfig.yaml

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.

[JoelSpeed]

@QiWang19 Can you try and get the OC PR merged before we merge this one please?

[QiWang19]

Can you try and get the OC PR merged before we merge this one please?

ok, openshift/kubernetes#2549 is open.