diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eef86ca8cb..42d66abedcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ BUG FIXES: * api: Fixed event stream connection initialization when there are no events to send [[GH-10637](https://github.com/hashicorp/nomad/issues/10637)] * cli: Fixed a bug where `quota status` and `namespace status` commands may panic if the CLI targets a pre-1.1.0 cluster [[GH-10620](https://github.com/hashicorp/nomad/pull/10620)] * cli: Fixed a bug where `alloc exec` may fail with "unexpected EOF" without returning the exit code after a command [[GH-10657](https://github.com/hashicorp/nomad/issues/10657)] +* consul: Fixed a bug where consul namespace API would be queried even when consul namespaces were not enabled [[GH-10715](https://github.com/hashicorp/nomad/pull/10715)] * csi: Fixed a bug where `mount_options` were not passed to CSI controller plugins for validation during volume creation and mounting. [[GH-10643](https://github.com/hashicorp/nomad/issues/10643)] * csi: Fixed a bug where `capability` blocks were not passed to CSI controller plugins for validation for `nomad volume register` commands. [[GH-10703](https://github.com/hashicorp/nomad/issues/10703)] * drivers/exec: Fixed a bug where `exec` and `java` tasks inherit the Nomad agent's `oom_score_adj` value [[GH-10698](https://github.com/hashicorp/nomad/issues/10698)] diff --git a/client/allocrunner/groupservice_hook_test.go b/client/allocrunner/groupservice_hook_test.go index 59d3b7a7a5c..5789079d850 100644 --- a/client/allocrunner/groupservice_hook_test.go +++ b/client/allocrunner/groupservice_hook_test.go @@ -238,7 +238,7 @@ func TestGroupServiceHook_Update08Alloc(t *testing.T) { consulConfig.Address = testconsul.HTTPAddr consulClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulClient.Namespaces(), consulClient.Agent()) serviceClient := agentconsul.NewServiceClient(consulClient.Agent(), namespacesClient, testlog.HCLogger(t), true) diff --git a/client/allocrunner/taskrunner/connect_native_hook_test.go b/client/allocrunner/taskrunner/connect_native_hook_test.go index 9ec54bc4609..a1c368a42ef 100644 --- a/client/allocrunner/taskrunner/connect_native_hook_test.go +++ b/client/allocrunner/taskrunner/connect_native_hook_test.go @@ -312,7 +312,7 @@ func TestTaskRunner_ConnectNativeHook_Ok(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) go consulClient.Run() @@ -377,7 +377,7 @@ func TestTaskRunner_ConnectNativeHook_with_SI_token(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) go consulClient.Run() @@ -454,7 +454,7 @@ func TestTaskRunner_ConnectNativeHook_shareTLS(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) go consulClient.Run() @@ -574,7 +574,7 @@ func TestTaskRunner_ConnectNativeHook_shareTLS_override(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) go consulClient.Run() diff --git a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go index 58e0ed988dd..98d20440650 100644 --- a/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go +++ b/client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go @@ -325,7 +325,7 @@ func TestEnvoyBootstrapHook_with_SI_token(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) go consulClient.Run() @@ -426,7 +426,7 @@ func TestTaskRunner_EnvoyBootstrapHook_sidecar_ok(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) go consulClient.Run() @@ -491,7 +491,7 @@ func TestTaskRunner_EnvoyBootstrapHook_gateway_ok(t *testing.T) { consulConfig.Address = testConsul.HTTPAddr consulAPIClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces()) + namespacesClient := agentconsul.NewNamespacesClient(consulAPIClient.Namespaces(), consulAPIClient.Agent()) // Register Group Services serviceClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), namespacesClient, logger, true) diff --git a/client/allocrunner/taskrunner/task_runner_test.go b/client/allocrunner/taskrunner/task_runner_test.go index 869042d58aa..c59f4765987 100644 --- a/client/allocrunner/taskrunner/task_runner_test.go +++ b/client/allocrunner/taskrunner/task_runner_test.go @@ -1155,9 +1155,12 @@ func TestTaskRunner_CheckWatcher_Restart(t *testing.T) { // Replace mock Consul ServiceClient, with the real ServiceClient // backed by a mock consul whose checks are always unhealthy. - consulAgent := agentconsul.NewMockAgent() + consulAgent := agentconsul.NewMockAgent(agentconsul.Features{ + Enterprise: false, + Namespaces: false, + }) consulAgent.SetStatus("critical") - namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil)) + namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil), consulAgent) consulClient := agentconsul.NewServiceClient(consulAgent, namespacesClient, conf.Logger, true) go consulClient.Run() defer consulClient.Shutdown() @@ -1835,8 +1838,11 @@ func TestTaskRunner_DriverNetwork(t *testing.T) { defer cleanup() // Use a mock agent to test for services - consulAgent := agentconsul.NewMockAgent() - namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil)) + consulAgent := agentconsul.NewMockAgent(agentconsul.Features{ + Enterprise: false, + Namespaces: false, + }) + namespacesClient := agentconsul.NewNamespacesClient(agentconsul.NewMockNamespaces(nil), consulAgent) consulClient := agentconsul.NewServiceClient(consulAgent, namespacesClient, conf.Logger, true) defer consulClient.Shutdown() go consulClient.Run() diff --git a/client/fingerprint/consul.go b/client/fingerprint/consul.go index e37ffb1a888..f562672332b 100644 --- a/client/fingerprint/consul.go +++ b/client/fingerprint/consul.go @@ -3,12 +3,11 @@ package fingerprint import ( "fmt" "strconv" - "strings" "time" - consul "github.com/hashicorp/consul/api" + consulapi "github.com/hashicorp/consul/api" log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/go-version" + agentconsul "github.com/hashicorp/nomad/command/agent/consul" ) const ( @@ -19,17 +18,14 @@ const ( // ConsulFingerprint is used to fingerprint for Consul type ConsulFingerprint struct { logger log.Logger - client *consul.Client + client *consulapi.Client lastState string extractors map[string]consulExtractor } -// consulInfo aliases the type returned from the Consul agent self endpoint. -type consulInfo = map[string]map[string]interface{} - // consulExtractor is used to parse out one attribute from consulInfo. Returns // the value of the attribute, and whether the attribute exists. -type consulExtractor func(consulInfo) (string, bool) +type consulExtractor func(agentconsul.Self) (string, bool) // NewConsulFingerprint is used to create a Consul fingerprint func NewConsulFingerprint(logger log.Logger) Fingerprint { @@ -95,7 +91,7 @@ func (f *ConsulFingerprint) initialize(req *FingerprintRequest) error { return fmt.Errorf("failed to initialize Consul client config: %v", err) } - f.client, err = consul.NewClient(consulConfig) + f.client, err = consulapi.NewClient(consulConfig) if err != nil { return fmt.Errorf("failed to initialize Consul client: %s", err) } @@ -117,7 +113,7 @@ func (f *ConsulFingerprint) initialize(req *FingerprintRequest) error { return nil } -func (f *ConsulFingerprint) query(resp *FingerprintResponse) consulInfo { +func (f *ConsulFingerprint) query(resp *FingerprintResponse) agentconsul.Self { // We'll try to detect consul by making a query to to the agent's self API. // If we can't hit this URL consul is probably not running on this machine. info, err := f.client.Agent().Self() @@ -144,48 +140,36 @@ func (f *ConsulFingerprint) link(resp *FingerprintResponse) { } } -func (f *ConsulFingerprint) server(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) server(info agentconsul.Self) (string, bool) { s, ok := info["Config"]["Server"].(bool) return strconv.FormatBool(s), ok } -func (f *ConsulFingerprint) version(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) version(info agentconsul.Self) (string, bool) { v, ok := info["Config"]["Version"].(string) return v, ok } -func (f *ConsulFingerprint) sku(info consulInfo) (string, bool) { - v, ok := info["Config"]["Version"].(string) - if !ok { - return "", ok - } - - ver, vErr := version.NewVersion(v) - if vErr != nil { - return "", false - } - if strings.Contains(ver.Metadata(), "ent") { - return "ent", true - } - return "oss", true +func (f *ConsulFingerprint) sku(info agentconsul.Self) (string, bool) { + return agentconsul.SKU(info) } -func (f *ConsulFingerprint) revision(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) revision(info agentconsul.Self) (string, bool) { r, ok := info["Config"]["Revision"].(string) return r, ok } -func (f *ConsulFingerprint) name(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) name(info agentconsul.Self) (string, bool) { n, ok := info["Config"]["NodeName"].(string) return n, ok } -func (f *ConsulFingerprint) dc(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) dc(info agentconsul.Self) (string, bool) { d, ok := info["Config"]["Datacenter"].(string) return d, ok } -func (f *ConsulFingerprint) segment(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) segment(info agentconsul.Self) (string, bool) { tags, tagsOK := info["Member"]["Tags"].(map[string]interface{}) if !tagsOK { return "", false @@ -194,38 +178,16 @@ func (f *ConsulFingerprint) segment(info consulInfo) (string, bool) { return s, ok } -func (f *ConsulFingerprint) connect(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) connect(info agentconsul.Self) (string, bool) { c, ok := info["DebugConfig"]["ConnectEnabled"].(bool) return strconv.FormatBool(c), ok } -func (f *ConsulFingerprint) grpc(info consulInfo) (string, bool) { +func (f *ConsulFingerprint) grpc(info agentconsul.Self) (string, bool) { p, ok := info["DebugConfig"]["GRPCPort"].(float64) return fmt.Sprintf("%d", int(p)), ok } -func (f *ConsulFingerprint) namespaces(info consulInfo) (string, bool) { - return f.feature("Namespaces", info) -} - -// possible values as of v1.9.5+ent: -// Automated Backups, Automated Upgrades, Enhanced Read Scalability, -// Network Segments, Redundancy Zone, Advanced Network Federation, -// Namespaces, SSO, Audit Logging -func (f *ConsulFingerprint) feature(name string, info consulInfo) (string, bool) { - lic, licOK := info["Stats"]["license"].(map[string]interface{}) - if !licOK { - return "", false - } - - features, exists := lic["features"].(string) - if !exists { - return "", false - } - - if !strings.Contains(features, name) { - return "", false - } - - return "true", true +func (f *ConsulFingerprint) namespaces(info agentconsul.Self) (string, bool) { + return strconv.FormatBool(agentconsul.Namespaces(info)), true } diff --git a/client/fingerprint/consul_test.go b/client/fingerprint/consul_test.go index e5eda50b292..fb7ff3ca8c0 100644 --- a/client/fingerprint/consul_test.go +++ b/client/fingerprint/consul_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/hashicorp/nomad/client/config" + agentconsul "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/nomad/structs" "github.com/stretchr/testify/require" @@ -51,7 +52,7 @@ func TestConsulFingerprint_server(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("is server", func(t *testing.T) { - s, ok := fp.server(consulInfo{ + s, ok := fp.server(agentconsul.Self{ "Config": {"Server": true}, }) require.True(t, ok) @@ -59,7 +60,7 @@ func TestConsulFingerprint_server(t *testing.T) { }) t.Run("is not server", func(t *testing.T) { - s, ok := fp.server(consulInfo{ + s, ok := fp.server(agentconsul.Self{ "Config": {"Server": false}, }) require.True(t, ok) @@ -67,14 +68,14 @@ func TestConsulFingerprint_server(t *testing.T) { }) t.Run("missing", func(t *testing.T) { - _, ok := fp.server(consulInfo{ + _, ok := fp.server(agentconsul.Self{ "Config": {}, }) require.False(t, ok) }) t.Run("malformed", func(t *testing.T) { - _, ok := fp.server(consulInfo{ + _, ok := fp.server(agentconsul.Self{ "Config": {"Server": 9000}, }) require.False(t, ok) @@ -87,7 +88,7 @@ func TestConsulFingerprint_version(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("oss", func(t *testing.T) { - v, ok := fp.version(consulInfo{ + v, ok := fp.version(agentconsul.Self{ "Config": {"Version": "v1.9.5"}, }) require.True(t, ok) @@ -95,7 +96,7 @@ func TestConsulFingerprint_version(t *testing.T) { }) t.Run("ent", func(t *testing.T) { - v, ok := fp.version(consulInfo{ + v, ok := fp.version(agentconsul.Self{ "Config": {"Version": "v1.9.5+ent"}, }) require.True(t, ok) @@ -103,14 +104,14 @@ func TestConsulFingerprint_version(t *testing.T) { }) t.Run("missing", func(t *testing.T) { - _, ok := fp.version(consulInfo{ + _, ok := fp.version(agentconsul.Self{ "Config": {}, }) require.False(t, ok) }) t.Run("malformed", func(t *testing.T) { - _, ok := fp.version(consulInfo{ + _, ok := fp.version(agentconsul.Self{ "Config": {"Version": 9000}, }) require.False(t, ok) @@ -123,7 +124,7 @@ func TestConsulFingerprint_sku(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("oss", func(t *testing.T) { - s, ok := fp.sku(consulInfo{ + s, ok := fp.sku(agentconsul.Self{ "Config": {"Version": "v1.9.5"}, }) require.True(t, ok) @@ -131,7 +132,7 @@ func TestConsulFingerprint_sku(t *testing.T) { }) t.Run("oss dev", func(t *testing.T) { - s, ok := fp.sku(consulInfo{ + s, ok := fp.sku(agentconsul.Self{ "Config": {"Version": "v1.9.5-dev"}, }) require.True(t, ok) @@ -139,7 +140,7 @@ func TestConsulFingerprint_sku(t *testing.T) { }) t.Run("ent", func(t *testing.T) { - s, ok := fp.sku(consulInfo{ + s, ok := fp.sku(agentconsul.Self{ "Config": {"Version": "v1.9.5+ent"}, }) require.True(t, ok) @@ -147,7 +148,7 @@ func TestConsulFingerprint_sku(t *testing.T) { }) t.Run("ent dev", func(t *testing.T) { - s, ok := fp.sku(consulInfo{ + s, ok := fp.sku(agentconsul.Self{ "Config": {"Version": "v1.9.5+ent-dev"}, }) require.True(t, ok) @@ -155,14 +156,14 @@ func TestConsulFingerprint_sku(t *testing.T) { }) t.Run("missing", func(t *testing.T) { - _, ok := fp.sku(consulInfo{ + _, ok := fp.sku(agentconsul.Self{ "Config": {}, }) require.False(t, ok) }) t.Run("malformed", func(t *testing.T) { - _, ok := fp.sku(consulInfo{ + _, ok := fp.sku(agentconsul.Self{ "Config": {"Version": "***"}, }) require.False(t, ok) @@ -175,7 +176,7 @@ func TestConsulFingerprint_revision(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("ok", func(t *testing.T) { - r, ok := fp.revision(consulInfo{ + r, ok := fp.revision(agentconsul.Self{ "Config": {"Revision": "3c1c22679"}, }) require.True(t, ok) @@ -183,14 +184,14 @@ func TestConsulFingerprint_revision(t *testing.T) { }) t.Run("malformed", func(t *testing.T) { - _, ok := fp.revision(consulInfo{ + _, ok := fp.revision(agentconsul.Self{ "Config": {"Revision": 9000}, }) require.False(t, ok) }) t.Run("missing", func(t *testing.T) { - _, ok := fp.revision(consulInfo{ + _, ok := fp.revision(agentconsul.Self{ "Config": {}, }) require.False(t, ok) @@ -203,7 +204,7 @@ func TestConsulFingerprint_dc(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("ok", func(t *testing.T) { - dc, ok := fp.dc(consulInfo{ + dc, ok := fp.dc(agentconsul.Self{ "Config": {"Datacenter": "dc1"}, }) require.True(t, ok) @@ -211,14 +212,14 @@ func TestConsulFingerprint_dc(t *testing.T) { }) t.Run("malformed", func(t *testing.T) { - _, ok := fp.dc(consulInfo{ + _, ok := fp.dc(agentconsul.Self{ "Config": {"Datacenter": 9000}, }) require.False(t, ok) }) t.Run("missing", func(t *testing.T) { - _, ok := fp.dc(consulInfo{ + _, ok := fp.dc(agentconsul.Self{ "Config": {}, }) require.False(t, ok) @@ -231,7 +232,7 @@ func TestConsulFingerprint_segment(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("ok", func(t *testing.T) { - s, ok := fp.segment(consulInfo{ + s, ok := fp.segment(agentconsul.Self{ "Member": {"Tags": map[string]interface{}{"segment": "seg1"}}, }) require.True(t, ok) @@ -239,21 +240,21 @@ func TestConsulFingerprint_segment(t *testing.T) { }) t.Run("segment missing", func(t *testing.T) { - _, ok := fp.segment(consulInfo{ + _, ok := fp.segment(agentconsul.Self{ "Member": {"Tags": map[string]interface{}{}}, }) require.False(t, ok) }) t.Run("tags missing", func(t *testing.T) { - _, ok := fp.segment(consulInfo{ + _, ok := fp.segment(agentconsul.Self{ "Member": {}, }) require.False(t, ok) }) t.Run("malformed", func(t *testing.T) { - _, ok := fp.segment(consulInfo{ + _, ok := fp.segment(agentconsul.Self{ "Member": {"Tags": map[string]interface{}{"segment": 9000}}, }) require.False(t, ok) @@ -266,7 +267,7 @@ func TestConsulFingerprint_connect(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("connect enabled", func(t *testing.T) { - s, ok := fp.connect(consulInfo{ + s, ok := fp.connect(agentconsul.Self{ "DebugConfig": {"ConnectEnabled": true}, }) require.True(t, ok) @@ -274,7 +275,7 @@ func TestConsulFingerprint_connect(t *testing.T) { }) t.Run("connect not enabled", func(t *testing.T) { - s, ok := fp.connect(consulInfo{ + s, ok := fp.connect(agentconsul.Self{ "DebugConfig": {"ConnectEnabled": false}, }) require.True(t, ok) @@ -282,7 +283,7 @@ func TestConsulFingerprint_connect(t *testing.T) { }) t.Run("connect missing", func(t *testing.T) { - _, ok := fp.connect(consulInfo{ + _, ok := fp.connect(agentconsul.Self{ "DebugConfig": {}, }) require.False(t, ok) @@ -295,7 +296,7 @@ func TestConsulFingerprint_grpc(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("grpc set", func(t *testing.T) { - s, ok := fp.grpc(consulInfo{ + s, ok := fp.grpc(agentconsul.Self{ "DebugConfig": {"GRPCPort": 8502.0}, // JSON numbers are floats }) require.True(t, ok) @@ -303,7 +304,7 @@ func TestConsulFingerprint_grpc(t *testing.T) { }) t.Run("grpc disabled", func(t *testing.T) { - s, ok := fp.grpc(consulInfo{ + s, ok := fp.grpc(agentconsul.Self{ "DebugConfig": {"GRPCPort": -1.0}, // JSON numbers are floats }) require.True(t, ok) @@ -311,7 +312,7 @@ func TestConsulFingerprint_grpc(t *testing.T) { }) t.Run("grpc missing", func(t *testing.T) { - _, ok := fp.grpc(consulInfo{ + _, ok := fp.grpc(agentconsul.Self{ "DebugConfig": {}, }) require.False(t, ok) @@ -325,33 +326,38 @@ func TestConsulFingerprint_namespaces(t *testing.T) { fp := newConsulFingerPrint(t) t.Run("supports namespaces", func(t *testing.T) { - s, ok := fp.namespaces(consulInfo{ + value, ok := fp.namespaces(agentconsul.Self{ "Stats": {"license": map[string]interface{}{"features": "Automated Backups, Automated Upgrades, Enhanced Read Scalability, Network Segments, Redundancy Zone, Advanced Network Federation, Namespaces, SSO, Audit Logging"}}, }) require.True(t, ok) - require.Equal(t, "true", s) + require.Equal(t, "true", value) }) t.Run("no namespaces", func(t *testing.T) { - _, ok := fp.namespaces(consulInfo{ + value, ok := fp.namespaces(agentconsul.Self{ "Stats": {"license": map[string]interface{}{"features": "Automated Backups, Automated Upgrades, Enhanced Read Scalability, Network Segments, Redundancy Zone, Advanced Network Federation, SSO, Audit Logging"}}, }) - require.False(t, ok) + require.True(t, ok) + require.Equal(t, "false", value) + }) t.Run("stats missing", func(t *testing.T) { - _, ok := fp.namespaces(consulInfo{}) - require.False(t, ok) + value, ok := fp.namespaces(agentconsul.Self{}) + require.True(t, ok) + require.Equal(t, "false", value) }) t.Run("license missing", func(t *testing.T) { - _, ok := fp.namespaces(consulInfo{"Stats": {}}) - require.False(t, ok) + value, ok := fp.namespaces(agentconsul.Self{"Stats": {}}) + require.True(t, ok) + require.Equal(t, "false", value) }) t.Run("features missing", func(t *testing.T) { - _, ok := fp.namespaces(consulInfo{"Stats": {"license": map[string]interface{}{}}}) - require.False(t, ok) + value, ok := fp.namespaces(agentconsul.Self{"Stats": {"license": map[string]interface{}{}}}) + require.True(t, ok) + require.Equal(t, "false", value) }) } @@ -371,15 +377,16 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) { err := cf.Fingerprint(&FingerprintRequest{Config: cfg, Node: node}, &resp) require.NoError(t, err) require.Equal(t, map[string]string{ - "consul.datacenter": "dc1", - "consul.revision": "3c1c22679", - "consul.segment": "seg1", - "consul.server": "true", - "consul.sku": "oss", - "consul.version": "1.9.5", - "consul.connect": "true", - "consul.grpc": "8502", - "unique.consul.name": "HAL9000", + "consul.datacenter": "dc1", + "consul.revision": "3c1c22679", + "consul.segment": "seg1", + "consul.server": "true", + "consul.sku": "oss", + "consul.version": "1.9.5", + "consul.connect": "true", + "consul.grpc": "8502", + "consul.ft.namespaces": "false", + "unique.consul.name": "HAL9000", }, resp.Attributes) require.True(t, resp.Detected) @@ -424,15 +431,16 @@ func TestConsulFingerprint_Fingerprint_oss(t *testing.T) { err3 := cf.Fingerprint(&FingerprintRequest{Config: cfg, Node: node}, &resp3) require.NoError(t, err3) require.Equal(t, map[string]string{ - "consul.datacenter": "dc1", - "consul.revision": "3c1c22679", - "consul.segment": "seg1", - "consul.server": "true", - "consul.sku": "oss", - "consul.version": "1.9.5", - "consul.connect": "true", - "consul.grpc": "8502", - "unique.consul.name": "HAL9000", + "consul.datacenter": "dc1", + "consul.revision": "3c1c22679", + "consul.segment": "seg1", + "consul.server": "true", + "consul.sku": "oss", + "consul.version": "1.9.5", + "consul.connect": "true", + "consul.grpc": "8502", + "consul.ft.namespaces": "false", + "unique.consul.name": "HAL9000", }, resp3.Attributes) // consul now available again diff --git a/command/agent/agent.go b/command/agent/agent.go index ff20dc99ab4..a500dea7f43 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -1197,7 +1197,7 @@ func (a *Agent) setupConsul(consulConfig *config.ConsulConfig) error { } // Create Consul Agent client for looking info about the agent. consulAgentClient := consulClient.Agent() - namespacesClient := consul.NewNamespacesClient(consulClient.Namespaces()) + namespacesClient := consul.NewNamespacesClient(consulClient.Namespaces(), consulAgentClient) a.consulService = consul.NewServiceClient(consulAgentClient, namespacesClient, a.logger, isClient) a.consulProxies = consul.NewConnectProxiesClient(consulAgentClient) diff --git a/command/agent/consul/catalog_testing.go b/command/agent/consul/catalog_testing.go index f34f0524414..69eb78f6e90 100644 --- a/command/agent/consul/catalog_testing.go +++ b/command/agent/consul/catalog_testing.go @@ -78,6 +78,13 @@ type MockAgent struct { // hits is the total number of times agent methods have been called hits int + // ent indicates whether the agent is mocking an enterprise consul + ent bool + + // namespaces indicates whether the agent is mocking consul with namespaces + // feature enabled + namespaces bool + // mu guards above fields mu sync.Mutex @@ -90,13 +97,21 @@ type MockAgent struct { var _ AgentAPI = (*MockAgent)(nil) +type Features struct { + Enterprise bool + Namespaces bool +} + // NewMockAgent that returns all checks as passing. -func NewMockAgent() *MockAgent { +func NewMockAgent(f Features) *MockAgent { return &MockAgent{ services: make(map[string]map[string]*api.AgentServiceRegistration), checks: make(map[string]map[string]*api.AgentCheckRegistration), checkTTLs: make(map[string]map[string]int), checkStatus: api.HealthPassing, + + ent: f.Enterprise, + namespaces: f.Namespaces, } } @@ -121,7 +136,34 @@ func (c *MockAgent) Self() (map[string]map[string]interface{}, error) { defer c.mu.Unlock() c.hits++ - s := map[string]map[string]interface{}{ + version := "1.9.5" + build := "1.9.5:22ce6c6a" + if c.ent { + version = "1.9.5+ent" + build = "1.9.5+ent:22ce6c6a" + } + + stats := make(map[string]interface{}) + if c.ent { + if c.namespaces { + stats = map[string]interface{}{ + "license": map[string]interface{}{ + "features": "Namespaces,", + }, + } + } + } + + return map[string]map[string]interface{}{ + "Config": { + "Datacenter": "dc1", + "NodeName": "x52", + "NodeID": "9e7bf42e-a0b4-61b7-24f9-66dead411f0f", + "Revision": "22ce6c6ad", + "Server": true, + "Version": version, + }, + "Stats": stats, "Member": { "Addr": "127.0.0.1", "DelegateCur": 4, @@ -134,7 +176,7 @@ func (c *MockAgent) Self() (map[string]map[string]interface{}, error) { "ProtocolMin": 1, "Status": 1, "Tags": map[string]interface{}{ - "build": "0.8.1:'e9ca44d", + "build": build, }, }, "xDS": { @@ -147,8 +189,7 @@ func (c *MockAgent) Self() (map[string]map[string]interface{}, error) { }, }, }, - } - return s, nil + }, nil } func getNamespace(q *api.QueryOptions) string { diff --git a/command/agent/consul/check_watcher_test.go b/command/agent/consul/check_watcher_test.go index 0267699c33c..c24c4d10d6c 100644 --- a/command/agent/consul/check_watcher_test.go +++ b/command/agent/consul/check_watcher_test.go @@ -151,7 +151,7 @@ func (c *fakeChecksAPI) ChecksWithFilterOpts(filter string, opts *api.QueryOptio func testWatcherSetup(t *testing.T) (*fakeChecksAPI, *checkWatcher) { logger := testlog.HCLogger(t) checksAPI := newFakeChecksAPI() - namespacesClient := NewNamespacesClient(NewMockNamespaces(nil)) + namespacesClient := NewNamespacesClient(NewMockNamespaces(nil), NewMockAgent(ossFeatures)) cw := newCheckWatcher(logger, checksAPI, namespacesClient) cw.pollFreq = 10 * time.Millisecond return checksAPI, cw @@ -180,7 +180,7 @@ func TestCheckWatcher_Skip(t *testing.T) { logger := testlog.HCLogger(t) checksAPI := newFakeChecksAPI() - namespacesClient := NewNamespacesClient(NewMockNamespaces(nil)) + namespacesClient := NewNamespacesClient(NewMockNamespaces(nil), NewMockAgent(ossFeatures)) cw := newCheckWatcher(logger, checksAPI, namespacesClient) restarter1 := newFakeCheckRestarter(cw, "testalloc1", "testtask1", "testcheck1", check) diff --git a/command/agent/consul/connect_proxies_test.go b/command/agent/consul/connect_proxies_test.go index 7ae8dac2f7c..235b319c115 100644 --- a/command/agent/consul/connect_proxies_test.go +++ b/command/agent/consul/connect_proxies_test.go @@ -7,8 +7,7 @@ import ( ) func TestConnectProxies_Proxies(t *testing.T) { - agentAPI := NewMockAgent() - pc := NewConnectProxiesClient(agentAPI) + pc := NewConnectProxiesClient(NewMockAgent(ossFeatures)) proxies, err := pc.Proxies() require.NoError(t, err) diff --git a/command/agent/consul/group_test.go b/command/agent/consul/group_test.go index 2dd506a2e10..b52b7f8c35a 100644 --- a/command/agent/consul/group_test.go +++ b/command/agent/consul/group_test.go @@ -32,7 +32,7 @@ func TestConsul_Connect(t *testing.T) { consulConfig.Address = testconsul.HTTPAddr consulClient, err := consulapi.NewClient(consulConfig) require.NoError(t, err) - namespacesClient := NewNamespacesClient(consulClient.Namespaces()) + namespacesClient := NewNamespacesClient(consulClient.Namespaces(), consulClient.Agent()) serviceClient := NewServiceClient(consulClient.Agent(), namespacesClient, testlog.HCLogger(t), true) // Lower periodicInterval to ensure periodic syncing doesn't improperly diff --git a/command/agent/consul/int_test.go b/command/agent/consul/int_test.go index 318a46f3483..f725afd638b 100644 --- a/command/agent/consul/int_test.go +++ b/command/agent/consul/int_test.go @@ -135,7 +135,7 @@ func TestConsul_Integration(t *testing.T) { consulClient, err := consulapi.NewClient(consulConfig) r.Nil(err) - namespacesClient := consul.NewNamespacesClient(consulClient.Namespaces()) + namespacesClient := consul.NewNamespacesClient(consulClient.Namespaces(), consulClient.Agent()) serviceClient := consul.NewServiceClient(consulClient.Agent(), namespacesClient, testlog.HCLogger(t), true) defer serviceClient.Shutdown() // just-in-case cleanup consulRan := make(chan struct{}) diff --git a/command/agent/consul/namespaces_client.go b/command/agent/consul/namespaces_client.go index 0bcbb10e2bb..974b2e705a3 100644 --- a/command/agent/consul/namespaces_client.go +++ b/command/agent/consul/namespaces_client.go @@ -2,34 +2,82 @@ package consul import ( "sort" - "strings" + "sync" + "time" +) + +const ( + // namespaceEnabledCacheTTL is how long to cache the response from Consul + // /v1/agent/self API, which is used to determine whether namespaces are + // available. + namespaceEnabledCacheTTL = 1 * time.Minute ) // NamespacesClient is a wrapper for the Consul NamespacesAPI, that is used to // deal with Consul OSS vs Consul Enterprise behavior in listing namespaces. type NamespacesClient struct { namespacesAPI NamespaceAPI + agentAPI AgentAPI + + lock sync.Mutex + enabled bool // namespaces requires Ent + Namespaces feature + updated time.Time // memoize response for a while } // NewNamespacesClient returns a NamespacesClient backed by a NamespaceAPI. -func NewNamespacesClient(namespacesAPI NamespaceAPI) *NamespacesClient { +func NewNamespacesClient(namespacesAPI NamespaceAPI, agentAPI AgentAPI) *NamespacesClient { return &NamespacesClient{ namespacesAPI: namespacesAPI, + agentAPI: agentAPI, } } +func stale(updated, now time.Time) bool { + return now.After(updated.Add(namespaceEnabledCacheTTL)) +} + +func (ns *NamespacesClient) allowable(now time.Time) bool { + ns.lock.Lock() + defer ns.lock.Unlock() + + if !stale(ns.updated, now) { + return ns.enabled + } + + self, err := ns.agentAPI.Self() + if err != nil { + return ns.enabled + } + + sku, ok := SKU(self) + if !ok { + return ns.enabled + } + + if sku != "ent" { + ns.enabled = false + ns.updated = now + return ns.enabled + } + + ns.enabled = Namespaces(self) + ns.updated = now + return ns.enabled +} + // List returns a list of Consul Namespaces. // -// If using Consul OSS, the list is a single element with the "default" namespace, -// even though the response from Consul OSS is an error. +// TODO(shoenig): return empty string instead of "default" when namespaces are not +// enabled. (Coming in followup PR). func (ns *NamespacesClient) List() ([]string, error) { + if !ns.allowable(time.Now()) { + // TODO(shoenig): lets return the empty string instead, that way we do not + // need to normalize at call sites later on + return []string{"default"}, nil + } + namespaces, _, err := ns.namespacesAPI.List(nil) if err != nil { - // check if the error was a 404, indicating Consul is the OSS version - // which does not have the /v1/namespace handler - if strings.Contains(err.Error(), "response code: 404") { - return []string{"default"}, nil - } return nil, err } diff --git a/command/agent/consul/namespaces_client_test.go b/command/agent/consul/namespaces_client_test.go new file mode 100644 index 00000000000..bc7ebdf37ad --- /dev/null +++ b/command/agent/consul/namespaces_client_test.go @@ -0,0 +1,117 @@ +package consul + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func TestNamespacesClient_List(t *testing.T) { + t.Parallel() + + t.Run("oss", func(t *testing.T) { + c := NewNamespacesClient(NewMockNamespaces(nil), NewMockAgent(Features{ + Enterprise: false, + Namespaces: false, + })) + list, err := c.List() + require.NoError(t, err) + require.Equal(t, []string{"default"}, list) // todo(shoenig): change in followup PR + }) + + t.Run("ent without namespaces", func(t *testing.T) { + c := NewNamespacesClient(NewMockNamespaces(nil), NewMockAgent(Features{ + Enterprise: true, + Namespaces: false, + })) + list, err := c.List() + require.NoError(t, err) + require.Equal(t, []string{"default"}, list) // todo(shoenig): change in followup PR + }) + + t.Run("ent with namespaces", func(t *testing.T) { + c := NewNamespacesClient(NewMockNamespaces([]string{"banana", "apple", "cherry"}), NewMockAgent(Features{ + Enterprise: true, + Namespaces: true, + })) + list, err := c.List() + require.NoError(t, err) + + // remember default always exists... if enterprise and namespaces are enabled + require.Equal(t, []string{"apple", "banana", "cherry", "default"}, list) + }) +} + +func TestNewNamespacesClient_stale(t *testing.T) { + t.Parallel() + + t.Run("ok", func(t *testing.T) { + now := time.Now() + updated := now.Add(-59 * time.Second) + result := stale(updated, now) + require.False(t, result) + }) + + t.Run("stale", func(t *testing.T) { + now := time.Now() + updated := now.Add(-61 * time.Second) + result := stale(updated, now) + require.True(t, result) + }) +} + +func TestNewNamespacesClient_allowable(t *testing.T) { + t.Parallel() + + try := func(ent, feature, enabled, exp bool, updated, now time.Time) { + expired := now.After(updated.Add(namespaceEnabledCacheTTL)) + name := fmt.Sprintf("ent:%t_feature:%t_enabled:%t_exp:%t_expired:%t", ent, feature, enabled, exp, expired) + t.Run(name, func(t *testing.T) { + c := NewNamespacesClient(NewMockNamespaces([]string{"a", "b"}), NewMockAgent(Features{ + Enterprise: ent, + Namespaces: feature, + })) + + // put the client into the state we want + c.enabled = enabled + c.updated = updated + + result := c.allowable(now) + require.Equal(t, exp, result) + require.Equal(t, exp, c.enabled) // cached value should match result + }) + } + + previous := time.Now() + over := previous.Add(namespaceEnabledCacheTTL + 1) + under := previous.Add(namespaceEnabledCacheTTL - 1) + + // oss, no refresh, no state change + try(false, false, false, false, previous, under) + + // oss, refresh, no state change + try(false, false, false, false, previous, over) + + // ent->oss, refresh, state change + try(false, false, true, false, previous, over) + + // ent, disabled, no refresh, no state change + try(true, false, false, false, previous, under) + + // ent, disabled, refresh, no state change + try(true, false, false, false, previous, over) + + // ent, enabled, no refresh, no state change + try(true, true, true, true, previous, under) + + // ent, enabled, refresh, no state change + try(true, true, true, true, previous, over) + + // ent, disabled, refresh, state change (i.e. new license with namespaces) + try(true, true, false, true, previous, over) // ??? + + // ent, disabled, refresh, no state change yet (i.e. new license with namespaces, still cached without) + try(true, true, false, false, previous, under) +} diff --git a/command/agent/consul/self.go b/command/agent/consul/self.go new file mode 100644 index 00000000000..d0edee2758c --- /dev/null +++ b/command/agent/consul/self.go @@ -0,0 +1,55 @@ +package consul + +import ( + "strings" + + "github.com/hashicorp/go-version" +) + +// Self represents the response body from Consul /v1/agent/self API endpoint. +// Care must always be taken to do type checks when casting, as structure could +// potentially change over time. +type Self = map[string]map[string]interface{} + +func SKU(info Self) (string, bool) { + v, ok := info["Config"]["Version"].(string) + if !ok { + return "", ok + } + + ver, vErr := version.NewVersion(v) + if vErr != nil { + return "", false + } + if strings.Contains(ver.Metadata(), "ent") { + return "ent", true + } + return "oss", true +} + +// Namespaces returns true if the "Namespaces" feature is enabled in Consul, and +// false otherwise. Consul OSS will always return false, and Consul ENT will return +// false if the license file does not contain the necessary feature. +func Namespaces(info Self) bool { + return feature("Namespaces", info) +} + +// feature returns whether the indicated feature is enabled by Consul and the +// associated License. +// possible values as of v1.9.5+ent: +// Automated Backups, Automated Upgrades, Enhanced Read Scalability, +// Network Segments, Redundancy Zone, Advanced Network Federation, +// Namespaces, SSO, Audit Logging +func feature(name string, info Self) bool { + lic, licOK := info["Stats"]["license"].(map[string]interface{}) + if !licOK { + return false + } + + features, exists := lic["features"].(string) + if !exists { + return false + } + + return strings.Contains(features, name) +} diff --git a/command/agent/consul/self_test.go b/command/agent/consul/self_test.go new file mode 100644 index 00000000000..3089c242204 --- /dev/null +++ b/command/agent/consul/self_test.go @@ -0,0 +1,97 @@ +package consul + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +var ( + ossFeatures = Features{ + Enterprise: false, + Namespaces: false, + } +) + +func TestSelf_SKU(t *testing.T) { + t.Parallel() + + t.Run("oss", func(t *testing.T) { + s, ok := SKU(Self{ + "Config": {"Version": "v1.9.5"}, + }) + require.True(t, ok) + require.Equal(t, "oss", s) + }) + + t.Run("oss dev", func(t *testing.T) { + s, ok := SKU(Self{ + "Config": {"Version": "v1.9.5-dev"}, + }) + require.True(t, ok) + require.Equal(t, "oss", s) + }) + + t.Run("ent", func(t *testing.T) { + s, ok := SKU(Self{ + "Config": {"Version": "v1.9.5+ent"}, + }) + require.True(t, ok) + require.Equal(t, "ent", s) + }) + + t.Run("ent dev", func(t *testing.T) { + s, ok := SKU(Self{ + "Config": {"Version": "v1.9.5+ent-dev"}, + }) + require.True(t, ok) + require.Equal(t, "ent", s) + }) + + t.Run("missing", func(t *testing.T) { + _, ok := SKU(Self{ + "Config": {}, + }) + require.False(t, ok) + }) + + t.Run("malformed", func(t *testing.T) { + _, ok := SKU(Self{ + "Config": {"Version": "***"}, + }) + require.False(t, ok) + }) +} + +func TestSelf_Namespaces(t *testing.T) { + t.Parallel() + + t.Run("supports namespaces", func(t *testing.T) { + enabled := Namespaces(Self{ + "Stats": {"license": map[string]interface{}{"features": "Automated Backups, Automated Upgrades, Enhanced Read Scalability, Network Segments, Redundancy Zone, Advanced Network Federation, Namespaces, SSO, Audit Logging"}}, + }) + require.True(t, enabled) + }) + + t.Run("no namespaces", func(t *testing.T) { + enabled := Namespaces(Self{ + "Stats": {"license": map[string]interface{}{"features": "Automated Backups, Automated Upgrades, Enhanced Read Scalability, Network Segments, Redundancy Zone, Advanced Network Federation, SSO, Audit Logging"}}, + }) + require.False(t, enabled) + }) + + t.Run("stats missing", func(t *testing.T) { + enabled := Namespaces(Self{}) + require.False(t, enabled) + }) + + t.Run("license missing", func(t *testing.T) { + enabled := Namespaces(Self{"Stats": {}}) + require.False(t, enabled) + }) + + t.Run("features missing", func(t *testing.T) { + enabled := Namespaces(Self{"Stats": {"license": map[string]interface{}{}}}) + require.False(t, enabled) + }) +} diff --git a/command/agent/consul/service_client_test.go b/command/agent/consul/service_client_test.go index 6bc91aaf382..9129882ee58 100644 --- a/command/agent/consul/service_client_test.go +++ b/command/agent/consul/service_client_test.go @@ -336,8 +336,8 @@ func TestSyncLogic_maybeTweakTags_emptySC(t *testing.T) { func TestServiceRegistration_CheckOnUpdate(t *testing.T) { t.Parallel() - mockAgent := NewMockAgent() - namespacesClient := NewNamespacesClient(NewMockNamespaces(nil)) + mockAgent := NewMockAgent(ossFeatures) + namespacesClient := NewNamespacesClient(NewMockNamespaces(nil), mockAgent) logger := testlog.HCLogger(t) sc := NewServiceClient(mockAgent, namespacesClient, logger, true) diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go index b9c385ae473..b3f035ad373 100644 --- a/command/agent/consul/unit_test.go +++ b/command/agent/consul/unit_test.go @@ -105,18 +105,18 @@ func (t *testFakeCtx) syncOnce(reason syncReason) error { // setupFake creates a testFakeCtx with a ServiceClient backed by a fakeConsul. // A test Workload is also provided. func setupFake(t *testing.T) *testFakeCtx { - fc := NewMockAgent() - nsc := NewNamespacesClient(NewMockNamespaces(nil)) - tw := testWorkload() + agentClient := NewMockAgent(ossFeatures) + nsClient := NewNamespacesClient(NewMockNamespaces(nil), agentClient) + workload := testWorkload() // by default start fake client being out of probation - sc := NewServiceClient(fc, nsc, testlog.HCLogger(t), true) - sc.deregisterProbationExpiry = time.Now().Add(-1 * time.Minute) + serviceClient := NewServiceClient(agentClient, nsClient, testlog.HCLogger(t), true) + serviceClient.deregisterProbationExpiry = time.Now().Add(-1 * time.Minute) return &testFakeCtx{ - ServiceClient: sc, - FakeConsul: fc, - Workload: tw, + ServiceClient: serviceClient, + FakeConsul: agentClient, + Workload: workload, } }