Fix rotation enable check

Author: dargudear-google

controllers/secretproviderclasspodstatus_controller.go

@@ -419,13 +419,12 @@ func (r *SecretProviderClassPodStatusReconciler) createOrUpdateK8sSecret(ctx con
 		return err
 	}
 
-	klog.InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
+	klog.V(5).InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
 	err = r.writer.Update(ctx, secret)
 	if err != nil {
-		klog.ErrorS(err, "Unable to update kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
 		return err
 	}
-	klog.InfoS("successfully updated Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
+	klog.V(5).InfoS("successfully updated Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
 	return nil
 }
 

manifest_staging/charts/secrets-store-csi-driver/values.yaml

@@ -219,6 +219,7 @@ syncSecret:
   enabled: false
 
 ## Enable secret rotation feature [alpha]
+## If this is set to true, make sure that `requiresRepublish` is true.
 enableSecretRotation: false
 
 ## Secret rotation poll interval duration
@@ -232,16 +233,11 @@ providerHealthCheckInterval: 2m
 
 imagePullSecrets: []
 
-## This allows CSI drivers to impersonate the pods that they mount the volumes for.
-# refer to https://kubernetes-csi.github.io/docs/token-requests.html for more details.
-# Supported only for Kubernetes v1.20+
 tokenRequests: []
 # - audience: aud1
 # - audience: aud2
 
-# To set the requiresRepublish which can be used to refresh the mounted secret periodically
-# refer to https://kubernetes-csi.github.io/docs/token-requests.html for more details.
-# Supported only for Kubernetes v1.20+
+# this will be set to true even if enableSecretRotation is true
 requiresRepublish: true
 # -- Labels to apply to all resources
 commonLabels: {}

pkg/secrets-store/nodeserver.go

@@ -121,6 +121,16 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	podNamespace = attrib[CSIPodNamespace]
 	podUID = attrib[CSIPodUID]
 
+	if rotationEnabled {
+		lastModificationTime, err := ns.getLastUpdateTime(targetPath)
+		if err != nil {
+			klog.InfoS("could not find last modification time for targetpath", targetPath, "error", err)
+		} else if startTime.Before(lastModificationTime.Add(ns.rotationConfig.rotationCacheDuration)) {
+			// if next rotation is not yet due, then skip the mount operation
+			return &csi.NodePublishVolumeResponse{}, nil
+		}
+	}
+
 	mounted, err = ns.ensureMountPoint(targetPath)
 	if err != nil {
 		// kubelet will not create the CSI NodePublishVolume target directory in 1.20+, in accordance with the CSI specification.
@@ -143,16 +153,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		return &csi.NodePublishVolumeResponse{}, nil
 	}
 
-	if rotationEnabled {
-		lastModificationTime, err := ns.getLastUpdateTime(targetPath)
-		if err != nil {
-			klog.InfoS("could not find last modification time for targetpath", targetPath, "error", err)
-		} else if startTime.Before(lastModificationTime.Add(ns.rotationConfig.rotationPollInterval)) {
-			// if next rotation is not yet due, then skip the mount operation
-			return &csi.NodePublishVolumeResponse{}, nil
-		}
-	}
-
 	klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "mount flags", mountFlags)
 
 	if isMockProvider(providerName) {
@@ -192,16 +192,12 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	for k, v := range attrib {
 		parameters[k] = v
 	}
-	// csi.storage.k8s.io/serviceAccount.tokens is empty for Kubernetes version < 1.20.
-	// For 1.20+, if tokenRequests is set in the CSI driver spec, kubelet will generate
-	// a token for the pod and send it to the CSI driver.
-	// This check is done for backward compatibility to support passing token from driver
-	// to provider irrespective of the Kubernetes version. If the token doesn't exist in the
-	// volume request context, the CSI driver will generate the token for the configured audience
-	// and send it to the provider in the parameters.
+	// If the token doesn't exist in the volume request context, the CSI driver
+	// will generate the token for the configured audience and send it to the
+	// provider in the parameters.
 	if parameters[CSIPodServiceAccountTokens] == "" {
 		// Inject pod service account token into volume attributes
-		klog.Info(err, "csi.storage.k8s.io/serviceAccount.tokens is not populated")
+		klog.Warning("csi.storage.k8s.io/serviceAccount.tokens is not populated")
 	}
 
 	// ensure it's read-only

pkg/secrets-store/nodeserver_test.go

@@ -297,8 +297,8 @@ func TestNodePublishVolume(t *testing.T) {
 				},
 			},
 			rotationConfig: &rotationConfig{
-				enabled:              false,
-				rotationPollInterval: time.Minute,
+				enabled:               false,
+				rotationCacheDuration: time.Minute,
 			},
 		},
 		{
@@ -331,8 +331,8 @@ func TestNodePublishVolume(t *testing.T) {
 				},
 			},
 			rotationConfig: &rotationConfig{
-				enabled:              true,
-				rotationPollInterval: -1 * time.Minute, // Using negative interval to pass the rotation interval check in unit tests
+				enabled:               true,
+				rotationCacheDuration: -1 * time.Minute, // Using negative interval to pass the rotation interval check in unit tests
 			},
 		},
 		{
@@ -362,8 +362,8 @@ func TestNodePublishVolume(t *testing.T) {
 				},
 			},
 			rotationConfig: &rotationConfig{
-				enabled:              true,
-				rotationPollInterval: time.Minute,
+				enabled:               true,
+				rotationCacheDuration: time.Minute,
 			},
 		},
 	}
@@ -407,7 +407,7 @@ func TestNodePublishVolume(t *testing.T) {
 					t.Fatalf("expected err to be nil, got: %v", err)
 				}
 				expectedMounts := 1
-				if ns.rotationConfig.enabled && ns.rotationConfig.rotationPollInterval > 0 {
+				if ns.rotationConfig.enabled && ns.rotationConfig.rotationCacheDuration > 0 {
 					// If due to rotation interval, NodePublishVolume has skipped, there should not be any mount operation
 					expectedMounts = 0
 				}

pkg/secrets-store/secrets-store.go

@@ -40,8 +40,8 @@ type SecretsStore struct {
 
 // rotationConfig stores the information required to rotate the secrets.
 type rotationConfig struct {
-	enabled              bool
-	rotationPollInterval time.Duration
+	enabled               bool
+	rotationCacheDuration time.Duration // After this much duration, NodePublishVolume will be acted.
 }
 
 func NewSecretsStoreDriver(driverName, nodeID, endpoint string,
@@ -91,8 +91,8 @@ func newNodeServer(nodeID string,
 
 func newRotationConfig(enabled bool, interval time.Duration) *rotationConfig {
 	return &rotationConfig{
-		enabled:              enabled,
-		rotationPollInterval: interval,
+		enabled:               enabled,
+		rotationCacheDuration: interval,
 	}
 }