fix: update cert secret if exists (#401)

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## Unreleased
 
+Bugs:
+* Preserve metadata when updating the cert secret [GH-401](https://github.com/hashicorp/vault-k8s/pull/401)
+
 ## 1.1.0 (November 17, 2022)
 
 Changes:

helper/cert/source_gen.go

@@ -236,25 +236,31 @@ func (s *GenSource) retryUpdateSecret(ctx context.Context, leaderCh chan bool, b
 }
 
 func (s *GenSource) updateSecret(ctx context.Context, bundle Bundle) error {
-	secret := &v1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: certSecretName,
-		},
-		Data: map[string][]byte{
-			"cert": bundle.Cert,
-			"key":  bundle.Key,
-		},
-	}
-	// Attempt updating the Secret first, and if it doesn't exist, fallback to
-	// create
-	_, err := s.K8sClient.CoreV1().Secrets(s.Namespace).Update(ctx, secret, metav1.UpdateOptions{})
+	secret, err := s.SecretsCache.Lister().Secrets(s.Namespace).Get(certSecretName)
+	if err != nil && !errors.IsNotFound(err) {
+		return err
+	}
 	if errors.IsNotFound(err) {
+		// The Secret does not exist, so create it.
+		secret = &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: certSecretName,
+			},
+			Data: map[string][]byte{
+				"cert": bundle.Cert,
+				"key":  bundle.Key,
+			},
+		}
 		_, err = s.K8sClient.CoreV1().Secrets(s.Namespace).Create(ctx, secret, metav1.CreateOptions{})
-	}
-	if err != nil {
 		return err
 	}
-	return nil
+	// The Secret exists, so update it.
+	secret.Data = map[string][]byte{
+		"cert": bundle.Cert,
+		"key":  bundle.Key,
+	}
+	_, err = s.K8sClient.CoreV1().Secrets(s.Namespace).Update(ctx, secret, metav1.UpdateOptions{})
+	return err
 }
 
 func (s *GenSource) getBundleFromSecret() (Bundle, error) {

helper/cert/source_gen_test.go

@@ -142,12 +142,23 @@ func TestGenSource_leader(t *testing.T) {
 
 	source.Namespace = "default"
 	source.K8sClient = fake.NewSimpleClientset()
-	bundle, err := source.Certificate(context.Background(), nil)
+
+	// setup a Secret informer cache with the fake clientset for the leader to use
+	factory := informers.NewSharedInformerFactoryWithOptions(source.K8sClient, 0, informers.WithNamespace(source.Namespace))
+	secrets := factory.Core().V1().Secrets()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go secrets.Informer().Run(ctx.Done())
+	synced := cache.WaitForCacheSync(ctx.Done(), secrets.Informer().HasSynced)
+	require.True(t, synced, "timeout syncing Secrets informer")
+	source.SecretsCache = secrets
+
+	bundle, err := source.Certificate(ctx, nil)
 	require.NoError(t, err)
 	testBundleVerify(t, &bundle)
 
 	// check that the Secret has been created
-	checkSecret, err := source.K8sClient.CoreV1().Secrets(source.Namespace).Get(context.Background(), certSecretName, metav1.GetOptions{})
+	checkSecret, err := source.K8sClient.CoreV1().Secrets(source.Namespace).Get(ctx, certSecretName, metav1.GetOptions{})
 	require.NoError(t, err)
 	require.Equal(t, checkSecret.Data["cert"], bundle.Cert,
 		"cert in the Secret should've matched what was returned from source.Certificate()",
@@ -157,6 +168,77 @@ func TestGenSource_leader(t *testing.T) {
 	)
 }
 
+func TestGenSource_leader_expiry(t *testing.T) {
+
+	if !hasOpenSSL {
+		t.Skip("openssl not found")
+		return
+	}
+
+	source := testGenSource()
+	source.Expiry = 5 * time.Second
+	source.ExpiryWithin = 2 * time.Second
+	source.Namespace = "default"
+	source.K8sClient = fake.NewSimpleClientset()
+
+	// Pretend this host is the leader
+	source.LeaderElector = newFakeLeader(true)
+
+	// setup a Secret informer cache with the fake clientset for the leader to use
+	factory := informers.NewSharedInformerFactoryWithOptions(source.K8sClient, 0, informers.WithNamespace(source.Namespace))
+	secrets := factory.Core().V1().Secrets()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	go secrets.Informer().Run(ctx.Done())
+	synced := cache.WaitForCacheSync(ctx.Done(), secrets.Informer().HasSynced)
+	require.True(t, synced, "timeout syncing Secrets informer")
+	source.SecretsCache = secrets
+
+	// Generate the first bundle, storing it in a secret
+	bundle, err := source.Certificate(ctx, nil)
+	require.NoError(t, err)
+	testBundleVerify(t, &bundle)
+
+	// Check that the Secret has been created correctly
+	secret, err := source.K8sClient.CoreV1().Secrets(source.Namespace).Get(ctx, certSecretName, metav1.GetOptions{})
+	require.NoError(t, err)
+	require.Equal(t, secret.Data["cert"], bundle.Cert,
+		"cert in the Secret should've matched what was returned from source.Certificate()",
+	)
+	require.Equal(t, secret.Data["key"], bundle.Key,
+		"key in the Secret should've matched what was returned from source.Certificate()",
+	)
+
+	// Add some metadata to the Secret
+	addedAnnotations := map[string]string{
+		"not-owner-by-vault-agent": "some-value",
+	}
+	secret.SetAnnotations(addedAnnotations)
+	_, err = source.K8sClient.CoreV1().Secrets(source.Namespace).Update(ctx, secret, metav1.UpdateOptions{})
+	require.NoError(t, err)
+
+	// Generate a new bundle, updating the Secret
+	next, err := source.Certificate(context.Background(), &bundle)
+	require.NoError(t, err)
+	require.False(t, bundle.Equal(&next))
+	testBundleVerify(t, &next)
+
+	// Check that the secret contains the new bundle data
+	secret, err = source.K8sClient.CoreV1().Secrets(source.Namespace).Get(ctx, certSecretName, metav1.GetOptions{})
+	require.NoError(t, err)
+	require.Equal(t, secret.Data["cert"], next.Cert,
+		"cert in the Secret should've matched what was returned from source.Certificate()",
+	)
+	require.Equal(t, secret.Data["key"], next.Key,
+		"key in the Secret should've matched what was returned from source.Certificate()",
+	)
+
+	// Check that the secret still has the metadata added prior to its update
+	require.Equal(t, addedAnnotations, secret.Annotations,
+		"updating the secret should not remove metadata like annotations",
+	)
+}
+
 func TestGenSource_follower(t *testing.T) {
 
 	if !hasOpenSSL {