diff --git a/agent/config/builder.go b/agent/config/builder.go index 8bfde347a6..02a99d0087 100644 --- a/agent/config/builder.go +++ b/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)), }, diff --git a/agent/config/config.go b/agent/config/config.go index acb54d7a88..10687d1b6d 100644 --- a/agent/config/config.go +++ b/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 diff --git a/agent/config/deprecated.go b/agent/config/deprecated.go new file mode 100644 index 0000000000..4a327e560c --- /dev/null +++ b/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 +} diff --git a/agent/config/merge_test.go b/agent/config/merge_test.go index 4c4c7b69f4..f10499303f 100644 --- a/agent/config/merge_test.go +++ b/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 } diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 2db98314b5..486d095ce2 100644 --- a/agent/config/runtime_test.go +++ b/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...) diff --git a/lib/decode/decode.go b/lib/decode/decode.go index 7a6534e64d..0461962c3c 100644 --- a/lib/decode/decode.go +++ b/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{} diff --git a/lib/decode/decode_test.go b/lib/decode/decode_test.go index 8c1e6da5c5..b8243233d1 100644 --- a/lib/decode/decode_test.go +++ b/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) + }) + } +}