Merge pull request #11514

ca: properly handle the case where the secondary initializes after the primary

.changelog/11514.txt

@@ -0,0 +1,6 @@
+```release-note:improvement
+connect/ca: Return an error when querying roots from uninitialized CA.
+```
+```release-note:bug
+connect/ca: Allow secondary initialization to resume after being deferred due to unreachable or incompatible primary DC servers.
+```

agent/consul/leader_connect_ca.go

@@ -39,7 +39,7 @@ type caServerDelegate interface {
 	generateCASignRequest(csr string) *structs.CASignRequest
 	raftApply(t structs.MessageType, msg interface{}) (interface{}, error)
 
-	checkServersProvider
+	ServersSupportMultiDCConnectCA() error
 }
 
 // CAManager is a wrapper around CA operations such as updating roots, an intermediate
@@ -77,6 +77,17 @@ func (c *caDelegateWithState) State() *state.Store {
 	return c.fsm.State()
 }
 
+func (c *caDelegateWithState) ServersSupportMultiDCConnectCA() error {
+	versionOk, primaryFound := ServersInDCMeetMinimumVersion(c.Server, c.Server.config.PrimaryDatacenter, minMultiDCConnectVersion)
+	if !primaryFound {
+		return fmt.Errorf("primary datacenter is unreachable")
+	}
+	if !versionOk {
+		return fmt.Errorf("all servers in the primary datacenter are not at the minimum version %v", minMultiDCConnectVersion)
+	}
+	return nil
+}
+
 func NewCAManager(delegate caServerDelegate, leaderRoutineManager *routine.Manager, logger hclog.Logger, config *Config) *CAManager {
 	return &CAManager{
 		delegate:             delegate,
@@ -158,7 +169,8 @@ func (c *CAManager) initializeCAConfig() (*structs.CAConfiguration, error) {
 	}
 	if config == nil {
 		config = c.serverConf.CAConfig
-		if config.ClusterID == "" {
+
+		if c.serverConf.Datacenter == c.serverConf.PrimaryDatacenter && config.ClusterID == "" {
 			id, err := uuid.GenerateUUID()
 			if err != nil {
 				return nil, err
@@ -354,19 +366,12 @@ func (c *CAManager) InitializeCA() (reterr error) {
 	if c.serverConf.PrimaryDatacenter == c.serverConf.Datacenter {
 		return c.initializeRootCA(provider, conf)
 	}
+	return c.secondaryInitialize(provider, conf)
+}
 
-	// If this isn't the primary DC, run the secondary DC routine if the primary has already been upgraded to at least 1.6.0
-	versionOk, foundPrimary := ServersInDCMeetMinimumVersion(c.delegate, c.serverConf.PrimaryDatacenter, minMultiDCConnectVersion)
-	if !foundPrimary {
-		c.logger.Warn("primary datacenter is configured but unreachable - deferring initialization of the secondary datacenter CA")
-		// return nil because we will initialize the secondary CA later
-		return nil
-	} else if !versionOk {
-		// return nil because we will initialize the secondary CA later
-		c.logger.Warn("servers in the primary datacenter are not at least at the minimum version - deferring initialization of the secondary datacenter CA",
-			"min_version", minMultiDCConnectVersion.String(),
-		)
-		return nil
+func (c *CAManager) secondaryInitialize(provider ca.Provider, conf *structs.CAConfiguration) error {
+	if err := c.delegate.ServersSupportMultiDCConnectCA(); err != nil {
+		return fmt.Errorf("initialization will be deferred: %w", err)
 	}
 
 	// Get the root CA to see if we need to refresh our intermediate.
@@ -1200,15 +1205,12 @@ func (c *CAManager) UpdateRoots(roots structs.IndexedCARoots) error {
 		return nil
 	}
 	if !c.configuredSecondaryCA() {
-		versionOk, primaryFound := ServersInDCMeetMinimumVersion(c.delegate, c.serverConf.PrimaryDatacenter, minMultiDCConnectVersion)
-		if !primaryFound {
-			return fmt.Errorf("Primary datacenter is unreachable - deferring secondary CA initialization")
+		if err := c.delegate.ServersSupportMultiDCConnectCA(); err != nil {
+			return fmt.Errorf("failed to initialize while updating primary roots: %w", err)
 		}
 
-		if versionOk {
-			if err := c.initializeSecondaryProvider(provider, roots); err != nil {
-				return fmt.Errorf("Failed to initialize secondary CA provider: %v", err)
-			}
+		if err := c.initializeSecondaryProvider(provider, roots); err != nil {
+			return fmt.Errorf("Failed to initialize secondary CA provider: %v", err)
 		}
 	}
 

agent/consul/leader_connect_ca_test.go

@@ -8,15 +8,13 @@ import (
 	"testing"
 	"time"
 
+	"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"
 )
 
 // TODO(kyhavlov): replace with t.Deadline()
@@ -51,12 +49,8 @@ func (m *mockCAServerDelegate) IsLeader() bool {
 	return true
 }
 
-func (m *mockCAServerDelegate) CheckServers(datacenter string, fn func(*metadata.Server) bool) {
-	ver, _ := version.NewVersion("1.6.0")
-	fn(&metadata.Server{
-		Status: serf.StatusAlive,
-		Build:  *ver,
-	})
+func (m *mockCAServerDelegate) ServersSupportMultiDCConnectCA() error {
+	return nil
 }
 
 func (m *mockCAServerDelegate) ApplyCARequest(req *structs.CARequest) (interface{}, error) {

agent/consul/leader_connect_test.go

@@ -932,7 +932,6 @@ func TestLeader_SecondaryCA_UpgradeBeforePrimary(t *testing.T) {
 
 	// Wait for the secondary transition to happen and then verify the secondary DC
 	// has both roots present.
-	secondaryProvider, _ := getCAProviderWithLock(s2)
 	retry.Run(t, func(r *retry.R) {
 		state1 := s1.fsm.State()
 		_, roots1, err := state1.CARoots(nil)
@@ -948,15 +947,18 @@ func TestLeader_SecondaryCA_UpgradeBeforePrimary(t *testing.T) {
 		require.Equal(r, roots1[0].ID, roots2[0].ID)
 		require.Equal(r, roots1[0].RootCert, roots2[0].RootCert)
 
+		secondaryProvider, _ := getCAProviderWithLock(s2)
 		inter, err := secondaryProvider.ActiveIntermediate()
 		require.NoError(r, err)
 		require.NotEmpty(r, inter, "should have valid intermediate")
 	})
 
-	_, caRoot := getCAProviderWithLock(s1)
+	secondaryProvider, _ := getCAProviderWithLock(s2)
 	intermediatePEM, err := secondaryProvider.ActiveIntermediate()
 	require.NoError(t, err)
 
+	_, caRoot := getCAProviderWithLock(s1)
+
 	// Have dc2 sign a leaf cert and make sure the chain is correct.
 	spiffeService := &connect.SpiffeIDService{
 		Host:       "node1",

agent/consul/server_connect.go

@@ -79,19 +79,23 @@ func (s *Server) getCARoots(ws memdb.WatchSet, state *state.Store) (*structs.Ind
 	if err != nil {
 		return nil, err
 	}
+	if config == nil {
+		return nil, fmt.Errorf("CA has not finished initializing")
+	}
 
 	indexedRoots := &structs.IndexedCARoots{}
 
-	if config != nil {
-		// Build TrustDomain based on the ClusterID stored.
-		signingID := connect.SpiffeIDSigningForCluster(config)
-		if signingID == nil {
-			// If CA is bootstrapped at all then this should never happen but be
-			// defensive.
-			return nil, fmt.Errorf("no cluster trust domain setup")
-		}
+	// Build TrustDomain based on the ClusterID stored.
+	signingID := connect.SpiffeIDSigningForCluster(config)
+	if signingID == nil {
+		// If CA is bootstrapped at all then this should never happen but be
+		// defensive.
+		return nil, fmt.Errorf("no cluster trust domain setup")
+	}
 
-		indexedRoots.TrustDomain = signingID.Host()
+	indexedRoots.TrustDomain = signingID.Host()
+	if indexedRoots.TrustDomain == "" {
+		return nil, fmt.Errorf("CA has not finished initializing")
 	}
 
 	indexedRoots.Index, indexedRoots.Roots = index, roots

agent/consul/state/connect_ca.go

@@ -180,8 +180,6 @@ func (s *Store) caSetConfigTxn(idx uint64, tx WriteTxn, config *structs.CAConfig
 	if prev != nil {
 		existing := prev.(*structs.CAConfiguration)
 		config.CreateIndex = existing.CreateIndex
-		// Allow the ClusterID to change if it's provided by an internal operation, such
-		// as a primary datacenter being switched to secondary mode.
 		if config.ClusterID == "" {
 			config.ClusterID = existing.ClusterID
 		}