Merge pull request #8500 from hashicorp/dnephin/auto-config-loader

auto-config: reduce awareness of config

agent/agent.go

@@ -433,7 +433,7 @@ func New(options ...AgentOption) (*Agent, error) {
 	}
 
 	// parse the configuration and handle the error/warnings
-	config, warnings, err := autoconf.LoadConfig(flat.builderOpts, nil, flat.overrides...)
+	cfg, warnings, err := config.Load(flat.builderOpts, nil, flat.overrides...)
 	if err != nil {
 		return nil, err
 	}
@@ -449,7 +449,7 @@ func New(options ...AgentOption) (*Agent, error) {
 
 	// set the config in the agent, this is just the preliminary configuration as we haven't
 	// loaded any auto-config sources yet.
-	a.config = config
+	a.config = cfg
 
 	// create the cache using the rate limiting settings from the config. Note that this means
 	// that these limits are not reloadable.
@@ -457,15 +457,15 @@ func New(options ...AgentOption) (*Agent, error) {
 
 	if flat.logger == nil {
 		logConf := &logging.Config{
-			LogLevel:          config.LogLevel,
+			LogLevel:          cfg.LogLevel,
-			LogJSON:           config.LogJSON,
+			LogJSON:           cfg.LogJSON,
 			Name:              logging.Agent,
-			EnableSyslog:      config.EnableSyslog,
+			EnableSyslog:      cfg.EnableSyslog,
-			SyslogFacility:    config.SyslogFacility,
+			SyslogFacility:    cfg.SyslogFacility,
-			LogFilePath:       config.LogFile,
+			LogFilePath:       cfg.LogFile,
-			LogRotateDuration: config.LogRotateDuration,
+			LogRotateDuration: cfg.LogRotateDuration,
-			LogRotateBytes:    config.LogRotateBytes,
+			LogRotateBytes:    cfg.LogRotateBytes,
-			LogRotateMaxFiles: config.LogRotateMaxFiles,
+			LogRotateMaxFiles: cfg.LogRotateMaxFiles,
 		}
 
 		a.logger, err = logging.Setup(logConf, flat.writers)
@@ -477,7 +477,7 @@ func New(options ...AgentOption) (*Agent, error) {
 	}
 
 	if flat.initTelemetry {
-		memSink, err := lib.InitTelemetry(config.Telemetry)
+		memSink, err := lib.InitTelemetry(cfg.Telemetry)
 		if err != nil {
 			return nil, fmt.Errorf("Failed to initialize telemetry: %w", err)
 		}
@@ -539,12 +539,14 @@ func New(options ...AgentOption) (*Agent, error) {
 		return nil, err
 	}
 
-	acConf := new(autoconf.Config).
+	acConf := autoconf.Config{
-		WithDirectRPC(a.connPool).
+		DirectRPC:   a.connPool,
-		WithBuilderOpts(flat.builderOpts).
+		Logger:      a.logger,
-		WithLogger(a.logger).
+		CertMonitor: acCertMon,
-		WithOverrides(flat.overrides...).
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-		WithCertMonitor(acCertMon)
+			return config.Load(flat.builderOpts, source, flat.overrides...)
+		},
+	}
 	ac, err := autoconf.New(acConf)
 	if err != nil {
 		return nil, err

agent/auto-config/auto_config.go

@@ -54,26 +54,20 @@ var (
 // then we will need to add some locking here. I am deferring that for now
 // to help ease the review of this already large PR.
 type AutoConfig struct {
-	builderOpts        config.BuilderOpts
+	acConfig           Config
 	logger             hclog.Logger
-	directRPC          DirectRPC
-	waiter             *lib.RetryWaiter
-	overrides          []config.Source
 	certMonitor        CertMonitor
 	config             *config.RuntimeConfig
 	autoConfigResponse *pbautoconf.AutoConfigResponse
 	autoConfigSource   config.Source
-	cancel             context.CancelFunc
 }
 
-// New creates a new AutoConfig object for providing automatic
+// New creates a new AutoConfig object for providing automatic Consul configuration.
-// Consul configuration.
+func New(config Config) (*AutoConfig, error) {
-func New(config *Config) (*AutoConfig, error) {
+	switch {
-	if config == nil {
+	case config.Loader == nil:
-		return nil, fmt.Errorf("must provide a config struct")
+		return nil, fmt.Errorf("must provide a config loader")
-	}
+	case config.DirectRPC == nil:
-
-	if config.DirectRPC == nil {
 		return nil, fmt.Errorf("must provide a direct RPC delegate")
 	}
 
@@ -84,27 +78,21 @@ func New(config *Config) (*AutoConfig, error) {
 		logger = logger.Named(logging.AutoConfig)
 	}
 
-	waiter := config.Waiter
+	if config.Waiter == nil {
-	if waiter == nil {
+		config.Waiter = lib.NewRetryWaiter(1, 0, 10*time.Minute, lib.NewJitterRandomStagger(25))
-		waiter = lib.NewRetryWaiter(1, 0, 10*time.Minute, lib.NewJitterRandomStagger(25))
 	}
 
-	ac := &AutoConfig{
+	return &AutoConfig{
-		builderOpts: config.BuilderOpts,
+		acConfig:    config,
 		logger:      logger,
-		directRPC:   config.DirectRPC,
-		waiter:      waiter,
-		overrides:   config.Overrides,
 		certMonitor: config.CertMonitor,
-	}
+	}, nil
-
-	return ac, nil
 }
 
 // ReadConfig will parse the current configuration and inject any
 // auto-config sources if present into the correct place in the parsing chain.
 func (ac *AutoConfig) ReadConfig() (*config.RuntimeConfig, error) {
-	cfg, warnings, err := LoadConfig(ac.builderOpts, ac.autoConfigSource, ac.overrides...)
+	cfg, warnings, err := ac.acConfig.Loader(ac.autoConfigSource)
 	if err != nil {
 		return cfg, err
 	}
@@ -377,7 +365,7 @@ func (ac *AutoConfig) getInitialConfigurationOnce(ctx context.Context, csr strin
 			}
 
 			ac.logger.Debug("making AutoConfig.InitialConfiguration RPC", "addr", addr.String())
-			if err = ac.directRPC.RPC(ac.config.Datacenter, ac.config.NodeName, &addr, "AutoConfig.InitialConfiguration", &request, &resp); err != nil {
+			if err = ac.acConfig.DirectRPC.RPC(ac.config.Datacenter, ac.config.NodeName, &addr, "AutoConfig.InitialConfiguration", &request, &resp); err != nil {
 				ac.logger.Error("AutoConfig.InitialConfiguration RPC failed", "addr", addr.String(), "error", err)
 				continue
 			}
@@ -405,7 +393,7 @@ func (ac *AutoConfig) getInitialConfiguration(ctx context.Context) error {
 	}
 
 	// this resets the failures so that we will perform immediate request
-	wait := ac.waiter.Success()
+	wait := ac.acConfig.Waiter.Success()
 	for {
 		select {
 		case <-wait:
@@ -417,7 +405,7 @@ func (ac *AutoConfig) getInitialConfiguration(ctx context.Context) error {
 			} else {
 				ac.logger.Error("No error returned when fetching the initial auto-configuration but no response was either")
 			}
-			wait = ac.waiter.Failed()
+			wait = ac.acConfig.Waiter.Failed()
 		case <-ctx.Done():
 			ac.logger.Info("interrupted during initial auto configuration", "err", ctx.Err())
 			return ctx.Err()

agent/auto-config/auto_config_test.go

@@ -87,11 +87,23 @@ func TestNew(t *testing.T) {
 
 	cases := map[string]testCase{
 		"no-direct-rpc": {
+			config: Config{
+				Loader: func(source config.Source) (cfg *config.RuntimeConfig, warnings []string, err error) {
+					return nil, nil, nil
+				},
+			},
 			err: "must provide a direct RPC delegate",
 		},
+
+		"no-config-loader": {
+			err: "must provide a config loader",
+		},
 		"ok": {
 			config: Config{
 				DirectRPC: &mockDirectRPC{},
+				Loader: func(source config.Source) (cfg *config.RuntimeConfig, warnings []string, err error) {
+					return nil, nil, nil
+				},
 			},
 			validate: func(t *testing.T, ac *AutoConfig) {
 				t.Helper()
@@ -102,7 +114,7 @@ func TestNew(t *testing.T) {
 
 	for name, tcase := range cases {
 		t.Run(name, func(t *testing.T) {
-			ac, err := New(&tcase.config)
+			ac, err := New(tcase.config)
 			if tcase.err != "" {
 				testutil.RequireErrorContains(t, err, tcase.err)
 			} else {
@@ -116,99 +128,64 @@ func TestNew(t *testing.T) {
 	}
 }
 
-func TestLoadConfig(t *testing.T) {
-	// Basically just testing that injection of the extra
-	// source works.
-	devMode := true
-	builderOpts := config.BuilderOpts{
-		// putting this in dev mode so that the config validates
-		// without having to specify a data directory
-		DevMode: &devMode,
-	}
-
-	cfg, warnings, err := LoadConfig(builderOpts, config.FileSource{
-		Name:   "test",
-		Format: "hcl",
-		Data:   `node_name = "hobbiton"`,
-	},
-		config.FileSource{
-			Name:   "overrides",
-			Format: "json",
-			Data:   `{"check_reap_interval": "1ms"}`,
-		})
-
-	require.NoError(t, err)
-	require.Empty(t, warnings)
-	require.NotNil(t, cfg)
-	require.Equal(t, "hobbiton", cfg.NodeName)
-	require.Equal(t, 1*time.Millisecond, cfg.CheckReapInterval)
-}
-
 func TestReadConfig(t *testing.T) {
 	// just testing that some auto config source gets injected
-	devMode := true
 	ac := AutoConfig{
 		autoConfigSource: config.LiteralSource{
 			Name:   autoConfigFileName,
 			Config: config.Config{NodeName: stringPointer("hobbiton")},
 		},
-		builderOpts: config.BuilderOpts{
-			// putting this in dev mode so that the config validates
-			// without having to specify a data directory
-			DevMode: &devMode,
-		},
 		logger: testutil.Logger(t),
+		acConfig: Config{
+			Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
+				cfg, _, err := source.Parse()
+				if err != nil {
+					return nil, nil, err
+				}
+				return &config.RuntimeConfig{
+					DevMode:  true,
+					NodeName: *cfg.NodeName,
+				}, nil, nil
+			},
+		},
 	}
 
 	cfg, err := ac.ReadConfig()
 	require.NoError(t, err)
 	require.NotNil(t, cfg)
 	require.Equal(t, "hobbiton", cfg.NodeName)
+	require.True(t, cfg.DevMode)
 	require.Same(t, ac.config, cfg)
 }
 
-func testSetupAutoConf(t *testing.T) (string, string, config.BuilderOpts) {
+func setupRuntimeConfig(t *testing.T) *config.RuntimeConfig {
 	t.Helper()
 
-	// create top level directory to hold both config and data
+	dataDir := testutil.TempDir(t, "auto-config")
-	tld := testutil.TempDir(t, "auto-config")
+	t.Cleanup(func() { os.RemoveAll(dataDir) })
-	t.Cleanup(func() { os.RemoveAll(tld) })
 
-	// create the data directory
+	rtConfig := &config.RuntimeConfig{
-	dataDir := filepath.Join(tld, "data")
+		DataDir:    dataDir,
-	require.NoError(t, os.Mkdir(dataDir, 0700))
+		Datacenter: "dc1",
-
+		NodeName:   "autoconf",
-	// create the config directory
+		BindAddr:   &net.IPAddr{IP: net.ParseIP("127.0.0.1")},
-	configDir := filepath.Join(tld, "config")
-	require.NoError(t, os.Mkdir(configDir, 0700))
-
-	builderOpts := config.BuilderOpts{
-		HCL: []string{
-			`data_dir = "` + dataDir + `"`,
-			`datacenter = "dc1"`,
-			`node_name = "autoconf"`,
-			`bind_addr = "127.0.0.1"`,
-		},
 	}
-
+	return rtConfig
-	return dataDir, configDir, builderOpts
 }
 
 func TestInitialConfiguration_disabled(t *testing.T) {
-	dataDir, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
 
-	cfgFile := filepath.Join(configDir, "test.json")
+	directRPC := new(mockDirectRPC)
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
+	directRPC.Test(t)
-		"primary_datacenter": "primary", 
+	conf := Config{
-		"auto_config": {"enabled": false}
+		DirectRPC: directRPC,
-	}`), 0600))
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-
+			rtConfig.PrimaryDatacenter = "primary"
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
+			rtConfig.AutoConfig.Enabled = false
-
+			return rtConfig, nil, nil
-	directRPC := mockDirectRPC{}
+		},
-	conf := new(Config).
+	}
-		WithBuilderOpts(builderOpts).
-		WithDirectRPC(&directRPC)
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)
@@ -217,26 +194,17 @@ func TestInitialConfiguration_disabled(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, cfg)
 	require.Equal(t, "primary", cfg.PrimaryDatacenter)
-	require.NoFileExists(t, filepath.Join(dataDir, autoConfigFileName))
+	require.NoFileExists(t, filepath.Join(rtConfig.DataDir, autoConfigFileName))
 
 	// ensure no RPC was made
 	directRPC.AssertExpectations(t)
 }
 
 func TestInitialConfiguration_cancelled(t *testing.T) {
-	_, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
-
-	cfgFile := filepath.Join(configDir, "test.json")
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
-		"primary_datacenter": "primary", 
-		"auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]},
-		"verify_outgoing": true
-	}`), 0600))
-
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
-
-	directRPC := mockDirectRPC{}
 
+	directRPC := new(mockDirectRPC)
+	directRPC.Test(t)
 	expectedRequest := pbautoconf.AutoConfigRequest{
 		Datacenter: "dc1",
 		Node:       "autoconf",
@@ -244,9 +212,19 @@ func TestInitialConfiguration_cancelled(t *testing.T) {
 	}
 
 	directRPC.On("RPC", "dc1", "autoconf", &net.TCPAddr{IP: net.IPv4(127, 0, 0, 1), Port: 8300}, "AutoConfig.InitialConfiguration", &expectedRequest, mock.Anything).Return(fmt.Errorf("injected error")).Times(0)
-	conf := new(Config).
+	conf := Config{
-		WithBuilderOpts(builderOpts).
+		DirectRPC: directRPC,
-		WithDirectRPC(&directRPC)
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
+			rtConfig.PrimaryDatacenter = "primary"
+			rtConfig.AutoConfig = config.AutoConfig{
+				Enabled:         true,
+				IntroToken:      "blarg",
+				ServerAddresses: []string{"127.0.0.1:8300"},
+			}
+			rtConfig.VerifyOutgoing = true
+			return rtConfig, nil, nil
+		},
+	}
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)
@@ -263,17 +241,10 @@ func TestInitialConfiguration_cancelled(t *testing.T) {
 }
 
 func TestInitialConfiguration_restored(t *testing.T) {
-	dataDir, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
-
-	cfgFile := filepath.Join(configDir, "test.json")
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
-		"auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]}, "verify_outgoing": true
-	}`), 0600))
-
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
 
 	// persist an auto config response to the data dir where it is expected
-	persistedFile := filepath.Join(dataDir, autoConfigFileName)
+	persistedFile := filepath.Join(rtConfig.DataDir, autoConfigFileName)
 	response := &pbautoconf.AutoConfigResponse{
 		Config: &pbconfig.Config{
 			PrimaryDatacenter: "primary",
@@ -312,11 +283,13 @@ func TestInitialConfiguration_restored(t *testing.T) {
 	require.NoError(t, err)
 	require.NoError(t, ioutil.WriteFile(persistedFile, []byte(data), 0600))
 
-	directRPC := mockDirectRPC{}
+	directRPC := new(mockDirectRPC)
+	directRPC.Test(t)
 
 	// setup the mock certificate monitor to ensure that the initial state gets
 	// updated appropriately during config restoration.
-	certMon := mockCertMonitor{}
+	certMon := new(mockCertMonitor)
+	certMon.Test(t)
 	certMon.On("Update", &structs.SignedResponse{
 		IssuedCert: structs.IssuedCert{
 			SerialNumber:  "1234",
@@ -349,10 +322,22 @@ func TestInitialConfiguration_restored(t *testing.T) {
 		VerifyServerHostname: true,
 	}).Return(nil).Once()
 
-	conf := new(Config).
+	conf := Config{
-		WithBuilderOpts(builderOpts).
+		DirectRPC: directRPC,
-		WithDirectRPC(&directRPC).
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-		WithCertMonitor(&certMon)
+			if err := setPrimaryDatacenterFromSource(rtConfig, source); err != nil {
+				return nil, nil, err
+			}
+			rtConfig.AutoConfig = config.AutoConfig{
+				Enabled:         true,
+				IntroToken:      "blarg",
+				ServerAddresses: []string{"127.0.0.1:8300"},
+			}
+			rtConfig.VerifyOutgoing = true
+			return rtConfig, nil, nil
+		},
+		CertMonitor: certMon,
+	}
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)
@@ -367,18 +352,22 @@ func TestInitialConfiguration_restored(t *testing.T) {
 	certMon.AssertExpectations(t)
 }
 
+func setPrimaryDatacenterFromSource(rtConfig *config.RuntimeConfig, source config.Source) error {
+	if source != nil {
+		cfg, _, err := source.Parse()
+		if err != nil {
+			return err
+		}
+		rtConfig.PrimaryDatacenter = *cfg.PrimaryDatacenter
+	}
+	return nil
+}
+
 func TestInitialConfiguration_success(t *testing.T) {
-	dataDir, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
 
-	cfgFile := filepath.Join(configDir, "test.json")
+	directRPC := new(mockDirectRPC)
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
+	directRPC.Test(t)
-		"auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]}, "verify_outgoing": true
-	}`), 0600))
-
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
-
-	persistedFile := filepath.Join(dataDir, autoConfigFileName)
-	directRPC := mockDirectRPC{}
 
 	populateResponse := func(val interface{}) {
 		resp, ok := val.(*pbautoconf.AutoConfigResponse)
@@ -434,7 +423,8 @@ func TestInitialConfiguration_success(t *testing.T) {
 
 	// setup the mock certificate monitor to ensure that the initial state gets
 	// updated appropriately during config restoration.
-	certMon := mockCertMonitor{}
+	certMon := new(mockCertMonitor)
+	certMon.Test(t)
 	certMon.On("Update", &structs.SignedResponse{
 		IssuedCert: structs.IssuedCert{
 			SerialNumber:  "1234",
@@ -465,10 +455,22 @@ func TestInitialConfiguration_success(t *testing.T) {
 		VerifyServerHostname: true,
 	}).Return(nil).Once()
 
-	conf := new(Config).
+	conf := Config{
-		WithBuilderOpts(builderOpts).
+		DirectRPC: directRPC,
-		WithDirectRPC(&directRPC).
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-		WithCertMonitor(&certMon)
+			if err := setPrimaryDatacenterFromSource(rtConfig, source); err != nil {
+				return nil, nil, err
+			}
+			rtConfig.AutoConfig = config.AutoConfig{
+				Enabled:         true,
+				IntroToken:      "blarg",
+				ServerAddresses: []string{"127.0.0.1:8300"},
+			}
+			rtConfig.VerifyOutgoing = true
+			return rtConfig, nil, nil
+		},
+		CertMonitor: certMon,
+	}
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)
@@ -479,6 +481,7 @@ func TestInitialConfiguration_success(t *testing.T) {
 	require.Equal(t, "primary", cfg.PrimaryDatacenter)
 
 	// the file was written to.
+	persistedFile := filepath.Join(rtConfig.DataDir, autoConfigFileName)
 	require.FileExists(t, persistedFile)
 
 	// ensure no RPC was made
@@ -487,17 +490,10 @@ func TestInitialConfiguration_success(t *testing.T) {
 }
 
 func TestInitialConfiguration_retries(t *testing.T) {
-	dataDir, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
 
-	cfgFile := filepath.Join(configDir, "test.json")
+	directRPC := new(mockDirectRPC)
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
+	directRPC.Test(t)
-		"auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["198.18.0.1", "198.18.0.2:8398", "198.18.0.3:8399", "127.0.0.1:1234"]}, "verify_outgoing": true
-	}`), 0600))
-
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
-
-	persistedFile := filepath.Join(dataDir, autoConfigFileName)
-	directRPC := mockDirectRPC{}
 
 	populateResponse := func(val interface{}) {
 		resp, ok := val.(*pbautoconf.AutoConfigResponse)
@@ -557,11 +553,27 @@ func TestInitialConfiguration_retries(t *testing.T) {
 		&expectedRequest,
 		&pbautoconf.AutoConfigResponse{}).Return(populateResponse)
 
-	waiter := lib.NewRetryWaiter(2, 0, 1*time.Millisecond, nil)
+	conf := Config{
-	conf := new(Config).
+		DirectRPC: directRPC,
-		WithBuilderOpts(builderOpts).
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-		WithDirectRPC(&directRPC).
+			if err := setPrimaryDatacenterFromSource(rtConfig, source); err != nil {
-		WithRetryWaiter(waiter)
+				return nil, nil, err
+			}
+			rtConfig.AutoConfig = config.AutoConfig{
+				Enabled:    true,
+				IntroToken: "blarg",
+				ServerAddresses: []string{
+					"198.18.0.1:8300",
+					"198.18.0.2:8398",
+					"198.18.0.3:8399",
+					"127.0.0.1:1234",
+				},
+			}
+			rtConfig.VerifyOutgoing = true
+			return rtConfig, nil, nil
+		},
+		Waiter: lib.NewRetryWaiter(2, 0, 1*time.Millisecond, nil),
+	}
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)
@@ -572,6 +584,7 @@ func TestInitialConfiguration_retries(t *testing.T) {
 	require.Equal(t, "primary", cfg.PrimaryDatacenter)
 
 	// the file was written to.
+	persistedFile := filepath.Join(rtConfig.DataDir, autoConfigFileName)
 	require.FileExists(t, persistedFile)
 
 	// ensure no RPC was made
@@ -584,25 +597,34 @@ func TestAutoConfig_StartStop(t *testing.T) {
 	// stopped and not that anything with regards to running the cert monitor
 	// actually work. Those are tested in the cert-monitor package.
 
-	_, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
 
-	cfgFile := filepath.Join(configDir, "test.json")
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
-		"auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["198.18.0.1", "198.18.0.2:8398", "198.18.0.3:8399", "127.0.0.1:1234"]}, "verify_outgoing": true
-	}`), 0600))
-
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
 	directRPC := &mockDirectRPC{}
+	directRPC.Test(t)
 	certMon := &mockCertMonitor{}
+	certMon.Test(t)
 
 	certMon.On("Start").Return((<-chan struct{})(nil), nil).Once()
 	certMon.On("Stop").Return(true).Once()
 
-	conf := new(Config).
+	conf := Config{
-		WithBuilderOpts(builderOpts).
+		DirectRPC: directRPC,
-		WithDirectRPC(directRPC).
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-		WithCertMonitor(certMon)
+			rtConfig.AutoConfig = config.AutoConfig{
-
+				Enabled:    true,
+				IntroToken: "blarg",
+				ServerAddresses: []string{
+					"198.18.0.1",
+					"198.18.0.2:8398",
+					"198.18.0.3:8399",
+					"127.0.0.1:1234",
+				},
+			}
+			rtConfig.VerifyOutgoing = true
+			return rtConfig, nil, nil
+		},
+		CertMonitor: certMon,
+	}
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)
@@ -618,16 +640,10 @@ func TestAutoConfig_StartStop(t *testing.T) {
 }
 
 func TestFallBackTLS(t *testing.T) {
-	_, configDir, builderOpts := testSetupAutoConf(t)
+	rtConfig := setupRuntimeConfig(t)
 
-	cfgFile := filepath.Join(configDir, "test.json")
+	directRPC := new(mockDirectRPC)
-	require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{
+	directRPC.Test(t)
-		"auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]}, "verify_outgoing": true
-	}`), 0600))
-
-	builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile)
-
-	directRPC := mockDirectRPC{}
 
 	populateResponse := func(val interface{}) {
 		resp, ok := val.(*pbautoconf.AutoConfigResponse)
@@ -684,12 +700,21 @@ func TestFallBackTLS(t *testing.T) {
 	// setup the mock certificate monitor we don't expect it to be used
 	// as the FallbackTLS method is mainly used by the certificate monitor
 	// if for some reason it fails to renew the TLS certificate in time.
-	certMon := mockCertMonitor{}
+	certMon := new(mockCertMonitor)
 
-	conf := new(Config).
+	conf := Config{
-		WithBuilderOpts(builderOpts).
+		DirectRPC: directRPC,
-		WithDirectRPC(&directRPC).
+		Loader: func(source config.Source) (*config.RuntimeConfig, []string, error) {
-		WithCertMonitor(&certMon)
+			rtConfig.AutoConfig = config.AutoConfig{
+				Enabled:         true,
+				IntroToken:      "blarg",
+				ServerAddresses: []string{"127.0.0.1:8300"},
+			}
+			rtConfig.VerifyOutgoing = true
+			return rtConfig, nil, nil
+		},
+		CertMonitor: certMon,
+	}
 	ac, err := New(conf)
 	require.NoError(t, err)
 	require.NotNil(t, ac)

agent/auto-config/builder.go

@@ -1,30 +0,0 @@
-package autoconf
-
-import (
-	"github.com/hashicorp/consul/agent/config"
-)
-
-// LoadConfig will build the configuration including the extraHead source injected
-// after all other defaults but before any user supplied configuration and the overrides
-// source injected as the final source in the configuration parsing chain.
-func LoadConfig(builderOpts config.BuilderOpts, extraHead config.Source, overrides ...config.Source) (*config.RuntimeConfig, []string, error) {
-	b, err := config.NewBuilder(builderOpts)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	if extraHead != nil {
-		b.Head = append(b.Head, extraHead)
-	}
-
-	if len(overrides) != 0 {
-		b.Tail = append(b.Tail, overrides...)
-	}
-
-	cfg, err := b.BuildAndValidate()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return &cfg, b.Warnings, nil
-}

agent/auto-config/config.go

@@ -37,17 +37,6 @@ type Config struct {
 	// configuration. Setting this field is required.
 	DirectRPC DirectRPC
 
-	// BuilderOpts are any configuration building options that should be
-	// used when loading the Consul configuration. This is mostly a pass
-	// through from what the CLI will generate. While this option is
-	// not strictly required, not setting it will prevent AutoConfig
-	// from doing anything useful. Enabling AutoConfig requires a
-	// CLI flag or a config file (also specified by the CLI) flag.
-	// So without providing the opts its equivalent to using the
-	// configuration of not specifying anything to the consul agent
-	// cli.
-	BuilderOpts config.BuilderOpts
-
 	// Waiter is a RetryWaiter to be used during retrieval of the
 	// initial configuration. When a round of requests fails we will
 	// wait and eventually make another round of requests (1 round
@@ -60,56 +49,14 @@ type Config struct {
 	// having the test take minutes/hours to complete.
 	Waiter *lib.RetryWaiter
 
-	// Overrides are a list of configuration sources to append to the tail of
-	// the config builder. This field is optional and mainly useful for testing
-	// to override values that would be otherwise not user-settable.
-	Overrides []config.Source
-
 	// CertMonitor is the Connect TLS Certificate Monitor to be used for ongoing
 	// certificate renewals and connect CA roots updates. This field is not
 	// strictly required but if not provided the TLS certificates retrieved
 	// through by the AutoConfig.InitialConfiguration RPC will not be used
 	// or renewed.
 	CertMonitor CertMonitor
-}
 
-// WithLogger will cause the created AutoConfig type to use the provided logger
+	// Loader merges source with the existing FileSources and returns the complete
-func (c *Config) WithLogger(logger hclog.Logger) *Config {
+	// RuntimeConfig.
-	c.Logger = logger
+	Loader func(source config.Source) (cfg *config.RuntimeConfig, warnings []string, err error)
-	return c
-}
-
-// WithConnectionPool will cause the created AutoConfig type to use the provided connection pool
-func (c *Config) WithDirectRPC(directRPC DirectRPC) *Config {
-	c.DirectRPC = directRPC
-	return c
-}
-
-// WithBuilderOpts will cause the created AutoConfig type to use the provided CLI builderOpts
-func (c *Config) WithBuilderOpts(builderOpts config.BuilderOpts) *Config {
-	c.BuilderOpts = builderOpts
-	return c
-}
-
-// WithRetryWaiter will cause the created AutoConfig type to use the provided retry waiter
-func (c *Config) WithRetryWaiter(waiter *lib.RetryWaiter) *Config {
-	c.Waiter = waiter
-	return c
-}
-
-// WithOverrides is used to provide a config source to append to the tail sources
-// during config building. It is really only useful for testing to tune non-user
-// configurable tunables to make various tests converge more quickly than they
-// could otherwise.
-func (c *Config) WithOverrides(overrides ...config.Source) *Config {
-	c.Overrides = overrides
-	return c
-}
-
-// WithCertMonitor is used to provide a certificate monitor to the auto-config.
-// This monitor is responsible for renewing the agents TLS certificate and keeping
-// the connect CA roots up to date.
-func (c *Config) WithCertMonitor(certMonitor CertMonitor) *Config {
-	c.CertMonitor = certMonitor
-	return c
 }

agent/config/builder.go

@@ -33,6 +33,31 @@ import (
 	"golang.org/x/time/rate"
 )
 
+// LoadConfig will build the configuration including the extraHead source injected
+// after all other defaults but before any user supplied configuration and the overrides
+// source injected as the final source in the configuration parsing chain.
+func Load(builderOpts BuilderOpts, extraHead Source, overrides ...Source) (*RuntimeConfig, []string, error) {
+	b, err := NewBuilder(builderOpts)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	if extraHead != nil {
+		b.Head = append(b.Head, extraHead)
+	}
+
+	if len(overrides) != 0 {
+		b.Tail = append(b.Tail, overrides...)
+	}
+
+	cfg, err := b.BuildAndValidate()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	return &cfg, b.Warnings, nil
+}
+
 // Builder constructs a valid runtime configuration from multiple
 // configuration sources.
 //

agent/config/builder_test.go

@@ -6,10 +6,39 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 )
 
+func TestLoad(t *testing.T) {
+	// Basically just testing that injection of the extra
+	// source works.
+	devMode := true
+	builderOpts := BuilderOpts{
+		// putting this in dev mode so that the config validates
+		// without having to specify a data directory
+		DevMode: &devMode,
+	}
+
+	cfg, warnings, err := Load(builderOpts, FileSource{
+		Name:   "test",
+		Format: "hcl",
+		Data:   `node_name = "hobbiton"`,
+	},
+		FileSource{
+			Name:   "overrides",
+			Format: "json",
+			Data:   `{"check_reap_interval": "1ms"}`,
+		})
+
+	require.NoError(t, err)
+	require.Empty(t, warnings)
+	require.NotNil(t, cfg)
+	require.Equal(t, "hobbiton", cfg.NodeName)
+	require.Equal(t, 1*time.Millisecond, cfg.CheckReapInterval)
+}
+
 func TestShouldParseFile(t *testing.T) {
 	var testcases = []struct {
 		filename     string