From 1c57b72a9f03a6ca80e025b7507ddfb565141f05 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Wed, 9 Sep 2020 16:36:37 -0700 Subject: [PATCH] Add a test for token renewal --- agent/connect/ca/provider_vault.go | 7 ++- agent/connect/ca/provider_vault_test.go | 74 +++++++++++++++++++++---- agent/consul/connect_ca_endpoint.go | 13 +++++ 3 files changed, 81 insertions(+), 13 deletions(-) diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index 41d36ac8b..4f05edd57 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -94,8 +94,9 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error { renewer, err := client.NewRenewer(&vaultapi.RenewerInput{ Secret: &vaultapi.Secret{ Auth: &vaultapi.SecretAuth{ - ClientToken: config.Token, - Renewable: renewable, + ClientToken: config.Token, + Renewable: renewable, + LeaseDuration: secret.LeaseDuration, }, }, Increment: int(increment), @@ -121,7 +122,7 @@ func (v *VaultProvider) renewToken(renewer *vaultapi.Renewer) { case err := <-renewer.DoneCh(): if err != nil { - v.logger.Error(fmt.Sprint("Error renewing token for Vault provider: %v", err)) + v.logger.Error(fmt.Sprintf("Error renewing token for Vault provider: %v", err)) } case <-renewer.RenewCh(): diff --git a/agent/connect/ca/provider_vault_test.go b/agent/connect/ca/provider_vault_test.go index 8c8c52b2f..6f432f751 100644 --- a/agent/connect/ca/provider_vault_test.go +++ b/agent/connect/ca/provider_vault_test.go @@ -2,6 +2,7 @@ package ca import ( "crypto/x509" + "encoding/json" "fmt" "io/ioutil" "os" @@ -14,6 +15,7 @@ import ( "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/sdk/freeport" "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/hashicorp/go-hclog" vaultapi "github.com/hashicorp/vault/api" "github.com/stretchr/testify/require" ) @@ -51,6 +53,46 @@ func TestVaultCAProvider_SecondaryActiveIntermediate(t *testing.T) { require.NoError(err) } +func TestVaultCAProvider_RenewToken(t *testing.T) { + t.Parallel() + require := require.New(t) + skipIfVaultNotPresent(t) + + testVault, err := runTestVault() + if err != nil { + t.Fatalf("err: %v", err) + } + + testVault.WaitUntilReady(t) + + // Create a token with a short TTL to be renewed by the provider. + ttl := 1 * time.Second + tcr := &vaultapi.TokenCreateRequest{ + TTL: ttl.String(), + } + secret, err := testVault.client.Auth().Token().Create(tcr) + require.NoError(err) + providerToken := secret.Auth.ClientToken + + _, err = createVaultProvider(true, testVault.addr, providerToken, nil) + require.NoError(err) + + // Check the last renewal time. + secret, err = testVault.client.Auth().Token().Lookup(providerToken) + require.NoError(err) + firstRenewal, err := secret.Data["last_renewal_time"].(json.Number).Int64() + require.NoError(err) + + time.Sleep(ttl * 2) + + // Wait past the TTL and make sure the token has been renewed. + secret, err = testVault.client.Auth().Token().Lookup(providerToken) + require.NoError(err) + lastRenewal, err := secret.Data["last_renewal_time"].(json.Number).Int64() + require.NoError(err) + require.Greater(lastRenewal, firstRenewal) +} + func TestVaultCAProvider_Bootstrap(t *testing.T) { t.Parallel() @@ -356,9 +398,18 @@ func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[strin testVault.WaitUntilReady(t) + provider, err := createVaultProvider(isPrimary, testVault.addr, testVault.rootToken, rawConf) + if err != nil { + testVault.Stop() + t.Fatalf("err: %v", err) + } + return provider, testVault +} + +func createVaultProvider(isPrimary bool, addr, token string, rawConf map[string]interface{}) (*VaultProvider, error) { conf := map[string]interface{}{ - "Address": testVault.addr, - "Token": testVault.rootToken, + "Address": addr, + "Token": token, "RootPKIPath": "pki-root/", "IntermediatePKIPath": "pki-intermediate/", // Tests duration parsing after msgpack type mangling during raft apply. @@ -377,26 +428,29 @@ func testVaultProviderWithConfig(t *testing.T, isPrimary bool, rawConf map[strin RawConfig: conf, } + logger := hclog.New(&hclog.LoggerOptions{ + Output: ioutil.Discard, + }) + provider.SetLogger(logger) + if !isPrimary { cfg.IsPrimary = false cfg.Datacenter = "dc2" } if err := provider.Configure(cfg); err != nil { - testVault.Stop() - t.Fatalf("err: %v", err) + return nil, err } if isPrimary { - if err = provider.GenerateRoot(); err != nil { - testVault.Stop() - t.Fatalf("err: %v", err) + if err := provider.GenerateRoot(); err != nil { + return nil, err } if _, err := provider.GenerateIntermediate(); err != nil { - testVault.Stop() - t.Fatalf("err: %v", err) + return nil, err } } - return provider, testVault + + return provider, nil } // skipIfVaultNotPresent skips the test if the vault binary is not in PATH. diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index ad922b4d3..6644030db 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -152,6 +152,17 @@ func (s *ConnectCA) ConfigurationSet( if err := newProvider.Configure(pCfg); err != nil { return fmt.Errorf("error configuring provider: %v", err) } + + // Set up a defer to clean up the new provider if we exit early due to an error. + cleanupNewProvider := true + defer func() { + if cleanupNewProvider { + if err := newProvider.Cleanup(); err != nil { + s.logger.Warn("failed to clean up temporary new CA provider", "provider", newProvider) + } + } + }() + if err := newProvider.GenerateRoot(); err != nil { return fmt.Errorf("error generating CA root certificate: %v", err) } @@ -195,6 +206,7 @@ func (s *ConnectCA) ConfigurationSet( } // If the config has been committed, update the local provider instance + cleanupNewProvider = false s.srv.setCAProvider(newProvider, newActiveRoot) s.logger.Info("CA provider config updated") @@ -291,6 +303,7 @@ func (s *ConnectCA) ConfigurationSet( // If the config has been committed, update the local provider instance // and call teardown on the old provider + cleanupNewProvider = false s.srv.setCAProvider(newProvider, newActiveRoot) if err := oldProvider.Cleanup(); err != nil {