SSCSI-245: Add Secrets Store CSI driver configuration to ClusterCSIDriver API#2846
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Hello @chiragkyal! Some important instructions when contributing to openshift/api: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request extends the CSI driver configuration API to support Secrets Store drivers. The Go API type CSIDriverType gains a new SecretsStore constant, CSIDriverConfigSpec gains a new secretsStore union field, and three new configuration struct types are introduced: SecretsStoreCSIDriverConfigSpec, SecretsStoreSecretRotation, and SecretsStoreTokenRequest. The corresponding CRD schemas in all five cluster-profile variants are updated with matching enum extensions, secretsStore schema definitions, and x-kubernetes-validations that require secretsStore presence only when driverType is SecretsStore. ChangesSecrets Store CSI driver support
🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
size/XL
|
/test all |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@operator/v1/types_csi_cluster_driver.go`:
- Around line 405-411: Update the field comments for TokenRequests (type
SecretsStoreTokenRequest) and the expirationSeconds field to document the
default behavior when omitted: state that if TokenRequests is nil/empty, no
service account tokens will be requested and the CSI driver will use the
kube-apiserver's default APIAudiences (or no tokens provided), and if
expirationSeconds is omitted the token lifetime defaults to the kubelet/cluster
provider default (or a specific default value used by the implementation).
Modify the comment blocks adjacent to the TokenRequests declaration and the
expirationSeconds declaration to include these omission semantics and expected
default values/behavior so the optional nature is clear.
- Around line 162-165: The new stable API field SecretsStore
(*SecretsStoreCSIDriverConfigSpec) must be gated with an OpenShift feature gate;
add the kubebuilder marker comment
+openshift:enable:FeatureGate=YourFeatureGateName immediately above the
`SecretsStore` field declaration (the SecretsStore
*SecretsStoreCSIDriverConfigSpec line) so the field is only enabled when the
feature gate is on, and update any related docs/tests to use that feature gate
name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c45c7b74-a5df-45a7-8fef-3c1a81dc9f5c
⛔ Files ignored due to path filters (11)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AWSEuropeanSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/VSphereConfigurableMaxAllowedBlockVolumesPerNode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (6)
operator/v1/types_csi_cluster_driver.gopayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yaml
chiragkyal
force-pushed
the
configure-secret-rotation-and-wif
branch
from
dd82f59 to
87f2c52
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
chiragkyal
force-pushed
the
configure-secret-rotation-and-wif
branch
2 times, most recently
from
eb0e5bd to
e08f398
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yaml`:
- Around line 310-317: The schema for the CRD field expirationSeconds does not
enforce the documented lower bound of 600; update the OpenAPI schema for the
property named expirationSeconds in the CRD (within the properties block where
type: integer and format: int64 are defined) to include minimum: 600 so
admission validation will reject values below 600; ensure the minimum is set
alongside the existing type/format entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 02b1535f-ee7f-421e-b9f7-a4e43b5dd6c5
⛔ Files ignored due to path filters (11)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AWSEuropeanSovereignCloudInstall.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/VSphereConfigurableMaxAllowedBlockVolumesPerNode.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (6)
operator/v1/types_csi_cluster_driver.gopayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yaml
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yaml
- operator/v1/types_csi_cluster_driver.go
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yaml
|
/cc @mytreya-rh |
chiragkyal
force-pushed
the
configure-secret-rotation-and-wif
branch
from
e08f398 to
4836779
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
operator/v1/types_csi_cluster_driver.go (1)
116-125:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftGate the new stable
SecretsStoreAPI surface.Lines 116-125 and 162-164 make
SecretsStorepart of the stable v1 discriminator/union, and the generated manifests in this PR already carry it into the ungated/default CRDs. If this is not intentionally GA-on-arrival, it needs to be feature-gated at the API layer so the stable surface does not expand for every cluster profile yet.As per coding guidelines,
**/types*.go: New fields on stable APIs should be introduced behind a feature gate using+openshift:enable:FeatureGate=MyFeatureGate.Also applies to: 162-164
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator/v1/types_csi_cluster_driver.go` around lines 116 - 125, The SecretsStore driver constant was added to the stable CSIDriverType union and must be feature-gated; add the openshift feature-gate annotation to the API so the new discriminator is not GA-on-arrival: locate the CSIDriverType declaration and the SecretsStoreDriverType constant (symbols: CSIDriverType and SecretsStoreDriverType) and add the kubebuilder/openshift annotation `+openshift:enable:FeatureGate=SecretsStoreCSIDriver` (or your chosen gate name) immediately above the relevant type/constant declarations (also apply the same annotation to the other occurrence referenced around lines 162-164) so CRD generation treats SecretsStore as gated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@operator/v1/types_csi_cluster_driver.go`:
- Around line 454-459: The ExpirationSeconds field on the CSIClusterDriver type
lacks the kubebuilder validation enforcing the documented minimum of 600
seconds; update the field tag for ExpirationSeconds (*int64
`json:"expirationSeconds,omitempty"`) to include
+kubebuilder:validation:Minimum=600 (and keep +optional) so the generated CRD
will reject values <600 and match the documented contract.
In
`@operator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/VSphereConfigurableMaxAllowedBlockVolumesPerNode.yaml`:
- Around line 289-296: The schema for the expirationSeconds field currently only
declares type/format but does not enforce the documented lower bound; update the
CRD manifest so the expirationSeconds property includes a numeric minimum of 600
(e.g. add "minimum: 600" alongside the existing "type: integer" and "format:
int64") to ensure values less than 600 are rejected by validation; locate the
expirationSeconds definition in the
VSphereConfigurableMaxAllowedBlockVolumesPerNode.yaml and add the minimum
constraint for that property.
---
Duplicate comments:
In `@operator/v1/types_csi_cluster_driver.go`:
- Around line 116-125: The SecretsStore driver constant was added to the stable
CSIDriverType union and must be feature-gated; add the openshift feature-gate
annotation to the API so the new discriminator is not GA-on-arrival: locate the
CSIDriverType declaration and the SecretsStoreDriverType constant (symbols:
CSIDriverType and SecretsStoreDriverType) and add the kubebuilder/openshift
annotation `+openshift:enable:FeatureGate=SecretsStoreCSIDriver` (or your chosen
gate name) immediately above the relevant type/constant declarations (also apply
the same annotation to the other occurrence referenced around lines 162-164) so
CRD generation treats SecretsStore as gated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dc4e1222-9530-4acb-9ae8-509a500a44a1
⛔ Files ignored due to path filters (3)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!**/zz_generated*operator/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (14)
operator/v1/types_csi_cluster_driver.gooperator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamloperator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamloperator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamloperator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamloperator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yamloperator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AAA_ungated.yamloperator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/AWSEuropeanSovereignCloudInstall.yamloperator/v1/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/VSphereConfigurableMaxAllowedBlockVolumesPerNode.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-Default.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yamlpayload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-CustomNoUpgrade.crd.yaml
chiragkyal
force-pushed
the
configure-secret-rotation-and-wif
branch
from
4836779 to
d7c890b
Compare
openshift-ci
Bot
added
size/XXL
chiragkyal
force-pushed
the
configure-secret-rotation-and-wif
branch
2 times, most recently
from
baa69d9 to
7a40d90
Compare
chiragkyal
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@chiragkyal: This pull request references SSCSI-245 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Scheduling tests matching the |
openshift-ci
Bot
added
the
approved
|
/retest |
1 similar comment
|
/retest |
openshift-ci
Bot
added
the
needs-rebase
Extends CSIDriverConfigSpec with a new SecretsStore discriminated union variant containing secretRotation and tokenRequests fields. Signed-off-by: chiragkyal <ckyal@redhat.com>
Signed-off-by: chiragkyal <ckyal@redhat.com>
chiragkyal
force-pushed
the
configure-secret-rotation-and-wif
branch
from
ff412df to
6f5089f
Compare
openshift-ci
Bot
removed
lgtm
|
/lgtm /testwith openshift/api/master/verify-client-go openshift/client-go#381 |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/verified by ci |
openshift-ci-robot
added
the
verified
|
@chiragkyal: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
|
/retest-required |
|
@chiragkyal: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Summary
Adds
SecretsStoreas a newCSIDriverTypeenum value in theClusterCSIDriverAPI (operator.openshift.io/v1)Introduces
SecretsStoreCSIDriverConfigSpecwith configuration for:the poll interval (defaults to enabled with 2m interval)
workload identity federation (WIF)
Implements: SSCSI-254: Configurable secret rotation and WIF support for SSCSI enhancements#2012
Summary by CodeRabbit
SecretsStoreCSI driver type.SecretsStoredriver type.