consul: probe consul namespace feature before using namespace api

This PR changes Nomad's wrapper around the Consul NamespaceAPI so that
it will detect if the Consul Namespaces feature is enabled before making
a request to the Namespaces API. Namespaces are not enabled in Consul OSS,
and require a suitable license to be used with Consul ENT.

Previously Nomad would check for a 404 status code when makeing a request
to the Namespaces API to "detect" if Consul OSS was being used. This does
not work for Consul ENT with Namespaces disabled, which returns a 500.

Now we avoid requesting the namespace API altogether if Consul is detected
to be the OSS sku, or if the Namespaces feature is not licensed. Since
Consul can be upgraded from OSS to ENT, or a new license applied, we cache
the value for 1 minute, refreshing on demand if expired.

Fixes https://github.com/hashicorp/nomad-enterprise/issues/575

Note that the ticket originally describes using attributes from https://github.com/hashicorp/nomad/issues/10688.
This turns out not to be possible due to a chicken-egg situation between
bootstrapping the agent and setting up the consul client. Also fun: the
Consul fingerprinter creates its own Consul client, because there is no
[currently] no way to pass the agent's client through the fingerprint factory.

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)]

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)
 

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()

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)

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()

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 agentconsul.Namespaces(info)
 }

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,7 +326,7 @@ func TestConsulFingerprint_namespaces(t *testing.T) {
 	fp := newConsulFingerPrint(t)
 
 	t.Run("supports namespaces", func(t *testing.T) {
-		s, ok := fp.namespaces(consulInfo{
+		s, 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)
@@ -333,24 +334,24 @@ func TestConsulFingerprint_namespaces(t *testing.T) {
 	})
 
 	t.Run("no namespaces", func(t *testing.T) {
-		_, ok := fp.namespaces(consulInfo{
+		_, 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)
 	})
 
 	t.Run("stats missing", func(t *testing.T) {
-		_, ok := fp.namespaces(consulInfo{})
+		_, ok := fp.namespaces(agentconsul.Self{})
 		require.False(t, ok)
 	})
 
 	t.Run("license missing", func(t *testing.T) {
-		_, ok := fp.namespaces(consulInfo{"Stats": {}})
+		_, ok := fp.namespaces(agentconsul.Self{"Stats": {}})
 		require.False(t, ok)
 	})
 
 	t.Run("features missing", func(t *testing.T) {
-		_, ok := fp.namespaces(consulInfo{"Stats": {"license": map[string]interface{}{}}})
+		_, ok := fp.namespaces(agentconsul.Self{"Stats": {"license": map[string]interface{}{}}})
 		require.False(t, ok)
 	})
 }

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)
 

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 {

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)

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)

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

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{})

command/agent/consul/namespaces_client.go

@@ -2,34 +2,87 @@ 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
+	}
+
+	enabledStr, ok := Namespaces(self)
+	if !ok {
+		return ns.enabled
+	}
+
+	ns.enabled = enabledStr == "true"
+	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
 	}
 

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)
+}

command/agent/consul/self.go

@@ -0,0 +1,56 @@
+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
+}
+
+func Namespaces(info Self) (string, 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) (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
+}

command/agent/consul/self_test.go

@@ -0,0 +1,98 @@
+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) {
+		s, ok := 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, ok)
+		require.Equal(t, "true", s)
+	})
+
+	t.Run("no namespaces", func(t *testing.T) {
+		_, ok := 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, ok)
+	})
+
+	t.Run("stats missing", func(t *testing.T) {
+		_, ok := Namespaces(Self{})
+		require.False(t, ok)
+	})
+
+	t.Run("license missing", func(t *testing.T) {
+		_, ok := Namespaces(Self{"Stats": {}})
+		require.False(t, ok)
+	})
+
+	t.Run("features missing", func(t *testing.T) {
+		_, ok := Namespaces(Self{"Stats": {"license": map[string]interface{}{}}})
+		require.False(t, ok)
+	})
+}

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)
 

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,
 	}
 }