Merge pull request #11126 from hashicorp/dnephin/acl-legacy-remove-resolve-and-get-policy

acl: remove ACL.GetPolicy RPC endpoint and ACLResolver.resolveTokenLegacy

acl/policy.go

@@ -453,54 +453,6 @@ func NewPolicyFromSource(id string, revision uint64, rules string, syntax Syntax
 	return policy, err
 }
 
-// TODO(ACL-Legacy): remove this
-func (policy *Policy) ConvertToLegacy() *Policy {
-	converted := &Policy{
-		ID:       policy.ID,
-		Revision: policy.Revision,
-		PolicyRules: PolicyRules{
-			ACL:      policy.ACL,
-			Keyring:  policy.Keyring,
-			Operator: policy.Operator,
-		},
-	}
-
-	converted.Agents = append(converted.Agents, policy.Agents...)
-	converted.Agents = append(converted.Agents, policy.AgentPrefixes...)
-	converted.Keys = append(converted.Keys, policy.Keys...)
-	converted.Keys = append(converted.Keys, policy.KeyPrefixes...)
-	converted.Nodes = append(converted.Nodes, policy.Nodes...)
-	converted.Nodes = append(converted.Nodes, policy.NodePrefixes...)
-	converted.Services = append(converted.Services, policy.Services...)
-	converted.Services = append(converted.Services, policy.ServicePrefixes...)
-	converted.Sessions = append(converted.Sessions, policy.Sessions...)
-	converted.Sessions = append(converted.Sessions, policy.SessionPrefixes...)
-	converted.Events = append(converted.Events, policy.Events...)
-	converted.Events = append(converted.Events, policy.EventPrefixes...)
-	converted.PreparedQueries = append(converted.PreparedQueries, policy.PreparedQueries...)
-	converted.PreparedQueries = append(converted.PreparedQueries, policy.PreparedQueryPrefixes...)
-	return converted
-}
-
-// TODO(ACL-Legacy): remove this
-func (policy *Policy) ConvertFromLegacy() *Policy {
-	return &Policy{
-		ID:       policy.ID,
-		Revision: policy.Revision,
-		PolicyRules: PolicyRules{
-			AgentPrefixes:         policy.Agents,
-			KeyPrefixes:           policy.Keys,
-			NodePrefixes:          policy.Nodes,
-			ServicePrefixes:       policy.Services,
-			SessionPrefixes:       policy.Sessions,
-			EventPrefixes:         policy.Events,
-			PreparedQueryPrefixes: policy.PreparedQueries,
-			Keyring:               policy.Keyring,
-			Operator:              policy.Operator,
-		},
-	}
-}
-
 // takesPrecedenceOver returns true when permission a
 // should take precedence over permission b
 func takesPrecedenceOver(a, b string) bool {

agent/acl_test.go

@@ -76,10 +76,6 @@ func NewTestACLAgent(t *testing.T, name string, hcl string, resolveAuthz authzRe
 	return a
 }
 
-func (a *TestACLAgent) UseLegacyACLs() bool {
-	return false
-}
-
 func (a *TestACLAgent) ResolveToken(secretID string) (acl.Authorizer, error) {
 	if a.resolveAuthzFn == nil {
 		return nil, fmt.Errorf("ResolveToken call is unexpected - no authz resolver callback set")

agent/agent.go

@@ -143,7 +143,6 @@ type delegate interface {
 	ResolveTokenAndDefaultMeta(token string, entMeta *structs.EnterpriseMeta, authzContext *acl.AuthorizerContext) (acl.Authorizer, error)
 
 	RPC(method string, args interface{}, reply interface{}) error
-	UseLegacyACLs() bool
 	SnapshotRPC(args *structs.SnapshotRequest, in io.Reader, out io.Writer, replyFn structs.SnapshotReplyFn) error
 	Shutdown() error
 	Stats() map[string]map[string]string

agent/consul/acl.go

@@ -83,10 +83,12 @@ const (
 	// currently used will backoff as it detects that it is remaining in legacy mode.
 	// However the initial min value is kept small so that new cluster creation
 	// can enter into new ACL mode quickly.
+	// TODO(ACL-Legacy-Compat): remove
 	aclModeCheckMinInterval = 50 * time.Millisecond
 
 	// aclModeCheckMaxInterval controls the maximum interval for how often the agent
 	// checks if it should be using the new or legacy ACL system.
+	// TODO(ACL-Legacy-Compat): remove
 	aclModeCheckMaxInterval = 30 * time.Second
 
 	// Maximum number of re-resolution requests to be made if the token is modified between
@@ -170,7 +172,6 @@ func tokenSecretCacheID(token string) string {
 
 type ACLResolverDelegate interface {
 	ACLDatacenter(legacy bool) string
-	UseLegacyACLs() bool
 	ResolveIdentityFromToken(token string) (bool, structs.ACLIdentity, error)
 	ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy, error)
 	ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error)
@@ -382,134 +383,6 @@ func (r *ACLResolver) Close() {
 	r.aclConf.Close()
 }
 
-func (r *ACLResolver) fetchAndCacheTokenLegacy(token string, cached *structs.AuthorizerCacheEntry) (acl.Authorizer, error) {
-	req := structs.ACLPolicyResolveLegacyRequest{
-		Datacenter: r.delegate.ACLDatacenter(true),
-		ACL:        token,
-	}
-
-	cacheTTL := r.config.ACLTokenTTL
-	if cached != nil {
-		cacheTTL = cached.TTL
-	}
-
-	var reply structs.ACLPolicyResolveLegacyResponse
-	err := r.delegate.RPC("ACL.GetPolicy", &req, &reply)
-	if err == nil {
-		parent := acl.RootAuthorizer(reply.Parent)
-		if parent == nil {
-			var authorizer acl.Authorizer
-			if cached != nil {
-				authorizer = cached.Authorizer
-			}
-			r.cache.PutAuthorizerWithTTL(token, authorizer, cacheTTL)
-			return authorizer, acl.ErrInvalidParent
-		}
-
-		var policies []*acl.Policy
-		policy := reply.Policy
-		if policy != nil {
-			policies = append(policies, policy.ConvertFromLegacy())
-		}
-
-		authorizer, err := acl.NewPolicyAuthorizerWithDefaults(parent, policies, r.aclConf)
-
-		r.cache.PutAuthorizerWithTTL(token, authorizer, reply.TTL)
-		return authorizer, err
-	}
-
-	if acl.IsErrNotFound(err) {
-		// Make sure to remove from the cache if it was deleted
-		r.cache.PutAuthorizerWithTTL(token, nil, cacheTTL)
-		return nil, acl.ErrNotFound
-
-	}
-
-	// some other RPC error
-	switch r.config.ACLDownPolicy {
-	case "allow":
-		r.cache.PutAuthorizerWithTTL(token, acl.AllowAll(), cacheTTL)
-		return acl.AllowAll(), nil
-	case "async-cache", "extend-cache":
-		if cached != nil {
-			r.cache.PutAuthorizerWithTTL(token, cached.Authorizer, cacheTTL)
-			return cached.Authorizer, nil
-		}
-		fallthrough
-	default:
-		r.cache.PutAuthorizerWithTTL(token, acl.DenyAll(), cacheTTL)
-		return acl.DenyAll(), nil
-	}
-}
-
-func (r *ACLResolver) resolveTokenLegacy(token string) (structs.ACLIdentity, acl.Authorizer, error) {
-	defer metrics.MeasureSince([]string{"acl", "resolveTokenLegacy"}, time.Now())
-
-	// Attempt to resolve locally first (local results are not cached)
-	// This is only useful for servers where either legacy replication is being
-	// done or the server is within the primary datacenter.
-	if done, identity, err := r.delegate.ResolveIdentityFromToken(token); done {
-		if err == nil && identity != nil {
-			policies, err := r.resolvePoliciesForIdentity(identity)
-			if err != nil {
-				return identity, nil, err
-			}
-
-			authz, err := policies.Compile(r.cache, r.aclConf)
-			if err != nil {
-				return identity, nil, err
-			}
-
-			return identity, acl.NewChainedAuthorizer([]acl.Authorizer{authz, acl.RootAuthorizer(r.config.ACLDefaultPolicy)}), nil
-		}
-
-		return nil, nil, err
-	}
-
-	identity := &missingIdentity{
-		reason: "legacy-token",
-		token:  token,
-	}
-
-	// Look in the cache prior to making a RPC request
-	entry := r.cache.GetAuthorizer(token)
-
-	if entry != nil && entry.Age() <= minTTL(entry.TTL, r.config.ACLTokenTTL) {
-		metrics.IncrCounter([]string{"acl", "token", "cache_hit"}, 1)
-		if entry.Authorizer != nil {
-			return identity, entry.Authorizer, nil
-		}
-		return identity, nil, acl.ErrNotFound
-	}
-
-	metrics.IncrCounter([]string{"acl", "token", "cache_miss"}, 1)
-
-	// Resolve the token in the background and wait on the result if we must
-	waitChan := r.legacyGroup.DoChan(token, func() (interface{}, error) {
-		authorizer, err := r.fetchAndCacheTokenLegacy(token, entry)
-		return authorizer, err
-	})
-
-	waitForResult := entry == nil || r.config.ACLDownPolicy != "async-cache"
-	if !waitForResult {
-		// waitForResult being false requires the cacheEntry to not be nil
-		if entry.Authorizer != nil {
-			return identity, entry.Authorizer, nil
-		}
-		return identity, nil, acl.ErrNotFound
-	}
-
-	// block waiting for the async RPC to finish.
-	res := <-waitChan
-
-	var authorizer acl.Authorizer
-	if res.Val != nil { // avoid a nil-not-nil bug
-		authorizer = res.Val.(acl.Authorizer)
-	}
-
-	return identity, authorizer, res.Err
-}
-
 func (r *ACLResolver) fetchAndCacheIdentityFromToken(token string, cached *structs.IdentityCacheEntry) (structs.ACLIdentity, error) {
 	cacheID := tokenSecretCacheID(token)
 
@@ -1244,13 +1117,6 @@ func (r *ACLResolver) ResolveTokenToIdentityAndAuthorizer(token string) (structs
 		return ident, authz, nil
 	}
 
-	if r.delegate.UseLegacyACLs() {
-		// TODO(partitions,acls): do we have to care about legacy acls?
-		identity, authorizer, err := r.resolveTokenLegacy(token)
-		r.handleACLDisabledError(err)
-		return identity, authorizer, err
-	}
-
 	defer metrics.MeasureSince([]string{"acl", "ResolveToken"}, time.Now())
 
 	identity, policies, err := r.resolveTokenToIdentityAndPolicies(token)
@@ -1310,12 +1176,6 @@ func (r *ACLResolver) ResolveTokenToIdentity(token string) (structs.ACLIdentity,
 		return ident, nil
 	}
 
-	if r.delegate.UseLegacyACLs() {
-		identity, _, err := r.resolveTokenLegacy(token)
-		r.handleACLDisabledError(err)
-		return identity, err
-	}
-
 	defer metrics.MeasureSince([]string{"acl", "ResolveTokenToIdentity"}, time.Now())
 
 	return r.resolveIdentityFromToken(token)
@@ -1337,19 +1197,6 @@ func (r *ACLResolver) ACLsEnabled() bool {
 	return true
 }
 
-func (r *ACLResolver) GetMergedPolicyForToken(token string) (structs.ACLIdentity, *acl.Policy, error) {
-	ident, policies, err := r.resolveTokenToIdentityAndPolicies(token)
-	if err != nil {
-		return nil, nil, err
-	}
-	if len(policies) == 0 {
-		return nil, nil, acl.ErrNotFound
-	}
-
-	policy, err := policies.Merge(r.cache, r.aclConf)
-	return ident, policy, err
-}
-
 // aclFilter is used to filter results from our state store based on ACL rules
 // configured for the provided token.
 type aclFilter struct {

agent/consul/acl_client.go

@@ -1,9 +1,6 @@
 package consul
 
 import (
-	"sync/atomic"
-	"time"
-
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib/serf"
@@ -27,36 +24,6 @@ var clientACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{
 	Roles: 128,
 }
 
-func (c *Client) UseLegacyACLs() bool {
-	return atomic.LoadInt32(&c.useNewACLs) == 0
-}
-
-func (c *Client) monitorACLMode() {
-	waitTime := aclModeCheckMinInterval
-	for {
-		foundServers, mode, _ := ServersGetACLMode(c, "", c.config.Datacenter)
-		if foundServers && mode == structs.ACLModeEnabled {
-			c.logger.Debug("transitioned out of legacy ACL mode")
-			c.updateSerfTags("acls", string(structs.ACLModeEnabled))
-			atomic.StoreInt32(&c.useNewACLs, 1)
-			return
-		}
-
-		select {
-		case <-c.shutdownCh:
-			return
-		case <-time.After(waitTime):
-			// do nothing
-		}
-
-		// calculate the amount of time to wait for the next round
-		waitTime = waitTime * 2
-		if waitTime > aclModeCheckMaxInterval {
-			waitTime = aclModeCheckMaxInterval
-		}
-	}
-}
-
 func (c *Client) ACLDatacenter(legacy bool) string {
 	// For resolution running on clients, when not in
 	// legacy mode the servers within the current datacenter

agent/consul/acl_endpoint.go

@@ -170,11 +170,6 @@ func (a *ACL) aclPreCheck() error {
 	if !a.srv.config.ACLsEnabled {
 		return acl.ErrDisabled
 	}
-
-	if a.srv.UseLegacyACLs() {
-		return fmt.Errorf("The ACL system is currently in legacy mode.")
-	}
-
 	return nil
 }
 
@@ -1372,56 +1367,6 @@ func (a *ACL) PolicyResolve(args *structs.ACLPolicyBatchGetRequest, reply *struc
 	return nil
 }
 
-// makeACLETag returns an ETag for the given parent and policy.
-func makeACLETag(parent string, policy *acl.Policy) string {
-	return fmt.Sprintf("%s:%s", parent, policy.ID)
-}
-
-// GetPolicy is used to retrieve a compiled policy object with a TTL. Does not
-// support a blocking query.
-//
-// TODO(ACL-Legacy): remove this
-func (a *ACL) GetPolicy(args *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-	if done, err := a.srv.ForwardRPC("ACL.GetPolicy", args, reply); done {
-		return err
-	}
-
-	// Verify we are allowed to serve this request
-	if a.srv.config.PrimaryDatacenter != a.srv.config.Datacenter {
-		return acl.ErrDisabled
-	}
-
-	// Get the policy via the cache
-	parent := a.srv.config.ACLResolverSettings.ACLDefaultPolicy
-
-	ident, policy, err := a.srv.acls.GetMergedPolicyForToken(args.ACL)
-	if err != nil {
-		return err
-	}
-
-	if token, ok := ident.(*structs.ACLToken); ok && token.Type == structs.ACLTokenTypeManagement {
-		parent = "manage"
-	}
-
-	// translates the structures internals to most closely match what could be expressed in the original rule language
-	policy = policy.ConvertToLegacy()
-
-	// Generate an ETag
-	etag := makeACLETag(parent, policy)
-
-	// Setup the response
-	reply.ETag = etag
-	reply.TTL = a.srv.config.ACLResolverSettings.ACLTokenTTL
-	a.srv.setQueryMeta(&reply.QueryMeta)
-
-	// Only send the policy on an Etag mis-match
-	if args.ETag != etag {
-		reply.Parent = parent
-		reply.Policy = policy
-	}
-	return nil
-}
-
 // ReplicationStatus is used to retrieve the current ACL replication status.
 func (a *ACL) ReplicationStatus(args *structs.DCSpecificRequest,
 	reply *structs.ACLReplicationStatus) error {

agent/consul/acl_endpoint_legacy.go

@@ -6,6 +6,12 @@ import (
 	"github.com/hashicorp/consul/agent/structs"
 )
 
+type LegacyACLGetPolicy struct{}
+
+func (a *ACL) GetPolicy(*LegacyACLGetPolicy, *LegacyACLGetPolicy) error {
+	return fmt.Errorf("ACL.GetPolicy: the legacy ACL system has been removed")
+}
+
 func (a *ACL) Bootstrap(*structs.DCSpecificRequest, *structs.ACL) error {
 	return fmt.Errorf("ACL.Bootstrap: the legacy ACL system has been removed")
 }

agent/consul/acl_endpoint_test.go

@@ -74,30 +74,6 @@ func TestACLEndpoint_BootstrapTokens(t *testing.T) {
 	require.Equal(t, out.CreateIndex, out.ModifyIndex)
 }
 
-func TestACLEndpoint_GetPolicy_Management(t *testing.T) {
-	if testing.Short() {
-		t.Skip("too slow for testing.Short")
-	}
-
-	t.Parallel()
-	_, srv, codec := testACLServerWithConfig(t, nil, false)
-
-	// wait for leader election and leader establishment to finish.
-	// after this the global management policy, master token and
-	// anonymous token will have been injected into the state store
-	// and we will be ready to resolve the master token
-	waitForLeaderEstablishment(t, srv)
-
-	req := structs.ACLPolicyResolveLegacyRequest{
-		Datacenter: srv.config.Datacenter,
-		ACL:        TestDefaultMasterToken,
-	}
-
-	var resp structs.ACLPolicyResolveLegacyResponse
-	require.NoError(t, msgpackrpc.CallWithCodec(codec, "ACL.GetPolicy", &req, &resp))
-	require.Equal(t, "manage", resp.Parent)
-}
-
 func TestACLEndpoint_ReplicationStatus(t *testing.T) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")

agent/consul/acl_server.go

@@ -95,6 +95,7 @@ func (s *Server) updateSerfTags(key, value string) {
 	s.updateEnterpriseSerfTags(key, value)
 }
 
+// TODO:
 func (s *Server) updateACLAdvertisement() {
 	// One thing to note is that once in new ACL mode the server will
 	// never transition to legacy ACL mode. This is not currently a

agent/consul/acl_test.go

@@ -434,7 +434,6 @@ type ACLResolverTestDelegate struct {
 	localTokens     bool
 	localPolicies   bool
 	localRoles      bool
-	getPolicyFn     func(*structs.ACLPolicyResolveLegacyRequest, *structs.ACLPolicyResolveLegacyResponse) error
 	tokenReadFn     func(*structs.ACLTokenGetRequest, *structs.ACLTokenResponse) error
 	policyResolveFn func(*structs.ACLPolicyBatchGetRequest, *structs.ACLPolicyBatchResponse) error
 	roleResolveFn   func(*structs.ACLRoleBatchGetRequest, *structs.ACLRoleBatchResponse) error
@@ -675,12 +674,6 @@ func (d *ACLResolverTestDelegate) ResolveRoleFromID(roleID string) (bool, *struc
 
 func (d *ACLResolverTestDelegate) RPC(method string, args interface{}, reply interface{}) error {
 	switch method {
-	case "ACL.GetPolicy":
-		atomic.AddInt32(&d.remoteLegacyResolutions, 1)
-		if d.getPolicyFn != nil {
-			return d.getPolicyFn(args.(*structs.ACLPolicyResolveLegacyRequest), reply.(*structs.ACLPolicyResolveLegacyResponse))
-		}
-		panic("Bad Test Implementation: should provide a getPolicyFn to the ACLResolverTestDelegate")
 	case "ACL.TokenRead":
 		atomic.AddInt32(&d.remoteTokenResolutions, 1)
 		if d.tokenReadFn != nil {
@@ -1571,49 +1564,6 @@ func TestACLResolver_Client(t *testing.T) {
 		require.EqualValues(t, 0, delegate.remoteLegacyResolutions)
 	})
 
-	t.Run("Resolve-Identity-Legacy", func(t *testing.T) {
-		t.Parallel()
-
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        true,
-			localTokens:   false,
-			localPolicies: false,
-			getPolicyFn: func(_ *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-				reply.Parent = "deny"
-				reply.TTL = 30
-				reply.ETag = "nothing"
-				reply.Policy = &acl.Policy{
-					ID: "not-needed",
-					PolicyRules: acl.PolicyRules{
-						Nodes: []*acl.NodeRule{
-							{
-								Name:   "foo",
-								Policy: acl.PolicyWrite,
-							},
-						},
-					},
-				}
-				return nil
-			},
-		}
-
-		r := newTestACLResolver(t, delegate, nil)
-
-		ident, err := r.ResolveTokenToIdentity("found-policy-and-role")
-		require.NoError(t, err)
-		require.NotNil(t, ident)
-		require.Equal(t, "legacy-token", ident.ID())
-		require.EqualValues(t, 0, delegate.localTokenResolutions)
-		require.EqualValues(t, 0, delegate.remoteTokenResolutions)
-		require.EqualValues(t, 0, delegate.localPolicyResolutions)
-		require.EqualValues(t, 0, delegate.remotePolicyResolutions)
-		require.EqualValues(t, 0, delegate.localRoleResolutions)
-		require.EqualValues(t, 0, delegate.remoteRoleResolutions)
-		require.EqualValues(t, 1, delegate.remoteLegacyResolutions)
-	})
-
 	t.Run("Concurrent-Token-Resolve", func(t *testing.T) {
 		t.Parallel()
 
@@ -2231,280 +2181,6 @@ func testACLResolver_variousTokens(t *testing.T, delegate *ACLResolverTestDelega
 	})
 }
 
-func TestACLResolver_Legacy(t *testing.T) {
-	t.Parallel()
-
-	t.Run("Cached", func(t *testing.T) {
-		t.Parallel()
-		cached := false
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        true,
-			localTokens:   false,
-			localPolicies: false,
-			getPolicyFn: func(_ *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-				if !cached {
-					reply.Parent = "deny"
-					reply.TTL = 30
-					reply.ETag = "nothing"
-					reply.Policy = &acl.Policy{
-						ID: "not-needed",
-						PolicyRules: acl.PolicyRules{
-							Nodes: []*acl.NodeRule{
-								{
-									Name:   "foo",
-									Policy: acl.PolicyWrite,
-								},
-							},
-						},
-					}
-					cached = true
-					return nil
-				}
-				return errRPC
-			},
-		}
-		r := newTestACLResolver(t, delegate, nil)
-
-		authz, err := r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-
-		// this should be from the cache
-		authz, err = r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-	})
-
-	t.Run("Cache-Expiry-Extend", func(t *testing.T) {
-		t.Parallel()
-		cached := false
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        true,
-			localTokens:   false,
-			localPolicies: false,
-			getPolicyFn: func(_ *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-				if !cached {
-					reply.Parent = "deny"
-					reply.TTL = 0
-					reply.ETag = "nothing"
-					reply.Policy = &acl.Policy{
-						ID: "not-needed",
-						PolicyRules: acl.PolicyRules{
-							Nodes: []*acl.NodeRule{
-								{
-									Name:   "foo",
-									Policy: acl.PolicyWrite,
-								},
-							},
-						},
-					}
-					cached = true
-					return nil
-				}
-				return errRPC
-			},
-		}
-		r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) {
-			config.Config.ACLTokenTTL = 0
-		})
-
-		authz, err := r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-
-		// this should be from the cache
-		authz, err = r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-	})
-
-	t.Run("Cache-Expiry-Allow", func(t *testing.T) {
-		t.Parallel()
-		cached := false
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        true,
-			localTokens:   false,
-			localPolicies: false,
-			getPolicyFn: func(_ *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-				if !cached {
-					reply.Parent = "deny"
-					reply.TTL = 0
-					reply.ETag = "nothing"
-					reply.Policy = &acl.Policy{
-						ID: "not-needed",
-						PolicyRules: acl.PolicyRules{
-							Nodes: []*acl.NodeRule{
-								{
-									Name:   "foo",
-									Policy: acl.PolicyWrite,
-								},
-							},
-						},
-					}
-					cached = true
-					return nil
-				}
-				return errRPC
-			},
-		}
-		r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) {
-			config.Config.ACLDownPolicy = "allow"
-			config.Config.ACLTokenTTL = 0
-		})
-
-		authz, err := r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-
-		// this should be from the cache
-		authz, err = r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("fo", nil))
-	})
-
-	t.Run("Cache-Expiry-Deny", func(t *testing.T) {
-		t.Parallel()
-		cached := false
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        true,
-			localTokens:   false,
-			localPolicies: false,
-			getPolicyFn: func(_ *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-				if !cached {
-					reply.Parent = "deny"
-					reply.TTL = 0
-					reply.ETag = "nothing"
-					reply.Policy = &acl.Policy{
-						ID: "not-needed",
-						PolicyRules: acl.PolicyRules{
-							Nodes: []*acl.NodeRule{
-								{
-									Name:   "foo",
-									Policy: acl.PolicyWrite,
-								},
-							},
-						},
-					}
-					cached = true
-					return nil
-				}
-				return errRPC
-			},
-		}
-		r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) {
-			config.Config.ACLDownPolicy = "deny"
-			config.Config.ACLTokenTTL = 0
-		})
-
-		authz, err := r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-
-		// this should be from the cache
-		authz, err = r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Deny, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-	})
-
-	t.Run("Cache-Expiry-Async-Cache", func(t *testing.T) {
-		t.Parallel()
-		cached := false
-		delegate := &ACLResolverTestDelegate{
-			enabled:       true,
-			datacenter:    "dc1",
-			legacy:        true,
-			localTokens:   false,
-			localPolicies: false,
-			getPolicyFn: func(_ *structs.ACLPolicyResolveLegacyRequest, reply *structs.ACLPolicyResolveLegacyResponse) error {
-				if !cached {
-					reply.Parent = "deny"
-					reply.TTL = 0
-					reply.ETag = "nothing"
-					reply.Policy = &acl.Policy{
-						ID: "not-needed",
-						PolicyRules: acl.PolicyRules{
-							Nodes: []*acl.NodeRule{
-								{
-									Name:   "foo",
-									Policy: acl.PolicyWrite,
-								},
-							},
-						},
-					}
-					cached = true
-					return nil
-				}
-				return acl.ErrNotFound
-			},
-		}
-		r := newTestACLResolver(t, delegate, func(config *ACLResolverConfig) {
-			config.Config.ACLDownPolicy = "async-cache"
-			config.Config.ACLTokenTTL = 0
-		})
-
-		authz, err := r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		// there is a bit of translation that happens
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo", nil))
-		require.Equal(t, acl.Allow, authz.NodeWrite("foo/bar", nil))
-		require.Equal(t, acl.Deny, authz.NodeWrite("fo", nil))
-
-		// delivered from the cache
-		authz2, err := r.ResolveToken("foo")
-		require.NoError(t, err)
-		require.NotNil(t, authz)
-		require.True(t, authz == authz2)
-
-		// the go routine spawned will eventually return and this will be a not found error
-		retry.Run(t, func(t *retry.R) {
-			authz3, err := r.ResolveToken("foo")
-			assert.Error(t, err)
-			assert.True(t, acl.IsErrNotFound(err))
-			assert.Nil(t, authz3)
-		})
-	})
-}
 func TestACL_filterHealthChecks(t *testing.T) {
 	t.Parallel()
 	// Create some health checks.

agent/consul/client.go

@@ -58,10 +58,6 @@ type Client struct {
 	// acls is used to resolve tokens to effective policies
 	acls *ACLResolver
 
-	// DEPRECATED (ACL-Legacy-Compat) - Only needed while we support both
-	// useNewACLs is a flag to indicate whether we are using the new ACL system
-	useNewACLs int32
-
 	// Connection pool to consul servers
 	connPool *pool.ConnPool
 
@@ -121,7 +117,6 @@ func NewClient(config *Config, deps Deps) (*Client, error) {
 		return nil, err
 	}
 
-	c.useNewACLs = 0
 	aclConfig := ACLResolverConfig{
 		Config:          config.ACLResolverSettings,
 		Delegate:        c,
@@ -154,12 +149,6 @@ func NewClient(config *Config, deps Deps) (*Client, error) {
 	// handlers depend on the router and the router depends on Serf.
 	go c.lanEventHandler()
 
-	// This needs to happen after initializing c.router to prevent a race
-	// condition where the router manager is used when the pointer is nil
-	if c.acls.ACLsEnabled() {
-		go c.monitorACLMode()
-	}
-
 	return c, nil
 }
 
@@ -365,11 +354,7 @@ func (c *Client) Stats() map[string]map[string]string {
 	}
 
 	if c.config.ACLsEnabled {
-		if c.UseLegacyACLs() {
-			stats["consul"]["acl"] = "legacy"
-		} else {
 		stats["consul"]["acl"] = "enabled"
-		}
 	} else {
 		stats["consul"]["acl"] = "disabled"
 	}

agent/consul/client_serf.go

@@ -9,7 +9,6 @@ import (
 	"github.com/hashicorp/serf/serf"
 
 	"github.com/hashicorp/consul/agent/metadata"
-	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib"
 	libserf "github.com/hashicorp/consul/lib/serf"
 	"github.com/hashicorp/consul/logging"
@@ -32,13 +31,6 @@ func (c *Client) setupSerf(conf *serf.Config, ch chan serf.Event, path string) (
 	if c.config.AdvertiseReconnectTimeout != 0 {
 		conf.Tags[libserf.ReconnectTimeoutTag] = c.config.AdvertiseReconnectTimeout.String()
 	}
-	if c.acls.ACLsEnabled() {
-		// we start in legacy mode and then transition to normal
-		// mode once we know the cluster can handle it.
-		conf.Tags["acls"] = string(structs.ACLModeLegacy)
-	} else {
-		conf.Tags["acls"] = string(structs.ACLModeDisabled)
-	}
 
 	// We use the Intercept variant here to ensure that serf and memberlist logs
 	// can be streamed via the monitor endpoint

agent/consul/leader_federation_state_ae_test.go

@@ -376,7 +376,6 @@ func TestLeader_FederationStateAntiEntropyPruning_ACLDeny(t *testing.T) {
 		c.ACLMasterToken = "root"
 		c.ACLResolverSettings.ACLDefaultPolicy = "deny"
 	})
-	testrpc.WaitForLeader(t, s2.RPC, "dc2")
 	defer os.RemoveAll(dir2)
 	defer s2.Shutdown()
 

agent/consul/server_serf.go

@@ -73,10 +73,9 @@ func (s *Server) setupSerf(conf *serf.Config, ch chan serf.Event, path string, w
 	}
 
 	if s.acls.ACLsEnabled() {
-		// we start in legacy mode and allow upgrading later
+		conf.Tags[metadata.TagACLs] = string(structs.ACLModeEnabled)
-		conf.Tags["acls"] = string(structs.ACLModeLegacy)
 	} else {
-		conf.Tags["acls"] = string(structs.ACLModeDisabled)
+		conf.Tags[metadata.TagACLs] = string(structs.ACLModeDisabled)
 	}
 
 	// feature flag: advertise support for federation states

agent/delegate_mock_test.go

@@ -3,12 +3,13 @@ package agent
 import (
 	"io"
 
+	"github.com/hashicorp/serf/serf"
+	"github.com/stretchr/testify/mock"
+
 	"github.com/hashicorp/consul/acl"
 	"github.com/hashicorp/consul/agent/consul"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/lib"
-	"github.com/hashicorp/serf/serf"
-	"github.com/stretchr/testify/mock"
 )
 
 type delegateMock struct {
@@ -65,10 +66,6 @@ func (m *delegateMock) RPC(method string, args interface{}, reply interface{}) e
 	return m.Called(method, args, reply).Error(0)
 }
 
-func (m *delegateMock) UseLegacyACLs() bool {
-	return m.Called().Bool(0)
-}
-
 func (m *delegateMock) SnapshotRPC(args *structs.SnapshotRequest, in io.Reader, out io.Writer, replyFn structs.SnapshotReplyFn) error {
 	return m.Called(args, in, out, replyFn).Error(0)
 }

agent/metadata/server.go

@@ -98,7 +98,7 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
 	}
 
 	var acls structs.ACLMode
-	if aclMode, ok := m.Tags["acls"]; ok {
+	if aclMode, ok := m.Tags[TagACLs]; ok {
 		acls = structs.ACLMode(aclMode)
 	} else {
 		acls = structs.ACLModeUnknown
@@ -194,6 +194,8 @@ func IsConsulServer(m serf.Member) (bool, *Server) {
 	return true, parts
 }
 
+const TagACLs = "acls"
+
 const featureFlagPrefix = "ft_"
 
 // AddFeatureFlags to the tags. The tags map is expected to be a serf.Config.Tags.

agent/structs/acl.go

@@ -811,15 +811,6 @@ func (policies ACLPolicies) Compile(cache *ACLCaches, entConf *acl.Config) (acl.
 	return authorizer, nil
 }
 
-func (policies ACLPolicies) Merge(cache *ACLCaches, entConf *acl.Config) (*acl.Policy, error) {
-	parsed, err := policies.resolveWithCache(cache, entConf)
-	if err != nil {
-		return nil, err
-	}
-
-	return acl.MergePolicies(parsed), nil
-}
-
 type ACLRoles []*ACLRole
 
 // HashKey returns a consistent hash for a set of roles.

agent/structs/acl_cache.go

@@ -3,8 +3,9 @@ package structs
 import (
 	"time"
 
-	"github.com/hashicorp/consul/acl"
 	lru "github.com/hashicorp/golang-lru"
+
+	"github.com/hashicorp/consul/acl"
 )
 
 type ACLCachesConfig struct {
@@ -218,14 +219,6 @@ func (c *ACLCaches) PutAuthorizer(id string, authorizer acl.Authorizer) {
 	c.authorizers.Add(id, &AuthorizerCacheEntry{Authorizer: authorizer, CacheTime: time.Now()})
 }
 
-func (c *ACLCaches) PutAuthorizerWithTTL(id string, authorizer acl.Authorizer, ttl time.Duration) {
-	if c == nil || c.authorizers == nil {
-		return
-	}
-
-	c.authorizers.Add(id, &AuthorizerCacheEntry{Authorizer: authorizer, CacheTime: time.Now(), TTL: ttl})
-}
-
 func (c *ACLCaches) PutRole(roleID string, role *ACLRole) {
 	if c == nil || c.roles == nil {
 		return

agent/structs/acl_legacy.go

@@ -8,9 +8,6 @@ package structs
 
 import (
 	"fmt"
-	"time"
-
-	"github.com/hashicorp/consul/acl"
 )
 
 const (
@@ -114,25 +111,3 @@ type ACLBootstrap struct {
 
 	RaftIndex
 }
-
-// ACLPolicyResolveLegacyRequest is used to request an ACL by Token SecretID, conditionally
-// filtering on an ID
-type ACLPolicyResolveLegacyRequest struct {
-	Datacenter string // The Datacenter the RPC may be sent to
-	ACL        string // The Tokens Secret ID
-	ETag       string // Caching ETag to prevent resending the policy when not needed
-	QueryOptions
-}
-
-// RequestDatacenter returns the DC this request is targeted to.
-func (r *ACLPolicyResolveLegacyRequest) RequestDatacenter() string {
-	return r.Datacenter
-}
-
-type ACLPolicyResolveLegacyResponse struct {
-	ETag   string
-	Parent string
-	Policy *acl.Policy
-	TTL    time.Duration
-	QueryMeta
-}

agent/structs/structs_test.go

@@ -64,7 +64,6 @@ func TestStructs_Implements(t *testing.T) {
 		_ RPCInfo          = &SessionRequest{}
 		_ RPCInfo          = &SessionSpecificRequest{}
 		_ RPCInfo          = &EventFireRequest{}
-		_ RPCInfo          = &ACLPolicyResolveLegacyRequest{}
 		_ RPCInfo          = &ACLPolicyBatchGetRequest{}
 		_ RPCInfo          = &ACLPolicyGetRequest{}
 		_ RPCInfo          = &ACLTokenGetRequest{}