Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consul/connect: implement initial support for connect native #8011

Merged
merged 7 commits into from
Jun 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion client/allocrunner/consulsock_hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
221 changes: 221 additions & 0 deletions client/allocrunner/taskrunner/connect_native_hook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
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
}

if h.consulShareTLS {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember you mentioning this shouldn't be enabled in production. Is it worth dropping a warn line when the hook is created so its emitted at startup?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that it is safe to enable in production when Consul ACLs are enabled. If that's true and clearly documented, I don't think we need to log every time.

// 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.pem"
secretCertfileFilename = "consul_cert_file.pem"
secretKeyfileFilename = "consul_key_file.pem"
)

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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These paths won't work for raw_exec. Dropping the leading slash so that it's relative should work. Not sure if we have any prior art to work from here though. 🤔

We can definitely fix raw_exec later and it shouldn't hold up this PR. Just file a ticket if you think it's a concern but don't have time to verify or fix now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately just dropping the root slash breaks in docker. Will open a follow up ticket to find a way to make everything happy.

}

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 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
// 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 {
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) {
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
}
Loading