ca: use the real FSM operation in tests

Previously we had a couple copies that reproduced the FSM operation.
These copies introduce risk that the test does not accurately match
production.

This PR removes the test versions of the FSM operation, and exports the
real production FSM operation so that it can be used in tests.

The consul provider tests did need to change because of this. Previously
we would return a hardcoded value of 2, but in production this value is
always incremented.

agent/connect/ca/provider_consul_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent/connect"
+	"github.com/hashicorp/consul/agent/consul/fsm"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
 )
@@ -23,7 +24,16 @@ func (c *consulCAMockDelegate) ProviderState(id string) (*structs.CAConsulProvid
 }
 
 func (c *consulCAMockDelegate) ApplyCARequest(req *structs.CARequest) (interface{}, error) {
-	return ApplyCARequestToStore(c.state, req)
+	idx, _, err := c.state.CAConfig(nil)
+	if err != nil {
+		return nil, err
+	}
+
+	result := fsm.ApplyConnectCAOperationFromRequest(c.state, req, idx+1)
+	if err, ok := result.(error); ok && err != nil {
+		return nil, err
+	}
+	return result, nil
 }
 
 func newMockDelegate(t *testing.T, conf *structs.CAConfiguration) *consulCAMockDelegate {
@@ -176,7 +186,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 				require.NoError(err)
 				require.Equal(spiffeService.URI(), parsed.URIs[0])
 				require.Empty(parsed.Subject.CommonName)
-				require.Equal(uint64(2), parsed.SerialNumber.Uint64())
+				require.Equal(uint64(3), parsed.SerialNumber.Uint64())
 				subjectKeyID, err := connect.KeyId(csr.PublicKey)
 				require.NoError(err)
 				require.Equal(subjectKeyID, parsed.SubjectKeyId)
@@ -205,7 +215,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 				require.NoError(err)
 				require.Equal(spiffeService.URI(), parsed.URIs[0])
 				require.Empty(parsed.Subject.CommonName)
-				require.Equal(parsed.SerialNumber.Uint64(), uint64(2))
+				require.Equal(uint64(4), parsed.SerialNumber.Uint64())
 				requireNotEncoded(t, parsed.SubjectKeyId)
 				requireNotEncoded(t, parsed.AuthorityKeyId)
 
@@ -233,7 +243,7 @@ func TestConsulCAProvider_SignLeaf(t *testing.T) {
 				require.NoError(err)
 				require.Equal(spiffeAgent.URI(), parsed.URIs[0])
 				require.Empty(parsed.Subject.CommonName)
-				require.Equal(uint64(2), parsed.SerialNumber.Uint64())
+				require.Equal(uint64(5), parsed.SerialNumber.Uint64())
 				requireNotEncoded(t, parsed.SubjectKeyId)
 				requireNotEncoded(t, parsed.AuthorityKeyId)
 

agent/connect/ca/testing.go

@@ -12,8 +12,6 @@ import (
 	"github.com/mitchellh/go-testing-interface"
 
 	"github.com/hashicorp/consul/agent/connect"
-	"github.com/hashicorp/consul/agent/consul/state"
-	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/sdk/freeport"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 )
@@ -222,32 +220,6 @@ func (v *TestVaultServer) Stop() error {
 	return nil
 }
 
-func ApplyCARequestToStore(store *state.Store, req *structs.CARequest) (interface{}, error) {
-	idx, _, err := store.CAConfig(nil)
-	if err != nil {
-		return nil, err
-	}
-
-	switch req.Op {
-	case structs.CAOpSetProviderState:
-		_, err := store.CASetProviderState(idx+1, req.ProviderState)
-		if err != nil {
-			return nil, err
-		}
-
-		return true, nil
-	case structs.CAOpDeleteProviderState:
-		if err := store.CADeleteProviderState(idx+1, req.ProviderState.ID); err != nil {
-			return nil, err
-		}
-
-		return true, nil
-	case structs.CAOpIncrementProviderSerialNumber:
-		return uint64(2), nil
-	default:
-		return nil, fmt.Errorf("Invalid CA operation '%s'", req.Op)
-	}
-}
 func requireTrailingNewline(t testing.T, leafPEM string) {
 	t.Helper()
 	if len(leafPEM) == 0 {

agent/consul/fsm/commands_oss.go

@@ -378,10 +378,19 @@ func (c *FSM) applyConnectCAOperation(buf []byte, index uint64) interface{} {
 		[]metrics.Label{{Name: "op", Value: string(req.Op)}})
 	defer metrics.MeasureSinceWithLabels([]string{"fsm", "ca"}, time.Now(),
 		[]metrics.Label{{Name: "op", Value: string(req.Op)}})
+
+	result := ApplyConnectCAOperationFromRequest(c.state, &req, index)
+	if err, ok := result.(error); ok && err != nil {
+		c.logger.Warn("Failed to apply CA operation", "operation", req.Op)
+	}
+	return result
+}
+
+func ApplyConnectCAOperationFromRequest(state *state.Store, req *structs.CARequest, index uint64) interface{} {
 	switch req.Op {
 	case structs.CAOpSetConfig:
 		if req.Config.ModifyIndex != 0 {
-			act, err := c.state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
+			act, err := state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
 			if err != nil {
 				return err
 			}
@@ -389,29 +398,29 @@ func (c *FSM) applyConnectCAOperation(buf []byte, index uint64) interface{} {
 			return act
 		}
 
-		return c.state.CASetConfig(index, req.Config)
+		return state.CASetConfig(index, req.Config)
 	case structs.CAOpSetRoots:
-		act, err := c.state.CARootSetCAS(index, req.Index, req.Roots)
+		act, err := state.CARootSetCAS(index, req.Index, req.Roots)
 		if err != nil {
 			return err
 		}
 
 		return act
 	case structs.CAOpSetProviderState:
-		act, err := c.state.CASetProviderState(index, req.ProviderState)
+		act, err := state.CASetProviderState(index, req.ProviderState)
 		if err != nil {
 			return err
 		}
 
 		return act
 	case structs.CAOpDeleteProviderState:
-		if err := c.state.CADeleteProviderState(index, req.ProviderState.ID); err != nil {
+		if err := state.CADeleteProviderState(index, req.ProviderState.ID); err != nil {
 			return err
 		}
 
 		return true
 	case structs.CAOpSetRootsAndConfig:
-		act, err := c.state.CARootSetCAS(index, req.Index, req.Roots)
+		act, err := state.CARootSetCAS(index, req.Index, req.Roots)
 		if err != nil {
 			return err
 		}
@@ -419,20 +428,19 @@ func (c *FSM) applyConnectCAOperation(buf []byte, index uint64) interface{} {
 			return act
 		}
 
-		act, err = c.state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
+		act, err = state.CACheckAndSetConfig(index, req.Config.ModifyIndex, req.Config)
 		if err != nil {
 			return err
 		}
 		return act
 	case structs.CAOpIncrementProviderSerialNumber:
-		sn, err := c.state.CAIncrementProviderSerialNumber(index)
+		sn, err := state.CAIncrementProviderSerialNumber(index)
 		if err != nil {
 			return err
 		}
 
 		return sn
 	default:
-		c.logger.Warn("Invalid CA operation", "operation", req.Op)
 		return fmt.Errorf("Invalid CA operation '%s'", req.Op)
 	}
 }

agent/consul/leader_connect_ca_test.go

@@ -14,18 +14,18 @@ import (
 	"testing"
 	"time"
 
-	"github.com/hashicorp/consul/testrpc"
-
 	"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/fsm"
 	"github.com/hashicorp/consul/agent/consul/state"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/agent/token"
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
+	"github.com/hashicorp/consul/testrpc"
 )
 
 // TODO(kyhavlov): replace with t.Deadline()
@@ -83,47 +83,11 @@ func (m *mockCAServerDelegate) ApplyCARequest(req *structs.CARequest) (interface
 
 	m.callbackCh <- fmt.Sprintf("raftApply/ConnectCA")
 
-	switch req.Op {
+	result := fsm.ApplyConnectCAOperationFromRequest(m.store, req, idx+1)
-	case structs.CAOpSetConfig:
+	if err, ok := result.(error); ok && err != nil {
-		if req.Config.ModifyIndex != 0 {
+		return nil, err
-			act, err := m.store.CACheckAndSetConfig(idx+1, req.Config.ModifyIndex, req.Config)
-			if err != nil {
-				return nil, err
-			}
-
-			return act, nil
-		}
-
-		return nil, m.store.CASetConfig(idx+1, req.Config)
-	case structs.CAOpSetRootsAndConfig:
-		act, err := m.store.CARootSetCAS(idx, req.Index, req.Roots)
-		if err != nil || !act {
-			return act, err
-		}
-
-		act, err = m.store.CACheckAndSetConfig(idx+1, req.Config.ModifyIndex, req.Config)
-		if err != nil {
-			return nil, err
-		}
-		return act, nil
-	case structs.CAOpSetProviderState:
-		_, err := m.store.CASetProviderState(idx+1, req.ProviderState)
-		if err != nil {
-			return nil, err
-		}
-
-		return true, nil
-	case structs.CAOpDeleteProviderState:
-		if err := m.store.CADeleteProviderState(idx+1, req.ProviderState.ID); err != nil {
-			return nil, err
-		}
-
-		return true, nil
-	case structs.CAOpIncrementProviderSerialNumber:
-		return uint64(2), nil
-	default:
-		return nil, fmt.Errorf("Invalid CA operation '%s'", req.Op)
 	}
+	return result, nil
 }
 
 func (m *mockCAServerDelegate) forwardDC(method, dc string, args interface{}, reply interface{}) error {