From 5001bf45476a8b18f2ba25950b9cdc00864fde7e Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Thu, 28 Sep 2023 16:50:21 -0400
Subject: [PATCH] consul: use constant instead of "default" literal (#18611)

Use the constant `structs.ConsulDefaultCluster` instead of the string literal
"default", as we've done for Vault.
---
 client/config/config.go                   |  2 +-
 client/config/config_ce.go                |  2 +-
 client/fingerprint/consul.go              |  3 +-
 client/fingerprint/consul_test.go         | 20 ++++++-------
 command/agent/command.go                  |  2 +-
 command/agent/config.go                   |  4 +--
 command/agent/config_parse.go             |  4 +--
 command/agent/config_parse_test.go        | 36 +++++++++++------------
 command/agent/config_test.go              |  4 +--
 command/agent/job_endpoint_test.go        |  8 ++---
 nomad/config.go                           |  2 +-
 nomad/job_endpoint_hook_consul_ce_test.go |  2 +-
 nomad/job_endpoint_hooks.go               |  2 +-
 nomad/structs/consul.go                   |  6 ++++
 14 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/client/config/config.go b/client/config/config.go
index 9ed6d8815..76faa4c7b 100644
--- a/client/config/config.go
+++ b/client/config/config.go
@@ -814,7 +814,7 @@ func DefaultConfig() *Config {
 	}
 
 	cfg.ConsulConfigs = map[string]*structsc.ConsulConfig{
-		"default": cfg.ConsulConfig}
+		structs.ConsulDefaultCluster: cfg.ConsulConfig}
 	cfg.VaultConfigs = map[string]*structsc.VaultConfig{
 		structs.VaultDefaultCluster: cfg.VaultConfig}
 
diff --git a/client/config/config_ce.go b/client/config/config_ce.go
index 2df577051..3fd4593c4 100644
--- a/client/config/config_ce.go
+++ b/client/config/config_ce.go
@@ -37,5 +37,5 @@ func (c *Config) GetConsulConfigs(logger hclog.Logger) map[string]*structsc.Cons
 		logger.Warn("multiple Consul configurations are only supported in Nomad Enterprise")
 	}
 
-	return map[string]*config.ConsulConfig{"default": c.ConsulConfig}
+	return map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: c.ConsulConfig}
 }
diff --git a/client/fingerprint/consul.go b/client/fingerprint/consul.go
index 2051df6dd..634ede1e6 100644
--- a/client/fingerprint/consul.go
+++ b/client/fingerprint/consul.go
@@ -16,6 +16,7 @@ import (
 	"github.com/hashicorp/go-version"
 	agentconsul "github.com/hashicorp/nomad/command/agent/consul"
 	"github.com/hashicorp/nomad/helper"
+	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/nomad/structs/config"
 )
 
@@ -151,7 +152,7 @@ func (cfs *consulFingerprintState) initialize(cfg *config.ConsulConfig, logger h
 		return fmt.Errorf("failed to initialize Consul client: %v", err)
 	}
 
-	if cfg.Name == "default" {
+	if cfg.Name == structs.ConsulDefaultCluster {
 		cfs.extractors = map[string]consulExtractor{
 			"consul.server":        cfs.server,
 			"consul.version":       cfs.version,
diff --git a/client/fingerprint/consul_test.go b/client/fingerprint/consul_test.go
index 654e862c0..76bd197b9 100644
--- a/client/fingerprint/consul_test.go
+++ b/client/fingerprint/consul_test.go
@@ -456,7 +456,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) {
 	node := &structs.Node{Attributes: make(map[string]string)}
 
 	// consul not available before first run
-	must.Nil(t, cf.states["default"])
+	must.Nil(t, cf.states[structs.ConsulDefaultCluster])
 
 	// execute first query with good response
 	var resp FingerprintResponse
@@ -477,7 +477,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) {
 	must.True(t, resp.Detected)
 
 	// consul now available
-	must.True(t, cf.states["default"].isAvailable)
+	must.True(t, cf.states[structs.ConsulDefaultCluster].isAvailable)
 
 	var resp2 FingerprintResponse
 
@@ -494,7 +494,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) {
 
 	// Reset the nextCheck time for testing purposes, or we won't pick up the
 	// change until the next period, up to 2min from now
-	cf.states["default"].nextCheck = time.Now()
+	cf.states[structs.ConsulDefaultCluster].nextCheck = time.Now()
 
 	// execute second query with error
 	err2 := cf.Fingerprint(&FingerprintRequest{Config: cfg, Node: node}, &resp2)
@@ -503,7 +503,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) {
 	must.True(t, resp.Detected)   // never downgrade
 
 	// consul no longer available
-	must.False(t, cf.states["default"].isAvailable)
+	must.False(t, cf.states[structs.ConsulDefaultCluster].isAvailable)
 
 	// execute third query no error
 	var resp3 FingerprintResponse
@@ -523,7 +523,7 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) {
 	}, resp3.Attributes)
 
 	// consul now available again
-	must.True(t, cf.states["default"].isAvailable)
+	must.True(t, cf.states[structs.ConsulDefaultCluster].isAvailable)
 	must.True(t, resp.Detected)
 }
 
@@ -538,7 +538,7 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) {
 	node := &structs.Node{Attributes: make(map[string]string)}
 
 	// consul not available before first run
-	must.Nil(t, cf.states["default"])
+	must.Nil(t, cf.states[structs.ConsulDefaultCluster])
 
 	// execute first query with good response
 	var resp FingerprintResponse
@@ -559,7 +559,7 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) {
 	must.True(t, resp.Detected)
 
 	// consul now available
-	must.True(t, cf.states["default"].isAvailable)
+	must.True(t, cf.states[structs.ConsulDefaultCluster].isAvailable)
 
 	var resp2 FingerprintResponse
 
@@ -577,7 +577,7 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) {
 
 	// Reset the nextCheck time for testing purposes, or we won't pick up the
 	// change until the next period, up to 2min from now
-	cf.states["default"].nextCheck = time.Now()
+	cf.states[structs.ConsulDefaultCluster].nextCheck = time.Now()
 
 	// execute second query with error
 	err2 := cf.Fingerprint(&FingerprintRequest{Config: cfg, Node: node}, &resp2)
@@ -586,7 +586,7 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) {
 	must.True(t, resp.Detected)   // never downgrade
 
 	// consul no longer available
-	must.False(t, cf.states["default"].isAvailable)
+	must.False(t, cf.states[structs.ConsulDefaultCluster].isAvailable)
 
 	// execute third query no error
 	var resp3 FingerprintResponse
@@ -606,6 +606,6 @@ func TestConsulFingerprint_Fingerprint_ent(t *testing.T) {
 	}, resp3.Attributes)
 
 	// consul now available again
-	must.True(t, cf.states["default"].isAvailable)
+	must.True(t, cf.states[structs.ConsulDefaultCluster].isAvailable)
 	must.True(t, resp.Detected)
 }
diff --git a/command/agent/command.go b/command/agent/command.go
index 59055b7ec..f18c6f9a0 100644
--- a/command/agent/command.go
+++ b/command/agent/command.go
@@ -79,7 +79,7 @@ func (c *Command) readConfig() *Config {
 		Reporting: &config.ReportingConfig{},
 	}
 	cmdConfig.Vaults = map[string]*config.VaultConfig{structs.VaultDefaultCluster: cmdConfig.Vault}
-	cmdConfig.Consuls = map[string]*config.ConsulConfig{"default": cmdConfig.Consul}
+	cmdConfig.Consuls = map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: cmdConfig.Consul}
 
 	flags := flag.NewFlagSet("agent", flag.ContinueOnError)
 	flags.Usage = func() { c.Ui.Error(c.Help()) }
diff --git a/command/agent/config.go b/command/agent/config.go
index 150d762c2..ed2ea0e35 100644
--- a/command/agent/config.go
+++ b/command/agent/config.go
@@ -1376,7 +1376,7 @@ func DefaultConfig() *Config {
 		Reporting:          config.DefaultReporting(),
 	}
 
-	cfg.Consuls = map[string]*config.ConsulConfig{"default": cfg.Consul}
+	cfg.Consuls = map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: cfg.Consul}
 	cfg.Vaults = map[string]*config.VaultConfig{structs.VaultDefaultCluster: cfg.Vault}
 	return cfg
 }
@@ -1548,7 +1548,7 @@ func (c *Config) Merge(b *Config) *Config {
 
 	// Apply the Consul Configurations and overwrite the default Consul config
 	result.Consuls = mergeConsulConfigs(result.Consuls, b.Consuls)
-	result.Consul = result.Consuls["default"]
+	result.Consul = result.Consuls[structs.ConsulDefaultCluster]
 
 	// Apply the Vault Configurations and overwrite the default Vault config
 	result.Vaults = mergeVaultConfigs(result.Vaults, b.Vaults)
diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go
index 9848ac8ee..0e6529838 100644
--- a/command/agent/config_parse.go
+++ b/command/agent/config_parse.go
@@ -462,7 +462,7 @@ func parseConsuls(c *Config, list *ast.ObjectList) error {
 			return err
 		}
 		if cc.Name == "" {
-			cc.Name = "default"
+			cc.Name = structs.ConsulDefaultCluster
 		}
 		if cc.TimeoutHCL != "" {
 			d, err := time.ParseDuration(cc.TimeoutHCL)
@@ -515,6 +515,6 @@ func parseConsuls(c *Config, list *ast.ObjectList) error {
 		}
 	}
 
-	c.Consul = c.Consuls["default"]
+	c.Consul = c.Consuls[structs.ConsulDefaultCluster]
 	return nil
 }
diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go
index 313ea1a17..d8c0e98c0 100644
--- a/command/agent/config_parse_test.go
+++ b/command/agent/config_parse_test.go
@@ -212,7 +212,7 @@ var basicConfig = &Config{
 	DisableUpdateCheck:        pointer.Of(true),
 	DisableAnonymousSignature: true,
 	Consul: &config.ConsulConfig{
-		Name:                 "default",
+		Name:                 structs.ConsulDefaultCluster,
 		ServerServiceName:    "nomad",
 		ServerHTTPCheckName:  "nomad-server-http-health-check",
 		ServerSerfCheckName:  "nomad-server-serf-health-check",
@@ -251,8 +251,8 @@ var basicConfig = &Config{
 		},
 	},
 	Consuls: map[string]*config.ConsulConfig{
-		"default": {
-			Name:                 "default",
+		structs.ConsulDefaultCluster: {
+			Name:                 structs.ConsulDefaultCluster,
 			ServerServiceName:    "nomad",
 			ServerHTTPCheckName:  "nomad-server-http-health-check",
 			ServerSerfCheckName:  "nomad-server-serf-health-check",
@@ -608,7 +608,7 @@ func TestConfig_Parse(t *testing.T) {
 			// defaults, which include additional settings
 			oldDefault := &Config{
 				Consul:    config.DefaultConsulConfig(),
-				Consuls:   map[string]*config.ConsulConfig{"default": config.DefaultConsulConfig()},
+				Consuls:   map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: config.DefaultConsulConfig()},
 				Vault:     config.DefaultVaultConfig(),
 				Vaults:    map[string]*config.VaultConfig{structs.VaultDefaultCluster: config.DefaultVaultConfig()},
 				Autopilot: config.DefaultAutopilotConfig(),
@@ -646,7 +646,7 @@ func (c *Config) addDefaults() {
 	}
 	if c.Consul == nil {
 		c.Consul = config.DefaultConsulConfig()
-		c.Consuls = map[string]*config.ConsulConfig{"default": c.Consul}
+		c.Consuls = map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: c.Consul}
 	}
 	if c.Autopilot == nil {
 		c.Autopilot = config.DefaultAutopilotConfig()
@@ -798,14 +798,14 @@ var sample0 = &Config{
 	EnableSyslog:   true,
 	SyslogFacility: "LOCAL0",
 	Consul: &config.ConsulConfig{
-		Name:           "default",
+		Name:           structs.ConsulDefaultCluster,
 		Token:          "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
 		ServerAutoJoin: pointer.Of(false),
 		ClientAutoJoin: pointer.Of(false),
 	},
 	Consuls: map[string]*config.ConsulConfig{
-		"default": {
-			Name:           "default",
+		structs.ConsulDefaultCluster: {
+			Name:           structs.ConsulDefaultCluster,
 			Token:          "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
 			ServerAutoJoin: pointer.Of(false),
 			ClientAutoJoin: pointer.Of(false),
@@ -912,15 +912,15 @@ var sample1 = &Config{
 	EnableSyslog:   true,
 	SyslogFacility: "LOCAL0",
 	Consul: &config.ConsulConfig{
-		Name:           "default",
+		Name:           structs.ConsulDefaultCluster,
 		EnableSSL:      pointer.Of(true),
 		Token:          "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
 		ServerAutoJoin: pointer.Of(false),
 		ClientAutoJoin: pointer.Of(false),
 	},
 	Consuls: map[string]*config.ConsulConfig{
-		"default": {
-			Name:           "default",
+		structs.ConsulDefaultCluster: {
+			Name:           structs.ConsulDefaultCluster,
 			EnableSSL:      pointer.Of(true),
 			Token:          "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
 			ServerAutoJoin: pointer.Of(false),
@@ -1112,28 +1112,28 @@ func TestConfig_MultipleConsul(t *testing.T) {
 
 	// verify the default Consul config is set from the list
 	cfg := DefaultConfig()
-	must.Eq(t, "default", cfg.Consul.Name)
+	must.Eq(t, structs.ConsulDefaultCluster, cfg.Consul.Name)
 	must.Eq(t, config.DefaultConsulConfig(), cfg.Consul)
 	must.True(t, *cfg.Consul.AllowUnauthenticated)
 	must.Eq(t, "127.0.0.1:8500", cfg.Consul.Addr)
 	must.Eq(t, "", cfg.Consul.Token)
 
 	must.MapLen(t, 1, cfg.Consuls)
-	must.Eq(t, cfg.Consul, cfg.Consuls["default"])
-	must.True(t, cfg.Consul == cfg.Consuls["default"]) // must be same pointer
+	must.Eq(t, cfg.Consul, cfg.Consuls[structs.ConsulDefaultCluster])
+	must.True(t, cfg.Consul == cfg.Consuls[structs.ConsulDefaultCluster]) // must be same pointer
 
 	// merge in the user's configuration
 	fc, err := LoadConfig("testdata/basic.hcl")
 	must.NoError(t, err)
 	cfg = cfg.Merge(fc)
 
-	must.Eq(t, "default", cfg.Consul.Name)
+	must.Eq(t, structs.ConsulDefaultCluster, cfg.Consul.Name)
 	must.True(t, *cfg.Consul.AllowUnauthenticated)
 	must.Eq(t, "127.0.0.1:9500", cfg.Consul.Addr)
 	must.Eq(t, "token1", cfg.Consul.Token)
 
 	must.MapLen(t, 1, cfg.Consuls)
-	must.Eq(t, cfg.Consul, cfg.Consuls["default"])
+	must.Eq(t, cfg.Consul, cfg.Consuls[structs.ConsulDefaultCluster])
 
 	// add an extra Consul config and override fields in the default
 	fc, err = LoadConfig("testdata/extra-consul.hcl")
@@ -1141,13 +1141,13 @@ func TestConfig_MultipleConsul(t *testing.T) {
 
 	cfg = cfg.Merge(fc)
 
-	must.Eq(t, "default", cfg.Consul.Name)
+	must.Eq(t, structs.ConsulDefaultCluster, cfg.Consul.Name)
 	must.False(t, *cfg.Consul.AllowUnauthenticated)
 	must.Eq(t, "127.0.0.1:9501", cfg.Consul.Addr)
 	must.Eq(t, "abracadabra", cfg.Consul.Token)
 
 	must.MapLen(t, 3, cfg.Consuls)
-	must.Eq(t, cfg.Consul, cfg.Consuls["default"])
+	must.Eq(t, cfg.Consul, cfg.Consuls[structs.ConsulDefaultCluster])
 
 	must.Eq(t, "alternate", cfg.Consuls["alternate"].Name)
 	must.True(t, *cfg.Consuls["alternate"].AllowUnauthenticated)
diff --git a/command/agent/config_test.go b/command/agent/config_test.go
index e2e4340e1..1a6703bdd 100644
--- a/command/agent/config_test.go
+++ b/command/agent/config_test.go
@@ -233,7 +233,7 @@ func TestConfig_Merge(t *testing.T) {
 			ChecksUseAdvertise:   &falseValue,
 		},
 		Consuls: map[string]*config.ConsulConfig{
-			"default": {
+			structs.ConsulDefaultCluster: {
 				ServerServiceName:    "1",
 				ClientServiceName:    "1",
 				AutoAdvertise:        &falseValue,
@@ -481,7 +481,7 @@ func TestConfig_Merge(t *testing.T) {
 			ChecksUseAdvertise:   &trueValue,
 		},
 		Consuls: map[string]*config.ConsulConfig{
-			"default": {
+			structs.ConsulDefaultCluster: {
 				ServerServiceName:    "2",
 				ClientServiceName:    "2",
 				AutoAdvertise:        &trueValue,
diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go
index 535932e79..364e977a0 100644
--- a/command/agent/job_endpoint_test.go
+++ b/command/agent/job_endpoint_test.go
@@ -2985,13 +2985,13 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 				},
 				Consul: &structs.Consul{
 					Namespace: "team-foo",
-					Cluster:   "default",
+					Cluster:   structs.ConsulDefaultCluster,
 				},
 				Services: []*structs.Service{
 					{
 						Name:              "groupserviceA",
 						Provider:          "consul",
-						Cluster:           "default",
+						Cluster:           structs.ConsulDefaultCluster,
 						Tags:              []string{"a", "b"},
 						CanaryTags:        []string{"d", "e"},
 						EnableTagOverride: true,
@@ -3093,7 +3093,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 							{
 								Name:              "serviceA",
 								Provider:          "consul",
-								Cluster:           "default",
+								Cluster:           structs.ConsulDefaultCluster,
 								Tags:              []string{"1", "2"},
 								CanaryTags:        []string{"3", "4"},
 								EnableTagOverride: true,
@@ -3433,7 +3433,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) {
 				},
 				Consul: &structs.Consul{
 					Namespace: "foo",
-					Cluster:   "default",
+					Cluster:   structs.ConsulDefaultCluster,
 				},
 				Tasks: []*structs.Task{
 					{
diff --git a/nomad/config.go b/nomad/config.go
index 8654a4918..e5f766407 100644
--- a/nomad/config.go
+++ b/nomad/config.go
@@ -617,7 +617,7 @@ func DefaultConfig() *Config {
 		JobTrackedVersions:       structs.JobDefaultTrackedVersions,
 	}
 
-	c.ConsulConfigs = map[string]*config.ConsulConfig{"default": c.ConsulConfig}
+	c.ConsulConfigs = map[string]*config.ConsulConfig{structs.ConsulDefaultCluster: c.ConsulConfig}
 	c.VaultConfigs = map[string]*config.VaultConfig{structs.VaultDefaultCluster: c.VaultConfig}
 
 	// Enable all known schedulers by default
diff --git a/nomad/job_endpoint_hook_consul_ce_test.go b/nomad/job_endpoint_hook_consul_ce_test.go
index 2c21d32b2..af7fef9f8 100644
--- a/nomad/job_endpoint_hook_consul_ce_test.go
+++ b/nomad/job_endpoint_hook_consul_ce_test.go
@@ -44,7 +44,7 @@ func TestJobEndpointHook_ConsulCE(t *testing.T) {
 	_, _, err := hook.Mutate(job)
 
 	must.NoError(t, err)
-	test.Eq(t, "default", job.TaskGroups[0].Services[0].Cluster)
+	test.Eq(t, structs.ConsulDefaultCluster, job.TaskGroups[0].Services[0].Cluster)
 	test.Eq(t, "infra", job.TaskGroups[0].Services[1].Cluster)
 	test.Eq(t, "nondefault", job.TaskGroups[0].Tasks[0].Services[0].Cluster)
 
diff --git a/nomad/job_endpoint_hooks.go b/nomad/job_endpoint_hooks.go
index 76ec21f20..5624dfa49 100644
--- a/nomad/job_endpoint_hooks.go
+++ b/nomad/job_endpoint_hooks.go
@@ -254,7 +254,7 @@ func vaultConstraintFn(vault *structs.Vault) *structs.Constraint {
 // well before we get here, so no need to split out the behavior to ENT-specific
 // code.
 func consulConstraintFn(service *structs.Service) *structs.Constraint {
-	if service.Cluster != "default" && service.Cluster != "" {
+	if service.Cluster != structs.ConsulDefaultCluster && service.Cluster != "" {
 		return &structs.Constraint{
 			LTarget: fmt.Sprintf("${attr.consul.%s.version}", service.Cluster),
 			RTarget: ">= 1.7.0",
diff --git a/nomad/structs/consul.go b/nomad/structs/consul.go
index 2fd793e10..039993de9 100644
--- a/nomad/structs/consul.go
+++ b/nomad/structs/consul.go
@@ -8,6 +8,12 @@ import (
 	"regexp"
 )
 
+const (
+	// ConsulDefaultCluster is the name used for the Consul cluster that doesn't
+	// have a name.
+	ConsulDefaultCluster = "default"
+)
+
 // Consul represents optional per-group consul configuration.
 type Consul struct {
 	// Namespace in which to operate in Consul.