Add PVC Mirror Provisioner#1067
Conversation
|
Hi @chia-me. Thanks for your PR. I am @kingmakerbot.
Make sure this PR appears in the CrownLabs changelog, adding one of the following labels:
|
|
/deploy-staging |
|
Something went wrong while deploying your staging environment! |
|
/undeploy-staging |
|
Your staging environment has been correctly teared-down! |
|
/deploy-staging |
|
Something went wrong while deploying your staging environment! |
|
/deploy-staging |
|
/deploy-staging |
|
/deploy-staging |
|
Something went wrong while deploying your staging environment! |
|
/deploy-staging |
|
/undeploy-staging |
|
Something went wrong while tearing-down your staging environment! |
|
/deploy-staging |
|
Something went wrong while deploying your staging environment! |
1 similar comment
|
Something went wrong while deploying your staging environment! |
|
/deploy-staging |
c6c8203 to
cf2156b
Compare
b86f60d to
ba5c777
Compare
ba5c777 to
9417a7f
Compare
9417a7f to
a38844e
Compare
a3cc286 to
2b08b1f
Compare
Co-authored-by: Alessio Giliberti <[email protected]>
17c6ca9 to
f2b098a
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “PVC Mirror Provisioner” (pmp) to dynamically provision mirror PVs for target PVCs that reference an origin PVC via spec.dataSourceRef (cross-namespace), gated by an authorization mechanism based on an annotation-defined label selector evaluated against the target Namespace labels.
Changes:
- Added the
pmpprovisioner implementation (plus a dedicated envtest suite) to mirror CSI-backed PVs based on an origin PVC referenced indataSourceRef. - Introduced an authorization annotation/label-selector mechanism and ensured origin PVCs (MyDrive / SharedVolume) are annotated accordingly.
- Updated Helm chart/operator wiring: new StorageClass template, new operator flags, and RBAC additions needed by the external provisioner.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| operators/pkg/utils/common.go | Adds namespace label-selector matching helper used by pmp; minor cleanup in label helper. |
| operators/pkg/forge/tenant.go | Adds helper to set MyDrive PVC authorization annotation. |
| operators/pkg/forge/tenant_test.go | Unit tests for MyDrive PVC annotation helper. |
| operators/pkg/forge/nfs.go | Extends MyDrive PVC configuration to support annotations. |
| operators/pkg/forge/nfs_test.go | Updates tests to cover annotation propagation to MyDrive PVCs. |
| operators/pkg/forge/labels.go | Adds constants for pmp authorization annotation key and values. |
| operators/pkg/controller/tenant/mydrive.go | Ensures MyDrive PVCs receive the authorization annotation during reconciliation. |
| operators/pkg/controller/tenant/mydrive_test.go | Verifies MyDrive PVC includes the authorization annotation. |
| operators/pkg/controller/sharedvolume/reconciler.go | Adds authorization annotation to SharedVolume PVCs. |
| operators/pkg/controller/sharedvolume/reconciler_test.go | Tests for SharedVolume PVC annotations; removes debug print. |
| operators/pkg/controller/pmp/suite_test.go | New envtest suite for the pmp provisioner (enables CrossNamespaceVolumeDataSource feature gate). |
| operators/pkg/controller/pmp/provisioner.go | New pmp provisioner implementation (Provision/Delete/Start/ShouldDelete). |
| operators/pkg/controller/pmp/provisioner_test.go | Extensive unit/envtest coverage for pmp behavior and edge cases. |
| operators/cmd/operator/pmp.go | Adds CLI flags and manager wiring to run the provisioner. |
| operators/cmd/operator/main.go | Adds --enable-pmp flag and starts pmp when enabled. |
| operators/deploy/operator/values.yaml | Adds Helm values for mirror provisioner name and StorageClass name. |
| operators/deploy/operator/templates/mirror-storageclass.yaml | New StorageClass template for mirror PVC provisioning. |
| operators/deploy/operator/templates/deployment.yaml | Passes new pmp flags to the operator container. |
| operators/deploy/operator/templates/clusterrole.yaml | Adds RBAC for storageclasses/leases/events and delete permission for PV/PVC. |
| operators/Makefile | Enables --enable-pmp in the local run target. |
| operators/go.mod | Adds external-provisioner dependency and bumps several module versions. |
| operators/go.sum | Updates dependency checksums accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This PR aims to add a new Storage Provisioner called “PVC Mirror Provisioner” (pmp), that will create a mirroring PV (for CSI-based storage, like Ceph NFS) for a target PVC in the given namespace.
This can be done only if the mirroring is authorized, i.e. the target namespace has labels that match the one in an annotation on the origin PVC.
This is part of the Cloud Programming project no.3 of a.y. 2025/26, done by @chia-me and @AleTopp
Rationale
To keep information about the PVC to be mirrored within the mirror one, we decided to use the
dataSourceReffield in the PVC to represent the source - the original PVC - that must then be mirrored by thepmp.Since the two PVCs are in different namespaces, the reference must also specify the namespace of the original PVC: the
namespacefield specified indataSourceRefcan only be used by enabling theCrossNamespaceVolumeDataSourcefeature gate [1]. Therefore, we investigated whether enabling this option was a safe choice or whether, by enabling it, users might be able to clone volumes they did not have access to (dataSourceRefwas designed for volume cloning, which is supported only by a minority of provisioners). However, since the option itself is in Alpha version, for security reasons it must be enabled both at the cluster level and at the individual provisioner level [2]; therefore, end users cannot take advantage of it, aspmpis the only component that supports it (and on which it is enabled).Alongside this feature gate, the use of a resource called
ReferenceGranthas been proposed, whose purpose is to grant permissions for transferring volumes across namespaces. This resource is part of the Gateway API, but since Kubernetes developers have observed how widely it is used outside the networking context (in sig/auth and sig/storage, [3]), they planned about four years ago to move it into a more generic API group [3] [4], without further updates.Finally, we checked whether there were any other proposed changes to the meaning of the
dataSourceReffield, but the only discussions available in the community were about making it more generic, allowing support for any resource as a volume source by introducing a new CR to register valid types [5] [6] [7]. However, this (uncertain) change does not directly affect our project, since we use PVCs as the source, which is already valid by default.Based on this, we believe it is safe to usedataSourceReftogether with theReferenceGrantresource to achieve our goals.[ReferenceGrant - example of use]Nonetheless, @QcFe suggested not to use
ReferenceGrantfor two reasons:ReferenceGrantwith (very large) NFromelements or to NReferenceGrants for a single SharedVolume in a Workspace (N as big as the number of tenants in CrownLabs, ~5k as of now).So after evaluating also a resource-specific RBAC mechanism, we landed on the - simpler - use of annotations and labels. In this way, on the origin
PVCthere will be an annotation:pmp.crownlabs.polito.it/required-target-ns-labels: <label_selector_here>and on the target
Namespace, there will be labels that have to match the label selector above defined to get authorization. In our specific case,MyDrives' andSharedVolumes' PVCs will have respectively these annotations:pmp.crownlabs.polito.it/required-target-ns-labels: "crownlabs.polito.it/type=tenant,crownlabs.polito.it/name=s123456"pmp.crownlabs.polito.it/required-target-ns-labels: "crownlabs.polito.it/type=tenant"using labels already present on the tenant's personal namespace.
The authorization for
SharedVolumes is not as strict as it could be, but actually it doesn't matter since normal users cannot create PVCs on their own.How Has This Been Tested?
All functions added and/or edited have been unit tested.
For the pmp per se, there is a new test suite that checks all of its functions.