From 77101eee82529a95cb832a221101ce28e7bf1b33 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 18:29:32 -0400 Subject: [PATCH] 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