Use requiresRepublish in staging manifest.

controllers/secretproviderclasspodstatus_controller.go

@@ -295,58 +295,44 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context,
 			errs = append(errs, fmt.Errorf("failed to validate secret object in spc %s/%s, err: %w", spc.Namespace, spc.Name, err))
 			continue
 		}
-		exists, err := r.secretExists(ctx, secretName, req.Namespace)
-		if err != nil {
-			klog.ErrorS(err, "failed to check if secret exists", "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus))
-			// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
-			// that would result in a forbidden error, so generate a warning that can be helpful for debugging
-			if apierrors.IsForbidden(err) {
-				klog.Warning(SyncSecretForbiddenWarning)
-			}
-			errs = append(errs, fmt.Errorf("failed to check if secret %s exists, err: %w", secretName, err))
-			continue
-		}
 
 		var funcs []func() (bool, error)
+		secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))
 
-		if !exists {
-			secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))
-
-			var datamap map[string][]byte
-			if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
-				r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
-				klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
-				errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
-				continue
-			}
+		var datamap map[string][]byte
+		if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
+			r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
+			klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
+			errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
+			continue
+		}
 
-			labelsMap := make(map[string]string)
-			if secretObj.Labels != nil {
-				labelsMap = secretObj.Labels
-			}
-			annotationsMap := make(map[string]string)
-			if secretObj.Annotations != nil {
-				annotationsMap = secretObj.Annotations
-			}
-			// Set secrets-store.csi.k8s.io/managed=true label on the secret that's created and managed
-			// by the secrets-store-csi-driver. This label will be used to perform a filtered list watch
-			// only on secrets created and managed by the driver
-			labelsMap[SecretManagedLabel] = "true"
-
-			createFn := func() (bool, error) {
-				if err := r.createK8sSecret(ctx, secretName, req.Namespace, datamap, labelsMap, annotationsMap, secretType); err != nil {
-					klog.ErrorS(err, "failed to create Kubernetes secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
-					// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
-					// that would result in a forbidden error, so generate a warning that can be helpful for debugging
-					if apierrors.IsForbidden(err) {
-						klog.Warning(SyncSecretForbiddenWarning)
-					}
-					return false, nil
+		labelsMap := make(map[string]string)
+		if secretObj.Labels != nil {
+			labelsMap = secretObj.Labels
+		}
+		annotationsMap := make(map[string]string)
+		if secretObj.Annotations != nil {
+			annotationsMap = secretObj.Annotations
+		}
+		// Set secrets-store.csi.k8s.io/managed=true label on the secret that's created and managed
+		// by the secrets-store-csi-driver. This label will be used to perform a filtered list watch
+		// only on secrets created and managed by the driver
+		labelsMap[SecretManagedLabel] = "true"
+
+		createFn := func() (bool, error) {
+			if err := r.createOrUpdateK8sSecret(ctx, secretName, req.Namespace, datamap, labelsMap, annotationsMap, secretType); err != nil {
+				klog.ErrorS(err, "failed to create Kubernetes secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
+				// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
+				// that would result in a forbidden error, so generate a warning that can be helpful for debugging
+				if apierrors.IsForbidden(err) {
+					klog.Warning(SyncSecretForbiddenWarning)
 				}
-				return true, nil
+				return false, nil
 			}
-			funcs = append(funcs, createFn)
+			return true, nil
 		}
+		funcs = append(funcs, createFn)
 
 		for _, f := range funcs {
 			if err := wait.ExponentialBackoff(wait.Backoff{
@@ -410,9 +396,9 @@ func (r *SecretProviderClassPodStatusReconciler) processIfBelongsToNode(objMeta
 	return true
 }
 
-// createK8sSecret creates K8s secret with data from mounted files
+// createOrUpdateK8sSecret creates K8s secret with data from mounted files
 // If a secret with the same name already exists in the namespace of the pod, the error is nil.
-func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
+func (r *SecretProviderClassPodStatusReconciler) createOrUpdateK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:   namespace,
@@ -430,6 +416,13 @@ func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Con
 		return nil
 	}
 	if apierrors.IsAlreadyExists(err) {
+		klog.InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
+		err := r.writer.Update(ctx, secret)
+		if err != nil {
+			klog.Errorf("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})
 		return nil
 	}
 	return err
@@ -477,23 +470,6 @@ func (r *SecretProviderClassPodStatusReconciler) patchSecretWithOwnerRef(ctx con
 	return nil
 }
 
-// secretExists checks if the secret with name and namespace already exists
-func (r *SecretProviderClassPodStatusReconciler) secretExists(ctx context.Context, name, namespace string) (bool, error) {
-	o := &corev1.Secret{}
-	secretKey := types.NamespacedName{
-		Namespace: namespace,
-		Name:      name,
-	}
-	err := r.Client.Get(ctx, secretKey, o)
-	if err == nil {
-		return true, nil
-	}
-	if apierrors.IsNotFound(err) {
-		return false, nil
-	}
-	return false, err
-}
-
 // generateEvent generates an event
 func (r *SecretProviderClassPodStatusReconciler) generateEvent(obj apiruntime.Object, eventType, reason, message string) {
 	if obj != nil {

controllers/secretproviderclasspodstatus_controller_test.go

@@ -121,31 +121,6 @@ func newReconciler(client client.Client, scheme *runtime.Scheme, nodeID string)
 	}
 }
 
-func TestSecretExists(t *testing.T) {
-	g := NewWithT(t)
-
-	scheme, err := setupScheme()
-	g.Expect(err).NotTo(HaveOccurred())
-
-	labels := map[string]string{"environment": "test"}
-	annotations := map[string]string{"kubed.appscode.com/sync": "app=test"}
-
-	initObjects := []client.Object{
-		newSecret("my-secret", "default", labels, annotations),
-	}
-
-	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build()
-	reconciler := newReconciler(client, scheme, "node1")
-
-	exists, err := reconciler.secretExists(context.TODO(), "my-secret", "default")
-	g.Expect(exists).To(Equal(true))
-	g.Expect(err).NotTo(HaveOccurred())
-
-	exists, err = reconciler.secretExists(context.TODO(), "my-secret2", "default")
-	g.Expect(exists).To(Equal(false))
-	g.Expect(err).NotTo(HaveOccurred())
-}
-
 func TestPatchSecretWithOwnerRef(t *testing.T) {
 	g := NewWithT(t)
 
@@ -183,7 +158,7 @@ func TestPatchSecretWithOwnerRef(t *testing.T) {
 	g.Expect(secret.GetOwnerReferences()).To(HaveLen(1))
 }
 
-func TestCreateK8sSecret(t *testing.T) {
+func TestCreateOrUpdateK8sSecret(t *testing.T) {
 	g := NewWithT(t)
 
 	scheme, err := setupScheme()
@@ -198,11 +173,11 @@ func TestCreateK8sSecret(t *testing.T) {
 	client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build()
 	reconciler := newReconciler(client, scheme, "node1")
 
-	// secret already exists
-	err = reconciler.createK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
+	// secret already exists, just update it.
+	err = reconciler.createOrUpdateK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
 	g.Expect(err).NotTo(HaveOccurred())
 
-	err = reconciler.createK8sSecret(context.TODO(), "my-secret2", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
+	err = reconciler.createOrUpdateK8sSecret(context.TODO(), "my-secret2", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
 	g.Expect(err).NotTo(HaveOccurred())
 	secret := &corev1.Secret{}
 	err = client.Get(context.TODO(), types.NamespacedName{Name: "my-secret2", Namespace: "default"}, secret)

deploy/csidriver.yaml

@@ -7,4 +7,3 @@ spec:
   attachRequired: false
   volumeLifecycleModes:
   - Ephemeral
-  requiresRepublish: true

manifest_staging/charts/secrets-store-csi-driver/templates/csidriver.yaml

@@ -11,6 +11,7 @@ spec:
   volumeLifecycleModes: 
   - Ephemeral
   {{- if and (semverCompare ">=1.20-0" .Capabilities.KubeVersion.Version) .Values.tokenRequests }}
+  requiresRepublish: true
   tokenRequests:
     {{- toYaml .Values.tokenRequests | nindent 2 }}
   {{- end }}

manifest_staging/deploy/csidriver.yaml

@@ -7,3 +7,4 @@ spec:
   attachRequired: false
   volumeLifecycleModes:
   - Ephemeral
+  requiresRepublish: true

pkg/secrets-store/nodeserver.go

@@ -93,15 +93,6 @@ 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")
@@ -128,6 +119,20 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	podNamespace = attrib[CSIPodNamespace]
 	podUID = attrib[CSIPodUID]
 
+	klog.InfoS("Checking object", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
+
+	if ns.rotationConfig.enabled {
+		lastModificationTime, err := ns.getLastUpdateTime(targetPath)
+		if err != nil {
+			klog.Infof("could not find last modification time for %s, error: %v\n", targetPath, err)
+		} else if startTime.Before(lastModificationTime.Add(ns.rotationConfig.interval)) {
+			// if next rotation is not yet, then skip the mount operation
+			return &csi.NodePublishVolumeResponse{}, nil
+		}
+	}
+
+	klog.InfoS("Processing object", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
+
 	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.
@@ -198,7 +203,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	if parameters[CSIPodServiceAccountTokens] == "" {
 		// Inject pod service account token into volume attributes
 		klog.Error("csi.storage.k8s.io/serviceAccount.tokens is not populated, set RequiresRepublish")
-
 	}
 
 	// ensure it's read-only

pkg/secrets-store/nodeserver_test.go

@@ -297,9 +297,8 @@ func TestNodePublishVolume(t *testing.T) {
 				},
 			},
 			rotationConfig: &RotationConfig{
-				enabled:          false,
-				nextRotationTime: time.Now(),
-				interval:         time.Minute,
+				enabled:  false,
+				interval: time.Minute,
 			},
 		},
 		{
@@ -332,9 +331,8 @@ 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,
+				enabled:  true,
+				interval: time.Minute,
 			},
 		},
 		{
@@ -364,9 +362,8 @@ func TestNodePublishVolume(t *testing.T) {
 				},
 			},
 			rotationConfig: &RotationConfig{
-				enabled:          true,
-				nextRotationTime: time.Now().Add(2 * time.Minute),
-				interval:         time.Minute,
+				enabled:  true,
+				interval: time.Minute,
 			},
 		},
 	}
@@ -410,8 +407,8 @@ func TestNodePublishVolume(t *testing.T) {
 					t.Fatalf("expected err to be nil, got: %v", err)
 				}
 				expectedMounts := 1
-				if ns.rotationConfig.enabled && ns.rotationConfig.nextRotationTime.After(time.Now()) {
-					// If rotation time is not reached, there should not be any mounts.
+				if ns.rotationConfig.enabled {
+					// If rotation time is not enabled, there should not be any mounts.
 					expectedMounts = 0
 				}
 				if len(mnts) != expectedMounts {

pkg/secrets-store/secrets-store.go

@@ -40,9 +40,8 @@ type SecretsStore struct {
 
 // RotationConfig stores the informarmation required to rotate the secrets.
 type RotationConfig struct {
-	enabled          bool
-	interval         time.Duration
-	nextRotationTime time.Time
+	enabled  bool
+	interval time.Duration
 }
 
 func NewSecretsStoreDriver(driverName, nodeID, endpoint string,
@@ -92,9 +91,8 @@ func newNodeServer(nodeID string,
 
 func NewRotationConfig(enabled bool, interval time.Duration) *RotationConfig {
 	return &RotationConfig{
-		enabled:          enabled,
-		interval:         interval,
-		nextRotationTime: time.Now(),
+		enabled:  enabled,
+		interval: interval,
 	}
 }
 

pkg/secrets-store/utils.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"strings"
+	"time"
 
 	secretsstorev1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1"
 	"sigs.k8s.io/secrets-store-csi-driver/pkg/util/runtimeutil"
@@ -75,6 +76,14 @@ func (ns *nodeServer) ensureMountPoint(target string) (bool, error) {
 	return false, nil
 }
 
+func (ns *nodeServer) getLastUpdateTime(target string) (time.Time, error) {
+	info, err := os.Stat(target)
+	if err != nil {
+		return time.Time{}, err
+	}
+	return info.ModTime(), nil
+}
+
 // getSecretProviderItem returns the secretproviderclass object by name and namespace
 func getSecretProviderItem(ctx context.Context, c client.Client, name, namespace string) (*secretsstorev1.SecretProviderClass, error) {
 	spc := &secretsstorev1.SecretProviderClass{}

test/bats/aws.bats

@@ -81,7 +81,7 @@ teardown_file() {
    [[ "${result//$'\r'}" == "BeforeRotation" ]]
 
    aws ssm put-parameter --name $PM_ROTATION_TEST_NAME --value AfterRotation --type SecureString --overwrite --region $REGION
-   sleep 40
+   sleep 180
    result=$(kubectl --namespace $NAMESPACE exec $POD_NAME -- cat /mnt/secrets-store/$PM_ROTATION_TEST_NAME)
    [[ "${result//$'\r'}" == "AfterRotation" ]]
 }
@@ -91,7 +91,7 @@ teardown_file() {
    [[ "${result//$'\r'}" == "BeforeRotation" ]]
 
    aws secretsmanager put-secret-value --secret-id $SM_ROT_TEST_NAME --secret-string AfterRotation --region $REGION
-   sleep 40
+   sleep 180
    result=$(kubectl --namespace $NAMESPACE exec $POD_NAME -- cat /mnt/secrets-store/$SM_ROT_TEST_NAME)
    [[ "${result//$'\r'}" == "AfterRotation" ]]
 }

test/bats/azure.bats

@@ -193,7 +193,7 @@ setup() {
   assert_success
 
   envsubst < $BATS_TESTS_DIR/deployment-synck8s-azure.yaml | kubectl apply -n negative-test-ns -f -
-  sleep 5
+  sleep 30
 
   POD=$(kubectl get pod -l app=busybox -n negative-test-ns -o jsonpath="{.items[0].metadata.name}")
   cmd="kubectl describe pod $POD -n negative-test-ns | grep 'FailedMount.*failed to get secretproviderclass negative-test-ns/azure-sync.*not found'"