Merge pull request #6218 from hashicorp/f-consul-defaults

consul: use Consul's defaults and env vars

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,
 	}
 }
 

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)
+}

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<string>: [])` - 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