From 140747240f76327118581dd1b120fc1c026bf8a8 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Wed, 29 May 2024 16:31:09 -0400 Subject: [PATCH] consul: include admin partition in JWT login requests (#22226) When logging into a JWT auth method, we need to explicitly supply the Consul admin partition if the local Consul agent is in a partition. We can't derive this from agent configuration because the Consul agent's configuration is canonical, so instead we get the partition from the fingerprint (if available). This changeset updates the Consul client constructor so that we close over the partition from the fingerprint. Ref: https://hashicorp.atlassian.net/browse/NET-9451 --- .changelog/22226.txt | 3 + client/allocrunner/alloc_runner_hooks.go | 2 +- client/allocrunner/consul_hook.go | 4 +- client/consul/consul.go | 78 ++++++++++++++---------- 4 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 .changelog/22226.txt diff --git a/.changelog/22226.txt b/.changelog/22226.txt new file mode 100644 index 000000000..3a06afd68 --- /dev/null +++ b/.changelog/22226.txt @@ -0,0 +1,3 @@ +```release-note:bug +consul: Fixed a bug where Consul admin partition was not used to login via Consul JWT auth method +``` diff --git a/client/allocrunner/alloc_runner_hooks.go b/client/allocrunner/alloc_runner_hooks.go index 088b64123..ae57c4c79 100644 --- a/client/allocrunner/alloc_runner_hooks.go +++ b/client/allocrunner/alloc_runner_hooks.go @@ -130,7 +130,7 @@ func (ar *allocRunner) initRunnerHooks(config *clientconfig.Config) error { allocdir: ar.allocDir, widmgr: ar.widmgr, consulConfigs: ar.clientConfig.GetConsulConfigs(hookLogger), - consulClientConstructor: consul.NewConsulClient, + consulClientConstructor: consul.NewConsulClientFactory(config.Node), hookResources: ar.hookResources, envBuilder: newEnvBuilder, logger: hookLogger, diff --git a/client/allocrunner/consul_hook.go b/client/allocrunner/consul_hook.go index 9f4da708e..54a64c8f2 100644 --- a/client/allocrunner/consul_hook.go +++ b/client/allocrunner/consul_hook.go @@ -23,7 +23,7 @@ type consulHook struct { allocdir allocdir.Interface widmgr widmgr.IdentityManager consulConfigs map[string]*structsc.ConsulConfig - consulClientConstructor func(*structsc.ConsulConfig, log.Logger) (consul.Client, error) + consulClientConstructor consul.ConsulClientFunc hookResources *cstructs.AllocHookResources envBuilder *taskenv.Builder @@ -39,7 +39,7 @@ type consulHookConfig struct { consulConfigs map[string]*structsc.ConsulConfig // consulClientConstructor injects the function that will return a consul // client (eases testing) - consulClientConstructor func(*structsc.ConsulConfig, log.Logger) (consul.Client, error) + consulClientConstructor consul.ConsulClientFunc // hookResources is used for storing and retrieving Consul tokens hookResources *cstructs.AllocHookResources diff --git a/client/consul/consul.go b/client/consul/consul.go index 8ebb8b2d2..26096319c 100644 --- a/client/consul/consul.go +++ b/client/consul/consul.go @@ -63,43 +63,56 @@ type consulClient struct { // client is the API client to interact with consul client *consulapi.Client + // partition is the Consul partition for the local agent + partition string + // config is the configuration to connect to consul config *config.ConsulConfig logger hclog.Logger } -// NewConsulClient creates a new Consul client -func NewConsulClient(config *config.ConsulConfig, logger hclog.Logger) (Client, error) { - if config == nil { - return nil, fmt.Errorf("nil consul config") +// ConsulClientFunc creates a new Consul client for the specific Consul config +type ConsulClientFunc func(config *config.ConsulConfig, logger hclog.Logger) (Client, error) + +// NewConsulClientFactory returns a ConsulClientFunc that closes over the +// partition +func NewConsulClientFactory(node *structs.Node) ConsulClientFunc { + partition := node.Attributes["consul.partition"] + + return func(config *config.ConsulConfig, logger hclog.Logger) (Client, error) { + if config == nil { + return nil, fmt.Errorf("nil consul config") + } + + logger = logger.Named("consul").With("name", config.Name) + + c := &consulClient{ + config: config, + logger: logger, + partition: partition, + } + + // Get the Consul API configuration + apiConf, err := config.ApiConfig() + if err != nil { + logger.Error("error creating default Consul API config", "error", err) + return nil, err + } + + // Create the API client + client, err := consulapi.NewClient(apiConf) + if err != nil { + logger.Error("error creating Consul client", "error", err) + return nil, err + } + + useragent.SetHeaders(client) + c.client = client + + return c, nil + } - - logger = logger.Named("consul").With("name", config.Name) - - c := &consulClient{ - config: config, - logger: logger, - } - - // Get the Consul API configuration - apiConf, err := config.ApiConfig() - if err != nil { - logger.Error("error creating default Consul API config", "error", err) - return nil, err - } - - // Create the API client - client, err := consulapi.NewClient(apiConf) - if err != nil { - logger.Error("error creating Consul client", "error", err) - return nil, err - } - - useragent.SetHeaders(client) - c.client = client - - return c, nil } // DeriveTokenWithJWT takes a JWT from request and returns a consul token. @@ -108,7 +121,10 @@ func (c *consulClient) DeriveTokenWithJWT(req JWTLoginRequest) (*consulapi.ACLTo AuthMethod: req.AuthMethodName, BearerToken: req.JWT, Meta: req.Meta, - }, nil) + }, &consulapi.WriteOptions{ + Partition: c.partition, + }) + return t, err }