Clarify some comments and names around CA bootstrapping

agent/connect/ca_provider.go

@@ -29,8 +29,8 @@ type CAProvider interface {
 	// SignCA signs a CA CSR and returns the resulting cross-signed cert.
 	SignCA(*x509.CertificateRequest) (string, error)
 
-	// Teardown performs any necessary cleanup that should happen when the provider
+	// Cleanup performs any necessary cleanup that should happen when the provider
 	// is shut down permanently, such as removing a temporary PKI backend in Vault
 	// created for an intermediate CA.
-	Teardown() error
+	Cleanup() error
 }

agent/consul/connect_ca_endpoint.go

@@ -52,7 +52,7 @@ func (s *ConnectCA) ConfigurationSet(
 		return err
 	}
 
-	// This action requires operator read access.
+	// This action requires operator write access.
 	rule, err := s.srv.resolveToken(args.Token)
 	if err != nil {
 		return err
@@ -133,7 +133,7 @@ func (s *ConnectCA) ConfigurationSet(
 	}
 
 	// Add the cross signed cert to the new root's intermediates
-	newActiveRoot.Intermediates = []string{xcCert}
+	newActiveRoot.IntermediateCerts = []string{xcCert}
 
 	// Update the roots and CA config in the state store at the same time
 	idx, roots, err := state.CARoots(nil)
@@ -166,11 +166,11 @@ func (s *ConnectCA) ConfigurationSet(
 	// and call teardown on the old provider
 	s.srv.setCAProvider(newProvider)
 
-	if err := oldProvider.Teardown(); err != nil {
+	if err := oldProvider.Cleanup(); err != nil {
-		return err
+		s.srv.logger.Printf("[WARN] connect: failed to clean up old provider %q", config.Provider)
 	}
 
-	s.srv.logger.Printf("[INFO] connect: CA rotated to the new root under %q provider", args.Config.Provider)
+	s.srv.logger.Printf("[INFO] connect: CA rotated to new root under provider %q", args.Config.Provider)
 
 	return nil
 }
@@ -205,12 +205,12 @@ func (s *ConnectCA) Roots(
 				// directly to the structure in the memdb store.
 
 				reply.Roots[i] = &structs.CARoot{
-					ID:            r.ID,
+					ID:                r.ID,
-					Name:          r.Name,
+					Name:              r.Name,
-					RootCert:      r.RootCert,
+					RootCert:          r.RootCert,
-					Intermediates: r.Intermediates,
+					IntermediateCerts: r.IntermediateCerts,
-					RaftIndex:     r.RaftIndex,
+					RaftIndex:         r.RaftIndex,
-					Active:        r.Active,
+					Active:            r.Active,
 				}
 
 				if r.Active {

agent/consul/connect_ca_provider.go

@@ -167,7 +167,7 @@ func (c *ConsulCAProvider) GenerateIntermediate() (*structs.CARoot, *x509.Certif
 		return nil, nil, err
 	}
 
-	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterSerial, Domain: "consul"}
+	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
 	template := &x509.CertificateRequest{
 		URIs: []*url.URL{id.URI()},
 	}
@@ -198,7 +198,7 @@ func (c *ConsulCAProvider) GenerateIntermediate() (*structs.CARoot, *x509.Certif
 }
 
 // Remove the state store entry for this provider instance.
-func (c *ConsulCAProvider) Teardown() error {
+func (c *ConsulCAProvider) Cleanup() error {
 	args := &structs.CARequest{
 		Op:            structs.CAOpDeleteProviderState,
 		ProviderState: &structs.CAConsulProviderState{ID: c.id},
@@ -336,7 +336,7 @@ func (c *ConsulCAProvider) SignCA(csr *x509.CertificateRequest) (string, error)
 	if err != nil {
 		return "", err
 	}
-	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterSerial, Domain: "consul"}
+	id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
 	keyId, err := connect.KeyId(privKey.Public())
 	if err != nil {
 		return "", err
@@ -423,7 +423,7 @@ func (c *ConsulCAProvider) generateCA(privateKey, contents string, sn uint64) (*
 
 	if pemContents == "" {
 		// The URI (SPIFFE compatible) for the cert
-		id := &connect.SpiffeIDSigning{ClusterID: config.ClusterSerial, Domain: "consul"}
+		id := &connect.SpiffeIDSigning{ClusterID: config.ClusterID, Domain: "consul"}
 		keyId, err := connect.KeyId(privKey.Public())
 		if err != nil {
 			return nil, err

agent/consul/fsm/commands_oss_test.go

@@ -1251,7 +1251,7 @@ func TestFSM_CAConfig(t *testing.T) {
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
-	var conf *connect.ConsulCAProviderConfig
+	var conf *structs.ConsulCAProviderConfig
 	if err := mapstructure.WeakDecode(config.Config, &conf); err != nil {
 		t.Fatalf("error decoding config: %s, %v", err, config.Config)
 	}

agent/consul/leader.go

@@ -7,16 +7,15 @@ import (
 	"sync"
 	"time"
 
-	"github.com/hashicorp/consul/agent/connect"
-	uuid "github.com/hashicorp/go-uuid"
-
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/autopilot"
 	"github.com/hashicorp/consul/agent/metadata"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/types"
+	uuid "github.com/hashicorp/go-uuid"
 	"github.com/hashicorp/go-version"
 	"github.com/hashicorp/raft"
 	"github.com/hashicorp/serf/serf"
@@ -215,7 +214,7 @@ func (s *Server) establishLeadership() error {
 	s.autopilot.Start()
 
 	// todo(kyhavlov): start a goroutine here for handling periodic CA rotation
-	s.bootstrapCA()
+	s.initializeCA()
 
 	s.setConsistentReadReady()
 	return nil
@@ -366,8 +365,9 @@ func (s *Server) getOrCreateAutopilotConfig() *autopilot.Config {
 	return config
 }
 
-// getOrCreateCAConfig is used to get the CA config, initializing it if necessary
+// initializeCAConfig is used to initialize the CA config if necessary
-func (s *Server) getOrCreateCAConfig() (*structs.CAConfiguration, error) {
+// when setting up the CA during establishLeadership
+func (s *Server) initializeCAConfig() (*structs.CAConfiguration, error) {
 	state := s.fsm.State()
 	_, config, err := state.CAConfig()
 	if err != nil {
@@ -377,13 +377,13 @@ func (s *Server) getOrCreateCAConfig() (*structs.CAConfiguration, error) {
 		return config, nil
 	}
 
-	sn, err := uuid.GenerateUUID()
+	id, err := uuid.GenerateUUID()
 	if err != nil {
 		return nil, err
 	}
 
 	config = s.config.CAConfig
-	config.ClusterSerial = sn
+	config.ClusterID = id
 	req := structs.CARequest{
 		Op:     structs.CAOpSetConfig,
 		Config: config,
@@ -395,9 +395,10 @@ func (s *Server) getOrCreateCAConfig() (*structs.CAConfiguration, error) {
 	return config, nil
 }
 
-// bootstrapCA creates a CA provider from the current configuration.
+// initializeCA sets up the CA provider when gaining leadership, bootstrapping
-func (s *Server) bootstrapCA() error {
+// the root in the state store if necessary.
-	conf, err := s.getOrCreateCAConfig()
+func (s *Server) initializeCA() error {
+	conf, err := s.initializeCAConfig()
 	if err != nil {
 		return err
 	}
@@ -424,7 +425,10 @@ func (s *Server) bootstrapCA() error {
 	if err != nil {
 		return err
 	}
-	if root != nil && root.ID == trustedCA.ID {
+	if root != nil {
+		if root.ID != trustedCA.ID {
+			s.logger.Printf("[WARN] connect: CA root %q is not the active root (%q)", trustedCA.ID, root.ID)
+		}
 		return nil
 	}
 

agent/consul/server.go

@@ -17,9 +17,8 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/hashicorp/consul/agent/connect"
-
 	"github.com/hashicorp/consul/acl"
+	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/consul/autopilot"
 	"github.com/hashicorp/consul/agent/consul/fsm"
 	"github.com/hashicorp/consul/agent/consul/state"
@@ -98,7 +97,8 @@ type Server struct {
 	// autopilotWaitGroup is used to block until Autopilot shuts down.
 	autopilotWaitGroup sync.WaitGroup
 
-	// caProvider is the current CA provider in use for Connect.
+	// caProvider is the current CA provider in use for Connect. This is
+	// only non-nil when we are the leader.
 	caProvider     connect.CAProvider
 	caProviderLock sync.RWMutex
 

agent/consul/state/connect_ca.go

@@ -8,17 +8,38 @@ import (
 )
 
 const (
-	caConfigTableName   = "connect-ca-config"
+	caBuiltinProviderTableName = "connect-ca-builtin"
-	caRootTableName     = "connect-ca-roots"
+	caConfigTableName          = "connect-ca-config"
-	caProviderTableName = "connect-ca-builtin"
+	caRootTableName            = "connect-ca-roots"
 )
 
+// caBuiltinProviderTableSchema returns a new table schema used for storing
+// the built-in CA provider's state for connect. This is only used by
+// the internal Consul CA provider.
+func caBuiltinProviderTableSchema() *memdb.TableSchema {
+	return &memdb.TableSchema{
+		Name: caBuiltinProviderTableName,
+		Indexes: map[string]*memdb.IndexSchema{
+			"id": &memdb.IndexSchema{
+				Name:         "id",
+				AllowMissing: false,
+				Unique:       true,
+				Indexer: &memdb.StringFieldIndex{
+					Field: "ID",
+				},
+			},
+		},
+	}
+}
+
 // caConfigTableSchema returns a new table schema used for storing
 // the CA config for Connect.
 func caConfigTableSchema() *memdb.TableSchema {
 	return &memdb.TableSchema{
 		Name: caConfigTableName,
 		Indexes: map[string]*memdb.IndexSchema{
+			// This table only stores one row, so this just ignores the ID field
+			// and always overwrites the same config object.
 			"id": &memdb.IndexSchema{
 				Name:         "id",
 				AllowMissing: true,
@@ -49,29 +70,10 @@ func caRootTableSchema() *memdb.TableSchema {
 	}
 }
 
-// caProviderTableSchema returns a new table schema used for storing
-// the built-in CA provider's state for connect. This is only used by
-// the internal Consul CA provider.
-func caProviderTableSchema() *memdb.TableSchema {
-	return &memdb.TableSchema{
-		Name: caProviderTableName,
-		Indexes: map[string]*memdb.IndexSchema{
-			"id": &memdb.IndexSchema{
-				Name:         "id",
-				AllowMissing: false,
-				Unique:       true,
-				Indexer: &memdb.StringFieldIndex{
-					Field: "ID",
-				},
-			},
-		},
-	}
-}
-
 func init() {
+	registerSchema(caBuiltinProviderTableSchema)
 	registerSchema(caConfigTableSchema)
 	registerSchema(caRootTableSchema)
-	registerSchema(caProviderTableSchema)
 }
 
 // CAConfig is used to pull the CA config from the snapshot.
@@ -170,7 +172,7 @@ func (s *Store) caSetConfigTxn(idx uint64, tx *memdb.Txn, config *structs.CAConf
 	if prev != nil {
 		existing := prev.(*structs.CAConfiguration)
 		config.CreateIndex = existing.CreateIndex
-		config.ClusterSerial = existing.ClusterSerial
+		config.ClusterID = existing.ClusterID
 	} else {
 		config.CreateIndex = idx
 	}
@@ -319,7 +321,7 @@ func (s *Store) CARootSetCAS(idx, cidx uint64, rs []*structs.CARoot) (bool, erro
 
 // CAProviderState is used to pull the built-in provider state from the snapshot.
 func (s *Snapshot) CAProviderState() (*structs.CAConsulProviderState, error) {
-	c, err := s.tx.First(caProviderTableName, "id")
+	c, err := s.tx.First(caBuiltinProviderTableName, "id")
 	if err != nil {
 		return nil, err
 	}
@@ -334,7 +336,7 @@ func (s *Snapshot) CAProviderState() (*structs.CAConsulProviderState, error) {
 
 // CAProviderState is used when restoring from a snapshot.
 func (s *Restore) CAProviderState(state *structs.CAConsulProviderState) error {
-	if err := s.tx.Insert(caProviderTableName, state); err != nil {
+	if err := s.tx.Insert(caBuiltinProviderTableName, state); err != nil {
 		return fmt.Errorf("failed restoring built-in CA state: %s", err)
 	}
 
@@ -347,10 +349,10 @@ func (s *Store) CAProviderState(id string) (uint64, *structs.CAConsulProviderSta
 	defer tx.Abort()
 
 	// Get the index
-	idx := maxIndexTxn(tx, caProviderTableName)
+	idx := maxIndexTxn(tx, caBuiltinProviderTableName)
 
 	// Get the provider config
-	c, err := tx.First(caProviderTableName, "id", id)
+	c, err := tx.First(caBuiltinProviderTableName, "id", id)
 	if err != nil {
 		return 0, nil, fmt.Errorf("failed built-in CA state lookup: %s", err)
 	}
@@ -369,10 +371,10 @@ func (s *Store) CAProviderStates() (uint64, []*structs.CAConsulProviderState, er
 	defer tx.Abort()
 
 	// Get the index
-	idx := maxIndexTxn(tx, caProviderTableName)
+	idx := maxIndexTxn(tx, caBuiltinProviderTableName)
 
 	// Get all
-	iter, err := tx.Get(caProviderTableName, "id")
+	iter, err := tx.Get(caBuiltinProviderTableName, "id")
 	if err != nil {
 		return 0, nil, fmt.Errorf("failed CA provider state lookup: %s", err)
 	}
@@ -390,7 +392,7 @@ func (s *Store) CASetProviderState(idx uint64, state *structs.CAConsulProviderSt
 	defer tx.Abort()
 
 	// Check for an existing config
-	existing, err := tx.First(caProviderTableName, "id", state.ID)
+	existing, err := tx.First(caBuiltinProviderTableName, "id", state.ID)
 	if err != nil {
 		return false, fmt.Errorf("failed built-in CA state lookup: %s", err)
 	}
@@ -403,12 +405,12 @@ func (s *Store) CASetProviderState(idx uint64, state *structs.CAConsulProviderSt
 	}
 	state.ModifyIndex = idx
 
-	if err := tx.Insert(caProviderTableName, state); err != nil {
+	if err := tx.Insert(caBuiltinProviderTableName, state); err != nil {
 		return false, fmt.Errorf("failed updating built-in CA state: %s", err)
 	}
 
 	// Update the index
-	if err := tx.Insert("index", &IndexEntry{caProviderTableName, idx}); err != nil {
+	if err := tx.Insert("index", &IndexEntry{caBuiltinProviderTableName, idx}); err != nil {
 		return false, fmt.Errorf("failed updating index: %s", err)
 	}
 
@@ -423,10 +425,10 @@ func (s *Store) CADeleteProviderState(id string) error {
 	defer tx.Abort()
 
 	// Get the index
-	idx := maxIndexTxn(tx, caProviderTableName)
+	idx := maxIndexTxn(tx, caBuiltinProviderTableName)
 
 	// Check for an existing config
-	existing, err := tx.First(caProviderTableName, "id", id)
+	existing, err := tx.First(caBuiltinProviderTableName, "id", id)
 	if err != nil {
 		return fmt.Errorf("failed built-in CA state lookup: %s", err)
 	}
@@ -437,10 +439,10 @@ func (s *Store) CADeleteProviderState(id string) error {
 	providerState := existing.(*structs.CAConsulProviderState)
 
 	// Do the delete and update the index
-	if err := tx.Delete(caProviderTableName, providerState); err != nil {
+	if err := tx.Delete(caBuiltinProviderTableName, providerState); err != nil {
 		return err
 	}
-	if err := tx.Insert("index", &IndexEntry{caProviderTableName, idx}); err != nil {
+	if err := tx.Insert("index", &IndexEntry{caBuiltinProviderTableName, idx}); err != nil {
 		return fmt.Errorf("failed updating index: %s", err)
 	}
 

agent/structs/connect_ca.go

@@ -31,9 +31,9 @@ type CARoot struct {
 	// RootCert is the PEM-encoded public certificate.
 	RootCert string
 
-	// Intermediates is a list of PEM-encoded intermediate certs to
+	// IntermediateCerts is a list of PEM-encoded intermediate certs to
 	// attach to any leaf certs signed by this CA.
-	Intermediates []string
+	IntermediateCerts []string
 
 	// SigningCert is the PEM-encoded signing certificate and SigningKey
 	// is the PEM-encoded private key for the signing certificate. These
@@ -146,8 +146,8 @@ const (
 
 // CAConfiguration is the configuration for the current CA plugin.
 type CAConfiguration struct {
-	// Unique identifier for the cluster
+	// ClusterID is a unique identifier for the cluster
-	ClusterSerial string `json:"-"`
+	ClusterID string `json:"-"`
 
 	// Provider is the CA provider implementation to use.
 	Provider string