From f95e8b6441a7b7bf7be6e99a1f5d7772d0ea4bdb Mon Sep 17 00:00:00 2001 From: "t.hosseini" Date: Tue, 15 Sep 2026 16:28:29 +0330 Subject: [PATCH 1/2] feat(manila-csi): support mutable NFS access rules Implement CSI ControllerModifyVolume for NFS shares so VolumeAttributesClass updates reconcile Manila IP access rules. Preserve non-rw rules, wait for asynchronous grant and revoke operations, and document the required RBAC and usage. --- .../controllerplugin-rules-clusterrole.yaml | 3 + .../using-manila-csi-plugin.md | 21 ++++ pkg/csi/manila/controllerserver.go | 90 +++++++++++++- pkg/csi/manila/controllerserver_test.go | 116 ++++++++++++++++++ pkg/csi/manila/driver.go | 10 +- pkg/csi/manila/driver_test.go | 28 +++++ pkg/csi/manila/shareadapters/accessrights.go | 39 ++++++ .../manila/shareadapters/accessrights_test.go | 54 +++++++- pkg/csi/manila/shareadapters/nfs.go | 58 ++++++++- 9 files changed, 410 insertions(+), 9 deletions(-) diff --git a/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml b/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml index 7e05c5c9c2..f60120eccd 100644 --- a/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml +++ b/charts/manila-csi-plugin/templates/controllerplugin-rules-clusterrole.yaml @@ -27,6 +27,9 @@ rules: - apiGroups: ["storage.k8s.io"] resources: ["storageclasses"] verbs: ["get", "list", "watch"] + - apiGroups: ["storage.k8s.io"] + resources: ["volumeattributesclasses"] + verbs: ["get", "list", "watch"] - apiGroups: ["storage.k8s.io"] resources: ["csinodes"] verbs: ["get", "list", "watch"] diff --git a/docs/manila-csi-plugin/using-manila-csi-plugin.md b/docs/manila-csi-plugin/using-manila-csi-plugin.md index e697cd0291..b94bf76da0 100644 --- a/docs/manila-csi-plugin/using-manila-csi-plugin.md +++ b/docs/manila-csi-plugin/using-manila-csi-plugin.md @@ -64,6 +64,27 @@ Parameter | Required | Description `cephfs-clientID` | _no_ | Relevant for CephFS Manila shares. Specifies the cephx client ID when creating an access rule for the provisioned share. The same cephx client ID may be shared with multiple Manila shares. If providing access to multiple cephx client IDs, set it as a comma separated list. If no value is provided, client ID for the provisioned Manila share will be set to some unique value (PersistentVolume name). `nfs-shareClient` | _no_ | Relevant for NFS Manila shares. Specifies what address has access to the NFS share. Use a comma separated list for granting access to multiple IP addresses or subnets. Defaults to `0.0.0.0/0`, i.e. anyone. +### Mutable NFS access rules + +For NFS shares on Kubernetes 1.34 or newer, `nfs-shareClient` can be changed after +provisioning by assigning a `VolumeAttributesClass` to the PVC. The value is treated as the desired set of `rw` +IP access rules: missing rules are granted before obsolete rules are revoked. Other +access types and `ro` rules are left unchanged. + +```yaml +apiVersion: storage.k8s.io/v1 +kind: VolumeAttributesClass +metadata: + name: manila-nfs-private +driverName: nfs.manila.csi.openstack.org +parameters: + nfs-shareClient: 10.0.0.0/24,192.0.2.10 +``` + +Set `spec.volumeAttributesClassName` on the PVC to this class. To change the list +again, create another `VolumeAttributesClass` and update the PVC to refer to it. +An empty `nfs-shareClient` value revokes all `rw` IP access rules. + ### Node Service volume context _Kubernetes PV CSI volume attributes for pre-provisioned volumes_ diff --git a/pkg/csi/manila/controllerserver.go b/pkg/csi/manila/controllerserver.go index 5dc8c17e32..41cca31ed1 100644 --- a/pkg/csi/manila/controllerserver.go +++ b/pkg/csi/manila/controllerserver.go @@ -19,6 +19,8 @@ package manila import ( "context" "encoding/json" + "fmt" + "net" "strings" "sync" @@ -91,6 +93,19 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol if params == nil { params = make(map[string]string) } + if cs.d.shareProto != "NFS" && len(req.GetMutableParameters()) != 0 { + return nil, status.Error(codes.InvalidArgument, "mutable parameters are only supported for NFS volumes") + } + for key, value := range req.GetMutableParameters() { + if key != "nfs-shareClient" { + return nil, status.Errorf(codes.InvalidArgument, "unsupported mutable parameter %q", key) + } + accessToList, err := parseNFSShareClients(value) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "invalid nfs-shareClient: %v", err) + } + params[key] = strings.Join(accessToList, ",") + } params["protocol"] = cs.d.shareProto @@ -227,8 +242,79 @@ func (cs *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol }, nil } -func (d *controllerServer) ControllerModifyVolume(ctx context.Context, req *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) { - return nil, status.Error(codes.Unimplemented, "") +func (cs *controllerServer) ControllerModifyVolume(ctx context.Context, req *csi.ControllerModifyVolumeRequest) (*csi.ControllerModifyVolumeResponse, error) { + if cs.d.shareProto != "NFS" { + return nil, status.Error(codes.InvalidArgument, "mutable parameters are only supported for NFS volumes") + } + if req.GetVolumeId() == "" { + return nil, status.Error(codes.InvalidArgument, "volume ID cannot be empty") + } + if len(req.GetSecrets()) == 0 { + return nil, status.Error(codes.InvalidArgument, "secrets cannot be nil or empty") + } + + mutableParameters := req.GetMutableParameters() + if len(mutableParameters) == 0 { + return nil, status.Error(codes.InvalidArgument, "mutable parameters cannot be nil or empty") + } + for key := range mutableParameters { + if key != "nfs-shareClient" { + return nil, status.Errorf(codes.InvalidArgument, "unsupported mutable parameter %q", key) + } + } + + accessToList, err := parseNFSShareClients(mutableParameters["nfs-shareClient"]) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "invalid nfs-shareClient: %v", err) + } + + osOpts, err := options.NewOpenstackOptions(req.GetSecrets()) + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "invalid OpenStack secrets: %v", err) + } + + manilaClient, err := cs.d.manilaClientBuilder.New(ctx, osOpts) + if err != nil { + return nil, status.Errorf(codes.Unauthenticated, "failed to create Manila v2 client: %v", err) + } + + share, err := manilaClient.GetShareByID(ctx, req.GetVolumeId()) + if err != nil { + if clouderrors.IsNotFound(err) { + return nil, status.Errorf(codes.NotFound, "volume %s not found: %v", req.GetVolumeId(), err) + } + return nil, status.Errorf(codes.Internal, "failed to retrieve volume %s: %v", req.GetVolumeId(), err) + } + if !compareProtocol(share.ShareProto, "NFS") { + return nil, status.Errorf(codes.InvalidArgument, "nfs-shareClient cannot be applied to a %s volume", share.ShareProto) + } + + if _, isPending := pendingVolumes.LoadOrStore(share.Name, true); isPending { + return nil, status.Errorf(codes.Aborted, "volume %s is already being processed", share.Name) + } + defer pendingVolumes.Delete(share.Name) + + if err := (shareadapters.NFS{}).ReconcileAccesses(ctx, manilaClient, share.ID, accessToList); err != nil { + if wait.Interrupted(err) { + return nil, status.Errorf(codes.DeadlineExceeded, "deadline exceeded while updating access rules for volume %s", share.Name) + } + return nil, status.Errorf(codes.Internal, "failed to update access rules for volume %s: %v", share.Name, err) + } + + return &csi.ControllerModifyVolumeResponse{}, nil +} + +func parseNFSShareClients(value string) ([]string, error) { + clients := util.Unique(util.SplitTrim(value, ',')) + for _, client := range clients { + if net.ParseIP(client) != nil { + continue + } + if _, _, err := net.ParseCIDR(client); err != nil { + return nil, fmt.Errorf("%q is not an IP address or CIDR", client) + } + } + return clients, nil } func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) { diff --git a/pkg/csi/manila/controllerserver_test.go b/pkg/csi/manila/controllerserver_test.go index 58fd464214..e2e5ccdee3 100644 --- a/pkg/csi/manila/controllerserver_test.go +++ b/pkg/csi/manila/controllerserver_test.go @@ -14,12 +14,128 @@ limitations under the License. package manila import ( + "context" "fmt" + "reflect" "testing" + csispec "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/gophercloud/gophercloud/v2/openstack/sharedfilesystems/v2/shares" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "k8s.io/cloud-provider-openstack/pkg/client" "k8s.io/cloud-provider-openstack/pkg/csi" + "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" ) +func TestParseNFSShareClients(t *testing.T) { + tests := []struct { + name string + value string + want []string + wantErr bool + }{ + {name: "addresses and CIDRs", value: " 10.0.0.1, 192.0.2.0/24,2001:db8::1 ", want: []string{"10.0.0.1", "192.0.2.0/24", "2001:db8::1"}}, + {name: "duplicates", value: "10.0.0.1,10.0.0.1", want: []string{"10.0.0.1"}}, + {name: "empty revokes all", value: "", want: []string{}}, + {name: "invalid", value: "not-an-address", wantErr: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := parseNFSShareClients(tc.value) + if (err != nil) != tc.wantErr { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("got %#v, want %#v", got, tc.want) + } + }) + } +} + +type modifyVolumeTestClient struct { + manilaclient.Interface + share *shares.Share + getCalls int + granted []shares.GrantAccessOpts + revoked []string + accessRights [][]shares.AccessRight +} + +func (c *modifyVolumeTestClient) GetShareByID(context.Context, string) (*shares.Share, error) { + return c.share, nil +} + +func (c *modifyVolumeTestClient) GetAccessRights(context.Context, string) ([]shares.AccessRight, error) { + idx := c.getCalls + if idx >= len(c.accessRights) { + idx = len(c.accessRights) - 1 + } + c.getCalls++ + return c.accessRights[idx], nil +} + +func (c *modifyVolumeTestClient) GrantAccess(_ context.Context, _ string, opts shares.GrantAccessOptsBuilder) (*shares.AccessRight, error) { + grantOpts := opts.(shares.GrantAccessOpts) + c.granted = append(c.granted, grantOpts) + return &shares.AccessRight{ID: "new-rule"}, nil +} + +func (c *modifyVolumeTestClient) RevokeAccess(_ context.Context, _ string, accessID string) error { + c.revoked = append(c.revoked, accessID) + return nil +} + +type modifyVolumeTestBuilder struct { + client manilaclient.Interface +} + +func (b *modifyVolumeTestBuilder) New(context.Context, *client.AuthOpts) (manilaclient.Interface, error) { + return b.client, nil +} + +func TestControllerModifyVolumeReconcilesNFSAccessRules(t *testing.T) { + manilaClient := &modifyVolumeTestClient{ + share: &shares.Share{ID: "share-1", Name: "pvc-share", ShareProto: "NFS"}, + accessRights: [][]shares.AccessRight{ + {{ID: "old-rule", AccessType: "ip", AccessLevel: "rw", AccessTo: "192.0.2.0/24", State: "active"}}, + {{ID: "new-rule", AccessType: "ip", AccessLevel: "rw", AccessTo: "10.0.0.0/24", State: "active"}}, + }, + } + server := &controllerServer{d: &Driver{ + shareProto: "NFS", + manilaClientBuilder: &modifyVolumeTestBuilder{client: manilaClient}, + }} + + _, err := server.ControllerModifyVolume(context.Background(), &csispec.ControllerModifyVolumeRequest{ + VolumeId: "share-1", + Secrets: map[string]string{"os-region": "RegionOne"}, + MutableParameters: map[string]string{"nfs-shareClient": "10.0.0.0/24"}, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(manilaClient.granted) != 1 || manilaClient.granted[0].AccessTo != "10.0.0.0/24" { + t.Fatalf("unexpected grants: %#v", manilaClient.granted) + } + if !reflect.DeepEqual(manilaClient.revoked, []string{"old-rule"}) { + t.Fatalf("unexpected revocations: %#v", manilaClient.revoked) + } +} + +func TestControllerModifyVolumeRejectsUnsupportedParameter(t *testing.T) { + server := &controllerServer{d: &Driver{shareProto: "NFS"}} + _, err := server.ControllerModifyVolume(context.Background(), &csispec.ControllerModifyVolumeRequest{ + VolumeId: "share-1", + Secrets: map[string]string{"os-region": "RegionOne"}, + MutableParameters: map[string]string{"unsupported": "value"}, + }) + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("got %v, want InvalidArgument", err) + } +} + func TestPrepareShareMetadata(t *testing.T) { ts := []struct { allVolumeParams map[string]string diff --git a/pkg/csi/manila/driver.go b/pkg/csi/manila/driver.go index 8303ba3755..9379e8b999 100644 --- a/pkg/csi/manila/driver.go +++ b/pkg/csi/manila/driver.go @@ -117,7 +117,7 @@ type nonBlockingGRPCServer struct { } const ( - specVersion = "1.8.0" + specVersion = "1.12.0" driverVersion = "0.9.0" topologyKey = "topology.manila.csi.openstack.org/zone" ) @@ -189,11 +189,15 @@ func NewDriver(o *DriverOpts) (*Driver, error) { func (d *Driver) SetupControllerService() error { klog.Info("Providing controller service") - d.addControllerServiceCapabilities([]csi.ControllerServiceCapability_RPC_Type{ + controllerCapabilities := []csi.ControllerServiceCapability_RPC_Type{ csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, csi.ControllerServiceCapability_RPC_CREATE_DELETE_SNAPSHOT, csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, - }) + } + if d.shareProto == "NFS" { + controllerCapabilities = append(controllerCapabilities, csi.ControllerServiceCapability_RPC_MODIFY_VOLUME) + } + d.addControllerServiceCapabilities(controllerCapabilities) d.addVolumeCapabilityAccessModes([]csi.VolumeCapability_AccessMode_Mode{ csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, diff --git a/pkg/csi/manila/driver_test.go b/pkg/csi/manila/driver_test.go index 29f9645d06..4b47383c5c 100644 --- a/pkg/csi/manila/driver_test.go +++ b/pkg/csi/manila/driver_test.go @@ -134,3 +134,31 @@ func TestInitProxiedDriverRetryOnUnavailable(t *testing.T) { t.Errorf("expected 4 ProbeForever calls (3 Unavailable + 1 success), got %d", idClient.calls) } } + +func TestSetupControllerServiceModifyVolumeCapability(t *testing.T) { + for _, tc := range []struct { + protocol string + want bool + }{ + {protocol: "NFS", want: true}, + {protocol: "CEPHFS", want: false}, + } { + t.Run(tc.protocol, func(t *testing.T) { + d := &Driver{shareProto: tc.protocol} + if err := d.SetupControllerService(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got := false + for _, capability := range d.cscaps { + if capability.GetRpc().GetType() == csi.ControllerServiceCapability_RPC_MODIFY_VOLUME { + got = true + break + } + } + if got != tc.want { + t.Fatalf("MODIFY_VOLUME capability = %t, want %t", got, tc.want) + } + }) + } +} diff --git a/pkg/csi/manila/shareadapters/accessrights.go b/pkg/csi/manila/shareadapters/accessrights.go index 0ac931a37a..12196abceb 100644 --- a/pkg/csi/manila/shareadapters/accessrights.go +++ b/pkg/csi/manila/shareadapters/accessrights.go @@ -32,6 +32,8 @@ const ( accessRuleStateError = "error" accessRuleStateQueuedApply = "queued_to_apply" accessRuleStateApplying = "applying" + accessRuleStateQueuedDeny = "queued_to_deny" + accessRuleStateDenying = "denying" waitForAccessRuleTimeout = 3 waitForAccessRuleRetries = 10 @@ -92,3 +94,40 @@ func waitForAccessRuleActive(ctx context.Context, manilaClient manilaclient.Inte return result, nil } + +func waitForAccessRuleDeleted(ctx context.Context, manilaClient manilaclient.Interface, shareID, accessID string) error { + backoff := wait.Backoff{ + Duration: time.Second * waitForAccessRuleTimeout, + Factor: 1.2, + Steps: waitForAccessRuleRetries, + } + + err := wait.ExponentialBackoffWithContext(ctx, backoff, func(ctx context.Context) (bool, error) { + rights, err := manilaClient.GetAccessRights(ctx, shareID) + if err != nil { + return false, fmt.Errorf("failed to get access rights for share %s: %v", shareID, err) + } + + for i := range rights { + if rights[i].ID != accessID { + continue + } + + switch rights[i].State { + case accessRuleStateActive, accessRuleStateQueuedDeny, accessRuleStateDenying: + klog.V(4).Infof("access rule %s for share %s is in state %s, waiting for deletion...", accessID, shareID, rights[i].State) + return false, nil + case accessRuleStateError: + return false, fmt.Errorf("access rule %s for share %s is in error state while being revoked", accessID, shareID) + default: + return false, fmt.Errorf("access rule %s for share %s is in unexpected state %s while being revoked", accessID, shareID, rights[i].State) + } + } + + return true, nil + }) + if wait.Interrupted(err) { + return fmt.Errorf("timed out waiting for access rule %s for share %s to be deleted", accessID, shareID) + } + return err +} diff --git a/pkg/csi/manila/shareadapters/accessrights_test.go b/pkg/csi/manila/shareadapters/accessrights_test.go index ee046f5e26..225262b8e6 100644 --- a/pkg/csi/manila/shareadapters/accessrights_test.go +++ b/pkg/csi/manila/shareadapters/accessrights_test.go @@ -34,6 +34,8 @@ type mockManilaClient struct { callCount int accessRights [][]shares.AccessRight revokeCalled bool + granted []shares.GrantAccessOpts + revoked []string } func (m *mockManilaClient) GetMicroversion() string { return "" } @@ -57,8 +59,10 @@ func (m *mockManilaClient) GetExportLocations(_ context.Context, _ string) ([]sh func (m *mockManilaClient) SetShareMetadata(_ context.Context, _ string, _ shares.SetMetadataOptsBuilder) (map[string]string, error) { return nil, nil } -func (m *mockManilaClient) GrantAccess(_ context.Context, _ string, _ shares.GrantAccessOptsBuilder) (*shares.AccessRight, error) { - return nil, nil +func (m *mockManilaClient) GrantAccess(_ context.Context, _ string, opts shares.GrantAccessOptsBuilder) (*shares.AccessRight, error) { + grantOpts := opts.(shares.GrantAccessOpts) + m.granted = append(m.granted, grantOpts) + return &shares.AccessRight{ID: "new-" + grantOpts.AccessTo}, nil } func (m *mockManilaClient) GetSnapshotByID(_ context.Context, _ string) (*snapshots.Snapshot, error) { return nil, nil @@ -92,8 +96,9 @@ func (m *mockManilaClient) GetAccessRights(_ context.Context, _ string) ([]share return m.accessRights[idx], nil } -func (m *mockManilaClient) RevokeAccess(_ context.Context, _ string, _ string) error { +func (m *mockManilaClient) RevokeAccess(_ context.Context, _ string, accessID string) error { m.revokeCalled = true + m.revoked = append(m.revoked, accessID) return nil } @@ -192,3 +197,46 @@ func TestWaitForAccessRuleNotFound(t *testing.T) { t.Errorf("expected 'not found' in error message, got: %s", err.Error()) } } + +func TestNFSReconcileAccesses(t *testing.T) { + mock := &mockManilaClient{ + accessRights: [][]shares.AccessRight{ + { + {ID: "keep", AccessType: "ip", AccessLevel: "rw", AccessTo: "10.0.0.0/24", State: "active"}, + {ID: "remove", AccessType: "ip", AccessLevel: "rw", AccessTo: "192.0.2.0/24", State: "active"}, + {ID: "preserve-ro", AccessType: "ip", AccessLevel: "ro", AccessTo: "198.51.100.0/24", State: "active"}, + {ID: "preserve-cephx", AccessType: "cephx", AccessLevel: "rw", AccessTo: "client", State: "active"}, + }, + { + {ID: "new-203.0.113.10", AccessType: "ip", AccessLevel: "rw", AccessTo: "203.0.113.10", State: "active"}, + }, + }, + } + + err := (NFS{}).ReconcileAccesses(context.Background(), mock, "share-1", []string{"10.0.0.0/24", "203.0.113.10"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(mock.granted) != 1 || mock.granted[0].AccessTo != "203.0.113.10" { + t.Fatalf("unexpected grants: %#v", mock.granted) + } + if len(mock.revoked) != 1 || mock.revoked[0] != "remove" { + t.Fatalf("unexpected revocations: %#v", mock.revoked) + } +} + +func TestNFSReconcileAccessesIsIdempotent(t *testing.T) { + mock := &mockManilaClient{ + accessRights: [][]shares.AccessRight{{ + {ID: "keep", AccessType: "ip", AccessLevel: "rw", AccessTo: "10.0.0.0/24", State: "active"}, + }}, + } + + err := (NFS{}).ReconcileAccesses(context.Background(), mock, "share-1", []string{"10.0.0.0/24"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(mock.granted) != 0 || len(mock.revoked) != 0 { + t.Fatalf("expected no changes, got grants %#v and revocations %#v", mock.granted, mock.revoked) + } +} diff --git a/pkg/csi/manila/shareadapters/nfs.go b/pkg/csi/manila/shareadapters/nfs.go index a92f25f580..f990450a9a 100644 --- a/pkg/csi/manila/shareadapters/nfs.go +++ b/pkg/csi/manila/shareadapters/nfs.go @@ -24,8 +24,10 @@ import ( "github.com/gophercloud/gophercloud/v2" "github.com/gophercloud/gophercloud/v2/openstack/sharedfilesystems/v2/shares" + "k8s.io/cloud-provider-openstack/pkg/csi/manila/manilaclient" "k8s.io/cloud-provider-openstack/pkg/csi/manila/runtimeconfig" manilautil "k8s.io/cloud-provider-openstack/pkg/csi/manila/util" + "k8s.io/cloud-provider-openstack/pkg/util" "k8s.io/klog/v2" ) @@ -43,7 +45,7 @@ func (NFS) GetOrGrantAccesses(ctx context.Context, args *GrantAccessArgs) ([]sha } } - accessToList := strings.Split(args.Options.NFSShareClient, ",") + accessToList := util.Unique(util.SplitTrim(args.Options.NFSShareClient, ',')) for _, at := range accessToList { // Try to find the access right @@ -76,6 +78,60 @@ func (NFS) GetOrGrantAccesses(ctx context.Context, args *GrantAccessArgs) ([]sha return rights, nil } +// ReconcileAccesses makes the share's rw IP access rules match accessToList. +// New rules are activated before obsolete rules are revoked to avoid an access gap. +func (NFS) ReconcileAccesses(ctx context.Context, manilaClient manilaclient.Interface, shareID string, accessToList []string) error { + rights, err := manilaClient.GetAccessRights(ctx, shareID) + if err != nil { + return fmt.Errorf("failed to list access rights: %v", err) + } + + desired := make(map[string]struct{}, len(accessToList)) + for _, accessTo := range accessToList { + desired[accessTo] = struct{}{} + } + + existing := make(map[string]shares.AccessRight) + for _, right := range rights { + if right.AccessType == "ip" && right.AccessLevel == "rw" { + existing[right.AccessTo] = right + } + } + + for accessTo := range desired { + if _, found := existing[accessTo]; found { + continue + } + + right, err := manilaClient.GrantAccess(ctx, shareID, shares.GrantAccessOpts{ + AccessType: "ip", + AccessLevel: "rw", + AccessTo: accessTo, + }) + if err != nil { + return fmt.Errorf("failed to grant access right for %s: %v", accessTo, err) + } + if _, err := waitForAccessRuleActive(ctx, manilaClient, shareID, right.ID); err != nil { + return err + } + } + + for accessTo, right := range existing { + if _, keep := desired[accessTo]; keep { + continue + } + + if err := manilaClient.RevokeAccess(ctx, shareID, right.ID); err != nil { + return fmt.Errorf("failed to revoke access right for %s: %v", accessTo, err) + } + if err := waitForAccessRuleDeleted(ctx, manilaClient, shareID, right.ID); err != nil { + return err + } + } + + return nil +} + func (NFS) BuildVolumeContext(args *VolumeContextArgs) (volumeContext map[string]string, err error) { chosenExportLocationIdx, err := nfsChooseExportLocation(args.Locations) if err != nil { From b19d12571c0ccdb0003aefa0c6167bea0a5446d8 Mon Sep 17 00:00:00 2001 From: "t.hosseini" Date: Tue, 15 Sep 2026 22:29:18 +0330 Subject: [PATCH 2/2] test(manila-csi): configure mutable volume sanity tests Provide ControllerModifyVolume secrets and an NFS share client parameter now that the driver advertises MODIFY_VOLUME. --- tests/sanity/manila/fake-secrets.yaml | 7 +++++++ tests/sanity/manila/sanity_test.go | 3 +++ 2 files changed, 10 insertions(+) diff --git a/tests/sanity/manila/fake-secrets.yaml b/tests/sanity/manila/fake-secrets.yaml index ae6930c339..730caec740 100644 --- a/tests/sanity/manila/fake-secrets.yaml +++ b/tests/sanity/manila/fake-secrets.yaml @@ -40,6 +40,13 @@ ControllerExpandVolumeSecret: os-password: fake-password os-domainID: fake-domain-id os-projectID: fake-project-id +ControllerModifyVolumeSecret: + os-authURL: fake-url + os-region: fake-region + os-userID: fake-user-id + os-password: fake-password + os-domainID: fake-domain-id + os-projectID: fake-project-id NodeStageVolumeSecret: os-authURL: fake-url os-region: fake-region diff --git a/tests/sanity/manila/sanity_test.go b/tests/sanity/manila/sanity_test.go index 8218ded280..fff4133772 100644 --- a/tests/sanity/manila/sanity_test.go +++ b/tests/sanity/manila/sanity_test.go @@ -64,6 +64,9 @@ func TestDriver(t *testing.T) { config := sanity.NewTestConfig() config.Address = endpoint config.SecretsFile = "fake-secrets.yaml" + config.TestVolumeMutableParameters = map[string]string{ + "nfs-shareClient": "192.0.2.0/24", + } sanity.Test(t, config) }