config: move testing shims to LoadOpts

This will allow us to eventually port these tests to use config.Load instead of
unexported functions.

agent/config/builder.go

@@ -480,25 +480,7 @@ func (b *builder) Build() (rt RuntimeConfig, err error) {
 	advertiseAddr := makeIPAddr(b.expandFirstIP("advertise_addr", c.AdvertiseAddrLAN), bindAddr)
 
 	if ipaddr.IsAny(advertiseAddr) {
-
+		addrtyp, detect := advertiseAddrFunc(b.opts, advertiseAddr)
-		var addrtyp string
-		var detect func() ([]*net.IPAddr, error)
-		switch {
-		case ipaddr.IsAnyV4(advertiseAddr):
-			addrtyp = "private IPv4"
-			detect = b.opts.getPrivateIPv4
-			if detect == nil {
-				detect = ipaddr.GetPrivateIPv4
-			}
-
-		case ipaddr.IsAnyV6(advertiseAddr):
-			addrtyp = "public IPv6"
-			detect = b.opts.getPublicIPv6
-			if detect == nil {
-				detect = ipaddr.GetPublicIPv6
-			}
-		}
-
 		advertiseAddrs, err := detect()
 		if err != nil {
 			return RuntimeConfig{}, fmt.Errorf("Error detecting %s address: %s", addrtyp, err)
@@ -1150,6 +1132,27 @@ func (b *builder) Build() (rt RuntimeConfig, err error) {
 	return rt, nil
 }
 
+func advertiseAddrFunc(opts LoadOpts, advertiseAddr *net.IPAddr) (string, func() ([]*net.IPAddr, error)) {
+	switch {
+	case ipaddr.IsAnyV4(advertiseAddr):
+		fn := opts.getPrivateIPv4
+		if fn == nil {
+			fn = ipaddr.GetPrivateIPv4
+		}
+		return "private IPv4", fn
+
+	case ipaddr.IsAnyV6(advertiseAddr):
+		fn := opts.getPublicIPv6
+		if fn == nil {
+			fn = ipaddr.GetPublicIPv6
+		}
+		return "public IPv6", fn
+
+	default:
+		panic("unsupported net.IPAddr Type")
+	}
+}
+
 // reBasicName validates that a field contains only lower case alphanumerics,
 // underscore and dash and is non-empty.
 var reBasicName = regexp.MustCompile("^[a-z0-9_-]+$")

agent/config/builder_test.go

@@ -142,7 +142,7 @@ func TestBuilder_BuildAndValidate_NodeName(t *testing.T) {
 				DataDir:  pString("dir"),
 			},
 		})
-		patchBuilderShims(b)
+		patchLoadOptsShims(&b.opts)
 		require.NoError(t, err)
 		_, err = b.BuildAndValidate()
 		require.NoError(t, err)
@@ -175,15 +175,21 @@ func TestBuilder_BuildAndValidate_NodeName(t *testing.T) {
 	}
 }
 
-func patchBuilderShims(b *builder) {
+func patchLoadOptsShims(opts *LoadOpts) {
-	b.opts.hostname = func() (string, error) {
+	if opts.hostname == nil {
-		return "thehostname", nil
+		opts.hostname = func() (string, error) {
+			return "thehostname", nil
+		}
 	}
-	b.opts.getPrivateIPv4 = func() ([]*net.IPAddr, error) {
+	if opts.getPrivateIPv4 == nil {
-		return []*net.IPAddr{ipAddr("10.0.0.1")}, nil
+		opts.getPrivateIPv4 = func() ([]*net.IPAddr, error) {
+			return []*net.IPAddr{ipAddr("10.0.0.1")}, nil
+		}
 	}
-	b.opts.getPublicIPv6 = func() ([]*net.IPAddr, error) {
+	if opts.getPublicIPv6 == nil {
-		return []*net.IPAddr{ipAddr("dead:beef::1")}, nil
+		opts.getPublicIPv6 = func() ([]*net.IPAddr, error) {
+			return []*net.IPAddr{ipAddr("dead:beef::1")}, nil
+		}
 	}
 }
 

agent/config/runtime_test.go

@@ -38,25 +38,25 @@ import (
 type testCase struct {
 	desc  string
 	args  []string
-	pre   func()
+	pre   func() // setup(t *testing.T)
 	json  []string
 	hcl   []string
 	patch func(rt *RuntimeConfig) // expected
 	err   string                  // expectedErr
 	warns []string                // expectedWarnings
 
+	opts LoadOpts
+
+	// TODO: move all of these to opts
 	hcltail, jsontail []string
-	privatev4         func() ([]*net.IPAddr, error)
-	publicv6          func() ([]*net.IPAddr, error)
-	hostname          func() (string, error)
 }
 
 // TestConfigFlagsAndEdgecases tests the command line flags and
 // edgecases for the config parsing. It provides a test structure which
 // checks for warnings on deprecated fields and flags.  These tests
 // should check one option at a time if possible
-func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
+func TestLoad_IntegrationWithFlags(t *testing.T) {
-	dataDir := testutil.TempDir(t, "consul")
+	dataDir := testutil.TempDir(t, "config")
 
 	defaultEntMeta := structs.DefaultEnterpriseMeta()
 
@@ -942,8 +942,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 				}
 				rt.DataDir = dataDir
 			},
-			publicv6: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return []*net.IPAddr{ipAddr("dead:beef::1")}, nil
+				getPublicIPv6: func() ([]*net.IPAddr, error) {
+					return []*net.IPAddr{ipAddr("dead:beef::1")}, nil
+				},
 			},
 		},
 		{
@@ -969,8 +971,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 				}
 				rt.DataDir = dataDir
 			},
-			privatev4: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return nil, fmt.Errorf("should not detect advertise_addr")
+				getPrivateIPv4: func() ([]*net.IPAddr, error) {
+					return nil, fmt.Errorf("should not detect advertise_addr")
+				},
 			},
 		},
 		{
@@ -1648,8 +1652,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`{ "bind_addr": "0.0.0.0"}`},
 			hcl:  []string{`bind_addr = "0.0.0.0"`},
-			privatev4: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return nil, errors.New("some error")
+				getPrivateIPv4: func() ([]*net.IPAddr, error) {
+					return nil, errors.New("some error")
+				},
 			},
 			err: "Error detecting private IPv4 address: some error",
 		},
@@ -1658,8 +1664,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`{ "bind_addr": "0.0.0.0"}`},
 			hcl:  []string{`bind_addr = "0.0.0.0"`},
-			privatev4: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return nil, nil
+				getPrivateIPv4: func() ([]*net.IPAddr, error) {
+					return nil, nil
+				},
 			},
 			err: "No private IPv4 address found",
 		},
@@ -1668,8 +1676,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`{ "bind_addr": "0.0.0.0"}`},
 			hcl:  []string{`bind_addr = "0.0.0.0"`},
-			privatev4: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return []*net.IPAddr{ipAddr("1.1.1.1"), ipAddr("2.2.2.2")}, nil
+				getPrivateIPv4: func() ([]*net.IPAddr, error) {
+					return []*net.IPAddr{ipAddr("1.1.1.1"), ipAddr("2.2.2.2")}, nil
+				},
 			},
 			err: "Multiple private IPv4 addresses found. Please configure one",
 		},
@@ -1678,8 +1688,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`{ "bind_addr": "::"}`},
 			hcl:  []string{`bind_addr = "::"`},
-			publicv6: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return nil, errors.New("some error")
+				getPublicIPv6: func() ([]*net.IPAddr, error) {
+					return nil, errors.New("some error")
+				},
 			},
 			err: "Error detecting public IPv6 address: some error",
 		},
@@ -1688,8 +1700,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`{ "bind_addr": "::"}`},
 			hcl:  []string{`bind_addr = "::"`},
-			publicv6: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return nil, nil
+				getPublicIPv6: func() ([]*net.IPAddr, error) {
+					return nil, nil
+				},
 			},
 			err: "No public IPv6 address found",
 		},
@@ -1698,8 +1712,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`{ "bind_addr": "::"}`},
 			hcl:  []string{`bind_addr = "::"`},
-			publicv6: func() ([]*net.IPAddr, error) {
+			opts: LoadOpts{
-				return []*net.IPAddr{ipAddr("dead:beef::1"), ipAddr("dead:beef::2")}, nil
+				getPublicIPv6: func() ([]*net.IPAddr, error) {
+					return []*net.IPAddr{ipAddr("dead:beef::1"), ipAddr("dead:beef::2")}, nil
+				},
 			},
 			err: "Multiple public IPv6 addresses found. Please configure one",
 		},
@@ -1999,8 +2015,10 @@ func TestBuilder_BuildAndValidate_ConfigFlagsAndEdgecases(t *testing.T) {
 				`-data-dir=` + dataDir,
 				`-node=`,
 			},
-			hostname: func() (string, error) { return "", nil },
+			opts: LoadOpts{
-			err:      "node_name cannot be empty",
+				hostname: func() (string, error) { return "", nil },
+			},
+			err: "node_name cannot be empty",
 		},
 		{
 			desc: "node_meta key too long",
@@ -4873,13 +4891,10 @@ func testConfig(t *testing.T, tests []testCase, dataDir string) {
 			}
 
 			t.Run(strings.Join(desc, ":"), func(t *testing.T) {
-				flags := LoadOpts{}
+				opts := tt.opts
 				fs := flag.NewFlagSet("", flag.ContinueOnError)
-				AddFlags(fs, &flags)
+				AddFlags(fs, &opts)
-				err := fs.Parse(tt.args)
+				require.NoError(t, fs.Parse(tt.args))
-				if err != nil {
-					t.Fatalf("ParseFlags failed: %s", err)
-				}
 				require.Len(t, fs.Args(), 0)
 
 				if tt.pre != nil {
@@ -4887,20 +4902,10 @@ func testConfig(t *testing.T, tests []testCase, dataDir string) {
 				}
 
 				// Then create a builder with the flags.
-				b, err := newBuilder(flags)
+				patchLoadOptsShims(&opts)
+				b, err := newBuilder(opts)
 				require.NoError(t, err)
 
-				patchBuilderShims(b)
-				if tt.hostname != nil {
-					b.opts.hostname = tt.hostname
-				}
-				if tt.privatev4 != nil {
-					b.opts.getPrivateIPv4 = tt.privatev4
-				}
-				if tt.publicv6 != nil {
-					b.opts.getPublicIPv6 = tt.publicv6
-				}
-
 				// read the source fragments
 				for i, data := range srcs {
 					b.Sources = append(b.Sources, FileSource{
@@ -4938,11 +4943,11 @@ func testConfig(t *testing.T, tests []testCase, 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(LoadOpts{})
+				expectedOpts := LoadOpts{}
-				if err != nil {
+				patchLoadOptsShims(&expectedOpts)
-					t.Fatal(err)
+				x, err := newBuilder(expectedOpts)
-				}
+				require.NoError(t, err)
-				patchBuilderShims(x)
+
 				expected, err := x.Build()
 				require.NoError(t, err)
 				if tt.patch != nil {