From 7e8d5c239201bc1f81af95ed04cfe7bfbd19cab2 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 13 May 2020 14:15:55 -0600 Subject: [PATCH 1/7] consul/connect: add support for running connect native tasks This PR adds the capability of running Connect Native Tasks on Nomad, particularly when TLS and ACLs are enabled on Consul. The `connect` stanza now includes a `native` parameter, which can be set to the name of task that backs the Connect Native Consul service. There is a new Client configuration parameter for the `consul` stanza called `share_ssl`. Like `allow_unauthenticated` the default value is true, but recommended to be disabled in production environments. When enabled, the Nomad Client's Consul TLS information is shared with Connect Native tasks through the normal Consul environment variables. This does NOT include auth or token information. If Consul ACLs are enabled, Service Identity Tokens are automatically and injected into the Connect Native task through the CONSUL_HTTP_TOKEN environment variable. Any of the automatically set environment variables can be overridden by the Connect Native task using the `env` stanza. Fixes #6083 --- api/services.go | 2 +- api/services_test.go | 2 +- client/allocrunner/consulsock_hook.go | 2 +- .../taskrunner/connect_native_hook.go | 215 +++++++ .../taskrunner/connect_native_hook_test.go | 541 ++++++++++++++++++ .../taskrunner/envoybootstrap_hook.go | 30 +- .../taskrunner/envoybootstrap_hook_test.go | 40 +- .../taskrunner/task_runner_hooks.go | 13 +- command/agent/consul/connect.go | 2 +- command/agent/consul/connect_test.go | 4 +- command/agent/job_endpoint_test.go | 20 +- jobspec/parse_test.go | 23 +- .../tg-service-connect-native.hcl | 11 + nomad/job_endpoint_hook_connect.go | 18 +- nomad/node_endpoint.go | 3 +- nomad/structs/config/consul.go | 12 + nomad/structs/diff_test.go | 54 +- nomad/structs/services.go | 18 +- nomad/structs/services_test.go | 8 +- nomad/structs/structs.go | 16 +- nomad/structs/structs_test.go | 17 +- .../hashicorp/nomad/api/services.go | 2 +- website/pages/docs/configuration/consul.mdx | 6 + .../docs/integrations/consul-connect.mdx | 3 +- .../pages/docs/job-specification/connect.mdx | 25 +- 25 files changed, 976 insertions(+), 111 deletions(-) create mode 100644 client/allocrunner/taskrunner/connect_native_hook.go create mode 100644 client/allocrunner/taskrunner/connect_native_hook_test.go create mode 100644 jobspec/test-fixtures/tg-service-connect-native.hcl diff --git a/api/services.go b/api/services.go index f66d80dd1..29a543a79 100644 --- a/api/services.go +++ b/api/services.go @@ -140,7 +140,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { // ConsulConnect represents a Consul Connect jobspec stanza. type ConsulConnect struct { - Native bool + Native string SidecarService *ConsulSidecarService `mapstructure:"sidecar_service"` SidecarTask *SidecarTask `mapstructure:"sidecar_task"` } diff --git a/api/services_test.go b/api/services_test.go index 4bf5f92ed..2734b126e 100644 --- a/api/services_test.go +++ b/api/services_test.go @@ -69,7 +69,7 @@ func TestService_Connect_Canonicalize(t *testing.T) { t.Run("empty connect", func(t *testing.T) { cc := new(ConsulConnect) cc.Canonicalize() - require.False(t, cc.Native) + require.Empty(t, cc.Native) require.Nil(t, cc.SidecarService) require.Nil(t, cc.SidecarTask) }) diff --git a/client/allocrunner/consulsock_hook.go b/client/allocrunner/consulsock_hook.go index 470b060b4..827e0b0f0 100644 --- a/client/allocrunner/consulsock_hook.go +++ b/client/allocrunner/consulsock_hook.go @@ -52,7 +52,7 @@ func (*consulSockHook) Name() string { func (h *consulSockHook) shouldRun() bool { tg := h.alloc.Job.LookupTaskGroup(h.alloc.TaskGroup) for _, s := range tg.Services { - if s.Connect != nil { + if s.Connect.HasSidecar() { return true } } diff --git a/client/allocrunner/taskrunner/connect_native_hook.go b/client/allocrunner/taskrunner/connect_native_hook.go new file mode 100644 index 000000000..d5497c16e --- /dev/null +++ b/client/allocrunner/taskrunner/connect_native_hook.go @@ -0,0 +1,215 @@ +package taskrunner + +import ( + "context" + "io" + "io/ioutil" + "os" + "path/filepath" + + hclog "github.com/hashicorp/go-hclog" + ifs "github.com/hashicorp/nomad/client/allocrunner/interfaces" + "github.com/hashicorp/nomad/nomad/structs" + "github.com/hashicorp/nomad/nomad/structs/config" + "github.com/pkg/errors" +) + +const ( + connectNativeHookName = "connect_native" +) + +type connectNativeHookConfig struct { + consulShareTLS bool + consul consulTransportConfig + alloc *structs.Allocation + logger hclog.Logger +} + +func newConnectNativeHookConfig(alloc *structs.Allocation, consul *config.ConsulConfig, logger hclog.Logger) *connectNativeHookConfig { + return &connectNativeHookConfig{ + alloc: alloc, + logger: logger, + consulShareTLS: consul.ShareSSL == nil || *consul.ShareSSL, // default enabled + consul: newConsulTransportConfig(consul), + } +} + +// connectNativeHook manages additional automagic configuration for a connect +// native task. +// +// If nomad client is configured to talk to Consul using TLS (or other special +// auth), the native task will inherit that configuration EXCEPT for the consul +// token. +// +// If consul is configured with ACLs enabled, a Service Identity token will be +// generated on behalf of the native service and supplied to the task. +type connectNativeHook struct { + // alloc is the allocation with the connect native task being run + alloc *structs.Allocation + + // consulShareTLS is used to toggle whether the TLS configuration of the + // Nomad Client may be shared with Connect Native applications. + consulShareTLS bool + + // consulConfig is used to enable the connect native enabled task to + // communicate with consul directly, as is necessary for the task to request + // its connect mTLS certificates. + consulConfig consulTransportConfig + + // logger is used to log things + logger hclog.Logger +} + +func newConnectNativeHook(c *connectNativeHookConfig) *connectNativeHook { + return &connectNativeHook{ + alloc: c.alloc, + consulShareTLS: c.consulShareTLS, + consulConfig: c.consul, + logger: c.logger.Named(connectNativeHookName), + } +} + +func (connectNativeHook) Name() string { + return connectNativeHookName +} + +func (h *connectNativeHook) Prestart( + ctx context.Context, + request *ifs.TaskPrestartRequest, + response *ifs.TaskPrestartResponse) error { + + if !request.Task.Kind.IsConnectNative() { + response.Done = true + return nil + } + + h.logger.Debug("share consul TLS configuration for connect native", "enabled", h.consulShareTLS, "task", request.Task.Name) + if h.consulShareTLS { + // copy TLS certificates + if err := h.copyCertificates(h.consulConfig, request.TaskDir.SecretsDir); err != nil { + h.logger.Error("failed to copy Consul TLS certificates", "error", err) + return err + } + + // set environment variables for communicating with Consul agent, but + // only if those environment variables are not already set + response.Env = h.tlsEnv(request.TaskEnv.EnvMap) + + } + + if err := h.maybeSetSITokenEnv(request.TaskDir.SecretsDir, request.Task.Name, response.Env); err != nil { + h.logger.Error("failed to load Consul Service Identity Token", "error", err, "task", request.Task.Name) + return err + } + + // tls/acl setup for native task done + response.Done = true + return nil +} + +const ( + secretCAFilename = "consul_ca_file" + secretCertfileFilename = "consul_cert_file" + secretKeyfileFilename = "consul_key_file" +) + +func (h *connectNativeHook) copyCertificates(consulConfig consulTransportConfig, dir string) error { + if err := h.copyCertificate(consulConfig.CAFile, dir, secretCAFilename); err != nil { + return err + } + if err := h.copyCertificate(consulConfig.CertFile, dir, secretCertfileFilename); err != nil { + return err + } + if err := h.copyCertificate(consulConfig.KeyFile, dir, secretKeyfileFilename); err != nil { + return err + } + return nil +} + +func (connectNativeHook) copyCertificate(source, dir, name string) error { + if source == "" { + return nil + } + + original, err := os.Open(source) + if err != nil { + return errors.Wrap(err, "failed to open consul TLS certificate") + } + defer original.Close() + + destination := filepath.Join(dir, name) + fd, err := os.Create(destination) + if err != nil { + return errors.Wrapf(err, "failed to create secrets/%s", name) + } + defer fd.Close() + + if _, err := io.Copy(fd, original); err != nil { + return errors.Wrapf(err, "failed to copy certificate secrets/%s", name) + } + + if err := fd.Sync(); err != nil { + return errors.Wrapf(err, "failed to write secrets/%s", name) + } + + return nil +} + +// tlsEnv creates a set of additional of environment variables to be used when launching +// the connect native task. This will enable the task to communicate with Consul +// if Consul has transport security turned on. +// +// We do NOT set CONSUL_HTTP_TOKEN from the nomad agent's consul config, as that +// is a separate security concern addressed by the service identity hook. +func (h *connectNativeHook) tlsEnv(env map[string]string) map[string]string { + m := make(map[string]string) + + if _, exists := env["CONSUL_CACERT"]; !exists && h.consulConfig.CAFile != "" { + m["CONSUL_CACERT"] = filepath.Join("/secrets", secretCAFilename) + } + + if _, exists := env["CONSUL_CLIENT_CERT"]; !exists && h.consulConfig.CertFile != "" { + m["CONSUL_CLIENT_CERT"] = filepath.Join("/secrets", secretCertfileFilename) + } + + if _, exists := env["CONSUL_CLIENT_KEY"]; !exists && h.consulConfig.KeyFile != "" { + m["CONSUL_CLIENT_KEY"] = filepath.Join("/secrets", secretKeyfileFilename) + } + + if _, exists := env["CONSUL_HTTP_SSL"]; !exists { + if v := h.consulConfig.SSL; v != "" { + m["CONSUL_HTTP_SSL"] = v + } + } + + if _, exists := env["CONSUL_HTTP_SSL_VERIFY"]; !exists { + if v := h.consulConfig.VerifySSL; v != "" { + m["CONSUL_HTTP_SSL_VERIFY"] = v + } + } + + return m +} + +// maybeSetSITokenEnv will set the CONSUL_HTTP_TOKEN environment variable in +// the given env map, if the token is found to exist in the task's secrets +// directory. +// +// Following the pattern of the envoy_bootstrap_hook, the Consul Service Identity +// ACL Token is generated prior to this hook, if Consul ACLs are enabled. This is +// done in the sids_hook, which places the token at secrets/si_token in the task +// workspace. The content of that file is the SI token specific to this task +// instance. +func (h *connectNativeHook) maybeSetSITokenEnv(dir, task string, env map[string]string) error { + token, err := ioutil.ReadFile(filepath.Join(dir, sidsTokenFile)) + if err != nil { + if !os.IsNotExist(err) { + return errors.Wrapf(err, "failed to load SI token for native task %s", task) + } + h.logger.Trace("no SI token to load for native task", "task", task) + return nil // token file DNE; acls not enabled + } + h.logger.Trace("recovered pre-existing SI token for native task", "task", task) + env["CONSUL_HTTP_TOKEN"] = string(token) + return nil +} diff --git a/client/allocrunner/taskrunner/connect_native_hook_test.go b/client/allocrunner/taskrunner/connect_native_hook_test.go new file mode 100644 index 000000000..e19caab76 --- /dev/null +++ b/client/allocrunner/taskrunner/connect_native_hook_test.go @@ -0,0 +1,541 @@ +package taskrunner + +import ( + "context" + "io/ioutil" + "os" + "path/filepath" + "testing" + + consulapi "github.com/hashicorp/consul/api" + consultest "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/nomad/client/allocdir" + "github.com/hashicorp/nomad/client/allocrunner/interfaces" + "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/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" +) + +func getTestConsul(t *testing.T) *consultest.TestServer { + testConsul, err := consultest.NewTestServerConfig(func(c *consultest.TestServerConfig) { + if !testing.Verbose() { // disable consul logging if -v not set + c.Stdout = ioutil.Discard + c.Stderr = ioutil.Discard + } + }) + require.NoError(t, err, "failed to start test consul server") + return testConsul +} + +func TestConnectNativeHook_Name(t *testing.T) { + t.Parallel() + name := new(connectNativeHook).Name() + require.Equal(t, "connect_native", name) +} + +func setupCertDirs(t *testing.T) (string, string) { + fd, err := ioutil.TempFile("", "connect_native_testcert") + require.NoError(t, err) + _, err = fd.WriteString("ABCDEF") + require.NoError(t, err) + err = fd.Close() + require.NoError(t, err) + + d, err := ioutil.TempDir("", "connect_native_testsecrets") + require.NoError(t, err) + return fd.Name(), d +} + +func cleanupCertDirs(t *testing.T, original, secrets string) { + err := os.Remove(original) + require.NoError(t, err) + err = os.RemoveAll(secrets) + require.NoError(t, err) +} + +func TestConnectNativeHook_copyCertificate(t *testing.T) { + t.Parallel() + + f, d := setupCertDirs(t) + defer cleanupCertDirs(t, f, d) + + t.Run("no source", func(t *testing.T) { + err := new(connectNativeHook).copyCertificate("", d, "out.pem") + require.NoError(t, err) + }) + + t.Run("normal", func(t *testing.T) { + err := new(connectNativeHook).copyCertificate(f, d, "out.pem") + require.NoError(t, err) + b, err := ioutil.ReadFile(filepath.Join(d, "out.pem")) + require.NoError(t, err) + require.Equal(t, "ABCDEF", string(b)) + }) +} + +func TestConnectNativeHook_copyCertificates(t *testing.T) { + t.Parallel() + + f, d := setupCertDirs(t) + defer cleanupCertDirs(t, f, d) + + t.Run("normal", func(t *testing.T) { + err := new(connectNativeHook).copyCertificates(consulTransportConfig{ + CAFile: f, + CertFile: f, + KeyFile: f, + }, d) + require.NoError(t, err) + ls, err := ioutil.ReadDir(d) + require.NoError(t, err) + require.Equal(t, 3, len(ls)) + }) + + t.Run("no source", func(t *testing.T) { + err := new(connectNativeHook).copyCertificates(consulTransportConfig{ + CAFile: "/does/not/exist.pem", + CertFile: "/does/not/exist.pem", + KeyFile: "/does/not/exist.pem", + }, d) + require.EqualError(t, err, "failed to open consul TLS certificate: open /does/not/exist.pem: no such file or directory") + }) +} + +func TestConnectNativeHook_tlsEnv(t *testing.T) { + t.Parallel() + + // the hook config comes from client config + emptyHook := new(connectNativeHook) + fullHook := &connectNativeHook{ + consulConfig: consulTransportConfig{ + Auth: "user:password", + SSL: "true", + VerifySSL: "true", + CAFile: "/not/real/ca.pem", + CertFile: "/not/real/cert.pem", + KeyFile: "/not/real/key.pem", + }, + } + + // existing config from task env stanza + taskEnv := map[string]string{ + "CONSUL_CACERT": "fakeCA.pem", + "CONSUL_CLIENT_CERT": "fakeCert.pem", + "CONSUL_CLIENT_KEY": "fakeKey.pem", + "CONSUL_HTTP_AUTH": "foo:bar", + "CONSUL_HTTP_SSL": "false", + "CONSUL_HTTP_SSL_VERIFY": "false", + } + + t.Run("empty hook and empty task", func(t *testing.T) { + result := emptyHook.tlsEnv(nil) + require.Empty(t, result) + }) + + t.Run("empty hook and non-empty task", func(t *testing.T) { + result := emptyHook.tlsEnv(taskEnv) + require.Empty(t, result) // tlsEnv only overrides; task env is actually set elsewhere + }) + + t.Run("non-empty hook and empty task", func(t *testing.T) { + result := fullHook.tlsEnv(nil) + require.Equal(t, map[string]string{ + // ca files are specifically copied into FS namespace + "CONSUL_CACERT": "/secrets/consul_ca_file", + "CONSUL_CLIENT_CERT": "/secrets/consul_cert_file", + "CONSUL_CLIENT_KEY": "/secrets/consul_key_file", + "CONSUL_HTTP_SSL": "true", + "CONSUL_HTTP_SSL_VERIFY": "true", + }, result) + }) + + t.Run("non-empty hook and non-empty task", func(t *testing.T) { + result := fullHook.tlsEnv(taskEnv) // task env takes precedence, nothing gets set here + require.Empty(t, result) + }) +} + +func TestTaskRunner_ConnectNativeHook_Noop(t *testing.T) { + t.Parallel() + logger := testlog.HCLogger(t) + + allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative") + defer cleanup() + + alloc := mock.Alloc() + task := alloc.Job.LookupTaskGroup(alloc.TaskGroup).Tasks[0] + + // run the connect native hook. use invalid consul address as it should not get hit + h := newConnectNativeHook(newConnectNativeHookConfig(alloc, &config.ConsulConfig{ + Addr: "http://127.0.0.2:1", + }, logger)) + + request := &interfaces.TaskPrestartRequest{ + Task: task, + TaskDir: allocDir.NewTaskDir(task.Name), + } + require.NoError(t, request.TaskDir.Build(false, nil)) + + response := new(interfaces.TaskPrestartResponse) + + // Run the hook + require.NoError(t, h.Prestart(context.Background(), request, response)) + + // Assert the hook is Done + require.True(t, response.Done) + + // Assert secrets dir is empty (no TLS config set) + checkFilesInDir(t, request.TaskDir.SecretsDir, + nil, + []string{sidsTokenFile, secretCAFilename, secretCertfileFilename, secretKeyfileFilename}, + ) +} + +func TestTaskRunner_ConnectNativeHook_Ok(t *testing.T) { + t.Parallel() + testutil.RequireConsul(t) + + testConsul := getTestConsul(t) + defer testConsul.Stop() + + alloc := mock.Alloc() + alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} + tg := alloc.Job.TaskGroups[0] + tg.Services = []*structs.Service{{ + Name: "cn-service", + Connect: &structs.ConsulConnect{ + Native: tg.Tasks[0].Name, + }}, + } + tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") + + logger := testlog.HCLogger(t) + + allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative") + defer cleanup() + + // register group services + consulConfig := consulapi.DefaultConfig() + consulConfig.Address = testConsul.HTTPAddr + consulAPIClient, err := consulapi.NewClient(consulConfig) + require.NoError(t, err) + + consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), logger, true) + go consulClient.Run() + defer consulClient.Shutdown() + require.NoError(t, consulClient.RegisterWorkload(agentconsul.BuildAllocServices(mock.Node(), alloc, agentconsul.NoopRestarter()))) + + // Run Connect Native hook + h := newConnectNativeHook(newConnectNativeHookConfig(alloc, &config.ConsulConfig{ + Addr: consulConfig.Address, + }, logger)) + request := &interfaces.TaskPrestartRequest{ + Task: tg.Tasks[0], + TaskDir: allocDir.NewTaskDir(tg.Tasks[0].Name), + TaskEnv: taskenv.NewEmptyTaskEnv(), + } + require.NoError(t, request.TaskDir.Build(false, nil)) + + response := new(interfaces.TaskPrestartResponse) + + // Run the Connect Native hook + require.NoError(t, h.Prestart(context.Background(), request, response)) + + // Assert the hook is Done + require.True(t, response.Done) + + // Assert no environment variables configured to be set + require.Empty(t, response.Env) + + // Assert no secrets were written + checkFilesInDir(t, request.TaskDir.SecretsDir, + nil, + []string{sidsTokenFile, secretCAFilename, secretCertfileFilename, secretKeyfileFilename}, + ) +} + +func TestTaskRunner_ConnectNativeHook_with_SI_token(t *testing.T) { + t.Parallel() + testutil.RequireConsul(t) + + testConsul := getTestConsul(t) + defer testConsul.Stop() + + alloc := mock.Alloc() + alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} + tg := alloc.Job.TaskGroups[0] + tg.Services = []*structs.Service{{ + Name: "cn-service", + Connect: &structs.ConsulConnect{ + Native: tg.Tasks[0].Name, + }}, + } + tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") + + logger := testlog.HCLogger(t) + + allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative") + defer cleanup() + + // register group services + consulConfig := consulapi.DefaultConfig() + consulConfig.Address = testConsul.HTTPAddr + consulAPIClient, err := consulapi.NewClient(consulConfig) + require.NoError(t, err) + + consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), logger, true) + go consulClient.Run() + defer consulClient.Shutdown() + require.NoError(t, consulClient.RegisterWorkload(agentconsul.BuildAllocServices(mock.Node(), alloc, agentconsul.NoopRestarter()))) + + // Run Connect Native hook + h := newConnectNativeHook(newConnectNativeHookConfig(alloc, &config.ConsulConfig{ + Addr: consulConfig.Address, + }, logger)) + request := &interfaces.TaskPrestartRequest{ + Task: tg.Tasks[0], + TaskDir: allocDir.NewTaskDir(tg.Tasks[0].Name), + TaskEnv: taskenv.NewEmptyTaskEnv(), + } + require.NoError(t, request.TaskDir.Build(false, nil)) + + // Insert service identity token in the secrets directory + token := uuid.Generate() + siTokenFile := filepath.Join(request.TaskDir.SecretsDir, sidsTokenFile) + err = ioutil.WriteFile(siTokenFile, []byte(token), 0440) + require.NoError(t, err) + + response := new(interfaces.TaskPrestartResponse) + response.Env = make(map[string]string) + + // Run the Connect Native hook + require.NoError(t, h.Prestart(context.Background(), request, response)) + + // Assert the hook is Done + require.True(t, response.Done) + + // Assert environment variable for token is set + require.NotEmpty(t, response.Env) + require.Equal(t, token, response.Env["CONSUL_HTTP_TOKEN"]) + + // Assert no additional secrets were written + checkFilesInDir(t, request.TaskDir.SecretsDir, + []string{sidsTokenFile}, + []string{secretCAFilename, secretCertfileFilename, secretKeyfileFilename}, + ) +} + +func TestTaskRunner_ConnectNativeHook_shareTLS(t *testing.T) { + t.Parallel() + testutil.RequireConsul(t) + + try := func(t *testing.T, shareSSL *bool) { + fakeCert, fakeCertDir := setupCertDirs(t) + defer cleanupCertDirs(t, fakeCert, fakeCertDir) + + testConsul := getTestConsul(t) + defer testConsul.Stop() + + alloc := mock.Alloc() + alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} + tg := alloc.Job.TaskGroups[0] + tg.Services = []*structs.Service{{ + Name: "cn-service", + Connect: &structs.ConsulConnect{ + Native: tg.Tasks[0].Name, + }}, + } + tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") + + logger := testlog.HCLogger(t) + + allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative") + defer cleanup() + + // register group services + consulConfig := consulapi.DefaultConfig() + consulConfig.Address = testConsul.HTTPAddr + consulAPIClient, err := consulapi.NewClient(consulConfig) + require.NoError(t, err) + + consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), logger, true) + go consulClient.Run() + defer consulClient.Shutdown() + require.NoError(t, consulClient.RegisterWorkload(agentconsul.BuildAllocServices(mock.Node(), alloc, agentconsul.NoopRestarter()))) + + // Run Connect Native hook + h := newConnectNativeHook(newConnectNativeHookConfig(alloc, &config.ConsulConfig{ + Addr: consulConfig.Address, + + // TLS config consumed by native application + ShareSSL: shareSSL, + EnableSSL: helper.BoolToPtr(true), + VerifySSL: helper.BoolToPtr(true), + CAFile: fakeCert, + CertFile: fakeCert, + KeyFile: fakeCert, + Auth: "user:password", + Token: uuid.Generate(), + }, logger)) + request := &interfaces.TaskPrestartRequest{ + Task: tg.Tasks[0], + TaskDir: allocDir.NewTaskDir(tg.Tasks[0].Name), + TaskEnv: taskenv.NewEmptyTaskEnv(), // nothing set in env stanza + } + require.NoError(t, request.TaskDir.Build(false, nil)) + + response := new(interfaces.TaskPrestartResponse) + response.Env = make(map[string]string) + + // Run the Connect Native hook + require.NoError(t, h.Prestart(context.Background(), request, response)) + + // Assert the hook is Done + require.True(t, response.Done) + + // Assert environment variable for token is set + require.NotEmpty(t, response.Env) + require.Equal(t, map[string]string{ + "CONSUL_CACERT": "/secrets/consul_ca_file", + "CONSUL_CLIENT_CERT": "/secrets/consul_cert_file", + "CONSUL_CLIENT_KEY": "/secrets/consul_key_file", + "CONSUL_HTTP_SSL": "true", + "CONSUL_HTTP_SSL_VERIFY": "true", + }, response.Env) + require.NotContains(t, response.Env, "CONSUL_HTTP_AUTH") // explicitly not shared + require.NotContains(t, response.Env, "CONSUL_HTTP_TOKEN") // explicitly not shared + + // Assert 3 pem files were written + checkFilesInDir(t, request.TaskDir.SecretsDir, + []string{secretCAFilename, secretCertfileFilename, secretKeyfileFilename}, + []string{sidsTokenFile}, + ) + } + + // The default behavior is that share_ssl is true (similar to allow_unauthenticated) + // so make sure an unset value turns the feature on. + + t.Run("share_ssl is true", func(t *testing.T) { + try(t, helper.BoolToPtr(true)) + }) + + t.Run("share_ssl is nil", func(t *testing.T) { + try(t, nil) + }) +} + +func checkFilesInDir(t *testing.T, dir string, includes, excludes []string) { + ls, err := ioutil.ReadDir(dir) + require.NoError(t, err) + + var present []string + for _, fInfo := range ls { + present = append(present, fInfo.Name()) + } + + for _, filename := range includes { + require.Contains(t, present, filename) + } + for _, filename := range excludes { + require.NotContains(t, present, filename) + } +} + +func TestTaskRunner_ConnectNativeHook_shareTLS_override(t *testing.T) { + t.Parallel() + testutil.RequireConsul(t) + + fakeCert, fakeCertDir := setupCertDirs(t) + defer cleanupCertDirs(t, fakeCert, fakeCertDir) + + testConsul := getTestConsul(t) + defer testConsul.Stop() + + alloc := mock.Alloc() + alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} + tg := alloc.Job.TaskGroups[0] + tg.Services = []*structs.Service{{ + Name: "cn-service", + Connect: &structs.ConsulConnect{ + Native: tg.Tasks[0].Name, + }}, + } + tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") + + logger := testlog.HCLogger(t) + + allocDir, cleanup := allocdir.TestAllocDir(t, logger, "ConnectNative") + defer cleanup() + + // register group services + consulConfig := consulapi.DefaultConfig() + consulConfig.Address = testConsul.HTTPAddr + consulAPIClient, err := consulapi.NewClient(consulConfig) + require.NoError(t, err) + + consulClient := agentconsul.NewServiceClient(consulAPIClient.Agent(), logger, true) + go consulClient.Run() + defer consulClient.Shutdown() + require.NoError(t, consulClient.RegisterWorkload(agentconsul.BuildAllocServices(mock.Node(), alloc, agentconsul.NoopRestarter()))) + + // Run Connect Native hook + h := newConnectNativeHook(newConnectNativeHookConfig(alloc, &config.ConsulConfig{ + Addr: consulConfig.Address, + + // TLS config consumed by native application + ShareSSL: helper.BoolToPtr(true), + EnableSSL: helper.BoolToPtr(true), + VerifySSL: helper.BoolToPtr(true), + CAFile: fakeCert, + CertFile: fakeCert, + KeyFile: fakeCert, + Auth: "user:password", + }, logger)) + + taskEnv := taskenv.NewEmptyTaskEnv() + taskEnv.EnvMap = map[string]string{ + "CONSUL_CACERT": "/foo/ca.pem", + "CONSUL_CLIENT_CERT": "/foo/cert.pem", + "CONSUL_CLIENT_KEY": "/foo/key.pem", + "CONSUL_HTTP_AUTH": "foo:bar", + "CONSUL_HTTP_SSL_VERIFY": "false", + // CONSUL_HTTP_SSL (check the default value is assumed from client config) + } + + request := &interfaces.TaskPrestartRequest{ + Task: tg.Tasks[0], + TaskDir: allocDir.NewTaskDir(tg.Tasks[0].Name), + TaskEnv: taskEnv, // env stanza is configured w/ non-default tls configs + } + require.NoError(t, request.TaskDir.Build(false, nil)) + + response := new(interfaces.TaskPrestartResponse) + response.Env = make(map[string]string) + + // Run the Connect Native hook + require.NoError(t, h.Prestart(context.Background(), request, response)) + + // Assert the hook is Done + require.True(t, response.Done) + + // Assert environment variable for CONSUL_HTTP_SSL is set, because it was + // the only one not overridden by task env stanza config + require.NotEmpty(t, response.Env) + require.Equal(t, map[string]string{ + "CONSUL_HTTP_SSL": "true", + }, response.Env) + + // Assert 3 pem files were written (even though they will be ignored) + // as this is gated by share_tls, not the presense of ca environment variables. + checkFilesInDir(t, request.TaskDir.SecretsDir, + []string{secretCAFilename, secretCertfileFilename, secretKeyfileFilename}, + []string{sidsTokenFile}, + ) +} diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook.go b/client/allocrunner/taskrunner/envoybootstrap_hook.go index 66d6508d0..297d3daa8 100644 --- a/client/allocrunner/taskrunner/envoybootstrap_hook.go +++ b/client/allocrunner/taskrunner/envoybootstrap_hook.go @@ -22,7 +22,7 @@ import ( const envoyBootstrapHookName = "envoy_bootstrap" -type envoyBootstrapConsulConfig struct { +type consulTransportConfig struct { HTTPAddr string // required Auth string // optional, env CONSUL_HTTP_AUTH SSL string // optional, env CONSUL_HTTP_SSL @@ -33,8 +33,20 @@ type envoyBootstrapConsulConfig struct { // CAPath (dir) not supported by Nomad's config object } +func newConsulTransportConfig(consul *config.ConsulConfig) consulTransportConfig { + return consulTransportConfig{ + HTTPAddr: consul.Addr, + Auth: consul.Auth, + SSL: decodeTriState(consul.EnableSSL), + VerifySSL: decodeTriState(consul.VerifySSL), + CAFile: consul.CAFile, + CertFile: consul.CertFile, + KeyFile: consul.KeyFile, + } +} + type envoyBootstrapHookConfig struct { - consul envoyBootstrapConsulConfig + consul consulTransportConfig alloc *structs.Allocation logger hclog.Logger } @@ -54,15 +66,7 @@ func newEnvoyBootstrapHookConfig(alloc *structs.Allocation, consul *config.Consu 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, - }, + consul: newConsulTransportConfig(consul), } } @@ -81,7 +85,7 @@ type envoyBootstrapHook struct { // the bootstrap.json config. Runtime Envoy configuration is done via // Consul's gRPC endpoint. There are many security parameters to configure // before contacting Consul. - consulConfig envoyBootstrapConsulConfig + consulConfig consulTransportConfig // logger is used to log things logger hclog.Logger @@ -269,7 +273,7 @@ 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 + consulConfig consulTransportConfig sidecarFor string grpcAddr string envoyAdminBind string diff --git a/client/allocrunner/taskrunner/envoybootstrap_hook_test.go b/client/allocrunner/taskrunner/envoybootstrap_hook_test.go index ed104bd4c..de3fa2ec8 100644 --- a/client/allocrunner/taskrunner/envoybootstrap_hook_test.go +++ b/client/allocrunner/taskrunner/envoybootstrap_hook_test.go @@ -13,7 +13,6 @@ import ( "testing" consulapi "github.com/hashicorp/consul/api" - consultest "github.com/hashicorp/consul/sdk/testutil" "github.com/hashicorp/nomad/client/allocdir" "github.com/hashicorp/nomad/client/allocrunner/interfaces" "github.com/hashicorp/nomad/client/taskenv" @@ -93,11 +92,11 @@ func TestEnvoyBootstrapHook_decodeTriState(t *testing.T) { } var ( - consulPlainConfig = envoyBootstrapConsulConfig{ + consulPlainConfig = consulTransportConfig{ HTTPAddr: "2.2.2.2", } - consulTLSConfig = envoyBootstrapConsulConfig{ + consulTLSConfig = consulTransportConfig{ HTTPAddr: "2.2.2.2", // arg Auth: "user:password", // env SSL: "true", // env @@ -220,16 +219,7 @@ func TestEnvoyBootstrapHook_with_SI_token(t *testing.T) { t.Parallel() testutil.RequireConsul(t) - testconsul, err := consultest.NewTestServerConfig(func(c *consultest.TestServerConfig) { - // If -v wasn't specified squelch consul logging - if !testing.Verbose() { - c.Stdout = ioutil.Discard - c.Stderr = ioutil.Discard - } - }) - if err != nil { - t.Fatalf("error starting test consul server: %v", err) - } + testconsul := getTestConsul(t) defer testconsul.Stop() alloc := mock.Alloc() @@ -328,16 +318,7 @@ func TestTaskRunner_EnvoyBootstrapHook_Ok(t *testing.T) { t.Parallel() testutil.RequireConsul(t) - testconsul, err := consultest.NewTestServerConfig(func(c *consultest.TestServerConfig) { - // If -v wasn't specified squelch consul logging - if !testing.Verbose() { - c.Stdout = ioutil.Discard - c.Stderr = ioutil.Discard - } - }) - if err != nil { - t.Fatalf("error starting test consul server: %v", err) - } + testconsul := getTestConsul(t) defer testconsul.Stop() alloc := mock.Alloc() @@ -470,16 +451,7 @@ func TestTaskRunner_EnvoyBootstrapHook_RecoverableError(t *testing.T) { t.Parallel() testutil.RequireConsul(t) - testconsul, err := consultest.NewTestServerConfig(func(c *consultest.TestServerConfig) { - // If -v wasn't specified squelch consul logging - if !testing.Verbose() { - c.Stdout = ioutil.Discard - c.Stderr = ioutil.Discard - } - }) - if err != nil { - t.Fatalf("error starting test consul server: %v", err) - } + testconsul := getTestConsul(t) defer testconsul.Stop() alloc := mock.Alloc() @@ -534,7 +506,7 @@ func TestTaskRunner_EnvoyBootstrapHook_RecoverableError(t *testing.T) { resp := &interfaces.TaskPrestartResponse{} // Run the hook - err = h.Prestart(context.Background(), req, resp) + err := h.Prestart(context.Background(), req, resp) require.EqualError(t, err, "error creating bootstrap configuration for Connect proxy sidecar: exit status 1") require.True(t, structs.IsRecoverable(err)) diff --git a/client/allocrunner/taskrunner/task_runner_hooks.go b/client/allocrunner/taskrunner/task_runner_hooks.go index 561ffbbb9..07325c012 100644 --- a/client/allocrunner/taskrunner/task_runner_hooks.go +++ b/client/allocrunner/taskrunner/task_runner_hooks.go @@ -127,10 +127,15 @@ func (tr *TaskRunner) initHooks() { })) } - // envoy bootstrap must execute after sidsHook maybe sets SI token - tr.runnerHooks = append(tr.runnerHooks, newEnvoyBootstrapHook( - newEnvoyBootstrapHookConfig(alloc, tr.clientConfig.ConsulConfig, hookLogger), - )) + if task.Kind.IsConnectProxy() { + tr.runnerHooks = append(tr.runnerHooks, newEnvoyBootstrapHook( + newEnvoyBootstrapHookConfig(alloc, tr.clientConfig.ConsulConfig, hookLogger), + )) + } else if task.Kind.IsConnectNative() { + tr.runnerHooks = append(tr.runnerHooks, newConnectNativeHook( + newConnectNativeHookConfig(alloc, tr.clientConfig.ConsulConfig, hookLogger), + )) + } } // If there are any script checks, add the hook diff --git a/command/agent/consul/connect.go b/command/agent/consul/connect.go index 888824766..ef2cd602f 100644 --- a/command/agent/consul/connect.go +++ b/command/agent/consul/connect.go @@ -17,7 +17,7 @@ func newConnect(serviceName string, nc *structs.ConsulConnect, networks structs. return nil, nil } - if nc.Native { + if nc.IsNative() { return &api.AgentServiceConnect{Native: true}, nil } diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go index 8ce74c1df..948e45443 100644 --- a/command/agent/consul/connect_test.go +++ b/command/agent/consul/connect_test.go @@ -32,7 +32,7 @@ func TestConnect_newConnect(t *testing.T) { t.Run("native", func(t *testing.T) { asr, err := newConnect("", &structs.ConsulConnect{ - Native: true, + Native: "foo", }, nil) require.NoError(t, err) require.True(t, asr.Native) @@ -41,7 +41,7 @@ func TestConnect_newConnect(t *testing.T) { t.Run("with sidecar", func(t *testing.T) { asr, err := newConnect("redis", &structs.ConsulConnect{ - Native: false, + Native: "", SidecarService: &structs.ConsulSidecarService{ Tags: []string{"foo", "bar"}, Port: "sidecarPort", diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index 2d9b615d0..ee2496fa9 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -1693,7 +1693,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, }, Connect: &api.ConsulConnect{ - Native: false, + Native: "", SidecarService: &api.ConsulSidecarService{ Tags: []string{"f", "g"}, Port: "9000", @@ -2061,7 +2061,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, }, Connect: &structs.ConsulConnect{ - Native: false, + Native: "", SidecarService: &structs.ConsulSidecarService{ Tags: []string{"f", "g"}, Port: "9000", @@ -2763,16 +2763,26 @@ func TestConversion_apiConnectSidecarServiceToStructs(t *testing.T) { })) } -func TestConversion_ApiConsulConnectToStructs(t *testing.T) { +func TestConversion_ApiConsulConnectToStructs_legacy(t *testing.T) { t.Parallel() require.Nil(t, ApiConsulConnectToStructs(nil)) require.Equal(t, &structs.ConsulConnect{ - Native: false, + Native: "", SidecarService: &structs.ConsulSidecarService{Port: "myPort"}, SidecarTask: &structs.SidecarTask{Name: "task"}, }, ApiConsulConnectToStructs(&api.ConsulConnect{ - Native: false, + Native: "", SidecarService: &api.ConsulSidecarService{Port: "myPort"}, SidecarTask: &api.SidecarTask{Name: "task"}, })) } + +func TestConversion_ApiConsulConnectToStructs_native(t *testing.T) { + t.Parallel() + require.Nil(t, ApiConsulConnectToStructs(nil)) + require.Equal(t, &structs.ConsulConnect{ + Native: "foo", + }, ApiConsulConnectToStructs(&api.ConsulConnect{ + Native: "foo", + })) +} diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index 8ebef45d7..f8595b085 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1168,7 +1168,7 @@ func TestParse(t *testing.T) { Services: []*api.Service{{ Name: "example", Connect: &api.ConsulConnect{ - Native: false, + Native: "", SidecarService: &api.ConsulSidecarService{}, SidecarTask: &api.SidecarTask{ Name: "my-sidecar", @@ -1190,7 +1190,7 @@ func TestParse(t *testing.T) { Services: []*api.Service{{ Name: "example", Connect: &api.ConsulConnect{ - Native: false, + Native: "", SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ LocalServiceAddress: "10.0.1.2", @@ -1237,7 +1237,7 @@ func TestParse(t *testing.T) { Services: []*api.Service{{ Name: "example", Connect: &api.ConsulConnect{ - Native: false, + Native: "", SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ LocalServiceAddress: "10.0.1.2", @@ -1276,6 +1276,23 @@ func TestParse(t *testing.T) { }, false, }, + { + "tg-service-connect-native.hcl", + &api.Job{ + ID: helper.StringToPtr("connect_native_service"), + Name: helper.StringToPtr("connect_native_service"), + TaskGroups: []*api.TaskGroup{{ + Name: helper.StringToPtr("group"), + Services: []*api.Service{{ + Name: "example", + Connect: &api.ConsulConnect{ + Native: "foo", + }, + }}, + }}, + }, + false, + }, { "tg-service-enable-tag-override.hcl", &api.Job{ diff --git a/jobspec/test-fixtures/tg-service-connect-native.hcl b/jobspec/test-fixtures/tg-service-connect-native.hcl new file mode 100644 index 000000000..08ac92440 --- /dev/null +++ b/jobspec/test-fixtures/tg-service-connect-native.hcl @@ -0,0 +1,11 @@ +job "connect_native_service" { + group "group" { + service { + name = "example" + + connect { + native = "foo" + } + } + } +} diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index 9dc91953c..3ae0d8edf 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -97,6 +97,15 @@ func isSidecarForService(t *structs.Task, svc string) bool { return t.Kind == structs.TaskKind(fmt.Sprintf("%s:%s", structs.ConnectProxyPrefix, svc)) } +func getNamedTaskForNativeService(tg *structs.TaskGroup, taskName string) *structs.Task { + for _, t := range tg.Tasks { + if t.Name == taskName { + return t + } + } + return nil +} + // probably need to hack this up to look for checks on the service, and if they // qualify, configure a port for envoy to use to expose their paths. func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { @@ -145,10 +154,15 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { // create a port for the sidecar task's proxy port makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name)) - // todo(shoenig) magic port for 'expose.checks' + } else if service.Connect.IsNative() { + nativeTaskName := service.Connect.Native + if t := getNamedTaskForNativeService(g, nativeTaskName); t != nil { + t.Kind = structs.NewTaskKind(structs.ConnectNativePrefix, service.Name) + } else { + return fmt.Errorf("native task %s named by %s->%s does not exist", nativeTaskName, g.Name, service.Name) + } } } - return nil } diff --git a/nomad/node_endpoint.go b/nomad/node_endpoint.go index 981522baf..c83e46312 100644 --- a/nomad/node_endpoint.go +++ b/nomad/node_endpoint.go @@ -1889,8 +1889,7 @@ func taskUsesConnect(task *structs.Task) bool { // not even in the task group return false } - - return task.Kind.IsConnectProxy() || task.Kind.IsConnectNative() + return task.UsesConnect() } func (n *Node) EmitEvents(args *structs.EmitNodeEventsRequest, reply *structs.EmitNodeEventsResponse) error { diff --git a/nomad/structs/config/consul.go b/nomad/structs/config/consul.go index 8d0103287..538cd2f11 100644 --- a/nomad/structs/config/consul.go +++ b/nomad/structs/config/consul.go @@ -85,6 +85,12 @@ type ConsulConfig struct { // Uses Consul's default and env var. EnableSSL *bool `hcl:"ssl"` + // ShareSSL enables Consul Connect Native applications to use the TLS + // configuration of the Nomad Client for establishing connections to Consul. + // + // Does not include sharing of ACL tokens. + ShareSSL *bool `hcl:"share_ssl"` + // VerifySSL enables or disables SSL verification when the transport scheme // for the consul api client is https // @@ -200,6 +206,9 @@ func (c *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig { if b.VerifySSL != nil { result.VerifySSL = helper.BoolToPtr(*b.VerifySSL) } + if b.ShareSSL != nil { + result.ShareSSL = helper.BoolToPtr(*b.ShareSSL) + } if b.CAFile != "" { result.CAFile = b.CAFile } @@ -301,6 +310,9 @@ func (c *ConsulConfig) Copy() *ConsulConfig { if nc.VerifySSL != nil { nc.VerifySSL = helper.BoolToPtr(*nc.VerifySSL) } + if nc.ShareSSL != nil { + nc.ShareSSL = helper.BoolToPtr(*nc.ShareSSL) + } if nc.ServerAutoJoin != nil { nc.ServerAutoJoin = helper.BoolToPtr(*nc.ServerAutoJoin) } diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 2266a93c5..4994d1eb9 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2788,8 +2788,8 @@ func TestTaskGroupDiff(t *testing.T) { { Type: DiffTypeNone, Name: "Native", - Old: "false", - New: "false", + Old: "", + New: "", }, }, Objects: []*ObjectDiff{ @@ -4915,6 +4915,48 @@ func TestTaskDiff(t *testing.T) { }, }, }, + Expected: &TaskDiff{ + Type: DiffTypeEdited, + Objects: []*ObjectDiff{ + { + Type: DiffTypeEdited, + Name: "Service", + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "ConsulConnect", + Objects: []*ObjectDiff{ + { + Type: DiffTypeAdded, + Name: "SidecarService", + }, + }, + }, + }, + }, + }, + }, + }, + + { + Name: "Service with Connect Native", + Old: &Task{ + Services: []*Service{ + { + Name: "foo", + }, + }, + }, + New: &Task{ + Services: []*Service{ + { + Name: "foo", + Connect: &ConsulConnect{ + Native: "task1", + }, + }, + }, + }, Expected: &TaskDiff{ Type: DiffTypeEdited, Objects: []*ObjectDiff{ @@ -4930,13 +4972,7 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeAdded, Name: "Native", Old: "", - New: "false", - }, - }, - Objects: []*ObjectDiff{ - { - Type: DiffTypeAdded, - Name: "SidecarService", + New: "task1", }, }, }, diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 578b1dd22..d070e9746 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -620,9 +620,13 @@ OUTER: // ConsulConnect represents a Consul Connect jobspec stanza. type ConsulConnect struct { - // Native is true if a service implements Connect directly and does not - // need a sidecar. - Native bool + // Native is empty if the service represents a legacy application that + // requires an Envoy (or otherwise configured via SidecarTask) sidecar + // that is managed via xDS by Consul. + // + // If non-empty, Native indicates the Connect-Native Task associated with + // the service definition in which this Connect stanza resides. + Native string // SidecarService is non-nil if a service requires a sidecar. SidecarService *ConsulSidecarService @@ -662,17 +666,21 @@ func (c *ConsulConnect) HasSidecar() bool { return c != nil && c.SidecarService != nil } +func (c *ConsulConnect) IsNative() bool { + return c != nil && c.Native != "" +} + // Validate that the Connect stanza has exactly one of Native or sidecar. func (c *ConsulConnect) Validate() error { if c == nil { return nil } - if c.Native && c.SidecarService != nil { + if c.IsNative() && c.HasSidecar() { return fmt.Errorf("Consul Connect must be native or use a sidecar service; not both") } - if !c.Native && c.SidecarService == nil { + if !c.IsNative() && !c.HasSidecar() { return fmt.Errorf("Consul Connect must be native or use a sidecar service") } diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index f6fed3c64..3405998fb 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -123,16 +123,16 @@ func TestConsulConnect_Validate(t *testing.T) { // An empty Connect stanza is invalid require.Error(t, c.Validate()) - // Native=true is valid - c.Native = true + // Native= is valid + c.Native = "foo" require.NoError(t, c.Validate()) // Native=true + Sidecar!=nil is invalid c.SidecarService = &ConsulSidecarService{} require.Error(t, c.Validate()) - // Native=false + Sidecar!=nil is valid - c.Native = false + // Native= + Sidecar!=nil is valid + c.Native = "" require.NoError(t, c.Validate()) } diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index 7c4c66802..aea3cfb31 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -5967,7 +5967,7 @@ func (tg *TaskGroup) LookupTask(name string) *Task { func (tg *TaskGroup) UsesConnect() bool { for _, service := range tg.Services { if service.Connect != nil { - if service.Connect.Native || service.Connect.SidecarService != nil { + if service.Connect.IsNative() || service.Connect.HasSidecar() { return true } } @@ -6167,18 +6167,10 @@ type Task struct { // UsesConnect is for conveniently detecting if the Task is able to make use // of Consul Connect features. This will be indicated in the TaskKind of the -// Task, which exports known types of Tasks. -// -// Currently only Consul Connect Proxy tasks are known. -// (Consul Connect Native tasks will be supported soon). +// Task, which exports known types of Tasks. UsesConnect will be true if the +// task is a connect proxy, or if the task is connect native. func (t *Task) UsesConnect() bool { - // todo(shoenig): native tasks - switch { - case t.Kind.IsConnectProxy(): - return true - default: - return false - } + return t.Kind.IsConnectProxy() || t.Kind.IsConnectNative() } func (t *Task) Copy() *Task { diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index a5d293ead..4c0a05fd6 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -814,13 +814,20 @@ func TestTask_UsesConnect(t *testing.T) { t.Run("sidecar proxy", func(t *testing.T) { task := &Task{ Name: "connect-proxy-task1", - Kind: "connect-proxy:task1", + Kind: NewTaskKind(ConnectProxyPrefix, "task1"), } usesConnect := task.UsesConnect() require.True(t, usesConnect) }) - // todo(shoenig): add native case + t.Run("native task", func(t *testing.T) { + task := &Task{ + Name: "task1", + Kind: NewTaskKind(ConnectNativePrefix, "task1"), + } + usesConnect := task.UsesConnect() + require.True(t, usesConnect) + }) } func TestTaskGroup_UsesConnect(t *testing.T) { @@ -835,7 +842,7 @@ func TestTaskGroup_UsesConnect(t *testing.T) { try(t, &TaskGroup{ Services: []*Service{ {Connect: nil}, - {Connect: &ConsulConnect{Native: true}}, + {Connect: &ConsulConnect{Native: "foo"}}, }, }, true) }) @@ -2709,7 +2716,7 @@ func TestService_Validate(t *testing.T) { // Native Connect should be valid s.Connect = &ConsulConnect{ - Native: true, + Native: "testtask", } require.NoError(t, s.Validate()) @@ -2756,7 +2763,7 @@ func TestService_Equals(t *testing.T) { o.Checks = []*ServiceCheck{{Name: "diff"}} assertDiff() - o.Connect = &ConsulConnect{Native: true} + o.Connect = &ConsulConnect{Native: "testtask"} assertDiff() o.EnableTagOverride = true diff --git a/vendor/github.com/hashicorp/nomad/api/services.go b/vendor/github.com/hashicorp/nomad/api/services.go index f66d80dd1..29a543a79 100644 --- a/vendor/github.com/hashicorp/nomad/api/services.go +++ b/vendor/github.com/hashicorp/nomad/api/services.go @@ -140,7 +140,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { // ConsulConnect represents a Consul Connect jobspec stanza. type ConsulConnect struct { - Native bool + Native string SidecarService *ConsulSidecarService `mapstructure:"sidecar_service"` SidecarTask *SidecarTask `mapstructure:"sidecar_task"` } diff --git a/website/pages/docs/configuration/consul.mdx b/website/pages/docs/configuration/consul.mdx index 29a782580..331619082 100644 --- a/website/pages/docs/configuration/consul.mdx +++ b/website/pages/docs/configuration/consul.mdx @@ -104,6 +104,12 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul Consul service name defined in the `server_service_name` option. This search only happens if the server does not have a leader. +- `share_ssl` `(bool: true)` - Specifies whether the Nomad client should share + its Consul SSL configuration with Connect Native applications. Includes values + of `ca_file`, `cert_file`, `key_file`, `auth`, `ssl`, and `verify_ssl`. Does + not include the value for the ACL `token`. This option should be disabled in + an untrusted environment. + - `ssl` `(bool: false)` - Specifies if the transport scheme should use HTTPS to communicate with the Consul agent. Will default to the `CONSUL_HTTP_SSL` environment variable if set. diff --git a/website/pages/docs/integrations/consul-connect.mdx b/website/pages/docs/integrations/consul-connect.mdx index 83e77d2d4..389182538 100644 --- a/website/pages/docs/integrations/consul-connect.mdx +++ b/website/pages/docs/integrations/consul-connect.mdx @@ -328,7 +328,7 @@ dashes (`-`) are converted to underscores (`_`) in environment variables so - The `consul` binary must be present in Nomad's `$PATH` to run the Envoy proxy sidecar on client nodes. -- Consul Connect Native is not yet supported ([#6083][gh6083]). +- Consul Connect Native requires host networking. - Only the Docker, `exec`, `raw_exec`, and `java` drivers support network namespaces and Connect. - Changes to the `connect` stanza may not properly trigger a job update @@ -337,7 +337,6 @@ dashes (`-`) are converted to underscores (`_`) in environment variables so - Consul Connect and network namespaces are only supported on Linux. [count-dashboard]: /img/count-dashboard.png -[gh6083]: https://github.com/hashicorp/nomad/issues/6083 [gh6120]: https://github.com/hashicorp/nomad/issues/6120 [gh6701]: https://github.com/hashicorp/nomad/issues/6701 [gh6459]: https://github.com/hashicorp/nomad/issues/6459 diff --git a/website/pages/docs/job-specification/connect.mdx b/website/pages/docs/job-specification/connect.mdx index 15567cd1e..df2216389 100644 --- a/website/pages/docs/job-specification/connect.mdx +++ b/website/pages/docs/job-specification/connect.mdx @@ -47,14 +47,20 @@ job "countdash" { ## `connect` Parameters +- `native` - `(string: "")` - If non-empty, this indicates the task represented + by this service, for use with [Connect Native](https://www.consul.io/docs/connect/native) + applications. Incompatible with `sidecar_service` and `sidecar_task`. + - `sidecar_service` - ([sidecar_service][]: nil) - This is used to configure the sidecar - service injected by Nomad for Consul Connect. + service injected by Nomad for Consul Connect. Incompatible with `native`. - `sidecar_task` - ([sidecar_task][]:nil) - This modifies the configuration of the Envoy - proxy task. + proxy task. Incompatible with `native`. ## `connect` Examples +### Using Sidecar Service + The following example is a minimal connect stanza with defaults and is sufficient to start an Envoy proxy sidecar for allowing incoming connections via Consul Connect. @@ -161,10 +167,21 @@ job "countdash" { } ``` +### Using Connect Native + +The following example is a minimal connect stanza for a +[Consul Connect Native](https://www.consul.io/docs/connect/native) +application with task name `myTask`. + +```hcl +connect { + native = "myTask" +} +``` + ### Limitations -[Consul Connect Native services][native] and [Nomad variable -interpolation][interpolation] are _not_ yet supported. +[Nomad variable interpolation][interpolation] is _not_ yet supported. [job]: /docs/job-specification/job 'Nomad job Job Specification' [group]: /docs/job-specification/group 'Nomad group Job Specification' From 520d35e0857a3df7ba1c4d26477486f5d9a60ed3 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 22 Jun 2020 12:55:59 -0500 Subject: [PATCH 2/7] consul/connect: split connect native flag and task in service --- api/services.go | 3 +- command/agent/job_endpoint.go | 1 + command/agent/job_endpoint_test.go | 12 ++-- jobspec/parse_service.go | 1 + jobspec/parse_test.go | 11 +-- .../tg-service-connect-native.hcl | 3 +- nomad/job_endpoint_hook_connect.go | 44 +++++++++--- nomad/job_endpoint_hook_connect_test.go | 67 ++++++++++++++++++- nomad/structs/diff_test.go | 56 ++++++++++++++-- nomad/structs/services.go | 27 +++++--- nomad/structs/services_test.go | 6 +- nomad/structs/structs_test.go | 12 ++-- .../hashicorp/nomad/api/services.go | 3 +- .../pages/docs/job-specification/connect.mdx | 23 +++++-- .../pages/docs/job-specification/service.mdx | 5 ++ 15 files changed, 216 insertions(+), 58 deletions(-) diff --git a/api/services.go b/api/services.go index 29a543a79..f0658d47e 100644 --- a/api/services.go +++ b/api/services.go @@ -110,6 +110,7 @@ type Service struct { Connect *ConsulConnect Meta map[string]string CanaryMeta map[string]string + TaskName string `mapstructure:"task"` } // Canonicalize the Service by ensuring its name and address mode are set. Task @@ -140,7 +141,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { // ConsulConnect represents a Consul Connect jobspec stanza. type ConsulConnect struct { - Native string + Native bool SidecarService *ConsulSidecarService `mapstructure:"sidecar_service"` SidecarTask *SidecarTask `mapstructure:"sidecar_task"` } diff --git a/command/agent/job_endpoint.go b/command/agent/job_endpoint.go index d3caf6b97..ef77a2617 100644 --- a/command/agent/job_endpoint.go +++ b/command/agent/job_endpoint.go @@ -1177,6 +1177,7 @@ func ApiServicesToStructs(in []*api.Service) []*structs.Service { out[i] = &structs.Service{ Name: s.Name, PortLabel: s.PortLabel, + TaskName: s.TaskName, Tags: s.Tags, CanaryTags: s.CanaryTags, EnableTagOverride: s.EnableTagOverride, diff --git a/command/agent/job_endpoint_test.go b/command/agent/job_endpoint_test.go index ee2496fa9..797e51094 100644 --- a/command/agent/job_endpoint_test.go +++ b/command/agent/job_endpoint_test.go @@ -1693,7 +1693,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, }, Connect: &api.ConsulConnect{ - Native: "", + Native: false, SidecarService: &api.ConsulSidecarService{ Tags: []string{"f", "g"}, Port: "9000", @@ -2061,7 +2061,7 @@ func TestJobs_ApiJobToStructsJob(t *testing.T) { }, }, Connect: &structs.ConsulConnect{ - Native: "", + Native: false, SidecarService: &structs.ConsulSidecarService{ Tags: []string{"f", "g"}, Port: "9000", @@ -2767,11 +2767,11 @@ func TestConversion_ApiConsulConnectToStructs_legacy(t *testing.T) { t.Parallel() require.Nil(t, ApiConsulConnectToStructs(nil)) require.Equal(t, &structs.ConsulConnect{ - Native: "", + Native: false, SidecarService: &structs.ConsulSidecarService{Port: "myPort"}, SidecarTask: &structs.SidecarTask{Name: "task"}, }, ApiConsulConnectToStructs(&api.ConsulConnect{ - Native: "", + Native: false, SidecarService: &api.ConsulSidecarService{Port: "myPort"}, SidecarTask: &api.SidecarTask{Name: "task"}, })) @@ -2781,8 +2781,8 @@ func TestConversion_ApiConsulConnectToStructs_native(t *testing.T) { t.Parallel() require.Nil(t, ApiConsulConnectToStructs(nil)) require.Equal(t, &structs.ConsulConnect{ - Native: "foo", + Native: true, }, ApiConsulConnectToStructs(&api.ConsulConnect{ - Native: "foo", + Native: true, })) } diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index 9f6fa4ad4..c0fa09096 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -47,6 +47,7 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) { "address_mode", "check_restart", "connect", + "task", // todo "meta", "canary_meta", } diff --git a/jobspec/parse_test.go b/jobspec/parse_test.go index f8595b085..35a79287b 100644 --- a/jobspec/parse_test.go +++ b/jobspec/parse_test.go @@ -1168,7 +1168,7 @@ func TestParse(t *testing.T) { Services: []*api.Service{{ Name: "example", Connect: &api.ConsulConnect{ - Native: "", + Native: false, SidecarService: &api.ConsulSidecarService{}, SidecarTask: &api.SidecarTask{ Name: "my-sidecar", @@ -1190,7 +1190,7 @@ func TestParse(t *testing.T) { Services: []*api.Service{{ Name: "example", Connect: &api.ConsulConnect{ - Native: "", + Native: false, SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ LocalServiceAddress: "10.0.1.2", @@ -1237,7 +1237,7 @@ func TestParse(t *testing.T) { Services: []*api.Service{{ Name: "example", Connect: &api.ConsulConnect{ - Native: "", + Native: false, SidecarService: &api.ConsulSidecarService{ Proxy: &api.ConsulProxy{ LocalServiceAddress: "10.0.1.2", @@ -1284,9 +1284,10 @@ func TestParse(t *testing.T) { TaskGroups: []*api.TaskGroup{{ Name: helper.StringToPtr("group"), Services: []*api.Service{{ - Name: "example", + Name: "example", + TaskName: "task1", Connect: &api.ConsulConnect{ - Native: "foo", + Native: true, }, }}, }}, diff --git a/jobspec/test-fixtures/tg-service-connect-native.hcl b/jobspec/test-fixtures/tg-service-connect-native.hcl index 08ac92440..a484674bf 100644 --- a/jobspec/test-fixtures/tg-service-connect-native.hcl +++ b/jobspec/test-fixtures/tg-service-connect-native.hcl @@ -2,9 +2,10 @@ job "connect_native_service" { group "group" { service { name = "example" + task = "task1" connect { - native = "foo" + native = true } } } diff --git a/nomad/job_endpoint_hook_connect.go b/nomad/job_endpoint_hook_connect.go index 3ae0d8edf..73b264006 100644 --- a/nomad/job_endpoint_hook_connect.go +++ b/nomad/job_endpoint_hook_connect.go @@ -155,7 +155,7 @@ func groupConnectHook(job *structs.Job, g *structs.TaskGroup) error { // create a port for the sidecar task's proxy port makePort(fmt.Sprintf("%s-%s", structs.ConnectProxyPrefix, service.Name)) } else if service.Connect.IsNative() { - nativeTaskName := service.Connect.Native + nativeTaskName := service.TaskName if t := getNamedTaskForNativeService(g, nativeTaskName); t != nil { t.Kind = structs.NewTaskKind(structs.ConnectNativePrefix, service.Name) } else { @@ -194,18 +194,42 @@ func newConnectTask(serviceName string) *structs.Task { func groupConnectValidate(g *structs.TaskGroup) (warnings []error, err error) { for _, s := range g.Services { if s.Connect.HasSidecar() { - if n := len(g.Networks); n != 1 { - return nil, fmt.Errorf("Consul Connect sidecars require exactly 1 network, found %d in group %q", n, g.Name) + if err := groupConnectSidecarValidate(g); err != nil { + return nil, err } - - if g.Networks[0].Mode != "bridge" { - return nil, fmt.Errorf("Consul Connect sidecar requires bridge network, found %q in group %q", g.Networks[0].Mode, g.Name) + } else if s.Connect.IsNative() { + if err := groupConnectNativeValidate(g, s); err != nil { + return nil, err } - - // Stopping loop, only need to do the validation once - break } } - return nil, nil } + +func groupConnectSidecarValidate(g *structs.TaskGroup) error { + if n := len(g.Networks); n != 1 { + return fmt.Errorf("Consul Connect sidecars require exactly 1 network, found %d in group %q", n, g.Name) + } + + if g.Networks[0].Mode != "bridge" { + return fmt.Errorf("Consul Connect sidecar requires bridge network, found %q in group %q", g.Networks[0].Mode, g.Name) + } + return nil +} + +func groupConnectNativeValidate(g *structs.TaskGroup, s *structs.Service) error { + // note that network mode is not enforced for connect native services + + // a native service must have the task identified in the service definition. + if len(s.TaskName) == 0 { + return fmt.Errorf("Consul Connect Native service %q requires task name", s.Name) + } + + // also make sure that task actually exists + for _, task := range g.Tasks { + if s.TaskName == task.Name { + return nil + } + } + return fmt.Errorf("Consul Connect Native service %q requires undefined task %q in group %q", s.Name, s.TaskName, g.Name) +} diff --git a/nomad/job_endpoint_hook_connect_test.go b/nomad/job_endpoint_hook_connect_test.go index 7e48c4cda..931554ed1 100644 --- a/nomad/job_endpoint_hook_connect_test.go +++ b/nomad/job_endpoint_hook_connect_test.go @@ -10,7 +10,9 @@ import ( "github.com/stretchr/testify/require" ) -func Test_isSidecarForService(t *testing.T) { +func TestJobEndpointConnect_isSidecarForService(t *testing.T) { + t.Parallel() + cases := []struct { t *structs.Task // task s string // service @@ -49,7 +51,9 @@ func Test_isSidecarForService(t *testing.T) { } } -func Test_groupConnectHook(t *testing.T) { +func TestJobEndpointConnect_groupConnectHook(t *testing.T) { + t.Parallel() + // Test that connect-proxy task is inserted for backend service job := mock.Job() job.TaskGroups[0] = &structs.TaskGroup{ @@ -110,7 +114,7 @@ func Test_groupConnectHook(t *testing.T) { // the service name is interpolated *before the task is created. // // See https://github.com/hashicorp/nomad/issues/6853 -func TestJobEndpoint_ConnectInterpolation(t *testing.T) { +func TestJobEndpointConnect_ConnectInterpolation(t *testing.T) { t.Parallel() server := &Server{logger: testlog.HCLogger(t)} @@ -125,3 +129,60 @@ func TestJobEndpoint_ConnectInterpolation(t *testing.T) { require.Len(t, j.TaskGroups[0].Tasks, 2) require.Equal(t, "connect-proxy-my-job-api", j.TaskGroups[0].Tasks[1].Name) } + +func TestJobEndpointConnect_groupConnectSidecarValidate(t *testing.T) { + t.Run("sidecar 0 networks", func(t *testing.T) { + require.EqualError(t, groupConnectSidecarValidate(&structs.TaskGroup{ + Name: "g1", + Networks: nil, + }), `Consul Connect sidecars require exactly 1 network, found 0 in group "g1"`) + }) + + t.Run("sidecar non bridge", func(t *testing.T) { + require.EqualError(t, groupConnectSidecarValidate(&structs.TaskGroup{ + Name: "g2", + Networks: structs.Networks{{ + Mode: "host", + }}, + }), `Consul Connect sidecar requires bridge network, found "host" in group "g2"`) + }) + + t.Run("sidecar okay", func(t *testing.T) { + require.NoError(t, groupConnectSidecarValidate(&structs.TaskGroup{ + Name: "g3", + Networks: structs.Networks{{ + Mode: "bridge", + }}, + })) + }) +} + +func TestJobEndpointConnect_groupConnectNativeValidate(t *testing.T) { + t.Run("no task in service", func(t *testing.T) { + require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{ + Name: "g1", + }, &structs.Service{ + Name: "s1", + TaskName: "", + }), `Consul Connect Native service "s1" requires task name`) + }) + + t.Run("no task for service", func(t *testing.T) { + require.EqualError(t, groupConnectNativeValidate(&structs.TaskGroup{ + Name: "g2", + }, &structs.Service{ + Name: "s2", + TaskName: "t1", + }), `Consul Connect Native service "s2" requires undefined task "t1" in group "g2"`) + }) + + t.Run("native okay", func(t *testing.T) { + require.NoError(t, groupConnectNativeValidate(&structs.TaskGroup{ + Name: "g2", + Tasks: []*structs.Task{{Name: "t0"}, {Name: "t1"}, {Name: "t3"}}, + }, &structs.Service{ + Name: "s2", + TaskName: "t1", + })) + }) +} diff --git a/nomad/structs/diff_test.go b/nomad/structs/diff_test.go index 4994d1eb9..29d6b04f6 100644 --- a/nomad/structs/diff_test.go +++ b/nomad/structs/diff_test.go @@ -2558,6 +2558,7 @@ func TestTaskGroupDiff(t *testing.T) { Services: []*Service{ { Name: "foo", + TaskName: "task1", EnableTagOverride: false, Checks: []*ServiceCheck{ { @@ -2573,6 +2574,7 @@ func TestTaskGroupDiff(t *testing.T) { }, }, Connect: &ConsulConnect{ + Native: false, SidecarTask: &SidecarTask{ Name: "sidecar", Driver: "docker", @@ -2592,6 +2594,7 @@ func TestTaskGroupDiff(t *testing.T) { Services: []*Service{ { Name: "foo", + TaskName: "task2", EnableTagOverride: true, Checks: []*ServiceCheck{ { @@ -2609,6 +2612,7 @@ func TestTaskGroupDiff(t *testing.T) { }, }, Connect: &ConsulConnect{ + Native: true, SidecarService: &ConsulSidecarService{ Port: "http", Proxy: &ConsulProxy{ @@ -2661,6 +2665,12 @@ func TestTaskGroupDiff(t *testing.T) { Old: "", New: "", }, + { + Type: DiffTypeEdited, + Name: "TaskName", + Old: "task1", + New: "task2", + }, }, Objects: []*ObjectDiff{ { @@ -2786,10 +2796,10 @@ func TestTaskGroupDiff(t *testing.T) { Name: "ConsulConnect", Fields: []*FieldDiff{ { - Type: DiffTypeNone, + Type: DiffTypeEdited, Name: "Native", - Old: "", - New: "", + Old: "false", + New: "true", }, }, Objects: []*ObjectDiff{ @@ -4726,6 +4736,7 @@ func TestTaskDiff(t *testing.T) { Name: "foo", PortLabel: "bar", AddressMode: "driver", + TaskName: "task1", }, }, }, @@ -4760,6 +4771,12 @@ func TestTaskDiff(t *testing.T) { Old: "foo", New: "bar", }, + { + Type: DiffTypeAdded, + Name: "TaskName", + Old: "", + New: "task1", + }, }, }, }, @@ -4890,6 +4907,10 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeNone, Name: "PortLabel", }, + { + Type: DiffTypeNone, + Name: "TaskName", + }, }, }, }, @@ -4925,6 +4946,14 @@ func TestTaskDiff(t *testing.T) { { Type: DiffTypeAdded, Name: "ConsulConnect", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "Native", + Old: "", + New: "false", + }, + }, Objects: []*ObjectDiff{ { Type: DiffTypeAdded, @@ -4950,9 +4979,10 @@ func TestTaskDiff(t *testing.T) { New: &Task{ Services: []*Service{ { - Name: "foo", + Name: "foo", + TaskName: "task1", Connect: &ConsulConnect{ - Native: "task1", + Native: true, }, }, }, @@ -4963,6 +4993,14 @@ func TestTaskDiff(t *testing.T) { { Type: DiffTypeEdited, Name: "Service", + Fields: []*FieldDiff{ + { + Type: DiffTypeAdded, + Name: "TaskName", + Old: "", + New: "task1", + }, + }, Objects: []*ObjectDiff{ { Type: DiffTypeAdded, @@ -4972,7 +5010,7 @@ func TestTaskDiff(t *testing.T) { Type: DiffTypeAdded, Name: "Native", Old: "", - New: "task1", + New: "true", }, }, }, @@ -5333,6 +5371,12 @@ func TestTaskDiff(t *testing.T) { Old: "", New: "", }, + { + Type: DiffTypeNone, + Name: "TaskName", + Old: "", + New: "", + }, }, Objects: []*ObjectDiff{ { diff --git a/nomad/structs/services.go b/nomad/structs/services.go index d070e9746..8d2436450 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -351,6 +351,12 @@ type Service struct { // as one of the seed values when generating a Consul ServiceID. Name string + // Name of the Task associated with this service. + // + // Currently only used to identify the implementing task of a Consul + // Connect Native enabled service. + TaskName string + // PortLabel is either the numeric port number or the `host:port`. // To specify the port number using the host's Consul Advertise // address, specify an empty host in the PortLabel (e.g. `:port`). @@ -422,8 +428,7 @@ func (s *Service) Canonicalize(job string, taskGroup string, task string) { "TASKGROUP": taskGroup, "TASK": task, "BASE": fmt.Sprintf("%s-%s-%s", job, taskGroup, task), - }, - ) + }) for _, check := range s.Checks { check.Canonicalize(s.Name) @@ -450,6 +455,7 @@ func (s *Service) Validate() error { mErr.Errors = append(mErr.Errors, fmt.Errorf("Service address_mode must be %q, %q, or %q; not %q", AddressModeAuto, AddressModeHost, AddressModeDriver, s.AddressMode)) } + // check checks for _, c := range s.Checks { if s.PortLabel == "" && c.PortLabel == "" && c.RequiresPort() { mErr.Errors = append(mErr.Errors, fmt.Errorf("Check %s invalid: check requires a port but neither check nor service %+q have a port", c.Name, s.Name)) @@ -469,10 +475,16 @@ func (s *Service) Validate() error { } } + // check connect if s.Connect != nil { if err := s.Connect.Validate(); err != nil { mErr.Errors = append(mErr.Errors, err) } + + // if service is connect native, service task must be set + if s.Connect.IsNative() && len(s.TaskName) == 0 { + mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is connect native and requires setting the task", s.Name)) + } } return mErr.ErrorOrNil() @@ -620,13 +632,8 @@ OUTER: // ConsulConnect represents a Consul Connect jobspec stanza. type ConsulConnect struct { - // Native is empty if the service represents a legacy application that - // requires an Envoy (or otherwise configured via SidecarTask) sidecar - // that is managed via xDS by Consul. - // - // If non-empty, Native indicates the Connect-Native Task associated with - // the service definition in which this Connect stanza resides. - Native string + // Native indicates whether the service is Consul Connect Native enabled. + Native bool // SidecarService is non-nil if a service requires a sidecar. SidecarService *ConsulSidecarService @@ -667,7 +674,7 @@ func (c *ConsulConnect) HasSidecar() bool { } func (c *ConsulConnect) IsNative() bool { - return c != nil && c.Native != "" + return c != nil && c.Native } // Validate that the Connect stanza has exactly one of Native or sidecar. diff --git a/nomad/structs/services_test.go b/nomad/structs/services_test.go index 3405998fb..0af3de8c7 100644 --- a/nomad/structs/services_test.go +++ b/nomad/structs/services_test.go @@ -123,16 +123,14 @@ func TestConsulConnect_Validate(t *testing.T) { // An empty Connect stanza is invalid require.Error(t, c.Validate()) - // Native= is valid - c.Native = "foo" + c.Native = true require.NoError(t, c.Validate()) // Native=true + Sidecar!=nil is invalid c.SidecarService = &ConsulSidecarService{} require.Error(t, c.Validate()) - // Native= + Sidecar!=nil is valid - c.Native = "" + c.Native = false require.NoError(t, c.Validate()) } diff --git a/nomad/structs/structs_test.go b/nomad/structs/structs_test.go index 4c0a05fd6..4cfff9e18 100644 --- a/nomad/structs/structs_test.go +++ b/nomad/structs/structs_test.go @@ -842,7 +842,7 @@ func TestTaskGroup_UsesConnect(t *testing.T) { try(t, &TaskGroup{ Services: []*Service{ {Connect: nil}, - {Connect: &ConsulConnect{Native: "foo"}}, + {Connect: &ConsulConnect{Native: true}}, }, }, true) }) @@ -2714,10 +2714,14 @@ func TestService_Validate(t *testing.T) { // Base service should be valid require.NoError(t, s.Validate()) - // Native Connect should be valid + // Native Connect requires task name on service s.Connect = &ConsulConnect{ - Native: "testtask", + Native: true, } + require.Error(t, s.Validate()) + + // Native Connect should work with task name on service set + s.TaskName = "testtask" require.NoError(t, s.Validate()) // Native Connect + Sidecar should be invalid @@ -2763,7 +2767,7 @@ func TestService_Equals(t *testing.T) { o.Checks = []*ServiceCheck{{Name: "diff"}} assertDiff() - o.Connect = &ConsulConnect{Native: "testtask"} + o.Connect = &ConsulConnect{Native: true} assertDiff() o.EnableTagOverride = true diff --git a/vendor/github.com/hashicorp/nomad/api/services.go b/vendor/github.com/hashicorp/nomad/api/services.go index 29a543a79..f0658d47e 100644 --- a/vendor/github.com/hashicorp/nomad/api/services.go +++ b/vendor/github.com/hashicorp/nomad/api/services.go @@ -110,6 +110,7 @@ type Service struct { Connect *ConsulConnect Meta map[string]string CanaryMeta map[string]string + TaskName string `mapstructure:"task"` } // Canonicalize the Service by ensuring its name and address mode are set. Task @@ -140,7 +141,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { // ConsulConnect represents a Consul Connect jobspec stanza. type ConsulConnect struct { - Native string + Native bool SidecarService *ConsulSidecarService `mapstructure:"sidecar_service"` SidecarTask *SidecarTask `mapstructure:"sidecar_task"` } diff --git a/website/pages/docs/job-specification/connect.mdx b/website/pages/docs/job-specification/connect.mdx index df2216389..2ece25182 100644 --- a/website/pages/docs/job-specification/connect.mdx +++ b/website/pages/docs/job-specification/connect.mdx @@ -47,9 +47,11 @@ job "countdash" { ## `connect` Parameters -- `native` - `(string: "")` - If non-empty, this indicates the task represented - by this service, for use with [Connect Native](https://www.consul.io/docs/connect/native) - applications. Incompatible with `sidecar_service` and `sidecar_task`. +- `native` - `(bool: false)` - This is used to configure the service as supporting + [Connect Native](https://www.consul.io/docs/connect/native) applications. If set, + the service definition must provide the name of the implementing task in the + [task][service_task] field. + Incompatible with `sidecar_service` and `sidecar_task`. - `sidecar_service` - ([sidecar_service][]: nil) - This is used to configure the sidecar service injected by Nomad for Consul Connect. Incompatible with `native`. @@ -169,13 +171,19 @@ job "countdash" { ### Using Connect Native -The following example is a minimal connect stanza for a +The following example is a minimal service stanza for a [Consul Connect Native](https://www.consul.io/docs/connect/native) -application with task name `myTask`. +application implemented by a task named `generate`. ```hcl -connect { - native = "myTask" +service { + name = "uuid-api" + port = "${NOMAD_PORT_api}" + task = "generate" + + connect { + native = true + } } ``` @@ -191,3 +199,4 @@ connect { [sidecar_task]: /docs/job-specification/sidecar_task 'Nomad sidecar task config Specification' [upstreams]: /docs/job-specification/upstreams 'Nomad sidecar service upstreams Specification' [native]: https://www.consul.io/docs/connect/native.html +[service_task]: /docs/job-specification/service#task-1 'Nomad service task' diff --git a/website/pages/docs/job-specification/service.mdx b/website/pages/docs/job-specification/service.mdx index 622d3481e..d3148b570 100644 --- a/website/pages/docs/job-specification/service.mdx +++ b/website/pages/docs/job-specification/service.mdx @@ -147,6 +147,11 @@ Connect][connect] integration. - `host` - Use the host IP and port. +- `task` `(string: "")` - Specifies the name of the Nomad Task associated with + this service definition. Only available on group services. Must be set if this + service definition represents a Consul Connect Native service. The Nomad Task + must exist in the same Task Group. + - `meta` ([Meta][]: nil) - Specifies a key-value map that annotates the Consul service with user-defined metadata. From a87130c0e8dd1aa7ce3068a8ba58052d253f9a3c Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 23 Jun 2020 12:06:28 -0500 Subject: [PATCH 3/7] connect/native: give tls files an extension --- client/allocrunner/taskrunner/connect_native_hook.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/allocrunner/taskrunner/connect_native_hook.go b/client/allocrunner/taskrunner/connect_native_hook.go index d5497c16e..347f2d209 100644 --- a/client/allocrunner/taskrunner/connect_native_hook.go +++ b/client/allocrunner/taskrunner/connect_native_hook.go @@ -108,9 +108,9 @@ func (h *connectNativeHook) Prestart( } const ( - secretCAFilename = "consul_ca_file" - secretCertfileFilename = "consul_cert_file" - secretKeyfileFilename = "consul_key_file" + secretCAFilename = "consul_ca_file.pem" + secretCertfileFilename = "consul_cert_file.pem" + secretKeyfileFilename = "consul_key_file.pem" ) func (h *connectNativeHook) copyCertificates(consulConfig consulTransportConfig, dir string) error { From 193e355ec3a95780cbd60d890a719b9d3a68d31a Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Tue, 23 Jun 2020 12:07:35 -0500 Subject: [PATCH 4/7] connect/native: update connect native hook tests --- .../taskrunner/connect_native_hook_test.go | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/client/allocrunner/taskrunner/connect_native_hook_test.go b/client/allocrunner/taskrunner/connect_native_hook_test.go index e19caab76..4ebbea058 100644 --- a/client/allocrunner/taskrunner/connect_native_hook_test.go +++ b/client/allocrunner/taskrunner/connect_native_hook_test.go @@ -148,9 +148,9 @@ func TestConnectNativeHook_tlsEnv(t *testing.T) { result := fullHook.tlsEnv(nil) require.Equal(t, map[string]string{ // ca files are specifically copied into FS namespace - "CONSUL_CACERT": "/secrets/consul_ca_file", - "CONSUL_CLIENT_CERT": "/secrets/consul_cert_file", - "CONSUL_CLIENT_KEY": "/secrets/consul_key_file", + "CONSUL_CACERT": "/secrets/consul_ca_file.pem", + "CONSUL_CLIENT_CERT": "/secrets/consul_cert_file.pem", + "CONSUL_CLIENT_KEY": "/secrets/consul_key_file.pem", "CONSUL_HTTP_SSL": "true", "CONSUL_HTTP_SSL_VERIFY": "true", }, result) @@ -209,9 +209,10 @@ func TestTaskRunner_ConnectNativeHook_Ok(t *testing.T) { alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} tg := alloc.Job.TaskGroups[0] tg.Services = []*structs.Service{{ - Name: "cn-service", + Name: "cn-service", + TaskName: tg.Tasks[0].Name, Connect: &structs.ConsulConnect{ - Native: tg.Tasks[0].Name, + Native: true, }}, } tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") @@ -272,9 +273,10 @@ func TestTaskRunner_ConnectNativeHook_with_SI_token(t *testing.T) { alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} tg := alloc.Job.TaskGroups[0] tg.Services = []*structs.Service{{ - Name: "cn-service", + Name: "cn-service", + TaskName: tg.Tasks[0].Name, Connect: &structs.ConsulConnect{ - Native: tg.Tasks[0].Name, + Native: true, }}, } tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") @@ -347,9 +349,10 @@ func TestTaskRunner_ConnectNativeHook_shareTLS(t *testing.T) { alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} tg := alloc.Job.TaskGroups[0] tg.Services = []*structs.Service{{ - Name: "cn-service", + Name: "cn-service", + TaskName: tg.Tasks[0].Name, Connect: &structs.ConsulConnect{ - Native: tg.Tasks[0].Name, + Native: true, }}, } tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") @@ -403,9 +406,9 @@ func TestTaskRunner_ConnectNativeHook_shareTLS(t *testing.T) { // Assert environment variable for token is set require.NotEmpty(t, response.Env) require.Equal(t, map[string]string{ - "CONSUL_CACERT": "/secrets/consul_ca_file", - "CONSUL_CLIENT_CERT": "/secrets/consul_cert_file", - "CONSUL_CLIENT_KEY": "/secrets/consul_key_file", + "CONSUL_CACERT": "/secrets/consul_ca_file.pem", + "CONSUL_CLIENT_CERT": "/secrets/consul_cert_file.pem", + "CONSUL_CLIENT_KEY": "/secrets/consul_key_file.pem", "CONSUL_HTTP_SSL": "true", "CONSUL_HTTP_SSL_VERIFY": "true", }, response.Env) @@ -462,9 +465,10 @@ func TestTaskRunner_ConnectNativeHook_shareTLS_override(t *testing.T) { alloc.AllocatedResources.Shared.Networks = []*structs.NetworkResource{{Mode: "host", IP: "1.1.1.1"}} tg := alloc.Job.TaskGroups[0] tg.Services = []*structs.Service{{ - Name: "cn-service", + Name: "cn-service", + TaskName: tg.Tasks[0].Name, Connect: &structs.ConsulConnect{ - Native: tg.Tasks[0].Name, + Native: true, }}, } tg.Tasks[0].Kind = structs.NewTaskKind("connect-native", "cn-service") From 3a1129c6b79f6dff69f328e7c88029467ff90dab Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 24 Jun 2020 09:05:56 -0500 Subject: [PATCH 5/7] connect/native: fixup command/agent/consul/connect test cases --- command/agent/consul/connect_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/agent/consul/connect_test.go b/command/agent/consul/connect_test.go index 948e45443..8ce74c1df 100644 --- a/command/agent/consul/connect_test.go +++ b/command/agent/consul/connect_test.go @@ -32,7 +32,7 @@ func TestConnect_newConnect(t *testing.T) { t.Run("native", func(t *testing.T) { asr, err := newConnect("", &structs.ConsulConnect{ - Native: "foo", + Native: true, }, nil) require.NoError(t, err) require.True(t, asr.Native) @@ -41,7 +41,7 @@ func TestConnect_newConnect(t *testing.T) { t.Run("with sidecar", func(t *testing.T) { asr, err := newConnect("redis", &structs.ConsulConnect{ - Native: "", + Native: false, SidecarService: &structs.ConsulSidecarService{ Tags: []string{"foo", "bar"}, Port: "sidecarPort", From cd3ec2f7833ee153b199df89795486e9a72fa427 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 24 Jun 2020 09:16:28 -0500 Subject: [PATCH 6/7] connect/native: check for pre-existing consul token --- client/allocrunner/taskrunner/connect_native_hook.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/allocrunner/taskrunner/connect_native_hook.go b/client/allocrunner/taskrunner/connect_native_hook.go index 347f2d209..df74841ef 100644 --- a/client/allocrunner/taskrunner/connect_native_hook.go +++ b/client/allocrunner/taskrunner/connect_native_hook.go @@ -193,7 +193,7 @@ func (h *connectNativeHook) tlsEnv(env map[string]string) map[string]string { // maybeSetSITokenEnv will set the CONSUL_HTTP_TOKEN environment variable in // the given env map, if the token is found to exist in the task's secrets -// directory. +// directory AND the CONSUL_HTTP_TOKEN environment variable is not already set. // // Following the pattern of the envoy_bootstrap_hook, the Consul Service Identity // ACL Token is generated prior to this hook, if Consul ACLs are enabled. This is @@ -201,6 +201,13 @@ func (h *connectNativeHook) tlsEnv(env map[string]string) map[string]string { // workspace. The content of that file is the SI token specific to this task // instance. func (h *connectNativeHook) maybeSetSITokenEnv(dir, task string, env map[string]string) error { + if _, exists := env["CONSUL_HTTP_TOKEN"]; exists { + // Consul token was already set - typically by using the Vault integration + // and a template stanza to set the environment. Ignore the SI token as + // the configured token takes precedence. + return nil + } + token, err := ioutil.ReadFile(filepath.Join(dir, sidsTokenFile)) if err != nil { if !os.IsNotExist(err) { From ef52328eb47ecb15171bfe88bc57646d05d16038 Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Wed, 24 Jun 2020 10:13:22 -0500 Subject: [PATCH 7/7] connect/native: doc and comment tweaks from PR --- client/allocrunner/taskrunner/connect_native_hook.go | 1 - jobspec/parse_service.go | 2 +- nomad/structs/services.go | 2 +- website/pages/docs/configuration/consul.mdx | 6 +++--- website/pages/docs/job-specification/connect.mdx | 3 ++- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/client/allocrunner/taskrunner/connect_native_hook.go b/client/allocrunner/taskrunner/connect_native_hook.go index df74841ef..8adc61137 100644 --- a/client/allocrunner/taskrunner/connect_native_hook.go +++ b/client/allocrunner/taskrunner/connect_native_hook.go @@ -83,7 +83,6 @@ func (h *connectNativeHook) Prestart( return nil } - h.logger.Debug("share consul TLS configuration for connect native", "enabled", h.consulShareTLS, "task", request.Task.Name) if h.consulShareTLS { // copy TLS certificates if err := h.copyCertificates(h.consulConfig, request.TaskDir.SecretsDir); err != nil { diff --git a/jobspec/parse_service.go b/jobspec/parse_service.go index c0fa09096..f3d56d3d6 100644 --- a/jobspec/parse_service.go +++ b/jobspec/parse_service.go @@ -47,7 +47,7 @@ func parseService(o *ast.ObjectItem) (*api.Service, error) { "address_mode", "check_restart", "connect", - "task", // todo + "task", "meta", "canary_meta", } diff --git a/nomad/structs/services.go b/nomad/structs/services.go index 8d2436450..5367e2731 100644 --- a/nomad/structs/services.go +++ b/nomad/structs/services.go @@ -483,7 +483,7 @@ func (s *Service) Validate() error { // if service is connect native, service task must be set if s.Connect.IsNative() && len(s.TaskName) == 0 { - mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is connect native and requires setting the task", s.Name)) + mErr.Errors = append(mErr.Errors, fmt.Errorf("Service %s is Connect Native and requires setting the task", s.Name)) } } diff --git a/website/pages/docs/configuration/consul.mdx b/website/pages/docs/configuration/consul.mdx index 331619082..09922fb1b 100644 --- a/website/pages/docs/configuration/consul.mdx +++ b/website/pages/docs/configuration/consul.mdx @@ -106,9 +106,9 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul - `share_ssl` `(bool: true)` - Specifies whether the Nomad client should share its Consul SSL configuration with Connect Native applications. Includes values - of `ca_file`, `cert_file`, `key_file`, `auth`, `ssl`, and `verify_ssl`. Does - not include the value for the ACL `token`. This option should be disabled in - an untrusted environment. + of `ca_file`, `cert_file`, `key_file`, `ssl`, and `verify_ssl`. Does not include + the values for the ACL `token` or `auth`. This option should be disabled in + environments where Consul ACLs are not enabled. - `ssl` `(bool: false)` - Specifies if the transport scheme should use HTTPS to communicate with the Consul agent. Will default to the `CONSUL_HTTP_SSL` diff --git a/website/pages/docs/job-specification/connect.mdx b/website/pages/docs/job-specification/connect.mdx index 2ece25182..d4869f140 100644 --- a/website/pages/docs/job-specification/connect.mdx +++ b/website/pages/docs/job-specification/connect.mdx @@ -189,8 +189,9 @@ service { ### Limitations -[Nomad variable interpolation][interpolation] is _not_ yet supported. +[Nomad variable interpolation][interpolation] is _not_ yet supported ([gh-7221]). +[gh-7221]: https://github.com/hashicorp/nomad/issues/7221 [job]: /docs/job-specification/job 'Nomad job Job Specification' [group]: /docs/job-specification/group 'Nomad group Job Specification' [task]: /docs/job-specification/task 'Nomad task Job Specification'