VDS: remediate deleted destination secret

Previously, the VDS controller did not detect when the destination
secret had been deleted after it had initially created it.

Fixes:
- ensure a deleted destination secret that is owned by the VDS
  controller results in a re-sync.
- add integration test to test this use-case
- update all other controller integration tests to test for destination
  deletion

controllers/vaultdynamicsecret_controller.go

@@ -111,9 +111,16 @@ func (r *VaultDynamicSecretReconciler) Reconcile(ctx context.Context, req ctrl.R
 		}
 	}
 
+	destExists, _ := helpers.CheckSecretExists(ctx, r.Client, o)
+	if !o.Spec.Destination.Create && !destExists {
+		logger.Info("Destination secret does not exist, either create it or "+
+			"set .spec.destination.create=true", "destination", o.Spec.Destination)
+		return ctrl.Result{RequeueAfter: requeueDurationOnError}, nil
+	}
+
 	// doSync indicates that the controller should perform the secret sync,
 	// skipping any lease renewals.
-	doSync := o.GetGeneration() != o.Status.LastGeneration
+	doSync := (o.GetGeneration() != o.Status.LastGeneration) || (o.Spec.Destination.Create && !destExists)
 	leaseID := o.Status.SecretLease.ID
 	if !doSync && r.runtimePodUID != "" && r.runtimePodUID != o.Status.LastRuntimePodUID {
 		// don't take part in the thundering herd on start up,

test/integration/hcpvaultsecretsapp_integration_test.go

@@ -13,6 +13,7 @@ import (
 	"path"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/google/uuid"
 	"github.com/gruntwork-io/terratest/modules/k8s"
@@ -333,6 +334,12 @@ func TestHCPVaultSecretsApp(t *testing.T) {
 					}
 				}
 			}
+
+			d, err := time.ParseDuration(obj.Spec.RefreshAfter)
+			if assert.NoError(t, err, "time.ParseDuration(%v)", obj.Spec.RefreshAfter) {
+				assertRemediationOnDestinationDeletion(t, ctx, crdClient, obj,
+					time.Millisecond*500, uint64(d.Seconds()*3))
+			}
 		})
 	}
 }

test/integration/integration_test.go

@@ -32,6 +32,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	authenticationv1 "k8s.io/api/authentication/v1"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrlruntime "k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/intstr"
@@ -61,6 +62,8 @@ var (
 	entTests           = os.Getenv("ENT_TESTS") != ""
 	// use the Helm chart to deploy the operator. The default is to use Kustomize.
 	testWithHelm = os.Getenv("TEST_WITH_HELM") != ""
+	// make tests more verbose
+	withExtraVerbosity = os.Getenv("WITH_EXTRA_VERBOSITY") != ""
 	// set in TestMain
 	clusterName       string
 	operatorImageRepo string
@@ -139,7 +142,11 @@ func setCommonTFOptions(t *testing.T, opts *terraform.Options) *terraform.Option
 	if os.Getenv("SUPPRESS_TF_OUTPUT") != "" {
 		opts.Logger = logger.Discard
 	}
-	return terraform.WithDefaultRetryableErrors(t, opts)
+
+	opts = terraform.WithDefaultRetryableErrors(t, opts)
+	opts.MaxRetries = 30
+	opts.TimeBetweenRetries = time.Millisecond * 500
+	return opts
 }
 
 func getVaultClient(t *testing.T, namespace string) *api.Client {
@@ -313,16 +320,19 @@ type authMethodsK8sOutputs struct {
 }
 
 type dynamicK8SOutputs struct {
-	NamePrefix       string `json:"name_prefix"`
-	Namespace        string `json:"namespace"`
-	K8sNamespace     string `json:"k8s_namespace"`
-	K8sConfigContext string `json:"k8s_config_context"`
-	AuthMount        string `json:"auth_mount"`
-	AuthPolicy       string `json:"auth_policy"`
-	AuthRole         string `json:"auth_role"`
-	DBRole           string `json:"db_role"`
-	DBRoleStatic     string `json:"db_role_static"`
-	DBRoleStaticUser string `json:"db_role_static_user"`
+	NamePrefix              string `json:"name_prefix"`
+	Namespace               string `json:"namespace"`
+	K8sNamespace            string `json:"k8s_namespace"`
+	K8sConfigContext        string `json:"k8s_config_context"`
+	AuthMount               string `json:"auth_mount"`
+	AuthPolicy              string `json:"auth_policy"`
+	AuthRole                string `json:"auth_role"`
+	DBRole                  string `json:"db_role"`
+	DBRoleStatic            string `json:"db_role_static"`
+	DefaultLeaseTTLSeconds  int    `json:"default_lease_ttl_seconds"`
+	DBRoleStaticUser        string `json:"db_role_static_user"`
+	StaticRotationPeriod    int    `json:"static_rotation_period"`
+	NonRenewableK8STokenTTL int    `json:"non_renewable_k8s_token_ttl"`
 	// should always be non-renewable
 	K8SSecretPath  string `json:"k8s_secret_path"`
 	K8SSecretRole  string `json:"k8s_secret_role"`
@@ -754,3 +764,66 @@ func createDeployment(t *testing.T, ctx context.Context, client ctrlclient.Clien
 
 	return depObj
 }
+
+func assertRemediationOnDestinationDeletion(t *testing.T, ctx context.Context, client ctrlclient.Client,
+	obj ctrlclient.Object, delay time.Duration, maxTries uint64,
+) bool {
+	t.Helper()
+
+	m, err := common.NewSyncableSecretMetaData(obj)
+	if !assert.NoError(t, err, "common.NewSyncableSecretMetaData(%v)", obj) {
+		return false
+	}
+
+	objKey := ctrlclient.ObjectKey{
+		Namespace: obj.GetNamespace(),
+		Name:      m.Destination.Name,
+	}
+
+	t.Logf("assertRemediationOnDestinationDeletion() objKey=%s, delay=%s, maxTries=%d", objKey, delay, maxTries)
+	orig, err := helpers.GetSecret(ctx, client, objKey)
+	if !assert.NoErrorf(t, err,
+		"helpers.GetSecret(%v, %v, %v)", ctx, objKey, &orig) {
+		return false
+	}
+
+	if !assert.NoError(t, client.Delete(ctx, orig)) {
+		return false
+	}
+
+	return assert.NoError(t, backoff.RetryNotify(func() error {
+		got, err := helpers.GetSecret(ctx, client, objKey)
+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				return err
+			} else {
+				return backoff.Permanent(err)
+			}
+		}
+
+		if got == nil {
+			return backoff.Permanent(fmt.Errorf(
+				"both secret and error are nil, should not be possible"))
+		}
+
+		if orig.GetUID() == got.GetUID() {
+			return fmt.Errorf("got the same secret after deletion %s", objKey)
+		}
+
+		assert.Equal(t, orig.Labels, got.Labels)
+		assert.NotEmpty(t, orig.GetUID(), "invalid Secret %v", orig)
+		assert.NotEmpty(t, got.GetUID(), "invalid Secret %v", got)
+		if !t.Failed() {
+			assert.NotEqual(t, orig.GetUID(), got.GetUID(),
+				"new Secret %v has the same UID as its predecessor %v", got, orig)
+		}
+
+		return nil
+	}, backoff.WithMaxRetries(backoff.NewConstantBackOff(delay), maxTries),
+		func(err error, d time.Duration) {
+			if withExtraVerbosity {
+				t.Logf("assertRemediationOnDestinationDeletion() got error %v, retry delay=%s", err, d)
+			}
+		},
+	))
+}

test/integration/vaultdynamicsecret/terraform/outputs.tf

@@ -52,3 +52,15 @@ output "k8s_config_context" {
 output "namespace" {
   value = local.namespace
 }
+
+output "static_rotation_period" {
+  value = vault_database_secret_backend_static_role.postgres.rotation_period
+}
+
+output "default_lease_ttl_seconds" {
+  value = vault_database_secrets_mount.db.default_lease_ttl_seconds
+}
+
+output "non_renewable_k8s_token_ttl" {
+  value = vault_kubernetes_secret_backend_role.k8s_secrets.token_default_ttl
+}

test/integration/vaultdynamicsecret_integration_test.go

@@ -72,7 +72,7 @@ func TestVaultDynamicSecret(t *testing.T) {
 			"vault_address":              os.Getenv("VAULT_ADDRESS"),
 			"vault_token":                os.Getenv("VAULT_TOKEN"),
 			"vault_token_period":         120,
-			"vault_db_default_lease_ttl": 30,
+			"vault_db_default_lease_ttl": 15,
 		},
 	}
 	if entTests {
@@ -402,7 +402,7 @@ func TestVaultDynamicSecret(t *testing.T) {
 						Params: map[string]string{
 							"kubernetes_namespace": outputs.K8sNamespace,
 						},
-						RenewalPercent: 5,
+						RenewalPercent: 1,
 						Path:           "creds/" + outputs.K8SSecretRole,
 						Destination: secretsv1beta1.Destination{
 							Name:   dest,
@@ -484,17 +484,33 @@ func TestVaultDynamicSecret(t *testing.T) {
 						"expected Status.LastGeneration")
 					assert.NotEmpty(t, vdsObjFinal.Status.LastRuntimePodUID)
 					assert.NotEmpty(t, vdsObjFinal.Status.LastRenewalTime)
+
+					// for a 1s interval between tries
+					var maxRetriesForRemediation uint64
 					if vdsObjFinal.Spec.AllowStaticCreds {
 						assert.Empty(t, vdsObjFinal.Status.SecretLease.ID)
+						maxRetriesForRemediation = uint64(outputs.StaticRotationPeriod)
 					} else {
 						assert.NotEmpty(t, vdsObjFinal.Status.SecretLease.ID)
+						var ttl float64
+						if vdsObjFinal.Status.SecretLease.Renewable {
+							ttl = float64(outputs.DefaultLeaseTTLSeconds)
+						} else {
+							ttl = float64(outputs.NonRenewableK8STokenTTL)
+						}
+						maxRetriesForRemediation = uint64(ttl*.10 + (ttl * (float64(vdsObjFinal.Spec.RenewalPercent) / 100)))
 					}
 
 					assertLastRuntimePodUID(t, ctx, crdClient, operatorNS, vdsObjFinal)
 					assertDynamicSecretRotation(t, ctx, crdClient, vdsObjFinal)
 
 					if vdsObjFinal.Spec.Destination.Create && !t.Failed() {
-						assertDynamicSecretNewGeneration(t, ctx, crdClient, vdsObjFinal)
+						// must be called before assertDynamicSecretNewGeneration, since
+						// that function changes the destination secret's name.
+						if assertRemediationOnDestinationDeletion(t, ctx, crdClient, obj,
+							time.Millisecond*500, maxRetriesForRemediation*3) {
+							assertDynamicSecretNewGeneration(t, ctx, crdClient, vdsObjFinal)
+						}
 					}
 				})
 			}
@@ -579,7 +595,7 @@ func assertDynamicSecretNewGeneration(t *testing.T,
 				}
 				return nil
 			},
-				backoff.WithMaxRetries(backoff.NewConstantBackOff(time.Second*1), 10)),
+				backoff.WithMaxRetries(backoff.NewConstantBackOff(time.Millisecond*500), 20)),
 			)
 
 			if !t.Failed() {

test/integration/vaultpkisecret_integration_test.go

@@ -268,7 +268,7 @@ func TestVaultPKISecret(t *testing.T) {
 						CommonName:   fmt.Sprintf("%s.example.com", dest),
 						Format:       "pem",
 						Revoke:       true,
-						ExpiryOffset: "1s",
+						ExpiryOffset: "2s",
 						TTL:          "10s",
 						VaultAuthRef: testVaultAuthMethodName,
 						AltNames:     []string{"alt1.example.com", "alt2.example.com"},
@@ -332,6 +332,14 @@ func TestVaultPKISecret(t *testing.T) {
 					if len(vpsObj.Spec.RolloutRestartTargets) > 0 {
 						awaitRolloutRestarts(t, ctx, crdClient, vpsObj, vpsObj.Spec.RolloutRestartTargets)
 					}
+
+					if vpsObj.Spec.Destination.Create && !t.Failed() {
+						d, err := time.ParseDuration(vpsObj.Spec.TTL)
+						if assert.NoError(t, err) {
+							assertRemediationOnDestinationDeletion(t, ctx, crdClient, vpsObj,
+								time.Millisecond*500, uint64(d.Seconds()*3))
+						}
+					}
 				})
 			}
 

test/integration/vaultstaticsecret_integration_test.go

@@ -33,7 +33,7 @@ import (
 	"github.com/hashicorp/vault-secrets-operator/internal/vault"
 )
 
-func TestVaultStaticSecret_kv(t *testing.T) {
+func TestVaultStaticSecret(t *testing.T) {
 	testID := strings.ToLower(random.UniqueId())
 	testK8sNamespace := "k8s-tenant-" + testID
 	testK8sNamespace2 := testK8sNamespace + "-test"
@@ -468,6 +468,14 @@ func TestVaultStaticSecret_kv(t *testing.T) {
 
 						assertSync(t, vssObj, expected, true)
 						assertSync(t, vssObj, expected, false)
+
+						if vssObj.Spec.RefreshAfter != "" && !t.Failed() {
+							d, err := time.ParseDuration(vssObj.Spec.RefreshAfter)
+							if assert.NoError(t, err, "time.ParseDuration(%v)", vssObj.Spec.RefreshAfter) {
+								assertRemediationOnDestinationDeletion(t, ctx, crdClient, vssObj,
+									time.Millisecond*500, uint64(d.Seconds()*3))
+							}
+						}
 					})
 				}
 			}