Merge pull request #10567 from hashicorp/dnephin/config-unexport-build

config: unexport the remaining builder methods

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) {
+// build constructs the runtime configuration from the config sources
-	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
 // 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.
+// validate performs semantic validation of the runtime configuration.
-func (b *builder) Validate(rt RuntimeConfig) error {
+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/_-]+$`)

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.Len(t, result.Warnings, 1)
-		require.NoError(t, err)
+		require.Contains(t, result.Warnings[0], tc.expectedWarn)
-		require.Len(t, b.Warnings, 1)
-		require.Contains(t, b.Warnings[0], tc.expectedWarn)
 	}
 
 	var testCases = []testCase{

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
+		patchLoadOptsShims(&opts)
-		actual, err := b.BuildAndValidate()
+		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.