Merge pull request #11514 (#11539)

Backport of #11514

---------

Fixes #11367

Previously `secondaryInitialize` would return nil in this case, which prevented the
deferred initialize from happening, and left the CA in an uninitialized state until a config
update or root rotation.

To fix this I extracted the common parts into the delegate implementation. However looking at this
again, it seems like the handling in secondaryUpdateRoots is impossible, because that function
should never be called before the secondary is initialized. I believe we can remove some of that
logic in a follow up.

.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
 		}