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