diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index c692f7ac4f..5fd363896f 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -244,7 +244,7 @@ func (v *VaultProvider) renewToken(ctx context.Context, watcher *vaultapi.Lifeti case err := <-watcher.DoneCh(): // Watcher has stopped if err != nil { - v.logger.Error("Error renewing token for Vault provider", "error", err, "fails", retrier.Failures()) + v.logger.Error("Error renewing token for Vault provider", "error", err, "retries", retrier.Failures()) } // Although the vault watcher has its own retry logic, we have encountered diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index ece7659d04..ae862b0452 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "io" + "runtime/pprof" "strconv" "strings" "sync/atomic" @@ -237,8 +238,68 @@ func TestVaultCAProvider_Configure(t *testing.T) { testcase.expectedValue(t, provider) }) } +} + +// This test must not run in parallel +func TestVaultCAProvider_ConfigureWithBadToken(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + SkipIfVaultNotPresent(t) + + testVault := NewTestVaultServer(t) + + attr := &VaultTokenAttributes{ + RootPath: "pki-root", + IntermediatePath: "pki-intermediate", + ConsulManaged: true, + } + token := CreateVaultTokenWithAttrs(t, testVault.client, attr) - return + provider := NewVaultProvider(hclog.New(&hclog.LoggerOptions{Name: "ca.vault"})) + + t.Run("bad token does not leak renewal routine on Configure", func(t *testing.T) { + config := map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "badbadbad/", + } + cfg := vaultProviderConfig(t, testVault.Addr, token, config) + + err := provider.Configure(cfg) + require.Error(t, err) + + retry.RunWith(retry.TwoSeconds(), t, func(r *retry.R) { + profile := pprof.Lookup("goroutine") + sb := strings.Builder{} + require.NoError(r, profile.WriteTo(&sb, 2)) + require.NotContains(r, sb.String(), + "created by github.com/hashicorp/consul/agent/connect/ca.(*VaultProvider).Configure", + "found renewal goroutine leak") + // If this test is failing because you added a new goroutine to + // (*VaultProvider).Configure AND that goroutine should persist + // even if Configure errored, then you should change the checked + // string to (*VaultProvider).renewToken. + }) + }) + + t.Run("correct token starts renewal routine", func(t *testing.T) { + config := map[string]any{ + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + } + cfg := vaultProviderConfig(t, testVault.Addr, token, config) + + require.NoError(t, provider.Configure(cfg)) + + retry.RunWith(retry.TwoSeconds(), t, func(r *retry.R) { + profile := pprof.Lookup("goroutine") + sb := strings.Builder{} + require.NoError(r, profile.WriteTo(&sb, 2)) + require.Contains(r, sb.String(), + "created by github.com/hashicorp/consul/agent/connect/ca.(*VaultProvider).Configure", + "expected renewal goroutine, got none") + }) + }) } func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) {