From 6a1bdf04c49bab5d07293859df8edbe1fa5f8493 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 27 Aug 2019 11:34:48 -0700 Subject: [PATCH] consul: use Consul's defaults and env vars Use Consul's API package defaults and env vars as Nomad's defaults. --- nomad/structs/config/consul.go | 24 ++++-- nomad/structs/config/consul_test.go | 80 +++++++++++++++++++ .../source/docs/configuration/consul.html.md | 12 ++- 3 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 nomad/structs/config/consul_test.go diff --git a/nomad/structs/config/consul.go b/nomad/structs/config/consul.go index 3660a94e8..9da4f08ad 100644 --- a/nomad/structs/config/consul.go +++ b/nomad/structs/config/consul.go @@ -56,7 +56,9 @@ type ConsulConfig struct { // address instead of bind address ChecksUseAdvertise *bool `hcl:"checks_use_advertise"` - // Addr is the address of the local Consul agent + // Addr is the HTTP endpoint address of the local Consul agent + // + // Uses Consul's default and env var. Addr string `hcl:"address"` // Timeout is used by Consul HTTP Client @@ -71,13 +73,19 @@ type ConsulConfig struct { Auth string `hcl:"auth"` // EnableSSL sets the transport scheme to talk to the Consul agent as https + // + // Uses Consul's default and env var. EnableSSL *bool `hcl:"ssl"` // VerifySSL enables or disables SSL verification when the transport scheme // for the consul api client is https + // + // Uses Consul's default and env var. VerifySSL *bool `hcl:"verify_ssl"` - // CAFile is the path to the ca certificate used for Consul communication + // CAFile is the path to the ca certificate used for Consul communication. + // + // Uses Consul's default and env var. CAFile string `hcl:"ca_file"` // CertFile is the path to the certificate for Consul communication @@ -99,8 +107,10 @@ type ConsulConfig struct { } // DefaultConsulConfig() returns the canonical defaults for the Nomad -// `consul` configuration. +// `consul` configuration. Uses Consul's default configuration which reads +// environment variables. func DefaultConsulConfig() *ConsulConfig { + def := consul.DefaultConfig() return &ConsulConfig{ ServerServiceName: "nomad", ServerHTTPCheckName: "Nomad Server HTTP Check", @@ -110,11 +120,15 @@ func DefaultConsulConfig() *ConsulConfig { ClientHTTPCheckName: "Nomad Client HTTP Check", AutoAdvertise: helper.BoolToPtr(true), ChecksUseAdvertise: helper.BoolToPtr(false), - EnableSSL: helper.BoolToPtr(false), - VerifySSL: helper.BoolToPtr(true), ServerAutoJoin: helper.BoolToPtr(true), ClientAutoJoin: helper.BoolToPtr(true), Timeout: 5 * time.Second, + + // From Consul api package defaults + Addr: def.Address, + EnableSSL: helper.BoolToPtr(def.Scheme == "https"), + VerifySSL: helper.BoolToPtr(!def.TLSConfig.InsecureSkipVerify), + CAFile: def.TLSConfig.CAFile, } } diff --git a/nomad/structs/config/consul_test.go b/nomad/structs/config/consul_test.go new file mode 100644 index 000000000..afaa86619 --- /dev/null +++ b/nomad/structs/config/consul_test.go @@ -0,0 +1,80 @@ +package config + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "testing" + + consulapi "github.com/hashicorp/consul/api" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMain(m *testing.M) { + if os.Getenv("NOMAD_ENV_TEST") != "1" { + os.Exit(m.Run()) + } + + // Encode the default config as json to stdout for testing env var + // handling. + if err := json.NewEncoder(os.Stdout).Encode(DefaultConsulConfig()); err != nil { + fmt.Fprintf(os.Stderr, "error encoding config: %v", err) + os.Exit(2) + } + + os.Exit(0) +} + +// TestConsulConfig_Defaults asserts Consul defaults are copied from their +// upstream API package defaults. +func TestConsulConfig_Defaults(t *testing.T) { + t.Parallel() + + nomadDef := DefaultConsulConfig() + consulDef := consulapi.DefaultConfig() + + require.Equal(t, consulDef.Address, nomadDef.Addr) + require.NotZero(t, nomadDef.Addr) + require.Equal(t, consulDef.Scheme == "https", *nomadDef.EnableSSL) + require.Equal(t, !consulDef.TLSConfig.InsecureSkipVerify, *nomadDef.VerifySSL) + require.Equal(t, consulDef.TLSConfig.CAFile, nomadDef.CAFile) +} + +// TestConsulConfig_Exec asserts Consul defaults use env vars when they are +// set by forking a subprocess. +func TestConsulConfig_Exec(t *testing.T) { + t.Parallel() + + self, err := os.Executable() + if err != nil { + t.Fatalf("error finding test binary: %v", err) + } + + cmd := exec.Command(self) + cmd.Env = []string{ + "NOMAD_ENV_TEST=1", + "CONSUL_CACERT=cacert", + "CONSUL_HTTP_ADDR=addr", + "CONSUL_HTTP_SSL=1", + "CONSUL_HTTP_SSL_VERIFY=1", + } + + out, err := cmd.Output() + if err != nil { + if eerr, ok := err.(*exec.ExitError); ok { + t.Fatalf("exit error code %d; output:\n%s", eerr.ExitCode(), string(eerr.Stderr)) + } + t.Fatalf("error running command %q: %v", self, err) + } + + conf := ConsulConfig{} + require.NoError(t, json.Unmarshal(out, &conf)) + assert.Equal(t, "cacert", conf.CAFile) + assert.Equal(t, "addr", conf.Addr) + require.NotNil(t, conf.EnableSSL) + assert.True(t, *conf.EnableSSL) + require.NotNil(t, conf.VerifySSL) + assert.True(t, *conf.VerifySSL) +} diff --git a/website/source/docs/configuration/consul.html.md b/website/source/docs/configuration/consul.html.md index 274c55e70..1fd8a9dec 100644 --- a/website/source/docs/configuration/consul.html.md +++ b/website/source/docs/configuration/consul.html.md @@ -51,7 +51,8 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul - `address` `(string: "127.0.0.1:8500")` - Specifies the address to the local Consul agent, given in the format `host:port`. Supports Unix sockets with the - format: `unix:///tmp/consul/consul.sock` + format: `unix:///tmp/consul/consul.sock`. Will default to the + `CONSUL_HTTP_ADDR` environment variable if set. - `auth` `(string: "")` - Specifies the HTTP Basic Authentication information to use for access to the Consul Agent, given in the format `username:password`. @@ -64,7 +65,7 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul - `ca_file` `(string: "")` - Specifies an optional path to the CA certificate used for Consul communication. This defaults to the system bundle if - unspecified. + unspecified. Will default to the `CONSUL_CACERT` environment variable if set. - `cert_file` `(string: "")` - Specifies the path to the certificate used for Consul communication. If this is set then you need to also set `key_file`. @@ -106,7 +107,8 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul only happens if the server does not have a leader. - `ssl` `(bool: false)` - Specifies if the transport scheme should use HTTPS to - communicate with the Consul agent. + communicate with the Consul agent. Will default to the `CONSUL_HTTP_SSL` + environment variable if set. - `tags` `(array: [])` - Specifies optional Consul tags to be registered with the Nomad server and agent services. @@ -117,7 +119,9 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul which may or may not allow writes. - `verify_ssl` `(bool: true)`- Specifies if SSL peer verification should be used - when communicating to the Consul API client over HTTPS + when communicating to the Consul API client over HTTPS. Will default to the + `CONSUL_HTTP_SSL_VERIFY` environment variable if set. + If the local Consul agent is configured and accessible by the Nomad agents, the