From fb0bd3c25fca710394ebc32f5fce246c4faf3383 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Thu, 2 Apr 2020 10:30:38 -0600 Subject: [PATCH] connect: set consul TLS options on envoy bootstrap Fixes #6594 #6711 #6714 #7567 e2e testing is still TBD in #6502 Before, we only passed the Nomad agent's configured Consul HTTP address onto the `consul connect envoy ...` bootstrap command. This meant any Consul setup with TLS enabled would not work with Nomad's Connect integration. This change now sets CLI args and Environment Variables for configuring TLS options for communicating with Consul when doing the envoy bootstrap, as described in https://www.consul.io/docs/commands/connect/envoy.html#usage --- .../taskrunner/envoybootstrap_hook.go | 107 ++++++++++++-- .../taskrunner/envoybootstrap_hook_test.go | 133 +++++++++++++----- .../taskrunner/task_runner_hooks.go | 8 +- 3 files changed, 196 insertions(+), 52 deletions(-) diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook.go b/client/allocrunner/taskrunner/envoybootstrap_hook.go index 09efccb798c..66d6508d0da 100644 --- a/client/allocrunner/taskrunner/envoybootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoybootstrap_hook.go @@ -16,15 +16,54 @@ import ( agentconsul "github.com/hashicorp/nomad/command/agent/consul" "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/nomad/structs/config" "github.com/pkg/errors" ) const envoyBootstrapHookName = "envoy_bootstrap" +type envoyBootstrapConsulConfig struct { + HTTPAddr string // required + Auth string // optional, env CONSUL_HTTP_AUTH + SSL string // optional, env CONSUL_HTTP_SSL + VerifySSL string // optional, env CONSUL_HTTP_SSL_VERIFY + CAFile string // optional, arg -ca-file + CertFile string // optional, arg -client-cert + KeyFile string // optional, arg -client-key + // CAPath (dir) not supported by Nomad's config object +} + type envoyBootstrapHookConfig struct { - alloc *structs.Allocation - consulHTTPAddr string - logger hclog.Logger + consul envoyBootstrapConsulConfig + alloc *structs.Allocation + logger hclog.Logger +} + +func decodeTriState(b *bool) string { + switch { + case b == nil: + return "" + case *b: + return "true" + default: + return "false" + } +} + +func newEnvoyBootstrapHookConfig(alloc *structs.Allocation, consul *config.ConsulConfig, logger hclog.Logger) *envoyBootstrapHookConfig { + return &envoyBootstrapHookConfig{ + alloc: alloc, + logger: logger, + consul: envoyBootstrapConsulConfig{ + HTTPAddr: consul.Addr, + Auth: consul.Auth, + SSL: decodeTriState(consul.EnableSSL), + VerifySSL: decodeTriState(consul.VerifySSL), + CAFile: consul.CAFile, + CertFile: consul.CertFile, + KeyFile: consul.KeyFile, + }, + } } const ( @@ -40,8 +79,9 @@ type envoyBootstrapHook struct { // Bootstrapping Envoy requires talking directly to Consul to generate // the bootstrap.json config. Runtime Envoy configuration is done via - // Consul's gRPC endpoint. - consulHTTPAddr string + // Consul's gRPC endpoint. There are many security parameters to configure + // before contacting Consul. + consulConfig envoyBootstrapConsulConfig // logger is used to log things logger hclog.Logger @@ -49,9 +89,9 @@ type envoyBootstrapHook struct { func newEnvoyBootstrapHook(c *envoyBootstrapHookConfig) *envoyBootstrapHook { return &envoyBootstrapHook{ - alloc: c.alloc, - consulHTTPAddr: c.consulHTTPAddr, - logger: c.logger.Named(envoyBootstrapHookName), + alloc: c.alloc, + consulConfig: c.consul, + logger: c.logger.Named(envoyBootstrapHookName), } } @@ -113,19 +153,23 @@ func (h *envoyBootstrapHook) Prestart(ctx context.Context, req *interfaces.TaskP } h.logger.Debug("check for SI token for task", "task", req.Task.Name, "exists", siToken != "") - bootstrapArgs := envoyBootstrapArgs{ + bootstrapBuilder := envoyBootstrapArgs{ + consulConfig: h.consulConfig, sidecarFor: id, grpcAddr: grpcAddr, - consulHTTPAddr: h.consulHTTPAddr, envoyAdminBind: envoyAdminBind, siToken: siToken, - }.args() + } + + bootstrapArgs := bootstrapBuilder.args() + bootstrapEnv := bootstrapBuilder.env(os.Environ()) // Since Consul services are registered asynchronously with this task // hook running, retry a small number of times with backoff. for tries := 3; ; tries-- { cmd := exec.CommandContext(ctx, "consul", bootstrapArgs...) + cmd.Env = bootstrapEnv // Redirect output to secrets/envoy_bootstrap.json fd, err := os.Create(bootstrapFilePath) @@ -225,10 +269,10 @@ func (h *envoyBootstrapHook) execute(cmd *exec.Cmd) (string, error) { // along to the exec invocation of consul which will then generate the bootstrap // configuration file for envoy. type envoyBootstrapArgs struct { + consulConfig envoyBootstrapConsulConfig sidecarFor string grpcAddr string envoyAdminBind string - consulHTTPAddr string siToken string } @@ -239,17 +283,50 @@ func (e envoyBootstrapArgs) args() []string { "connect", "envoy", "-grpc-addr", e.grpcAddr, - "-http-addr", e.consulHTTPAddr, + "-http-addr", e.consulConfig.HTTPAddr, "-admin-bind", e.envoyAdminBind, "-bootstrap", "-sidecar-for", e.sidecarFor, } - if e.siToken != "" { - arguments = append(arguments, "-token", e.siToken) + + if v := e.siToken; v != "" { + arguments = append(arguments, "-token", v) + } + + if v := e.consulConfig.CAFile; v != "" { + arguments = append(arguments, "-ca-file", v) + } + + if v := e.consulConfig.CertFile; v != "" { + arguments = append(arguments, "-client-cert", v) + } + + if v := e.consulConfig.KeyFile; v != "" { + arguments = append(arguments, "-client-key", v) } + return arguments } +// env creates the context of environment variables to be used when exec-ing +// the consul command for generating the envoy bootstrap config. It is expected +// the value of os.Environ() is passed in to be appended to. Because these are +// appended at the end of what will be passed into Cmd.Env, they will override +// any pre-existing values (i.e. what the Nomad agent was launched with). +// https://golang.org/pkg/os/exec/#Cmd +func (e envoyBootstrapArgs) env(env []string) []string { + if v := e.consulConfig.Auth; v != "" { + env = append(env, fmt.Sprintf("%s=%s", "CONSUL_HTTP_AUTH", v)) + } + if v := e.consulConfig.SSL; v != "" { + env = append(env, fmt.Sprintf("%s=%s", "CONSUL_HTTP_SSL", v)) + } + if v := e.consulConfig.VerifySSL; v != "" { + env = append(env, fmt.Sprintf("%s=%s", "CONSUL_HTTP_SSL_VERIFY", v)) + } + return env +} + // maybeLoadSIToken reads the SI token saved to disk in the secrets directory // by the service identities prestart hook. This envoy bootstrap hook blocks // until the sids hook completes, so if the SI token is required to exist (i.e. diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook_test.go b/client/allocrunner/taskrunner/envoybootstrap_hook_test.go index 7710fae2052..ed104bd4c13 100644 --- a/client/allocrunner/taskrunner/envoybootstrap_hook_test.go +++ b/client/allocrunner/taskrunner/envoybootstrap_hook_test.go @@ -19,11 +19,13 @@ import ( "github.com/hashicorp/nomad/client/taskenv" "github.com/hashicorp/nomad/client/testutil" agentconsul "github.com/hashicorp/nomad/command/agent/consul" + "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/helper/args" "github.com/hashicorp/nomad/helper/testlog" "github.com/hashicorp/nomad/helper/uuid" "github.com/hashicorp/nomad/nomad/mock" "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/nomad/structs/config" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" ) @@ -53,9 +55,9 @@ func TestEnvoyBootstrapHook_maybeLoadSIToken(t *testing.T) { t.Run("file does not exist", func(t *testing.T) { h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{logger: testlog.HCLogger(t)}) - config, err := h.maybeLoadSIToken("task1", "/does/not/exist") + cfg, err := h.maybeLoadSIToken("task1", "/does/not/exist") require.NoError(t, err) // absence of token is not an error - require.Equal(t, "", config) + require.Equal(t, "", cfg) }) t.Run("load token from file", func(t *testing.T) { @@ -64,9 +66,9 @@ func TestEnvoyBootstrapHook_maybeLoadSIToken(t *testing.T) { defer cleanupDir(t, f) h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{logger: testlog.HCLogger(t)}) - config, err := h.maybeLoadSIToken("task1", f) + cfg, err := h.maybeLoadSIToken("task1", f) require.NoError(t, err) - require.Equal(t, token, config) + require.Equal(t, token, cfg) }) t.Run("file is unreadable", func(t *testing.T) { @@ -75,13 +77,37 @@ func TestEnvoyBootstrapHook_maybeLoadSIToken(t *testing.T) { defer cleanupDir(t, f) h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{logger: testlog.HCLogger(t)}) - config, err := h.maybeLoadSIToken("task1", f) + cfg, err := h.maybeLoadSIToken("task1", f) require.Error(t, err) require.False(t, os.IsNotExist(err)) - require.Equal(t, "", config) + require.Equal(t, "", cfg) }) } +func TestEnvoyBootstrapHook_decodeTriState(t *testing.T) { + t.Parallel() + + require.Equal(t, "", decodeTriState(nil)) + require.Equal(t, "true", decodeTriState(helper.BoolToPtr(true))) + require.Equal(t, "false", decodeTriState(helper.BoolToPtr(false))) +} + +var ( + consulPlainConfig = envoyBootstrapConsulConfig{ + HTTPAddr: "2.2.2.2", + } + + consulTLSConfig = envoyBootstrapConsulConfig{ + HTTPAddr: "2.2.2.2", // arg + Auth: "user:password", // env + SSL: "true", // env + VerifySSL: "true", // env + CAFile: "/etc/tls/ca-file", // arg + CertFile: "/etc/tls/cert-file", // arg + KeyFile: "/etc/tls/key-file", // arg + } +) + func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { t.Parallel() @@ -89,17 +115,17 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { ebArgs := envoyBootstrapArgs{ sidecarFor: "s1", grpcAddr: "1.1.1.1", - consulHTTPAddr: "2.2.2.2", + consulConfig: consulPlainConfig, envoyAdminBind: "localhost:3333", } - args := ebArgs.args() + result := ebArgs.args() require.Equal(t, []string{"connect", "envoy", "-grpc-addr", "1.1.1.1", "-http-addr", "2.2.2.2", "-admin-bind", "localhost:3333", "-bootstrap", "-sidecar-for", "s1", - }, args) + }, result) }) t.Run("including SI token", func(t *testing.T) { @@ -107,11 +133,11 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { ebArgs := envoyBootstrapArgs{ sidecarFor: "s1", grpcAddr: "1.1.1.1", - consulHTTPAddr: "2.2.2.2", + consulConfig: consulPlainConfig, envoyAdminBind: "localhost:3333", siToken: token, } - args := ebArgs.args() + result := ebArgs.args() require.Equal(t, []string{"connect", "envoy", "-grpc-addr", "1.1.1.1", "-http-addr", "2.2.2.2", @@ -119,7 +145,58 @@ func TestEnvoyBootstrapHook_envoyBootstrapArgs(t *testing.T) { "-bootstrap", "-sidecar-for", "s1", "-token", token, - }, args) + }, result) + }) + + t.Run("including certificates", func(t *testing.T) { + ebArgs := envoyBootstrapArgs{ + sidecarFor: "s1", + grpcAddr: "1.1.1.1", + consulConfig: consulTLSConfig, + envoyAdminBind: "localhost:3333", + } + result := ebArgs.args() + require.Equal(t, []string{"connect", "envoy", + "-grpc-addr", "1.1.1.1", + "-http-addr", "2.2.2.2", + "-admin-bind", "localhost:3333", + "-bootstrap", + "-sidecar-for", "s1", + "-ca-file", "/etc/tls/ca-file", + "-client-cert", "/etc/tls/cert-file", + "-client-key", "/etc/tls/key-file", + }, result) + }) +} + +func TestEnvoyBootstrapHook_envoyBootstrapEnv(t *testing.T) { + t.Parallel() + + environment := []string{"foo=bar", "baz=1"} + + t.Run("plain consul config", func(t *testing.T) { + require.Equal(t, []string{ + "foo=bar", "baz=1", + }, envoyBootstrapArgs{ + sidecarFor: "s1", + grpcAddr: "1.1.1.1", + consulConfig: consulPlainConfig, + envoyAdminBind: "localhost:3333", + }.env(environment)) + }) + + t.Run("tls consul config", func(t *testing.T) { + require.Equal(t, []string{ + "foo=bar", "baz=1", + "CONSUL_HTTP_AUTH=user:password", + "CONSUL_HTTP_SSL=true", + "CONSUL_HTTP_SSL_VERIFY=true", + }, envoyBootstrapArgs{ + sidecarFor: "s1", + grpcAddr: "1.1.1.1", + consulConfig: consulTLSConfig, + envoyAdminBind: "localhost:3333", + }.env(environment)) }) } @@ -202,11 +279,9 @@ func TestEnvoyBootstrapHook_with_SI_token(t *testing.T) { require.NoError(t, consulClient.RegisterWorkload(agentconsul.BuildAllocServices(mock.Node(), alloc, agentconsul.NoopRestarter()))) // Run Connect bootstrap Hook - h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{ - alloc: alloc, - consulHTTPAddr: testconsul.HTTPAddr, - logger: logger, - }) + h := newEnvoyBootstrapHook(newEnvoyBootstrapHookConfig(alloc, &config.ConsulConfig{ + Addr: consulConfig.Address, + }, logger)) req := &interfaces.TaskPrestartRequest{ Task: sidecarTask, TaskDir: allocDir.NewTaskDir(sidecarTask.Name), @@ -312,11 +387,9 @@ func TestTaskRunner_EnvoyBootstrapHook_Ok(t *testing.T) { require.NoError(t, consulClient.RegisterWorkload(agentconsul.BuildAllocServices(mock.Node(), alloc, agentconsul.NoopRestarter()))) // Run Connect bootstrap Hook - h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{ - alloc: alloc, - consulHTTPAddr: testconsul.HTTPAddr, - logger: logger, - }) + h := newEnvoyBootstrapHook(newEnvoyBootstrapHookConfig(alloc, &config.ConsulConfig{ + Addr: consulConfig.Address, + }, logger)) req := &interfaces.TaskPrestartRequest{ Task: sidecarTask, TaskDir: allocDir.NewTaskDir(sidecarTask.Name), @@ -367,11 +440,9 @@ func TestTaskRunner_EnvoyBootstrapHook_Noop(t *testing.T) { // Run Envoy bootstrap Hook. Use invalid Consul address as it should // not get hit. - h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{ - alloc: alloc, - consulHTTPAddr: "http://127.0.0.2:1", - logger: logger, - }) + h := newEnvoyBootstrapHook(newEnvoyBootstrapHookConfig(alloc, &config.ConsulConfig{ + Addr: "http://127.0.0.2:1", + }, logger)) req := &interfaces.TaskPrestartRequest{ Task: task, TaskDir: allocDir.NewTaskDir(task.Name), @@ -451,11 +522,9 @@ func TestTaskRunner_EnvoyBootstrapHook_RecoverableError(t *testing.T) { // not running. // Run Connect bootstrap Hook - h := newEnvoyBootstrapHook(&envoyBootstrapHookConfig{ - alloc: alloc, - consulHTTPAddr: testconsul.HTTPAddr, - logger: logger, - }) + h := newEnvoyBootstrapHook(newEnvoyBootstrapHookConfig(alloc, &config.ConsulConfig{ + Addr: testconsul.HTTPAddr, + }, logger)) req := &interfaces.TaskPrestartRequest{ Task: sidecarTask, TaskDir: allocDir.NewTaskDir(sidecarTask.Name), diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 470ecd2db71..561ffbbb957 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -128,11 +128,9 @@ func (tr *TaskRunner) initHooks() { } // envoy bootstrap must execute after sidsHook maybe sets SI token - tr.runnerHooks = append(tr.runnerHooks, newEnvoyBootstrapHook(&envoyBootstrapHookConfig{ - alloc: alloc, - consulHTTPAddr: tr.clientConfig.ConsulConfig.Addr, - logger: hookLogger, - })) + tr.runnerHooks = append(tr.runnerHooks, newEnvoyBootstrapHook( + newEnvoyBootstrapHookConfig(alloc, tr.clientConfig.ConsulConfig, hookLogger), + )) } // If there are any script checks, add the hook