diff --git a/agent/config/builder.go b/agent/config/builder.go index e4faa4d2d4..932da22b53 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -316,8 +316,9 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { 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 - b.validateEnterpriseConfigKeys(&c2, md.Keys) + for _, err := range validateEnterpriseConfigKeys(&c2) { + b.warn("%s", err) + } // 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 @@ -1845,10 +1846,20 @@ func (b *Builder) stringValWithDefault(v *string, defaultVal string) string { return *v } +// Deprecated: use the stringVal() function instead of this Builder method. This +// method is being left here temporarily as there are many callers. It will be +// removed in the future. func (b *Builder) stringVal(v *string) string { return b.stringValWithDefault(v, "") } +func stringVal(v *string) string { + if v == nil { + return "" + } + return *v +} + func (b *Builder) float64ValWithDefault(v *float64, defaultVal float64) float64 { if v == nil { return defaultVal diff --git a/agent/config/builder_oss.go b/agent/config/builder_oss.go index 82a037994b..9e68a54609 100644 --- a/agent/config/builder_oss.go +++ b/agent/config/builder_oss.go @@ -4,47 +4,55 @@ package config import ( "fmt" - - "github.com/hashicorp/go-multierror" ) -var ( - enterpriseConfigMap map[string]func(*Config) = map[string]func(c *Config){ - "non_voting_server": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "read_replica": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "segment": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "segments": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "autopilot.redundancy_zone_tag": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "autopilot.upgrade_version_tag": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "autopilot.disable_upgrade_migration": func(c *Config) { - // to maintain existing compatibility we don't nullify the value - }, - "dns_config.prefer_namespace": func(c *Config) { - c.DNS.PreferNamespace = nil - }, - "acl.msp_disable_bootstrap": func(c *Config) { - c.ACL.MSPDisableBootstrap = nil - }, - "acl.tokens.managed_service_provider": func(c *Config) { - c.ACL.Tokens.ManagedServiceProvider = nil - }, - "audit": func(c *Config) { - c.Audit = nil - }, +// validateEnterpriseConfig is a function to validate the enterprise specific +// configuration items after Parsing but before merging into the overall +// configuration. The original intent is to use it to ensure that we warn +// for enterprise configurations used in OSS. +func validateEnterpriseConfigKeys(config *Config) []error { + var result []error + add := func(k string) { + result = append(result, enterpriseConfigKeyError{key: k}) } -) + + if config.ReadReplica != nil { + add(`read_replica (or the deprecated non_voting_server)`) + } + if stringVal(config.SegmentName) != "" { + add("segment") + } + if len(config.Segments) > 0 { + add("segments") + } + if stringVal(config.Autopilot.RedundancyZoneTag) != "" { + add("autopilot.redundancy_zone_tag") + } + if stringVal(config.Autopilot.UpgradeVersionTag) != "" { + add("autopilot.upgrade_version_tag") + } + if config.Autopilot.DisableUpgradeMigration != nil { + add("autopilot.disable_upgrade_migration") + } + if config.DNS.PreferNamespace != nil { + add("dns_config.prefer_namespace") + config.DNS.PreferNamespace = nil + } + if config.ACL.MSPDisableBootstrap != nil { + add("acl.msp_disable_bootstrap") + config.ACL.MSPDisableBootstrap = nil + } + if len(config.ACL.Tokens.ManagedServiceProvider) > 0 { + add("acl.tokens.managed_service_provider") + config.ACL.Tokens.ManagedServiceProvider = nil + } + if config.Audit != nil { + add("audit") + config.Audit = nil + } + + return result +} type enterpriseConfigKeyError struct { key string @@ -61,23 +69,3 @@ func (*Builder) BuildEnterpriseRuntimeConfig(_ *RuntimeConfig, _ *Config) error func (*Builder) validateEnterpriseConfig(_ RuntimeConfig) error { return nil } - -// validateEnterpriseConfig is a function to validate the enterprise specific -// configuration items after Parsing but before merging into the overall -// configuration. The original intent is to use it to ensure that we warn -// for enterprise configurations used in OSS. -func (b *Builder) validateEnterpriseConfigKeys(config *Config, keys []string) error { - var err error - - for _, k := range keys { - if unset, ok := enterpriseConfigMap[k]; ok { - keyErr := enterpriseConfigKeyError{key: k} - - b.warn(keyErr.Error()) - err = multierror.Append(err, keyErr) - unset(config) - } - } - - return err -} diff --git a/agent/config/builder_oss_test.go b/agent/config/builder_oss_test.go index e94fec68a2..d03ef165b6 100644 --- a/agent/config/builder_oss_test.go +++ b/agent/config/builder_oss_test.go @@ -5,15 +5,13 @@ package config import ( "testing" - "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/require" ) -func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { +func TestValidateEnterpriseConfigKeys(t *testing.T) { // ensure that all the enterprise configurations type testCase struct { config Config - keys []string badKeys []string check func(t *testing.T, c *Config) } @@ -22,34 +20,22 @@ func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { stringVal := "string" cases := map[string]testCase{ - "non_voting_server": { - config: Config{ - ReadReplica: &boolVal, - }, - keys: []string{"non_voting_server"}, - badKeys: []string{"non_voting_server"}, - }, "read_replica": { config: Config{ ReadReplica: &boolVal, }, - keys: []string{"read_replica"}, - badKeys: []string{"read_replica"}, + badKeys: []string{"read_replica (or the deprecated non_voting_server)"}, }, "segment": { config: Config{ SegmentName: &stringVal, }, - keys: []string{"segment"}, badKeys: []string{"segment"}, }, "segments": { config: Config{ - Segments: []Segment{ - {Name: &stringVal}, - }, + Segments: []Segment{{Name: &stringVal}}, }, - keys: []string{"segments"}, badKeys: []string{"segments"}, }, "autopilot.redundancy_zone_tag": { @@ -58,7 +44,6 @@ func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { RedundancyZoneTag: &stringVal, }, }, - keys: []string{"autopilot.redundancy_zone_tag"}, badKeys: []string{"autopilot.redundancy_zone_tag"}, }, "autopilot.upgrade_version_tag": { @@ -67,25 +52,18 @@ func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { UpgradeVersionTag: &stringVal, }, }, - keys: []string{"autopilot.upgrade_version_tag"}, badKeys: []string{"autopilot.upgrade_version_tag"}, }, "autopilot.disable_upgrade_migration": { config: Config{ - Autopilot: Autopilot{ - DisableUpgradeMigration: &boolVal, - }, + Autopilot: Autopilot{DisableUpgradeMigration: &boolVal}, }, - keys: []string{"autopilot.disable_upgrade_migration"}, badKeys: []string{"autopilot.disable_upgrade_migration"}, }, "dns_config.prefer_namespace": { config: Config{ - DNS: DNS{ - PreferNamespace: &boolVal, - }, + DNS: DNS{PreferNamespace: &boolVal}, }, - keys: []string{"dns_config.prefer_namespace"}, badKeys: []string{"dns_config.prefer_namespace"}, check: func(t *testing.T, c *Config) { require.Nil(t, c.DNS.PreferNamespace) @@ -93,11 +71,8 @@ func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { }, "acl.msp_disable_bootstrap": { config: Config{ - ACL: ACL{ - MSPDisableBootstrap: &boolVal, - }, + ACL: ACL{MSPDisableBootstrap: &boolVal}, }, - keys: []string{"acl.msp_disable_bootstrap"}, badKeys: []string{"acl.msp_disable_bootstrap"}, check: func(t *testing.T, c *Config) { require.Nil(t, c.ACL.MSPDisableBootstrap) @@ -116,7 +91,6 @@ func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { }, }, }, - keys: []string{"acl.tokens.managed_service_provider"}, badKeys: []string{"acl.tokens.managed_service_provider"}, check: func(t *testing.T, c *Config) { require.Empty(t, c.ACL.Tokens.ManagedServiceProvider) @@ -127,39 +101,28 @@ func TestBuilder_validateEnterpriseConfigKeys(t *testing.T) { config: Config{ ReadReplica: &boolVal, SegmentName: &stringVal, + ACL: ACL{Tokens: Tokens{AgentMaster: &stringVal}}, }, - keys: []string{"non_voting_server", "read_replica", "segment", "acl.tokens.agent_master"}, - badKeys: []string{"non_voting_server", "read_replica", "segment"}, + badKeys: []string{"read_replica (or the deprecated non_voting_server)", "segment"}, }, } for name, tcase := range cases { t.Run(name, func(t *testing.T) { - b := &Builder{} + errs := validateEnterpriseConfigKeys(&tcase.config) + if len(tcase.badKeys) == 0 { + require.Len(t, errs, 0) + return + } - err := b.validateEnterpriseConfigKeys(&tcase.config, tcase.keys) - if len(tcase.badKeys) > 0 { - require.Error(t, err) + var expected []error + for _, k := range tcase.badKeys { + expected = append(expected, enterpriseConfigKeyError{key: k}) + } + require.ElementsMatch(t, expected, errs) - multiErr, ok := err.(*multierror.Error) - require.True(t, ok) - - var badKeys []string - for _, e := range multiErr.Errors { - if keyErr, ok := e.(enterpriseConfigKeyError); ok { - badKeys = append(badKeys, keyErr.key) - require.Contains(t, b.Warnings, keyErr.Error()) - } - } - - require.ElementsMatch(t, tcase.badKeys, badKeys) - - if tcase.check != nil { - tcase.check(t, &tcase.config) - } - - } else { - require.NoError(t, err) + if tcase.check != nil { + tcase.check(t, &tcase.config) } }) } diff --git a/agent/config/default.go b/agent/config/default.go index 72888250d6..0220399dfb 100644 --- a/agent/config/default.go +++ b/agent/config/default.go @@ -4,10 +4,11 @@ import ( "fmt" "strconv" + "github.com/hashicorp/raft" + "github.com/hashicorp/consul/agent/checks" "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/version" - "github.com/hashicorp/raft" ) // DefaultSource is the default agent configuration. diff --git a/agent/config/runtime_oss_test.go b/agent/config/runtime_oss_test.go index e3b51568bf..059d8223ff 100644 --- a/agent/config/runtime_oss_test.go +++ b/agent/config/runtime_oss_test.go @@ -12,16 +12,17 @@ var entTokenConfigSanitize = `"EnterpriseConfig": {},` func entFullRuntimeConfig(rt *RuntimeConfig) {} -var enterpriseReadReplicaWarnings = []string{enterpriseConfigKeyError{key: "read_replica"}.Error()} +var enterpriseReadReplicaWarnings = []string{enterpriseConfigKeyError{key: "read_replica (or the deprecated non_voting_server)"}.Error()} -var enterpriseConfigKeyWarnings []string - -func init() { - for k := range enterpriseConfigMap { - if k == "non_voting_server" { - // this is an alias for "read_replica" so we shouldn't see it in warnings - continue - } - enterpriseConfigKeyWarnings = append(enterpriseConfigKeyWarnings, enterpriseConfigKeyError{key: k}.Error()) - } +var enterpriseConfigKeyWarnings = []string{ + enterpriseConfigKeyError{key: "read_replica (or the deprecated non_voting_server)"}.Error(), + enterpriseConfigKeyError{key: "segment"}.Error(), + enterpriseConfigKeyError{key: "segments"}.Error(), + enterpriseConfigKeyError{key: "autopilot.redundancy_zone_tag"}.Error(), + enterpriseConfigKeyError{key: "autopilot.upgrade_version_tag"}.Error(), + enterpriseConfigKeyError{key: "autopilot.disable_upgrade_migration"}.Error(), + enterpriseConfigKeyError{key: "dns_config.prefer_namespace"}.Error(), + enterpriseConfigKeyError{key: "acl.msp_disable_bootstrap"}.Error(), + enterpriseConfigKeyError{key: "acl.tokens.managed_service_provider"}.Error(), + enterpriseConfigKeyError{key: "audit"}.Error(), }