Add tests to use rotation config.

kubernetes-sigs · Sep 7, 2024 · 3b9d617 · 3b9d617
1 parent b482f3a
commit 3b9d617
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 12 deletions.
diff --git 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
@@ -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))
 				}
 			}
 		})