From 035b569e6a6a87baabb9a52e6b15cf581d6e8a97 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 23 Jun 2021 15:47:30 -0400 Subject: [PATCH 1/3] ca: replace ca.PrimaryIntermediateProviders With an optional interface that providers can use to indicate if they use an intermediate cert in the primary DC. This removes the need to look up the provider config when renewing the intermediate. --- agent/connect/ca/provider.go | 11 ++++++----- agent/connect/ca/provider_vault.go | 11 +++++++---- agent/consul/leader_connect_ca.go | 12 ++++-------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/agent/connect/ca/provider.go b/agent/connect/ca/provider.go index 1e0d19b36df7..02d5efecbaf1 100644 --- a/agent/connect/ca/provider.go +++ b/agent/connect/ca/provider.go @@ -16,11 +16,12 @@ import ( // on servers and CA provider. var ErrRateLimited = errors.New("operation rate limited by CA provider") -// PrimaryIntermediateProviders is a list of CA providers that make use use of an -// 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": {}, +// PrimaryUsesIntermediate is an optional interface that CA providers may implement +// to indicate that they use an 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. +type PrimaryUsesIntermediate interface { + PrimaryUsesIntermediate() } // 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 d08b66325047..d727a4b601ba 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -11,12 +11,13 @@ import ( "strings" "time" - "github.com/hashicorp/consul/agent/connect" - "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/consul/logging" "github.com/hashicorp/go-hclog" vaultapi "github.com/hashicorp/vault/api" "github.com/mitchellh/mapstructure" + + "github.com/hashicorp/consul/agent/connect" + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/logging" ) const VaultCALeafCertRole = "leaf-cert" @@ -518,7 +519,7 @@ func (v *VaultProvider) CrossSignCA(cert *x509.Certificate) (string, error) { } // SupportsCrossSigning implements Provider -func (c *VaultProvider) SupportsCrossSigning() (bool, error) { +func (v *VaultProvider) SupportsCrossSigning() (bool, error) { return true, nil } @@ -557,6 +558,8 @@ func (v *VaultProvider) Stop() { v.shutdown() } +func (v *VaultProvider) PrimaryUsesIntermediate() {} + func ParseVaultCAConfig(raw map[string]interface{}) (*structs.VaultCAProviderConfig, error) { config := structs.VaultCAProviderConfig{ CommonCAProviderConfig: defaultCommonConfig(), diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index e96214639123..867d6f14ce79 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -9,12 +9,13 @@ import ( "sync" "time" + "github.com/hashicorp/go-hclog" + uuid "github.com/hashicorp/go-uuid" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" - "github.com/hashicorp/go-hclog" - uuid "github.com/hashicorp/go-uuid" ) type caState string @@ -1077,12 +1078,7 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error // If this is the primary, check if this is a provider that uses an intermediate cert. If // it isn't, we don't need to check for a renewal. if isPrimary { - _, config, err := state.CAConfig(nil) - if err != nil { - return err - } - - if _, ok := ca.PrimaryIntermediateProviders[config.Provider]; !ok { + if _, ok := provider.(ca.PrimaryUsesIntermediate); !ok { return nil } } From 2706dd9841612cb753370c3822dddde69c021611 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 2 Dec 2021 16:02:24 -0500 Subject: [PATCH 2/3] Merge pull request #11671 from hashicorp/dnephin/ca-fix-storing-vault-intermediate ca: fix storing the leaf signing cert with Vault provider --- .changelog/11671.txt | 3 ++ agent/consul/leader_connect_ca.go | 62 +++++++++++++++++++++++------ agent/consul/leader_connect_test.go | 42 +++++++++++++++---- agent/structs/connect_ca.go | 13 +++++- 4 files changed, 97 insertions(+), 23 deletions(-) create mode 100644 .changelog/11671.txt diff --git a/.changelog/11671.txt b/.changelog/11671.txt new file mode 100644 index 000000000000..0f97366fb5ff --- /dev/null +++ b/.changelog/11671.txt @@ -0,0 +1,3 @@ +```release-note:bug +ca: fixes a bug that caused the intermediate cert used to sign leaf certs to be missing from the /connect/ca/roots API response when the Vault provider was used. +``` diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 867d6f14ce79..8c1428b321f1 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -448,12 +448,26 @@ func (c *CAManager) initializeRootCA(provider ca.Provider, conf *structs.CAConfi } } + var rootUpdateRequired bool + // Versions prior to 1.9.3, 1.8.8, and 1.7.12 incorrectly used the primary // rootCA's subjectKeyID here instead of the intermediate. For // provider=consul this didn't matter since there are no intermediates in // the primaryDC, but for vault it does matter. expectedSigningKeyID := connect.EncodeSigningKeyID(intermediateCert.SubjectKeyId) - needsSigningKeyUpdate := (rootCA.SigningKeyID != expectedSigningKeyID) + if rootCA.SigningKeyID != expectedSigningKeyID { + c.logger.Info("Correcting stored CARoot values", + "previous-signing-key", rootCA.SigningKeyID, "updated-signing-key", expectedSigningKeyID) + rootCA.SigningKeyID = expectedSigningKeyID + rootUpdateRequired = true + } + + // Add the local leaf signing cert to the rootCA struct. This handles both + // upgrades of existing state, and new rootCA. + if c.getLeafSigningCertFromRoot(rootCA) != interPEM { + rootCA.IntermediateCerts = append(rootCA.IntermediateCerts, interPEM) + rootUpdateRequired = true + } // Check if the CA root is already initialized and exit if it is, // adding on any existing intermediate certs since they aren't directly @@ -465,26 +479,21 @@ func (c *CAManager) initializeRootCA(provider ca.Provider, conf *structs.CAConfi if err != nil { return err } - if activeRoot != nil && needsSigningKeyUpdate { - c.logger.Info("Correcting stored SigningKeyID value", "previous", rootCA.SigningKeyID, "updated", expectedSigningKeyID) - - } else if activeRoot != nil && !needsSigningKeyUpdate { + if activeRoot != nil && !rootUpdateRequired { // This state shouldn't be possible to get into because we update the root and // CA config in the same FSM operation. if activeRoot.ID != rootCA.ID { return fmt.Errorf("stored CA root %q is not the active root (%s)", rootCA.ID, activeRoot.ID) } + // TODO: why doesn't this c.setCAProvider(provider, activeRoot) ? rootCA.IntermediateCerts = activeRoot.IntermediateCerts c.setCAProvider(provider, rootCA) + c.logger.Info("initialized primary datacenter CA from existing CARoot with provider", "provider", conf.Provider) return nil } - if needsSigningKeyUpdate { - rootCA.SigningKeyID = expectedSigningKeyID - } - // Get the highest index idx, _, err := state.CARoots(nil) if err != nil { @@ -512,6 +521,22 @@ func (c *CAManager) initializeRootCA(provider ca.Provider, conf *structs.CAConfi return nil } +// getLeafSigningCertFromRoot returns the PEM encoded certificate that should be used to +// sign leaf certificates in the local datacenter. The SubjectKeyId of the +// returned cert should always match the SigningKeyID of the CARoot. +// +// TODO: fix the data model so that we don't need this complicated lookup to +// find the leaf signing cert. See github.com/hashicorp/consul/issues/11347. +func (c *CAManager) getLeafSigningCertFromRoot(root *structs.CARoot) string { + if !c.isIntermediateUsedToSignLeaf() { + return root.RootCert + } + if len(root.IntermediateCerts) == 0 { + return "" + } + return root.IntermediateCerts[len(root.IntermediateCerts)-1] +} + // initializeSecondaryCA runs the routine for generating an intermediate CA CSR and getting // it signed by the primary DC if the root CA of the primary DC has changed since the last // intermediate. It should only be called while the state lock is held by setting the state @@ -1077,10 +1102,8 @@ func (c *CAManager) RenewIntermediate(ctx context.Context, isPrimary bool) error // If this is the primary, check if this is a provider that uses an intermediate cert. If // it isn't, we don't need to check for a renewal. - if isPrimary { - if _, ok := provider.(ca.PrimaryUsesIntermediate); !ok { - return nil - } + if isPrimary && !primaryUsesIntermediate(provider) { + return nil } activeIntermediate, err := provider.ActiveIntermediate() @@ -1264,3 +1287,16 @@ func (c *CAManager) configuredSecondaryCA() bool { defer c.stateLock.Unlock() return c.actingSecondaryCA } + +func primaryUsesIntermediate(provider ca.Provider) bool { + _, ok := provider.(ca.PrimaryUsesIntermediate) + return ok +} + +func (c *CAManager) isIntermediateUsedToSignLeaf() bool { + if c.serverConf.Datacenter != c.serverConf.PrimaryDatacenter { + return true + } + provider, _ := c.getCAProvider() + return primaryUsesIntermediate(provider) +} diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index a7cafed621e6..2179cb590548 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -237,23 +237,34 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { provider, _ := getCAProviderWithLock(s1) intermediatePEM, err := provider.ActiveIntermediate() require.NoError(err) - _, err = connect.ParseCert(intermediatePEM) + intermediateCert, err := connect.ParseCert(intermediatePEM) require.NoError(err) + // Check that the state store has the correct intermediate + store := s1.caManager.delegate.State() + _, activeRoot, err := store.CARootActive(nil) + require.NoError(err) + require.Equal(intermediatePEM, s1.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) + // Wait for dc1's intermediate to be refreshed. // It is possible that test fails when the blocking query doesn't return. retry.Run(t, func(r *retry.R) { provider, _ = getCAProviderWithLock(s1) newIntermediatePEM, err := provider.ActiveIntermediate() r.Check(err) - _, err = connect.ParseCert(intermediatePEM) - r.Check(err) if newIntermediatePEM == intermediatePEM { r.Fatal("not a renewed intermediate") } + intermediateCert, err = connect.ParseCert(newIntermediatePEM) + r.Check(err) intermediatePEM = newIntermediatePEM }) + + _, activeRoot, err = store.CARootActive(nil) require.NoError(err) + require.Equal(intermediatePEM, s1.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) // Get the root from dc1 and validate a chain of: // dc1 leaf -> dc1 intermediate -> dc1 root @@ -280,6 +291,8 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { // Check that the leaf signed by the new intermediate can be verified using the // returned cert chain (signed intermediate + remote root). intermediatePool := x509.NewCertPool() + // TODO: do not explicitly add the intermediatePEM, we should have it available + // from leafPEM. Use connect.ParseLeafCerts to do the right thing. intermediatePool.AppendCertsFromPEM([]byte(intermediatePEM)) rootPool := x509.NewCertPool() rootPool.AppendCertsFromPEM([]byte(caRoot.RootCert)) @@ -347,10 +360,10 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { secondaryProvider, _ := getCAProviderWithLock(s2) intermediatePEM, err := secondaryProvider.ActiveIntermediate() require.NoError(err) - cert, err := connect.ParseCert(intermediatePEM) + intermediateCert, err := connect.ParseCert(intermediatePEM) require.NoError(err) - currentCertSerialNumber := cert.SerialNumber - currentCertAuthorityKeyId := cert.AuthorityKeyId + currentCertSerialNumber := intermediateCert.SerialNumber + currentCertAuthorityKeyId := intermediateCert.AuthorityKeyId // Capture the current root var originalRoot *structs.CARoot @@ -364,6 +377,12 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { waitForActiveCARoot(t, s1, originalRoot) waitForActiveCARoot(t, s2, originalRoot) + store := s2.fsm.State() + _, activeRoot, err := store.CARootActive(nil) + require.NoError(err) + require.Equal(intermediatePEM, s2.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) + // Wait for dc2's intermediate to be refreshed. // It is possible that test fails when the blocking query doesn't return. // When https://github.com/hashicorp/consul/pull/3777 is merged @@ -380,8 +399,13 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { currentCertAuthorityKeyId = cert.AuthorityKeyId r.Fatal("not a renewed intermediate") } + intermediateCert = cert }) + + _, activeRoot, err = store.CARootActive(nil) require.NoError(err) + require.Equal(intermediatePEM, s2.caManager.getLeafSigningCertFromRoot(activeRoot)) + require.Equal(connect.HexString(intermediateCert.SubjectKeyId), activeRoot.SigningKeyID) // Get the root from dc1 and validate a chain of: // dc2 leaf -> dc2 intermediate -> dc1 root @@ -402,17 +426,19 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { leafPEM, err := secondaryProvider.Sign(leafCsr) require.NoError(err) - cert, err = connect.ParseCert(leafPEM) + intermediateCert, err = connect.ParseCert(leafPEM) require.NoError(err) // Check that the leaf signed by the new intermediate can be verified using the // returned cert chain (signed intermediate + remote root). intermediatePool := x509.NewCertPool() + // TODO: do not explicitly add the intermediatePEM, we should have it available + // from leafPEM. Use connect.ParseLeafCerts to do the right thing. intermediatePool.AppendCertsFromPEM([]byte(intermediatePEM)) rootPool := x509.NewCertPool() rootPool.AppendCertsFromPEM([]byte(caRoot.RootCert)) - _, err = cert.Verify(x509.VerifyOptions{ + _, err = intermediateCert.Verify(x509.VerifyOptions{ Intermediates: intermediatePool, Roots: rootPool, }) diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 54f156141dd6..5e9c8880d994 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -85,11 +85,20 @@ type CARoot struct { NotBefore time.Time NotAfter time.Time - // RootCert is the PEM-encoded public certificate. + // RootCert is the PEM-encoded public certificate for the root CA. The + // certificate is the same for all federated clusters. RootCert string // IntermediateCerts is a list of PEM-encoded intermediate certs to - // attach to any leaf certs signed by this CA. + // attach to any leaf certs signed by this CA. The list may include a + // certificate cross-signed by an old root CA, any subordinate CAs below the + // root CA, and the intermediate CA used to sign leaf certificates in the + // local Datacenter. + // + // If the provider which created this root uses an intermediate to sign + // leaf certificates (Vault provider), or this is a secondary Datacenter then + // the intermediate used to sign leaf certificates will be the last in the + // list. IntermediateCerts []string // SigningCert is the PEM-encoded signing certificate and SigningKey From 9a884a209c6594d2a577371949dcf352551747fa Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 2 Dec 2021 16:05:42 -0500 Subject: [PATCH 3/3] Merge pull request #11713 from hashicorp/dnephin/ca-test-names ca: make test naming consistent --- agent/consul/leader_connect_ca.go | 10 ++-- agent/consul/leader_connect_ca_test.go | 49 +++++++++++++--- agent/consul/leader_connect_test.go | 77 +++++++++++++++++++------- agent/consul/server_test.go | 35 +----------- 4 files changed, 105 insertions(+), 66 deletions(-) diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 8c1428b321f1..bd7fec5d9d95 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -260,7 +260,7 @@ func (c *CAManager) Start() { // Attempt to initialize the Connect CA now. This will // happen during leader establishment and it would be great // if the CA was ready to go once that process was finished. - if err := c.InitializeCA(); err != nil { + if err := c.Initialize(); err != nil { c.logger.Error("Failed to initialize Connect CA", "error", err) // we failed to fully initialize the CA so we need to spawn a @@ -290,7 +290,7 @@ func (c *CAManager) startPostInitializeRoutines() { } func (c *CAManager) backgroundCAInitialization(ctx context.Context) error { - retryLoopBackoffAbortOnSuccess(ctx, c.InitializeCA, func(err error) { + retryLoopBackoffAbortOnSuccess(ctx, c.Initialize, func(err error) { c.logger.Error("Failed to initialize Connect CA", "routine", backgroundCAInitializationRoutineName, "error", err, @@ -307,10 +307,10 @@ func (c *CAManager) backgroundCAInitialization(ctx context.Context) error { return nil } -// InitializeCA sets up the CA provider when gaining leadership, either bootstrapping +// Initialize sets up the CA provider when gaining leadership, either bootstrapping // the CA if this is the primary DC or making a remote RPC for intermediate signing // if this is a secondary DC. -func (c *CAManager) InitializeCA() (reterr error) { +func (c *CAManager) Initialize() (reterr error) { // Bail if connect isn't enabled. if !c.serverConf.ConnectEnabled { return nil @@ -758,7 +758,7 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error) } }() - // Attempt to initialize the config if we failed to do so in InitializeCA for some reason + // Attempt to initialize the config if we failed to do so in Initialize for some reason _, err = c.initializeCAConfig() if err != nil { return err diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index 3fc460dc4df5..90c1077496a7 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -1,6 +1,7 @@ package consul import ( + "bytes" "context" "crypto/x509" "errors" @@ -8,15 +9,18 @@ import ( "testing" "time" + "github.com/hashicorp/go-version" + "github.com/hashicorp/serf/serf" + "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/agent/connect" ca "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/metadata" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/sdk/testutil" - "github.com/hashicorp/go-version" - "github.com/hashicorp/serf/serf" - "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/sdk/testutil/retry" + "github.com/hashicorp/consul/testrpc" ) // TODO(kyhavlov): replace with t.Deadline() @@ -179,7 +183,7 @@ func testCAConfig() *structs.CAConfiguration { func initTestManager(t *testing.T, manager *CAManager, delegate *mockCAServerDelegate) { initCh := make(chan struct{}) go func() { - require.NoError(t, manager.InitializeCA()) + require.NoError(t, manager.Initialize()) close(initCh) }() for i := 0; i < 5; i++ { @@ -204,12 +208,12 @@ func TestCAManager_Initialize(t *testing.T) { delegate := NewMockCAServerDelegate(t, conf) manager := NewCAManager(delegate, nil, testutil.Logger(t), conf) - // Call InitializeCA and then confirm the RPCs and provider calls + // Call Initialize and then confirm the RPCs and provider calls // happen in the expected order. require.EqualValues(t, caStateUninitialized, manager.state) errCh := make(chan error) go func() { - errCh <- manager.InitializeCA() + errCh <- manager.Initialize() }() waitForCh(t, delegate.callbackCh, "forwardDC/ConnectCA.Roots") @@ -220,7 +224,7 @@ func TestCAManager_Initialize(t *testing.T) { waitForCh(t, delegate.callbackCh, "raftApply/ConnectCA") waitForEmptyCh(t, delegate.callbackCh) - // Make sure the InitializeCA call returned successfully. + // Make sure the Initialize call returned successfully. select { case err := <-errCh: require.NoError(t, err) @@ -285,3 +289,34 @@ func TestCAManager_UpdateConfigWhileRenewIntermediate(t *testing.T) { require.EqualValues(t, caStateInitialized, manager.state) } + +func TestCAManager_Initialize_Logging(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + _, conf1 := testServerConfig(t) + + // Setup dummy logger to catch output + var buf bytes.Buffer + logger := testutil.LoggerWithOutput(t, &buf) + + deps := newDefaultDeps(t, conf1) + deps.Logger = logger + + s1, err := NewServer(conf1, deps) + require.NoError(t, err) + defer s1.Shutdown() + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + // Wait til CA root is setup + retry.Run(t, func(r *retry.R) { + var out structs.IndexedCARoots + r.Check(s1.RPC("ConnectCA.Roots", structs.DCSpecificRequest{ + Datacenter: conf1.Datacenter, + }, &out)) + }) + + require.Contains(t, buf.String(), "consul CA provider configured") +} diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index 2179cb590548..046396241106 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -12,19 +12,24 @@ import ( "testing" "time" + uuid "github.com/hashicorp/go-uuid" + msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/token" "github.com/hashicorp/consul/sdk/testutil/retry" "github.com/hashicorp/consul/testrpc" - uuid "github.com/hashicorp/go-uuid" - msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func TestLeader_SecondaryCA_Initialize(t *testing.T) { +func TestCAManager_Initialize_Secondary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() tests := []struct { @@ -178,7 +183,11 @@ func getCAProviderWithLock(s *Server) (ca.Provider, *structs.CARoot) { return s.caManager.getCAProvider() } -func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { +func TestCAManager_RenewIntermediate_Vault_Primary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + ca.SkipIfVaultNotPresent(t) // no parallel execution because we change globals @@ -304,7 +313,11 @@ func TestLeader_Vault_PrimaryCA_IntermediateRenew(t *testing.T) { require.NoError(err) } -func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { +func TestCAManager_RenewIntermediate_Secondary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + // no parallel execution because we change globals origInterval := structs.IntermediateCertRenewInterval origMinTTL := structs.MinLeafCertTTL @@ -445,7 +458,11 @@ func TestLeader_SecondaryCA_IntermediateRenew(t *testing.T) { require.NoError(err) } -func TestLeader_SecondaryCA_IntermediateRefresh(t *testing.T) { +func TestConnectCA_ConfigurationSet_RootRotation_Secondary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() require := require.New(t) @@ -592,7 +609,7 @@ func TestLeader_SecondaryCA_IntermediateRefresh(t *testing.T) { require.NoError(err) } -func TestLeader_Vault_PrimaryCA_FixSigningKeyID_OnRestart(t *testing.T) { +func TestCAManager_Initialize_Vault_FixesSigningKeyID_Primary(t *testing.T) { ca.SkipIfVaultNotPresent(t) if testing.Short() { @@ -697,7 +714,11 @@ func TestLeader_Vault_PrimaryCA_FixSigningKeyID_OnRestart(t *testing.T) { }) } -func TestLeader_SecondaryCA_FixSigningKeyID_via_IntermediateRefresh(t *testing.T) { +func TestCAManager_Initialize_FixesSigningKeyID_Secondary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() dir1, s1 := testServerWithConfig(t, func(c *Config) { @@ -796,7 +817,11 @@ func TestLeader_SecondaryCA_FixSigningKeyID_via_IntermediateRefresh(t *testing.T }) } -func TestLeader_SecondaryCA_TransitionFromPrimary(t *testing.T) { +func TestCAManager_Initialize_TransitionFromPrimaryToSecondary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() // Initialize dc1 as the primary DC @@ -884,7 +909,11 @@ func TestLeader_SecondaryCA_TransitionFromPrimary(t *testing.T) { }) } -func TestLeader_SecondaryCA_UpgradeBeforePrimary(t *testing.T) { +func TestCAManager_Initialize_SecondaryBeforePrimary(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() // Initialize dc1 as the primary DC @@ -1087,7 +1116,11 @@ func TestLeader_CARootPruning(t *testing.T) { require.NotEqual(roots[0].ID, oldRoot.ID) } -func TestLeader_PersistIntermediateCAs(t *testing.T) { +func TestConnectCA_ConfigurationSet_PersistsRoots(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + t.Parallel() require := require.New(t) @@ -1167,7 +1200,7 @@ func TestLeader_PersistIntermediateCAs(t *testing.T) { }) } -func TestLeader_ParseCARoot(t *testing.T) { +func TestParseCARoot(t *testing.T) { type test struct { name string pem string @@ -1250,7 +1283,7 @@ func readTestData(t *testing.T, name string) string { return string(bs) } -func TestLeader_lessThanHalfTimePassed(t *testing.T) { +func TestLessThanHalfTimePassed(t *testing.T) { now := time.Now() require.False(t, lessThanHalfTimePassed(now, now.Add(-10*time.Second), now.Add(-5*time.Second))) require.False(t, lessThanHalfTimePassed(now, now.Add(-10*time.Second), now)) @@ -1260,7 +1293,11 @@ func TestLeader_lessThanHalfTimePassed(t *testing.T) { require.True(t, lessThanHalfTimePassed(now, now.Add(-10*time.Second), now.Add(20*time.Second))) } -func TestLeader_retryLoopBackoffHandleSuccess(t *testing.T) { +func TestRetryLoopBackoffHandleSuccess(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + type test struct { desc string loopFn func() error @@ -1300,7 +1337,7 @@ func TestLeader_retryLoopBackoffHandleSuccess(t *testing.T) { } } -func TestLeader_Vault_BadCAConfigShouldntPreventLeaderEstablishment(t *testing.T) { +func TestCAManager_Initialize_Vault_BadCAConfigDoesNotPreventLeaderEstablishment(t *testing.T) { ca.SkipIfVaultNotPresent(t) testVault := ca.NewTestVaultServer(t) @@ -1357,7 +1394,7 @@ func TestLeader_Vault_BadCAConfigShouldntPreventLeaderEstablishment(t *testing.T require.NotNil(t, activeRoot) } -func TestLeader_Consul_BadCAConfigShouldntPreventLeaderEstablishment(t *testing.T) { +func TestCAManager_Initialize_BadCAConfigDoesNotPreventLeaderEstablishment(t *testing.T) { ca.SkipIfVaultNotPresent(t) _, s1 := testServerWithConfig(t, func(c *Config) { @@ -1401,7 +1438,7 @@ func TestLeader_Consul_BadCAConfigShouldntPreventLeaderEstablishment(t *testing. require.NotNil(t, activeRoot) } -func TestLeader_Consul_ForceWithoutCrossSigning(t *testing.T) { +func TestConnectCA_ConfigurationSet_ForceWithoutCrossSigning(t *testing.T) { require := require.New(t) dir1, s1 := testServer(t) defer os.RemoveAll(dir1) @@ -1458,7 +1495,7 @@ func TestLeader_Consul_ForceWithoutCrossSigning(t *testing.T) { } } -func TestLeader_Vault_ForceWithoutCrossSigning(t *testing.T) { +func TestConnectCA_ConfigurationSet_Vault_ForceWithoutCrossSigning(t *testing.T) { ca.SkipIfVaultNotPresent(t) require := require.New(t) diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index 09325b6a2c2d..9a64c93d6a0f 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -1,7 +1,6 @@ package consul import ( - "bytes" "crypto/x509" "fmt" "net" @@ -13,12 +12,11 @@ import ( "time" "github.com/google/tcpproxy" + uuid "github.com/hashicorp/go-uuid" "github.com/hashicorp/memberlist" - "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/ipaddr" - "github.com/hashicorp/go-uuid" "golang.org/x/time/rate" "github.com/hashicorp/consul/agent/connect" @@ -1457,37 +1455,6 @@ func TestServer_RPC_RateLimit(t *testing.T) { }) } -func TestServer_CALogging(t *testing.T) { - t.Parallel() - _, conf1 := testServerConfig(t) - - // Setup dummy logger to catch output - var buf bytes.Buffer - logger := testutil.LoggerWithOutput(t, &buf) - - deps := newDefaultDeps(t, conf1) - deps.Logger = logger - - s1, err := NewServer(conf1, deps) - require.NoError(t, err) - defer s1.Shutdown() - testrpc.WaitForLeader(t, s1.RPC, "dc1") - - if _, ok := s1.caManager.provider.(ca.NeedsLogger); !ok { - t.Fatalf("provider does not implement NeedsLogger") - } - - // Wait til CA root is setup - retry.Run(t, func(r *retry.R) { - var out structs.IndexedCARoots - r.Check(s1.RPC("ConnectCA.Roots", structs.DCSpecificRequest{ - Datacenter: conf1.Datacenter, - }, &out)) - }) - - require.Contains(t, buf.String(), "consul CA provider configured") -} - func TestServer_DatacenterJoinAddresses(t *testing.T) { conf := testClusterConfig{ Datacenter: "primary",