From 4d41ee3887f0f4c3c332774ad13fa5ae8fa32e1d Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Thu, 23 Jul 2020 16:05:28 -0400 Subject: [PATCH] Move generation of the CA Configuration from the agent code into a method on the RuntimeConfig (#8363) This allows this to be reused elsewhere. --- agent/agent.go | 34 ++---------- agent/config/runtime.go | 55 +++++++++++++++++++ agent/config/runtime_test.go | 100 +++++++++++++++++++++++++++++++++++ agent/consul/config.go | 6 +-- agent/structs/connect_ca.go | 6 +++ 5 files changed, 168 insertions(+), 33 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index d41aef31e1..1062fce748 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1455,38 +1455,12 @@ func (a *Agent) consulConfig() (*consul.Config, error) { base.ConnectEnabled = true base.ConnectMeshGatewayWANFederationEnabled = a.config.ConnectMeshGatewayWANFederationEnabled - // Allow config to specify cluster_id provided it's a valid UUID. This is - // meant only for tests where a deterministic ID makes fixtures much simpler - // to work with but since it's only read on initial cluster bootstrap it's not - // that much of a liability in production. The worst a user could do is - // configure logically separate clusters with same ID by mistake but we can - // avoid documenting this is even an option. - if clusterID, ok := a.config.ConnectCAConfig["cluster_id"]; ok { - if cIDStr, ok := clusterID.(string); ok { - if _, err := uuid.ParseUUID(cIDStr); err == nil { - // Valid UUID configured, use that - base.CAConfig.ClusterID = cIDStr - } - } - if base.CAConfig.ClusterID == "" { - // If the tried to specify an ID but typoed it don't ignore as they will - // then bootstrap with a new ID and have to throw away the whole cluster - // and start again. - a.logger.Error("connect CA config cluster_id specified but " + - "is not a valid UUID, aborting startup") - return nil, fmt.Errorf("cluster_id was supplied but was not a valid UUID") - } + ca, err := a.config.ConnectCAConfiguration() + if err != nil { + return nil, err } - if a.config.ConnectCAProvider != "" { - base.CAConfig.Provider = a.config.ConnectCAProvider - } - - // Merge connect CA Config regardless of provider (since there are some - // common config options valid to all like leaf TTL). - for k, v := range a.config.ConnectCAConfig { - base.CAConfig.Config[k] = v - } + base.CAConfig = ca } // copy over auto config settings diff --git a/agent/config/runtime.go b/agent/config/runtime.go index 1ef292d646..0cbeec8b49 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/consul/types" + "github.com/hashicorp/go-uuid" "golang.org/x/time/rate" ) @@ -1686,6 +1687,60 @@ func (c *RuntimeConfig) ClientAddress() (unixAddr, httpAddr, httpsAddr string) { return } +func (c *RuntimeConfig) ConnectCAConfiguration() (*structs.CAConfiguration, error) { + if !c.ConnectEnabled { + return nil, nil + } + + ca := &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "RotationPeriod": structs.DefaultCARotationPeriod, + "LeafCertTTL": structs.DefaultLeafCertTTL, + "IntermediateCertTTL": structs.DefaultIntermediateCertTTL, + }, + } + + // Allow config to specify cluster_id provided it's a valid UUID. This is + // meant only for tests where a deterministic ID makes fixtures much simpler + // to work with but since it's only read on initial cluster bootstrap it's not + // that much of a liability in production. The worst a user could do is + // configure logically separate clusters with same ID by mistake but we can + // avoid documenting this is even an option. + if clusterID, ok := c.ConnectCAConfig["cluster_id"]; ok { + // If they tried to specify an ID but typoed it then don't ignore as they + // will then bootstrap with a new ID and have to throw away the whole cluster + // and start again. + + // ensure the cluster_id value in the opaque config is a string + cIDStr, ok := clusterID.(string) + if !ok { + return nil, fmt.Errorf("cluster_id was supplied but was not a string") + } + + // ensure that the cluster_id string is a valid UUID + _, err := uuid.ParseUUID(cIDStr) + if err != nil { + return nil, fmt.Errorf("cluster_id was supplied but was not a valid UUID") + } + + // now that we know the cluster_id is okay we can set it in the CAConfiguration + ca.ClusterID = cIDStr + } + + if c.ConnectCAProvider != "" { + ca.Provider = c.ConnectCAProvider + } + + // Merge connect CA Config regardless of provider (since there are some + // common config options valid to all like leaf TTL). + for k, v := range c.ConnectCAConfig { + ca.Config[k] = v + } + + return ca, nil +} + func (c *RuntimeConfig) APIConfig(includeClientCerts bool) (*api.Config, error) { cfg := &api.Config{ Datacenter: c.Datacenter, diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index ff11f33b60..07a042d3f2 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -7450,3 +7450,103 @@ func metaPairs(n int, format string) string { panic("invalid format: " + format) } } + +func TestConnectCAConfiguration(t *testing.T) { + type testCase struct { + config RuntimeConfig + expected *structs.CAConfiguration + err string + } + + cases := map[string]testCase{ + "connect-disabled": { + config: RuntimeConfig{ + ConnectEnabled: false, + }, + expected: nil, + }, + "defaults": { + config: RuntimeConfig{ + ConnectEnabled: true, + }, + expected: &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "RotationPeriod": "2160h", + "LeafCertTTL": "72h", + "IntermediateCertTTL": "8760h", // 365 * 24h + }, + }, + }, + "cluster-id-override": { + config: RuntimeConfig{ + ConnectEnabled: true, + ConnectCAConfig: map[string]interface{}{ + "cluster_id": "adfe7697-09b4-413a-ac0a-fa81ed3a3001", + }, + }, + expected: &structs.CAConfiguration{ + Provider: "consul", + ClusterID: "adfe7697-09b4-413a-ac0a-fa81ed3a3001", + Config: map[string]interface{}{ + "RotationPeriod": "2160h", + "LeafCertTTL": "72h", + "IntermediateCertTTL": "8760h", // 365 * 24h + "cluster_id": "adfe7697-09b4-413a-ac0a-fa81ed3a3001", + }, + }, + }, + "cluster-id-non-uuid": { + config: RuntimeConfig{ + ConnectEnabled: true, + ConnectCAConfig: map[string]interface{}{ + "cluster_id": "foo", + }, + }, + err: "cluster_id was supplied but was not a valid UUID", + }, + "provider-override": { + config: RuntimeConfig{ + ConnectEnabled: true, + ConnectCAProvider: "vault", + }, + expected: &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "RotationPeriod": "2160h", + "LeafCertTTL": "72h", + "IntermediateCertTTL": "8760h", // 365 * 24h + }, + }, + }, + "other-config": { + config: RuntimeConfig{ + ConnectEnabled: true, + ConnectCAConfig: map[string]interface{}{ + "foo": "bar", + }, + }, + expected: &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "RotationPeriod": "2160h", + "LeafCertTTL": "72h", + "IntermediateCertTTL": "8760h", // 365 * 24h + "foo": "bar", + }, + }, + }, + } + + for name, tcase := range cases { + t.Run(name, func(t *testing.T) { + actual, err := tcase.config.ConnectCAConfiguration() + if tcase.err != "" { + testutil.RequireErrorContains(t, err, tcase.err) + } else { + require.NoError(t, err) + require.Equal(t, tcase.expected, actual) + } + }) + } +} diff --git a/agent/consul/config.go b/agent/consul/config.go index 29f069352e..9498865057 100644 --- a/agent/consul/config.go +++ b/agent/consul/config.go @@ -594,9 +594,9 @@ func DefaultConfig() *Config { CAConfig: &structs.CAConfiguration{ Provider: "consul", Config: map[string]interface{}{ - "RotationPeriod": "2160h", - "LeafCertTTL": "72h", - "IntermediateCertTTL": "8760h", // 365 * 24h + "RotationPeriod": structs.DefaultCARotationPeriod, + "LeafCertTTL": structs.DefaultLeafCertTTL, + "IntermediateCertTTL": structs.DefaultIntermediateCertTTL, }, }, diff --git a/agent/structs/connect_ca.go b/agent/structs/connect_ca.go index 37facbf783..3755929f19 100644 --- a/agent/structs/connect_ca.go +++ b/agent/structs/connect_ca.go @@ -9,6 +9,12 @@ import ( "github.com/mitchellh/mapstructure" ) +const ( + DefaultCARotationPeriod = "2160h" + DefaultLeafCertTTL = "72h" + DefaultIntermediateCertTTL = "8760h" // 365 * 24h +) + // IndexedCARoots is the list of currently trusted CA Roots. type IndexedCARoots struct { // ActiveRootID is the ID of a root in Roots that is the active CA root.