Merge pull request #9456 from hashicorp/dnephin/config-deprecation

config: Use DeprecatedConfig struct for deprecated config fields

agent/config/builder.go

@@ -346,10 +346,12 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
 		for _, err := range validateEnterpriseConfigKeys(&c2) {
 			b.warn("%s", err)
 		}
+		b.Warnings = append(b.Warnings, md.Warnings...)
 
 		// if we have a single 'check' or 'service' we need to add them to the
 		// list of checks and services first since we cannot merge them
 		// generically and later values would clobber earlier ones.
+		// TODO: move to applyDeprecatedConfig
 		if c2.Check != nil {
 			c2.Checks = append(c2.Checks, *c2.Check)
 			c2.Check = nil
@@ -733,16 +735,6 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
 
 	aclsEnabled := false
 	primaryDatacenter := strings.ToLower(stringVal(c.PrimaryDatacenter))
-	if c.ACLDatacenter != nil {
-		b.warn("The 'acl_datacenter' field is deprecated. Use the 'primary_datacenter' field instead.")
-
-		if primaryDatacenter == "" {
-			primaryDatacenter = strings.ToLower(stringVal(c.ACLDatacenter))
-		}
-
-		// when the acl_datacenter config is used it implicitly enables acls
-		aclsEnabled = true
-	}
 
 	if c.ACL.Enabled != nil {
 		aclsEnabled = boolVal(c.ACL.Enabled)
@@ -887,7 +879,7 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
 			EnablePersistence:   boolValWithDefault(c.ACL.EnableTokenPersistence, false),
 			ACLDefaultToken:     stringValWithDefault(c.ACL.Tokens.Default, stringVal(c.ACLToken)),
 			ACLAgentToken:       stringValWithDefault(c.ACL.Tokens.Agent, stringVal(c.ACLAgentToken)),
-			ACLAgentMasterToken: stringValWithDefault(c.ACL.Tokens.AgentMaster, stringVal(c.ACLAgentMasterToken)),
+			ACLAgentMasterToken: stringVal(c.ACL.Tokens.AgentMaster),
 			ACLReplicationToken: stringValWithDefault(c.ACL.Tokens.Replication, stringVal(c.ACLReplicationToken)),
 		},
 

agent/config/config.go

@@ -15,7 +15,7 @@ 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)
+	Parse() (Config, Metadata, error)
 }
 
 // ErrNoData indicates to Builder.Build that the source contained no data, and
@@ -34,9 +34,10 @@ func (f FileSource) Source() string {
 }
 
 // Parse a config file in either JSON or HCL format.
-func (f FileSource) Parse() (Config, mapstructure.Metadata, error) {
+func (f FileSource) Parse() (Config, Metadata, error) {
+	m := Metadata{}
 	if f.Name == "" || f.Data == "" {
-		return Config{}, mapstructure.Metadata{}, ErrNoData
+		return Config{}, m, ErrNoData
 	}
 
 	var raw map[string]interface{}
@@ -51,10 +52,10 @@ func (f FileSource) Parse() (Config, mapstructure.Metadata, error) {
 		err = fmt.Errorf("invalid format: %s", f.Format)
 	}
 	if err != nil {
-		return Config{}, md, err
+		return Config{}, m, err
 	}
 
-	var c Config
+	var target decodeTarget
 	d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
 			// decode.HookWeakDecodeFromSlice is only necessary when reading from
@@ -66,16 +67,30 @@ func (f FileSource) Parse() (Config, mapstructure.Metadata, error) {
 			decode.HookTranslateKeys,
 		),
 		Metadata: &md,
-		Result:   &c,
+		Result:   &target,
 	})
 	if err != nil {
-		return Config{}, md, err
+		return Config{}, m, err
 	}
 	if err := d.Decode(raw); err != nil {
-		return Config{}, md, err
+		return Config{}, m, err
 	}
 
-	return c, md, nil
+	c, warns := applyDeprecatedConfig(&target)
+	m.Unused = md.Unused
+	m.Keys = md.Keys
+	m.Warnings = warns
+	return c, m, nil
+}
+
+// Metadata created by Source.Parse
+type Metadata struct {
+	// Keys used in the config file.
+	Keys []string
+	// Unused keys that did not match any struct fields.
+	Unused []string
+	// Warnings caused by deprecated fields
+	Warnings []string
 }
 
 // LiteralSource implements Source and returns an existing Config struct.
@@ -88,8 +103,13 @@ func (l LiteralSource) Source() string {
 	return l.Name
 }
 
-func (l LiteralSource) Parse() (Config, mapstructure.Metadata, error) {
-	return l.Config, mapstructure.Metadata{}, nil
+func (l LiteralSource) Parse() (Config, Metadata, error) {
+	return l.Config, Metadata{}, nil
+}
+
+type decodeTarget struct {
+	DeprecatedConfig `mapstructure:",squash"`
+	Config           `mapstructure:",squash"`
 }
 
 // Cache configuration for the agent/cache.
@@ -110,12 +130,8 @@ type Cache struct {
 // configuration it should be treated as an external API which cannot be
 // changed and refactored at will since this will break existing setups.
 type Config struct {
-	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl.tokens" stanza
-	ACLAgentMasterToken *string `mapstructure:"acl_agent_master_token"`
 	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl.tokens" stanza
 	ACLAgentToken *string `mapstructure:"acl_agent_token"`
-	// DEPRECATED (ACL-Legacy-Compat) - moved to "primary_datacenter"
-	ACLDatacenter *string `mapstructure:"acl_datacenter"`
 	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl" stanza
 	ACLDefaultPolicy *string `mapstructure:"acl_default_policy"`
 	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl" stanza

agent/config/deprecated.go

@@ -0,0 +1,42 @@
+package config
+
+import "fmt"
+
+type DeprecatedConfig struct {
+	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl.tokens" stanza
+	ACLAgentMasterToken *string `mapstructure:"acl_agent_master_token"`
+	// DEPRECATED (ACL-Legacy-Compat) - moved to "primary_datacenter"
+	ACLDatacenter *string `mapstructure:"acl_datacenter"`
+}
+
+func applyDeprecatedConfig(d *decodeTarget) (Config, []string) {
+	dep := d.DeprecatedConfig
+	var warns []string
+
+	if dep.ACLAgentMasterToken != nil {
+		if d.Config.ACL.Tokens.AgentMaster == nil {
+			d.Config.ACL.Tokens.AgentMaster = dep.ACLAgentMasterToken
+		}
+		warns = append(warns, deprecationWarning("acl_agent_master_token", "acl.tokens.agent_master"))
+	}
+
+	if dep.ACLDatacenter != nil {
+		if d.Config.PrimaryDatacenter == nil {
+			d.Config.PrimaryDatacenter = dep.ACLDatacenter
+		}
+
+		// when the acl_datacenter config is used it implicitly enables acls
+		d.Config.ACL.Enabled = pBool(true)
+		warns = append(warns, deprecationWarning("acl_datacenter", "primary_datacenter"))
+	}
+
+	return d.Config, warns
+}
+
+func deprecationWarning(old, new string) string {
+	return fmt.Sprintf("The '%v' field is deprecated. Use the '%v' field instead.", old, new)
+}
+
+func pBool(v bool) *bool {
+	return &v
+}

agent/config/merge_test.go

@@ -52,7 +52,6 @@ func TestMerge(t *testing.T) {
 	}
 }
 
-func pBool(v bool) *bool                { return &v }
 func pInt(v int) *int                   { return &v }
 func pString(v string) *string          { return &v }
 func pDuration(v time.Duration) *string { s := v.String(); return &s }

agent/config/runtime_test.go

@@ -5903,6 +5903,7 @@ func TestLoad_FullConfig(t *testing.T) {
 
 	expectedWarns := []string{
 		`The 'acl_datacenter' field is deprecated. Use the 'primary_datacenter' field instead.`,
+		`The 'acl_agent_master_token' field is deprecated. Use the 'acl.tokens.agent_master' field instead.`,
 		`bootstrap_expect > 0: expecting 53 servers`,
 	}
 	expectedWarns = append(expectedWarns, enterpriseConfigKeyWarnings...)

lib/decode/decode.go

@@ -73,12 +73,28 @@ func translationsForType(to reflect.Type) map[string]string {
 	translations := map[string]string{}
 	for i := 0; i < to.NumField(); i++ {
 		field := to.Field(i)
+		tags := fieldTags(field)
+		if tags.squash {
+			embedded := field.Type
+			if embedded.Kind() == reflect.Ptr {
+				embedded = embedded.Elem()
+			}
+			if embedded.Kind() != reflect.Struct {
+				// mapstructure will handle reporting this error
+				continue
+			}
+
+			for k, v := range translationsForType(embedded) {
+				translations[k] = v
+			}
+			continue
+		}
+
 		tag, ok := field.Tag.Lookup("alias")
 		if !ok {
 			continue
 		}
-
-		canonKey := strings.ToLower(canonicalFieldKey(field))
+		canonKey := strings.ToLower(tags.name)
 		for _, alias := range strings.Split(tag, ",") {
 			translations[strings.ToLower(alias)] = canonKey
 		}
@@ -86,19 +102,31 @@ func translationsForType(to reflect.Type) map[string]string {
 	return translations
 }
 
-func canonicalFieldKey(field reflect.StructField) string {
+func fieldTags(field reflect.StructField) mapstructureFieldTags {
 	tag, ok := field.Tag.Lookup("mapstructure")
 	if !ok {
-		return field.Name
+		return mapstructureFieldTags{name: field.Name}
 	}
-	parts := strings.SplitN(tag, ",", 2)
-	switch {
-	case len(parts) < 1:
-		return field.Name
-	case parts[0] == "":
-		return field.Name
+
+	tags := mapstructureFieldTags{name: field.Name}
+	parts := strings.Split(tag, ",")
+	if len(parts) == 0 {
+		return tags
 	}
-	return parts[0]
+	if parts[0] != "" {
+		tags.name = parts[0]
+	}
+	for _, part := range parts[1:] {
+		if part == "squash" {
+			tags.squash = true
+		}
+	}
+	return tags
+}
+
+type mapstructureFieldTags struct {
+	name   string
+	squash bool
 }
 
 // HookWeakDecodeFromSlice looks for []map[string]interface{} and []interface{}

lib/decode/decode_test.go

@@ -1,6 +1,7 @@
 package decode
 
 import (
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -210,16 +211,29 @@ type translateExample struct {
 	FieldWithMapstructureTag     string `alias:"second" mapstructure:"field_with_mapstruct_tag"`
 	FieldWithMapstructureTagOmit string `mapstructure:"field_with_mapstruct_omit,omitempty" alias:"third"`
 	FieldWithEmptyTag            string `mapstructure:"" alias:"forth"`
+	EmbeddedStruct               `mapstructure:",squash"`
+	*PtrEmbeddedStruct           `mapstructure:",squash"`
+	BadField                     string `mapstructure:",squash"`
+}
+
+type EmbeddedStruct struct {
+	NextField string `alias:"next"`
+}
+
+type PtrEmbeddedStruct struct {
+	OtherNextField string `alias:"othernext"`
 }
 
 func TestTranslationsForType(t *testing.T) {
 	to := reflect.TypeOf(translateExample{})
 	actual := translationsForType(to)
 	expected := map[string]string{
-		"first":  "fielddefaultcanonical",
-		"second": "field_with_mapstruct_tag",
-		"third":  "field_with_mapstruct_omit",
-		"forth":  "fieldwithemptytag",
+		"first":     "fielddefaultcanonical",
+		"second":    "field_with_mapstruct_tag",
+		"third":     "field_with_mapstruct_omit",
+		"forth":     "fieldwithemptytag",
+		"next":      "nextfield",
+		"othernext": "othernextfield",
 	}
 	require.Equal(t, expected, actual)
 }
@@ -389,3 +403,35 @@ service {
 	}
 	require.Equal(t, target, expected)
 }
+
+func TestFieldTags(t *testing.T) {
+	type testCase struct {
+		tags     string
+		expected mapstructureFieldTags
+	}
+
+	fn := func(t *testing.T, tc testCase) {
+		tag := fmt.Sprintf(`mapstructure:"%v"`, tc.tags)
+		field := reflect.StructField{
+			Tag:  reflect.StructTag(tag),
+			Name: "Original",
+		}
+		actual := fieldTags(field)
+		require.Equal(t, tc.expected, actual)
+	}
+
+	var testCases = []testCase{
+		{tags: "", expected: mapstructureFieldTags{name: "Original"}},
+		{tags: "just-a-name", expected: mapstructureFieldTags{name: "just-a-name"}},
+		{tags: "name,squash", expected: mapstructureFieldTags{name: "name", squash: true}},
+		{tags: ",squash", expected: mapstructureFieldTags{name: "Original", squash: true}},
+		{tags: ",omitempty,squash", expected: mapstructureFieldTags{name: "Original", squash: true}},
+		{tags: "named,omitempty,squash", expected: mapstructureFieldTags{name: "named", squash: true}},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.tags, func(t *testing.T) {
+			fn(t, tc)
+		})
+	}
+}