Merge pull request #10715 from hashicorp/f-cns-attrs

consul: probe consul namespace feature before using namespace api

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)]
 * client: Fixed a bug where `alloc exec` sessions may terminate abruptly after a few minutes [[GH-10710](https://github.com/hashicorp/nomad/issues/10710)]

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 strconv.FormatBool(agentconsul.Namespaces(info)), true
 }