Add Secrets Store CSI driver configuration to ClusterCSIDriver API#2846
Add Secrets Store CSI driver configuration to ClusterCSIDriver API#2846chiragkyal wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Hello @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:
WalkthroughThis 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
openshift-ci
Bot
added
the
size/XL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/test 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
| // secretsStore is used to configure the Secrets Store CSI driver. | ||
| // +optional | ||
| SecretsStore *SecretsStoreCSIDriverConfigSpec `json:"secretsStore,omitempty"` | ||
| } |
There was a problem hiding this comment.
Gate the new stable v1 field behind a feature gate.
Line 162 introduces a new stable API field without a +openshift:enable:FeatureGate=... marker. That violates the stable API rollout requirement and can expose partially-supported config in GA surface.
Suggested direction
// secretsStore is used to configure the Secrets Store CSI driver.
+ // +openshift:enable:FeatureGate=SecretsStoreCSIDriver
// +optional
SecretsStore *SecretsStoreCSIDriverConfigSpec `json:"secretsStore,omitempty"`As per coding guidelines, **/types*.go: New fields on stable APIs should be introduced behind a feature gate using +openshift:enable:FeatureGate=MyFeatureGate.
📝 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.
| // secretsStore is used to configure the Secrets Store CSI driver. | |
| // +optional | |
| SecretsStore *SecretsStoreCSIDriverConfigSpec `json:"secretsStore,omitempty"` | |
| } | |
| // secretsStore is used to configure the Secrets Store CSI driver. | |
| // +openshift:enable:FeatureGate=SecretsStoreCSIDriver | |
| // +optional | |
| SecretsStore *SecretsStoreCSIDriverConfigSpec `json:"secretsStore,omitempty"` | |
| } |
🤖 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 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.
There was a problem hiding this comment.
This is a day2 operator, not an OpenShift core operator. I think we can add this feature to GA directly.
@mytreya-rh what do you think?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
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
| expirationSeconds: | ||
| description: |- | ||
| expirationSeconds is the requested duration of validity of the service account token. | ||
| The token issuer may return a token with a different validity duration. | ||
| When omitted, the token expiration is determined by the kube-apiserver. | ||
| Must be at least 600 seconds (10 minutes). | ||
| format: int64 | ||
| type: integer |
There was a problem hiding this comment.
Enforce the documented lower bound for token expiration.
Line [315] says expirationSeconds must be at least 600, but the schema at Lines [310-317] doesn’t enforce it. This allows invalid configs through admission.
Proposed fix
expirationSeconds:
description: |-
expirationSeconds is the requested duration of validity of the service account token.
The token issuer may return a token with a different validity duration.
When omitted, the token expiration is determined by the kube-apiserver.
Must be at least 600 seconds (10 minutes).
format: int64
+ minimum: 600
type: integer📝 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.
| expirationSeconds: | |
| description: |- | |
| expirationSeconds is the requested duration of validity of the service account token. | |
| The token issuer may return a token with a different validity duration. | |
| When omitted, the token expiration is determined by the kube-apiserver. | |
| Must be at least 600 seconds (10 minutes). | |
| format: int64 | |
| type: integer | |
| expirationSeconds: | |
| description: |- | |
| expirationSeconds is the requested duration of validity of the service account token. | |
| The token issuer may return a token with a different validity duration. | |
| When omitted, the token expiration is determined by the kube-apiserver. | |
| Must be at least 600 seconds (10 minutes). | |
| format: int64 | |
| minimum: 600 | |
| type: integer |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@payload-manifests/crds/0000_50_csi-driver_01_clustercsidrivers-TechPreviewNoUpgrade.crd.yaml`
around lines 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.
There was a problem hiding this comment.
The 600 minimum is enforced by the kube-apiserver's TokenRequest API. Also the upstream CSIDriver's tokenRequests[].expirationSeconds does NOT enforce this either.
If Kube ever changes the minimum, our validation becomes stale/wrong.
Following the same pattern as the upstream CSIDriver type keeps things consistent.
https://kubernetes-csi.github.io/docs/token-requests.html
/cc @mytreya-rh
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
/cc @mytreya-rh |
Extends CSIDriverConfigSpec with a new SecretsStore discriminated union variant containing secretRotation and tokenRequests fields. Signed-off-by: chiragkyal <ckyal@redhat.com>
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
| // expirationSeconds is the requested duration of validity of the service account token. | ||
| // The token issuer may return a token with a different validity duration. | ||
| // When omitted, the token expiration is determined by the kube-apiserver. | ||
| // Must be at least 600 seconds (10 minutes). | ||
| // +optional | ||
| ExpirationSeconds *int64 `json:"expirationSeconds,omitempty"` |
There was a problem hiding this comment.
Enforce the documented 600-second minimum.
Line 457 says expirationSeconds must be at least 600, but the field has no +kubebuilder:validation:Minimum=600, so the generated CRDs currently accept smaller values too. That turns a documented API contract into a late runtime failure path.
Suggested fix
// expirationSeconds is the requested duration of validity of the service account token.
// The token issuer may return a token with a different validity duration.
// When omitted, the token expiration is determined by the kube-apiserver.
// Must be at least 600 seconds (10 minutes).
+ // +kubebuilder:validation:Minimum=600
// +optional
ExpirationSeconds *int64 `json:"expirationSeconds,omitempty"`📝 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.
| // expirationSeconds is the requested duration of validity of the service account token. | |
| // The token issuer may return a token with a different validity duration. | |
| // When omitted, the token expiration is determined by the kube-apiserver. | |
| // Must be at least 600 seconds (10 minutes). | |
| // +optional | |
| ExpirationSeconds *int64 `json:"expirationSeconds,omitempty"` | |
| // expirationSeconds is the requested duration of validity of the service account token. | |
| // The token issuer may return a token with a different validity duration. | |
| // When omitted, the token expiration is determined by the kube-apiserver. | |
| // Must be at least 600 seconds (10 minutes). | |
| // +kubebuilder:validation:Minimum=600 | |
| // +optional | |
| ExpirationSeconds *int64 `json:"expirationSeconds,omitempty"` |
🤖 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 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.
| expirationSeconds: | ||
| description: |- | ||
| expirationSeconds is the requested duration of validity of the service account token. | ||
| The token issuer may return a token with a different validity duration. | ||
| When omitted, the token expiration is determined by the kube-apiserver. | ||
| Must be at least 600 seconds (10 minutes). | ||
| format: int64 | ||
| type: integer |
There was a problem hiding this comment.
Enforce the documented lower bound for expirationSeconds.
Line 294 says this value must be at least 600 seconds, but the schema only sets type/format. Values <600 would currently pass CRD validation.
Proposed fix
expirationSeconds:
description: |-
expirationSeconds is the requested duration of validity of the service account token.
The token issuer may return a token with a different validity duration.
When omitted, the token expiration is determined by the kube-apiserver.
Must be at least 600 seconds (10 minutes).
format: int64
+ minimum: 600
type: integer🤖 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/zz_generated.featuregated-crd-manifests/clustercsidrivers.operator.openshift.io/VSphereConfigurableMaxAllowedBlockVolumesPerNode.yaml`
around lines 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.
|
@chiragkyal: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
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.