From b92084b8e85becfe14dde3839fe0428b2ca43c2e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 11 Nov 2021 19:03:52 -0500 Subject: [PATCH] ca: reduce consul provider backend interface a bit This makes it easier to fake, which will allow me to use the ConsulProvider as an 'external PKI' to test a customer setup where the actual root CA is not the root we use for the Consul CA. Replaces a call to the state store to fetch the clusterID with the clusterID field already available on the built-in provider. --- agent/connect/ca/provider_consul.go | 10 ++++------ agent/connect/ca/provider_consul_test.go | 5 +++-- agent/connect/ca/provider_vault.go | 1 - agent/connect/ca/testing.go | 5 ++++- agent/consul/leader_connect_ca.go | 7 +++++++ agent/consul/leader_connect_ca_test.go | 5 +++++ agent/consul/server.go | 2 +- 7 files changed, 24 insertions(+), 11 deletions(-) diff --git a/agent/connect/ca/provider_consul.go b/agent/connect/ca/provider_consul.go index bac728cb86..dea91e5d7d 100644 --- a/agent/connect/ca/provider_consul.go +++ b/agent/connect/ca/provider_consul.go @@ -17,7 +17,6 @@ import ( "github.com/hashicorp/go-hclog" "github.com/hashicorp/consul/agent/connect" - "github.com/hashicorp/consul/agent/consul/state" "github.com/hashicorp/consul/agent/structs" ) @@ -56,7 +55,7 @@ func NewConsulProvider(delegate ConsulProviderStateDelegate, logger hclog.Logger } type ConsulProviderStateDelegate interface { - State() *state.Store + ProviderState(id string) (*structs.CAConsulProviderState, error) ApplyCARequest(*structs.CARequest) (interface{}, error) } @@ -82,7 +81,7 @@ func (c *ConsulProvider) Configure(cfg ProviderConfig) error { c.parseTestState(cfg.RawConfig, cfg.State) // 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.ProviderState(c.id) if err != nil { return err } @@ -98,7 +97,7 @@ func (c *ConsulProvider) Configure(cfg ProviderConfig) error { // Check if there are any entries with old ID schemes. for _, oldID := range oldIDs { - _, providerState, err = c.Delegate.State().CAProviderState(oldID) + providerState, err = c.Delegate.ProviderState(oldID) if err != nil { return err } @@ -589,8 +588,7 @@ func (c *ConsulProvider) SupportsCrossSigning() (bool, error) { // getState returns the current provider state from the state delegate, and returns // ErrNotInitialized if no entry is found. func (c *ConsulProvider) getState() (*structs.CAConsulProviderState, error) { - stateStore := c.Delegate.State() - _, providerState, err := stateStore.CAProviderState(c.id) + providerState, err := c.Delegate.ProviderState(c.id) if err != nil { return nil, err } diff --git a/agent/connect/ca/provider_consul_test.go b/agent/connect/ca/provider_consul_test.go index f4e7c7923f..6b4f318377 100644 --- a/agent/connect/ca/provider_consul_test.go +++ b/agent/connect/ca/provider_consul_test.go @@ -17,8 +17,9 @@ type consulCAMockDelegate struct { state *state.Store } -func (c *consulCAMockDelegate) State() *state.Store { - return c.state +func (c *consulCAMockDelegate) ProviderState(id string) (*structs.CAConsulProviderState, error) { + _, s, err := c.state.CAProviderState(id) + return s, err } func (c *consulCAMockDelegate) ApplyCARequest(req *structs.CARequest) (interface{}, error) { diff --git a/agent/connect/ca/provider_vault.go b/agent/connect/ca/provider_vault.go index adc9851d0d..ef1920d7af 100644 --- a/agent/connect/ca/provider_vault.go +++ b/agent/connect/ca/provider_vault.go @@ -246,7 +246,6 @@ func (v *VaultProvider) GenerateRoot() error { DefaultLeaseTTL: v.config.RootCertTTL.String(), }, }) - if err != nil { return err } diff --git a/agent/connect/ca/testing.go b/agent/connect/ca/testing.go index 00f49c5790..3b470063f0 100644 --- a/agent/connect/ca/testing.go +++ b/agent/connect/ca/testing.go @@ -168,8 +168,11 @@ func runTestVault(t testing.T) (*TestVaultServer, error) { returnPortsFn: returnPortsFn, } t.Cleanup(func() { - testVault.Stop() + if err := testVault.Stop(); err != nil { + t.Log("failed to stop vault server: %w", err) + } }) + return testVault, nil } diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index 22a4ec766e..e03b1ed185 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -38,6 +38,8 @@ const ( // easier testing. type caServerDelegate interface { ca.ConsulProviderStateDelegate + + State() *state.Store IsLeader() bool ApplyCALeafRequest() (uint64, error) @@ -138,6 +140,11 @@ func (c *caDelegateWithState) ServersSupportMultiDCConnectCA() error { return nil } +func (c *caDelegateWithState) ProviderState(id string) (*structs.CAConsulProviderState, error) { + _, s, err := c.fsm.State().CAProviderState(id) + return s, err +} + func NewCAManager(delegate caServerDelegate, leaderRoutineManager *routine.Manager, logger hclog.Logger, config *Config) *CAManager { return &CAManager{ delegate: delegate, diff --git a/agent/consul/leader_connect_ca_test.go b/agent/consul/leader_connect_ca_test.go index 9ea55fd2d1..a1ca8c12df 100644 --- a/agent/consul/leader_connect_ca_test.go +++ b/agent/consul/leader_connect_ca_test.go @@ -53,6 +53,11 @@ func (m *mockCAServerDelegate) State() *state.Store { return m.store } +func (m *mockCAServerDelegate) ProviderState(id string) (*structs.CAConsulProviderState, error) { + _, s, err := m.store.CAProviderState(id) + return s, err +} + func (m *mockCAServerDelegate) IsLeader() bool { return true } diff --git a/agent/consul/server.go b/agent/consul/server.go index 1d4bedb6b5..dee51b15b9 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -472,7 +472,7 @@ func NewServer(config *Config, flat Deps) (*Server, error) { return nil, fmt.Errorf("Failed to start Raft: %v", err) } - s.caManager = NewCAManager(&caDelegateWithState{s}, s.leaderRoutineManager, s.logger.ResetNamed("connect.ca"), s.config) + s.caManager = NewCAManager(&caDelegateWithState{Server: s}, s.leaderRoutineManager, s.logger.ResetNamed("connect.ca"), s.config) if s.config.ConnectEnabled && (s.config.AutoEncryptAllowTLS || s.config.AutoConfigAuthzEnabled) { go s.connectCARootsMonitor(&lib.StopChannelContext{StopCh: s.shutdownCh}) }