luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

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.
This commit is contained in:
Daniel Nephin 2021-12-02 17:10:25 -05:00
parent d2f53d20ac
commit be1ddc5942
4 changed files with 37 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
agent/connect/ca/provider_consul_test.go
View File

@ -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)

28
agent/connect/ca/testing.go
View File

@ -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 {

26
agent/consul/fsm/commands_oss.go
View File

@ -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)
}
}

48
agent/consul/leader_connect_ca_test.go
View File

@ -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 {
case structs.CAOpSetConfig:
if req.Config.ModifyIndex != 0 {
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)
result := fsm.ApplyConnectCAOperationFromRequest(m.store, req, idx+1)
if err, ok := result.(error); ok && err != nil {
return nil, err
}
return result, nil
}
func (m *mockCAServerDelegate) forwardDC(method, dc string, args interface{}, reply interface{}) error {