Merge pull request #7762 from hashicorp/dnephin/warn-on-unknown-service-file

config: warn if a config file is being skipped because of its file extension

agent/config/agent_limits_test.go

@@ -14,7 +14,7 @@ func TestBuildAndValidate_HTTPMaxConnsPerClientExceedsRLimit(t *testing.T) {
 			# This value is more than max on Windows as well
 			http_max_conns_per_client = 16777217
 		}`
-	b, err := NewBuilder(Flags{})
+	b, err := NewBuilder(BuilderOpts{})
 	assert.NoError(t, err)
 	testsrc := Source{
 		Name:   "test",

agent/config/builder.go

@@ -58,8 +58,8 @@ import (
 // since not all pre-conditions have to be satisfied when performing
 // syntactical tests.
 type Builder struct {
-	// Flags contains the parsed command line arguments.
+	// options contains the input values used to construct a RuntimeConfig
-	Flags Flags
+	options BuilderOpts
 
 	// Head, Sources, and Tail are used to manage the order of the
 	// config sources, as described in the comments above.
@@ -85,15 +85,8 @@ type Builder struct {
 	err error
 }
 
-// NewBuilder returns a new configuration builder based on the given command
+// NewBuilder returns a new configuration Builder from the BuilderOpts.
-// line flags.
+func NewBuilder(opts BuilderOpts) (*Builder, error) {
-func NewBuilder(flags Flags) (*Builder, error) {
-	// We expect all flags to be parsed and flags.Args to be empty.
-	// Therefore, we bail if we find unparsed args.
-	if len(flags.Args) > 0 {
-		return nil, fmt.Errorf("config: Unknown extra arguments: %v", flags.Args)
-	}
-
 	newSource := func(name string, v interface{}) Source {
 		b, err := json.MarshalIndent(v, "", "    ")
 		if err != nil {
@@ -103,11 +96,11 @@ func NewBuilder(flags Flags) (*Builder, error) {
 	}
 
 	b := &Builder{
-		Flags: flags,
+		options: opts,
 		Head:    []Source{DefaultSource(), DefaultEnterpriseSource()},
 	}
 
-	if b.boolVal(b.Flags.DevMode) {
+	if b.boolVal(opts.DevMode) {
 		b.Head = append(b.Head, DevSource())
 	}
 
@@ -116,17 +109,17 @@ func NewBuilder(flags Flags) (*Builder, error) {
 	// we need to merge all slice values defined in flags before we
 	// merge the config files since the flag values for slices are
 	// otherwise appended instead of prepended.
-	slices, values := b.splitSlicesAndValues(b.Flags.Config)
+	slices, values := b.splitSlicesAndValues(opts.Config)
 	b.Head = append(b.Head, newSource("flags.slices", slices))
-	for _, path := range b.Flags.ConfigFiles {
+	for _, path := range opts.ConfigFiles {
-		sources, err := b.ReadPath(path)
+		sources, err := b.sourcesFromPath(path, opts.ConfigFormat)
 		if err != nil {
 			return nil, err
 		}
 		b.Sources = append(b.Sources, sources...)
 	}
 	b.Tail = append(b.Tail, newSource("flags.values", values))
-	for i, s := range b.Flags.HCL {
+	for i, s := range opts.HCL {
 		b.Tail = append(b.Tail, Source{
 			Name:   fmt.Sprintf("flags-%d.hcl", i),
 			Format: "hcl",
@@ -134,16 +127,16 @@ func NewBuilder(flags Flags) (*Builder, error) {
 		})
 	}
 	b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), DefaultVersionSource())
-	if b.boolVal(b.Flags.DevMode) {
+	if b.boolVal(opts.DevMode) {
 		b.Tail = append(b.Tail, DevConsulSource())
 	}
 	return b, nil
 }
 
-// ReadPath reads a single config file or all files in a directory (but
+// sourcesFromPath reads a single config file or all files in a directory (but
-// not its sub-directories) and appends them to the list of config
+// not its sub-directories) and returns Sources created from the
-// sources.
+// files.
-func (b *Builder) ReadPath(path string) ([]Source, error) {
+func (b *Builder) sourcesFromPath(path string, format string) ([]Source, error) {
 	f, err := os.Open(path)
 	if err != nil {
 		return nil, fmt.Errorf("config: Open failed on %s. %s", path, err)
@@ -156,7 +149,12 @@ func (b *Builder) ReadPath(path string) ([]Source, error) {
 	}
 
 	if !fi.IsDir() {
-		src, err := b.ReadFile(path)
+		if !shouldParseFile(path, format) {
+			b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", path)
+			return nil, nil
+		}
+
+		src, err := newSourceFromFile(path, format)
 		if err != nil {
 			return nil, err
 		}
@@ -191,37 +189,46 @@ func (b *Builder) ReadPath(path string) ([]Source, error) {
 			continue
 		}
 
-		if b.shouldParseFile(fp) {
+		if !shouldParseFile(fp, format) {
-			src, err := b.ReadFile(fp)
+			b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", fp)
+			continue
+		}
+		src, err := newSourceFromFile(fp, format)
 		if err != nil {
 			return nil, err
 		}
 		sources = append(sources, src)
 	}
-	}
 	return sources, nil
 }
 
-// ReadFile parses a JSON or HCL config file and appends it to the list of
+// newSourceFromFile creates a Source from the contents of the file at path.
-// config sources.
+func newSourceFromFile(path string, format string) (Source, error) {
-func (b *Builder) ReadFile(path string) (Source, error) {
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
-		return Source{}, fmt.Errorf("config: ReadFile failed on %s: %s", path, err)
+		return Source{}, fmt.Errorf("config: failed to read %s: %s", path, err)
 	}
-	return Source{Name: path, Data: string(data)}, nil
+	if format == "" {
+		format = formatFromFileExtension(path)
+	}
+	return Source{Name: path, Data: string(data), Format: format}, nil
 }
 
 // shouldParse file determines whether the file to be read is of a supported extension
-func (b *Builder) shouldParseFile(path string) bool {
+func shouldParseFile(path string, configFormat string) bool {
-	configFormat := b.stringVal(b.Flags.ConfigFormat)
+	srcFormat := formatFromFileExtension(path)
-	srcFormat := FormatFrom(path)
+	return configFormat != "" || srcFormat == "hcl" || srcFormat == "json"
+}
 
-	// If config-format is not set, only read files with supported extensions
+func formatFromFileExtension(name string) string {
-	if configFormat == "" && srcFormat != "hcl" && srcFormat != "json" {
+	switch {
-		return false
+	case strings.HasSuffix(name, ".json"):
+		return "json"
+	case strings.HasSuffix(name, ".hcl"):
+		return "hcl"
+	default:
+		return ""
 	}
-	return true
 }
 
 type byName []os.FileInfo
@@ -248,13 +255,8 @@ func (b *Builder) BuildAndValidate() (RuntimeConfig, error) {
 // warnings can still contain deprecation or format warnings that should
 // be presented to the user.
 func (b *Builder) Build() (rt RuntimeConfig, err error) {
-	b.err = nil
+	// TODO: move to NewBuilder to remove Builder.options field
-	b.Warnings = nil
+	configFormat := b.options.ConfigFormat
-
-	// ----------------------------------------------------------------
-	// merge config sources as follows
-	//
-	configFormat := b.stringVal(b.Flags.ConfigFormat)
 	if configFormat != "" && configFormat != "json" && configFormat != "hcl" {
 		return RuntimeConfig{}, fmt.Errorf("config: -config-format must be either 'hcl' or 'json'")
 	}
@@ -262,19 +264,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 	// build the list of config sources
 	var srcs []Source
 	srcs = append(srcs, b.Head...)
-	for _, src := range b.Sources {
+	srcs = append(srcs, b.Sources...)
-		// skip file if it should not be parsed
-		if !b.shouldParseFile(src.Name) {
-			continue
-		}
-
-		// if config-format is set, files of any extension will be interpreted in that format
-		src.Format = FormatFrom(src.Name)
-		if configFormat != "" {
-			src.Format = configFormat
-		}
-		srcs = append(srcs, src)
-	}
 	srcs = append(srcs, b.Tail...)
 
 	// parse the config sources into a configuration
@@ -921,7 +911,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		DataDir:                                b.stringVal(c.DataDir),
 		Datacenter:                             datacenter,
 		DefaultQueryTime:                       b.durationVal("default_query_time", c.DefaultQueryTime),
-		DevMode:                                b.boolVal(b.Flags.DevMode),
+		DevMode:                                b.boolVal(b.options.DevMode),
 		DisableAnonymousSignature:              b.boolVal(c.DisableAnonymousSignature),
 		DisableCoordinates:                     b.boolVal(c.DisableCoordinates),
 		DisableHostNodeID:                      b.boolVal(c.DisableHostNodeID),

agent/config/builder_test.go

@@ -0,0 +1,94 @@
+package config
+
+import (
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestShouldParseFile(t *testing.T) {
+	var testcases = []struct {
+		filename     string
+		configFormat string
+		expected     bool
+	}{
+		{filename: "config.json", expected: true},
+		{filename: "config.hcl", expected: true},
+		{filename: "config", configFormat: "hcl", expected: true},
+		{filename: "config.js", configFormat: "json", expected: true},
+		{filename: "config.yaml", expected: false},
+	}
+
+	for _, tc := range testcases {
+		name := fmt.Sprintf("filename=%s, format=%s", tc.filename, tc.configFormat)
+		t.Run(name, func(t *testing.T) {
+			require.Equal(t, tc.expected, shouldParseFile(tc.filename, tc.configFormat))
+		})
+	}
+}
+
+func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) {
+	paths := setupConfigFiles(t)
+
+	b, err := NewBuilder(BuilderOpts{ConfigFiles: paths})
+	require.NoError(t, err)
+
+	expected := []Source{
+		{Name: paths[0], Format: "hcl", Data: "content a"},
+		{Name: paths[1], Format: "json", Data: "content b"},
+		{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
+		{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
+	}
+	require.Equal(t, expected, b.Sources)
+	require.Len(t, b.Warnings, 2)
+}
+
+func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.T) {
+	paths := setupConfigFiles(t)
+
+	b, err := NewBuilder(BuilderOpts{ConfigFiles: paths, ConfigFormat: "hcl"})
+	require.NoError(t, err)
+
+	expected := []Source{
+		{Name: paths[0], Format: "hcl", Data: "content a"},
+		{Name: paths[1], Format: "hcl", Data: "content b"},
+		{Name: paths[2], Format: "hcl", Data: "content c"},
+		{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
+		{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b"},
+		{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c"},
+	}
+	require.Equal(t, expected, b.Sources)
+}
+
+// TODO: this would be much nicer with gotest.tools/fs
+func setupConfigFiles(t *testing.T) []string {
+	t.Helper()
+	path, err := ioutil.TempDir("", t.Name())
+	require.NoError(t, err)
+	t.Cleanup(func() { os.RemoveAll(path) })
+
+	subpath := filepath.Join(path, "sub")
+	err = os.Mkdir(subpath, 0755)
+	require.NoError(t, err)
+
+	for _, dir := range []string{path, subpath} {
+		err = ioutil.WriteFile(filepath.Join(dir, "a.hcl"), []byte("content a"), 0644)
+		require.NoError(t, err)
+
+		err = ioutil.WriteFile(filepath.Join(dir, "b.json"), []byte("content b"), 0644)
+		require.NoError(t, err)
+
+		err = ioutil.WriteFile(filepath.Join(dir, "c.yaml"), []byte("content c"), 0644)
+		require.NoError(t, err)
+	}
+	return []string{
+		filepath.Join(path, "a.hcl"),
+		filepath.Join(path, "b.json"),
+		filepath.Join(path, "c.yaml"),
+		subpath,
+	}
+}

agent/config/config.go

@@ -3,7 +3,6 @@ package config
 import (
 	"encoding/json"
 	"fmt"
-	"strings"
 
 	"github.com/hashicorp/consul/lib/decode"
 	"github.com/hashicorp/hcl"
@@ -21,17 +20,6 @@ type Source struct {
 	Data   string
 }
 
-func FormatFrom(name string) string {
-	switch {
-	case strings.HasSuffix(name, ".json"):
-		return "json"
-	case strings.HasSuffix(name, ".hcl"):
-		return "hcl"
-	default:
-		return ""
-	}
-}
-
 // Parse parses a config fragment in either JSON or HCL format.
 func Parse(data string, format string) (c Config, md mapstructure.Metadata, err error) {
 	var raw map[string]interface{}

agent/config/default.go

@@ -264,7 +264,7 @@ func DevConsulSource() Source {
 }
 
 func DefaultRuntimeConfig(hcl string) *RuntimeConfig {
-	b, err := NewBuilder(Flags{HCL: []string{hcl}})
+	b, err := NewBuilder(BuilderOpts{HCL: []string{hcl}})
 	if err != nil {
 		panic(err)
 	}

agent/config/flags.go

@@ -6,8 +6,8 @@ import (
 	"time"
 )
 
-// Flags defines the command line flags.
+// BuilderOpts used by Builder to construct and validate a RuntimeConfig.
-type Flags struct {
+type BuilderOpts struct {
 	// Config contains the command line arguments that can also be set
 	// in a config file.
 	Config Config
@@ -18,21 +18,18 @@ type Flags struct {
 
 	// ConfigFormat forces all config files to be interpreted as this
 	// format independent of their extension.
-	ConfigFormat *string
+	ConfigFormat string
 
-	// HCL contains an arbitrary config in hcl format.
 	// DevMode indicates whether the agent should be started in development
 	// mode. This cannot be configured in a config file.
 	DevMode *bool
 
+	// HCL contains an arbitrary config in hcl format.
 	HCL []string
-
-	// Args contains the remaining unparsed flags.
-	Args []string
 }
 
 // AddFlags adds the command line flags for the agent.
-func AddFlags(fs *flag.FlagSet, f *Flags) {
+func AddFlags(fs *flag.FlagSet, f *BuilderOpts) {
 	add := func(p interface{}, name, help string) {
 		switch x := p.(type) {
 		case **bool:
@@ -63,7 +60,7 @@ func AddFlags(fs *flag.FlagSet, f *Flags) {
 	add(&f.Config.CheckOutputMaxSize, "check_output_max_size", "Sets the maximum output size for checks on this agent")
 	add(&f.ConfigFiles, "config-dir", "Path to a directory to read configuration files from. This will read every file ending in '.json' as configuration in this directory in alphabetical order. Can be specified multiple times.")
 	add(&f.ConfigFiles, "config-file", "Path to a file in JSON or HCL format with a matching file extension. Can be specified multiple times.")
-	add(&f.ConfigFormat, "config-format", "Config files are in this format irrespective of their extension. Must be 'hcl' or 'json'")
+	fs.StringVar(&f.ConfigFormat, "config-format", "", "Config files are in this format irrespective of their extension. Must be 'hcl' or 'json'")
 	add(&f.Config.DataDir, "data-dir", "Path to a data directory to store agent state.")
 	add(&f.Config.Datacenter, "datacenter", "Datacenter of the agent.")
 	add(&f.Config.DefaultQueryTime, "default-query-time", "the amount of time a blocking query will wait before Consul will force a response. This value can be overridden by the 'wait' query parameter.")

agent/config/flags_test.go

@@ -2,91 +2,91 @@ package config
 
 import (
 	"flag"
-	"reflect"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 )
 
-// TestParseFlags tests whether command line flags are properly parsed
+// TestAddFlags_WithParse tests whether command line flags are properly parsed
 // into the Flags/File structure. It contains an example for every type
 // that is parsed. It does not test the conversion into the final
 // runtime configuration. See TestConfig for that.
-func TestParseFlags(t *testing.T) {
+func TestAddFlags_WithParse(t *testing.T) {
 	tests := []struct {
 		args     []string
-		flags Flags
+		expected BuilderOpts
-		err   error
+		extra    []string
 	}{
 		{},
 		{
 			args:     []string{`-bind`, `a`},
-			flags: Flags{Config: Config{BindAddr: pString("a")}},
+			expected: BuilderOpts{Config: Config{BindAddr: pString("a")}},
 		},
 		{
 			args:     []string{`-bootstrap`},
-			flags: Flags{Config: Config{Bootstrap: pBool(true)}},
+			expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}},
 		},
 		{
 			args:     []string{`-bootstrap=true`},
-			flags: Flags{Config: Config{Bootstrap: pBool(true)}},
+			expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}},
 		},
 		{
 			args:     []string{`-bootstrap=false`},
-			flags: Flags{Config: Config{Bootstrap: pBool(false)}},
+			expected: BuilderOpts{Config: Config{Bootstrap: pBool(false)}},
 		},
 		{
 			args:     []string{`-config-file`, `a`, `-config-dir`, `b`, `-config-file`, `c`, `-config-dir`, `d`},
-			flags: Flags{ConfigFiles: []string{"a", "b", "c", "d"}},
+			expected: BuilderOpts{ConfigFiles: []string{"a", "b", "c", "d"}},
 		},
 		{
 			args:     []string{`-datacenter`, `a`},
-			flags: Flags{Config: Config{Datacenter: pString("a")}},
+			expected: BuilderOpts{Config: Config{Datacenter: pString("a")}},
 		},
 		{
 			args:     []string{`-dns-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{DNS: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{DNS: pInt(1)}}},
 		},
 		{
 			args:     []string{`-grpc-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{GRPC: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{GRPC: pInt(1)}}},
 		},
 		{
 			args:     []string{`-http-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{HTTP: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{HTTP: pInt(1)}}},
 		},
 		{
 			args:     []string{`-https-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{HTTPS: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{HTTPS: pInt(1)}}},
 		},
 		{
 			args:     []string{`-serf-lan-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{SerfLAN: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{SerfLAN: pInt(1)}}},
 		},
 		{
 			args:     []string{`-serf-wan-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{SerfWAN: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{SerfWAN: pInt(1)}}},
 		},
 		{
 			args:     []string{`-server-port`, `1`},
-			flags: Flags{Config: Config{Ports: Ports{Server: pInt(1)}}},
+			expected: BuilderOpts{Config: Config{Ports: Ports{Server: pInt(1)}}},
 		},
 		{
 			args:     []string{`-join`, `a`, `-join`, `b`},
-			flags: Flags{Config: Config{StartJoinAddrsLAN: []string{"a", "b"}}},
+			expected: BuilderOpts{Config: Config{StartJoinAddrsLAN: []string{"a", "b"}}},
 		},
 		{
 			args:     []string{`-node-meta`, `a:b`, `-node-meta`, `c:d`},
-			flags: Flags{Config: Config{NodeMeta: map[string]string{"a": "b", "c": "d"}}},
+			expected: BuilderOpts{Config: Config{NodeMeta: map[string]string{"a": "b", "c": "d"}}},
 		},
 		{
 			args:     []string{`-bootstrap`, `true`},
-			flags: Flags{Config: Config{Bootstrap: pBool(true)}, Args: []string{"true"}},
+			expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}},
+			extra:    []string{"true"},
 		},
 		{
 			args: []string{`-primary-gateway`, `foo.local`, `-primary-gateway`, `bar.local`},
-			flags: Flags{Config: Config{PrimaryGateways: []string{
+			expected: BuilderOpts{Config: Config{PrimaryGateways: []string{
 				"foo.local", "bar.local",
 			}}},
 		},
@@ -94,25 +94,23 @@ func TestParseFlags(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(strings.Join(tt.args, " "), func(t *testing.T) {
-			flags := Flags{}
+			flags := BuilderOpts{}
 			fs := flag.NewFlagSet("", flag.ContinueOnError)
 			AddFlags(fs, &flags)
+
 			err := fs.Parse(tt.args)
-			if got, want := err, tt.err; !reflect.DeepEqual(got, want) {
+			require.NoError(t, err)
-				t.Fatalf("got error %v want %v", got, want)
-			}
-			flags.Args = fs.Args()
 
 			// Normalize the expected value because require.Equal considers
 			// empty slices/maps and nil slices/maps to be different.
-			if len(tt.flags.Args) == 0 && flags.Args != nil {
+			if tt.extra == nil && fs.Args() != nil {
-				tt.flags.Args = []string{}
+				tt.extra = []string{}
 			}
-			if len(tt.flags.Config.NodeMeta) == 0 {
+			if len(tt.expected.Config.NodeMeta) == 0 {
-				tt.flags.Config.NodeMeta = map[string]string{}
+				tt.expected.Config.NodeMeta = map[string]string{}
 			}
-
+			require.Equal(t, tt.extra, fs.Args())
-			require.Equal(t, tt.flags, flags)
+			require.Equal(t, tt.expected, flags)
 		})
 	}
 }

agent/config/runtime_test.go

@@ -451,6 +451,9 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
 				writeFile(filepath.Join(dataDir, "conf", "valid.json"), []byte(`{"datacenter":"a"}`))
 				writeFile(filepath.Join(dataDir, "conf", "invalid.skip"), []byte(`NOPE`))
 			},
+			warns: []string{
+				"skipping file " + filepath.Join(dataDir, "conf", "invalid.skip") + ", extension must be .hcl or .json, or config format must be set",
+			},
 		},
 		{
 			desc: "-config-format=json",
@@ -4111,14 +4114,14 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) {
 
 			t.Run(strings.Join(desc, ":"), func(t *testing.T) {
 				// first parse the flags
-				flags := Flags{}
+				flags := BuilderOpts{}
 				fs := flag.NewFlagSet("", flag.ContinueOnError)
 				AddFlags(fs, &flags)
 				err := fs.Parse(tt.args)
 				if err != nil {
 					t.Fatalf("ParseFlags failed: %s", err)
 				}
-				flags.Args = fs.Args()
+				require.Len(t, fs.Args(), 0)
 
 				if tt.pre != nil {
 					tt.pre()
@@ -4197,7 +4200,7 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) {
 				// build a default configuration, then patch the fields we expect to change
 				// and compare it with the generated configuration. Since the expected
 				// runtime config has been validated we do not need to validate it again.
-				x, err := NewBuilder(Flags{})
+				x, err := NewBuilder(BuilderOpts{})
 				if err != nil {
 					t.Fatal(err)
 				}
@@ -6346,23 +6349,19 @@ func TestFullConfig(t *testing.T) {
 		t.Run(format, func(t *testing.T) {
 			// parse the flags since this is the only way we can set the
 			// DevMode flag
-			var flags Flags
+			var flags BuilderOpts
 			fs := flag.NewFlagSet("", flag.ContinueOnError)
 			AddFlags(fs, &flags)
 			if err := fs.Parse(flagSrc); err != nil {
 				t.Fatalf("ParseFlags: %s", err)
 			}
-
+			require.Len(t, fs.Args(), 0)
-			// ensure that all fields are set to unique non-zero values
-			// if err := nonZero("Config", nil, c); err != nil {
-			// 	t.Fatal(err)
-			// }
 
 			b, err := NewBuilder(flags)
 			if err != nil {
 				t.Fatalf("NewBuilder: %s", err)
 			}
-			b.Sources = append(b.Sources, Source{Name: "full." + format, Data: data})
+			b.Sources = append(b.Sources, Source{Name: "full." + format, Data: data, Format: format})
 			b.Tail = append(b.Tail, tail[format]...)
 			b.Tail = append(b.Tail, VersionSource("JNtPSav3", "R909Hblt", "ZT1JOQLn"))
 

agent/testagent.go

@@ -436,7 +436,7 @@ func TestConfig(logger hclog.Logger, sources ...config.Source) *config.RuntimeCo
 		`,
 	}
 
-	b, err := config.NewBuilder(config.Flags{})
+	b, err := config.NewBuilder(config.BuilderOpts{})
 	if err != nil {
 		panic("NewBuilder failed: " + err.Error())
 	}

command/agent/agent.go

@@ -57,7 +57,7 @@ type cmd struct {
 	versionPrerelease string
 	versionHuman      string
 	shutdownCh        <-chan struct{}
-	flagArgs          config.Flags
+	flagArgs          config.BuilderOpts
 	logger            hclog.InterceptLogger
 }
 
@@ -174,14 +174,17 @@ func (c *cmd) startupJoinWan(agent *agent.Agent, cfg *config.RuntimeConfig) erro
 }
 
 func (c *cmd) run(args []string) int {
-	// Parse our configs
 	if err := c.flags.Parse(args); err != nil {
 		if !strings.Contains(err.Error(), "help requested") {
 			c.UI.Error(fmt.Sprintf("error parsing flags: %v", err))
 		}
 		return 1
 	}
-	c.flagArgs.Args = c.flags.Args()
+	if len(c.flags.Args()) > 0 {
+		c.UI.Error(fmt.Sprintf("Unexpected extra arguments: %v", c.flags.Args()))
+		return 1
+	}
+
 	config := c.readConfig()
 	if config == nil {
 		return 1

command/agent/agent_test.go

@@ -34,7 +34,7 @@ func TestConfigFail(t *testing.T) {
 		},
 		{
 			args: []string{"agent", "-server", "-bind=10.0.0.1", "-datacenter=foo", "some-other-arg"},
-			out:  "==> config: Unknown extra arguments: [some-other-arg]\n",
+			out:  "==> Unexpected extra arguments: [some-other-arg]\n",
 		},
 		{
 			args: []string{"agent", "-server", "-bind=10.0.0.1"},

command/services/config.go

@@ -7,17 +7,18 @@ import (
 	"github.com/hashicorp/consul/agent/config"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/api"
+	"github.com/mitchellh/cli"
 	"github.com/mitchellh/mapstructure"
 )
 
 // ServicesFromFiles returns the list of agent service registration structs
 // from a set of file arguments.
-func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error) {
+func ServicesFromFiles(ui cli.Ui, files []string) ([]*api.AgentServiceRegistration, error) {
 	// We set devMode to true so we can get the basic valid default
 	// configuration. devMode doesn't set any services by default so this
 	// is okay since we only look at services.
 	devMode := true
-	b, err := config.NewBuilder(config.Flags{
+	b, err := config.NewBuilder(config.BuilderOpts{
 		ConfigFiles: files,
 		DevMode:     &devMode,
 	})
@@ -29,6 +30,9 @@ func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error)
 	if err != nil {
 		return nil, err
 	}
+	for _, w := range b.Warnings {
+		ui.Warn(w)
+	}
 
 	// The services are now in "structs.ServiceDefinition" form and we need
 	// them in "api.AgentServiceRegistration" form so do the conversion.

command/services/config_test.go

@@ -18,7 +18,7 @@ func TestDevModeHasNoServices(t *testing.T) {
 	require := require.New(t)
 
 	devMode := true
-	b, err := config.NewBuilder(config.Flags{
+	b, err := config.NewBuilder(config.BuilderOpts{
 		DevMode: &devMode,
 	})
 	require.NoError(err)

command/services/deregister/deregister.go

@@ -55,7 +55,7 @@ func (c *cmd) Run(args []string) int {
 		ID: c.flagId}}
 	if len(args) > 0 {
 		var err error
-		svcs, err = services.ServicesFromFiles(args)
+		svcs, err = services.ServicesFromFiles(c.UI, args)
 		if err != nil {
 			c.UI.Error(fmt.Sprintf("Error: %s", err))
 			return 1

command/services/register/register.go

@@ -103,7 +103,7 @@ func (c *cmd) Run(args []string) int {
 
 	if len(args) > 0 {
 		var err error
-		svcs, err = services.ServicesFromFiles(args)
+		svcs, err = services.ServicesFromFiles(c.UI, args)
 		if err != nil {
 			c.UI.Error(fmt.Sprintf("Error: %s", err))
 			return 1

command/validate/validate.go

@@ -51,7 +51,7 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}
 
-	b, err := config.NewBuilder(config.Flags{ConfigFiles: configFiles, ConfigFormat: &c.configFormat})
+	b, err := config.NewBuilder(config.BuilderOpts{ConfigFiles: configFiles, ConfigFormat: c.configFormat})
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("Config validation failed: %v", err.Error()))
 		return 1
@@ -61,6 +61,9 @@ func (c *cmd) Run(args []string) int {
 		return 1
 	}
 	if !c.quiet {
+		for _, w := range b.Warnings {
+			c.UI.Warn(w)
+		}
 		c.UI.Output("Configuration is valid!")
 	}
 	return 0