From 85e0338136ecc64d765b983da54334a6b4bdf87e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 18:17:28 -0400 Subject: [PATCH 1/5] config: remove Args field from Flags This field was populated for one reason, to test that it was empty. Of all the callers, only a single one used this functionality. The rest constructed a `Flags{}` struct which did not set Args. I think this shows that the logic was in the wrong place. Only the agent command needs to care about validating the args. This commit removes the field, and moves the logic to the one caller that cares. Also fix some comments. --- agent/config/builder.go | 6 --- agent/config/flags.go | 5 +- agent/config/flags_test.go | 96 ++++++++++++++++++------------------ agent/config/runtime_test.go | 2 +- command/agent/agent.go | 7 ++- command/agent/agent_test.go | 2 +- 6 files changed, 55 insertions(+), 63 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index 026fa35673..0185a66634 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -84,12 +84,6 @@ type Builder struct { // 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) - } - newSource := func(name string, v interface{}) Source { b, err := json.MarshalIndent(v, "", " ") if err != nil { diff --git a/agent/config/flags.go b/agent/config/flags.go index fccee4c1d0..8d1fc8a018 100644 --- a/agent/config/flags.go +++ b/agent/config/flags.go @@ -20,15 +20,12 @@ type Flags struct { // format independent of their extension. 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. diff --git a/agent/config/flags_test.go b/agent/config/flags_test.go index 63563b298f..91f097a5e1 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 Flags + extra []string }{ {}, { - args: []string{`-bind`, `a`}, - flags: Flags{Config: Config{BindAddr: pString("a")}}, + args: []string{`-bind`, `a`}, + expected: Flags{Config: Config{BindAddr: pString("a")}}, }, { - args: []string{`-bootstrap`}, - flags: Flags{Config: Config{Bootstrap: pBool(true)}}, + args: []string{`-bootstrap`}, + expected: Flags{Config: Config{Bootstrap: pBool(true)}}, }, { - args: []string{`-bootstrap=true`}, - flags: Flags{Config: Config{Bootstrap: pBool(true)}}, + args: []string{`-bootstrap=true`}, + expected: Flags{Config: Config{Bootstrap: pBool(true)}}, }, { - args: []string{`-bootstrap=false`}, - flags: Flags{Config: Config{Bootstrap: pBool(false)}}, + args: []string{`-bootstrap=false`}, + expected: Flags{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: Flags{ConfigFiles: []string{"a", "b", "c", "d"}}, }, { - args: []string{`-datacenter`, `a`}, - flags: Flags{Config: Config{Datacenter: pString("a")}}, + args: []string{`-datacenter`, `a`}, + expected: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{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: Flags{Config: Config{PrimaryGateways: []string{ "foo.local", "bar.local", }}}, }, @@ -97,22 +97,20 @@ func TestParseFlags(t *testing.T) { flags := Flags{} 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 20d9e70be7..d8e137a596 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -3851,7 +3851,7 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) { 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() diff --git a/command/agent/agent.go b/command/agent/agent.go index f1df46ad22..ee088dce8b 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -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"}, From 77101eee82529a95cb832a221101ce28e7bf1b33 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 18:29:32 -0400 Subject: [PATCH 2/5] config: rename Flags to BuilderOpts Flags is an overloaded term in this context. It generally is used to refer to command line flags. This struct, however, is a data object used as input to the construction. It happens to be partially populated by command line flags, but otherwise has very little to do with them. Renaming this struct should make the actual responsibility of this struct more obvious, and remove the possibility that it is confused with command line flags. This change is in preparation for adding additional fields to BuilderOpts. --- agent/config/agent_limits_test.go | 2 +- agent/config/builder.go | 26 ++++++++++----------- agent/config/default.go | 2 +- agent/config/flags.go | 6 ++--- agent/config/flags_test.go | 38 +++++++++++++++---------------- agent/config/runtime_test.go | 6 ++--- agent/testagent.go | 2 +- command/agent/agent.go | 2 +- command/services/config.go | 2 +- command/services/config_test.go | 2 +- command/validate/validate.go | 2 +- 11 files changed, 45 insertions(+), 45 deletions(-) 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 0185a66634..4fcdfce947 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -54,8 +54,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. @@ -83,7 +83,7 @@ type Builder struct { // NewBuilder returns a new configuration builder based on the given command // line flags. -func NewBuilder(flags Flags) (*Builder, error) { +func NewBuilder(opts BuilderOpts) (*Builder, error) { newSource := func(name string, v interface{}) Source { b, err := json.MarshalIndent(v, "", " ") if err != nil { @@ -93,11 +93,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(b.options.DevMode) { b.Head = append(b.Head, DevSource()) } @@ -106,9 +106,9 @@ 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(b.options.Config) b.Head = append(b.Head, newSource("flags.slices", slices)) - for _, path := range b.Flags.ConfigFiles { + for _, path := range b.options.ConfigFiles { sources, err := b.ReadPath(path) if err != nil { return nil, err @@ -116,7 +116,7 @@ func NewBuilder(flags Flags) (*Builder, error) { 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 b.options.HCL { b.Tail = append(b.Tail, Source{ Name: fmt.Sprintf("flags-%d.hcl", i), Format: "hcl", @@ -124,7 +124,7 @@ func NewBuilder(flags Flags) (*Builder, error) { }) } b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), DefaultVersionSource()) - if b.boolVal(b.Flags.DevMode) { + if b.boolVal(b.options.DevMode) { b.Tail = append(b.Tail, DevConsulSource()) } return b, nil @@ -204,7 +204,7 @@ func (b *Builder) ReadFile(path string) (Source, error) { // 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) + configFormat := b.stringVal(b.options.ConfigFormat) srcFormat := FormatFrom(path) // If config-format is not set, only read files with supported extensions @@ -244,7 +244,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { // ---------------------------------------------------------------- // merge config sources as follows // - configFormat := b.stringVal(b.Flags.ConfigFormat) + configFormat := b.stringVal(b.options.ConfigFormat) if configFormat != "" && configFormat != "json" && configFormat != "hcl" { return RuntimeConfig{}, fmt.Errorf("config: -config-format must be either 'hcl' or 'json'") } @@ -910,7 +910,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/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 8d1fc8a018..3f13121829 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 @@ -29,7 +29,7 @@ type Flags struct { } // 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: diff --git a/agent/config/flags_test.go b/agent/config/flags_test.go index 91f097a5e1..10edb2204e 100644 --- a/agent/config/flags_test.go +++ b/agent/config/flags_test.go @@ -15,78 +15,78 @@ import ( func TestAddFlags_WithParse(t *testing.T) { tests := []struct { args []string - expected Flags + expected BuilderOpts extra []string }{ {}, { args: []string{`-bind`, `a`}, - expected: Flags{Config: Config{BindAddr: pString("a")}}, + expected: BuilderOpts{Config: Config{BindAddr: pString("a")}}, }, { args: []string{`-bootstrap`}, - expected: Flags{Config: Config{Bootstrap: pBool(true)}}, + expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}}, }, { args: []string{`-bootstrap=true`}, - expected: Flags{Config: Config{Bootstrap: pBool(true)}}, + expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}}, }, { args: []string{`-bootstrap=false`}, - expected: 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`}, - expected: Flags{ConfigFiles: []string{"a", "b", "c", "d"}}, + expected: BuilderOpts{ConfigFiles: []string{"a", "b", "c", "d"}}, }, { args: []string{`-datacenter`, `a`}, - expected: Flags{Config: Config{Datacenter: pString("a")}}, + expected: BuilderOpts{Config: Config{Datacenter: pString("a")}}, }, { args: []string{`-dns-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{DNS: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{DNS: pInt(1)}}}, }, { args: []string{`-grpc-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{GRPC: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{GRPC: pInt(1)}}}, }, { args: []string{`-http-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{HTTP: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{HTTP: pInt(1)}}}, }, { args: []string{`-https-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{HTTPS: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{HTTPS: pInt(1)}}}, }, { args: []string{`-serf-lan-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{SerfLAN: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{SerfLAN: pInt(1)}}}, }, { args: []string{`-serf-wan-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{SerfWAN: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{SerfWAN: pInt(1)}}}, }, { args: []string{`-server-port`, `1`}, - expected: Flags{Config: Config{Ports: Ports{Server: pInt(1)}}}, + expected: BuilderOpts{Config: Config{Ports: Ports{Server: pInt(1)}}}, }, { args: []string{`-join`, `a`, `-join`, `b`}, - expected: 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`}, - expected: 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`}, - expected: Flags{Config: Config{Bootstrap: pBool(true)}}, + expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}}, extra: []string{"true"}, }, { args: []string{`-primary-gateway`, `foo.local`, `-primary-gateway`, `bar.local`}, - expected: Flags{Config: Config{PrimaryGateways: []string{ + expected: BuilderOpts{Config: Config{PrimaryGateways: []string{ "foo.local", "bar.local", }}}, }, @@ -94,7 +94,7 @@ func TestAddFlags_WithParse(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) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index d8e137a596..9131280869 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -3844,7 +3844,7 @@ 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) @@ -3930,7 +3930,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) } @@ -5996,7 +5996,7 @@ 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 { 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 ee088dce8b..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 } diff --git a/command/services/config.go b/command/services/config.go index ee2477d3c4..94b026ff5b 100644 --- a/command/services/config.go +++ b/command/services/config.go @@ -17,7 +17,7 @@ func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error) // 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, }) 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/validate/validate.go b/command/validate/validate.go index fe3e646533..f85fc64224 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 From 219790ca49666a42521dbda331befae501060b7c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 18:43:15 -0400 Subject: [PATCH 3/5] config: Make ConfigFormat not a pointer The nil value was never used. We can avoid a bunch of complications by making the field a string value instead of a pointer. This change is in preparation for fixing a silent config failure. --- agent/config/builder.go | 5 ++--- agent/config/flags.go | 4 ++-- command/validate/validate.go | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index 4fcdfce947..3110c5a145 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -204,11 +204,10 @@ func (b *Builder) ReadFile(path string) (Source, error) { // 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.options.ConfigFormat) srcFormat := FormatFrom(path) // If config-format is not set, only read files with supported extensions - if configFormat == "" && srcFormat != "hcl" && srcFormat != "json" { + if b.options.ConfigFormat == "" && srcFormat != "hcl" && srcFormat != "json" { return false } return true @@ -244,7 +243,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { // ---------------------------------------------------------------- // merge config sources as follows // - configFormat := b.stringVal(b.options.ConfigFormat) + configFormat := b.options.ConfigFormat if configFormat != "" && configFormat != "json" && configFormat != "hcl" { return RuntimeConfig{}, fmt.Errorf("config: -config-format must be either 'hcl' or 'json'") } diff --git a/agent/config/flags.go b/agent/config/flags.go index 3f13121829..8cc596a499 100644 --- a/agent/config/flags.go +++ b/agent/config/flags.go @@ -18,7 +18,7 @@ type BuilderOpts struct { // ConfigFormat forces all config files to be interpreted as this // format independent of their extension. - ConfigFormat *string + ConfigFormat string // DevMode indicates whether the agent should be started in development // mode. This cannot be configured in a config file. @@ -60,7 +60,7 @@ func AddFlags(fs *flag.FlagSet, f *BuilderOpts) { 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/command/validate/validate.go b/command/validate/validate.go index f85fc64224..fb3d55f475 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.BuilderOpts{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 From 476b57fe2261d28e04ae6700a57f473f4f59a5fa Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 18:45:15 -0400 Subject: [PATCH 4/5] config: refactor to consolidate all File->Source loading Previously the logic for reading ConfigFiles and produces Sources was split between NewBuilder and Build. This commit moves all of the logic into NewBuilder so that Build() can operate entirely on Sources. This change is in preparation for logging warnings when files have an unsupported extension. It also reduces the scope of BuilderOpts, and gets us very close to removing Builder.options. --- agent/config/builder.go | 92 +++++++++++++++++------------------ agent/config/builder_test.go | 93 ++++++++++++++++++++++++++++++++++++ agent/config/config.go | 12 ----- agent/config/runtime_test.go | 7 +-- 4 files changed, 140 insertions(+), 64 deletions(-) create mode 100644 agent/config/builder_test.go diff --git a/agent/config/builder.go b/agent/config/builder.go index 3110c5a145..f1d787ebc7 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -81,8 +81,7 @@ type Builder struct { err error } -// NewBuilder returns a new configuration builder based on the given command -// line flags. +// 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, "", " ") @@ -97,7 +96,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) { Head: []Source{DefaultSource(), DefaultEnterpriseSource()}, } - if b.boolVal(b.options.DevMode) { + if b.boolVal(opts.DevMode) { b.Head = append(b.Head, DevSource()) } @@ -106,17 +105,17 @@ func NewBuilder(opts BuilderOpts) (*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.options.Config) + slices, values := b.splitSlicesAndValues(opts.Config) b.Head = append(b.Head, newSource("flags.slices", slices)) - for _, path := range b.options.ConfigFiles { - sources, err := b.ReadPath(path) + for _, path := range opts.ConfigFiles { + sources, err := sourcesFromPath(path, opts) 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.options.HCL { + for i, s := range opts.HCL { b.Tail = append(b.Tail, Source{ Name: fmt.Sprintf("flags-%d.hcl", i), Format: "hcl", @@ -124,16 +123,16 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) { }) } b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), DefaultVersionSource()) - if b.boolVal(b.options.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 sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { f, err := os.Open(path) if err != nil { return nil, fmt.Errorf("config: Open failed on %s. %s", path, err) @@ -146,7 +145,12 @@ func (b *Builder) ReadPath(path string) ([]Source, error) { } if !fi.IsDir() { - src, err := b.ReadFile(path) + if !shouldParseFile(path, options.ConfigFormat) { + // TODO: log warning + return nil, nil + } + + src, err := newSourceFromFile(path, options.ConfigFormat) if err != nil { return nil, err } @@ -181,36 +185,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, options.ConfigFormat) { + // TODO: log warning + continue } + src, err := newSourceFromFile(fp, options.ConfigFormat) + 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 { - 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 b.options.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 @@ -240,9 +254,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { b.err = nil b.Warnings = nil - // ---------------------------------------------------------------- - // merge config sources as follows - // + // 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'") @@ -251,19 +263,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 diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go new file mode 100644 index 0000000000..aa6cec17f4 --- /dev/null +++ b/agent/config/builder_test.go @@ -0,0 +1,93 @@ +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) +} + +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 c11e7fd2e4..8e13b80494 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/runtime_test.go b/agent/config/runtime_test.go index 9131280869..2598946bb3 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -6003,16 +6003,11 @@ func TestFullConfig(t *testing.T) { 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) - // } - 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")) From be29d6bf75aa10681518272dcabb653b18d8453a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 20:17:27 -0400 Subject: [PATCH 5/5] config: warn when a config file is skipped All commands which read config (agent, services, and validate) will now print warnings when one of the config files is skipped because it did not match an expected format. Also ensures that config validate prints all warnings. --- agent/config/builder.go | 19 ++++++++----------- agent/config/builder_test.go | 1 + agent/config/runtime_test.go | 4 ++++ command/services/config.go | 6 +++++- command/services/deregister/deregister.go | 2 +- command/services/register/register.go | 2 +- command/validate/validate.go | 3 +++ 7 files changed, 23 insertions(+), 14 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index f1d787ebc7..919f2fdbb2 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -108,7 +108,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) { slices, values := b.splitSlicesAndValues(opts.Config) b.Head = append(b.Head, newSource("flags.slices", slices)) for _, path := range opts.ConfigFiles { - sources, err := sourcesFromPath(path, opts) + sources, err := b.sourcesFromPath(path, opts.ConfigFormat) if err != nil { return nil, err } @@ -132,7 +132,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, 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 sourcesFromPath(path string, options BuilderOpts) ([]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) @@ -145,12 +145,12 @@ func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { } if !fi.IsDir() { - if !shouldParseFile(path, options.ConfigFormat) { - // TODO: log warning + 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, options.ConfigFormat) + src, err := newSourceFromFile(path, format) if err != nil { return nil, err } @@ -185,11 +185,11 @@ func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { continue } - if !shouldParseFile(fp, options.ConfigFormat) { - // TODO: log warning + 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, options.ConfigFormat) + src, err := newSourceFromFile(fp, format) if err != nil { return nil, err } @@ -251,9 +251,6 @@ 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 - // TODO: move to NewBuilder to remove Builder.options field configFormat := b.options.ConfigFormat if configFormat != "" && configFormat != "json" && configFormat != "hcl" { diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index aa6cec17f4..a2eddaf75d 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -44,6 +44,7 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) { {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) { diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 2598946bb3..424f87afce 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", @@ -6002,6 +6005,7 @@ func TestFullConfig(t *testing.T) { if err := fs.Parse(flagSrc); err != nil { t.Fatalf("ParseFlags: %s", err) } + require.Len(t, fs.Args(), 0) b, err := NewBuilder(flags) if err != nil { diff --git a/command/services/config.go b/command/services/config.go index 94b026ff5b..677757da39 100644 --- a/command/services/config.go +++ b/command/services/config.go @@ -7,12 +7,13 @@ 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. @@ -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/deregister/deregister.go b/command/services/deregister/deregister.go index fc8efedd09..2cae8172a6 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 96835de09a..52f030edb2 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 fb3d55f475..754f0e5ded 100644 --- a/command/validate/validate.go +++ b/command/validate/validate.go @@ -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