diff --git a/agent/config/builder.go b/agent/config/builder.go index 530d4495d..7c4d965a2 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -81,6 +81,11 @@ type LoadOpts struct { // specify a replacement for ipaddr.GetPrivateIPv4 and ipaddr.GetPublicIPv6. getPrivateIPv4 func() ([]*net.IPAddr, error) getPublicIPv6 func() ([]*net.IPAddr, error) + + // sources is a shim for testing. Many test cases used explicit sources instead + // paths to config files. This shim allows us to preserve those test cases + // while using Load as the entrypoint. + sources []Source } // Load will build the configuration including the config source injected @@ -94,10 +99,13 @@ func Load(opts LoadOpts) (LoadResult, error) { if err != nil { return r, err } - cfg, err := b.BuildAndValidate() + cfg, err := b.build() if err != nil { return r, err } + if err := b.validate(cfg); err != nil { + return r, err + } return LoadResult{RuntimeConfig: &cfg, Warnings: b.Warnings}, nil } @@ -166,6 +174,7 @@ func newBuilder(opts LoadOpts) (*builder, error) { b.Head = append(b.Head, opts.DefaultConfig) } + b.Sources = opts.sources for _, path := range opts.ConfigFiles { sources, err := b.sourcesFromPath(path, opts.ConfigFormat) if err != nil { @@ -295,24 +304,13 @@ func (a byName) Len() int { return len(a) } func (a byName) Swap(i, j int) { a[i], a[j] = a[j], a[i] } func (a byName) Less(i, j int) bool { return a[i].Name() < a[j].Name() } -func (b *builder) BuildAndValidate() (RuntimeConfig, error) { - rt, err := b.Build() - if err != nil { - return RuntimeConfig{}, err - } - if err := b.Validate(rt); err != nil { - return RuntimeConfig{}, err - } - return rt, nil -} - -// Build constructs the runtime configuration from the config sources +// build constructs the runtime configuration from the config sources // and the command line flags. The config sources are processed in the // order they were added with the flags being processed last to give // precedence over the other sources. If the error is nil then // warnings can still contain deprecation or format warnings that should // be presented to the user. -func (b *builder) Build() (rt RuntimeConfig, err error) { +func (b *builder) build() (rt RuntimeConfig, err error) { srcs := make([]Source, 0, len(b.Head)+len(b.Sources)+len(b.Tail)) srcs = append(srcs, b.Head...) srcs = append(srcs, b.Sources...) @@ -1175,8 +1173,8 @@ func validateBasicName(field, value string, allowEmpty bool) error { return nil } -// Validate performs semantic validation of the runtime configuration. -func (b *builder) Validate(rt RuntimeConfig) error { +// validate performs semantic validation of the runtime configuration. +func (b *builder) validate(rt RuntimeConfig) error { // validContentPath defines a regexp for a valid content path name. var validContentPath = regexp.MustCompile(`^[A-Za-z0-9/_-]+$`) diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index 2354f4211..5e3987701 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -128,7 +128,7 @@ func setupConfigFiles(t *testing.T) []string { } } -func TestBuilder_BuildAndValidate_NodeName(t *testing.T) { +func TestLoad_NodeName(t *testing.T) { type testCase struct { name string nodeName string @@ -136,18 +136,17 @@ func TestBuilder_BuildAndValidate_NodeName(t *testing.T) { } fn := func(t *testing.T, tc testCase) { - b, err := newBuilder(LoadOpts{ + opts := LoadOpts{ FlagValues: Config{ NodeName: pString(tc.nodeName), DataDir: pString("dir"), }, - }) - patchLoadOptsShims(&b.opts) + } + patchLoadOptsShims(&opts) + result, err := Load(opts) require.NoError(t, err) - _, err = b.BuildAndValidate() - require.NoError(t, err) - require.Len(t, b.Warnings, 1) - require.Contains(t, b.Warnings[0], tc.expectedWarn) + require.Len(t, result.Warnings, 1) + require.Contains(t, result.Warnings[0], tc.expectedWarn) } var testCases = []testCase{ diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index bb7bd5f16..869b67cbe 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -1507,8 +1507,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) { "start_join":["a", "b"] }`, }, - hcl: []string{ - ` + hcl: []string{` advertise_addr = "1.2.3.4" advertise_addr_wan = "5.6.7.8" bootstrap = true @@ -5098,27 +5097,22 @@ func (tc testCase) run(format string, dataDir string) func(t *testing.T) { } opts := tc.opts + fs := flag.NewFlagSet("", flag.ContinueOnError) AddFlags(fs, &opts) require.NoError(t, fs.Parse(tc.args)) require.Len(t, fs.Args(), 0) - // Then create a builder with the flags. - patchLoadOptsShims(&opts) - b, err := newBuilder(opts) - require.NoError(t, err) - - // read the source fragments for i, data := range tc.source(format) { - b.Sources = append(b.Sources, FileSource{ + opts.sources = append(opts.sources, FileSource{ Name: fmt.Sprintf("src-%d.%s", i, format), Format: format, Data: data, }) } - // build/merge the config fragments - actual, err := b.BuildAndValidate() + patchLoadOptsShims(&opts) + result, err := Load(opts) switch { case err == nil && tc.expectedErr != "": t.Fatalf("got nil want error to contain %q", tc.expectedErr) @@ -5130,7 +5124,7 @@ func (tc testCase) run(format string, dataDir string) func(t *testing.T) { if tc.expectedErr != "" { return } - require.Equal(t, tc.expectedWarnings, b.Warnings, "warnings") + require.Equal(t, tc.expectedWarnings, result.Warnings, "warnings") // build a default configuration, then patch the fields we expect to change // and compare it with the generated configuration. Since the expected @@ -5140,12 +5134,13 @@ func (tc testCase) run(format string, dataDir string) func(t *testing.T) { x, err := newBuilder(expectedOpts) require.NoError(t, err) - expected, err := x.Build() + expected, err := x.build() require.NoError(t, err) if tc.expected != nil { tc.expected(&expected) } + actual := *result.RuntimeConfig // both DataDir fields should always be the same, so test for the // invariant, and than updated the expected, so that every test // case does not need to set this field.