From 968fd8660d7e5de68697cf2f5a5455987aef6c45 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 9 Oct 2020 05:02:18 -0700 Subject: [PATCH] Update CI for leader renew CA test using Vault --- GNUmakefile | 2 ++ agent/connect/ca/provider.go | 2 +- agent/connect/ca/provider_vault.go | 21 ++++++++-------- agent/connect/ca/provider_vault_test.go | 32 ++++++------------------- agent/connect/ca/testing.go | 16 +++++++++++++ agent/consul/leader_connect_test.go | 8 ++++--- 6 files changed, 42 insertions(+), 39 deletions(-) diff --git a/GNUmakefile b/GNUmakefile index 7c4fe143b..3253280e2 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -357,6 +357,8 @@ test-connect-ca-providers: ifeq ("$(CIRCLECI)","true") # Run in CI gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report.xml" -- -cover -coverprofile=coverage.txt ./agent/connect/ca +# Run leader tests that require Vault + gotestsum --format=short-verbose --junitfile "$(TEST_RESULTS_DIR)/gotestsum-report-leader.xml" -- -cover -coverprofile=coverage-leader.txt -run TestLeader_Vault_ ./agent/consul else # Run locally @echo "Running /agent/connect/ca tests in verbose mode" diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 1dd1408cf..17538ee6f 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -20,7 +20,7 @@ var ErrRateLimited = errors.New("operation rate limited by CA provider") // intermediate cert in the primary datacenter as well as the secondary. This is used // when determining whether to run the intermediate renewal routine in the primary. var PrimaryIntermediateProviders = map[string]struct{}{ - "vault": struct{}{}, + "vault": {}, } // ProviderConfig encapsulates all the data Consul passes to `Configure` on a diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 0eee7d6d2..c74a2e460 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -93,7 +93,7 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error { // Set up a renewer to renew the token automatically, if supported. if token.Renewable { - renewer, err := client.NewRenewer(&vaultapi.RenewerInput{ + lifetimeWatcher, err := client.NewLifetimeWatcher(&vaultapi.LifetimeWatcherInput{ Secret: &vaultapi.Secret{ Auth: &vaultapi.SecretAuth{ ClientToken: config.Token, @@ -101,7 +101,8 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error { LeaseDuration: secret.LeaseDuration, }, }, - Increment: token.TTL, + Increment: token.TTL, + RenewBehavior: vaultapi.RenewBehaviorIgnoreErrors, }) if err != nil { return fmt.Errorf("Error beginning Vault provider token renewal: %v", err) @@ -109,31 +110,31 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error { ctx, cancel := context.WithCancel(context.TODO()) v.shutdown = cancel - go v.renewToken(ctx, renewer) + go v.renewToken(ctx, lifetimeWatcher) } return nil } // renewToken uses a vaultapi.Renewer to repeatedly renew our token's lease. -func (v *VaultProvider) renewToken(ctx context.Context, renewer *vaultapi.Renewer) { - go renewer.Renew() - defer renewer.Stop() +func (v *VaultProvider) renewToken(ctx context.Context, watcher *vaultapi.LifetimeWatcher) { + go watcher.Start() + defer watcher.Stop() for { select { case <-ctx.Done(): return - case err := <-renewer.DoneCh(): + case err := <-watcher.DoneCh(): if err != nil { v.logger.Error("Error renewing token for Vault provider", "error", err) } - // Renewer routine has finished, so start it again. - go renewer.Renew() + // Watcher routine has finished, so start it again. + go watcher.Start() - case <-renewer.RenewCh(): + case <-watcher.RenewCh(): v.logger.Error("Successfully renewed token for Vault provider") } } diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index b1890e08e..d14ceeefc 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -5,8 +5,6 @@ import ( "encoding/json" "fmt" "io/ioutil" - "os" - "os/exec" "testing" "time" @@ -40,7 +38,7 @@ func TestVaultCAProvider_VaultTLSConfig(t *testing.T) { func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) provider, testVault := testVaultProviderWithConfig(t, false, nil) defer testVault.Stop() @@ -53,7 +51,7 @@ func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { func TestVaultCAProvider_RenewToken(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) testVault, err := runTestVault(t) require.NoError(t, err) @@ -90,7 +88,7 @@ func TestVaultCAProvider_RenewToken(t *testing.T) { func TestVaultCAProvider_Bootstrap(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) provider, testVault := testVaultProvider(t) defer testVault.Stop() @@ -151,7 +149,7 @@ func assertCorrectKeyType(t *testing.T, want, certPEM string) { func TestVaultCAProvider_SignLeaf(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) for _, tc := range KeyTestCases { tc := tc @@ -235,7 +233,7 @@ func TestVaultCAProvider_SignLeaf(t *testing.T) { func TestVaultCAProvider_CrossSignCA(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) tests := CASigningKeyTypeCases() @@ -290,7 +288,7 @@ func TestVaultCAProvider_CrossSignCA(t *testing.T) { func TestVaultProvider_SignIntermediate(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) tests := CASigningKeyTypeCases() @@ -319,7 +317,7 @@ func TestVaultProvider_SignIntermediate(t *testing.T) { func TestVaultProvider_SignIntermediateConsul(t *testing.T) { t.Parallel() - skipIfVaultNotPresent(t) + SkipIfVaultNotPresent(t) // primary = Vault, secondary = Consul t.Run("pri=vault,sec=consul", func(t *testing.T) { @@ -441,19 +439,3 @@ func createVaultProvider(t *testing.T, isPrimary bool, addr, token string, rawCo return provider, nil } - -// skipIfVaultNotPresent skips the test if the vault binary is not in PATH. -// -// These tests may be skipped in CI. They are run as part of a separate -// integration test suite. -func skipIfVaultNotPresent(t *testing.T) { - vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") - if vaultBinaryName == "" { - vaultBinaryName = "vault" - } - - path, err := exec.LookPath(vaultBinaryName) - if err != nil || path == "" { - t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName) - } -} diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go index 81995275f..25533f8dd 100644 --- a/agent/connect/ca/testing.go +++ b/agent/connect/ca/testing.go @@ -83,6 +83,22 @@ func TestConsulProvider(t testing.T, d ConsulProviderStateDelegate) *ConsulProvi return provider } +// SkipIfVaultNotPresent skips the test if the vault binary is not in PATH. +// +// These tests may be skipped in CI. They are run as part of a separate +// integration test suite. +func SkipIfVaultNotPresent(t testing.T) { + vaultBinaryName := os.Getenv("VAULT_BINARY_NAME") + if vaultBinaryName == "" { + vaultBinaryName = "vault" + } + + path, err := exec.LookPath(vaultBinaryName) + if err != nil || path == "" { + t.Skipf("%q not found on $PATH - download and install to run this test", vaultBinaryName) + } +} + func NewTestVaultServer(t testing.T) *TestVaultServer { testVault, err := runTestVault(t) if err != nil { diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index bca6790b2..960981e28 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -180,7 +180,9 @@ func getCAProviderWithLock(s *Server) (ca.Provider, *structs.CARoot) { return s.getCAProvider() } -func TestLeader_PrimaryCA_IntermediateRenew(t *testing.T) { +func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { + ca.SkipIfVaultNotPresent(t) + // no parallel execution because we change globals origInterval := structs.IntermediateCertRenewInterval origMinTTL := structs.MinLeafCertTTL @@ -240,7 +242,7 @@ func TestLeader_PrimaryCA_IntermediateRenew(t *testing.T) { provider, _ := getCAProviderWithLock(s1) intermediatePEM, err := provider.ActiveIntermediate() require.NoError(err) - cert, err := connect.ParseCert(intermediatePEM) + _, err = connect.ParseCert(intermediatePEM) require.NoError(err) // Wait for dc1's intermediate to be refreshed. @@ -277,7 +279,7 @@ func TestLeader_PrimaryCA_IntermediateRenew(t *testing.T) { leafPEM, err := provider.Sign(leafCsr) require.NoError(err) - cert, err = connect.ParseCert(leafPEM) + cert, err := connect.ParseCert(leafPEM) require.NoError(err) // Check that the leaf signed by the new intermediate can be verified using the