diff --git a/CHANGELOG.md b/CHANGELOG.md index 93a62d86..570ccf2a 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/helper/cert/source_gen.go b/helper/cert/source_gen.go index 39a7afe3..716043dd 100644 --- a/helper/cert/source_gen.go +++ b/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) { diff --git a/helper/cert/source_gen_test.go b/helper/cert/source_gen_test.go index 058c291d..a0c3edd1 100644 --- a/helper/cert/source_gen_test.go +++ b/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 {