ca/provider: remove ActiveRoot from Provider

agent/connect/ca/provider.go

@@ -129,14 +129,7 @@ type PrimaryProvider interface {
 	//
 	// The provider should return an existing root certificate if one exists,
 	// otherwise it should generate a new root certificate and return it.
-	GenerateRoot() error
-
-	// ActiveRoot returns the currently active root CA for this
-	// provider. This should be a parent of the certificate returned by
-	// ActiveIntermediate()
-	//
-	// TODO: currently called from secondaries, but shouldn't be so is on PrimaryProvider
-	ActiveRoot() (string, error)
+	GenerateRoot() (RootResult, error)
 
 	// GenerateIntermediate returns a new intermediate signing cert and sets it to
 	// the active intermediate. If multiple intermediates are needed to complete
@@ -189,6 +182,14 @@ type SecondaryProvider interface {
 	SetIntermediate(intermediatePEM, rootPEM string) error
 }
 
+// RootResult is the result returned by PrimaryProvider.GenerateRoot.
+//
+// TODO: rename this struct
+type RootResult struct {
+	// PEM encoded certificate that will be used as the primary CA.
+	PEM string
+}
+
 // NeedsStop is an optional interface that allows a CA to define a function
 // to be called when the CA instance is no longer in use. This is different
 // from Cleanup(), as only the local provider instance is being shut down

agent/connect/ca/provider_aws.go

@@ -134,12 +134,19 @@ func (a *AWSProvider) State() (map[string]string, error) {
 }
 
 // GenerateRoot implements Provider
-func (a *AWSProvider) GenerateRoot() error {
+func (a *AWSProvider) GenerateRoot() (RootResult, error) {
 	if !a.isPrimary {
-		return fmt.Errorf("provider is not the root certificate authority")
+		return RootResult{}, fmt.Errorf("provider is not the root certificate authority")
 	}
 
-	return a.ensureCA()
+	if err := a.ensureCA(); err != nil {
+		return RootResult{}, err
+	}
+
+	if a.rootPEM == "" {
+		return RootResult{}, fmt.Errorf("AWS CA provider not fully Initialized")
+	}
+	return RootResult{PEM: a.rootPEM}, nil
 }
 
 // ensureCA loads the CA resource to check it exists if configured by User or in
@@ -489,19 +496,6 @@ func (a *AWSProvider) signCSR(csrPEM string, templateARN string, ttl time.Durati
 		})
 }
 
-// ActiveRoot implements Provider
-func (a *AWSProvider) ActiveRoot() (string, error) {
-	err := a.ensureCA()
-	if err != nil {
-		return "", err
-	}
-
-	if a.rootPEM == "" {
-		return "", fmt.Errorf("Secondary AWS CA provider not fully Initialized")
-	}
-	return a.rootPEM, nil
-}
-
 // GenerateIntermediateCSR implements Provider
 func (a *AWSProvider) GenerateIntermediateCSR() (string, error) {
 	if a.isPrimary {

agent/connect/ca/provider_aws_test.go

@@ -47,12 +47,9 @@ func TestAWSBootstrapAndSignPrimary(t *testing.T) {
 			provider := testAWSProvider(t, testProviderConfigPrimary(t, cfg))
 			defer provider.Cleanup(true, nil)
 
-			// Generate the root
-			require.NoError(provider.GenerateRoot())
-
-			// Fetch Active Root
-			rootPEM, err := provider.ActiveRoot()
+			root, err := provider.GenerateRoot()
 			require.NoError(err)
+			rootPEM := root.PEM
 
 			// Generate Intermediate (not actually needed for this provider for now
 			// but this simulates the calls in Server.initializeRoot).
@@ -82,16 +79,12 @@ func TestAWSBootstrapAndSignPrimary(t *testing.T) {
 	}
 
 	t.Run("Test default root ttl for aws ca provider", func(t *testing.T) {
-
 		provider := testAWSProvider(t, testProviderConfigPrimary(t, nil))
 		defer provider.Cleanup(true, nil)
 
-		// Generate the root
-		require.NoError(t, provider.GenerateRoot())
-
-		// Fetch Active Root
-		rootPEM, err := provider.ActiveRoot()
+		root, err := provider.GenerateRoot()
 		require.NoError(t, err)
+		rootPEM := root.PEM
 
 		// Ensure they use the right key type
 		rootCert, err := connect.ParseCert(rootPEM)
@@ -124,8 +117,9 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 
 	p1 := testAWSProvider(t, testProviderConfigPrimary(t, nil))
 	defer p1.Cleanup(true, nil)
-	rootPEM, err := p1.ActiveRoot()
+	root, err := p1.GenerateRoot()
 	require.NoError(t, err)
+	rootPEM := root.PEM
 
 	p2 := testAWSProvider(t, testProviderConfigSecondary(t, nil))
 	defer p2.Cleanup(true, nil)
@@ -152,8 +146,9 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 		cfg1 := testProviderConfigPrimary(t, nil)
 		cfg1.State = p1State
 		p1 = testAWSProvider(t, cfg1)
-		newRootPEM, err := p1.ActiveRoot()
+		root, err := p1.GenerateRoot()
 		require.NoError(t, err)
+		newRootPEM := root.PEM
 
 		cfg2 := testProviderConfigPrimary(t, nil)
 		cfg2.State = p2State
@@ -185,8 +180,9 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 			"ExistingARN": p1State[AWSStateCAARNKey],
 		})
 		p1 = testAWSProvider(t, cfg1)
-		newRootPEM, err := p1.ActiveRoot()
+		root, err := p1.GenerateRoot()
 		require.NoError(t, err)
+		newRootPEM := root.PEM
 
 		cfg2 := testProviderConfigPrimary(t, map[string]interface{}{
 			"ExistingARN": p2State[AWSStateCAARNKey],
@@ -223,8 +219,9 @@ func TestAWSBootstrapAndSignSecondary(t *testing.T) {
 		p2 = testAWSProvider(t, cfg2)
 		require.NoError(t, p2.SetIntermediate(newIntPEM, newRootPEM))
 
-		newRootPEM, err = p1.ActiveRoot()
+		root, err = p1.GenerateRoot()
 		require.NoError(t, err)
+		newRootPEM = root.PEM
 		newIntPEM, err = p2.ActiveIntermediate()
 		require.NoError(t, err)
 
@@ -244,7 +241,8 @@ func TestAWSBootstrapAndSignSecondaryConsul(t *testing.T) {
 		p1 := TestConsulProvider(t, delegate)
 		cfg := testProviderConfig(conf)
 		require.NoError(t, p1.Configure(cfg))
-		require.NoError(t, p1.GenerateRoot())
+		_, err := p1.GenerateRoot()
+		require.NoError(t, err)
 
 		p2 := testAWSProvider(t, testProviderConfigSecondary(t, nil))
 		defer p2.Cleanup(true, nil)
@@ -255,7 +253,9 @@ func TestAWSBootstrapAndSignSecondaryConsul(t *testing.T) {
 	t.Run("pri=aws,sec=consul", func(t *testing.T) {
 		p1 := testAWSProvider(t, testProviderConfigPrimary(t, nil))
 		defer p1.Cleanup(true, nil)
-		require.NoError(t, p1.GenerateRoot())
+
+		_, err := p1.GenerateRoot()
+		require.NoError(t, err)
 
 		conf := testConsulCAConfig()
 		delegate := newMockDelegate(t, conf)
@@ -316,11 +316,13 @@ func TestAWSProvider_Cleanup(t *testing.T) {
 	}
 
 	requirePCADeleted := func(t *testing.T, provider *AWSProvider) {
+		t.Helper()
 		deleted, err := describeCA(t, provider)
 		require.True(t, err != nil || deleted, "The AWS PCA instance has not been deleted")
 	}
 
 	requirePCANotDeleted := func(t *testing.T, provider *AWSProvider) {
+		t.Helper()
 		deleted, err := describeCA(t, provider)
 		require.NoError(t, err)
 		require.False(t, deleted, "The AWS PCA instance should not have been deleted")

agent/connect/ca/provider_consul.go

@@ -149,36 +149,26 @@ func (c *ConsulProvider) State() (map[string]string, error) {
 	return c.testState, nil
 }
 
-// ActiveRoot returns the active root CA certificate.
-func (c *ConsulProvider) ActiveRoot() (string, error) {
-	providerState, err := c.getState()
-	if err != nil {
-		return "", err
-	}
-
-	return providerState.RootCert, nil
-}
-
 // GenerateRoot initializes a new root certificate and private key if needed.
-func (c *ConsulProvider) GenerateRoot() error {
+func (c *ConsulProvider) GenerateRoot() (RootResult, error) {
 	providerState, err := c.getState()
 	if err != nil {
-		return err
+		return RootResult{}, err
 	}
 
 	if !c.isPrimary {
-		return fmt.Errorf("provider is not the root certificate authority")
+		return RootResult{}, fmt.Errorf("provider is not the root certificate authority")
 	}
 	if providerState.RootCert != "" {
-		return nil
+		return RootResult{PEM: providerState.RootCert}, nil
 	}
 
 	// Generate a private key if needed
 	newState := *providerState
 	if c.config.PrivateKey == "" {
 		_, pk, err := connect.GeneratePrivateKeyWithConfig(c.config.PrivateKeyType, c.config.PrivateKeyBits)
 		if err != nil {
-			return err
+			return RootResult{}, err
 		}
 		newState.PrivateKey = pk
 	} else {
@@ -189,12 +179,12 @@ func (c *ConsulProvider) GenerateRoot() error {
 	if c.config.RootCert == "" {
 		nextSerial, err := c.incrementAndGetNextSerialNumber()
 		if err != nil {
-			return fmt.Errorf("error computing next serial number: %v", err)
+			return RootResult{}, fmt.Errorf("error computing next serial number: %v", err)
 		}
 
 		ca, err := c.generateCA(newState.PrivateKey, nextSerial, c.config.RootCertTTL)
 		if err != nil {
-			return fmt.Errorf("error generating CA: %v", err)
+			return RootResult{}, fmt.Errorf("error generating CA: %v", err)
 		}
 		newState.RootCert = ca
 	} else {
@@ -207,10 +197,10 @@ func (c *ConsulProvider) GenerateRoot() error {
 		ProviderState: &newState,
 	}
 	if _, err := c.Delegate.ApplyCARequest(args); err != nil {
-		return err
+		return RootResult{}, err
 	}
 
-	return nil
+	return RootResult{PEM: newState.RootCert}, nil
 }
 
 // GenerateIntermediateCSR creates a private key and generates a CSR
@@ -287,18 +277,15 @@ func (c *ConsulProvider) SetIntermediate(intermediatePEM, rootPEM string) error
 	return nil
 }
 
-// We aren't maintaining separate root/intermediate CAs for the builtin
-// provider, so just return the root.
 func (c *ConsulProvider) ActiveIntermediate() (string, error) {
-	if c.isPrimary {
-		return c.ActiveRoot()
-	}
-
 	providerState, err := c.getState()
 	if err != nil {
 		return "", err
 	}
 
+	if c.isPrimary {
+		return providerState.RootCert, nil
+	}
 	return providerState.IntermediateCert, nil
 }