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.
This commit is contained in:
Daniel Nephin 2020-05-01 18:29:32 -04:00
parent 5ac012dddf
commit b7b652e8c9
11 changed files with 45 additions and 45 deletions

View File

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

View File

@ -54,8 +54,8 @@ import (
// since not all pre-conditions have to be satisfied when performing // since not all pre-conditions have to be satisfied when performing
// syntactical tests. // syntactical tests.
type Builder struct { type Builder struct {
// Flags contains the parsed command line arguments. // options contains the input values used to construct a RuntimeConfig
Flags Flags options BuilderOpts
// Head, Sources, and Tail are used to manage the order of the // Head, Sources, and Tail are used to manage the order of the
// config sources, as described in the comments above. // 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 // NewBuilder returns a new configuration builder based on the given command
// line flags. // line flags.
func NewBuilder(flags Flags) (*Builder, error) { func NewBuilder(opts BuilderOpts) (*Builder, error) {
newSource := func(name string, v interface{}) Source { newSource := func(name string, v interface{}) Source {
b, err := json.MarshalIndent(v, "", " ") b, err := json.MarshalIndent(v, "", " ")
if err != nil { if err != nil {
@ -93,11 +93,11 @@ func NewBuilder(flags Flags) (*Builder, error) {
} }
b := &Builder{ b := &Builder{
Flags: flags, options: opts,
Head: []Source{DefaultSource(), DefaultEnterpriseSource()}, Head: []Source{DefaultSource(), DefaultEnterpriseSource()},
} }
if b.boolVal(b.Flags.DevMode) { if b.boolVal(b.options.DevMode) {
b.Head = append(b.Head, DevSource()) 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 // we need to merge all slice values defined in flags before we
// merge the config files since the flag values for slices are // merge the config files since the flag values for slices are
// otherwise appended instead of prepended. // 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)) 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) sources, err := b.ReadPath(path)
if err != nil { if err != nil {
return nil, err return nil, err
@ -116,7 +116,7 @@ func NewBuilder(flags Flags) (*Builder, error) {
b.Sources = append(b.Sources, sources...) b.Sources = append(b.Sources, sources...)
} }
b.Tail = append(b.Tail, newSource("flags.values", values)) 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{ b.Tail = append(b.Tail, Source{
Name: fmt.Sprintf("flags-%d.hcl", i), Name: fmt.Sprintf("flags-%d.hcl", i),
Format: "hcl", Format: "hcl",
@ -124,7 +124,7 @@ func NewBuilder(flags Flags) (*Builder, error) {
}) })
} }
b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), DefaultVersionSource()) 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()) b.Tail = append(b.Tail, DevConsulSource())
} }
return b, nil 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 // shouldParse file determines whether the file to be read is of a supported extension
func (b *Builder) shouldParseFile(path string) bool { func (b *Builder) shouldParseFile(path string) bool {
configFormat := b.stringVal(b.Flags.ConfigFormat) configFormat := b.stringVal(b.options.ConfigFormat)
srcFormat := FormatFrom(path) srcFormat := FormatFrom(path)
// If config-format is not set, only read files with supported extensions // 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 // merge config sources as follows
// //
configFormat := b.stringVal(b.Flags.ConfigFormat) configFormat := b.stringVal(b.options.ConfigFormat)
if configFormat != "" && configFormat != "json" && configFormat != "hcl" { if configFormat != "" && configFormat != "json" && configFormat != "hcl" {
return RuntimeConfig{}, fmt.Errorf("config: -config-format must be either 'hcl' or 'json'") 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), DataDir: b.stringVal(c.DataDir),
Datacenter: datacenter, Datacenter: datacenter,
DefaultQueryTime: b.durationVal("default_query_time", c.DefaultQueryTime), 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), DisableAnonymousSignature: b.boolVal(c.DisableAnonymousSignature),
DisableCoordinates: b.boolVal(c.DisableCoordinates), DisableCoordinates: b.boolVal(c.DisableCoordinates),
DisableHostNodeID: b.boolVal(c.DisableHostNodeID), DisableHostNodeID: b.boolVal(c.DisableHostNodeID),

View File

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

View File

@ -6,8 +6,8 @@ import (
"time" "time"
) )
// Flags defines the command line flags. // BuilderOpts used by Builder to construct and validate a RuntimeConfig.
type Flags struct { type BuilderOpts struct {
// Config contains the command line arguments that can also be set // Config contains the command line arguments that can also be set
// in a config file. // in a config file.
Config Config Config Config
@ -29,7 +29,7 @@ type Flags struct {
} }
// AddFlags adds the command line flags for the agent. // 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) { add := func(p interface{}, name, help string) {
switch x := p.(type) { switch x := p.(type) {
case **bool: case **bool:

View File

@ -15,78 +15,78 @@ import (
func TestAddFlags_WithParse(t *testing.T) { func TestAddFlags_WithParse(t *testing.T) {
tests := []struct { tests := []struct {
args []string args []string
expected Flags expected BuilderOpts
extra []string extra []string
}{ }{
{}, {},
{ {
args: []string{`-bind`, `a`}, args: []string{`-bind`, `a`},
expected: Flags{Config: Config{BindAddr: pString("a")}}, expected: BuilderOpts{Config: Config{BindAddr: pString("a")}},
}, },
{ {
args: []string{`-bootstrap`}, args: []string{`-bootstrap`},
expected: Flags{Config: Config{Bootstrap: pBool(true)}}, expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}},
}, },
{ {
args: []string{`-bootstrap=true`}, args: []string{`-bootstrap=true`},
expected: Flags{Config: Config{Bootstrap: pBool(true)}}, expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}},
}, },
{ {
args: []string{`-bootstrap=false`}, 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`}, 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`}, args: []string{`-datacenter`, `a`},
expected: Flags{Config: Config{Datacenter: pString("a")}}, expected: BuilderOpts{Config: Config{Datacenter: pString("a")}},
}, },
{ {
args: []string{`-dns-port`, `1`}, 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`}, 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`}, 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`}, 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`}, 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`}, 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`}, 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`}, 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`}, 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`}, args: []string{`-bootstrap`, `true`},
expected: Flags{Config: Config{Bootstrap: pBool(true)}}, expected: BuilderOpts{Config: Config{Bootstrap: pBool(true)}},
extra: []string{"true"}, extra: []string{"true"},
}, },
{ {
args: []string{`-primary-gateway`, `foo.local`, `-primary-gateway`, `bar.local`}, 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", "foo.local", "bar.local",
}}}, }}},
}, },
@ -94,7 +94,7 @@ func TestAddFlags_WithParse(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(strings.Join(tt.args, " "), func(t *testing.T) { t.Run(strings.Join(tt.args, " "), func(t *testing.T) {
flags := Flags{} flags := BuilderOpts{}
fs := flag.NewFlagSet("", flag.ContinueOnError) fs := flag.NewFlagSet("", flag.ContinueOnError)
AddFlags(fs, &flags) AddFlags(fs, &flags)

View File

@ -3844,7 +3844,7 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) {
t.Run(strings.Join(desc, ":"), func(t *testing.T) { t.Run(strings.Join(desc, ":"), func(t *testing.T) {
// first parse the flags // first parse the flags
flags := Flags{} flags := BuilderOpts{}
fs := flag.NewFlagSet("", flag.ContinueOnError) fs := flag.NewFlagSet("", flag.ContinueOnError)
AddFlags(fs, &flags) AddFlags(fs, &flags)
err := fs.Parse(tt.args) 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 // build a default configuration, then patch the fields we expect to change
// and compare it with the generated configuration. Since the expected // and compare it with the generated configuration. Since the expected
// runtime config has been validated we do not need to validate it again. // runtime config has been validated we do not need to validate it again.
x, err := NewBuilder(Flags{}) x, err := NewBuilder(BuilderOpts{})
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -5996,7 +5996,7 @@ func TestFullConfig(t *testing.T) {
t.Run(format, func(t *testing.T) { t.Run(format, func(t *testing.T) {
// parse the flags since this is the only way we can set the // parse the flags since this is the only way we can set the
// DevMode flag // DevMode flag
var flags Flags var flags BuilderOpts
fs := flag.NewFlagSet("", flag.ContinueOnError) fs := flag.NewFlagSet("", flag.ContinueOnError)
AddFlags(fs, &flags) AddFlags(fs, &flags)
if err := fs.Parse(flagSrc); err != nil { if err := fs.Parse(flagSrc); err != nil {

View File

@ -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 { if err != nil {
panic("NewBuilder failed: " + err.Error()) panic("NewBuilder failed: " + err.Error())
} }

View File

@ -57,7 +57,7 @@ type cmd struct {
versionPrerelease string versionPrerelease string
versionHuman string versionHuman string
shutdownCh <-chan struct{} shutdownCh <-chan struct{}
flagArgs config.Flags flagArgs config.BuilderOpts
logger hclog.InterceptLogger logger hclog.InterceptLogger
} }

View File

@ -17,7 +17,7 @@ func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error)
// configuration. devMode doesn't set any services by default so this // configuration. devMode doesn't set any services by default so this
// is okay since we only look at services. // is okay since we only look at services.
devMode := true devMode := true
b, err := config.NewBuilder(config.Flags{ b, err := config.NewBuilder(config.BuilderOpts{
ConfigFiles: files, ConfigFiles: files,
DevMode: &devMode, DevMode: &devMode,
}) })

View File

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

View File

@ -51,7 +51,7 @@ func (c *cmd) Run(args []string) int {
return 1 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 { if err != nil {
c.UI.Error(fmt.Sprintf("Config validation failed: %v", err.Error())) c.UI.Error(fmt.Sprintf("Config validation failed: %v", err.Error()))
return 1 return 1