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
Signed-off-by: chiragkyal <ckyal@redhat.com>
|
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: |
📝 WalkthroughWalkthroughThis pull request extends the CSI driver configuration API to support Secrets Store drivers. The Go API type 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (11 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.
| // tokenRequests specifies service account token audiences that kubelet will provide | ||
| // to the CSI driver during NodePublishVolume calls. These tokens enable workload | ||
| // identity federation (WIF) with cloud providers such as AWS, Azure, and GCP. | ||
| // An empty audience string means the token uses the kube-apiserver's default APIAudiences. | ||
| // +optional | ||
| // +listType=atomic | ||
| TokenRequests []SecretsStoreTokenRequest `json:"tokenRequests,omitempty"` |
There was a problem hiding this comment.
Document omitted behavior for new optional fields.
tokenRequests and expirationSeconds are marked +optional, but their comments don’t describe behavior when omitted.
As per coding guidelines, Documentation for +optional fields must explain the behavior when the field is omitted.
Also applies to: 445-448
🤖 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 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.
|
@chiragkyal: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
Summary
SecretsStoreas a newCSIDriverTypeenum value in theClusterCSIDriverAPI (operator.openshift.io/v1)SecretsStoreCSIDriverConfigSpecwith configuration for:the poll interval (defaults to enabled with 2m interval)
workload identity federation (WIF) with AWS, Azure, GCP, etc.
secretsStoreconfigis set if and only if
driverTypeisSecretsStore