Merge pull request #540 from alexandrevilain/fix/secret-copier

fix(SecretCopier): update secret when orginal secret is updated
alexandrevilain · Oct 31, 2023 · 218ea6f · 218ea6f
2 parents 7f15150 + a4411a0
commit 218ea6f
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 7 deletions.
diff --git a/pkg/kubernetes/secret.go b/pkg/kubernetes/secret.go
@@ -47,21 +47,30 @@ func (c *SecretCopier) Copy(ctx context.Context, owner client.Object, original c
 		return fmt.Errorf("can't retrieve original secret: %w", err)
 	}
 
-	destinationSecret := secret.DeepCopy()
-	// Override object meta to ensure no UUID or resource version can conflict.
-	destinationSecret.ObjectMeta = metav1.ObjectMeta{
+	secretMeta := metav1.ObjectMeta{
 		Name:        secret.GetName(),
 		Namespace:   destinationNS,
 		Labels:      secret.Labels,
 		Annotations: secret.Annotations,
 	}
 
-	err = controllerutil.SetOwnerReference(owner, destinationSecret, c.scheme)
-	if err != nil {
-		return fmt.Errorf("failed setting controller reference: %w", err)
-	}
+	destinationSecret := &corev1.Secret{}
+	destinationSecret.ObjectMeta = secretMeta
 
 	_, err = controllerutil.CreateOrUpdate(ctx, c.Client, destinationSecret, func() error {
+		destinationSecret.Labels = secretMeta.Labels
+		destinationSecret.Annotations = secretMeta.Annotations
+
+		destinationSecret.Data = secret.Data
+		destinationSecret.StringData = secret.StringData
+		destinationSecret.Immutable = secret.Immutable
+		destinationSecret.Type = secret.Type
+
+		err = controllerutil.SetOwnerReference(owner, destinationSecret, c.scheme)
+		if err != nil {
+			return fmt.Errorf("failed setting controller reference: %w", err)
+		}
+
 		return nil
 	})
 	if err != nil {

diff --git a/pkg/kubernetes/secret_test.go b/pkg/kubernetes/secret_test.go
@@ -38,6 +38,7 @@ func TestSecretCopier(t *testing.T) {
 	tests := map[string]struct {
 		original    client.Object
 		owner       client.Object
+		update      client.Object
 		destination string
 		expected    client.Object
 		expectedErr string
@@ -84,6 +85,57 @@ func TestSecretCopier(t *testing.T) {
 				},
 			},
 		},
+		"works when secret is updated": {
+			original: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "test",
+				},
+				StringData: map[string]string{
+					"test": "test",
+				},
+			},
+			update: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "test",
+				},
+				StringData: map[string]string{
+					"test": "new-value",
+				},
+			},
+			owner: &v1beta1.TemporalCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "fakecluster",
+					Namespace: "default",
+				},
+				Spec: v1beta1.TemporalClusterSpec{
+					Version: version.MustNewVersionFromString("1.20.0"),
+				},
+			},
+			destination: "default",
+			expected: &corev1.Secret{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "Secret",
+					APIVersion: "v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test",
+					Namespace: "default",
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							APIVersion: "temporal.io/v1beta1",
+							Kind:       "TemporalCluster",
+							Name:       "fakecluster",
+						},
+					},
+					ResourceVersion: "2",
+				},
+				StringData: map[string]string{
+					"test": "new-value",
+				},
+			},
+		},
 		"error with cross namespace owner reference": {
 			original: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
@@ -126,6 +178,14 @@ func TestSecretCopier(t *testing.T) {
 				return
 			}
 			assert.NoError(tt, err)
+
+			if test.update != nil {
+				err := fakeClient.Update(ctx, test.update)
+				require.NoError(tt, err)
+				err = copier.Copy(ctx, test.owner, client.ObjectKeyFromObject(test.update), test.destination)
+				assert.NoError(tt, err)
+			}
+
 			result := &corev1.Secret{}
 			require.NoError(tt, fakeClient.Get(ctx, client.ObjectKey{Name: test.original.GetName(), Namespace: test.destination}, result))