Increase sleep time to get enough time for rotation.

controllers/secretproviderclasspodstatus_controller.go

@@ -42,6 +42,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -295,58 +296,45 @@ 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)
 
-		if !exists {
-			secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))
+		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 +398,9 @@ func (r *SecretProviderClassPodStatusReconciler) processIfBelongsToNode(objMeta
 	return true
 }
 
-// createK8sSecret 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 {
+// createOrUpdateK8sSecret creates K8s secret with data from mounted files if not exists.
+// If a secret with the same name already exists in the namespace of the pod, it'll be updated.
+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,
@@ -424,14 +412,11 @@ func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Con
 		Data: datamap,
 	}
 
-	err := r.writer.Create(ctx, secret)
-	if err == nil {
-		klog.InfoS("successfully created Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
-		return nil
-	}
-	if apierrors.IsAlreadyExists(err) {
+	_, err := controllerutil.CreateOrUpdate(ctx, r.Client, secret, func() error {
+		// if already exists, update the data
+		secret.Data = datamap
 		return nil
-	}
+	})
 	return err
 }
 
@@ -477,23 +462,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()
@@ -199,10 +174,10 @@ func TestCreateK8sSecret(t *testing.T) {
 	reconciler := newReconciler(client, scheme, "node1")
 
 	// secret already exists
-	err = reconciler.createK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
+	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)

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

@@ -11,7 +11,6 @@ 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,4 +7,3 @@ spec:
   attachRequired: false
   volumeLifecycleModes:
   - Ephemeral
-  requiresRepublish: true

pkg/secrets-store/nodeserver_test.go

@@ -228,7 +228,7 @@ func TestNodePublishVolume_Errors(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, &RotationConfig{enabled: false})
 			if err != nil {
 				t.Fatalf("expected error to be nil, got: %+v", err)
 			}