diff --git a/pkg/secrets-store/nodeserver.go b/pkg/secrets-store/nodeserver.go index b7f1d7a97..4a9cc9f41 100644 --- a/pkg/secrets-store/nodeserver.go +++ b/pkg/secrets-store/nodeserver.go @@ -76,14 +76,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis errorReason := internalerrors.FailedToMount rotationEnabled := ns.rotationConfig.enabled - if ns.rotationConfig.enabled { - if ns.rotationConfig.nextRotationTime.After(startTime) { - klog.InfoS("Too soon !!!!, will rotate secret after", ns.rotationConfig.nextRotationTime) - return &csi.NodePublishVolumeResponse{}, nil - } - ns.rotationConfig.nextRotationTime = ns.rotationConfig.nextRotationTime.Add(ns.rotationConfig.interval) - } - defer func() { if err != nil { // if there is an error at any stage during node publish volume and if the path @@ -101,6 +93,15 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis ns.reporter.ReportNodePublishCtMetric(ctx, providerName) }() + if ns.rotationConfig.enabled { + // Node server retries nodepublish volume continuously, so to rotate secret after every `nextRotationTime`, + // nodeserver should skip secret mount till the next `nextRotationTime` + if ns.rotationConfig.nextRotationTime.After(startTime) { + return &csi.NodePublishVolumeResponse{}, nil + } + ns.rotationConfig.nextRotationTime = ns.rotationConfig.nextRotationTime.Add(ns.rotationConfig.interval) + } + // Check arguments if req.GetVolumeCapability() == nil { return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request") @@ -140,6 +141,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis return nil, status.Errorf(codes.Internal, "failed to check if target path %s is mount point, err: %v", targetPath, err) } } + // If rotation is not enabled, don't remount the already mounted secrets. if !rotationEnabled && mounted { klog.InfoS("target path is already mounted", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}) return &csi.NodePublishVolumeResponse{}, nil diff --git a/pkg/secrets-store/nodeserver_test.go b/pkg/secrets-store/nodeserver_test.go index 6a02ee82f..0ad7738be 100644 --- a/pkg/secrets-store/nodeserver_test.go +++ b/pkg/secrets-store/nodeserver_test.go @@ -21,6 +21,7 @@ import ( "os" "path/filepath" "testing" + "time" secretsstorev1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" "sigs.k8s.io/secrets-store-csi-driver/pkg/secrets-store/mocks" @@ -267,6 +268,7 @@ func TestNodePublishVolume(t *testing.T) { name string nodePublishVolReq *csi.NodePublishVolumeRequest initObjects []client.Object + rotationConfig *RotationConfig }{ { name: "volume mount", @@ -294,9 +296,14 @@ func TestNodePublishVolume(t *testing.T) { }, }, }, + rotationConfig: &RotationConfig{ + enabled: false, + nextRotationTime: time.Now(), + interval: time.Minute, + }, }, { - name: "volume mount with refresh token", + name: "volume mount with refresh token ", nodePublishVolReq: &csi.NodePublishVolumeRequest{ VolumeCapability: &csi.VolumeCapability{}, VolumeId: "testvolid1", @@ -324,6 +331,43 @@ func TestNodePublishVolume(t *testing.T) { }, }, }, + rotationConfig: &RotationConfig{ + enabled: true, + nextRotationTime: time.Now().Add(-3 * time.Minute), // so that rotation period is passed and secret will be mounted. + interval: time.Minute, + }, + }, + { + name: "volume mount with rotation but skipped", + nodePublishVolReq: &csi.NodePublishVolumeRequest{ + VolumeCapability: &csi.VolumeCapability{}, + VolumeId: "testvolid1", + TargetPath: targetPath(t), + VolumeContext: map[string]string{ + "secretProviderClass": "provider1", + CSIPodName: "pod1", + CSIPodNamespace: "default", + CSIPodUID: "poduid1", + }, + Readonly: true, + }, + initObjects: []client.Object{ + &secretsstorev1.SecretProviderClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "provider1", + Namespace: "default", + }, + Spec: secretsstorev1.SecretProviderClassSpec{ + Provider: "provider1", + Parameters: map[string]string{"parameter1": "value1"}, + }, + }, + }, + rotationConfig: &RotationConfig{ + enabled: true, + nextRotationTime: time.Now().Add(2 * time.Minute), + interval: time.Minute, + }, }, } @@ -338,7 +382,7 @@ func TestNodePublishVolume(t *testing.T) { t.Run(test.name, func(t *testing.T) { r := mocks.NewFakeReporter() - ns, err := testNodeServer(t, fake.NewClientBuilder().WithScheme(s).WithObjects(test.initObjects...).Build(), r, &RotationConfig{}) + ns, err := testNodeServer(t, fake.NewClientBuilder().WithScheme(s).WithObjects(test.initObjects...).Build(), r, test.rotationConfig) if err != nil { t.Fatalf("expected error to be nil, got: %+v", err) } @@ -365,8 +409,13 @@ func TestNodePublishVolume(t *testing.T) { if err != nil { t.Fatalf("expected err to be nil, got: %v", err) } - if len(mnts) == 0 { - t.Errorf("expected mounts...: %v", mnts) + expectedMounts := 1 + if ns.rotationConfig.enabled && ns.rotationConfig.nextRotationTime.After(time.Now()) { + // If rotation time is not reached, there should not be any mounts. + expectedMounts = 0 + } + if len(mnts) != expectedMounts { + t.Errorf("[Number of mounts] want : %d, got mount: %d", expectedMounts, len(mnts)) } } })