config: Make Source an interface

This will allow us to accept config from auto-config without needing to
go through a serialziation cycle.

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, config.Source{}, flat.overrides...)
+	config, warnings, err := autoconf.LoadConfig(flat.builderOpts, nil, flat.overrides...)
 	if err != nil {
 		return nil, err
 	}

agent/agent_endpoint_test.go

@@ -1332,7 +1332,7 @@ func TestAgent_Reload(t *testing.T) {
 		t.Fatal("missing redis service")
 	}
 
-	cfg2 := TestConfig(testutil.Logger(t), config.Source{
+	cfg2 := TestConfig(testutil.Logger(t), config.FileSource{
 		Name:   "reload",
 		Format: "hcl",
 		Data: `
@@ -1466,7 +1466,7 @@ func TestAgent_ReloadDoesNotTriggerWatch(t *testing.T) {
 	})
 
 	// Let's take almost the same config
-	cfg2 := TestConfig(testutil.Logger(t), config.Source{
+	cfg2 := TestConfig(testutil.Logger(t), config.FileSource{
 		Name:   "reload",
 		Format: "hcl",
 		Data: `

agent/agent_test.go

@@ -3669,7 +3669,7 @@ func TestAgent_ReloadConfigOutgoingRPCConfig(t *testing.T) {
 		key_file = "../test/key/ourdomain.key"
 		verify_server_hostname = true
 	`
-	c := TestConfig(testutil.Logger(t), config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	c := TestConfig(testutil.Logger(t), config.FileSource{Name: t.Name(), Format: "hcl", Data: hcl})
 	require.NoError(t, a.reloadConfigInternal(c))
 	tlsConf = a.tlsConfigurator.OutgoingRPCConfig()
 	require.False(t, tlsConf.InsecureSkipVerify)
@@ -3699,7 +3699,7 @@ func TestAgent_ReloadConfigAndKeepChecksStatus(t *testing.T) {
 		require.Equal(t, "passing", check.Status, "check %q is wrong", id)
 	}
 
-	c := TestConfig(testutil.Logger(t), config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	c := TestConfig(testutil.Logger(t), config.FileSource{Name: t.Name(), Format: "hcl", Data: hcl})
 	require.NoError(t, a.reloadConfigInternal(c))
 	// After reload, should be passing directly (no critical state)
 	for id, check := range a.State.Checks(nil) {
@@ -3738,7 +3738,7 @@ func TestAgent_ReloadConfigIncomingRPCConfig(t *testing.T) {
 		key_file = "../test/key/ourdomain.key"
 		verify_server_hostname = true
 	`
-	c := TestConfig(testutil.Logger(t), config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	c := TestConfig(testutil.Logger(t), config.FileSource{Name: t.Name(), Format: "hcl", Data: hcl})
 	require.NoError(t, a.reloadConfigInternal(c))
 	tlsConf, err = tlsConf.GetConfigForClient(nil)
 	require.NoError(t, err)
@@ -3767,7 +3767,7 @@ func TestAgent_ReloadConfigTLSConfigFailure(t *testing.T) {
 		data_dir = "` + dataDir + `"
 		verify_incoming = true
 	`
-	c := TestConfig(testutil.Logger(t), config.Source{Name: t.Name(), Format: "hcl", Data: hcl})
+	c := TestConfig(testutil.Logger(t), config.FileSource{Name: t.Name(), Format: "hcl", Data: hcl})
 	require.Error(t, a.reloadConfigInternal(c))
 	tlsConf, err := tlsConf.GetConfigForClient(nil)
 	require.NoError(t, err)

agent/auto-config/auto_config.go

@@ -105,7 +105,7 @@ func New(config *Config) (*AutoConfig, error) {
 // 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) {
-	src := config.Source{
+	src := config.FileSource{
 		Name:   autoConfigFileName,
 		Format: "json",
 		Data:   ac.autoConfigData,

agent/auto-config/auto_config_test.go

@@ -126,12 +126,12 @@ func TestLoadConfig(t *testing.T) {
 		DevMode: &devMode,
 	}
 
-	cfg, warnings, err := LoadConfig(builderOpts, config.Source{
+	cfg, warnings, err := LoadConfig(builderOpts, config.FileSource{
 		Name:   "test",
 		Format: "hcl",
 		Data:   `node_name = "hobbiton"`,
 	},
-		config.Source{
+		config.FileSource{
 			Name:   "overrides",
 			Format: "json",
 			Data:   `{"check_reap_interval": "1ms"}`,

agent/auto-config/builder.go

@@ -13,7 +13,7 @@ func LoadConfig(builderOpts config.BuilderOpts, extraHead config.Source, overrid
 		return nil, nil, err
 	}
 
-	if extraHead.Data != "" {
+	if extraHead != nil {
 		b.Head = append(b.Head, extraHead)
 	}
 

agent/auto-config/config_translate_test.go

@@ -121,7 +121,12 @@ func TestConfig_translateConfig(t *testing.T) {
 	data, err := json.Marshal(translated)
 	require.NoError(t, err)
 
-	actual, _, err := config.Parse(string(data), "json")
+	src := config.FileSource{
+		Name:   "test",
+		Format: "json",
+		Data:   string(data),
+	}
+	actual, _, err := src.Parse()
 	require.NoError(t, err)
 	require.Equal(t, expected, &actual)
 }

agent/config/agent_limits_test.go

@@ -16,7 +16,7 @@ func TestBuildAndValidate_HTTPMaxConnsPerClientExceedsRLimit(t *testing.T) {
 		}`
 	b, err := NewBuilder(BuilderOpts{})
 	assert.NoError(t, err)
-	testsrc := Source{
+	testsrc := FileSource{
 		Name:   "test",
 		Format: "hcl",
 		Data: `
@@ -33,7 +33,7 @@ func TestBuildAndValidate_HTTPMaxConnsPerClientExceedsRLimit(t *testing.T) {
 	}
 	b.Head = append(b.Head, testsrc)
 	b.Tail = append(b.Tail, DefaultConsulSource(), DevConsulSource())
-	b.Tail = append(b.Head, Source{Name: "hcl", Format: "hcl", Data: hcl})
+	b.Tail = append(b.Head, FileSource{Name: "hcl", Format: "hcl", Data: hcl})
 
 	_, validationError := b.BuildAndValidate()
 	if validationError == nil {

agent/config/builder.go

@@ -93,7 +93,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) {
 		if err != nil {
 			panic(err)
 		}
-		return Source{Name: name, Format: "json", Data: string(b)}
+		return FileSource{Name: name, Format: "json", Data: string(b)}
 	}
 
 	b := &Builder{
@@ -121,7 +121,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) {
 	}
 	b.Tail = append(b.Tail, newSource("flags.values", values))
 	for i, s := range opts.HCL {
-		b.Tail = append(b.Tail, Source{
+		b.Tail = append(b.Tail, FileSource{
 			Name:   fmt.Sprintf("flags-%d.hcl", i),
 			Format: "hcl",
 			Data:   s,
@@ -207,12 +207,12 @@ func (b *Builder) sourcesFromPath(path string, format string) ([]Source, error)
 func newSourceFromFile(path string, format string) (Source, error) {
 	data, err := ioutil.ReadFile(path)
 	if err != nil {
-		return Source{}, fmt.Errorf("config: failed to read %s: %s", path, err)
+		return nil, fmt.Errorf("config: failed to read %s: %s", path, err)
 	}
 	if format == "" {
 		format = formatFromFileExtension(path)
 	}
-	return Source{Name: path, Data: string(data), Format: format}, nil
+	return FileSource{Name: path, Data: string(data), Format: format}, nil
 }
 
 // shouldParse file determines whether the file to be read is of a supported extension
@@ -271,12 +271,13 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 	// parse the config sources into a configuration
 	var c Config
 	for _, s := range srcs {
-		if s.Name == "" || s.Data == "" {
+
+		c2, md, err := s.Parse()
+		switch {
+		case err == ErrNoData:
 			continue
-		}
+		case err != nil:
-		c2, md, err := Parse(s.Data, s.Format)
+			return RuntimeConfig{}, fmt.Errorf("failed to parse %v: %w", s.Source(), err)
-		if err != nil {
-			return RuntimeConfig{}, fmt.Errorf("Error parsing %s: %s", s.Name, err)
 		}
 
 		var unusedErr error
@@ -289,7 +290,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 			}
 		}
 		if unusedErr != nil {
-			return RuntimeConfig{}, fmt.Errorf("Error parsing %s: %s", s.Name, unusedErr)
+			return RuntimeConfig{}, fmt.Errorf("failed to parse %v: %s", s.Source(), unusedErr)
 		}
 
 		// for now this is a soft failure that will cause warnings but not actual problems

agent/config/builder_test.go

@@ -38,10 +38,10 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) {
 	require.NoError(t, err)
 
 	expected := []Source{
-		{Name: paths[0], Format: "hcl", Data: "content a"},
+		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		{Name: paths[1], Format: "json", Data: "content b"},
+		FileSource{Name: paths[1], Format: "json", Data: "content b"},
-		{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
+		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
+		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
 	}
 	require.Equal(t, expected, b.Sources)
 	require.Len(t, b.Warnings, 2)
@@ -54,12 +54,12 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.
 	require.NoError(t, err)
 
 	expected := []Source{
-		{Name: paths[0], Format: "hcl", Data: "content a"},
+		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		{Name: paths[1], Format: "hcl", Data: "content b"},
+		FileSource{Name: paths[1], Format: "hcl", Data: "content b"},
-		{Name: paths[2], Format: "hcl", Data: "content c"},
+		FileSource{Name: paths[2], Format: "hcl", Data: "content c"},
-		{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
+		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b"},
+		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b"},
-		{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c"},
+		FileSource{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c"},
 	}
 	require.Equal(t, expected, b.Sources)
 }

agent/config/config.go

@@ -14,27 +14,52 @@ const (
 	SerfWANKeyring = "serf/remote.keyring"
 )
 
-type Source struct {
+// Source parses configuration from some source.
+type Source interface {
+	// Source returns an identifier for the Source that can be used in error message
+	Source() string
+	// Parse a configuration and return the result.
+	Parse() (Config, mapstructure.Metadata, error)
+}
+
+// ErrNoData indicates to Builder.Build that the source contained no data, and
+// it can be skipped.
+var ErrNoData = fmt.Errorf("config source contained no data")
+
+// FileSource implements Source and parses a config from a file.
+type FileSource struct {
 	Name   string
 	Format string
 	Data   string
 }
 
-// Parse parses a config fragment in either JSON or HCL format.
+func (f FileSource) Source() string {
-func Parse(data string, format string) (c Config, md mapstructure.Metadata, err error) {
+	return f.Name
-	var raw map[string]interface{}
-	switch format {
-	case "json":
-		err = json.Unmarshal([]byte(data), &raw)
-	case "hcl":
-		err = hcl.Decode(&raw, data)
-	default:
-		err = fmt.Errorf("invalid format: %s", format)
-	}
-	if err != nil {
-		return Config{}, mapstructure.Metadata{}, err
 }
 
+// Parse a config file in either JSON or HCL format.
+func (f FileSource) Parse() (Config, mapstructure.Metadata, error) {
+	// TODO: remove once rawSource is used instead of a FileSource with no data.
+	if f.Name == "" || f.Data == "" {
+		return Config{}, mapstructure.Metadata{}, ErrNoData
+	}
+
+	var raw map[string]interface{}
+	var err error
+	var md mapstructure.Metadata
+	switch f.Format {
+	case "json":
+		err = json.Unmarshal([]byte(f.Data), &raw)
+	case "hcl":
+		err = hcl.Decode(&raw, f.Data)
+	default:
+		err = fmt.Errorf("invalid format: %s", f.Format)
+	}
+	if err != nil {
+		return Config{}, md, err
+	}
+
+	var c Config
 	d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
 			// decode.HookWeakDecodeFromSlice is only necessary when reading from
@@ -49,10 +74,10 @@ func Parse(data string, format string) (c Config, md mapstructure.Metadata, err
 		Result:   &c,
 	})
 	if err != nil {
-		return Config{}, mapstructure.Metadata{}, err
+		return Config{}, md, err
 	}
 	if err := d.Decode(raw); err != nil {
-		return Config{}, mapstructure.Metadata{}, err
+		return Config{}, md, err
 	}
 
 	return c, md, nil

agent/config/default.go

@@ -12,9 +12,7 @@ import (
 
 // DefaultSource is the default agent configuration.
 // This needs to be merged first in the head.
-// todo(fs): The values are sourced from multiple sources.
+// TODO: return a rawSource (no decoding) instead of a FileSource
-// todo(fs): IMO, this should be the definitive default for all configurable values
-// todo(fs): and whatever is in here should clobber every default value. Hence, no sourcing.
 func DefaultSource() Source {
 	cfg := consul.DefaultConfig()
 	serfLAN := cfg.SerfLANConfig.MemberlistConfig
@@ -25,7 +23,7 @@ func DefaultSource() Source {
 	//   acl stanza for now we need to be able to detect the new entries not being set (not
 	//   just set to the defaults here) so that we can use the old entries. So the true
 	//   default still needs to reside in the original config values
-	return Source{
+	return FileSource{
 		Name:   "default",
 		Format: "hcl",
 		Data: `
@@ -131,8 +129,9 @@ func DefaultSource() Source {
 
 // DevSource is the additional default configuration for dev mode.
 // This should be merged in the head after the default configuration.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func DevSource() Source {
-	return Source{
+	return FileSource{
 		Name:   "dev",
 		Format: "hcl",
 		Data: `
@@ -171,8 +170,9 @@ func DevSource() Source {
 
 // NonUserSource contains the values the user cannot configure.
 // This needs to be merged in the tail.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func NonUserSource() Source {
-	return Source{
+	return FileSource{
 		Name:   "non-user",
 		Format: "hcl",
 		Data: `
@@ -203,8 +203,9 @@ func NonUserSource() Source {
 // VersionSource creates a config source for the version parameters.
 // This should be merged in the tail since these values are not
 // user configurable.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func VersionSource(rev, ver, verPre string) Source {
-	return Source{
+	return FileSource{
 		Name:   "version",
 		Format: "hcl",
 		Data:   fmt.Sprintf(`revision = %q version = %q version_prerelease = %q`, rev, ver, verPre),
@@ -219,10 +220,11 @@ func DefaultVersionSource() Source {
 
 // DefaultConsulSource returns the default configuration for the consul agent.
 // This should be merged in the tail since these values are not user configurable.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func DefaultConsulSource() Source {
 	cfg := consul.DefaultConfig()
 	raft := cfg.RaftConfig
-	return Source{
+	return FileSource{
 		Name:   "consul",
 		Format: "hcl",
 		Data: `
@@ -247,8 +249,9 @@ func DefaultConsulSource() Source {
 
 // DevConsulSource returns the consul agent configuration for the dev mode.
 // This should be merged in the tail after the DefaultConsulSource.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func DevConsulSource() Source {
-	return Source{
+	return FileSource{
 		Name:   "consul-dev",
 		Format: "hcl",
 		Data: `

agent/config/default_oss.go

@@ -5,8 +5,9 @@ package config
 // DefaultEnterpriseSource returns the consul agent configuration for enterprise mode.
 // These can be overridden by the user and therefore this source should be merged in the
 // head and processed before user configuration.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func DefaultEnterpriseSource() Source {
-	return Source{
+	return FileSource{
 		Name:   "enterprise-defaults",
 		Format: "hcl",
 		Data:   ``,
@@ -15,8 +16,9 @@ func DefaultEnterpriseSource() Source {
 
 // OverrideEnterpriseSource returns the consul agent configuration for the enterprise mode.
 // This should be merged in the tail after the DefaultConsulSource.
+// TODO: return a rawSource (no decoding) instead of a FileSource
 func OverrideEnterpriseSource() Source {
-	return Source{
+	return FileSource{
 		Name:   "enterprise-overrides",
 		Format: "hcl",
 		Data:   ``,

agent/config/runtime_test.go

@@ -1584,7 +1584,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
 			args: []string{`-data-dir=` + dataDir},
 			json: []string{`this is not JSON`},
 			hcl:  []string{`*** 0123 this is not HCL`},
-			err:  "Error parsing",
+			err:  "failed to parse",
 		},
 		{
 			desc: "datacenter is lower-cased",
@@ -4312,14 +4312,14 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) {
 
 				// read the source fragements
 				for i, data := range srcs {
-					b.Sources = append(b.Sources, Source{
+					b.Sources = append(b.Sources, FileSource{
 						Name:   fmt.Sprintf("src-%d.%s", i, format),
 						Format: format,
 						Data:   data,
 					})
 				}
 				for i, data := range tails {
-					b.Tail = append(b.Tail, Source{
+					b.Tail = append(b.Tail, FileSource{
 						Name:   fmt.Sprintf("tail-%d.%s", i, format),
 						Format: format,
 						Data:   data,
@@ -5727,7 +5727,7 @@ func TestFullConfig(t *testing.T) {
 
 	tail := map[string][]Source{
 		"json": {
-			{
+			FileSource{
 				Name:   "tail.non-user.json",
 				Format: "json",
 				Data: `
@@ -5746,7 +5746,7 @@ func TestFullConfig(t *testing.T) {
 					"sync_coordinate_rate_target": 137.81
 				}`,
 			},
-			{
+			FileSource{
 				Name:   "tail.consul.json",
 				Format: "json",
 				Data: `
@@ -5770,7 +5770,7 @@ func TestFullConfig(t *testing.T) {
 			},
 		},
 		"hcl": {
-			{
+			FileSource{
 				Name:   "tail.non-user.hcl",
 				Format: "hcl",
 				Data: `
@@ -5788,7 +5788,7 @@ func TestFullConfig(t *testing.T) {
 					sync_coordinate_rate_target = 137.81
 				`,
 			},
-			{
+			FileSource{
 				Name:   "tail.consul.hcl",
 				Format: "hcl",
 				Data: `
@@ -6525,7 +6525,7 @@ func TestFullConfig(t *testing.T) {
 			if err != nil {
 				t.Fatalf("NewBuilder: %s", err)
 			}
-			b.Sources = append(b.Sources, Source{Name: "full." + format, Data: data, Format: format})
+			b.Sources = append(b.Sources, FileSource{Name: "full." + format, Data: data, Format: format})
 			b.Tail = append(b.Tail, tail[format]...)
 			b.Tail = append(b.Tail, VersionSource("JNtPSav3", "R909Hblt", "ZT1JOQLn"))
 

agent/testagent.go

@@ -212,7 +212,7 @@ func (a *TestAgent) Start(t *testing.T) (err error) {
 				hclDataDir,
 			},
 		}),
-		WithOverrides(config.Source{
+		WithOverrides(config.FileSource{
 			Name:   "test-overrides",
 			Format: "hcl",
 			Data:   a.Overrides},
@@ -466,7 +466,7 @@ func NodeID() string {
 // agent.
 func TestConfig(logger hclog.Logger, sources ...config.Source) *config.RuntimeConfig {
 	nodeID := NodeID()
-	testsrc := config.Source{
+	testsrc := config.FileSource{
 		Name:   "test",
 		Format: "hcl",
 		Data: `

agent/testagent_test.go

@@ -13,7 +13,7 @@ func TestDefaultConfig(t *testing.T) {
 		t.Run("", func(t *testing.T) {
 			t.Parallel()
 			var c config.Config
-			data := config.DefaultSource().Data
+			data := config.DefaultSource().(config.FileSource).Data
 			hcl.Decode(&c, data)
 			hcl.Decode(&c, data)
 			hcl.Decode(&c, data)