Skip to content

Commit

Permalink
consul: probe consul namespace feature before using namespace api
Browse files Browse the repository at this point in the history 
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 hashicorp/nomad-enterprise#575

Note that the ticket originally describes using attributes from #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.
  • Loading branch information
@shoenig
shoenig committed Jun 7, 2021
1 parent bdf2555 commit 0bc8a33
Show file tree
Hide file tree
Showing 19 changed files with 468 additions and 134 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/groupservice_hook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions client/allocrunner/taskrunner/connect_native_hook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions client/allocrunner/taskrunner/envoy_bootstrap_hook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 10 additions & 4 deletions client/allocrunner/taskrunner/task_runner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
74 changes: 18 additions & 56 deletions client/fingerprint/consul.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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)
}
Loading

0 comments on commit 0bc8a33

Please sign in to comment.