connect/ca: some cleanup and reorganizing of the new methods

agent/connect/ca/provider_consul.go

@@ -21,9 +21,11 @@ import (
 var ErrNotInitialized = errors.New("provider not initialized")
 
 type ConsulProvider struct {
-	config   *structs.ConsulCAProviderConfig
+	Delegate ConsulProviderStateDelegate
-	id       string
+
-	delegate ConsulProviderStateDelegate
+	config *structs.ConsulCAProviderConfig
+	id     string
+	isRoot bool
 	sync.RWMutex
 }
 
@@ -32,14 +34,7 @@ type ConsulProviderStateDelegate interface {
 	ApplyCARequest(*structs.CARequest) error
 }
 
-// NewConsulProvider returns a new instance of the Consul CA provider,
+// Configure sets up the provider using the given configuration.
-// bootstrapping its state in the state store necessary
-func NewConsulProvider(delegate ConsulProviderStateDelegate) *ConsulProvider {
-	return &ConsulProvider{
-		delegate: delegate,
-	}
-}
-
 func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error {
 	// Parse the raw config and update our ID.
 	config, err := ParseConsulCAConfig(rawConfig)
@@ -47,10 +42,11 @@ func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[
 		return err
 	}
 	c.config = config
+	c.isRoot = isRoot
 	c.id = fmt.Sprintf("%s,%s", config.PrivateKey, config.RootCert)
 
 	// Exit early if the state store has an entry for this provider's config.
-	_, providerState, err := c.delegate.State().CAProviderState(c.id)
+	_, providerState, err := c.Delegate.State().CAProviderState(c.id)
 	if err != nil {
 		return err
 	}
@@ -61,24 +57,23 @@ func (c *ConsulProvider) Configure(clusterId string, isRoot bool, rawConfig map[
 
 	// Write the provider state to the state store.
 	newState := structs.CAConsulProviderState{
-		ID:     c.id,
+		ID: c.id,
-		IsRoot: isRoot,
 	}
 
 	args := &structs.CARequest{
 		Op:            structs.CAOpSetProviderState,
 		ProviderState: &newState,
 	}
-	if err := c.delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-// Return the active root CA and generate a new one if needed
+// ActiveRoot returns the active root CA certificate.
 func (c *ConsulProvider) ActiveRoot() (string, error) {
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	_, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
 		return "", err
@@ -87,8 +82,10 @@ func (c *ConsulProvider) ActiveRoot() (string, error) {
 	return providerState.RootCert, nil
 }
 
+// GenerateRoot initializes a new root certificate and private key
+// if needed.
 func (c *ConsulProvider) GenerateRoot() error {
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	idx, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
 		return err
@@ -97,7 +94,7 @@ func (c *ConsulProvider) GenerateRoot() error {
 	if providerState == nil {
 		return ErrNotInitialized
 	}
-	if !providerState.IsRoot {
+	if !c.isRoot {
 		return fmt.Errorf("provider is not the root certificate authority")
 	}
 	if providerState.RootCert != "" {
@@ -132,7 +129,7 @@ func (c *ConsulProvider) GenerateRoot() error {
 		Op:            structs.CAOpSetProviderState,
 		ProviderState: &newState,
 	}
-	if err := c.delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
 		return err
 	}
 
@@ -157,7 +154,7 @@ func (c *ConsulProvider) Cleanup() error {
 		Op:            structs.CAOpDeleteProviderState,
 		ProviderState: &structs.CAConsulProviderState{ID: c.id},
 	}
-	if err := c.delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
 		return err
 	}
 
@@ -173,7 +170,7 @@ func (c *ConsulProvider) Sign(csr *x509.CertificateRequest) (string, error) {
 	defer c.Unlock()
 
 	// Get the provider state
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	idx, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
 		return "", err
@@ -265,7 +262,7 @@ func (c *ConsulProvider) CrossSignCA(cert *x509.Certificate) (string, error) {
 	defer c.Unlock()
 
 	// Get the provider state
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	idx, providerState, err := state.CAProviderState(c.id)
 	if err != nil {
 		return "", err
@@ -333,7 +330,7 @@ func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulP
 		Op:            structs.CAOpSetProviderState,
 		ProviderState: &newState,
 	}
-	if err := c.delegate.ApplyCARequest(args); err != nil {
+	if err := c.Delegate.ApplyCARequest(args); err != nil {
 		return err
 	}
 
@@ -342,7 +339,7 @@ func (c *ConsulProvider) incrementProviderIndex(providerState *structs.CAConsulP
 
 // generateCA makes a new root CA using the current private key
 func (c *ConsulProvider) generateCA(privateKey string, sn uint64) (string, error) {
-	state := c.delegate.State()
+	state := c.Delegate.State()
 	_, config, err := state.CAConfig()
 	if err != nil {
 		return "", err

agent/connect/ca/provider_consul_test.go

@@ -75,7 +75,7 @@ func TestConsulCAProvider_Bootstrap(t *testing.T) {
 	conf := testConsulCAConfig()
 	delegate := newMockDelegate(t, conf)
 
-	provider := NewConsulProvider(delegate)
+	provider := &ConsulProvider{Delegate: delegate}
 	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
 	require.NoError(provider.GenerateRoot())
 
@@ -106,7 +106,7 @@ func TestConsulCAProvider_Bootstrap_WithCert(t *testing.T) {
 	}
 	delegate := newMockDelegate(t, conf)
 
-	provider := NewConsulProvider(delegate)
+	provider := &ConsulProvider{Delegate: delegate}
 	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
 	require.NoError(provider.GenerateRoot())
 
@@ -123,7 +123,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 	conf.Config["LeafCertTTL"] = "1h"
 	delegate := newMockDelegate(t, conf)
 
-	provider := NewConsulProvider(delegate)
+	provider := &ConsulProvider{Delegate: delegate}
 	require.NoError(provider.Configure(conf.ClusterID, true, conf.Config))
 	require.NoError(provider.GenerateRoot())
 
@@ -186,14 +186,14 @@ func TestConsulCAProvider_CrossSignCA(t *testing.T) {
 
 	conf1 := testConsulCAConfig()
 	delegate1 := newMockDelegate(t, conf1)
-	provider1 := NewConsulProvider(delegate1)
+	provider1 := &ConsulProvider{Delegate: delegate1}
 	require.NoError(provider1.Configure(conf1.ClusterID, true, conf1.Config))
 	require.NoError(provider1.GenerateRoot())
 
 	conf2 := testConsulCAConfig()
 	conf2.CreateIndex = 10
 	delegate2 := newMockDelegate(t, conf2)
-	provider2 := NewConsulProvider(delegate2)
+	provider2 := &ConsulProvider{Delegate: delegate2}
 	require.NoError(provider2.Configure(conf2.ClusterID, true, conf2.Config))
 	require.NoError(provider2.GenerateRoot())
 

agent/connect/ca/provider_vault.go

@@ -28,14 +28,7 @@ type VaultProvider struct {
 	clusterId string
 }
 
-// NewVaultProvider returns a vault provider with its root and intermediate PKI
+// Configure sets up the provider using the given configuration.
-// backends mounted and initialized. If the root backend is not set up already,
-// it will be mounted/generated as needed, but any existing state will not be
-// overwritten.
-func NewVaultProvider() *VaultProvider {
-	return &VaultProvider{}
-}
-
 func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[string]interface{}) error {
 	config, err := ParseVaultCAConfig(rawConfig)
 	if err != nil {
@@ -59,6 +52,12 @@ func (v *VaultProvider) Configure(clusterId string, isRoot bool, rawConfig map[s
 	return nil
 }
 
+// ActiveRoot returns the active root CA certificate.
+func (v *VaultProvider) ActiveRoot() (string, error) {
+	return v.getCA(v.config.RootPKIPath)
+}
+
+// GenerateRoot mounts and initializes a new root PKI backend if needed.
 func (v *VaultProvider) GenerateRoot() error {
 	if !v.isRoot {
 		return fmt.Errorf("provider is not the root certificate authority")
@@ -103,10 +102,7 @@ func (v *VaultProvider) GenerateRoot() error {
 	return nil
 }
 
-func (v *VaultProvider) ActiveRoot() (string, error) {
+// ActiveIntermediate returns the current intermediate certificate.
-	return v.getCA(v.config.RootPKIPath)
-}
-
 func (v *VaultProvider) ActiveIntermediate() (string, error) {
 	return v.getCA(v.config.IntermediatePKIPath)
 }

agent/connect/ca/provider_vault_test.go

@@ -38,7 +38,7 @@ func testVaultClusterWithConfig(t *testing.T, rawConf map[string]interface{}) (*
 	}
 
 	require := require.New(t)
-	provider := NewVaultProvider()
+	provider := &VaultProvider{}
 	require.NoError(provider.Configure("asdf", true, conf))
 	require.NoError(provider.GenerateRoot())
 	_, err := provider.GenerateIntermediate()

agent/consul/leader.go

@@ -526,9 +526,9 @@ func parseCARoot(pemValue, provider string) (*structs.CARoot, error) {
 func (s *Server) createCAProvider(conf *structs.CAConfiguration) (ca.Provider, error) {
 	switch conf.Provider {
 	case structs.ConsulCAProvider:
-		return ca.NewConsulProvider(&consulCADelegate{s}), nil
+		return &ca.ConsulProvider{Delegate: &consulCADelegate{s}}, nil
 	case structs.VaultCAProvider:
-		return ca.NewVaultProvider(), nil
+		return &ca.VaultProvider{}, nil
 	default:
 		return nil, fmt.Errorf("unknown CA provider %q", conf.Provider)
 	}

agent/structs/connect_ca.go

@@ -253,7 +253,6 @@ type CAConsulProviderState struct {
 	ID               string
 	PrivateKey       string
 	RootCert         string
-	IsRoot           bool
 	IntermediateCert string
 
 	RaftIndex