Skip to content

Add namespace label selector for cross-namespace volume sharing - #1202

Open
kmizumar wants to merge 1 commit into
NetApp:masterfrom
kmizumar:share-namespace-selector-pr
Open

kmizumar wants to merge 1 commit into
NetApp:masterfrom
kmizumar:share-namespace-selector-pr

Conversation

@kmizumar

Copy link
Copy Markdown

Change description

Add namespace label selector for cross-namespace volume sharing

Add the trident.netapp.io/shareToNamespaceSelector annotation for source PVCs. When it is set, a new subordinate volume is allowed only if the labels of its namespace match the Kubernetes label selector in the annotation. The namespace is read from the Kubernetes API when the subordinate volume configuration is validated, so removing a required label denies subsequent provisioning. Existing subordinate volumes and mounts are not affected.

The selector is an alternative to trident.netapp.io/shareToNamespace, as suggested in #1196. A source PVC that carries both annotations is rejected with an error that names the source PVC and asks the user to set only one of them. Empty or invalid selectors and namespace lookup failures also deny sharing. Behavior is unchanged when the selector annotation is absent, and the subordinate PVC still requires shareFromPVC and a TridentVolumeReference.

Project tracking

Related issue: #1196

  • This PR does not require a JIRA ticket (explain below why)

This is an external contribution, and I do not have access to NetApp's Jira. The feature is tracked in GitHub issue #1196.

Do any added TODOs have an issue in the backlog?

No TODOs were added.

Did you add unit tests? Why not?

Yes, in frontend/csi/controller_helpers/kubernetes/helper_test.go:

  • TestValidateNamespaceSharing: 19 policy cases. These cover legacy shareToNamespace behavior, selector-only matching, mismatched and missing labels, empty, whitespace, and invalid selectors, set, existence, and negative expressions, and six cases where both annotations are set. The test also verifies that the Namespace API is not called for legacy, conflicting, or invalid policies.
  • TestValidateNamespaceSharing_ConflictMessage: the error identifies the source PVC, the destination namespace, both annotations, and the required action.
  • TestValidateNamespaceSharing_NamespaceErrors: NotFound, Forbidden, and timeout errors from the Namespace lookup deny sharing and preserve the API error.
  • TestValidateSubordinateVolumeConfig_NamespaceSelector: the full subordinate validation path using PVC, PV, and TVR caches with a fake client. It covers selector-only success, the existing storage-class, binding, and TVR checks, rejection when both annotations are added, and label removal and restoration.

These checks passed locally with Go 1.27.3 on this branch:

go test -count=1 ./frontend/csi/controller_helpers/kubernetes ./frontend/csi
go vet ./frontend/csi/controller_helpers/kubernetes
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.15.0 run ./frontend/csi/controller_helpers/kubernetes   # 0 issues

Does this code need functional testing?

Yes. The following scenarios on an NFS backend that supports volume sharing:

  1. A subordinate PVC binds when the destination namespace labels match the selector.
  2. After the label is removed, a new subordinate PVC stays Pending with a ProvisioningFailed event. The existing subordinate PVC and its mounts keep working.
  3. After the label is restored, the pending PVC binds on retry.
  4. When the source PVC has both annotations, the new subordinate PVC stays Pending with the mutual-exclusion error. After one annotation is removed, it binds on retry.

I have not yet run these on a live cluster. Only the unit tests above have been run.

Is a code review walkthrough needed? why or why not?

Probably not. The change is limited to validateSubordinateVolumeConfig and a new helper, validateNamespaceSharing, in the Kubernetes controller helper. I am happy to walk through it if that would help.

Should additional test coverage be executed in addition to pre-merge?

Please also run the existing cross-namespace volume sharing scenarios with shareToNamespace only, to confirm that legacy behavior is unchanged.

Does this code need a note in the changelog?

Yes. I cannot set labels on this repository, so could a maintainer please add needs-changelog?

Changelog

Added the trident.netapp.io/shareToNamespaceSelector source PVC annotation, which allows a volume to be shared with namespaces whose labels match a Kubernetes label selector.

Does this code require documentation changes?

Yes. The cross-namespace sharing page (trident-use/volume-share.adoc in NetAppDocs/trident) should describe the new annotation, its mutual exclusion with shareToNamespace, and the fact that label removal affects only new provisioning. I can open a PR there if the code change is accepted.

Additional Information

  • Scope: Namespace labels are checked only when a new subordinate volume is provisioned. Existing volumes and mounts, operations that were already admitted, and idempotent CreateVolume retries for volumes that already exist are not revoked. Cross-namespace clone authorization is unchanged.
  • Selector semantics: Standard Kubernetes label-selector semantics apply. Negative expressions such as team notin (red) also match namespaces that have no team label. Only trusted administrators should be able to set the labels used to authorize sharing.
  • RBAC: The controller already has get on namespaces. No new permissions or informers are added, and go.mod is unchanged.
  • CCLA: Our Corporate CLA has been signed and submitted.
  • AI assistance: This change was developed with the help of an AI coding assistant (Claude Code by Anthropic), which was used to write the implementation and tests and to draft this description. I have reviewed and tested the changes and take responsibility for them, and they are submitted under our CCLA. I did not find any guidance on AI-assisted contributions in CONTRIBUTING.md or the CCLA. Could you let me know whether NetApp has a preferred way to disclose AI assistance, for example a Co-authored-by trailer, a note in the PR description, or neither, or any restrictions I should follow? The commit currently includes a Co-Authored-By: Claude trailer, and I am happy to adjust or remove it.

Add the trident.netapp.io/shareToNamespaceSelector annotation for source
PVCs. When it is set, a new subordinate volume is allowed only if the
labels of its namespace match the Kubernetes label selector in the
annotation. The namespace is read from the Kubernetes API when the
subordinate volume configuration is validated, so removing a required
label denies subsequent provisioning. Existing subordinate volumes and
mounts are not affected.

The selector is an alternative to trident.netapp.io/shareToNamespace. A
source PVC that carries both annotations is rejected with an error that
names the source PVC and asks the user to set only one of them. Empty or
invalid selectors and namespace lookup failures also deny sharing.
Behavior is unchanged when the selector annotation is absent, and the
subordinate PVC still requires shareFromPVC and a TridentVolumeReference.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant