acl: remove the deprecated acl_enforce_version_8 option (#7991)

Fixes #7292

agent/acl.go

@@ -25,11 +25,6 @@ func (a *Agent) resolveTokenAndDefaultMeta(id string, entMeta *structs.Enterpris
 		return nil, nil
 	}
 
-	// Disable ACLs if version 8 enforcement isn't enabled.
-	if !a.config.ACLEnforceVersion8 {
-		return nil, nil
-	}
-
 	if acl.RootAuthorizer(id) != nil {
 		return nil, acl.ErrRootDenied
 	}

agent/acl_test.go

@@ -181,33 +181,19 @@ func (a *TestACLAgent) ReloadConfig(config *consul.Config) error {
 	return fmt.Errorf("Unimplemented")
 }
 
-func TestACL_Version8(t *testing.T) {
+func TestACL_Version8EnabledByDefault(t *testing.T) {
 	t.Parallel()
 
-	t.Run("version 8 disabled", func(t *testing.T) {
-		a := NewTestACLAgent(t, t.Name(), TestACLConfig()+`
- 		acl_enforce_version_8 = false
- 	`, nil, nil)
-
-		token, err := a.resolveToken("nope")
-		require.Nil(t, token)
-		require.Nil(t, err)
-	})
+	called := false
+	resolveFn := func(string) (structs.ACLIdentity, acl.Authorizer, error) {
+		called = true
+		return nil, nil, acl.ErrNotFound
+	}
+	a := NewTestACLAgent(t, t.Name(), TestACLConfig(), resolveFn, nil)
 
-	t.Run("version 8 enabled", func(t *testing.T) {
-		called := false
-		resolveFn := func(string) (structs.ACLIdentity, acl.Authorizer, error) {
-			called = true
-			return nil, nil, acl.ErrNotFound
-		}
-		a := NewTestACLAgent(t, t.Name(), TestACLConfig()+`
- 		acl_enforce_version_8 = true
- 	`, resolveFn, nil)
-
-		_, err := a.resolveToken("nope")
-		require.Error(t, err)
-		require.True(t, called)
-	})
+	_, err := a.resolveToken("nope")
+	require.Error(t, err)
+	require.True(t, called)
 }
 
 func TestACL_AgentMasterToken(t *testing.T) {

agent/agent.go

@@ -1278,7 +1278,6 @@ func (a *Agent) consulConfig() (*consul.Config, error) {
 	if a.config.ACLDownPolicy != "" {
 		base.ACLDownPolicy = a.config.ACLDownPolicy
 	}
-	base.ACLEnforceVersion8 = a.config.ACLEnforceVersion8
 	base.ACLTokenReplication = a.config.ACLTokenReplication
 	base.ACLsEnabled = a.config.ACLsEnabled
 	if a.config.ACLEnableKeyListPolicy {

agent/agent_endpoint_test.go

@@ -1307,7 +1307,6 @@ func TestAgent_Reload(t *testing.T) {
 	t.Parallel()
 	dc1 := "dc1"
 	a := NewTestAgent(t, `
-		acl_enforce_version_8 = false
 		services = [
 			{
 				name = "redis"
@@ -1341,7 +1340,6 @@ func TestAgent_Reload(t *testing.T) {
 			node_id = "` + string(a.Config.NodeID) + `"
 			node_name = "` + a.Config.NodeName + `"
 
-			acl_enforce_version_8 = false
 			services = [
 				{
 					name = "redis-reloaded"
@@ -1387,7 +1385,6 @@ func TestAgent_ReloadDoesNotTriggerWatch(t *testing.T) {
 	handlerShell := fmt.Sprintf("(cat ; echo CONSUL_INDEX $CONSUL_INDEX) | tee '%s.atomic' ; mv '%s.atomic' '%s'", tmpFile, tmpFile, tmpFile)
 
 	a := NewTestAgent(t, `
-		acl_enforce_version_8 = false
 		services = [
 			{
 				name = "redis"
@@ -1477,7 +1474,6 @@ func TestAgent_ReloadDoesNotTriggerWatch(t *testing.T) {
 			node_id = "` + string(a.Config.NodeID) + `"
 			node_name = "` + a.Config.NodeName + `"
 
-			acl_enforce_version_8 = false
 			services = [
 				{
 					name = "redis"
@@ -6031,7 +6027,6 @@ func TestAgentConnectAuthorize_defaultAllow(t *testing.T) {
 		acl_master_token = "root"
 		acl_agent_token = "root"
 		acl_agent_master_token = "towel"
-		acl_enforce_version_8 = true
 	`)
 	defer a.Shutdown()
 	testrpc.WaitForTestAgent(t, a.RPC, dc1)
@@ -6063,7 +6058,6 @@ func TestAgent_Host(t *testing.T) {
 	acl_master_token = "master"
 	acl_agent_token = "agent"
 	acl_agent_master_token = "towel"
-	acl_enforce_version_8 = true
 `)
 	defer a.Shutdown()
 
@@ -6091,7 +6085,6 @@ func TestAgent_HostBadACL(t *testing.T) {
 	acl_master_token = "root"
 	acl_agent_token = "agent"
 	acl_agent_master_token = "towel"
-	acl_enforce_version_8 = true
 `)
 	defer a.Shutdown()
 

agent/config/builder.go

@@ -279,13 +279,26 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		if s.Name == "" || s.Data == "" {
 			continue
 		}
-		c2, keys, err := Parse(s.Data, s.Format)
+		c2, md, err := Parse(s.Data, s.Format)
 		if err != nil {
 			return RuntimeConfig{}, fmt.Errorf("Error parsing %s: %s", s.Name, err)
 		}
 
+		var unusedErr error
+		for _, k := range md.Unused {
+			switch k {
+			case "acl_enforce_version_8":
+				b.warn("config key %q is deprecated and should be removed", k)
+			default:
+				unusedErr = multierror.Append(unusedErr, fmt.Errorf("invalid config key %s", k))
+			}
+		}
+		if unusedErr != nil {
+			return RuntimeConfig{}, fmt.Errorf("Error parsing %s: %s", s.Name, unusedErr)
+		}
+
 		// for now this is a soft failure that will cause warnings but not actual problems
-		b.validateEnterpriseConfigKeys(&c2, keys)
+		b.validateEnterpriseConfigKeys(&c2, md.Keys)
 
 		// 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
@@ -790,7 +803,6 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		GossipWANRetransmitMult: b.intVal(c.GossipWAN.RetransmitMult),
 
 		// ACL
-		ACLEnforceVersion8:        b.boolValWithDefault(c.ACLEnforceVersion8, true),
 		ACLsEnabled:               aclsEnabled,
 		ACLAgentMasterToken:       b.stringValWithDefault(c.ACL.Tokens.AgentMaster, b.stringVal(c.ACLAgentMasterToken)),
 		ACLAgentToken:             b.stringValWithDefault(c.ACL.Tokens.Agent, b.stringVal(c.ACLAgentToken)),

agent/config/config.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/hashicorp/consul/lib"
 	"github.com/hashicorp/consul/lib/decode"
-	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/hcl"
 	"github.com/mitchellh/mapstructure"
 )
@@ -35,7 +34,7 @@ func FormatFrom(name string) string {
 }
 
 // Parse parses a config fragment in either JSON or HCL format.
-func Parse(data string, format string) (c Config, keys []string, err error) {
+func Parse(data string, format string) (c Config, md mapstructure.Metadata, err error) {
 	var raw map[string]interface{}
 	switch format {
 	case "json":
@@ -46,7 +45,7 @@ func Parse(data string, format string) (c Config, keys []string, err error) {
 		err = fmt.Errorf("invalid format: %s", format)
 	}
 	if err != nil {
-		return Config{}, nil, err
+		return Config{}, mapstructure.Metadata{}, err
 	}
 
 	// We want to be able to report fields which we cannot map as an
@@ -109,28 +108,19 @@ func Parse(data string, format string) (c Config, keys []string, err error) {
 		"config_entries.bootstrap", // completely ignore this tree (fixed elsewhere)
 	})
 
-	var md mapstructure.Metadata
 	d, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{
 		DecodeHook: decode.HookTranslateKeys,
 		Metadata:   &md,
 		Result:     &c,
 	})
 	if err != nil {
-		return Config{}, nil, err
+		return Config{}, mapstructure.Metadata{}, err
 	}
 	if err := d.Decode(m); err != nil {
-		return Config{}, nil, err
+		return Config{}, mapstructure.Metadata{}, err
 	}
 
-	for _, k := range md.Unused {
-		err = multierror.Append(err, fmt.Errorf("invalid config key %s", k))
-	}
-
-	// Don't check these here. The builder can emit warnings for fields it
-	// doesn't like
-	keys = md.Keys
-
-	return
+	return c, md, nil
 }
 
 // Config defines the format of a configuration file in either JSON or
@@ -155,8 +145,6 @@ type Config struct {
 	ACLDownPolicy *string `json:"acl_down_policy,omitempty" hcl:"acl_down_policy" mapstructure:"acl_down_policy"`
 	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl" stanza
 	ACLEnableKeyListPolicy *bool `json:"acl_enable_key_list_policy,omitempty" hcl:"acl_enable_key_list_policy" mapstructure:"acl_enable_key_list_policy"`
-	// DEPRECATED (ACL-Legacy-Compat) -  pre-version8 enforcement is deprecated.
-	ACLEnforceVersion8 *bool `json:"acl_enforce_version_8,omitempty" hcl:"acl_enforce_version_8" mapstructure:"acl_enforce_version_8"`
 	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl" stanza
 	ACLMasterToken *string `json:"acl_master_token,omitempty" hcl:"acl_master_token" mapstructure:"acl_master_token"`
 	// DEPRECATED (ACL-Legacy-Compat) - moved into the "acl.tokens" stanza

agent/config/default.go

@@ -10,18 +10,6 @@ import (
 	"github.com/hashicorp/raft"
 )
 
-func DefaultRPCProtocol() (int, error) {
-	src := DefaultSource()
-	c, _, err := Parse(src.Data, src.Format)
-	if err != nil {
-		return 0, fmt.Errorf("Error parsing default config: %s", err)
-	}
-	if c.RPCProtocol == nil {
-		return 0, fmt.Errorf("No default RPC protocol set")
-	}
-	return *c.RPCProtocol, nil
-}
-
 // DefaultSource is the default agent configuration.
 // This needs to be merged first in the head.
 // todo(fs): The values are sourced from multiple sources.
@@ -43,7 +31,6 @@ func DefaultSource() Source {
 		Data: `
 		acl_default_policy = "allow"
 		acl_down_policy = "extend-cache"
-		acl_enforce_version_8 = true
 		acl_ttl = "30s"
 		acl = {
 			policy_ttl = "30s"
@@ -65,7 +52,7 @@ func DefaultSource() Source {
 		log_level = "INFO"
 		max_query_time = "600s"
 		primary_gateways_interval = "30s"
-		protocol =  2
+		protocol = ` + strconv.Itoa(consul.DefaultRPCProtocol) + `
 		retry_interval = "30s"
 		retry_interval_wan = "30s"
 		server = false

agent/config/runtime.go

@@ -104,13 +104,6 @@ type RuntimeConfig struct {
 	// hcl: acl.down_policy = ("allow"|"deny"|"extend-cache"|"async-cache")
 	ACLDownPolicy string
 
-	// DEPRECATED (ACL-Legacy-Compat)
-	// ACLEnforceVersion8 is used to gate a set of ACL policy features that
-	// are opt-in prior to Consul 0.8 and opt-out in Consul 0.8 and later.
-	//
-	// hcl: acl_enforce_version_8 = (true|false)
-	ACLEnforceVersion8 bool
-
 	// ACLEnableKeyListPolicy is used to opt-in to the "list" policy added to
 	// KV ACLs in Consul 1.0.
 	//

agent/config/runtime_test.go

@@ -1619,6 +1619,16 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
 				rt.DataDir = dataDir
 			},
 		},
+		{
+			desc: "acl_enforce_version_8 is deprecated",
+			args: []string{`-data-dir=` + dataDir},
+			json: []string{`{ "acl_enforce_version_8": true }`},
+			hcl:  []string{`acl_enforce_version_8 = true`},
+			patch: func(rt *RuntimeConfig) {
+				rt.DataDir = dataDir
+			},
+			warns: []string{`config key "acl_enforce_version_8" is deprecated and should be removed`},
+		},
 		{
 			desc: "advertise address detect fails v4",
 			args: []string{`-data-dir=` + dataDir},
@@ -3934,7 +3944,6 @@ func TestFullConfig(t *testing.T) {
 			"acl_datacenter": "m3urck3z",
 			"acl_default_policy": "ArK3WIfE",
 			"acl_down_policy": "vZXMfMP0",
-			"acl_enforce_version_8": true,
 			"acl_enable_key_list_policy": true,
 			"acl_master_token": "C1Q1oIwh",
 			"acl_replication_token": "LMmgy5dO",
@@ -4570,7 +4579,6 @@ func TestFullConfig(t *testing.T) {
 			acl_datacenter = "m3urck3z"
 			acl_default_policy = "ArK3WIfE"
 			acl_down_policy = "vZXMfMP0"
-			acl_enforce_version_8 = true
 			acl_enable_key_list_policy = true
 			acl_master_token = "C1Q1oIwh"
 			acl_replication_token = "LMmgy5dO"
@@ -5332,7 +5340,6 @@ func TestFullConfig(t *testing.T) {
 		ACLDatacenter:                    "ejtmd43d",
 		ACLDefaultPolicy:                 "72c2e7a0",
 		ACLDownPolicy:                    "03eb2aee",
-		ACLEnforceVersion8:               true,
 		ACLEnableKeyListPolicy:           true,
 		ACLEnableTokenPersistence:        true,
 		ACLMasterToken:                   "8a19ac27",
@@ -6251,7 +6258,6 @@ func TestSanitize(t *testing.T) {
 		"ACLDownPolicy": "",
 		"ACLEnableKeyListPolicy": false,
 		"ACLEnableTokenPersistence": false,
-		"ACLEnforceVersion8": false,
 		"ACLMasterToken": "hidden",
 		"ACLPolicyTTL": "0s",
 		"ACLReplicationToken": "hidden",

agent/consul/acl.go

@@ -1169,29 +1169,23 @@ func (r *ACLResolver) GetMergedPolicyForToken(token string) (*acl.Policy, error)
 // aclFilter is used to filter results from our state store based on ACL rules
 // configured for the provided token.
 type aclFilter struct {
-	authorizer      acl.Authorizer
-	logger          hclog.Logger
-	enforceVersion8 bool
+	authorizer acl.Authorizer
+	logger     hclog.Logger
 }
 
 // newACLFilter constructs a new aclFilter.
-func newACLFilter(authorizer acl.Authorizer, logger hclog.Logger, enforceVersion8 bool) *aclFilter {
+func newACLFilter(authorizer acl.Authorizer, logger hclog.Logger) *aclFilter {
 	if logger == nil {
 		logger = hclog.New(&hclog.LoggerOptions{})
 	}
 	return &aclFilter{
-		authorizer:      authorizer,
-		logger:          logger,
-		enforceVersion8: enforceVersion8,
+		authorizer: authorizer,
+		logger:     logger,
 	}
 }
 
 // allowNode is used to determine if a node is accessible for an ACL.
 func (f *aclFilter) allowNode(node string, ent *acl.AuthorizerContext) bool {
-	if !f.enforceVersion8 {
-		return true
-	}
-
 	return f.authorizer.NodeRead(node, ent) == acl.Allow
 }
 
@@ -1218,18 +1212,12 @@ func (f *aclFilter) allowService(service string, ent *acl.AuthorizerContext) boo
 		return true
 	}
 
-	if !f.enforceVersion8 && service == structs.ConsulServiceID {
-		return true
-	}
 	return f.authorizer.ServiceRead(service, ent) == acl.Allow
 }
 
 // allowSession is used to determine if a session for a node is accessible for
 // an ACL.
 func (f *aclFilter) allowSession(node string, ent *acl.AuthorizerContext) bool {
-	if !f.enforceVersion8 {
-		return true
-	}
 	return f.authorizer.SessionRead(node, ent) == acl.Allow
 }
 
@@ -1793,7 +1781,7 @@ func (r *ACLResolver) filterACLWithAuthorizer(authorizer acl.Authorizer, subj in
 		return nil
 	}
 	// Create the filter
-	filt := newACLFilter(authorizer, r.logger, r.config.ACLEnforceVersion8)
+	filt := newACLFilter(authorizer, r.logger)
 
 	switch v := subj.(type) {
 	case *structs.CheckServiceNodes: