Merge pull request #9251 from hashicorp/dnephin/config-cleanup-flags-and-opts

config: move testing shims and cleanup some defaults

agent/config/builder.go

@@ -92,8 +92,7 @@ func Load(opts BuilderOpts, extraHead Source, overrides ...Source) (*RuntimeConf
 // since not all pre-conditions have to be satisfied when performing
 // syntactical tests.
 type Builder struct {
-	// devMode stores the value of the -dev flag, and enables development mode.
-	devMode *bool
+	opts BuilderOpts
 
 	// Head, Sources, and Tail are used to manage the order of the
 	// config sources, as described in the comments above.
@@ -105,15 +104,6 @@ type Builder struct {
 	// parsing the configuration.
 	Warnings []string
 
-	// hostname is a shim for testing, allowing tests to specify a replacement
-	// for os.Hostname.
-	hostname func() (string, error)
-
-	// getPrivateIPv4 and getPublicIPv6 are shims for testing, allowing tests to
-	// specify a replacement for ipaddr.GetPrivateIPv4 and ipaddr.GetPublicIPv6.
-	getPrivateIPv4 func() ([]*net.IPAddr, error)
-	getPublicIPv6  func() ([]*net.IPAddr, error)
-
 	// err contains the first error that occurred during
 	// building the runtime configuration.
 	err error
@@ -135,8 +125,8 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) {
 	}
 
 	b := &Builder{
-		devMode: opts.DevMode,
-		Head:    []Source{DefaultSource(), DefaultEnterpriseSource()},
+		opts: opts,
+		Head: []Source{DefaultSource(), DefaultEnterpriseSource()},
 	}
 
 	if b.boolVal(opts.DevMode) {
@@ -165,7 +155,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) {
 			Data:   s,
 		})
 	}
-	b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), DefaultVersionSource())
+	b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), defaultVersionSource())
 	if b.boolVal(opts.DevMode) {
 		b.Tail = append(b.Tail, DevConsulSource())
 	}
@@ -467,14 +457,14 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		switch {
 		case ipaddr.IsAnyV4(advertiseAddr):
 			addrtyp = "private IPv4"
-			detect = b.getPrivateIPv4
+			detect = b.opts.getPrivateIPv4
 			if detect == nil {
 				detect = ipaddr.GetPrivateIPv4
 			}
 
 		case ipaddr.IsAnyV6(advertiseAddr):
 			addrtyp = "public IPv6"
-			detect = b.getPublicIPv6
+			detect = b.opts.getPublicIPv6
 			if detect == nil {
 				detect = ipaddr.GetPublicIPv6
 			}
@@ -995,7 +985,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		DataDir:                                dataDir,
 		Datacenter:                             datacenter,
 		DefaultQueryTime:                       b.durationVal("default_query_time", c.DefaultQueryTime),
-		DevMode:                                b.boolVal(b.devMode),
+		DevMode:                                b.boolVal(b.opts.DevMode),
 		DisableAnonymousSignature:              b.boolVal(c.DisableAnonymousSignature),
 		DisableCoordinates:                     b.boolVal(c.DisableCoordinates),
 		DisableHostNodeID:                      b.boolVal(c.DisableHostNodeID),
@@ -1901,7 +1891,7 @@ func (b *Builder) tlsCipherSuites(name string, v *string) []uint16 {
 func (b *Builder) nodeName(v *string) string {
 	nodeName := b.stringVal(v)
 	if nodeName == "" {
-		fn := b.hostname
+		fn := b.opts.hostname
 		if fn == nil {
 			fn = os.Hostname
 		}

agent/config/builder_test.go

@@ -172,13 +172,13 @@ func TestBuilder_BuildAndValidate_NodeName(t *testing.T) {
 }
 
 func patchBuilderShims(b *Builder) {
-	b.hostname = func() (string, error) {
+	b.opts.hostname = func() (string, error) {
 		return "thehostname", nil
 	}
-	b.getPrivateIPv4 = func() ([]*net.IPAddr, error) {
+	b.opts.getPrivateIPv4 = func() ([]*net.IPAddr, error) {
 		return []*net.IPAddr{ipAddr("10.0.0.1")}, nil
 	}
-	b.getPublicIPv6 = func() ([]*net.IPAddr, error) {
+	b.opts.getPublicIPv6 = func() ([]*net.IPAddr, error) {
 		return []*net.IPAddr{ipAddr("dead:beef::1")}, nil
 	}
 }

agent/config/config.go

@@ -10,11 +10,6 @@ import (
 	"github.com/hashicorp/consul/lib/decode"
 )
 
-const (
-	SerfLANKeyring = "serf/local.keyring"
-	SerfWANKeyring = "serf/remote.keyring"
-)
-
 // Source parses configuration from some source.
 type Source interface {
 	// Source returns an identifier for the Source that can be used in error message

agent/config/default.go

@@ -1,13 +1,13 @@
 package config
 
 import (
-	"fmt"
 	"strconv"
 
+	"github.com/hashicorp/raft"
+
 	"github.com/hashicorp/consul/agent/checks"
 	"github.com/hashicorp/consul/agent/consul"
 	"github.com/hashicorp/consul/version"
-	"github.com/hashicorp/raft"
 )
 
 // DefaultSource is the default agent configuration.
@@ -205,22 +205,24 @@ func NonUserSource() Source {
 	}
 }
 
-// VersionSource creates a config source for the version parameters.
+// versionSource creates a config source for the version parameters.
 // This should be merged in the tail since these values are not
 // user configurable.
-// TODO: return a LiteralSource (no decoding) instead of a FileSource
-func VersionSource(rev, ver, verPre string) Source {
-	return FileSource{
-		Name:   "version",
-		Format: "hcl",
-		Data:   fmt.Sprintf(`revision = %q version = %q version_prerelease = %q`, rev, ver, verPre),
+func versionSource(rev, ver, verPre string) Source {
+	return LiteralSource{
+		Name: "version",
+		Config: Config{
+			Revision:          &rev,
+			Version:           &ver,
+			VersionPrerelease: &verPre,
+		},
 	}
 }
 
-// DefaultVersionSource returns the version config source for the embedded
+// defaultVersionSource returns the version config source for the embedded
 // version numbers.
-func DefaultVersionSource() Source {
-	return VersionSource(version.GitCommit, version.Version, version.VersionPrerelease)
+func defaultVersionSource() Source {
+	return versionSource(version.GitCommit, version.Version, version.VersionPrerelease)
 }
 
 // DefaultConsulSource returns the default configuration for the consul agent.
@@ -254,27 +256,18 @@ func DefaultConsulSource() Source {
 
 // DevConsulSource returns the consul agent configuration for the dev mode.
 // This should be merged in the tail after the DefaultConsulSource.
-// TODO: return a LiteralSource (no decoding) instead of a FileSource
 func DevConsulSource() Source {
-	return FileSource{
-		Name:   "consul-dev",
-		Format: "hcl",
-		Data: `
-		consul = {
-			coordinate = {
-				update_period = "100ms"
-			}
-			raft = {
-				election_timeout = "52ms"
-				heartbeat_timeout = "35ms"
-				leader_lease_timeout = "20ms"
-			}
-			server = {
-				health_interval = "10ms"
-			}
-		}
-	`,
-	}
+	c := Config{}
+	c.Consul.Coordinate.UpdatePeriod = strPtr("100ms")
+	c.Consul.Raft.ElectionTimeout = strPtr("52ms")
+	c.Consul.Raft.HeartbeatTimeout = strPtr("35ms")
+	c.Consul.Raft.LeaderLeaseTimeout = strPtr("20ms")
+	c.Consul.Server.HealthInterval = strPtr("10ms")
+	return LiteralSource{Name: "consul-dev", Config: c}
+}
+
+func strPtr(v string) *string {
+	return &v
 }
 
 func DefaultRuntimeConfig(hcl string) *RuntimeConfig {

agent/config/default_oss.go

@@ -5,22 +5,12 @@ package config
 // DefaultEnterpriseSource returns the consul agent configuration for enterprise mode.
 // These can be overridden by the user and therefore this source should be merged in the
 // head and processed before user configuration.
-// TODO: return a LiteralSource (no decoding) instead of a FileSource
 func DefaultEnterpriseSource() Source {
-	return FileSource{
-		Name:   "enterprise-defaults",
-		Format: "hcl",
-		Data:   ``,
-	}
+	return LiteralSource{Name: "enterprise-defaults"}
 }
 
 // OverrideEnterpriseSource returns the consul agent configuration for the enterprise mode.
 // This should be merged in the tail after the DefaultConsulSource.
-// TODO: return a LiteralSource (no decoding) instead of a FileSource
 func OverrideEnterpriseSource() Source {
-	return FileSource{
-		Name:   "enterprise-overrides",
-		Format: "hcl",
-		Data:   ``,
-	}
+	return LiteralSource{Name: "enterprise-overrides"}
 }

agent/config/flags.go

@@ -3,6 +3,7 @@ package config
 import (
 	"flag"
 	"fmt"
+	"net"
 	"time"
 )
 
@@ -26,6 +27,15 @@ type BuilderOpts struct {
 
 	// HCL contains an arbitrary config in hcl format.
 	HCL []string
+
+	// hostname is a shim for testing, allowing tests to specify a replacement
+	// for os.Hostname.
+	hostname func() (string, error)
+
+	// getPrivateIPv4 and getPublicIPv6 are shims for testing, allowing tests to
+	// specify a replacement for ipaddr.GetPrivateIPv4 and ipaddr.GetPublicIPv6.
+	getPrivateIPv4 func() ([]*net.IPAddr, error)
+	getPublicIPv6  func() ([]*net.IPAddr, error)
 }
 
 // AddFlags adds the command line flags for the agent.

agent/config/runtime_test.go

@@ -4890,13 +4890,13 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) {
 
 				patchBuilderShims(b)
 				if tt.hostname != nil {
-					b.hostname = tt.hostname
+					b.opts.hostname = tt.hostname
 				}
 				if tt.privatev4 != nil {
-					b.getPrivateIPv4 = tt.privatev4
+					b.opts.getPrivateIPv4 = tt.privatev4
 				}
 				if tt.publicv6 != nil {
-					b.getPublicIPv6 = tt.publicv6
+					b.opts.getPublicIPv6 = tt.publicv6
 				}
 
 				// read the source fragements
@@ -4941,9 +4941,7 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) {
 				if err != nil {
 					t.Fatal(err)
 				}
-				x.hostname = b.hostname
-				x.getPrivateIPv4 = func() ([]*net.IPAddr, error) { return []*net.IPAddr{ipAddr("10.0.0.1")}, nil }
-				x.getPublicIPv6 = func() ([]*net.IPAddr, error) { return []*net.IPAddr{ipAddr("dead:beef::1")}, nil }
+				patchBuilderShims(x)
 				expected, err := x.Build()
 				if err != nil {
 					t.Fatalf("build default failed: %s", err)
@@ -7212,7 +7210,7 @@ func TestFullConfig(t *testing.T) {
 			}
 			b.Sources = append(b.Sources, FileSource{Name: "full." + format, Data: data, Format: format})
 			b.Tail = append(b.Tail, tail[format]...)
-			b.Tail = append(b.Tail, VersionSource("JNtPSav3", "R909Hblt", "ZT1JOQLn"))
+			b.Tail = append(b.Tail, versionSource("JNtPSav3", "R909Hblt", "ZT1JOQLn"))
 
 			// construct the runtime config
 			rt, err := b.Build()