From 364ef3d052e975a680736e1ee2e2063e36d941bc Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 6 Aug 2021 18:02:55 -0400 Subject: [PATCH] server: remove defaulting of PrimaryDatacenter The constructor for Server is not at all the appropriate place to be setting default values for a config struct that was passed in. In production this value is always set from agent/config. In tests we should set the default in a test helper. --- agent/consul/auto_encrypt_endpoint_test.go | 1 + agent/consul/connect_ca_endpoint_test.go | 4 ++++ agent/consul/federation_state_endpoint_test.go | 4 ++++ agent/consul/leader_connect_test.go | 3 +++ agent/consul/leader_test.go | 1 + agent/consul/server.go | 14 -------------- agent/consul/server_test.go | 3 +++ 7 files changed, 16 insertions(+), 14 deletions(-) diff --git a/agent/consul/auto_encrypt_endpoint_test.go b/agent/consul/auto_encrypt_endpoint_test.go index 40bc8e507..800392b95 100644 --- a/agent/consul/auto_encrypt_endpoint_test.go +++ b/agent/consul/auto_encrypt_endpoint_test.go @@ -57,6 +57,7 @@ func TestAutoEncryptSign(t *testing.T) { } dir, s := testServerWithConfig(t, func(c *Config) { c.AutoEncryptAllowTLS = true + c.PrimaryDatacenter = "dc1" c.Bootstrap = true c.TLSConfig.CAFile = root c.TLSConfig.VerifyOutgoing = true diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index 808cfe703..448286094 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -670,6 +670,7 @@ func TestConnectCAConfig_UpdateSecondary(t *testing.T) { // Initialize primary as the primary DC dir1, s1 := testServerWithConfig(t, func(c *Config) { c.Datacenter = "primary" + c.PrimaryDatacenter = "primary" }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -842,6 +843,7 @@ func TestConnectCASign(t *testing.T) { assert := assert.New(t) require := require.New(t) dir1, s1 := testServerWithConfig(t, func(cfg *Config) { + cfg.PrimaryDatacenter = "dc1" cfg.CAConfig.Config["PrivateKeyType"] = tt.caKeyType cfg.CAConfig.Config["PrivateKeyBits"] = tt.caKeyBits }) @@ -931,6 +933,7 @@ func TestConnectCASign_rateLimit(t *testing.T) { require := require.New(t) dir1, s1 := testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc1" + c.PrimaryDatacenter = "dc1" c.Bootstrap = true c.CAConfig.Config = map[string]interface{}{ // It actually doesn't work as expected with some higher values because @@ -996,6 +999,7 @@ func TestConnectCASign_concurrencyLimit(t *testing.T) { require := require.New(t) dir1, s1 := testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc1" + c.PrimaryDatacenter = "dc1" c.Bootstrap = true c.CAConfig.Config = map[string]interface{}{ // Must disable the rate limit since it takes precedence diff --git a/agent/consul/federation_state_endpoint_test.go b/agent/consul/federation_state_endpoint_test.go index 92dbbe497..b48f42b21 100644 --- a/agent/consul/federation_state_endpoint_test.go +++ b/agent/consul/federation_state_endpoint_test.go @@ -27,6 +27,7 @@ func TestFederationState_Apply_Upsert(t *testing.T) { dir1, s1 := testServerWithConfig(t, func(c *Config) { c.DisableFederationStateAntiEntropy = true + c.PrimaryDatacenter = "dc1" }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -190,6 +191,7 @@ func TestFederationState_Get(t *testing.T) { dir1, s1 := testServerWithConfig(t, func(c *Config) { c.DisableFederationStateAntiEntropy = true + c.PrimaryDatacenter = "dc1" }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -306,6 +308,7 @@ func TestFederationState_List(t *testing.T) { dir1, s1 := testServerWithConfig(t, func(c *Config) { c.DisableFederationStateAntiEntropy = true + c.PrimaryDatacenter = "dc1" }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -596,6 +599,7 @@ func TestFederationState_Apply_Delete(t *testing.T) { dir1, s1 := testServerWithConfig(t, func(c *Config) { c.DisableFederationStateAntiEntropy = true + c.PrimaryDatacenter = "dc1" }) defer os.RemoveAll(dir1) defer s1.Shutdown() diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index ea10e5afe..038f2f0ff 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -55,6 +55,7 @@ func TestLeader_Builtin_PrimaryCA_ChangeKeyConfig(t *testing.T) { // Initialize primary as the primary DC dir1, srv := testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc1" + c.PrimaryDatacenter = "dc1" c.Build = "1.6.0" c.CAConfig.Config["PrivateKeyType"] = src.keyType c.CAConfig.Config["PrivateKeyBits"] = src.keyBits @@ -600,6 +601,7 @@ func TestLeader_SecondaryCA_IntermediateRefresh(t *testing.T) { dir1, s1 := testServerWithConfig(t, func(c *Config) { c.Build = "1.6.0" + c.PrimaryDatacenter = "dc1" }) defer os.RemoveAll(dir1) defer s1.Shutdown() @@ -850,6 +852,7 @@ func TestLeader_SecondaryCA_FixSigningKeyID_via_IntermediateRefresh(t *testing.T dir1, s1 := testServerWithConfig(t, func(c *Config) { c.Build = "1.6.0" + c.PrimaryDatacenter = "dc1" }) defer os.RemoveAll(dir1) defer s1.Shutdown() diff --git a/agent/consul/leader_test.go b/agent/consul/leader_test.go index cedf8386b..8527ea9e9 100644 --- a/agent/consul/leader_test.go +++ b/agent/consul/leader_test.go @@ -1232,6 +1232,7 @@ func TestLeader_ACLUpgrade(t *testing.T) { t.Parallel() dir1, s1 := testServerWithConfig(t, func(c *Config) { c.ACLsEnabled = true + c.PrimaryDatacenter = "dc1" c.ACLMasterToken = "root" }) defer os.RemoveAll(dir1) diff --git a/agent/consul/server.go b/agent/consul/server.go index 99957c6e1..4cab854e0 100644 --- a/agent/consul/server.go +++ b/agent/consul/server.go @@ -331,20 +331,6 @@ func NewServer(config *Config, flat Deps) (*Server, error) { return nil, err } - // Set the primary DC if it wasn't set. - // TODO: remove - if config.PrimaryDatacenter == "" { - if config.PrimaryDatacenter != "" { - config.PrimaryDatacenter = config.PrimaryDatacenter - } else { - config.PrimaryDatacenter = config.Datacenter - } - } - - if config.PrimaryDatacenter != "" { - config.PrimaryDatacenter = config.PrimaryDatacenter - } - // Create the tombstone GC. gc, err := state.NewTombstoneGC(config.TombstoneTTL, config.TombstoneTTLGranularity) if err != nil { diff --git a/agent/consul/server_test.go b/agent/consul/server_test.go index d3dfa6584..0dd19156c 100644 --- a/agent/consul/server_test.go +++ b/agent/consul/server_test.go @@ -125,6 +125,7 @@ func testServerConfig(t *testing.T) (string, *Config) { config.NodeName = uniqueNodeName(t.Name()) config.Bootstrap = true config.Datacenter = "dc1" + config.PrimaryDatacenter = "dc1" config.DataDir = dir // bind the rpc server to a random port. config.RPCAdvertise will be @@ -195,6 +196,7 @@ func testServerConfig(t *testing.T) (string, *Config) { func testServer(t *testing.T) (string, *Server) { return testServerWithConfig(t, func(c *Config) { c.Datacenter = "dc1" + c.PrimaryDatacenter = "dc1" c.Bootstrap = true }) } @@ -209,6 +211,7 @@ func testServerDC(t *testing.T, dc string) (string, *Server) { func testServerDCBootstrap(t *testing.T, dc string, bootstrap bool) (string, *Server) { return testServerWithConfig(t, func(c *Config) { c.Datacenter = dc + c.PrimaryDatacenter = dc c.Bootstrap = bootstrap }) }