diff --git a/agent/config/agent_limits_test.go b/agent/config/agent_limits_test.go index ee87306b41..eb1b6fd348 100644 --- a/agent/config/agent_limits_test.go +++ b/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", diff --git a/agent/config/builder.go b/agent/config/builder.go index f6638cbb5c..ab158158ea 100644 --- a/agent/config/builder.go +++ b/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. - Flags Flags + // options contains the input values used to construct a RuntimeConfig + 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 -// line flags. -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) - } - +// NewBuilder returns a new configuration Builder from the BuilderOpts. +func NewBuilder(opts BuilderOpts) (*Builder, error) { 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, - Head: []Source{DefaultSource(), DefaultEnterpriseSource()}, + 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 { - sources, err := b.ReadPath(path) + for _, path := range opts.ConfigFiles { + 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 -// not its sub-directories) and appends them to the list of config -// sources. -func (b *Builder) ReadPath(path string) ([]Source, error) { +// sourcesFromPath reads a single config file or all files in a directory (but +// not its sub-directories) and returns Sources created from the +// files. +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) { - src, err := b.ReadFile(fp) - if err != nil { - return nil, err - } - sources = append(sources, src) + if !shouldParseFile(fp, format) { + 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 -// config sources. -func (b *Builder) ReadFile(path string) (Source, error) { +// newSourceFromFile creates a Source from the contents of the file at path. +func newSourceFromFile(path string, format 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 { - configFormat := b.stringVal(b.Flags.ConfigFormat) - srcFormat := FormatFrom(path) +func shouldParseFile(path string, configFormat string) bool { + srcFormat := formatFromFileExtension(path) + return configFormat != "" || srcFormat == "hcl" || srcFormat == "json" +} - // If config-format is not set, only read files with supported extensions - if configFormat == "" && srcFormat != "hcl" && srcFormat != "json" { - return false +func formatFromFileExtension(name string) string { + switch { + 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 - b.Warnings = nil - - // ---------------------------------------------------------------- - // merge config sources as follows - // - configFormat := b.stringVal(b.Flags.ConfigFormat) + // TODO: move to NewBuilder to remove Builder.options field + configFormat := b.options.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 { - // 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.Sources...) 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), diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go new file mode 100644 index 0000000000..a2eddaf75d --- /dev/null +++ b/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, + } +} diff --git a/agent/config/config.go b/agent/config/config.go index 43e001ffe9..db7b57823e 100644 --- a/agent/config/config.go +++ b/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{} diff --git a/agent/config/default.go b/agent/config/default.go index 531f0469d2..6afc93ccb6 100644 --- a/agent/config/default.go +++ b/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) } diff --git a/agent/config/flags.go b/agent/config/flags.go index fccee4c1d0..8cc596a499 100644 --- a/agent/config/flags.go +++ b/agent/config/flags.go @@ -6,8 +6,8 @@ import ( "time" ) -// Flags defines the command line flags. -type Flags struct { +// BuilderOpts used by Builder to construct and validate a RuntimeConfig. +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.") diff --git a/agent/config/flags_test.go b/agent/config/flags_test.go index 63563b298f..10edb2204e 100644 --- a/agent/config/flags_test.go +++ b/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 - err error + args []string + expected BuilderOpts + extra []string }{ {}, { - args: []string{`-bind`, `a`}, - flags: Flags{Config: Config{BindAddr: pString("a")}}, + args: []string{`-bind`, `a`}, + expected: BuilderOpts{Config: Config{BindAddr: pString("a")}}, }, { - args: []string{`-bootstrap`}, - flags: Flags{Config: Config{Bootstrap: pBool(true)}}, + args: []string{`-bootstrap`}, + expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}}, }, { - args: []string{`-bootstrap=true`}, - flags: Flags{Config: Config{Bootstrap: pBool(true)}}, + args: []string{`-bootstrap=true`}, + expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}}, }, { - args: []string{`-bootstrap=false`}, - flags: Flags{Config: Config{Bootstrap: pBool(false)}}, + args: []string{`-bootstrap=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"}}, + args: []string{`-config-file`, `a`, `-config-dir`, `b`, `-config-file`, `c`, `-config-dir`, `d`}, + expected: BuilderOpts{ConfigFiles: []string{"a", "b", "c", "d"}}, }, { - args: []string{`-datacenter`, `a`}, - flags: Flags{Config: Config{Datacenter: pString("a")}}, + args: []string{`-datacenter`, `a`}, + expected: BuilderOpts{Config: Config{Datacenter: pString("a")}}, }, { - args: []string{`-dns-port`, `1`}, - flags: Flags{Config: Config{Ports: Ports{DNS: pInt(1)}}}, + args: []string{`-dns-port`, `1`}, + expected: BuilderOpts{Config: Config{Ports: Ports{DNS: pInt(1)}}}, }, { - args: []string{`-grpc-port`, `1`}, - flags: Flags{Config: Config{Ports: Ports{GRPC: pInt(1)}}}, + args: []string{`-grpc-port`, `1`}, + expected: BuilderOpts{Config: Config{Ports: Ports{GRPC: pInt(1)}}}, }, { - args: []string{`-http-port`, `1`}, - flags: Flags{Config: Config{Ports: Ports{HTTP: pInt(1)}}}, + args: []string{`-http-port`, `1`}, + expected: BuilderOpts{Config: Config{Ports: Ports{HTTP: pInt(1)}}}, }, { - args: []string{`-https-port`, `1`}, - flags: Flags{Config: Config{Ports: Ports{HTTPS: pInt(1)}}}, + args: []string{`-https-port`, `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)}}}, + args: []string{`-serf-lan-port`, `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)}}}, + args: []string{`-serf-wan-port`, `1`}, + expected: BuilderOpts{Config: Config{Ports: Ports{SerfWAN: pInt(1)}}}, }, { - args: []string{`-server-port`, `1`}, - flags: Flags{Config: Config{Ports: Ports{Server: pInt(1)}}}, + args: []string{`-server-port`, `1`}, + expected: BuilderOpts{Config: Config{Ports: Ports{Server: pInt(1)}}}, }, { - args: []string{`-join`, `a`, `-join`, `b`}, - flags: Flags{Config: Config{StartJoinAddrsLAN: []string{"a", "b"}}}, + args: []string{`-join`, `a`, `-join`, `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"}}}, + args: []string{`-node-meta`, `a:b`, `-node-meta`, `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"}}, + args: []string{`-bootstrap`, `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) { - t.Fatalf("got error %v want %v", got, want) - } - flags.Args = fs.Args() + require.NoError(t, err) // 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 { - tt.flags.Args = []string{} + if tt.extra == nil && fs.Args() != nil { + tt.extra = []string{} } - if len(tt.flags.Config.NodeMeta) == 0 { - tt.flags.Config.NodeMeta = map[string]string{} + if len(tt.expected.Config.NodeMeta) == 0 { + tt.expected.Config.NodeMeta = map[string]string{} } - - require.Equal(t, tt.flags, flags) + require.Equal(t, tt.extra, fs.Args()) + require.Equal(t, tt.expected, flags) }) } } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 93f92d83ec..bf1e37d9c9 100644 --- a/agent/config/runtime_test.go +++ b/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) } - - // ensure that all fields are set to unique non-zero values - // if err := nonZero("Config", nil, c); err != nil { - // t.Fatal(err) - // } + require.Len(t, fs.Args(), 0) 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")) diff --git a/agent/testagent.go b/agent/testagent.go index 776c381573..439dede687 100644 --- a/agent/testagent.go +++ b/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()) } diff --git a/command/agent/agent.go b/command/agent/agent.go index f1df46ad22..8e50e37da2 100644 --- a/command/agent/agent.go +++ b/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 diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 2f37a52d0a..835d98850c 100644 --- a/command/agent/agent_test.go +++ b/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"}, diff --git a/command/services/config.go b/command/services/config.go index ee2477d3c4..677757da39 100644 --- a/command/services/config.go +++ b/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. diff --git a/command/services/config_test.go b/command/services/config_test.go index d6e93b3dba..71cdd96e82 100644 --- a/command/services/config_test.go +++ b/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) diff --git a/command/services/deregister/deregister.go b/command/services/deregister/deregister.go index 07c8c13cdf..724a22feda 100644 --- a/command/services/deregister/deregister.go +++ b/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 diff --git a/command/services/register/register.go b/command/services/register/register.go index da6818bb1e..5a31f1070e 100644 --- a/command/services/register/register.go +++ b/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 diff --git a/command/validate/validate.go b/command/validate/validate.go index fe3e646533..754f0e5ded 100644 --- a/command/validate/validate.go +++ b/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