ca: return an error when secondary fails to initialize

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 initialzied. I beleive we can remove some of that
logic in a follow up.
This commit is contained in:
Daniel Nephin 2021-11-05 16:37:10 -04:00
parent c7c5013edd
commit 01bd3d118d
3 changed files with 24 additions and 32 deletions

View File

@ -44,7 +44,7 @@ type caServerDelegate interface {
forwardDC(method, dc string, args interface{}, reply interface{}) error
generateCASignRequest(csr string) *structs.CASignRequest
checkServersProvider
ServersSupportMultiDCConnectCA() error
}
// CAManager is a wrapper around CA operations such as updating roots, an intermediate
@ -127,6 +127,17 @@ func (c *caDelegateWithState) generateCASignRequest(csr string) *structs.CASignR
}
}
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,
@ -408,18 +419,8 @@ func (c *CAManager) InitializeCA() (reterr error) {
}
func (c *CAManager) secondaryInitialize(provider ca.Provider, conf *structs.CAConfiguration) error {
// 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
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.
@ -1266,17 +1267,13 @@ func (c *CAManager) secondaryUpdateRoots(roots structs.IndexedCARoots) error {
return nil
}
if !c.secondaryIsCAConfigured() {
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.secondaryInitializeProvider(provider, roots); err != nil {
return fmt.Errorf("Failed to initialize secondary CA provider: %v", err)
}
}
}
// Run the secondary CA init routine to see if we need to request a new
// intermediate.

View File

@ -14,15 +14,12 @@ import (
"testing"
"time"
"github.com/hashicorp/go-version"
"github.com/hashicorp/serf/serf"
"github.com/stretchr/testify/assert"
"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/agent/token"
"github.com/hashicorp/consul/sdk/testutil"
@ -60,12 +57,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) ApplyCALeafRequest() (uint64, error) {

View File

@ -1094,7 +1094,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)
@ -1110,15 +1109,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",