acl: adding Roles to Tokens (#5514)

Roles are named and can express the same bundle of permissions that can
currently be assigned to a Token (lists of Policies and Service
Identities). The difference with a Role is that it not itself a bearer
token, but just another entity that can be tied to a Token.

This lets an operator potentially curate a set of smaller reusable
Policies and compose them together into reusable Roles, rather than
always exploding that same list of Policies on any Token that needs
similar permissions.

This also refactors the acl replication code to be semi-generic to avoid
3x copypasta.

agent/acl_endpoint.go

@@ -254,6 +254,7 @@ func (s *HTTPServer) ACLPolicyRead(resp http.ResponseWriter, req *http.Request,
 	}
 
 	if out.Policy == nil {
+		// TODO(rb): should this return a normal 404?
 		return nil, acl.ErrNotFound
 	}
 
@@ -374,6 +375,7 @@ func (s *HTTPServer) ACLTokenList(resp http.ResponseWriter, req *http.Request) (
 	}
 
 	args.Policy = req.URL.Query().Get("policy")
+	args.Role = req.URL.Query().Get("role")
 
 	var out structs.ACLTokenListResponse
 	defer setMeta(resp, &out.QueryMeta)
@@ -548,3 +550,154 @@ func (s *HTTPServer) ACLTokenClone(resp http.ResponseWriter, req *http.Request,
 
 	return &out, nil
 }
+
+func (s *HTTPServer) ACLRoleList(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+	if s.checkACLDisabled(resp, req) {
+		return nil, nil
+	}
+
+	var args structs.ACLRoleListRequest
+	if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done {
+		return nil, nil
+	}
+
+	if args.Datacenter == "" {
+		args.Datacenter = s.agent.config.Datacenter
+	}
+
+	args.Policy = req.URL.Query().Get("policy")
+
+	var out structs.ACLRoleListResponse
+	defer setMeta(resp, &out.QueryMeta)
+	if err := s.agent.RPC("ACL.RoleList", &args, &out); err != nil {
+		return nil, err
+	}
+
+	// make sure we return an array and not nil
+	if out.Roles == nil {
+		out.Roles = make(structs.ACLRoles, 0)
+	}
+
+	return out.Roles, nil
+}
+
+func (s *HTTPServer) ACLRoleCRUD(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+	if s.checkACLDisabled(resp, req) {
+		return nil, nil
+	}
+
+	var fn func(resp http.ResponseWriter, req *http.Request, roleID string) (interface{}, error)
+
+	switch req.Method {
+	case "GET":
+		fn = s.ACLRoleReadByID
+
+	case "PUT":
+		fn = s.ACLRoleWrite
+
+	case "DELETE":
+		fn = s.ACLRoleDelete
+
+	default:
+		return nil, MethodNotAllowedError{req.Method, []string{"GET", "PUT", "DELETE"}}
+	}
+
+	roleID := strings.TrimPrefix(req.URL.Path, "/v1/acl/role/")
+	if roleID == "" && req.Method != "PUT" {
+		return nil, BadRequestError{Reason: "Missing role ID"}
+	}
+
+	return fn(resp, req, roleID)
+}
+
+func (s *HTTPServer) ACLRoleReadByName(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+	if s.checkACLDisabled(resp, req) {
+		return nil, nil
+	}
+
+	roleName := strings.TrimPrefix(req.URL.Path, "/v1/acl/role/name/")
+	if roleName == "" {
+		return nil, BadRequestError{Reason: "Missing role Name"}
+	}
+
+	return s.ACLRoleRead(resp, req, "", roleName)
+}
+
+func (s *HTTPServer) ACLRoleReadByID(resp http.ResponseWriter, req *http.Request, roleID string) (interface{}, error) {
+	return s.ACLRoleRead(resp, req, roleID, "")
+}
+
+func (s *HTTPServer) ACLRoleRead(resp http.ResponseWriter, req *http.Request, roleID, roleName string) (interface{}, error) {
+	args := structs.ACLRoleGetRequest{
+		Datacenter: s.agent.config.Datacenter,
+		RoleID:     roleID,
+		RoleName:   roleName,
+	}
+	if done := s.parse(resp, req, &args.Datacenter, &args.QueryOptions); done {
+		return nil, nil
+	}
+
+	if args.Datacenter == "" {
+		args.Datacenter = s.agent.config.Datacenter
+	}
+
+	var out structs.ACLRoleResponse
+	defer setMeta(resp, &out.QueryMeta)
+	if err := s.agent.RPC("ACL.RoleRead", &args, &out); err != nil {
+		return nil, err
+	}
+
+	if out.Role == nil {
+		resp.WriteHeader(http.StatusNotFound)
+		return nil, nil
+	}
+
+	return out.Role, nil
+}
+
+func (s *HTTPServer) ACLRoleCreate(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+	if s.checkACLDisabled(resp, req) {
+		return nil, nil
+	}
+
+	return s.ACLRoleWrite(resp, req, "")
+}
+
+func (s *HTTPServer) ACLRoleWrite(resp http.ResponseWriter, req *http.Request, roleID string) (interface{}, error) {
+	args := structs.ACLRoleSetRequest{
+		Datacenter: s.agent.config.Datacenter,
+	}
+	s.parseToken(req, &args.Token)
+
+	if err := decodeBody(req, &args.Role, fixTimeAndHashFields); err != nil {
+		return nil, BadRequestError{Reason: fmt.Sprintf("Role decoding failed: %v", err)}
+	}
+
+	if args.Role.ID != "" && args.Role.ID != roleID {
+		return nil, BadRequestError{Reason: "Role ID in URL and payload do not match"}
+	} else if args.Role.ID == "" {
+		args.Role.ID = roleID
+	}
+
+	var out structs.ACLRole
+	if err := s.agent.RPC("ACL.RoleSet", args, &out); err != nil {
+		return nil, err
+	}
+
+	return &out, nil
+}
+
+func (s *HTTPServer) ACLRoleDelete(resp http.ResponseWriter, req *http.Request, roleID string) (interface{}, error) {
+	args := structs.ACLRoleDeleteRequest{
+		Datacenter: s.agent.config.Datacenter,
+		RoleID:     roleID,
+	}
+	s.parseToken(req, &args.Token)
+
+	var ignored string
+	if err := s.agent.RPC("ACL.RoleDelete", args, &ignored); err != nil {
+		return nil, err
+	}
+
+	return true, nil
+}

agent/agent.go

@@ -1001,6 +1001,9 @@ func (a *Agent) consulConfig() (*consul.Config, error) {
 	if a.config.ACLPolicyTTL != 0 {
 		base.ACLPolicyTTL = a.config.ACLPolicyTTL
 	}
+	if a.config.ACLRoleTTL != 0 {
+		base.ACLRoleTTL = a.config.ACLRoleTTL
+	}
 	if a.config.ACLDefaultPolicy != "" {
 		base.ACLDefaultPolicy = a.config.ACLDefaultPolicy
 	}

agent/config/builder.go

@@ -702,6 +702,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) {
 		ACLReplicationToken:       b.stringValWithDefault(c.ACL.Tokens.Replication, b.stringVal(c.ACLReplicationToken)),
 		ACLTokenTTL:               b.durationValWithDefault("acl.token_ttl", c.ACL.TokenTTL, b.durationVal("acl_ttl", c.ACLTTL)),
 		ACLPolicyTTL:              b.durationVal("acl.policy_ttl", c.ACL.PolicyTTL),
+		ACLRoleTTL:                b.durationVal("acl.role_ttl", c.ACL.RoleTTL),
 		ACLToken:                  b.stringValWithDefault(c.ACL.Tokens.Default, b.stringVal(c.ACLToken)),
 		ACLTokenReplication:       b.boolValWithDefault(c.ACL.TokenReplication, b.boolValWithDefault(c.EnableACLReplication, enableTokenReplication)),
 		ACLEnableTokenPersistence: b.boolValWithDefault(c.ACL.EnableTokenPersistence, false),

agent/config/config.go

@@ -635,6 +635,7 @@ type ACL struct {
 	Enabled                *bool   `json:"enabled,omitempty" hcl:"enabled" mapstructure:"enabled"`
 	TokenReplication       *bool   `json:"enable_token_replication,omitempty" hcl:"enable_token_replication" mapstructure:"enable_token_replication"`
 	PolicyTTL              *string `json:"policy_ttl,omitempty" hcl:"policy_ttl" mapstructure:"policy_ttl"`
+	RoleTTL                *string `json:"role_ttl,omitempty" hcl:"role_ttl" mapstructure:"role_ttl"`
 	TokenTTL               *string `json:"token_ttl,omitempty" hcl:"token_ttl" mapstructure:"token_ttl"`
 	DownPolicy             *string `json:"down_policy,omitempty" hcl:"down_policy" mapstructure:"down_policy"`
 	DefaultPolicy          *string `json:"default_policy,omitempty" hcl:"default_policy" mapstructure:"default_policy"`

agent/config/runtime.go

@@ -155,6 +155,12 @@ type RuntimeConfig struct {
 	// hcl: acl.token_ttl = "duration"
 	ACLPolicyTTL time.Duration
 
+	// ACLRoleTTL is used to control the time-to-live of cached ACL roles. This has
+	// a major impact on performance. By default, it is set to 30 seconds.
+	//
+	// hcl: acl.role_ttl = "duration"
+	ACLRoleTTL time.Duration
+
 	// ACLToken is the default token used to make requests if a per-request
 	// token is not provided. If not configured the 'anonymous' token is used.
 	//

agent/config/runtime_test.go

@@ -2901,6 +2901,7 @@ func TestFullConfig(t *testing.T) {
 				"enable_key_list_policy": false,
 				"enable_token_persistence": true,
 				"policy_ttl": "1123s",
+				"role_ttl": "9876s",
 				"token_ttl": "3321s",
 				"enable_token_replication" : true,
 				"tokens" : {
@@ -3464,6 +3465,7 @@ func TestFullConfig(t *testing.T) {
 				enable_key_list_policy = false
 				enable_token_persistence = true
 				policy_ttl = "1123s"
+				role_ttl = "9876s"
 				token_ttl = "3321s"
 				enable_token_replication = true
 				tokens = {
@@ -4145,6 +4147,7 @@ func TestFullConfig(t *testing.T) {
 		ACLReplicationToken:              "5795983a",
 		ACLTokenTTL:                      3321 * time.Second,
 		ACLPolicyTTL:                     1123 * time.Second,
+		ACLRoleTTL:                       9876 * time.Second,
 		ACLToken:                         "418fdff1",
 		ACLTokenReplication:              true,
 		AdvertiseAddrLAN:                 ipAddr("17.99.29.16"),
@@ -4975,6 +4978,7 @@ func TestSanitize(t *testing.T) {
 		"ACLMasterToken": "hidden",
 		"ACLPolicyTTL": "0s",
 		"ACLReplicationToken": "hidden",
+		"ACLRoleTTL": "0s",
 		"ACLTokenReplication": false,
 		"ACLTokenTTL": "0s",
 		"ACLToken": "hidden",

agent/consul/acl.go

@@ -65,6 +65,10 @@ const (
 	// Maximum number of re-resolution requests to be made if the token is modified between
 	// resolving the token and resolving its policies that would remove one of its policies.
 	tokenPolicyResolutionMaxRetries = 5
+
+	// Maximum number of re-resolution requests to be made if the token is modified between
+	// resolving the token and resolving its roles that would remove one of its roles.
+	tokenRoleResolutionMaxRetries = 5
 )
 
 func minTTL(a time.Duration, b time.Duration) time.Duration {
@@ -93,15 +97,16 @@ type ACLResolverDelegate interface {
 	UseLegacyACLs() bool
 	ResolveIdentityFromToken(token string) (bool, structs.ACLIdentity, error)
 	ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy, error)
+	ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error)
 	RPC(method string, args interface{}, reply interface{}) error
 }
 
-type policyTokenError struct {
+type policyOrRoleTokenError struct {
 	Err   error
 	token string
 }
 
-func (e policyTokenError) Error() string {
+func (e policyOrRoleTokenError) Error() string {
 	return e.Err.Error()
 }
 
@@ -129,9 +134,11 @@ type ACLResolverConfig struct {
 // Supports:
 //   - Resolving tokens locally via the ACLResolverDelegate
 //   - Resolving policies locally via the ACLResolverDelegate
+//   - Resolving roles locally via the ACLResolverDelegate
 //   - Resolving legacy tokens remotely via a ACL.GetPolicy RPC
 //   - Resolving tokens remotely via an ACL.TokenRead RPC
 //   - Resolving policies remotely via an ACL.PolicyResolve RPC
+//   - Resolving roles remotely via an ACL.RoleResolve RPC
 //
 // Remote Resolution:
 //   Remote resolution can be done synchronously or asynchronously depending
@@ -141,7 +148,7 @@ type ACLResolverConfig struct {
 //   then go routines will be spawned to perform the RPCs in the background
 //   and then will update the cache with either the positive or negative result.
 //
-//   When the down policy is set to extend-cache or the token/policy is not already
+//   When the down policy is set to extend-cache or the token/policy/role is not already
 //   cached then the same go routines are spawned to do the RPCs in the background.
 //   However in this mode channels are created to receive the results of the RPC
 //   and are registered with the resolver. Those channels are immediately read/blocked
@@ -157,6 +164,7 @@ type ACLResolver struct {
 	cache         *structs.ACLCaches
 	identityGroup singleflight.Group
 	policyGroup   singleflight.Group
+	roleGroup     singleflight.Group
 	legacyGroup   singleflight.Group
 
 	down acl.Authorizer
@@ -447,7 +455,7 @@ func (r *ACLResolver) fetchAndCachePoliciesForIdentity(identity structs.ACLIdent
 		// Do not touch the policy cache. Getting a top level ACL not found error
 		// only indicates that the secret token used in the request
 		// no longer exists
-		return nil, &policyTokenError{acl.ErrNotFound, identity.SecretToken()}
+		return nil, &policyOrRoleTokenError{acl.ErrNotFound, identity.SecretToken()}
 	}
 
 	if acl.IsErrPermissionDenied(err) {
@@ -457,7 +465,7 @@ func (r *ACLResolver) fetchAndCachePoliciesForIdentity(identity structs.ACLIdent
 
 		// Do not remove from the policy cache for permission denied
 		// what this does indicate is that our view of the token is out of date
-		return nil, &policyTokenError{acl.ErrPermissionDenied, identity.SecretToken()}
+		return nil, &policyOrRoleTokenError{acl.ErrPermissionDenied, identity.SecretToken()}
 	}
 
 	// other RPC error - use cache if available
@@ -483,6 +491,78 @@ func (r *ACLResolver) fetchAndCachePoliciesForIdentity(identity structs.ACLIdent
 	return out, nil
 }
 
+func (r *ACLResolver) fetchAndCacheRolesForIdentity(identity structs.ACLIdentity, roleIDs []string, cached map[string]*structs.RoleCacheEntry) (map[string]*structs.ACLRole, error) {
+	req := structs.ACLRoleBatchGetRequest{
+		Datacenter: r.delegate.ACLDatacenter(false),
+		RoleIDs:    roleIDs,
+		QueryOptions: structs.QueryOptions{
+			Token:      identity.SecretToken(),
+			AllowStale: true,
+		},
+	}
+
+	var resp structs.ACLRoleBatchResponse
+	err := r.delegate.RPC("ACL.RoleResolve", &req, &resp)
+	if err == nil {
+		out := make(map[string]*structs.ACLRole)
+		for _, role := range resp.Roles {
+			out[role.ID] = role
+		}
+
+		for _, roleID := range roleIDs {
+			if role, ok := out[roleID]; ok {
+				r.cache.PutRole(roleID, role)
+			} else {
+				r.cache.PutRole(roleID, nil)
+			}
+		}
+		return out, nil
+	}
+
+	if acl.IsErrNotFound(err) {
+		// make sure to indicate that this identity is no longer valid within
+		// the cache
+		r.cache.PutIdentity(identity.SecretToken(), nil)
+
+		// Do not touch the cache. Getting a top level ACL not found error
+		// only indicates that the secret token used in the request
+		// no longer exists
+		return nil, &policyOrRoleTokenError{acl.ErrNotFound, identity.SecretToken()}
+	}
+
+	if acl.IsErrPermissionDenied(err) {
+		// invalidate our ID cache so that identity resolution will take place
+		// again in the future
+		r.cache.RemoveIdentity(identity.SecretToken())
+
+		// Do not remove from the cache for permission denied
+		// what this does indicate is that our view of the token is out of date
+		return nil, &policyOrRoleTokenError{acl.ErrPermissionDenied, identity.SecretToken()}
+	}
+
+	// other RPC error - use cache if available
+
+	extendCache := r.config.ACLDownPolicy == "extend-cache" || r.config.ACLDownPolicy == "async-cache"
+
+	out := make(map[string]*structs.ACLRole)
+	insufficientCache := false
+	for _, roleID := range roleIDs {
+		if entry, ok := cached[roleID]; extendCache && ok {
+			r.cache.PutRole(roleID, entry.Role)
+			if entry.Role != nil {
+				out[roleID] = entry.Role
+			}
+		} else {
+			r.cache.PutRole(roleID, nil)
+			insufficientCache = true
+		}
+	}
+	if insufficientCache {
+		return nil, ACLRemoteError{Err: err}
+	}
+	return out, nil
+}
+
 func (r *ACLResolver) filterPoliciesByScope(policies structs.ACLPolicies) structs.ACLPolicies {
 	var out structs.ACLPolicies
 	for _, policy := range policies {
@@ -504,9 +584,10 @@ func (r *ACLResolver) filterPoliciesByScope(policies structs.ACLPolicies) struct
 
 func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (structs.ACLPolicies, error) {
 	policyIDs := identity.PolicyIDs()
+	roleIDs := identity.RoleIDs()
 	serviceIdentities := identity.ServiceIdentityList()
 
-	if len(policyIDs) == 0 && len(serviceIdentities) == 0 {
+	if len(policyIDs) == 0 && len(serviceIdentities) == 0 && len(roleIDs) == 0 {
 		policy := identity.EmbeddedPolicy()
 		if policy != nil {
 			return []*structs.ACLPolicy{policy}, nil
@@ -516,6 +597,25 @@ func (r *ACLResolver) resolvePoliciesForIdentity(identity structs.ACLIdentity) (
 		return nil, nil
 	}
 
+	// Collect all of the roles tied to this token.
+	roles, err := r.collectRolesForIdentity(identity, roleIDs)
+	if err != nil {
+		return nil, err
+	}
+
+	// Merge the policies and service identities across Token and Role fields.
+	for _, role := range roles {
+		for _, link := range role.Policies {
+			policyIDs = append(policyIDs, link.ID)
+		}
+		serviceIdentities = append(serviceIdentities, role.ServiceIdentities...)
+	}
+
+	// Now deduplicate any policies or service identities that occur more than once.
+	policyIDs = dedupeStringSlice(policyIDs)
+	serviceIdentities = dedupeServiceIdentities(serviceIdentities)
+
+	// Generate synthetic policies for all service identities in effect.
 	syntheticPolicies := r.synthesizePoliciesForServiceIdentities(serviceIdentities)
 
 	// For the new ACLs policy replication is mandatory for correct operation on servers. Therefore
@@ -535,9 +635,6 @@ func (r *ACLResolver) synthesizePoliciesForServiceIdentities(serviceIdentities [
 		return nil
 	}
 
-	// Collect and dedupe service identities. Prefer increasing datacenter scope.
-	serviceIdentities = dedupeServiceIdentities(serviceIdentities)
-
 	syntheticPolicies := make([]*structs.ACLPolicy, 0, len(serviceIdentities))
 	for _, s := range serviceIdentities {
 		syntheticPolicies = append(syntheticPolicies, s.SyntheticPolicy())
@@ -590,6 +687,10 @@ func mergeStringSlice(a, b []string) []string {
 func dedupeStringSlice(in []string) []string {
 	// From: https://github.com/golang/go/wiki/SliceTricks#in-place-deduplicate-comparable
 
+	if len(in) <= 1 {
+		return in
+	}
+
 	sort.Strings(in)
 
 	j := 0
@@ -635,7 +736,7 @@ func (r *ACLResolver) collectPoliciesForIdentity(identity structs.ACLIdentity, p
 		}
 
 		if entry.Policy == nil {
-			// this happens when we cache a negative response for the policies existence
+			// this happens when we cache a negative response for the policy's existence
 			continue
 		}
 
@@ -689,6 +790,99 @@ func (r *ACLResolver) collectPoliciesForIdentity(identity structs.ACLIdentity, p
 	return policies, nil
 }
 
+func (r *ACLResolver) resolveRolesForIdentity(identity structs.ACLIdentity) (structs.ACLRoles, error) {
+	return r.collectRolesForIdentity(identity, identity.RoleIDs())
+}
+
+func (r *ACLResolver) collectRolesForIdentity(identity structs.ACLIdentity, roleIDs []string) (structs.ACLRoles, error) {
+	if len(roleIDs) == 0 {
+		return nil, nil
+	}
+
+	// For the new ACLs policy & role replication is mandatory for correct operation
+	// on servers. Therefore we only attempt to resolve roles locally
+	roles := make([]*structs.ACLRole, 0, len(roleIDs))
+
+	var missing []string
+	var expired []*structs.ACLRole
+	expCacheMap := make(map[string]*structs.RoleCacheEntry)
+
+	for _, roleID := range roleIDs {
+		if done, role, err := r.delegate.ResolveRoleFromID(roleID); done {
+			if err != nil && !acl.IsErrNotFound(err) {
+				return nil, err
+			}
+
+			if role != nil {
+				roles = append(roles, role)
+			} else {
+				r.logger.Printf("[WARN] acl: role %q not found for identity %q", roleID, identity.ID())
+			}
+
+			continue
+		}
+
+		// create the missing list which we can execute an RPC to get all the missing roles at once
+		entry := r.cache.GetRole(roleID)
+		if entry == nil {
+			missing = append(missing, roleID)
+			continue
+		}
+
+		if entry.Role == nil {
+			// this happens when we cache a negative response for the role's existence
+			continue
+		}
+
+		if entry.Age() >= r.config.ACLRoleTTL {
+			expired = append(expired, entry.Role)
+			expCacheMap[roleID] = entry
+		} else {
+			roles = append(roles, entry.Role)
+		}
+	}
+
+	// Hot-path if we have no missing or expired roles
+	if len(missing)+len(expired) == 0 {
+		return roles, nil
+	}
+
+	hasMissing := len(missing) > 0
+
+	fetchIDs := missing
+	for _, role := range expired {
+		fetchIDs = append(fetchIDs, role.ID)
+	}
+
+	waitChan := r.roleGroup.DoChan(identity.SecretToken(), func() (interface{}, error) {
+		roles, err := r.fetchAndCacheRolesForIdentity(identity, fetchIDs, expCacheMap)
+		return roles, err
+	})
+
+	waitForResult := hasMissing || r.config.ACLDownPolicy != "async-cache"
+	if !waitForResult {
+		// waitForResult being false requires that all the roles were cached already
+		roles = append(roles, expired...)
+		return roles, nil
+	}
+
+	res := <-waitChan
+
+	if res.Err != nil {
+		return nil, res.Err
+	}
+
+	if res.Val != nil {
+		foundRoles := res.Val.(map[string]*structs.ACLRole)
+
+		for _, role := range foundRoles {
+			roles = append(roles, role)
+		}
+	}
+
+	return roles, nil
+}
+
 func (r *ACLResolver) resolveTokenToPolicies(token string) (structs.ACLPolicies, error) {
 	_, policies, err := r.resolveTokenToIdentityAndPolicies(token)
 	return policies, err
@@ -717,13 +911,52 @@ func (r *ACLResolver) resolveTokenToIdentityAndPolicies(token string) (structs.A
 		}
 		lastErr = err
 
-		if tokenErr, ok := err.(*policyTokenError); ok {
+		if tokenErr, ok := err.(*policyOrRoleTokenError); ok {
 			if acl.IsErrNotFound(err) && tokenErr.token == identity.SecretToken() {
 				// token was deleted while resolving policies
 				return nil, nil, acl.ErrNotFound
 			}
 
-			// other types of policyTokenErrors should cause retrying the whole token
+			// other types of policyOrRoleTokenErrors should cause retrying the whole token
+			// resolution process
+		} else {
+			return identity, nil, err
+		}
+	}
+
+	return lastIdentity, nil, lastErr
+}
+
+func (r *ACLResolver) resolveTokenToIdentityAndRoles(token string) (structs.ACLIdentity, structs.ACLRoles, error) {
+	var lastErr error
+	var lastIdentity structs.ACLIdentity
+
+	for i := 0; i < tokenRoleResolutionMaxRetries; i++ {
+		// Resolve the token to an ACLIdentity
+		identity, err := r.resolveIdentityFromToken(token)
+		if err != nil {
+			return nil, nil, err
+		} else if identity == nil {
+			return nil, nil, acl.ErrNotFound
+		} else if identity.IsExpired(time.Now()) {
+			return nil, nil, acl.ErrNotFound
+		}
+
+		lastIdentity = identity
+
+		roles, err := r.resolveRolesForIdentity(identity)
+		if err == nil {
+			return identity, roles, nil
+		}
+		lastErr = err
+
+		if tokenErr, ok := err.(*policyOrRoleTokenError); ok {
+			if acl.IsErrNotFound(err) && tokenErr.token == identity.SecretToken() {
+				// token was deleted while resolving roles
+				return nil, nil, acl.ErrNotFound
+			}
+
+			// other types of policyOrRoleTokenErrors should cause retrying the whole token
 			// resolution process
 		} else {
 			return identity, nil, err

agent/consul/acl_client.go

@@ -25,6 +25,8 @@ var clientACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{
 	ParsedPolicies: 128,
 	// Authorizers - number of compiled multi-policy effective policies that can be cached
 	Authorizers: 256,
+	// Roles - number of ACL roles that can be cached
+	Roles: 128,
 }
 
 func (c *Client) UseLegacyACLs() bool {
@@ -96,6 +98,11 @@ func (c *Client) ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy,
 	return false, nil, nil
 }
 
+func (c *Client) ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error) {
+	// clients do no local role resolution at the moment
+	return false, nil, nil
+}
+
 func (c *Client) ResolveToken(token string) (acl.Authorizer, error) {
 	return c.acls.ResolveToken(token)
 }

agent/consul/acl_endpoint.go

@@ -28,6 +28,7 @@ var (
 	validPolicyName              = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,128}$`)
 	validServiceIdentityName     = regexp.MustCompile(`^[a-z0-9]([a-z0-9\-_]*[a-z0-9])?$`)
 	serviceIdentityNameMaxLength = 256
+	validRoleName                = regexp.MustCompile(`^[A-Za-z0-9\-_]{1,256}$`)
 )
 
 // ACL endpoint is used to manipulate ACLs
@@ -463,6 +464,33 @@ func (a *ACL) tokenSetInternal(args *structs.ACLTokenSetRequest, reply *structs.
 	}
 	token.Policies = policies
 
+	roleIDs := make(map[string]struct{})
+	var roles []structs.ACLTokenRoleLink
+
+	// Validate all the role names and convert them to role IDs
+	for _, link := range token.Roles {
+		if link.ID == "" {
+			_, role, err := state.ACLRoleGetByName(nil, link.Name)
+			if err != nil {
+				return fmt.Errorf("Error looking up role for name %q: %v", link.Name, err)
+			}
+			if role == nil {
+				return fmt.Errorf("No such ACL role with name %q", link.Name)
+			}
+			link.ID = role.ID
+		}
+
+		// Do not store the role name within raft/memdb as the role could be renamed in the future.
+		link.Name = ""
+
+		// dedup role links by id
+		if _, ok := roleIDs[link.ID]; !ok {
+			roles = append(roles, link)
+			roleIDs[link.ID] = struct{}{}
+		}
+	}
+	token.Roles = roles
+
 	for _, svcid := range token.ServiceIdentities {
 		if svcid.ServiceName == "" {
 			return fmt.Errorf("Service identity is missing the service name field on this token")
@@ -624,7 +652,7 @@ func (a *ACL) TokenList(args *structs.ACLTokenListRequest, reply *structs.ACLTok
 
 	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
 		func(ws memdb.WatchSet, state *state.Store) error {
-			index, tokens, err := state.ACLTokenList(ws, args.IncludeLocal, args.IncludeGlobal, args.Policy)
+			index, tokens, err := state.ACLTokenList(ws, args.IncludeLocal, args.IncludeGlobal, args.Policy, args.Role)
 			if err != nil {
 				return err
 			}
@@ -1053,3 +1081,334 @@ func (a *ACL) ReplicationStatus(args *structs.DCSpecificRequest,
 func timePointer(t time.Time) *time.Time {
 	return &t
 }
+
+func (a *ACL) RoleRead(args *structs.ACLRoleGetRequest, reply *structs.ACLRoleResponse) error {
+	if err := a.aclPreCheck(); err != nil {
+		return err
+	}
+
+	if done, err := a.srv.forward("ACL.RoleRead", args, args, reply); done {
+		return err
+	}
+
+	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
+		return err
+	} else if rule == nil || !rule.ACLRead() {
+		return acl.ErrPermissionDenied
+	}
+
+	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
+		func(ws memdb.WatchSet, state *state.Store) error {
+			var (
+				index uint64
+				role  *structs.ACLRole
+				err   error
+			)
+			if args.RoleID != "" {
+				index, role, err = state.ACLRoleGetByID(ws, args.RoleID)
+			} else {
+				index, role, err = state.ACLRoleGetByName(ws, args.RoleName)
+			}
+
+			if err != nil {
+				return err
+			}
+
+			reply.Index, reply.Role = index, role
+			return nil
+		})
+}
+
+func (a *ACL) RoleBatchRead(args *structs.ACLRoleBatchGetRequest, reply *structs.ACLRoleBatchResponse) error {
+	if err := a.aclPreCheck(); err != nil {
+		return err
+	}
+
+	if done, err := a.srv.forward("ACL.RoleBatchRead", args, args, reply); done {
+		return err
+	}
+
+	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
+		return err
+	} else if rule == nil || !rule.ACLRead() {
+		return acl.ErrPermissionDenied
+	}
+
+	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
+		func(ws memdb.WatchSet, state *state.Store) error {
+			index, roles, err := state.ACLRoleBatchGet(ws, args.RoleIDs)
+			if err != nil {
+				return err
+			}
+
+			reply.Index, reply.Roles = index, roles
+			return nil
+		})
+}
+
+func (a *ACL) RoleSet(args *structs.ACLRoleSetRequest, reply *structs.ACLRole) error {
+	if err := a.aclPreCheck(); err != nil {
+		return err
+	}
+
+	if !a.srv.InACLDatacenter() {
+		args.Datacenter = a.srv.config.ACLDatacenter
+	}
+
+	if done, err := a.srv.forward("ACL.RoleSet", args, args, reply); done {
+		return err
+	}
+
+	defer metrics.MeasureSince([]string{"acl", "role", "upsert"}, time.Now())
+
+	// Verify token is permitted to modify ACLs
+	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
+		return err
+	} else if rule == nil || !rule.ACLWrite() {
+		return acl.ErrPermissionDenied
+	}
+
+	role := &args.Role
+	state := a.srv.fsm.State()
+
+	// Almost all of the checks here are also done in the state store. However,
+	// we want to prevent the raft operations when we know they are going to fail
+	// so we still do them here.
+
+	// ensure a name is set
+	if role.Name == "" {
+		return fmt.Errorf("Invalid Role: no Name is set")
+	}
+
+	if !validRoleName.MatchString(role.Name) {
+		return fmt.Errorf("Invalid Role: invalid Name. Only alphanumeric characters, '-' and '_' are allowed")
+	}
+
+	if role.ID == "" {
+		// with no role ID one will be generated
+		var err error
+
+		role.ID, err = lib.GenerateUUID(a.srv.checkRoleUUID)
+		if err != nil {
+			return err
+		}
+
+		// validate the name is unique
+		if _, existing, err := state.ACLRoleGetByName(nil, role.Name); err != nil {
+			return fmt.Errorf("acl role lookup by name failed: %v", err)
+		} else if existing != nil {
+			return fmt.Errorf("Invalid Role: A Role with Name %q already exists", role.Name)
+		}
+	} else {
+		if _, err := uuid.ParseUUID(role.ID); err != nil {
+			return fmt.Errorf("Role ID invalid UUID")
+		}
+
+		// Verify the role exists
+		_, existing, err := state.ACLRoleGetByID(nil, role.ID)
+		if err != nil {
+			return fmt.Errorf("acl role lookup failed: %v", err)
+		} else if existing == nil {
+			return fmt.Errorf("cannot find role %s", role.ID)
+		}
+
+		if existing.Name != role.Name {
+			if _, nameMatch, err := state.ACLRoleGetByName(nil, role.Name); err != nil {
+				return fmt.Errorf("acl role lookup by name failed: %v", err)
+			} else if nameMatch != nil {
+				return fmt.Errorf("Invalid Role: A role with name %q already exists", role.Name)
+			}
+		}
+	}
+
+	policyIDs := make(map[string]struct{})
+	var policies []structs.ACLRolePolicyLink
+
+	// Validate all the policy names and convert them to policy IDs
+	for _, link := range role.Policies {
+		if link.ID == "" {
+			_, policy, err := state.ACLPolicyGetByName(nil, link.Name)
+			if err != nil {
+				return fmt.Errorf("Error looking up policy for name %q: %v", link.Name, err)
+			}
+			if policy == nil {
+				return fmt.Errorf("No such ACL policy with name %q", link.Name)
+			}
+			link.ID = policy.ID
+		}
+
+		// Do not store the policy name within raft/memdb as the policy could be renamed in the future.
+		link.Name = ""
+
+		// dedup policy links by id
+		if _, ok := policyIDs[link.ID]; !ok {
+			policies = append(policies, link)
+			policyIDs[link.ID] = struct{}{}
+		}
+	}
+	role.Policies = policies
+
+	for _, svcid := range role.ServiceIdentities {
+		if svcid.ServiceName == "" {
+			return fmt.Errorf("Service identity is missing the service name field on this role")
+		}
+		// TODO(rb): ugh if a local token gets a role that has a service
+		// identity that has datacenters set, we won't be anble to enforce this
+		// next blob here. This makes me lean more towards nuking ServiceIdentity.Datacenters again
+		//
+		// if token.Local && len(svcid.Datacenters) > 0 {
+		// 	return fmt.Errorf("Service identity %q cannot specify a list of datacenters on a local token", svcid.ServiceName)
+		// }
+		if !isValidServiceIdentityName(svcid.ServiceName) {
+			return fmt.Errorf("Service identity %q has an invalid name. Only alphanumeric characters, '-' and '_' are allowed", svcid.ServiceName)
+		}
+	}
+
+	// calculate the hash for this role
+	role.SetHash(true)
+
+	req := &structs.ACLRoleBatchSetRequest{
+		Roles: structs.ACLRoles{role},
+	}
+
+	resp, err := a.srv.raftApply(structs.ACLRoleSetRequestType, req)
+	if err != nil {
+		return fmt.Errorf("Failed to apply role upsert request: %v", err)
+	}
+
+	// Remove from the cache to prevent stale cache usage
+	a.srv.acls.cache.RemoveRole(role.ID)
+
+	if respErr, ok := resp.(error); ok {
+		return respErr
+	}
+
+	if _, role, err := a.srv.fsm.State().ACLRoleGetByID(nil, role.ID); err == nil && role != nil {
+		*reply = *role
+	}
+
+	return nil
+}
+
+func (a *ACL) RoleDelete(args *structs.ACLRoleDeleteRequest, reply *string) error {
+	if err := a.aclPreCheck(); err != nil {
+		return err
+	}
+
+	if !a.srv.InACLDatacenter() {
+		args.Datacenter = a.srv.config.ACLDatacenter
+	}
+
+	if done, err := a.srv.forward("ACL.RoleDelete", args, args, reply); done {
+		return err
+	}
+
+	defer metrics.MeasureSince([]string{"acl", "role", "delete"}, time.Now())
+
+	// Verify token is permitted to modify ACLs
+	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
+		return err
+	} else if rule == nil || !rule.ACLWrite() {
+		return acl.ErrPermissionDenied
+	}
+
+	_, role, err := a.srv.fsm.State().ACLRoleGetByID(nil, args.RoleID)
+	if err != nil {
+		return err
+	}
+
+	if role == nil {
+		return nil
+	}
+
+	req := structs.ACLRoleBatchDeleteRequest{
+		RoleIDs: []string{args.RoleID},
+	}
+
+	resp, err := a.srv.raftApply(structs.ACLRoleDeleteRequestType, &req)
+	if err != nil {
+		return fmt.Errorf("Failed to apply role delete request: %v", err)
+	}
+
+	a.srv.acls.cache.RemoveRole(role.ID)
+
+	if respErr, ok := resp.(error); ok {
+		return respErr
+	}
+
+	if role != nil {
+		*reply = role.Name
+	}
+
+	return nil
+}
+
+func (a *ACL) RoleList(args *structs.ACLRoleListRequest, reply *structs.ACLRoleListResponse) error {
+	if err := a.aclPreCheck(); err != nil {
+		return err
+	}
+
+	if done, err := a.srv.forward("ACL.RoleList", args, args, reply); done {
+		return err
+	}
+
+	if rule, err := a.srv.ResolveToken(args.Token); err != nil {
+		return err
+	} else if rule == nil || !rule.ACLRead() {
+		return acl.ErrPermissionDenied
+	}
+
+	return a.srv.blockingQuery(&args.QueryOptions, &reply.QueryMeta,
+		func(ws memdb.WatchSet, state *state.Store) error {
+			index, roles, err := state.ACLRoleList(ws, args.Policy)
+			if err != nil {
+				return err
+			}
+
+			reply.Index, reply.Roles = index, roles
+			return nil
+		})
+}
+
+// RoleResolve is used to retrieve a subset of the roles associated with a given token
+// The role ids in the args simply act as a filter on the role set assigned to the token
+func (a *ACL) RoleResolve(args *structs.ACLRoleBatchGetRequest, reply *structs.ACLRoleBatchResponse) error {
+	if err := a.aclPreCheck(); err != nil {
+		return err
+	}
+
+	if done, err := a.srv.forward("ACL.RoleResolve", args, args, reply); done {
+		return err
+	}
+
+	// get full list of roles for this token
+	identity, roles, err := a.srv.acls.resolveTokenToIdentityAndRoles(args.Token)
+	if err != nil {
+		return err
+	}
+
+	idMap := make(map[string]*structs.ACLRole)
+	for _, roleID := range identity.RoleIDs() {
+		idMap[roleID] = nil
+	}
+	for _, role := range roles {
+		idMap[role.ID] = role
+	}
+
+	for _, roleID := range args.RoleIDs {
+		if role, ok := idMap[roleID]; ok {
+			// only add non-deleted roles
+			if role != nil {
+				reply.Roles = append(reply.Roles, role)
+			}
+		} else {
+			// send a permission denied to indicate that the request included
+			// role ids not associated with this token
+			return acl.ErrPermissionDenied
+		}
+	}
+
+	a.srv.setQueryMeta(&reply.QueryMeta)
+
+	return nil
+}

agent/consul/acl_endpoint_legacy.go

@@ -255,7 +255,7 @@ func (a *ACL) List(args *structs.DCSpecificRequest,
 	return a.srv.blockingQuery(&args.QueryOptions,
 		&reply.QueryMeta,
 		func(ws memdb.WatchSet, state *state.Store) error {
-			index, tokens, err := state.ACLTokenList(ws, false, true, "")
+			index, tokens, err := state.ACLTokenList(ws, false, true, "", "")
 			if err != nil {
 				return err
 			}

agent/consul/acl_endpoint_test.go

@@ -919,6 +919,51 @@ func TestACLEndpoint_TokenSet(t *testing.T) {
 		require.Len(t, token.Policies, 0)
 	})
 
+	t.Run("Create it using Roles linked by id and name", func(t *testing.T) {
+		role1, err := upsertTestRole(codec, "root", "dc1")
+		require.NoError(t, err)
+		role2, err := upsertTestRole(codec, "root", "dc1")
+		require.NoError(t, err)
+
+		req := structs.ACLTokenSetRequest{
+			Datacenter: "dc1",
+			ACLToken: structs.ACLToken{
+				Description: "foobar",
+				Roles: []structs.ACLTokenRoleLink{
+					structs.ACLTokenRoleLink{
+						ID: role1.ID,
+					},
+					structs.ACLTokenRoleLink{
+						Name: role2.Name,
+					},
+				},
+				Local: false,
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+
+		resp := structs.ACLToken{}
+
+		err = acl.TokenSet(&req, &resp)
+		require.NoError(t, err)
+
+		// Delete both roles to ensure that we skip resolving ID->Name
+		// in the returned data.
+		require.NoError(t, deleteTestRole(codec, "root", "dc1", role1.ID))
+		require.NoError(t, deleteTestRole(codec, "root", "dc1", role2.ID))
+
+		// Get the token directly to validate that it exists
+		tokenResp, err := retrieveTestToken(codec, "root", "dc1", resp.AccessorID)
+		require.NoError(t, err)
+		token := tokenResp.Token
+
+		require.NotNil(t, token.AccessorID)
+		require.Equal(t, token.Description, "foobar")
+		require.Equal(t, token.AccessorID, resp.AccessorID)
+
+		require.Len(t, token.Roles, 0)
+	})
+
 	t.Run("Create it with invalid service identity (empty)", func(t *testing.T) {
 		req := structs.ACLTokenSetRequest{
 			Datacenter: "dc1",
@@ -1783,8 +1828,7 @@ func TestACLEndpoint_PolicySet(t *testing.T) {
 	acl := ACL{srv: s1}
 	var policyID string
 
-	// Create it
-	{
+	t.Run("Create it", func(t *testing.T) {
 		req := structs.ACLPolicySetRequest{
 			Datacenter: "dc1",
 			Policy: structs.ACLPolicy{
@@ -1811,10 +1855,9 @@ func TestACLEndpoint_PolicySet(t *testing.T) {
 		require.Equal(t, policy.Rules, "service \"\" { policy = \"read\" }")
 
 		policyID = policy.ID
-	}
+	})
 
-	// Update it
-	{
+	t.Run("Update it", func(t *testing.T) {
 		req := structs.ACLPolicySetRequest{
 			Datacenter: "dc1",
 			Policy: structs.ACLPolicy{
@@ -1840,7 +1883,7 @@ func TestACLEndpoint_PolicySet(t *testing.T) {
 		require.Equal(t, policy.Description, "bat")
 		require.Equal(t, policy.Name, "bar")
 		require.Equal(t, policy.Rules, "service \"\" { policy = \"write\" }")
-	}
+	})
 }
 
 func TestACLEndpoint_PolicySet_globalManagement(t *testing.T) {
@@ -2080,6 +2123,482 @@ func TestACLEndpoint_PolicyResolve(t *testing.T) {
 	require.EqualValues(t, retrievedPolicies, policies)
 }
 
+func TestACLEndpoint_RoleRead(t *testing.T) {
+	t.Parallel()
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	role, err := upsertTestRole(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	acl := ACL{srv: s1}
+
+	req := structs.ACLRoleGetRequest{
+		Datacenter:   "dc1",
+		RoleID:       role.ID,
+		QueryOptions: structs.QueryOptions{Token: "root"},
+	}
+
+	resp := structs.ACLRoleResponse{}
+
+	err = acl.RoleRead(&req, &resp)
+	require.NoError(t, err)
+	require.Equal(t, role, resp.Role)
+}
+
+func TestACLEndpoint_RoleBatchRead(t *testing.T) {
+	t.Parallel()
+
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	r1, err := upsertTestRole(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	r2, err := upsertTestRole(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	acl := ACL{srv: s1}
+	roles := []string{r1.ID, r2.ID}
+
+	req := structs.ACLRoleBatchGetRequest{
+		Datacenter:   "dc1",
+		RoleIDs:      roles,
+		QueryOptions: structs.QueryOptions{Token: "root"},
+	}
+
+	resp := structs.ACLRoleBatchResponse{}
+
+	err = acl.RoleBatchRead(&req, &resp)
+	require.NoError(t, err)
+
+	var retrievedRoles []string
+
+	for _, v := range resp.Roles {
+		retrievedRoles = append(retrievedRoles, v.ID)
+	}
+	require.EqualValues(t, retrievedRoles, roles)
+}
+
+func TestACLEndpoint_RoleSet(t *testing.T) {
+	t.Parallel()
+
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	acl := ACL{srv: s1}
+	var roleID string
+
+	testPolicy1, err := upsertTestPolicy(codec, "root", "dc1")
+	require.NoError(t, err)
+	testPolicy2, err := upsertTestPolicy(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	t.Run("Create it", func(t *testing.T) {
+		req := structs.ACLRoleSetRequest{
+			Datacenter: "dc1",
+			Role: structs.ACLRole{
+				Description: "foobar",
+				Name:        "baz",
+				Policies: []structs.ACLRolePolicyLink{
+					structs.ACLRolePolicyLink{
+						ID: testPolicy1.ID,
+					},
+				},
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		resp := structs.ACLRole{}
+
+		err := acl.RoleSet(&req, &resp)
+		require.NoError(t, err)
+		require.NotNil(t, resp.ID)
+
+		// Get the role directly to validate that it exists
+		roleResp, err := retrieveTestRole(codec, "root", "dc1", resp.ID)
+		require.NoError(t, err)
+		role := roleResp.Role
+
+		require.NotNil(t, role.ID)
+		require.Equal(t, role.Description, "foobar")
+		require.Equal(t, role.Name, "baz")
+		require.Len(t, role.Policies, 1)
+		require.Equal(t, testPolicy1.ID, role.Policies[0].ID)
+
+		roleID = role.ID
+	})
+
+	t.Run("Update it", func(t *testing.T) {
+		req := structs.ACLRoleSetRequest{
+			Datacenter: "dc1",
+			Role: structs.ACLRole{
+				ID:          roleID,
+				Description: "bat",
+				Name:        "bar",
+				Policies: []structs.ACLRolePolicyLink{
+					structs.ACLRolePolicyLink{
+						ID: testPolicy2.ID,
+					},
+				},
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		resp := structs.ACLRole{}
+
+		err := acl.RoleSet(&req, &resp)
+		require.NoError(t, err)
+		require.NotNil(t, resp.ID)
+
+		// Get the role directly to validate that it exists
+		roleResp, err := retrieveTestRole(codec, "root", "dc1", resp.ID)
+		require.NoError(t, err)
+		role := roleResp.Role
+
+		require.NotNil(t, role.ID)
+		require.Equal(t, role.Description, "bat")
+		require.Equal(t, role.Name, "bar")
+		require.Len(t, role.Policies, 1)
+		require.Equal(t, testPolicy2.ID, role.Policies[0].ID)
+	})
+
+	t.Run("Create it using Policies linked by id and name", func(t *testing.T) {
+		policy1, err := upsertTestPolicy(codec, "root", "dc1")
+		require.NoError(t, err)
+		policy2, err := upsertTestPolicy(codec, "root", "dc1")
+		require.NoError(t, err)
+
+		req := structs.ACLRoleSetRequest{
+			Datacenter: "dc1",
+			Role: structs.ACLRole{
+				Description: "foobar",
+				Name:        "baz",
+				Policies: []structs.ACLRolePolicyLink{
+					structs.ACLRolePolicyLink{
+						ID: policy1.ID,
+					},
+					structs.ACLRolePolicyLink{
+						Name: policy2.Name,
+					},
+				},
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		resp := structs.ACLRole{}
+
+		err = acl.RoleSet(&req, &resp)
+		require.NoError(t, err)
+		require.NotNil(t, resp.ID)
+
+		// Delete both policies to ensure that we skip resolving ID->Name
+		// in the returned data.
+		require.NoError(t, deleteTestPolicy(codec, "root", "dc1", policy1.ID))
+		require.NoError(t, deleteTestPolicy(codec, "root", "dc1", policy2.ID))
+
+		// Get the role directly to validate that it exists
+		roleResp, err := retrieveTestRole(codec, "root", "dc1", resp.ID)
+		require.NoError(t, err)
+		role := roleResp.Role
+
+		require.NotNil(t, role.ID)
+		require.Equal(t, role.Description, "foobar")
+		require.Equal(t, role.Name, "baz")
+
+		require.Len(t, role.Policies, 0)
+	})
+
+	roleNameGen := func(t *testing.T) string {
+		t.Helper()
+		name, err := uuid.GenerateUUID()
+		require.NoError(t, err)
+		return name
+	}
+
+	t.Run("Create it with invalid service identity (empty)", func(t *testing.T) {
+		req := structs.ACLRoleSetRequest{
+			Datacenter: "dc1",
+			Role: structs.ACLRole{
+				Description: "foobar",
+				Name:        roleNameGen(t),
+				ServiceIdentities: []*structs.ACLServiceIdentity{
+					&structs.ACLServiceIdentity{ServiceName: ""},
+				},
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		resp := structs.ACLRole{}
+
+		err := acl.RoleSet(&req, &resp)
+		requireErrorContains(t, err, "Service identity is missing the service name field")
+	})
+
+	t.Run("Create it with invalid service identity (too large)", func(t *testing.T) {
+		long := strings.Repeat("x", serviceIdentityNameMaxLength+1)
+		req := structs.ACLRoleSetRequest{
+			Datacenter: "dc1",
+			Role: structs.ACLRole{
+				Description: "foobar",
+				Name:        roleNameGen(t),
+				ServiceIdentities: []*structs.ACLServiceIdentity{
+					&structs.ACLServiceIdentity{ServiceName: long},
+				},
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		resp := structs.ACLRole{}
+
+		err := acl.RoleSet(&req, &resp)
+		require.NotNil(t, err)
+	})
+
+	for _, test := range []struct {
+		name string
+		ok   bool
+	}{
+		{"-abc", false},
+		{"abc-", false},
+		{"a-bc", true},
+		{"_abc", false},
+		{"abc_", false},
+		{"a_bc", true},
+		{":abc", false},
+		{"abc:", false},
+		{"a:bc", false},
+		{"Abc", false},
+		{"aBc", false},
+		{"abC", false},
+		{"0abc", true},
+		{"abc0", true},
+		{"a0bc", true},
+	} {
+		var testName string
+		if test.ok {
+			testName = "Create it with valid service identity (by regex): " + test.name
+		} else {
+			testName = "Create it with invalid service identity (by regex): " + test.name
+		}
+		t.Run(testName, func(t *testing.T) {
+			req := structs.ACLRoleSetRequest{
+				Datacenter: "dc1",
+				Role: structs.ACLRole{
+					Description: "foobar",
+					Name:        roleNameGen(t),
+					ServiceIdentities: []*structs.ACLServiceIdentity{
+						&structs.ACLServiceIdentity{ServiceName: test.name},
+					},
+				},
+				WriteRequest: structs.WriteRequest{Token: "root"},
+			}
+
+			resp := structs.ACLRole{}
+
+			err := acl.RoleSet(&req, &resp)
+			if test.ok {
+				require.NoError(t, err)
+
+				// Get the token directly to validate that it exists
+				roleResp, err := retrieveTestRole(codec, "root", "dc1", resp.ID)
+				require.NoError(t, err)
+				role := roleResp.Role
+				require.ElementsMatch(t, req.Role.ServiceIdentities, role.ServiceIdentities)
+			} else {
+				require.NotNil(t, err)
+			}
+		})
+	}
+}
+
+func TestACLEndpoint_RoleSet_names(t *testing.T) {
+	t.Parallel()
+
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	acl := ACL{srv: s1}
+
+	testPolicy1, err := upsertTestPolicy(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	for _, test := range []struct {
+		name string
+		ok   bool
+	}{
+		{"", false},
+		{"-bad", true},
+		{"bad-", true},
+		{"bad?bad", false},
+		{strings.Repeat("x", 257), false},
+		{strings.Repeat("x", 256), true},
+		{"-abc", true},
+		{"abc-", true},
+		{"a-bc", true},
+		{"_abc", true},
+		{"abc_", true},
+		{"a_bc", true},
+		{":abc", false},
+		{"abc:", false},
+		{"a:bc", false},
+		{"Abc", true},
+		{"aBc", true},
+		{"abC", true},
+		{"0abc", true},
+		{"abc0", true},
+		{"a0bc", true},
+	} {
+		var testName string
+		if test.ok {
+			testName = "create with valid name: " + test.name
+		} else {
+			testName = "create with invalid name: " + test.name
+		}
+
+		t.Run(testName, func(t *testing.T) {
+			// cleanup from a prior insertion that may have succeeded
+			require.NoError(t, deleteTestRoleByName(codec, "root", "dc1", test.name))
+
+			req := structs.ACLRoleSetRequest{
+				Datacenter: "dc1",
+				Role: structs.ACLRole{
+					Name:        test.name,
+					Description: "foobar",
+					Policies: []structs.ACLRolePolicyLink{
+						structs.ACLRolePolicyLink{
+							ID: testPolicy1.ID,
+						},
+					},
+				},
+				WriteRequest: structs.WriteRequest{Token: "root"},
+			}
+			resp := structs.ACLRole{}
+
+			err := acl.RoleSet(&req, &resp)
+			if test.ok {
+				require.NoError(t, err)
+
+				roleResp, err := retrieveTestRole(codec, "root", "dc1", resp.ID)
+				require.NoError(t, err)
+				role := roleResp.Role
+				require.Equal(t, test.name, role.Name)
+			} else {
+				require.Error(t, err)
+			}
+		})
+	}
+}
+
+func TestACLEndpoint_RoleDelete(t *testing.T) {
+	t.Parallel()
+
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	existingRole, err := upsertTestRole(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	acl := ACL{srv: s1}
+
+	req := structs.ACLRoleDeleteRequest{
+		Datacenter:   "dc1",
+		RoleID:       existingRole.ID,
+		WriteRequest: structs.WriteRequest{Token: "root"},
+	}
+
+	var resp string
+
+	err = acl.RoleDelete(&req, &resp)
+	require.NoError(t, err)
+
+	// Make sure the role is gone
+	roleResp, err := retrieveTestRole(codec, "root", "dc1", existingRole.ID)
+	require.Nil(t, roleResp.Role)
+}
+
+func TestACLEndpoint_RoleList(t *testing.T) {
+	t.Parallel()
+
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	codec := rpcClient(t, s1)
+	defer codec.Close()
+
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+
+	r1, err := upsertTestRole(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	r2, err := upsertTestRole(codec, "root", "dc1")
+	require.NoError(t, err)
+
+	acl := ACL{srv: s1}
+
+	req := structs.ACLRoleListRequest{
+		Datacenter:   "dc1",
+		QueryOptions: structs.QueryOptions{Token: "root"},
+	}
+
+	resp := structs.ACLRoleListResponse{}
+
+	err = acl.RoleList(&req, &resp)
+	require.NoError(t, err)
+
+	roles := []string{r1.ID, r2.ID}
+	var retrievedRoles []string
+
+	for _, v := range resp.Roles {
+		retrievedRoles = append(retrievedRoles, v.ID)
+	}
+	require.ElementsMatch(t, retrievedRoles, roles)
+}
+
 // upsertTestToken creates a token for testing purposes
 func upsertTestToken(codec rpc.ClientCodec, masterToken string, datacenter string,
 	tokenModificationFn func(token *structs.ACLToken)) (*structs.ACLToken, error) {
@@ -2217,6 +2736,106 @@ func retrieveTestPolicy(codec rpc.ClientCodec, masterToken string, datacenter st
 	return &out, nil
 }
 
+func deleteTestRole(codec rpc.ClientCodec, masterToken string, datacenter string, roleID string) error {
+	arg := structs.ACLRoleDeleteRequest{
+		Datacenter:   datacenter,
+		RoleID:       roleID,
+		WriteRequest: structs.WriteRequest{Token: masterToken},
+	}
+
+	var ignored string
+	err := msgpackrpc.CallWithCodec(codec, "ACL.RoleDelete", &arg, &ignored)
+	return err
+}
+
+func deleteTestRoleByName(codec rpc.ClientCodec, masterToken string, datacenter string, roleName string) error {
+	resp, err := retrieveTestRoleByName(codec, masterToken, datacenter, roleName)
+	if err != nil {
+		return err
+	}
+	if resp.Role == nil {
+		return nil
+	}
+
+	return deleteTestRole(codec, masterToken, datacenter, resp.Role.ID)
+}
+
+// upsertTestRole creates a role for testing purposes
+func upsertTestRole(codec rpc.ClientCodec, masterToken string, datacenter string) (*structs.ACLRole, error) {
+	// Make sure test roles can't collide
+	roleUnq, err := uuid.GenerateUUID()
+	if err != nil {
+		return nil, err
+	}
+	policyID, err := uuid.GenerateUUID()
+	if err != nil {
+		return nil, err
+	}
+
+	arg := structs.ACLRoleSetRequest{
+		Datacenter: datacenter,
+		Role: structs.ACLRole{
+			Name: fmt.Sprintf("test-role-%s", roleUnq),
+			Policies: []structs.ACLRolePolicyLink{
+				structs.ACLRolePolicyLink{
+					ID: policyID,
+				},
+			},
+		},
+		WriteRequest: structs.WriteRequest{Token: masterToken},
+	}
+
+	var out structs.ACLRole
+
+	err = msgpackrpc.CallWithCodec(codec, "ACL.RoleSet", &arg, &out)
+
+	if err != nil {
+		return nil, err
+	}
+
+	if out.ID == "" {
+		return nil, fmt.Errorf("ID is nil: %v", out)
+	}
+
+	return &out, nil
+}
+
+func retrieveTestRole(codec rpc.ClientCodec, masterToken string, datacenter string, id string) (*structs.ACLRoleResponse, error) {
+	arg := structs.ACLRoleGetRequest{
+		Datacenter:   datacenter,
+		RoleID:       id,
+		QueryOptions: structs.QueryOptions{Token: masterToken},
+	}
+
+	var out structs.ACLRoleResponse
+
+	err := msgpackrpc.CallWithCodec(codec, "ACL.RoleRead", &arg, &out)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return &out, nil
+}
+
+func retrieveTestRoleByName(codec rpc.ClientCodec, masterToken string, datacenter string, name string) (*structs.ACLRoleResponse, error) {
+	arg := structs.ACLRoleGetRequest{
+		Datacenter:   datacenter,
+		RoleName:     name,
+		QueryOptions: structs.QueryOptions{Token: masterToken},
+	}
+
+	var out structs.ACLRoleResponse
+
+	err := msgpackrpc.CallWithCodec(codec, "ACL.RoleRead", &arg, &out)
+
+	if err != nil {
+		return nil, err
+	}
+
+	return &out, nil
+}
+
 func requireTimeEquals(t *testing.T, expect, got *time.Time) {
 	t.Helper()
 	if expect == nil && got == nil {

agent/consul/acl_replication.go

@@ -3,6 +3,7 @@ package consul
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -15,123 +16,108 @@ const (
 	aclReplicationMaxRetryBackoff = 64
 )
 
-func diffACLPolicies(local structs.ACLPolicies, remote structs.ACLPolicyListStubs, lastRemoteIndex uint64) ([]string, []string) {
-	local.Sort()
-	remote.Sort()
+// aclTypeReplicator allows the machinery of acl replication to be shared between
+// types with minimal code duplication (barring generics magically popping into
+// existence).
+//
+// Concrete implementations of this interface should internally contain a
+// pointer to the server so that data lookups can occur, and they should
+// maintain the smallest quantity of type-specific state they can.
+//
+// Implementations of this interface are short-lived and recreated on every
+// iteration.
+type aclTypeReplicator interface {
+	// Type is variant of replication in use. Used for updating the replication
+	// status tracker.
+	Type() structs.ACLReplicationType
 
-	var deletions []string
-	var updates []string
-	var localIdx int
-	var remoteIdx int
-	for localIdx, remoteIdx = 0, 0; localIdx < len(local) && remoteIdx < len(remote); {
-		if local[localIdx].ID == remote[remoteIdx].ID {
-			// policy is in both the local and remote state - need to check raft indices and the Hash
-			if remote[remoteIdx].ModifyIndex > lastRemoteIndex && !bytes.Equal(remote[remoteIdx].Hash, local[localIdx].Hash) {
-				updates = append(updates, remote[remoteIdx].ID)
-			}
-			// increment both indices when equal
-			localIdx += 1
-			remoteIdx += 1
-		} else if local[localIdx].ID < remote[remoteIdx].ID {
-			// policy no longer in remoted state - needs deleting
-			deletions = append(deletions, local[localIdx].ID)
+	// SingularNoun is the singular form of the item being replicated.
+	SingularNoun() string
 
-			// increment just the local index
-			localIdx += 1
-		} else {
-			// local state doesn't have this policy - needs updating
-			updates = append(updates, remote[remoteIdx].ID)
+	// PluralNoun is the plural form of the item being replicated.
+	PluralNoun() string
 
-			// increment just the remote index
-			remoteIdx += 1
-		}
-	}
+	// FetchRemote retrieves items newer than the provided index from the
+	// remote datacenter (for diffing purposes).
+	FetchRemote(srv *Server, lastRemoteIndex uint64) (int, uint64, error)
 
-	for ; localIdx < len(local); localIdx += 1 {
-		deletions = append(deletions, local[localIdx].ID)
-	}
+	// FetchLocal retrieves items from the current datacenter (for diffing
+	// purposes).
+	FetchLocal(srv *Server) (int, uint64, error)
 
-	for ; remoteIdx < len(remote); remoteIdx += 1 {
-		updates = append(updates, remote[remoteIdx].ID)
-	}
+	// SortState sorts the internal working state output of FetchRemote and
+	// FetchLocal so that a sane diff can be performed.
+	SortState() (lenLocal, lenRemote int)
 
-	return deletions, updates
+	// LocalMeta allows for type-agnostic metadata from the sorted local state
+	// can be retrieved for the purposes of diffing.
+	LocalMeta(i int) (id string, modIndex uint64, hash []byte)
+
+	// RemoteMeta allows for type-agnostic metadata from the sorted remote
+	// state can be retrieved for the purposes of diffing.
+	RemoteMeta(i int) (id string, modIndex uint64, hash []byte)
+
+	// FetchUpdated retrieves the specific items from the remote (during the
+	// correction phase).
+	FetchUpdated(srv *Server, updates []string) (int, error)
+
+	// LenPendingUpdates should be the size of the data retrieved in
+	// FetchUpdated.
+	LenPendingUpdates() int
+
+	// PendingUpdateIsRedacted returns true if the update contains redacted
+	// data. Really only valid for tokens.
+	PendingUpdateIsRedacted(i int) bool
+
+	// PendingUpdateEstimatedSize is the item's EstimatedSize in the state
+	// populated by FetchUpdated.
+	PendingUpdateEstimatedSize(i int) int
+
+	// UpdateLocalBatch applies a portion of the state populated by
+	// FetchUpdated to the current datacenter.
+	UpdateLocalBatch(ctx context.Context, srv *Server, start, end int) error
+
+	// DeleteLocalBatch removes items from the current datacenter.
+	DeleteLocalBatch(srv *Server, batch []string) error
 }
 
-func (s *Server) deleteLocalACLPolicies(deletions []string, ctx context.Context) (bool, error) {
-	ticker := time.NewTicker(time.Second / time.Duration(s.config.ACLReplicationApplyLimit))
-	defer ticker.Stop()
+var errContainsRedactedData = errors.New("replication results contain redacted data")
 
-	for i := 0; i < len(deletions); i += aclBatchDeleteSize {
-		req := structs.ACLPolicyBatchDeleteRequest{}
-
-		if i+aclBatchDeleteSize > len(deletions) {
-			req.PolicyIDs = deletions[i:]
-		} else {
-			req.PolicyIDs = deletions[i : i+aclBatchDeleteSize]
-		}
-
-		resp, err := s.raftApply(structs.ACLPolicyDeleteRequestType, &req)
-		if err != nil {
-			return false, fmt.Errorf("Failed to apply policy deletions: %v", err)
-		}
-		if respErr, ok := resp.(error); ok && err != nil {
-			return false, fmt.Errorf("Failed to apply policy deletions: %v", respErr)
-		}
-
-		if i+aclBatchDeleteSize < len(deletions) {
-			select {
-			case <-ctx.Done():
-				return true, nil
-			case <-ticker.C:
-				// do nothing - ready for the next batch
-			}
-		}
+func (s *Server) fetchACLRolesBatch(roleIDs []string) (*structs.ACLRoleBatchResponse, error) {
+	req := structs.ACLRoleBatchGetRequest{
+		Datacenter: s.config.ACLDatacenter,
+		RoleIDs:    roleIDs,
+		QueryOptions: structs.QueryOptions{
+			AllowStale: true,
+			Token:      s.tokens.ReplicationToken(),
+		},
 	}
 
-	return false, nil
+	var response structs.ACLRoleBatchResponse
+	if err := s.RPC("ACL.RoleBatchRead", &req, &response); err != nil {
+		return nil, err
+	}
+
+	return &response, nil
 }
 
-func (s *Server) updateLocalACLPolicies(policies structs.ACLPolicies, ctx context.Context) (bool, error) {
-	ticker := time.NewTicker(time.Second / time.Duration(s.config.ACLReplicationApplyLimit))
-	defer ticker.Stop()
+func (s *Server) fetchACLRoles(lastRemoteIndex uint64) (*structs.ACLRoleListResponse, error) {
+	defer metrics.MeasureSince([]string{"leader", "replication", "acl", "role", "fetch"}, time.Now())
 
-	// outer loop handles submitting a batch
-	for batchStart := 0; batchStart < len(policies); {
-		// inner loop finds the last element to include in this batch.
-		batchSize := 0
-		batchEnd := batchStart
-		for ; batchEnd < len(policies) && batchSize < aclBatchUpsertSize; batchEnd += 1 {
-			batchSize += policies[batchEnd].EstimateSize()
-		}
-
-		req := structs.ACLPolicyBatchSetRequest{
-			Policies: policies[batchStart:batchEnd],
-		}
-
-		resp, err := s.raftApply(structs.ACLPolicySetRequestType, &req)
-		if err != nil {
-			return false, fmt.Errorf("Failed to apply policy upserts: %v", err)
-		}
-		if respErr, ok := resp.(error); ok && respErr != nil {
-			return false, fmt.Errorf("Failed to apply policy upsert: %v", respErr)
-		}
-		s.logger.Printf("[DEBUG] acl: policy replication - upserted 1 batch with %d policies of size %d", batchEnd-batchStart, batchSize)
-
-		// policies[batchEnd] wasn't include as the slicing doesn't include the element at the stop index
-		batchStart = batchEnd
-
-		// prevent waiting if we are done
-		if batchEnd < len(policies) {
-			select {
-			case <-ctx.Done():
-				return true, nil
-			case <-ticker.C:
-				// nothing to do - just rate limiting
-			}
-		}
+	req := structs.ACLRoleListRequest{
+		Datacenter: s.config.ACLDatacenter,
+		QueryOptions: structs.QueryOptions{
+			AllowStale:    true,
+			MinQueryIndex: lastRemoteIndex,
+			Token:         s.tokens.ReplicationToken(),
+		},
 	}
-	return false, nil
+
+	var response structs.ACLRoleListResponse
+	if err := s.RPC("ACL.RoleList", &req, &response); err != nil {
+		return nil, err
+	}
+	return &response, nil
 }
 
 func (s *Server) fetchACLPoliciesBatch(policyIDs []string) (*structs.ACLPolicyBatchResponse, error) {
@@ -171,66 +157,72 @@ func (s *Server) fetchACLPolicies(lastRemoteIndex uint64) (*structs.ACLPolicyLis
 	return &response, nil
 }
 
-type tokenDiffResults struct {
+type itemDiffResults struct {
 	LocalDeletes  []string
 	LocalUpserts  []string
 	LocalSkipped  int
 	RemoteSkipped int
 }
 
-func diffACLTokens(local structs.ACLTokens, remote structs.ACLTokenListStubs, lastRemoteIndex uint64) tokenDiffResults {
-	// Note: items with empty AccessorIDs will bubble up to the top.
-	local.Sort()
-	remote.Sort()
+func diffACLType(tr aclTypeReplicator, lastRemoteIndex uint64) itemDiffResults {
+	// Note: items with empty IDs will bubble up to the top (like legacy, unmigrated Tokens)
 
-	var res tokenDiffResults
+	lenLocal, lenRemote := tr.SortState()
+
+	var res itemDiffResults
 	var localIdx int
 	var remoteIdx int
-	for localIdx, remoteIdx = 0, 0; localIdx < len(local) && remoteIdx < len(remote); {
-		if local[localIdx].AccessorID == "" {
+	for localIdx, remoteIdx = 0, 0; localIdx < lenLocal && remoteIdx < lenRemote; {
+		localID, _, localHash := tr.LocalMeta(localIdx)
+		remoteID, remoteMod, remoteHash := tr.RemoteMeta(remoteIdx)
+
+		if localID == "" {
 			res.LocalSkipped++
 			localIdx += 1
 			continue
 		}
-		if remote[remoteIdx].AccessorID == "" {
+		if remoteID == "" {
 			res.RemoteSkipped++
 			remoteIdx += 1
 			continue
 		}
-		if local[localIdx].AccessorID == remote[remoteIdx].AccessorID {
-			// policy is in both the local and remote state - need to check raft indices and Hash
-			if remote[remoteIdx].ModifyIndex > lastRemoteIndex && !bytes.Equal(remote[remoteIdx].Hash, local[localIdx].Hash) {
-				res.LocalUpserts = append(res.LocalUpserts, remote[remoteIdx].AccessorID)
+
+		if localID == remoteID {
+			// item is in both the local and remote state - need to check raft indices and the Hash
+			if remoteMod > lastRemoteIndex && !bytes.Equal(remoteHash, localHash) {
+				res.LocalUpserts = append(res.LocalUpserts, remoteID)
 			}
 			// increment both indices when equal
 			localIdx += 1
 			remoteIdx += 1
-		} else if local[localIdx].AccessorID < remote[remoteIdx].AccessorID {
-			// policy no longer in remoted state - needs deleting
-			res.LocalDeletes = append(res.LocalDeletes, local[localIdx].AccessorID)
+		} else if localID < remoteID {
+			// item no longer in remote state - needs deleting
+			res.LocalDeletes = append(res.LocalDeletes, localID)
 
 			// increment just the local index
 			localIdx += 1
 		} else {
-			// local state doesn't have this policy - needs updating
-			res.LocalUpserts = append(res.LocalUpserts, remote[remoteIdx].AccessorID)
+			// local state doesn't have this item - needs updating
+			res.LocalUpserts = append(res.LocalUpserts, remoteID)
 
 			// increment just the remote index
 			remoteIdx += 1
 		}
 	}
 
-	for ; localIdx < len(local); localIdx += 1 {
-		if local[localIdx].AccessorID != "" {
-			res.LocalDeletes = append(res.LocalDeletes, local[localIdx].AccessorID)
+	for ; localIdx < lenLocal; localIdx += 1 {
+		localID, _, _ := tr.LocalMeta(localIdx)
+		if localID != "" {
+			res.LocalDeletes = append(res.LocalDeletes, localID)
 		} else {
 			res.LocalSkipped++
 		}
 	}
 
-	for ; remoteIdx < len(remote); remoteIdx += 1 {
-		if remote[remoteIdx].AccessorID != "" {
-			res.LocalUpserts = append(res.LocalUpserts, remote[remoteIdx].AccessorID)
+	for ; remoteIdx < lenRemote; remoteIdx += 1 {
+		remoteID, _, _ := tr.RemoteMeta(remoteIdx)
+		if remoteID != "" {
+			res.LocalUpserts = append(res.LocalUpserts, remoteID)
 		} else {
 			res.RemoteSkipped++
 		}
@@ -239,25 +231,21 @@ func diffACLTokens(local structs.ACLTokens, remote structs.ACLTokenListStubs, la
 	return res
 }
 
-func (s *Server) deleteLocalACLTokens(deletions []string, ctx context.Context) (bool, error) {
+func (s *Server) deleteLocalACLType(ctx context.Context, tr aclTypeReplicator, deletions []string) (bool, error) {
 	ticker := time.NewTicker(time.Second / time.Duration(s.config.ACLReplicationApplyLimit))
 	defer ticker.Stop()
 
 	for i := 0; i < len(deletions); i += aclBatchDeleteSize {
-		req := structs.ACLTokenBatchDeleteRequest{}
+		var batch []string
 
 		if i+aclBatchDeleteSize > len(deletions) {
-			req.TokenIDs = deletions[i:]
+			batch = deletions[i:]
 		} else {
-			req.TokenIDs = deletions[i : i+aclBatchDeleteSize]
+			batch = deletions[i : i+aclBatchDeleteSize]
 		}
 
-		resp, err := s.raftApply(structs.ACLTokenDeleteRequestType, &req)
-		if err != nil {
-			return false, fmt.Errorf("Failed to apply token deletions: %v", err)
-		}
-		if respErr, ok := resp.(error); ok && err != nil {
-			return false, fmt.Errorf("Failed to apply token deletions: %v", respErr)
+		if err := tr.DeleteLocalBatch(s, batch); err != nil {
+			return false, fmt.Errorf("Failed to apply %s deletions: %v", tr.SingularNoun(), err)
 		}
 
 		if i+aclBatchDeleteSize < len(deletions) {
@@ -273,47 +261,50 @@ func (s *Server) deleteLocalACLTokens(deletions []string, ctx context.Context) (
 	return false, nil
 }
 
-func (s *Server) updateLocalACLTokens(tokens structs.ACLTokens, ctx context.Context) (bool, error) {
+func (s *Server) updateLocalACLType(ctx context.Context, tr aclTypeReplicator) (bool, error) {
 	ticker := time.NewTicker(time.Second / time.Duration(s.config.ACLReplicationApplyLimit))
 	defer ticker.Stop()
 
+	lenPending := tr.LenPendingUpdates()
+
 	// outer loop handles submitting a batch
-	for batchStart := 0; batchStart < len(tokens); {
+	for batchStart := 0; batchStart < lenPending; {
 		// inner loop finds the last element to include in this batch.
 		batchSize := 0
 		batchEnd := batchStart
-		for ; batchEnd < len(tokens) && batchSize < aclBatchUpsertSize; batchEnd += 1 {
-			if tokens[batchEnd].SecretID == redactedToken {
-				return false, fmt.Errorf("Detected redacted token secrets: stopping token update round - verify that the replication token in use has acl:write permissions.")
+		for ; batchEnd < lenPending && batchSize < aclBatchUpsertSize; batchEnd += 1 {
+			if tr.PendingUpdateIsRedacted(batchEnd) {
+				return false, fmt.Errorf(
+					"Detected redacted %s secrets: stopping %s update round - verify that the replication token in use has acl:write permissions.",
+					tr.SingularNoun(),
+					tr.SingularNoun(),
+				)
 			}
-			batchSize += tokens[batchEnd].EstimateSize()
+			batchSize += tr.PendingUpdateEstimatedSize(batchEnd)
 		}
 
-		req := structs.ACLTokenBatchSetRequest{
-			Tokens: tokens[batchStart:batchEnd],
-			CAS:    false,
-		}
-
-		resp, err := s.raftApply(structs.ACLTokenSetRequestType, &req)
+		err := tr.UpdateLocalBatch(ctx, s, batchStart, batchEnd)
 		if err != nil {
-			return false, fmt.Errorf("Failed to apply token upserts: %v", err)
-		}
-		if respErr, ok := resp.(error); ok && respErr != nil {
-			return false, fmt.Errorf("Failed to apply token upserts: %v", respErr)
+			return false, fmt.Errorf("Failed to apply %s upserts: %v", tr.SingularNoun(), err)
 		}
+		s.logger.Printf(
+			"[DEBUG] acl: %s replication - upserted 1 batch with %d %s of size %d",
+			tr.SingularNoun(),
+			batchEnd-batchStart,
+			tr.PluralNoun(),
+			batchSize,
+		)
 
-		s.logger.Printf("[DEBUG] acl: token replication - upserted 1 batch with %d tokens of size %d", batchEnd-batchStart, batchSize)
-
-		// tokens[batchEnd] wasn't include as the slicing doesn't include the element at the stop index
+		// items[batchEnd] wasn't include as the slicing doesn't include the element at the stop index
 		batchStart = batchEnd
 
 		// prevent waiting if we are done
-		if batchEnd < len(tokens) {
+		if batchEnd < lenPending {
 			select {
 			case <-ctx.Done():
 				return true, nil
 			case <-ticker.C:
-				// nothing to do - just rate limiting here
+				// nothing to do - just rate limiting
 			}
 		}
 	}
@@ -359,95 +350,28 @@ func (s *Server) fetchACLTokens(lastRemoteIndex uint64) (*structs.ACLTokenListRe
 	return &response, nil
 }
 
-func (s *Server) replicateACLPolicies(lastRemoteIndex uint64, ctx context.Context) (uint64, bool, error) {
-	remote, err := s.fetchACLPolicies(lastRemoteIndex)
-	if err != nil {
-		return 0, false, fmt.Errorf("failed to retrieve remote ACL policies: %v", err)
-	}
-
-	s.logger.Printf("[DEBUG] acl: finished fetching policies tokens: %d", len(remote.Policies))
-
-	// Need to check if we should be stopping. This will be common as the fetching process is a blocking
-	// RPC which could have been hanging around for a long time and during that time leadership could
-	// have been lost.
-	select {
-	case <-ctx.Done():
-		return 0, true, nil
-	default:
-		// do nothing
-	}
-
-	// Measure everything after the remote query, which can block for long
-	// periods of time. This metric is a good measure of how expensive the
-	// replication process is.
-	defer metrics.MeasureSince([]string{"leader", "replication", "acl", "policy", "apply"}, time.Now())
-
-	_, local, err := s.fsm.State().ACLPolicyList(nil)
-	if err != nil {
-		return 0, false, fmt.Errorf("failed to retrieve local ACL policies: %v", err)
-	}
-
-	// If the remote index ever goes backwards, it's a good indication that
-	// the remote side was rebuilt and we should do a full sync since we
-	// can't make any assumptions about what's going on.
-	if remote.QueryMeta.Index < lastRemoteIndex {
-		s.logger.Printf("[WARN] consul: ACL policy replication remote index moved backwards (%d to %d), forcing a full ACL policy sync", lastRemoteIndex, remote.QueryMeta.Index)
-		lastRemoteIndex = 0
-	}
-
-	s.logger.Printf("[DEBUG] acl: policy replication - local: %d, remote: %d", len(local), len(remote.Policies))
-	// Calculate the changes required to bring the state into sync and then
-	// apply them.
-	deletions, updates := diffACLPolicies(local, remote.Policies, lastRemoteIndex)
-
-	s.logger.Printf("[DEBUG] acl: policy replication - deletions: %d, updates: %d", len(deletions), len(updates))
-
-	var policies *structs.ACLPolicyBatchResponse
-	if len(updates) > 0 {
-		policies, err = s.fetchACLPoliciesBatch(updates)
-		if err != nil {
-			return 0, false, fmt.Errorf("failed to retrieve ACL policy updates: %v", err)
-		}
-		s.logger.Printf("[DEBUG] acl: policy replication - downloaded %d policies", len(policies.Policies))
-	}
-
-	if len(deletions) > 0 {
-		s.logger.Printf("[DEBUG] acl: policy replication - performing deletions")
-
-		exit, err := s.deleteLocalACLPolicies(deletions, ctx)
-		if exit {
-			return 0, true, nil
-		}
-		if err != nil {
-			return 0, false, fmt.Errorf("failed to delete local ACL policies: %v", err)
-		}
-		s.logger.Printf("[DEBUG] acl: policy replication - finished deletions")
-	}
-
-	if len(updates) > 0 {
-		s.logger.Printf("[DEBUG] acl: policy replication - performing updates")
-		exit, err := s.updateLocalACLPolicies(policies.Policies, ctx)
-		if exit {
-			return 0, true, nil
-		}
-		if err != nil {
-			return 0, false, fmt.Errorf("failed to update local ACL policies: %v", err)
-		}
-		s.logger.Printf("[DEBUG] acl: policy replication - finished updates")
-	}
-
-	// Return the index we got back from the remote side, since we've synced
-	// up with the remote state as of that index.
-	return remote.QueryMeta.Index, false, nil
+func (s *Server) replicateACLTokens(ctx context.Context, lastRemoteIndex uint64) (uint64, bool, error) {
+	tr := &aclTokenReplicator{}
+	return s.replicateACLType(ctx, tr, lastRemoteIndex)
 }
 
-func (s *Server) replicateACLTokens(lastRemoteIndex uint64, ctx context.Context) (uint64, bool, error) {
-	remote, err := s.fetchACLTokens(lastRemoteIndex)
+func (s *Server) replicateACLPolicies(ctx context.Context, lastRemoteIndex uint64) (uint64, bool, error) {
+	tr := &aclPolicyReplicator{}
+	return s.replicateACLType(ctx, tr, lastRemoteIndex)
+}
+
+func (s *Server) replicateACLRoles(ctx context.Context, lastRemoteIndex uint64) (uint64, bool, error) {
+	tr := &aclRoleReplicator{}
+	return s.replicateACLType(ctx, tr, lastRemoteIndex)
+}
+
+func (s *Server) replicateACLType(ctx context.Context, tr aclTypeReplicator, lastRemoteIndex uint64) (uint64, bool, error) {
+	lenRemote, remoteIndex, err := tr.FetchRemote(s, lastRemoteIndex)
 	if err != nil {
-		return 0, false, fmt.Errorf("failed to retrieve remote ACL tokens: %v", err)
+		return 0, false, fmt.Errorf("failed to retrieve remote ACL %s: %v", tr.PluralNoun(), err)
 	}
 
-	s.logger.Printf("[DEBUG] acl: finished fetching remote tokens: %d", len(remote.Tokens))
+	s.logger.Printf("[DEBUG] acl: finished fetching %s: %d", tr.PluralNoun(), lenRemote)
 
 	// Need to check if we should be stopping. This will be common as the fetching process is a blocking
 	// RPC which could have been hanging around for a long time and during that time leadership could
@@ -462,74 +386,99 @@ func (s *Server) replicateACLTokens(lastRemoteIndex uint64, ctx context.Context)
 	// Measure everything after the remote query, which can block for long
 	// periods of time. This metric is a good measure of how expensive the
 	// replication process is.
-	defer metrics.MeasureSince([]string{"leader", "replication", "acl", "token", "apply"}, time.Now())
+	defer metrics.MeasureSince([]string{"leader", "replication", "acl", tr.SingularNoun(), "apply"}, time.Now())
 
-	_, local, err := s.fsm.State().ACLTokenList(nil, false, true, "")
+	lenLocal, _, err := tr.FetchLocal(s)
 	if err != nil {
-		return 0, false, fmt.Errorf("failed to retrieve local ACL tokens: %v", err)
+		return 0, false, fmt.Errorf("failed to retrieve local ACL %s: %v", tr.PluralNoun(), err)
 	}
-	// Do not filter by expiration times. Wait until the tokens are explicitly deleted.
 
 	// If the remote index ever goes backwards, it's a good indication that
 	// the remote side was rebuilt and we should do a full sync since we
 	// can't make any assumptions about what's going on.
-	if remote.QueryMeta.Index < lastRemoteIndex {
-		s.logger.Printf("[WARN] consul: ACL token replication remote index moved backwards (%d to %d), forcing a full ACL token sync", lastRemoteIndex, remote.QueryMeta.Index)
+	if remoteIndex < lastRemoteIndex {
+		s.logger.Printf(
+			"[WARN] consul: ACL %s replication remote index moved backwards (%d to %d), forcing a full ACL %s sync",
+			tr.SingularNoun(),
+			lastRemoteIndex,
+			remoteIndex,
+			tr.SingularNoun(),
+		)
 		lastRemoteIndex = 0
 	}
 
-	s.logger.Printf("[DEBUG] acl: token replication - local: %d, remote: %d", len(local), len(remote.Tokens))
-
-	// Calculate the changes required to bring the state into sync and then
-	// apply them.
-	res := diffACLTokens(local, remote.Tokens, lastRemoteIndex)
+	s.logger.Printf(
+		"[DEBUG] acl: %s replication - local: %d, remote: %d",
+		tr.SingularNoun(),
+		lenLocal,
+		lenRemote,
+	)
+	// Calculate the changes required to bring the state into sync and then apply them.
+	res := diffACLType(tr, lastRemoteIndex)
 	if res.LocalSkipped > 0 || res.RemoteSkipped > 0 {
-		s.logger.Printf("[DEBUG] acl: token replication - deletions: %d, updates: %d, skipped: %d, skippedRemote: %d",
-			len(res.LocalDeletes), len(res.LocalUpserts), res.LocalSkipped, res.RemoteSkipped)
+		s.logger.Printf(
+			"[DEBUG] acl: %s replication - deletions: %d, updates: %d, skipped: %d, skippedRemote: %d",
+			tr.SingularNoun(),
+			len(res.LocalDeletes),
+			len(res.LocalUpserts),
+			res.LocalSkipped,
+			res.RemoteSkipped,
+		)
 	} else {
-		s.logger.Printf("[DEBUG] acl: token replication - deletions: %d, updates: %d", len(res.LocalDeletes), len(res.LocalUpserts))
+		s.logger.Printf(
+			"[DEBUG] acl: %s replication - deletions: %d, updates: %d",
+			tr.SingularNoun(),
+			len(res.LocalDeletes),
+			len(res.LocalUpserts),
+		)
 	}
 
-	var tokens *structs.ACLTokenBatchResponse
 	if len(res.LocalUpserts) > 0 {
-		tokens, err = s.fetchACLTokensBatch(res.LocalUpserts)
-		if err != nil {
-			return 0, false, fmt.Errorf("failed to retrieve ACL token updates: %v", err)
-		} else if tokens.Redacted {
-			return 0, false, fmt.Errorf("failed to retrieve unredacted tokens - replication token in use does not grant acl:write")
+		lenUpdated, err := tr.FetchUpdated(s, res.LocalUpserts)
+		if err == errContainsRedactedData {
+			return 0, false, fmt.Errorf("failed to retrieve unredacted %s - replication token in use does not grant acl:write", tr.PluralNoun())
+		} else if err != nil {
+			return 0, false, fmt.Errorf("failed to retrieve ACL %s updates: %v", tr.SingularNoun(), err)
 		}
-
-		s.logger.Printf("[DEBUG] acl: token replication - downloaded %d tokens", len(tokens.Tokens))
+		s.logger.Printf(
+			"[DEBUG] acl: %s replication - downloaded %d %s",
+			tr.SingularNoun(),
+			lenUpdated,
+			tr.PluralNoun(),
+		)
 	}
 
 	if len(res.LocalDeletes) > 0 {
-		s.logger.Printf("[DEBUG] acl: token replication - performing deletions")
+		s.logger.Printf(
+			"[DEBUG] acl: %s replication - performing deletions",
+			tr.SingularNoun(),
+		)
 
-		exit, err := s.deleteLocalACLTokens(res.LocalDeletes, ctx)
+		exit, err := s.deleteLocalACLType(ctx, tr, res.LocalDeletes)
 		if exit {
 			return 0, true, nil
 		}
 		if err != nil {
-			return 0, false, fmt.Errorf("failed to delete local ACL tokens: %v", err)
+			return 0, false, fmt.Errorf("failed to delete local ACL %s: %v", tr.PluralNoun(), err)
 		}
-		s.logger.Printf("[DEBUG] acl: token replication - finished deletions")
+		s.logger.Printf("[DEBUG] acl: %s replication - finished deletions", tr.SingularNoun())
 	}
 
 	if len(res.LocalUpserts) > 0 {
-		s.logger.Printf("[DEBUG] acl: token replication - performing updates")
-		exit, err := s.updateLocalACLTokens(tokens.Tokens, ctx)
+		s.logger.Printf("[DEBUG] acl: %s replication - performing updates", tr.SingularNoun())
+		exit, err := s.updateLocalACLType(ctx, tr)
 		if exit {
 			return 0, true, nil
 		}
 		if err != nil {
-			return 0, false, fmt.Errorf("failed to update local ACL tokens: %v", err)
+			return 0, false, fmt.Errorf("failed to update local ACL %s: %v", tr.PluralNoun(), err)
 		}
-		s.logger.Printf("[DEBUG] acl: token replication - finished updates")
+		s.logger.Printf("[DEBUG] acl: %s replication - finished updates", tr.SingularNoun())
 	}
 
 	// Return the index we got back from the remote side, since we've synced
 	// up with the remote state as of that index.
-	return remote.QueryMeta.Index, false, nil
+	return remoteIndex, false, nil
 }
 
 // IsACLReplicationEnabled returns true if ACL replication is enabled.
@@ -547,20 +496,23 @@ func (s *Server) updateACLReplicationStatusError() {
 	s.aclReplicationStatus.LastError = time.Now().Round(time.Second).UTC()
 }
 
-func (s *Server) updateACLReplicationStatusIndex(index uint64) {
+func (s *Server) updateACLReplicationStatusIndex(replicationType structs.ACLReplicationType, index uint64) {
 	s.aclReplicationStatusLock.Lock()
 	defer s.aclReplicationStatusLock.Unlock()
 
 	s.aclReplicationStatus.LastSuccess = time.Now().Round(time.Second).UTC()
-	s.aclReplicationStatus.ReplicatedIndex = index
-}
-
-func (s *Server) updateACLReplicationStatusTokenIndex(index uint64) {
-	s.aclReplicationStatusLock.Lock()
-	defer s.aclReplicationStatusLock.Unlock()
-
-	s.aclReplicationStatus.LastSuccess = time.Now().Round(time.Second).UTC()
-	s.aclReplicationStatus.ReplicatedTokenIndex = index
+	switch replicationType {
+	case structs.ACLReplicateLegacy:
+		s.aclReplicationStatus.ReplicatedIndex = index
+	case structs.ACLReplicateTokens:
+		s.aclReplicationStatus.ReplicatedTokenIndex = index
+	case structs.ACLReplicatePolicies:
+		s.aclReplicationStatus.ReplicatedIndex = index
+	case structs.ACLReplicateRoles:
+		s.aclReplicationStatus.ReplicatedRoleIndex = index
+	default:
+		panic("unknown replication type: " + replicationType.SingularNoun())
+	}
 }
 
 func (s *Server) initReplicationStatus() {
@@ -583,6 +535,21 @@ func (s *Server) updateACLReplicationStatusRunning(replicationType structs.ACLRe
 	s.aclReplicationStatusLock.Lock()
 	defer s.aclReplicationStatusLock.Unlock()
 
+	// The running state represents which type of overall replication has been
+	// configured. Though there are various types of internal plumbing for acl
+	// replication, to the end user there are only 3 distinctly configurable
+	// variants: legacy, policy, token. Roles replicate with policies so we
+	// round that up here.
+	if replicationType == structs.ACLReplicateRoles {
+		replicationType = structs.ACLReplicatePolicies
+	}
+
 	s.aclReplicationStatus.Running = true
 	s.aclReplicationStatus.ReplicationType = replicationType
 }
+
+func (s *Server) getACLReplicationStatusRunningType() (structs.ACLReplicationType, bool) {
+	s.aclReplicationStatusLock.RLock()
+	defer s.aclReplicationStatusLock.RUnlock()
+	return s.aclReplicationStatus.ReplicationType, s.aclReplicationStatus.Running
+}

agent/consul/acl_replication_legacy.go

@@ -138,7 +138,7 @@ func reconcileLegacyACLs(local, remote structs.ACLs, lastRemoteIndex uint64) str
 
 // FetchLocalACLs returns the ACLs in the local state store.
 func (s *Server) fetchLocalLegacyACLs() (structs.ACLs, error) {
-	_, local, err := s.fsm.State().ACLTokenList(nil, false, true, "")
+	_, local, err := s.fsm.State().ACLTokenList(nil, false, true, "", "")
 	if err != nil {
 		return nil, err
 	}

agent/consul/acl_replication_legacy_test.go

@@ -335,6 +335,10 @@ func TestACLReplication_IsACLReplicationEnabled(t *testing.T) {
 	}
 }
 
+// Note that this test is testing that legacy token data is replicated, NOT
+// directly testing the legacy acl replication goroutine code.
+//
+// Actually testing legacy replication is difficult to do without old binaries.
 func TestACLReplication_LegacyTokens(t *testing.T) {
 	t.Parallel()
 	dir1, s1 := testServerWithConfig(t, func(c *Config) {
@@ -367,6 +371,12 @@ func TestACLReplication_LegacyTokens(t *testing.T) {
 	testrpc.WaitForLeader(t, s1.RPC, "dc1")
 	testrpc.WaitForLeader(t, s1.RPC, "dc2")
 
+	// Wait for legacy acls to be disabled so we are clear that
+	// legacy replication isn't meddling.
+	waitForNewACLs(t, s1)
+	waitForNewACLs(t, s2)
+	waitForNewACLReplication(t, s2, structs.ACLReplicateTokens)
+
 	// Create a bunch of new tokens.
 	var id string
 	for i := 0; i < 50; i++ {
@@ -386,14 +396,15 @@ func TestACLReplication_LegacyTokens(t *testing.T) {
 	}
 
 	checkSame := func() error {
-		index, remote, err := s1.fsm.State().ACLTokenList(nil, true, true, "")
+		index, remote, err := s1.fsm.State().ACLTokenList(nil, true, true, "", "")
 		if err != nil {
 			return err
 		}
-		_, local, err := s2.fsm.State().ACLTokenList(nil, true, true, "")
+		_, local, err := s2.fsm.State().ACLTokenList(nil, true, true, "", "")
 		if err != nil {
 			return err
 		}
+
 		if got, want := len(remote), len(local); got != want {
 			return fmt.Errorf("got %d remote ACLs want %d", got, want)
 		}

agent/consul/acl_replication_test.go

@@ -3,6 +3,7 @@ package consul
 import (
 	"fmt"
 	"os"
+	"strconv"
 	"testing"
 	"time"
 
@@ -15,6 +16,11 @@ import (
 )
 
 func TestACLReplication_diffACLPolicies(t *testing.T) {
+	diffACLPolicies := func(local structs.ACLPolicies, remote structs.ACLPolicyListStubs, lastRemoteIndex uint64) ([]string, []string) {
+		tr := &aclPolicyReplicator{local: local, remote: remote}
+		res := diffACLType(tr, lastRemoteIndex)
+		return res.LocalDeletes, res.LocalUpserts
+	}
 	local := structs.ACLPolicies{
 		&structs.ACLPolicy{
 			ID:          "44ef9aec-7654-4401-901b-4d4a8b3c80fc",
@@ -127,6 +133,15 @@ func TestACLReplication_diffACLPolicies(t *testing.T) {
 }
 
 func TestACLReplication_diffACLTokens(t *testing.T) {
+	diffACLTokens := func(
+		local structs.ACLTokens,
+		remote structs.ACLTokenListStubs,
+		lastRemoteIndex uint64,
+	) itemDiffResults {
+		tr := &aclTokenReplicator{local: local, remote: remote}
+		return diffACLType(tr, lastRemoteIndex)
+	}
+
 	local := structs.ACLTokens{
 		// When a just-upgraded (1.3->1.4+) secondary DC is replicating from an
 		// upgraded primary DC (1.4+), the local state for tokens predating the
@@ -307,6 +322,12 @@ func TestACLReplication_Tokens(t *testing.T) {
 	testrpc.WaitForLeader(t, s1.RPC, "dc1")
 	testrpc.WaitForLeader(t, s1.RPC, "dc2")
 
+	// Wait for legacy acls to be disabled so we are clear that
+	// legacy replication isn't meddling.
+	waitForNewACLs(t, s1)
+	waitForNewACLs(t, s2)
+	waitForNewACLReplication(t, s2, structs.ACLReplicateTokens)
+
 	// Create a bunch of new tokens and policies
 	var tokens structs.ACLTokens
 	for i := 0; i < 50; i++ {
@@ -328,11 +349,11 @@ func TestACLReplication_Tokens(t *testing.T) {
 		tokens = append(tokens, &token)
 	}
 
-	checkSame := func(t *retry.R) error {
+	checkSame := func(t *retry.R) {
 		// only account for global tokens - local tokens shouldn't be replicated
-		index, remote, err := s1.fsm.State().ACLTokenList(nil, false, true, "")
+		index, remote, err := s1.fsm.State().ACLTokenList(nil, false, true, "", "")
 		require.NoError(t, err)
-		_, local, err := s2.fsm.State().ACLTokenList(nil, false, true, "")
+		_, local, err := s2.fsm.State().ACLTokenList(nil, false, true, "", "")
 		require.NoError(t, err)
 
 		require.Len(t, local, len(remote))
@@ -340,18 +361,15 @@ func TestACLReplication_Tokens(t *testing.T) {
 			require.Equal(t, token.Hash, local[i].Hash)
 		}
 
-		var status structs.ACLReplicationStatus
 		s2.aclReplicationStatusLock.RLock()
-		status = s2.aclReplicationStatus
+		status := s2.aclReplicationStatus
 		s2.aclReplicationStatusLock.RUnlock()
-		if !status.Enabled || !status.Running ||
-			status.ReplicationType != structs.ACLReplicateTokens ||
-			status.ReplicatedTokenIndex != index ||
-			status.SourceDatacenter != "dc1" {
-			return fmt.Errorf("ACL replication status differs")
-		}
 
-		return nil
+		require.True(t, status.Enabled)
+		require.True(t, status.Running)
+		require.Equal(t, status.ReplicationType, structs.ACLReplicateTokens)
+		require.Equal(t, status.ReplicatedTokenIndex, index)
+		require.Equal(t, status.SourceDatacenter, "dc1")
 	}
 	// Wait for the replica to converge.
 	retry.Run(t, func(r *retry.R) {
@@ -426,7 +444,7 @@ func TestACLReplication_Tokens(t *testing.T) {
 	})
 
 	// verify dc2 local tokens didn't get blown away
-	_, local, err := s2.fsm.State().ACLTokenList(nil, true, false, "")
+	_, local, err := s2.fsm.State().ACLTokenList(nil, true, false, "", "")
 	require.NoError(t, err)
 	require.Len(t, local, 50)
 
@@ -479,6 +497,12 @@ func TestACLReplication_Policies(t *testing.T) {
 	testrpc.WaitForLeader(t, s1.RPC, "dc1")
 	testrpc.WaitForLeader(t, s1.RPC, "dc2")
 
+	// Wait for legacy acls to be disabled so we are clear that
+	// legacy replication isn't meddling.
+	waitForNewACLs(t, s1)
+	waitForNewACLs(t, s2)
+	waitForNewACLReplication(t, s2, structs.ACLReplicatePolicies)
+
 	// Create a bunch of new policies
 	var policies structs.ACLPolicies
 	for i := 0; i < 50; i++ {
@@ -496,7 +520,7 @@ func TestACLReplication_Policies(t *testing.T) {
 		policies = append(policies, &policy)
 	}
 
-	checkSame := func(t *retry.R) error {
+	checkSame := func(t *retry.R) {
 		// only account for global tokens - local tokens shouldn't be replicated
 		index, remote, err := s1.fsm.State().ACLPolicyList(nil)
 		require.NoError(t, err)
@@ -508,18 +532,15 @@ func TestACLReplication_Policies(t *testing.T) {
 			require.Equal(t, policy.Hash, local[i].Hash)
 		}
 
-		var status structs.ACLReplicationStatus
 		s2.aclReplicationStatusLock.RLock()
-		status = s2.aclReplicationStatus
+		status := s2.aclReplicationStatus
 		s2.aclReplicationStatusLock.RUnlock()
-		if !status.Enabled || !status.Running ||
-			status.ReplicationType != structs.ACLReplicatePolicies ||
-			status.ReplicatedIndex != index ||
-			status.SourceDatacenter != "dc1" {
-			return fmt.Errorf("ACL replication status differs")
-		}
 
-		return nil
+		require.True(t, status.Enabled)
+		require.True(t, status.Running)
+		require.Equal(t, status.ReplicationType, structs.ACLReplicatePolicies)
+		require.Equal(t, status.ReplicatedIndex, index)
+		require.Equal(t, status.SourceDatacenter, "dc1")
 	}
 	// Wait for the replica to converge.
 	retry.Run(t, func(r *retry.R) {
@@ -709,3 +730,249 @@ func TestACLReplication_TokensRedacted(t *testing.T) {
 		require.True(r, status.LastError.After(minErrorTime), "Replication LastError not after the minErrorTime")
 	})
 }
+
+func TestACLReplication_AllTypes(t *testing.T) {
+	t.Parallel()
+	dir1, s1 := testServerWithConfig(t, func(c *Config) {
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLMasterToken = "root"
+	})
+	defer os.RemoveAll(dir1)
+	defer s1.Shutdown()
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+	client := rpcClient(t, s1)
+	defer client.Close()
+
+	dir2, s2 := testServerWithConfig(t, func(c *Config) {
+		c.Datacenter = "dc2"
+		c.ACLDatacenter = "dc1"
+		c.ACLsEnabled = true
+		c.ACLTokenReplication = true
+		c.ACLReplicationRate = 100
+		c.ACLReplicationBurst = 100
+		c.ACLReplicationApplyLimit = 1000000
+	})
+	s2.tokens.UpdateReplicationToken("root", tokenStore.TokenSourceConfig)
+	testrpc.WaitForLeader(t, s2.RPC, "dc2")
+	defer os.RemoveAll(dir2)
+	defer s2.Shutdown()
+
+	// Try to join.
+	joinWAN(t, s2, s1)
+	testrpc.WaitForLeader(t, s1.RPC, "dc1")
+	testrpc.WaitForLeader(t, s1.RPC, "dc2")
+
+	// Wait for legacy acls to be disabled so we are clear that
+	// legacy replication isn't meddling.
+	waitForNewACLs(t, s1)
+	waitForNewACLs(t, s2)
+	waitForNewACLReplication(t, s2, structs.ACLReplicateTokens)
+
+	const (
+		numItems             = 50
+		numItemsThatAreLocal = 10
+	)
+
+	// Create some data.
+	policyIDs, roleIDs, tokenIDs := createACLTestData(t, s1, "b1", numItems, numItemsThatAreLocal)
+
+	checkSameTokens := func(t *retry.R) {
+		// only account for global tokens - local tokens shouldn't be replicated
+		index, remote, err := s1.fsm.State().ACLTokenList(nil, false, true, "", "")
+		require.NoError(t, err)
+		// Query for all of them, so that we can prove that no globals snuck in.
+		_, local, err := s2.fsm.State().ACLTokenList(nil, true, true, "", "")
+		require.NoError(t, err)
+
+		require.Len(t, remote, len(local))
+		for i, token := range remote {
+			require.Equal(t, token.Hash, local[i].Hash)
+		}
+
+		s2.aclReplicationStatusLock.RLock()
+		status := s2.aclReplicationStatus
+		s2.aclReplicationStatusLock.RUnlock()
+
+		require.True(t, status.Enabled)
+		require.True(t, status.Running)
+		require.Equal(t, status.ReplicationType, structs.ACLReplicateTokens)
+		require.Equal(t, status.ReplicatedTokenIndex, index)
+		require.Equal(t, status.SourceDatacenter, "dc1")
+	}
+	checkSamePolicies := func(t *retry.R) {
+		index, remote, err := s1.fsm.State().ACLPolicyList(nil)
+		require.NoError(t, err)
+		_, local, err := s2.fsm.State().ACLPolicyList(nil)
+		require.NoError(t, err)
+
+		require.Len(t, remote, len(local))
+		for i, policy := range remote {
+			require.Equal(t, policy.Hash, local[i].Hash)
+		}
+
+		s2.aclReplicationStatusLock.RLock()
+		status := s2.aclReplicationStatus
+		s2.aclReplicationStatusLock.RUnlock()
+
+		require.True(t, status.Enabled)
+		require.True(t, status.Running)
+		require.Equal(t, status.ReplicationType, structs.ACLReplicateTokens)
+		require.Equal(t, status.ReplicatedIndex, index)
+		require.Equal(t, status.SourceDatacenter, "dc1")
+	}
+	checkSameRoles := func(t *retry.R) {
+		index, remote, err := s1.fsm.State().ACLRoleList(nil, "")
+		require.NoError(t, err)
+		_, local, err := s2.fsm.State().ACLRoleList(nil, "")
+		require.NoError(t, err)
+
+		require.Len(t, remote, len(local))
+		for i, role := range remote {
+			require.Equal(t, role.Hash, local[i].Hash)
+		}
+
+		s2.aclReplicationStatusLock.RLock()
+		status := s2.aclReplicationStatus
+		s2.aclReplicationStatusLock.RUnlock()
+
+		require.True(t, status.Enabled)
+		require.True(t, status.Running)
+		require.Equal(t, status.ReplicationType, structs.ACLReplicateTokens)
+		require.Equal(t, status.ReplicatedRoleIndex, index)
+		require.Equal(t, status.SourceDatacenter, "dc1")
+	}
+	checkSame := func(t *retry.R) {
+		checkSameTokens(t)
+		checkSamePolicies(t)
+		checkSameRoles(t)
+	}
+	// Wait for the replica to converge.
+	retry.Run(t, func(r *retry.R) {
+		checkSame(r)
+	})
+
+	// Create additional data to replicate.
+	_, _, _ = createACLTestData(t, s1, "b2", numItems, numItemsThatAreLocal)
+
+	// Wait for the replica to converge.
+	retry.Run(t, func(r *retry.R) {
+		checkSame(r)
+	})
+
+	// Delete one piece of each type of data from batch 1.
+	const itemToDelete = numItems - 1
+	{
+		id := tokenIDs[itemToDelete]
+
+		arg := structs.ACLTokenDeleteRequest{
+			Datacenter:   "dc1",
+			TokenID:      id,
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var dontCare string
+		if err := s1.RPC("ACL.TokenDelete", &arg, &dontCare); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	}
+	{
+		id := roleIDs[itemToDelete]
+
+		arg := structs.ACLRoleDeleteRequest{
+			Datacenter:   "dc1",
+			RoleID:       id,
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var dontCare string
+		if err := s1.RPC("ACL.RoleDelete", &arg, &dontCare); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	}
+	{
+		id := policyIDs[itemToDelete]
+
+		arg := structs.ACLPolicyDeleteRequest{
+			Datacenter:   "dc1",
+			PolicyID:     id,
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var dontCare string
+		if err := s1.RPC("ACL.PolicyDelete", &arg, &dontCare); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+	}
+	// Wait for the replica to converge.
+	retry.Run(t, func(r *retry.R) {
+		checkSame(r)
+	})
+}
+
+func createACLTestData(t *testing.T, srv *Server, namePrefix string, numObjects, numItemsThatAreLocal int) (policyIDs, roleIDs, tokenIDs []string) {
+	require.True(t, numItemsThatAreLocal <= numObjects, 0, "numItemsThatAreLocal <= numObjects")
+
+	// Create some policies.
+	for i := 0; i < numObjects; i++ {
+		str := strconv.Itoa(i)
+		arg := structs.ACLPolicySetRequest{
+			Datacenter: "dc1",
+			Policy: structs.ACLPolicy{
+				Name:        namePrefix + "-policy-" + str,
+				Description: namePrefix + "-policy " + str,
+				Rules:       testACLPolicyNew,
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var out structs.ACLPolicy
+		if err := srv.RPC("ACL.PolicySet", &arg, &out); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		policyIDs = append(policyIDs, out.ID)
+	}
+
+	// Create some roles.
+	for i := 0; i < numObjects; i++ {
+		str := strconv.Itoa(i)
+		arg := structs.ACLRoleSetRequest{
+			Datacenter: "dc1",
+			Role: structs.ACLRole{
+				Name:        namePrefix + "-role-" + str,
+				Description: namePrefix + "-role " + str,
+				Policies: []structs.ACLRolePolicyLink{
+					{ID: policyIDs[i]},
+				},
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var out structs.ACLRole
+		if err := srv.RPC("ACL.RoleSet", &arg, &out); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		roleIDs = append(roleIDs, out.ID)
+	}
+
+	// Create a bunch of new tokens.
+	for i := 0; i < numObjects; i++ {
+		str := strconv.Itoa(i)
+		arg := structs.ACLTokenSetRequest{
+			Datacenter: "dc1",
+			ACLToken: structs.ACLToken{
+				Description: namePrefix + "-token " + str,
+				Policies: []structs.ACLTokenPolicyLink{
+					{ID: policyIDs[i]},
+				},
+				Roles: []structs.ACLTokenRoleLink{
+					{ID: roleIDs[i]},
+				},
+				Local: (i < numItemsThatAreLocal),
+			},
+			WriteRequest: structs.WriteRequest{Token: "root"},
+		}
+		var out structs.ACLToken
+		if err := srv.RPC("ACL.TokenSet", &arg, &out); err != nil {
+			t.Fatalf("err: %v", err)
+		}
+		tokenIDs = append(tokenIDs, out.AccessorID)
+	}
+
+	return policyIDs, roleIDs, tokenIDs
+}

agent/consul/acl_replication_types.go

@@ -0,0 +1,370 @@
+package consul
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/hashicorp/consul/agent/structs"
+)
+
+type aclTokenReplicator struct {
+	local   structs.ACLTokens
+	remote  structs.ACLTokenListStubs
+	updated []*structs.ACLToken
+}
+
+var _ aclTypeReplicator = (*aclTokenReplicator)(nil)
+
+func (r *aclTokenReplicator) Type() structs.ACLReplicationType { return structs.ACLReplicateTokens }
+func (r *aclTokenReplicator) SingularNoun() string             { return "token" }
+func (r *aclTokenReplicator) PluralNoun() string               { return "tokens" }
+
+func (r *aclTokenReplicator) FetchRemote(srv *Server, lastRemoteIndex uint64) (int, uint64, error) {
+	r.remote = nil
+
+	remote, err := srv.fetchACLTokens(lastRemoteIndex)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	r.remote = remote.Tokens
+	return len(remote.Tokens), remote.QueryMeta.Index, nil
+}
+
+func (r *aclTokenReplicator) FetchLocal(srv *Server) (int, uint64, error) {
+	r.local = nil
+
+	idx, local, err := srv.fsm.State().ACLTokenList(nil, false, true, "", "")
+	if err != nil {
+		return 0, 0, err
+	}
+
+	// Do not filter by expiration times. Wait until the tokens are explicitly
+	// deleted.
+
+	r.local = local
+	return len(local), idx, nil
+}
+
+func (r *aclTokenReplicator) SortState() (int, int) {
+	r.local.Sort()
+	r.remote.Sort()
+
+	return len(r.local), len(r.remote)
+}
+func (r *aclTokenReplicator) LocalMeta(i int) (id string, modIndex uint64, hash []byte) {
+	v := r.local[i]
+	return v.AccessorID, v.ModifyIndex, v.Hash
+}
+func (r *aclTokenReplicator) RemoteMeta(i int) (id string, modIndex uint64, hash []byte) {
+	v := r.remote[i]
+	return v.AccessorID, v.ModifyIndex, v.Hash
+}
+
+func (r *aclTokenReplicator) FetchUpdated(srv *Server, updates []string) (int, error) {
+	r.updated = nil
+
+	if len(updates) > 0 {
+		tokens, err := srv.fetchACLTokensBatch(updates)
+		if err != nil {
+			return 0, err
+		} else if tokens.Redacted {
+			return 0, errContainsRedactedData
+		}
+
+		// Do not filter by expiration times. Wait until the tokens are
+		// explicitly deleted.
+
+		r.updated = tokens.Tokens
+	}
+
+	return len(r.updated), nil
+}
+
+func (r *aclTokenReplicator) DeleteLocalBatch(srv *Server, batch []string) error {
+	req := structs.ACLTokenBatchDeleteRequest{
+		TokenIDs: batch,
+	}
+
+	resp, err := srv.raftApply(structs.ACLTokenDeleteRequestType, &req)
+	if err != nil {
+		return err
+	}
+	if respErr, ok := resp.(error); ok && err != nil {
+		return respErr
+	}
+	return nil
+}
+
+func (r *aclTokenReplicator) LenPendingUpdates() int {
+	return len(r.updated)
+}
+
+func (r *aclTokenReplicator) PendingUpdateEstimatedSize(i int) int {
+	return r.updated[i].EstimateSize()
+}
+
+func (r *aclTokenReplicator) PendingUpdateIsRedacted(i int) bool {
+	return r.updated[i].SecretID == redactedToken
+}
+
+func (r *aclTokenReplicator) UpdateLocalBatch(ctx context.Context, srv *Server, start, end int) error {
+	req := structs.ACLTokenBatchSetRequest{
+		Tokens: r.updated[start:end],
+		CAS:    false,
+	}
+
+	resp, err := srv.raftApply(structs.ACLTokenSetRequestType, &req)
+	if err != nil {
+		return err
+	}
+	if respErr, ok := resp.(error); ok && err != nil {
+		return respErr
+	}
+
+	return nil
+}
+
+///////////////////////
+
+type aclPolicyReplicator struct {
+	local   structs.ACLPolicies
+	remote  structs.ACLPolicyListStubs
+	updated []*structs.ACLPolicy
+}
+
+var _ aclTypeReplicator = (*aclPolicyReplicator)(nil)
+
+func (r *aclPolicyReplicator) Type() structs.ACLReplicationType { return structs.ACLReplicatePolicies }
+func (r *aclPolicyReplicator) SingularNoun() string             { return "policy" }
+func (r *aclPolicyReplicator) PluralNoun() string               { return "policies" }
+
+func (r *aclPolicyReplicator) FetchRemote(srv *Server, lastRemoteIndex uint64) (int, uint64, error) {
+	r.remote = nil
+
+	remote, err := srv.fetchACLPolicies(lastRemoteIndex)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	r.remote = remote.Policies
+	return len(remote.Policies), remote.QueryMeta.Index, nil
+}
+
+func (r *aclPolicyReplicator) FetchLocal(srv *Server) (int, uint64, error) {
+	r.local = nil
+
+	idx, local, err := srv.fsm.State().ACLPolicyList(nil)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	r.local = local
+	return len(local), idx, nil
+}
+
+func (r *aclPolicyReplicator) SortState() (int, int) {
+	r.local.Sort()
+	r.remote.Sort()
+
+	return len(r.local), len(r.remote)
+}
+func (r *aclPolicyReplicator) LocalMeta(i int) (id string, modIndex uint64, hash []byte) {
+	v := r.local[i]
+	return v.ID, v.ModifyIndex, v.Hash
+}
+func (r *aclPolicyReplicator) RemoteMeta(i int) (id string, modIndex uint64, hash []byte) {
+	v := r.remote[i]
+	return v.ID, v.ModifyIndex, v.Hash
+}
+
+func (r *aclPolicyReplicator) FetchUpdated(srv *Server, updates []string) (int, error) {
+	r.updated = nil
+
+	if len(updates) > 0 {
+		policies, err := srv.fetchACLPoliciesBatch(updates)
+		if err != nil {
+			return 0, err
+		}
+		r.updated = policies.Policies
+	}
+
+	return len(r.updated), nil
+}
+
+func (r *aclPolicyReplicator) DeleteLocalBatch(srv *Server, batch []string) error {
+	req := structs.ACLPolicyBatchDeleteRequest{
+		PolicyIDs: batch,
+	}
+
+	resp, err := srv.raftApply(structs.ACLPolicyDeleteRequestType, &req)
+	if err != nil {
+		return err
+	}
+	if respErr, ok := resp.(error); ok && err != nil {
+		return respErr
+	}
+	return nil
+}
+
+func (r *aclPolicyReplicator) LenPendingUpdates() int {
+	return len(r.updated)
+}
+
+func (r *aclPolicyReplicator) PendingUpdateEstimatedSize(i int) int {
+	return r.updated[i].EstimateSize()
+}
+
+func (r *aclPolicyReplicator) PendingUpdateIsRedacted(i int) bool {
+	return false
+}
+
+func (r *aclPolicyReplicator) UpdateLocalBatch(ctx context.Context, srv *Server, start, end int) error {
+	req := structs.ACLPolicyBatchSetRequest{
+		Policies: r.updated[start:end],
+	}
+
+	resp, err := srv.raftApply(structs.ACLPolicySetRequestType, &req)
+	if err != nil {
+		return err
+	}
+	if respErr, ok := resp.(error); ok && err != nil {
+		return respErr
+	}
+
+	return nil
+}
+
+////////////////////////////////
+
+type aclRoleReplicator struct {
+	local   structs.ACLRoles
+	remote  structs.ACLRoles
+	updated []*structs.ACLRole
+}
+
+var _ aclTypeReplicator = (*aclRoleReplicator)(nil)
+
+func (r *aclRoleReplicator) Type() structs.ACLReplicationType { return structs.ACLReplicateRoles }
+func (r *aclRoleReplicator) SingularNoun() string             { return "role" }
+func (r *aclRoleReplicator) PluralNoun() string               { return "roles" }
+
+func (r *aclRoleReplicator) FetchRemote(srv *Server, lastRemoteIndex uint64) (int, uint64, error) {
+	r.remote = nil
+
+	remote, err := srv.fetchACLRoles(lastRemoteIndex)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	r.remote = remote.Roles
+	return len(remote.Roles), remote.QueryMeta.Index, nil
+}
+
+func (r *aclRoleReplicator) FetchLocal(srv *Server) (int, uint64, error) {
+	r.local = nil
+
+	idx, local, err := srv.fsm.State().ACLRoleList(nil, "")
+	if err != nil {
+		return 0, 0, err
+	}
+
+	r.local = local
+	return len(local), idx, nil
+}
+
+func (r *aclRoleReplicator) SortState() (int, int) {
+	r.local.Sort()
+	r.remote.Sort()
+
+	return len(r.local), len(r.remote)
+}
+func (r *aclRoleReplicator) LocalMeta(i int) (id string, modIndex uint64, hash []byte) {
+	v := r.local[i]
+	return v.ID, v.ModifyIndex, v.Hash
+}
+func (r *aclRoleReplicator) RemoteMeta(i int) (id string, modIndex uint64, hash []byte) {
+	v := r.remote[i]
+	return v.ID, v.ModifyIndex, v.Hash
+}
+
+func (r *aclRoleReplicator) FetchUpdated(srv *Server, updates []string) (int, error) {
+	r.updated = nil
+
+	if len(updates) > 0 {
+		// Since ACLRoles do not have a "list entry" variation, all of the data
+		// to replicate a role is already present in the "r.remote" list.
+		//
+		// We avoid a second query by just repurposing the data we already have
+		// access to in a way that is compatible with the generic ACL type
+		// replicator.
+		keep := make(map[string]struct{})
+		for _, id := range updates {
+			keep[id] = struct{}{}
+		}
+
+		subset := make([]*structs.ACLRole, 0, len(updates))
+		for _, role := range r.remote {
+			if _, ok := keep[role.ID]; ok {
+				subset = append(subset, role)
+			}
+		}
+
+		if len(subset) != len(keep) { // only possible via programming bug
+			for _, role := range subset {
+				delete(keep, role.ID)
+			}
+			missing := make([]string, 0, len(keep))
+			for id, _ := range keep {
+				missing = append(missing, id)
+			}
+			return 0, fmt.Errorf("role replication trying to replicated uncached roles with IDs: %v", missing)
+		}
+		r.updated = subset
+	}
+
+	return len(r.updated), nil
+}
+
+func (r *aclRoleReplicator) DeleteLocalBatch(srv *Server, batch []string) error {
+	req := structs.ACLRoleBatchDeleteRequest{
+		RoleIDs: batch,
+	}
+
+	resp, err := srv.raftApply(structs.ACLRoleDeleteRequestType, &req)
+	if err != nil {
+		return err
+	}
+	if respErr, ok := resp.(error); ok && err != nil {
+		return respErr
+	}
+	return nil
+}
+
+func (r *aclRoleReplicator) LenPendingUpdates() int {
+	return len(r.updated)
+}
+
+func (r *aclRoleReplicator) PendingUpdateEstimatedSize(i int) int {
+	return r.updated[i].EstimateSize()
+}
+
+func (r *aclRoleReplicator) PendingUpdateIsRedacted(i int) bool {
+	return false
+}
+
+func (r *aclRoleReplicator) UpdateLocalBatch(ctx context.Context, srv *Server, start, end int) error {
+	req := structs.ACLRoleBatchSetRequest{
+		Roles: r.updated[start:end],
+	}
+
+	resp, err := srv.raftApply(structs.ACLRoleSetRequestType, &req)
+	if err != nil {
+		return err
+	}
+	if respErr, ok := resp.(error); ok && err != nil {
+		return respErr
+	}
+
+	return nil
+}

agent/consul/acl_server.go

@@ -13,7 +13,7 @@ var serverACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{
 	// The server's ACL caching has a few underlying assumptions:
 	//
 	// 1 - All policies can be resolved locally. Hence we do not cache any
-	//     unparsed policies as we have memdb for that.
+	//     unparsed policies/roles as we have memdb for that.
 	// 2 - While there could be many identities being used within a DC the
 	//     number of distinct policies and combined multi-policy authorizers
 	//     will be much less.
@@ -26,6 +26,7 @@ var serverACLCacheConfig *structs.ACLCachesConfig = &structs.ACLCachesConfig{
 	Policies:       0,
 	ParsedPolicies: 512,
 	Authorizers:    1024,
+	Roles:          0,
 }
 
 func (s *Server) checkTokenUUID(id string) (bool, error) {
@@ -61,6 +62,17 @@ func (s *Server) checkPolicyUUID(id string) (bool, error) {
 	return !structs.ACLIDReserved(id), nil
 }
 
+func (s *Server) checkRoleUUID(id string) (bool, error) {
+	state := s.fsm.State()
+	if _, role, err := state.ACLRoleGetByID(nil, id); err != nil {
+		return false, err
+	} else if role != nil {
+		return false, nil
+	}
+
+	return !structs.ACLIDReserved(id), nil
+}
+
 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
@@ -172,6 +184,20 @@ func (s *Server) ResolvePolicyFromID(policyID string) (bool, *structs.ACLPolicy,
 	return s.InACLDatacenter() || index > 0, policy, acl.ErrNotFound
 }
 
+func (s *Server) ResolveRoleFromID(roleID string) (bool, *structs.ACLRole, error) {
+	index, role, err := s.fsm.State().ACLRoleGetByID(nil, roleID)
+	if err != nil {
+		return true, nil, err
+	} else if role != nil {
+		return true, role, nil
+	}
+
+	// If the max index of the roles table is non-zero then we have acls, until then
+	// we may need to allow remote resolution. This is particularly useful to allow updating
+	// the replication token via the API in a non-primary dc.
+	return s.InACLDatacenter() || index > 0, role, acl.ErrNotFound
+}
+
 func (s *Server) ResolveToken(token string) (acl.Authorizer, error) {
 	return s.acls.ResolveToken(token)
 }

agent/consul/config.go

@@ -243,6 +243,11 @@ type Config struct {
 	// a substantial cost.
 	ACLPolicyTTL time.Duration
 
+	// ACLRoleTTL controls the time-to-live of cached ACL roles.
+	// It can be set to zero to disable caching, but this adds
+	// a substantial cost.
+	ACLRoleTTL time.Duration
+
 	// ACLDisabledTTL is the time between checking if ACLs should be
 	// enabled. This
 	ACLDisabledTTL time.Duration
@@ -470,6 +475,7 @@ func DefaultConfig() *Config {
 		SerfFloodInterval:           60 * time.Second,
 		ReconcileInterval:           60 * time.Second,
 		ProtocolVersion:             ProtocolVersion2Compatible,
+		ACLRoleTTL:                  30 * time.Second,
 		ACLPolicyTTL:                30 * time.Second,
 		ACLTokenTTL:                 30 * time.Second,
 		ACLDefaultPolicy:            "allow",

agent/consul/fsm/commands_oss.go

@@ -30,6 +30,8 @@ func init() {
 	registerCommand(structs.ACLPolicyDeleteRequestType, (*FSM).applyACLPolicyDeleteOperation)
 	registerCommand(structs.ConnectCALeafRequestType, (*FSM).applyConnectCALeafOperation)
 	registerCommand(structs.ConfigEntryRequestType, (*FSM).applyConfigEntryOperation)
+	registerCommand(structs.ACLRoleSetRequestType, (*FSM).applyACLRoleSetOperation)
+	registerCommand(structs.ACLRoleDeleteRequestType, (*FSM).applyACLRoleDeleteOperation)
 }
 
 func (c *FSM) applyRegister(buf []byte, index uint64) interface{} {
@@ -452,3 +454,25 @@ func (c *FSM) applyConfigEntryOperation(buf []byte, index uint64) interface{} {
 		return fmt.Errorf("invalid config entry operation type: %v", req.Op)
 	}
 }
+
+func (c *FSM) applyACLRoleSetOperation(buf []byte, index uint64) interface{} {
+	var req structs.ACLRoleBatchSetRequest
+	if err := structs.Decode(buf, &req); err != nil {
+		panic(fmt.Errorf("failed to decode request: %v", err))
+	}
+	defer metrics.MeasureSinceWithLabels([]string{"fsm", "acl", "role"}, time.Now(),
+		[]metrics.Label{{Name: "op", Value: "upsert"}})
+
+	return c.state.ACLRoleBatchSet(index, req.Roles)
+}
+
+func (c *FSM) applyACLRoleDeleteOperation(buf []byte, index uint64) interface{} {
+	var req structs.ACLRoleBatchDeleteRequest
+	if err := structs.Decode(buf, &req); err != nil {
+		panic(fmt.Errorf("failed to decode request: %v", err))
+	}
+	defer metrics.MeasureSinceWithLabels([]string{"fsm", "acl", "role"}, time.Now(),
+		[]metrics.Label{{Name: "op", Value: "delete"}})
+
+	return c.state.ACLRoleBatchDelete(index, req.RoleIDs)
+}

agent/consul/fsm/snapshot_oss.go

@@ -28,6 +28,7 @@ func init() {
 	registerRestorer(structs.ACLTokenSetRequestType, restoreToken)
 	registerRestorer(structs.ACLPolicySetRequestType, restorePolicy)
 	registerRestorer(structs.ConfigEntryRequestType, restoreConfigEntry)
+	registerRestorer(structs.ACLRoleSetRequestType, restoreRole)
 }
 
 func persistOSS(s *snapshot, sink raft.SnapshotSink, encoder *codec.Encoder) error {
@@ -203,6 +204,20 @@ func (s *snapshot) persistACLs(sink raft.SnapshotSink,
 		}
 	}
 
+	roles, err := s.state.ACLRoles()
+	if err != nil {
+		return err
+	}
+
+	for role := roles.Next(); role != nil; role = roles.Next() {
+		if _, err := sink.Write([]byte{byte(structs.ACLRoleSetRequestType)}); err != nil {
+			return err
+		}
+		if err := encoder.Encode(role.(*structs.ACLRole)); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
@@ -603,3 +618,11 @@ func restoreConfigEntry(header *snapshotHeader, restore *state.Restore, decoder
 	}
 	return restore.ConfigEntry(req.Entry)
 }
+
+func restoreRole(header *snapshotHeader, restore *state.Restore, decoder *codec.Decoder) error {
+	var req structs.ACLRole
+	if err := decoder.Decode(&req); err != nil {
+		return err
+	}
+	return restore.ACLRole(&req)
+}

agent/consul/helper_test.go

@@ -11,7 +11,7 @@ import (
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/types"
-	"github.com/hashicorp/net-rpc-msgpackrpc"
+	msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc"
 	"github.com/hashicorp/raft"
 	"github.com/hashicorp/serf/serf"
 	"github.com/stretchr/testify/require"
@@ -166,6 +166,21 @@ func waitForNewACLs(t *testing.T, server *Server) {
 	require.False(t, server.UseLegacyACLs(), "Server cannot use new ACLs")
 }
 
+func waitForNewACLReplication(t *testing.T, server *Server, expectedReplicationType structs.ACLReplicationType) {
+	var (
+		replTyp structs.ACLReplicationType
+		running bool
+	)
+	retry.Run(t, func(r *retry.R) {
+		replTyp, running = server.getACLReplicationStatusRunningType()
+		require.Equal(r, expectedReplicationType, replTyp, "Server not running new replicator yet")
+		require.True(r, running, "Server not running new replicator yet")
+	})
+
+	require.Equal(t, expectedReplicationType, replTyp, "Server not running new replicator yet")
+	require.True(t, running, "Server not running new replicator yet")
+}
+
 func seeEachOther(a, b []serf.Member, addra, addrb string) bool {
 	return serfMembersContains(a, addrb) && serfMembersContains(b, addra)
 }

agent/consul/leader.go

@@ -660,6 +660,7 @@ func (s *Server) startACLUpgrade() {
 				// Assign the global-management policy to legacy management tokens
 				if len(newToken.Policies) == 0 &&
 					len(newToken.ServiceIdentities) == 0 &&
+					len(newToken.Roles) == 0 &&
 					newToken.Type == structs.ACLTokenTypeManagement {
 					newToken.Policies = append(newToken.Policies, structs.ACLTokenPolicyLink{ID: structs.ACLPolicyGlobalManagementID})
 				}
@@ -738,7 +739,7 @@ func (s *Server) startLegacyACLReplication() {
 				s.logger.Printf("[WARN] consul: Legacy ACL replication error (will retry if still leader): %v", err)
 			} else {
 				lastRemoteIndex = index
-				s.updateACLReplicationStatusIndex(index)
+				s.updateACLReplicationStatusIndex(structs.ACLReplicateLegacy, index)
 				s.logger.Printf("[DEBUG] consul: Legacy ACL replication completed through remote index %d", index)
 			}
 		}
@@ -760,8 +761,22 @@ func (s *Server) startACLReplication() {
 	ctx, cancel := context.WithCancel(context.Background())
 	s.aclReplicationCancel = cancel
 
-	replicationType := structs.ACLReplicatePolicies
+	s.startACLReplicator(ctx, structs.ACLReplicatePolicies, s.replicateACLPolicies)
+	s.startACLReplicator(ctx, structs.ACLReplicateRoles, s.replicateACLRoles)
 
+	if s.config.ACLTokenReplication {
+		s.startACLReplicator(ctx, structs.ACLReplicateTokens, s.replicateACLTokens)
+		s.updateACLReplicationStatusRunning(structs.ACLReplicateTokens)
+	} else {
+		s.updateACLReplicationStatusRunning(structs.ACLReplicatePolicies)
+	}
+
+	s.aclReplicationEnabled = true
+}
+
+type replicateFunc func(ctx context.Context, lastRemoteIndex uint64) (uint64, bool, error)
+
+func (s *Server) startACLReplicator(ctx context.Context, replicationType structs.ACLReplicationType, replicateFunc replicateFunc) {
 	go func() {
 		var failedAttempts uint
 		limiter := rate.NewLimiter(rate.Limit(s.config.ACLReplicationRate), s.config.ACLReplicationBurst)
@@ -776,7 +791,7 @@ func (s *Server) startACLReplication() {
 				continue
 			}
 
-			index, exit, err := s.replicateACLPolicies(lastRemoteIndex, ctx)
+			index, exit, err := replicateFunc(ctx, lastRemoteIndex)
 			if exit {
 				return
 			}
@@ -784,7 +799,7 @@ func (s *Server) startACLReplication() {
 			if err != nil {
 				lastRemoteIndex = 0
 				s.updateACLReplicationStatusError()
-				s.logger.Printf("[WARN] consul: ACL policy replication error (will retry if still leader): %v", err)
+				s.logger.Printf("[WARN] consul: ACL %s replication error (will retry if still leader): %v", replicationType.SingularNoun(), err)
 				if (1 << failedAttempts) < aclReplicationMaxRetryBackoff {
 					failedAttempts++
 				}
@@ -797,65 +812,14 @@ func (s *Server) startACLReplication() {
 				}
 			} else {
 				lastRemoteIndex = index
-				s.updateACLReplicationStatusIndex(index)
-				s.logger.Printf("[DEBUG] consul: ACL policy replication completed through remote index %d", index)
+				s.updateACLReplicationStatusIndex(replicationType, index)
+				s.logger.Printf("[DEBUG] consul: ACL %s replication completed through remote index %d", replicationType.SingularNoun(), index)
 				failedAttempts = 0
 			}
 		}
 	}()
 
-	s.logger.Printf("[INFO] acl: started ACL Policy replication")
-
-	if s.config.ACLTokenReplication {
-		replicationType = structs.ACLReplicateTokens
-		go func() {
-			var failedAttempts uint
-			limiter := rate.NewLimiter(rate.Limit(s.config.ACLReplicationRate), s.config.ACLReplicationBurst)
-
-			var lastRemoteIndex uint64
-			for {
-				if err := limiter.Wait(ctx); err != nil {
-					return
-				}
-
-				if s.tokens.ReplicationToken() == "" {
-					continue
-				}
-
-				index, exit, err := s.replicateACLTokens(lastRemoteIndex, ctx)
-				if exit {
-					return
-				}
-
-				if err != nil {
-					lastRemoteIndex = 0
-					s.updateACLReplicationStatusError()
-					s.logger.Printf("[WARN] consul: ACL token replication error (will retry if still leader): %v", err)
-					if (1 << failedAttempts) < aclReplicationMaxRetryBackoff {
-						failedAttempts++
-					}
-
-					select {
-					case <-ctx.Done():
-						return
-					case <-time.After((1 << failedAttempts) * time.Second):
-						// do nothing
-					}
-				} else {
-					lastRemoteIndex = index
-					s.updateACLReplicationStatusTokenIndex(index)
-					s.logger.Printf("[DEBUG] consul: ACL token replication completed through remote index %d", index)
-					failedAttempts = 0
-				}
-			}
-		}()
-
-		s.logger.Printf("[INFO] acl: started ACL Token replication")
-	}
-
-	s.updateACLReplicationStatusRunning(replicationType)
-
-	s.aclReplicationEnabled = true
+	s.logger.Printf("[INFO] acl: started ACL %s replication", replicationType.SingularNoun())
 }
 
 func (s *Server) stopACLReplication() {

agent/consul/state/acl.go

@@ -60,6 +60,108 @@ func (s *TokenPoliciesIndex) PrefixFromArgs(args ...interface{}) ([]byte, error)
 	return val, nil
 }
 
+type TokenRolesIndex struct {
+}
+
+func (s *TokenRolesIndex) FromObject(obj interface{}) (bool, [][]byte, error) {
+	token, ok := obj.(*structs.ACLToken)
+	if !ok {
+		return false, nil, fmt.Errorf("object is not an ACLToken")
+	}
+
+	links := token.Roles
+
+	numLinks := len(links)
+	if numLinks == 0 {
+		return false, nil, nil
+	}
+
+	vals := make([][]byte, 0, numLinks)
+	for _, link := range links {
+		vals = append(vals, []byte(link.ID+"\x00"))
+	}
+
+	return true, vals, nil
+}
+
+func (s *TokenRolesIndex) FromArgs(args ...interface{}) ([]byte, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("must provide only a single argument")
+	}
+	arg, ok := args[0].(string)
+	if !ok {
+		return nil, fmt.Errorf("argument must be a string: %#v", args[0])
+	}
+	// Add the null character as a terminator
+	arg += "\x00"
+	return []byte(arg), nil
+}
+
+func (s *TokenRolesIndex) PrefixFromArgs(args ...interface{}) ([]byte, error) {
+	val, err := s.FromArgs(args...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Strip the null terminator, the rest is a prefix
+	n := len(val)
+	if n > 0 {
+		return val[:n-1], nil
+	}
+	return val, nil
+}
+
+type RolePoliciesIndex struct {
+}
+
+func (s *RolePoliciesIndex) FromObject(obj interface{}) (bool, [][]byte, error) {
+	role, ok := obj.(*structs.ACLRole)
+	if !ok {
+		return false, nil, fmt.Errorf("object is not an ACLRole")
+	}
+
+	links := role.Policies
+
+	numLinks := len(links)
+	if numLinks == 0 {
+		return false, nil, nil
+	}
+
+	vals := make([][]byte, 0, numLinks)
+	for _, link := range links {
+		vals = append(vals, []byte(link.ID+"\x00"))
+	}
+
+	return true, vals, nil
+}
+
+func (s *RolePoliciesIndex) FromArgs(args ...interface{}) ([]byte, error) {
+	if len(args) != 1 {
+		return nil, fmt.Errorf("must provide only a single argument")
+	}
+	arg, ok := args[0].(string)
+	if !ok {
+		return nil, fmt.Errorf("argument must be a string: %#v", args[0])
+	}
+	// Add the null character as a terminator
+	arg += "\x00"
+	return []byte(arg), nil
+}
+
+func (s *RolePoliciesIndex) PrefixFromArgs(args ...interface{}) ([]byte, error) {
+	val, err := s.FromArgs(args...)
+	if err != nil {
+		return nil, err
+	}
+
+	// Strip the null terminator, the rest is a prefix
+	n := len(val)
+	if n > 0 {
+		return val[:n-1], nil
+	}
+	return val, nil
+}
+
 type TokenExpirationIndex struct {
 	LocalFilter bool
 }
@@ -137,6 +239,13 @@ func tokensTableSchema() *memdb.TableSchema {
 				Unique:       false,
 				Indexer:      &TokenPoliciesIndex{},
 			},
+			"roles": &memdb.IndexSchema{
+				Name: "roles",
+				// Need to allow missing for the anonymous token
+				AllowMissing: true,
+				Unique:       false,
+				Indexer:      &TokenRolesIndex{},
+			},
 			"local": &memdb.IndexSchema{
 				Name:         "local",
 				AllowMissing: false,
@@ -208,9 +317,42 @@ func policiesTableSchema() *memdb.TableSchema {
 	}
 }
 
+func rolesTableSchema() *memdb.TableSchema {
+	return &memdb.TableSchema{
+		Name: "acl-roles",
+		Indexes: map[string]*memdb.IndexSchema{
+			"id": &memdb.IndexSchema{
+				Name:         "id",
+				AllowMissing: false,
+				Unique:       true,
+				Indexer: &memdb.UUIDFieldIndex{
+					Field: "ID",
+				},
+			},
+			"name": &memdb.IndexSchema{
+				Name:         "name",
+				AllowMissing: false,
+				Unique:       true,
+				Indexer: &memdb.StringFieldIndex{
+					Field:     "Name",
+					Lowercase: true,
+				},
+			},
+			"policies": &memdb.IndexSchema{
+				Name: "policies",
+				// Need to allow missing for the anonymous token
+				AllowMissing: true,
+				Unique:       false,
+				Indexer:      &RolePoliciesIndex{},
+			},
+		},
+	}
+}
+
 func init() {
 	registerSchema(tokensTableSchema)
 	registerSchema(policiesTableSchema)
+	registerSchema(rolesTableSchema)
 }
 
 // ACLTokens is used when saving a snapshot
@@ -255,6 +397,26 @@ func (s *Restore) ACLPolicy(policy *structs.ACLPolicy) error {
 	return nil
 }
 
+// ACLRoles is used when saving a snapshot
+func (s *Snapshot) ACLRoles() (memdb.ResultIterator, error) {
+	iter, err := s.tx.Get("acl-roles", "id")
+	if err != nil {
+		return nil, err
+	}
+	return iter, nil
+}
+
+func (s *Restore) ACLRole(role *structs.ACLRole) error {
+	if err := s.tx.Insert("acl-roles", role); err != nil {
+		return fmt.Errorf("failed restoring acl role: %s", err)
+	}
+
+	if err := indexUpdateMaxTxn(s.tx, role.ModifyIndex, "acl-roles"); err != nil {
+		return fmt.Errorf("failed updating index: %s", err)
+	}
+	return nil
+}
+
 // ACLBootstrap is used to perform a one-time ACL bootstrap operation on a
 // cluster to get the first management token.
 func (s *Store) ACLBootstrap(idx, resetIndex uint64, token *structs.ACLToken, legacy bool) error {
@@ -369,6 +531,7 @@ func (s *Store) fixupTokenPolicyLinks(tx *memdb.Txn, original *structs.ACLToken)
 
 			// append the corrected policy
 			token.Policies = append(token.Policies, structs.ACLTokenPolicyLink{ID: link.ID, Name: policy.Name})
+
 		} else if owned {
 			token.Policies = append(token.Policies, link)
 		}
@@ -377,6 +540,150 @@ func (s *Store) fixupTokenPolicyLinks(tx *memdb.Txn, original *structs.ACLToken)
 	return token, nil
 }
 
+func (s *Store) resolveTokenRoleLinks(tx *memdb.Txn, token *structs.ACLToken, allowMissing bool) error {
+	for linkIndex, link := range token.Roles {
+		if link.ID != "" {
+			role, err := s.getRoleWithTxn(tx, nil, link.ID, "id")
+
+			if err != nil {
+				return err
+			}
+
+			if role != nil {
+				// the name doesn't matter here
+				token.Roles[linkIndex].Name = role.Name
+			} else if !allowMissing {
+				return fmt.Errorf("No such role with ID: %s", link.ID)
+			}
+		} else {
+			return fmt.Errorf("Encountered a Token with roles linked by Name in the state store")
+		}
+	}
+	return nil
+}
+
+// fixupTokenRoleLinks is to be used when retrieving tokens from memdb. The role links could have gotten
+// stale when a linked role was deleted or renamed. This will correct them and generate a newly allocated
+// token only when fixes are needed. If the role links are still accurate then we just return the original
+// token.
+func (s *Store) fixupTokenRoleLinks(tx *memdb.Txn, original *structs.ACLToken) (*structs.ACLToken, error) {
+	owned := false
+	token := original
+
+	cloneToken := func(t *structs.ACLToken, copyNumLinks int) *structs.ACLToken {
+		clone := *t
+		clone.Roles = make([]structs.ACLTokenRoleLink, copyNumLinks)
+		copy(clone.Roles, t.Roles[:copyNumLinks])
+		return &clone
+	}
+
+	for linkIndex, link := range original.Roles {
+		if link.ID == "" {
+			return nil, fmt.Errorf("Detected corrupted token within the state store - missing role link ID")
+		}
+
+		role, err := s.getRoleWithTxn(tx, nil, link.ID, "id")
+
+		if err != nil {
+			return nil, err
+		}
+
+		if role == nil {
+			if !owned {
+				// clone the token as we cannot touch the original
+				token = cloneToken(original, linkIndex)
+				owned = true
+			}
+			// if already owned then we just don't append it.
+		} else if role.Name != link.Name {
+			if !owned {
+				token = cloneToken(original, linkIndex)
+				owned = true
+			}
+
+			// append the corrected policy
+			token.Roles = append(token.Roles, structs.ACLTokenRoleLink{ID: link.ID, Name: role.Name})
+
+		} else if owned {
+			token.Roles = append(token.Roles, link)
+		}
+	}
+
+	return token, nil
+}
+
+func (s *Store) resolveRolePolicyLinks(tx *memdb.Txn, role *structs.ACLRole, allowMissing bool) error {
+	for linkIndex, link := range role.Policies {
+		if link.ID != "" {
+			policy, err := s.getPolicyWithTxn(tx, nil, link.ID, "id")
+
+			if err != nil {
+				return err
+			}
+
+			if policy != nil {
+				// the name doesn't matter here
+				role.Policies[linkIndex].Name = policy.Name
+			} else if !allowMissing {
+				return fmt.Errorf("No such policy with ID: %s", link.ID)
+			}
+		} else {
+			return fmt.Errorf("Encountered a Role with policies linked by Name in the state store")
+		}
+	}
+	return nil
+}
+
+// fixupRolePolicyLinks is to be used when retrieving roles from memdb. The policy links could have gotten
+// stale when a linked policy was deleted or renamed. This will correct them and generate a newly allocated
+// role only when fixes are needed. If the policy links are still accurate then we just return the original
+// role.
+func (s *Store) fixupRolePolicyLinks(tx *memdb.Txn, original *structs.ACLRole) (*structs.ACLRole, error) {
+	owned := false
+	role := original
+
+	cloneRole := func(t *structs.ACLRole, copyNumLinks int) *structs.ACLRole {
+		clone := *t
+		clone.Policies = make([]structs.ACLRolePolicyLink, copyNumLinks)
+		copy(clone.Policies, t.Policies[:copyNumLinks])
+		return &clone
+	}
+
+	for linkIndex, link := range original.Policies {
+		if link.ID == "" {
+			return nil, fmt.Errorf("Detected corrupted role within the state store - missing policy link ID")
+		}
+
+		policy, err := s.getPolicyWithTxn(tx, nil, link.ID, "id")
+
+		if err != nil {
+			return nil, err
+		}
+
+		if policy == nil {
+			if !owned {
+				// clone the token as we cannot touch the original
+				role = cloneRole(original, linkIndex)
+				owned = true
+			}
+			// if already owned then we just don't append it.
+		} else if policy.Name != link.Name {
+			if !owned {
+				role = cloneRole(original, linkIndex)
+				owned = true
+			}
+
+			// append the corrected policy
+			role.Policies = append(role.Policies, structs.ACLRolePolicyLink{ID: link.ID, Name: policy.Name})
+
+		} else if owned {
+			role.Policies = append(role.Policies, link)
+		}
+	}
+
+	return role, nil
+}
+
 // ACLTokenSet is used to insert an ACL rule into the state store.
 func (s *Store) ACLTokenSet(idx uint64, token *structs.ACLToken, legacy bool) error {
 	tx := s.db.Txn(true)
@@ -417,7 +724,7 @@ func (s *Store) ACLTokenBatchSet(idx uint64, tokens structs.ACLTokens, cas bool)
 
 // aclTokenSetTxn is the inner method used to insert an ACL token with the
 // proper indexes into the state store.
-func (s *Store) aclTokenSetTxn(tx *memdb.Txn, idx uint64, token *structs.ACLToken, cas, allowMissingPolicyIDs, legacy bool) error {
+func (s *Store) aclTokenSetTxn(tx *memdb.Txn, idx uint64, token *structs.ACLToken, cas, allowMissingPolicyAndRoleIDs, legacy bool) error {
 	// Check that the ID is set
 	if token.SecretID == "" {
 		return ErrMissingACLTokenSecret
@@ -474,7 +781,11 @@ func (s *Store) aclTokenSetTxn(tx *memdb.Txn, idx uint64, token *structs.ACLToke
 		token.AccessorID = original.AccessorID
 	}
 
-	if err := s.resolveTokenPolicyLinks(tx, token, allowMissingPolicyIDs); err != nil {
+	if err := s.resolveTokenPolicyLinks(tx, token, allowMissingPolicyAndRoleIDs); err != nil {
+		return err
+	}
+
+	if err := s.resolveTokenRoleLinks(tx, token, allowMissingPolicyAndRoleIDs); err != nil {
 		return err
 	}
 
@@ -563,7 +874,12 @@ func (s *Store) aclTokenGetTxn(tx *memdb.Txn, ws memdb.WatchSet, value, index st
 	ws.Add(watchCh)
 
 	if rawToken != nil {
-		token, err := s.fixupTokenPolicyLinks(tx, rawToken.(*structs.ACLToken))
+		token := rawToken.(*structs.ACLToken)
+		token, err := s.fixupTokenPolicyLinks(tx, token)
+		if err != nil {
+			return nil, err
+		}
+		token, err = s.fixupTokenRoleLinks(tx, token)
 		if err != nil {
 			return nil, err
 		}
@@ -574,7 +890,7 @@ func (s *Store) aclTokenGetTxn(tx *memdb.Txn, ws memdb.WatchSet, value, index st
 }
 
 // ACLTokenList is used to list out all of the ACLs in the state store.
-func (s *Store) ACLTokenList(ws memdb.WatchSet, local, global bool, policy string) (uint64, structs.ACLTokens, error) {
+func (s *Store) ACLTokenList(ws memdb.WatchSet, local, global bool, policy, role string) (uint64, structs.ACLTokens, error) {
 	tx := s.db.Txn(false)
 	defer tx.Abort()
 
@@ -585,6 +901,10 @@ func (s *Store) ACLTokenList(ws memdb.WatchSet, local, global bool, policy strin
 	// to false but for defaulted structs (zero values for both) we want it to list out
 	// all tokens so our checks just ensure that global == local
 
+	if policy != "" && role != "" {
+		return 0, nil, fmt.Errorf("cannot filter by role and policy at the same time")
+	}
+
 	if policy != "" {
 		iter, err = tx.Get("acl-tokens", "policies", policy)
 		if err == nil && global != local {
@@ -600,6 +920,24 @@ func (s *Store) ACLTokenList(ws memdb.WatchSet, local, global bool, policy strin
 					return false
 				}
 
+				return true
+			})
+		}
+	} else if role != "" {
+		iter, err = tx.Get("acl-tokens", "roles", role)
+		if err == nil && global != local {
+			iter = memdb.NewFilterIterator(iter, func(raw interface{}) bool {
+				token, ok := raw.(*structs.ACLToken)
+				if !ok {
+					return true
+				}
+
+				if global && !token.Local {
+					return false
+				} else if local && token.Local {
+					return false
+				}
+
 				return true
 			})
 		}
@@ -618,8 +956,12 @@ func (s *Store) ACLTokenList(ws memdb.WatchSet, local, global bool, policy strin
 
 	var result structs.ACLTokens
 	for raw := iter.Next(); raw != nil; raw = iter.Next() {
-		token, err := s.fixupTokenPolicyLinks(tx, raw.(*structs.ACLToken))
-
+		token := raw.(*structs.ACLToken)
+		token, err := s.fixupTokenPolicyLinks(tx, token)
+		if err != nil {
+			return 0, nil, err
+		}
+		token, err = s.fixupTokenRoleLinks(tx, token)
 		if err != nil {
 			return 0, nil, err
 		}
@@ -1000,3 +1342,240 @@ func (s *Store) aclPolicyDeleteTxn(tx *memdb.Txn, idx uint64, value, index strin
 	}
 	return nil
 }
+
+func (s *Store) ACLRoleBatchSet(idx uint64, roles structs.ACLRoles) error {
+	tx := s.db.Txn(true)
+	defer tx.Abort()
+
+	for _, role := range roles {
+		if err := s.aclRoleSetTxn(tx, idx, role, true); err != nil {
+			return err
+		}
+	}
+
+	if err := indexUpdateMaxTxn(tx, idx, "acl-roles"); err != nil {
+		return fmt.Errorf("failed updating index: %s", err)
+	}
+
+	tx.Commit()
+	return nil
+}
+
+func (s *Store) ACLRoleSet(idx uint64, role *structs.ACLRole) error {
+	tx := s.db.Txn(true)
+	defer tx.Abort()
+
+	if err := s.aclRoleSetTxn(tx, idx, role, false); err != nil {
+		return err
+	}
+	if err := indexUpdateMaxTxn(tx, idx, "acl-roles"); err != nil {
+		return fmt.Errorf("failed updating index: %s", err)
+	}
+
+	tx.Commit()
+	return nil
+}
+
+func (s *Store) aclRoleSetTxn(tx *memdb.Txn, idx uint64, role *structs.ACLRole, allowMissing bool) error {
+	// Check that the ID is set
+	if role.ID == "" {
+		return ErrMissingACLRoleID
+	}
+
+	if role.Name == "" {
+		return ErrMissingACLRoleName
+	}
+
+	existing, err := tx.First("acl-roles", "id", role.ID)
+	if err != nil {
+		return fmt.Errorf("failed acl role lookup: %v", err)
+	}
+
+	// ensure the name is unique (cannot conflict with another role with a different ID)
+	nameMatch, err := tx.First("acl-roles", "name", role.Name)
+	if err != nil {
+		return fmt.Errorf("failed acl role lookup: %v", err)
+	}
+	if nameMatch != nil && role.ID != nameMatch.(*structs.ACLRole).ID {
+		return fmt.Errorf("A role with name %q already exists", role.Name)
+	}
+
+	if err := s.resolveRolePolicyLinks(tx, role, allowMissing); err != nil {
+		return err
+	}
+
+	for _, svcid := range role.ServiceIdentities {
+		if svcid.ServiceName == "" {
+			return fmt.Errorf("Encountered a Role with an empty service identity name in the state store")
+		}
+	}
+
+	// Set the indexes
+	if existing != nil {
+		role.CreateIndex = existing.(*structs.ACLRole).CreateIndex
+		role.ModifyIndex = idx
+	} else {
+		role.CreateIndex = idx
+		role.ModifyIndex = idx
+	}
+
+	if err := tx.Insert("acl-roles", role); err != nil {
+		return fmt.Errorf("failed inserting acl role: %v", err)
+	}
+	return nil
+}
+
+func (s *Store) ACLRoleGetByID(ws memdb.WatchSet, id string) (uint64, *structs.ACLRole, error) {
+	return s.aclRoleGet(ws, id, "id")
+}
+
+func (s *Store) ACLRoleGetByName(ws memdb.WatchSet, name string) (uint64, *structs.ACLRole, error) {
+	return s.aclRoleGet(ws, name, "name")
+}
+
+func (s *Store) ACLRoleBatchGet(ws memdb.WatchSet, ids []string) (uint64, structs.ACLRoles, error) {
+	tx := s.db.Txn(false)
+	defer tx.Abort()
+
+	roles := make(structs.ACLRoles, 0)
+	for _, rid := range ids {
+		role, err := s.getRoleWithTxn(tx, ws, rid, "id")
+		if err != nil {
+			return 0, nil, err
+		}
+
+		if role != nil {
+			roles = append(roles, role)
+		}
+	}
+
+	idx := maxIndexTxn(tx, "acl-roles")
+
+	return idx, roles, nil
+}
+
+func (s *Store) getRoleWithTxn(tx *memdb.Txn, ws memdb.WatchSet, value, index string) (*structs.ACLRole, error) {
+	watchCh, rawRole, err := tx.FirstWatch("acl-roles", index, value)
+	if err != nil {
+		return nil, fmt.Errorf("failed acl role lookup: %v", err)
+	}
+	ws.Add(watchCh)
+
+	if rawRole != nil {
+		role := rawRole.(*structs.ACLRole)
+		role, err := s.fixupRolePolicyLinks(tx, role)
+		if err != nil {
+			return nil, err
+		}
+		return role, nil
+	}
+
+	return nil, nil
+}
+
+func (s *Store) aclRoleGet(ws memdb.WatchSet, value, index string) (uint64, *structs.ACLRole, error) {
+	tx := s.db.Txn(false)
+	defer tx.Abort()
+
+	role, err := s.getRoleWithTxn(tx, ws, value, index)
+	if err != nil {
+		return 0, nil, err
+	}
+
+	idx := maxIndexTxn(tx, "acl-roles")
+
+	return idx, role, nil
+}
+
+func (s *Store) ACLRoleList(ws memdb.WatchSet, policy string) (uint64, structs.ACLRoles, error) {
+	tx := s.db.Txn(false)
+	defer tx.Abort()
+
+	var iter memdb.ResultIterator
+	var err error
+
+	if policy != "" {
+		iter, err = tx.Get("acl-roles", "policies", policy)
+	} else {
+		iter, err = tx.Get("acl-roles", "id")
+	}
+
+	if err != nil {
+		return 0, nil, fmt.Errorf("failed acl role lookup: %v", err)
+	}
+	ws.Add(iter.WatchCh())
+
+	var result structs.ACLRoles
+	for raw := iter.Next(); raw != nil; raw = iter.Next() {
+		role := raw.(*structs.ACLRole)
+		role, err := s.fixupRolePolicyLinks(tx, role)
+		if err != nil {
+			return 0, nil, err
+		}
+		result = append(result, role)
+	}
+
+	// Get the table index.
+	idx := maxIndexTxn(tx, "acl-roles")
+
+	return idx, result, nil
+}
+
+func (s *Store) ACLRoleDeleteByID(idx uint64, id string) error {
+	return s.aclRoleDelete(idx, id, "id")
+}
+
+func (s *Store) ACLRoleDeleteByName(idx uint64, name string) error {
+	return s.aclRoleDelete(idx, name, "name")
+}
+
+func (s *Store) ACLRoleBatchDelete(idx uint64, roleIDs []string) error {
+	tx := s.db.Txn(true)
+	defer tx.Abort()
+
+	for _, roleID := range roleIDs {
+		if err := s.aclRoleDeleteTxn(tx, idx, roleID, "id"); err != nil {
+			return err
+		}
+	}
+
+	if err := indexUpdateMaxTxn(tx, idx, "acl-roles"); err != nil {
+		return fmt.Errorf("failed updating index: %v", err)
+	}
+	tx.Commit()
+	return nil
+}
+
+func (s *Store) aclRoleDelete(idx uint64, value, index string) error {
+	tx := s.db.Txn(true)
+	defer tx.Abort()
+
+	if err := s.aclRoleDeleteTxn(tx, idx, value, index); err != nil {
+		return err
+	}
+	if err := indexUpdateMaxTxn(tx, idx, "acl-roles"); err != nil {
+		return fmt.Errorf("failed updating index: %v", err)
+	}
+
+	tx.Commit()
+	return nil
+}
+
+func (s *Store) aclRoleDeleteTxn(tx *memdb.Txn, idx uint64, value, index string) error {
+	// Look up the existing role
+	rawRole, err := tx.First("acl-roles", index, value)
+	if err != nil {
+		return fmt.Errorf("failed acl role lookup: %v", err)
+	}
+
+	if rawRole == nil {
+		return nil
+	}
+
+	role := rawRole.(*structs.ACLRole)
+
+	if err := tx.Delete("acl-roles", role); err != nil {
+		return fmt.Errorf("failed deleting acl role: %v", err)
+	}
+	return nil
+}

agent/consul/state/state_store.go

@@ -5,7 +5,7 @@ import (
 	"fmt"
 
 	"github.com/hashicorp/consul/types"
-	"github.com/hashicorp/go-memdb"
+	memdb "github.com/hashicorp/go-memdb"
 )
 
 var (
@@ -37,6 +37,18 @@ var (
 	// policy with an empty Name.
 	ErrMissingACLPolicyName = errors.New("Missing ACL Policy Name")
 
+	// ErrMissingACLRoleID is returned when an role set is called on
+	// a role with an empty ID.
+	ErrMissingACLRoleID = errors.New("Missing ACL Role ID")
+
+	// ErrMissingACLRoleName is returned when an role set is called on
+	// a role with an empty Name.
+	ErrMissingACLRoleName = errors.New("Missing ACL Role Name")
+
+	// ErrInvalidACLRoleName is returned when an role set is called on
+	// a role with an invalid Name.
+	ErrInvalidACLRoleName = errors.New("Invalid ACL Role Name")
+
 	// ErrMissingQueryID is returned when a Query set is called on
 	// a Query with an empty ID.
 	ErrMissingQueryID = errors.New("Missing Query ID")

agent/http_oss.go

@@ -14,6 +14,10 @@ func init() {
 	registerEndpoint("/v1/acl/policies", []string{"GET"}, (*HTTPServer).ACLPolicyList)
 	registerEndpoint("/v1/acl/policy", []string{"PUT"}, (*HTTPServer).ACLPolicyCreate)
 	registerEndpoint("/v1/acl/policy/", []string{"GET", "PUT", "DELETE"}, (*HTTPServer).ACLPolicyCRUD)
+	registerEndpoint("/v1/acl/roles", []string{"GET"}, (*HTTPServer).ACLRoleList)
+	registerEndpoint("/v1/acl/role", []string{"PUT"}, (*HTTPServer).ACLRoleCreate)
+	registerEndpoint("/v1/acl/role/name/", []string{"GET"}, (*HTTPServer).ACLRoleReadByName)
+	registerEndpoint("/v1/acl/role/", []string{"GET", "PUT", "DELETE"}, (*HTTPServer).ACLRoleCRUD)
 	registerEndpoint("/v1/acl/rules/translate", []string{"POST"}, (*HTTPServer).ACLRulesTranslate)
 	registerEndpoint("/v1/acl/rules/translate/", []string{"GET"}, (*HTTPServer).ACLRulesTranslateLegacyToken)
 	registerEndpoint("/v1/acl/tokens", []string{"GET"}, (*HTTPServer).ACLTokenList)

agent/structs/acl.go

@@ -129,6 +129,7 @@ type ACLIdentity interface {
 	ID() string
 	SecretToken() string
 	PolicyIDs() []string
+	RoleIDs() []string
 	EmbeddedPolicy() *ACLPolicy
 	ServiceIdentityList() []*ACLServiceIdentity
 	IsExpired(asOf time.Time) bool
@@ -139,6 +140,11 @@ type ACLTokenPolicyLink struct {
 	Name string `hash:"ignore"`
 }
 
+type ACLTokenRoleLink struct {
+	ID   string
+	Name string `hash:"ignore"`
+}
+
 // ACLServiceIdentity represents a high-level grant of all necessary privileges
 // to assume the identity of the named Service in the Catalog and within
 // Connect.
@@ -166,6 +172,14 @@ func (s *ACLServiceIdentity) AddToHash(h hash.Hash) {
 	}
 }
 
+func (s *ACLServiceIdentity) EstimateSize() int {
+	size := len(s.ServiceName)
+	for _, dc := range s.Datacenters {
+		size += len(dc)
+	}
+	return size
+}
+
 func (s *ACLServiceIdentity) SyntheticPolicy() *ACLPolicy {
 	// Given that we validate this string name before persisting, we do not
 	// have to escape it before doing the following interpolation.
@@ -197,6 +211,11 @@ type ACLToken struct {
 	// the list of policy names gets validated and the policy IDs get stored herein
 	Policies []ACLTokenPolicyLink `json:",omitempty"`
 
+	// List of role links. Note this is the list of IDs and not the names.
+	// Prior to token creation the list of role names gets validated and the
+	// role IDs get stored herein
+	Roles []ACLTokenRoleLink `json:",omitempty"`
+
 	// List of services to generate synthetic policies for.
 	ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
 
@@ -249,12 +268,17 @@ type ACLToken struct {
 func (t *ACLToken) Clone() *ACLToken {
 	t2 := *t
 	t2.Policies = nil
+	t2.Roles = nil
 	t2.ServiceIdentities = nil
 
 	if len(t.Policies) > 0 {
 		t2.Policies = make([]ACLTokenPolicyLink, len(t.Policies))
 		copy(t2.Policies, t.Policies)
 	}
+	if len(t.Roles) > 0 {
+		t2.Roles = make([]ACLTokenRoleLink, len(t.Roles))
+		copy(t2.Roles, t.Roles)
+	}
 	if len(t.ServiceIdentities) > 0 {
 		t2.ServiceIdentities = make([]*ACLServiceIdentity, len(t.ServiceIdentities))
 		for i, s := range t.ServiceIdentities {
@@ -284,6 +308,14 @@ func (t *ACLToken) PolicyIDs() []string {
 	return ids
 }
 
+func (t *ACLToken) RoleIDs() []string {
+	var ids []string
+	for _, link := range t.Roles {
+		ids = append(ids, link.ID)
+	}
+	return ids
+}
+
 func (t *ACLToken) ServiceIdentityList() []*ACLServiceIdentity {
 	if len(t.ServiceIdentities) == 0 {
 		return nil
@@ -310,6 +342,7 @@ func (t *ACLToken) HasExpirationTime() bool {
 func (t *ACLToken) UsesNonLegacyFields() bool {
 	return len(t.Policies) > 0 ||
 		len(t.ServiceIdentities) > 0 ||
+		len(t.Roles) > 0 ||
 		t.Type == "" ||
 		t.HasExpirationTime() ||
 		t.ExpirationTTL != 0
@@ -376,6 +409,10 @@ func (t *ACLToken) SetHash(force bool) []byte {
 			hash.Write([]byte(link.ID))
 		}
 
+		for _, link := range t.Roles {
+			hash.Write([]byte(link.ID))
+		}
+
 		for _, srvid := range t.ServiceIdentities {
 			srvid.AddToHash(hash)
 		}
@@ -395,11 +432,11 @@ func (t *ACLToken) EstimateSize() int {
 	for _, link := range t.Policies {
 		size += len(link.ID) + len(link.Name)
 	}
+	for _, link := range t.Roles {
+		size += len(link.ID) + len(link.Name)
+	}
 	for _, srvid := range t.ServiceIdentities {
-		size += len(srvid.ServiceName)
-		for _, dc := range srvid.Datacenters {
-			size += len(dc)
-		}
+		size += srvid.EstimateSize()
 	}
 	return size
 }
@@ -411,6 +448,7 @@ type ACLTokenListStub struct {
 	AccessorID        string
 	Description       string
 	Policies          []ACLTokenPolicyLink  `json:",omitempty"`
+	Roles             []ACLTokenRoleLink    `json:",omitempty"`
 	ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
 	Local             bool
 	ExpirationTime    *time.Time `json:",omitempty"`
@@ -428,6 +466,7 @@ func (token *ACLToken) Stub() *ACLTokenListStub {
 		AccessorID:        token.AccessorID,
 		Description:       token.Description,
 		Policies:          token.Policies,
+		Roles:             token.Roles,
 		ServiceIdentities: token.ServiceIdentities,
 		Local:             token.Local,
 		ExpirationTime:    token.ExpirationTime,
@@ -650,14 +689,160 @@ func (policies ACLPolicies) Merge(cache *ACLCaches, sentinel sentinel.Evaluator)
 	return acl.MergePolicies(parsed), nil
 }
 
+type ACLRoles []*ACLRole
+
+// HashKey returns a consistent hash for a set of roles.
+func (roles ACLRoles) HashKey() string {
+	cacheKeyHash, err := blake2b.New256(nil)
+	if err != nil {
+		panic(err)
+	}
+	for _, role := range roles {
+		cacheKeyHash.Write([]byte(role.ID))
+		// including the modify index prevents a role set from being
+		// cached if one of the roles has changed
+		binary.Write(cacheKeyHash, binary.BigEndian, role.ModifyIndex)
+	}
+	return fmt.Sprintf("%x", cacheKeyHash.Sum(nil))
+}
+
+func (roles ACLRoles) Sort() {
+	sort.Slice(roles, func(i, j int) bool {
+		return roles[i].ID < roles[j].ID
+	})
+}
+
+type ACLRolePolicyLink struct {
+	ID   string
+	Name string `hash:"ignore"`
+}
+
+type ACLRole struct {
+	// ID is the internal UUID associated with the role
+	ID string
+
+	// Name is the unique name to reference the role by.
+	//
+	// Validated with structs.isValidRoleName()
+	Name string
+
+	// Description is a human readable description (Optional)
+	Description string
+
+	// List of policy links.
+	// Note this is the list of IDs and not the names. Prior to role creation
+	// the list of policy names gets validated and the policy IDs get stored herein
+	Policies []ACLRolePolicyLink `json:",omitempty"`
+
+	// List of services to generate synthetic policies for.
+	ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
+
+	// Hash of the contents of the role
+	// This does not take into account the ID (which is immutable)
+	// nor the raft metadata.
+	//
+	// This is needed mainly for replication purposes. When replicating from
+	// one DC to another keeping the content Hash will allow us to avoid
+	// unnecessary calls to the authoritative DC
+	Hash []byte
+
+	// Embedded Raft Metadata
+	RaftIndex `hash:"ignore"`
+}
+
+func (r *ACLRole) Clone() *ACLRole {
+	r2 := *r
+	r2.Policies = nil
+	r2.ServiceIdentities = nil
+
+	if len(r.Policies) > 0 {
+		r2.Policies = make([]ACLRolePolicyLink, len(r.Policies))
+		copy(r2.Policies, r.Policies)
+	}
+	if len(r.ServiceIdentities) > 0 {
+		r2.ServiceIdentities = make([]*ACLServiceIdentity, len(r.ServiceIdentities))
+		for i, s := range r.ServiceIdentities {
+			r2.ServiceIdentities[i] = s.Clone()
+		}
+	}
+	return &r2
+}
+
+func (r *ACLRole) SetHash(force bool) []byte {
+	if force || r.Hash == nil {
+		// Initialize a 256bit Blake2 hash (32 bytes)
+		hash, err := blake2b.New256(nil)
+		if err != nil {
+			panic(err)
+		}
+
+		// Any non-immutable "content" fields should be involved with the
+		// overall hash. The ID is immutable which is why it isn't here.  The
+		// raft indices are metadata similar to the hash which is why they
+		// aren't incorporated. CreateTime is similarly immutable
+		//
+		// The Hash is really only used for replication to determine if a role
+		// has changed and should be updated locally.
+
+		// Write all the user set fields
+		hash.Write([]byte(r.Name))
+		hash.Write([]byte(r.Description))
+		for _, link := range r.Policies {
+			hash.Write([]byte(link.ID))
+		}
+		for _, srvid := range r.ServiceIdentities {
+			srvid.AddToHash(hash)
+		}
+
+		// Finalize the hash
+		hashVal := hash.Sum(nil)
+
+		// Set and return the hash
+		r.Hash = hashVal
+	}
+	return r.Hash
+}
+
+func (r *ACLRole) EstimateSize() int {
+	// This is just an estimate. There is other data structure overhead
+	// pointers etc that this does not account for.
+
+	// 60 = 36 (uuid) + 16 (RaftIndex) + 8 (Hash)
+	size := 60 + len(r.Name) + len(r.Description)
+	for _, link := range r.Policies {
+		size += len(link.ID) + len(link.Name)
+	}
+	for _, srvid := range r.ServiceIdentities {
+		size += srvid.EstimateSize()
+	}
+
+	return size
+}
+
 type ACLReplicationType string
 
 const (
 	ACLReplicateLegacy   ACLReplicationType = "legacy"
 	ACLReplicatePolicies ACLReplicationType = "policies"
+	ACLReplicateRoles    ACLReplicationType = "roles"
 	ACLReplicateTokens   ACLReplicationType = "tokens"
 )
 
+func (t ACLReplicationType) SingularNoun() string {
+	switch t {
+	case ACLReplicateLegacy:
+		return "legacy"
+	case ACLReplicatePolicies:
+		return "policy"
+	case ACLReplicateRoles:
+		return "role"
+	case ACLReplicateTokens:
+		return "token"
+	default:
+		return "<UNKNOWN>"
+	}
+}
+
 // ACLReplicationStatus provides information about the health of the ACL
 // replication system.
 type ACLReplicationStatus struct {
@@ -666,6 +851,7 @@ type ACLReplicationStatus struct {
 	SourceDatacenter     string
 	ReplicationType      ACLReplicationType
 	ReplicatedIndex      uint64
+	ReplicatedRoleIndex  uint64
 	ReplicatedTokenIndex uint64
 	LastSuccess          time.Time
 	LastError            time.Time
@@ -711,6 +897,7 @@ type ACLTokenListRequest struct {
 	IncludeLocal  bool   // Whether local tokens should be included
 	IncludeGlobal bool   // Whether global tokens should be included
 	Policy        string // Policy filter
+	Role          string // Role filter
 	Datacenter    string // The datacenter to perform the request within
 	QueryOptions
 }
@@ -878,3 +1065,92 @@ func cloneStringSlice(s []string) []string {
 	copy(out, s)
 	return out
 }
+
+// ACLRoleSetRequest is used at the RPC layer for creation and update requests
+type ACLRoleSetRequest struct {
+	Role       ACLRole // The policy to upsert
+	Datacenter string  // The datacenter to perform the request within
+	WriteRequest
+}
+
+func (r *ACLRoleSetRequest) RequestDatacenter() string {
+	return r.Datacenter
+}
+
+// ACLRoleDeleteRequest is used at the RPC layer deletion requests
+type ACLRoleDeleteRequest struct {
+	RoleID     string // id of the role to delete
+	Datacenter string // The datacenter to perform the request within
+	WriteRequest
+}
+
+func (r *ACLRoleDeleteRequest) RequestDatacenter() string {
+	return r.Datacenter
+}
+
+// ACLRoleGetRequest is used at the RPC layer to perform role read operations
+type ACLRoleGetRequest struct {
+	RoleID     string // id used for the role lookup (one of RoleID or RoleName is allowed)
+	RoleName   string // name used for the role lookup (one of RoleID or RoleName is allowed)
+	Datacenter string // The datacenter to perform the request within
+	QueryOptions
+}
+
+func (r *ACLRoleGetRequest) RequestDatacenter() string {
+	return r.Datacenter
+}
+
+// ACLRoleListRequest is used at the RPC layer to request a listing of roles
+type ACLRoleListRequest struct {
+	Policy     string // Policy filter
+	Datacenter string // The datacenter to perform the request within
+	QueryOptions
+}
+
+func (r *ACLRoleListRequest) RequestDatacenter() string {
+	return r.Datacenter
+}
+
+type ACLRoleListResponse struct {
+	Roles ACLRoles
+	QueryMeta
+}
+
+// ACLRoleBatchGetRequest is used at the RPC layer to request a subset of
+// the roles associated with the token used for retrieval
+type ACLRoleBatchGetRequest struct {
+	RoleIDs    []string // List of role ids to fetch
+	Datacenter string   // The datacenter to perform the request within
+	QueryOptions
+}
+
+func (r *ACLRoleBatchGetRequest) RequestDatacenter() string {
+	return r.Datacenter
+}
+
+// ACLRoleResponse returns a single role + metadata
+type ACLRoleResponse struct {
+	Role *ACLRole
+	QueryMeta
+}
+
+type ACLRoleBatchResponse struct {
+	Roles []*ACLRole
+	QueryMeta
+}
+
+// ACLRoleBatchSetRequest is used at the Raft layer for batching
+// multiple role creations and updates
+//
+// This is particularly useful during replication
+type ACLRoleBatchSetRequest struct {
+	Roles ACLRoles
+}
+
+// ACLRoleBatchDeleteRequest is used at the Raft layer for batching
+// multiple role deletions
+//
+// This is particularly useful during replication
+type ACLRoleBatchDeleteRequest struct {
+	RoleIDs []string
+}

agent/structs/acl_cache.go

@@ -4,7 +4,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/consul/acl"
-	"github.com/hashicorp/golang-lru"
+	lru "github.com/hashicorp/golang-lru"
 )
 
 type ACLCachesConfig struct {
@@ -12,6 +12,7 @@ type ACLCachesConfig struct {
 	Policies       int
 	ParsedPolicies int
 	Authorizers    int
+	Roles          int
 }
 
 type ACLCaches struct {
@@ -19,6 +20,7 @@ type ACLCaches struct {
 	parsedPolicies *lru.TwoQueueCache // policy content hash -> acl.Policy
 	policies       *lru.TwoQueueCache // policy ID -> ACLPolicy
 	authorizers    *lru.TwoQueueCache // token secret -> acl.Authorizer
+	roles          *lru.TwoQueueCache // role ID -> ACLRole
 }
 
 type IdentityCacheEntry struct {
@@ -58,6 +60,16 @@ func (e *AuthorizerCacheEntry) Age() time.Duration {
 	return time.Since(e.CacheTime)
 }
 
+// RoleCacheEntry is the payload for by by-id and by-name caches.
+type RoleCacheEntry struct {
+	Role      *ACLRole
+	CacheTime time.Time
+}
+
+func (e *RoleCacheEntry) Age() time.Duration {
+	return time.Since(e.CacheTime)
+}
+
 func NewACLCaches(config *ACLCachesConfig) (*ACLCaches, error) {
 	cache := &ACLCaches{}
 
@@ -97,6 +109,15 @@ func NewACLCaches(config *ACLCachesConfig) (*ACLCaches, error) {
 		cache.authorizers = authCache
 	}
 
+	if config != nil && config.Roles > 0 {
+		roleCache, err := lru.New2Q(config.Roles)
+		if err != nil {
+			return nil, err
+		}
+
+		cache.roles = roleCache
+	}
+
 	return cache, nil
 }
 
@@ -152,6 +173,19 @@ func (c *ACLCaches) GetAuthorizer(id string) *AuthorizerCacheEntry {
 	return nil
 }
 
+// GetRoleByID fetches a role from the cache by id and returns it
+func (c *ACLCaches) GetRole(roleID string) *RoleCacheEntry {
+	if c == nil || c.roles == nil {
+		return nil
+	}
+
+	if raw, ok := c.roles.Get(roleID); ok {
+		return raw.(*RoleCacheEntry)
+	}
+
+	return nil
+}
+
 // PutIdentity adds a new identity to the cache
 func (c *ACLCaches) PutIdentity(id string, ident ACLIdentity) {
 	if c == nil || c.identities == nil {
@@ -193,6 +227,12 @@ func (c *ACLCaches) PutAuthorizerWithTTL(id string, authorizer acl.Authorizer, t
 	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 {
+		c.roles.Add(roleID, &RoleCacheEntry{Role: role, CacheTime: time.Now()})
+	}
+}
+
 func (c *ACLCaches) RemoveIdentity(id string) {
 	if c != nil && c.identities != nil {
 		c.identities.Remove(id)
@@ -205,6 +245,12 @@ func (c *ACLCaches) RemovePolicy(policyID string) {
 	}
 }
 
+func (c *ACLCaches) RemoveRole(roleID string) {
+	if c != nil && c.roles != nil && roleID != "" {
+		c.roles.Remove(roleID)
+	}
+}
+
 func (c *ACLCaches) Purge() {
 	if c != nil {
 		if c.identities != nil {
@@ -219,5 +265,8 @@ func (c *ACLCaches) Purge() {
 		if c.authorizers != nil {
 			c.authorizers.Purge()
 		}
+		if c.roles != nil {
+			c.roles.Purge()
+		}
 	}
 }

agent/structs/acl_cache_test.go

@@ -16,7 +16,7 @@ func TestStructs_ACLCaches(t *testing.T) {
 		t.Run("Valid Sizes", func(t *testing.T) {
 			t.Parallel()
 			// 1 isn't valid due to a bug in golang-lru library
-			config := ACLCachesConfig{2, 2, 2, 2}
+			config := ACLCachesConfig{2, 2, 2, 2, 2}
 
 			cache, err := NewACLCaches(&config)
 			require.NoError(t, err)
@@ -30,7 +30,7 @@ func TestStructs_ACLCaches(t *testing.T) {
 		t.Run("Zero Sizes", func(t *testing.T) {
 			t.Parallel()
 			// 1 isn't valid due to a bug in golang-lru library
-			config := ACLCachesConfig{0, 0, 0, 0}
+			config := ACLCachesConfig{0, 0, 0, 0, 0}
 
 			cache, err := NewACLCaches(&config)
 			require.NoError(t, err)
@@ -102,4 +102,19 @@ func TestStructs_ACLCaches(t *testing.T) {
 		require.NotNil(t, entry.Authorizer)
 		require.True(t, entry.Authorizer == acl.DenyAll())
 	})
+
+	t.Run("Roles", func(t *testing.T) {
+		t.Parallel()
+		// 1 isn't valid due to a bug in golang-lru library
+		config := ACLCachesConfig{Roles: 4}
+
+		cache, err := NewACLCaches(&config)
+		require.NoError(t, err)
+		require.NotNil(t, cache)
+
+		cache.PutRole("foo", &ACLRole{})
+		entry := cache.GetRole("foo")
+		require.NotNil(t, entry)
+		require.NotNil(t, entry.Role)
+	})
 }

agent/structs/acl_test.go

@@ -514,6 +514,7 @@ func TestStructs_ACLPolicies_resolveWithCache(t *testing.T) {
 		Policies:       0,
 		ParsedPolicies: 4,
 		Authorizers:    0,
+		Roles:          0,
 	}
 	cache, err := NewACLCaches(&config)
 	require.NoError(t, err)
@@ -606,6 +607,7 @@ func TestStructs_ACLPolicies_Compile(t *testing.T) {
 		Policies:       0,
 		ParsedPolicies: 4,
 		Authorizers:    2,
+		Roles:          0,
 	}
 	cache, err := NewACLCaches(&config)
 	require.NoError(t, err)

agent/structs/structs.go

@@ -56,6 +56,8 @@ const (
 	ACLPolicyDeleteRequestType             = 20
 	ConnectCALeafRequestType               = 21
 	ConfigEntryRequestType                 = 22
+	ACLRoleSetRequestType                  = 23
+	ACLRoleDeleteRequestType               = 24
 )
 
 const (

api/acl.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net/url"
 	"time"
 )
 
@@ -19,6 +20,10 @@ type ACLTokenPolicyLink struct {
 	ID   string
 	Name string
 }
+type ACLTokenRoleLink struct {
+	ID   string
+	Name string
+}
 
 // ACLToken represents an ACL Token
 type ACLToken struct {
@@ -28,6 +33,7 @@ type ACLToken struct {
 	SecretID          string
 	Description       string
 	Policies          []*ACLTokenPolicyLink `json:",omitempty"`
+	Roles             []*ACLTokenRoleLink   `json:",omitempty"`
 	ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
 	Local             bool
 	ExpirationTTL     time.Duration `json:",omitempty"`
@@ -46,6 +52,7 @@ type ACLTokenListEntry struct {
 	AccessorID        string
 	Description       string
 	Policies          []*ACLTokenPolicyLink `json:",omitempty"`
+	Roles             []*ACLTokenRoleLink   `json:",omitempty"`
 	ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
 	Local             bool
 	ExpirationTime    *time.Time `json:",omitempty"`
@@ -72,6 +79,7 @@ type ACLReplicationStatus struct {
 	SourceDatacenter     string
 	ReplicationType      string
 	ReplicatedIndex      uint64
+	ReplicatedRoleIndex  uint64
 	ReplicatedTokenIndex uint64
 	LastSuccess          time.Time
 	LastError            time.Time
@@ -107,6 +115,23 @@ type ACLPolicyListEntry struct {
 	ModifyIndex uint64
 }
 
+type ACLRolePolicyLink struct {
+	ID   string
+	Name string
+}
+
+// ACLRole represents an ACL Role.
+type ACLRole struct {
+	ID                string
+	Name              string
+	Description       string
+	Policies          []*ACLRolePolicyLink  `json:",omitempty"`
+	ServiceIdentities []*ACLServiceIdentity `json:",omitempty"`
+	Hash              []byte
+	CreateIndex       uint64
+	ModifyIndex       uint64
+}
+
 // ACL can be used to query the ACL endpoints
 type ACL struct {
 	c *Client
@@ -599,3 +624,142 @@ func (a *ACL) RulesTranslateToken(tokenID string) (string, error) {
 
 	return string(ruleBytes), nil
 }
+
+// RoleCreate will create a new role. It is not allowed for the role parameters
+// ID field to be set as this will be generated by Consul while processing the request.
+func (a *ACL) RoleCreate(role *ACLRole, q *WriteOptions) (*ACLRole, *WriteMeta, error) {
+	if role.ID != "" {
+		return nil, nil, fmt.Errorf("Cannot specify an ID in Role Creation")
+	}
+
+	r := a.c.newRequest("PUT", "/v1/acl/role")
+	r.setWriteOptions(q)
+	r.obj = role
+	rtt, resp, err := requireOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	wm := &WriteMeta{RequestTime: rtt}
+	var out ACLRole
+	if err := decodeBody(resp, &out); err != nil {
+		return nil, nil, err
+	}
+
+	return &out, wm, nil
+}
+
+// RoleUpdate updates a role. The ID field of the role parameter must be set to an
+// existing role ID
+func (a *ACL) RoleUpdate(role *ACLRole, q *WriteOptions) (*ACLRole, *WriteMeta, error) {
+	if role.ID == "" {
+		return nil, nil, fmt.Errorf("Must specify an ID in Role Creation")
+	}
+
+	r := a.c.newRequest("PUT", "/v1/acl/role/"+role.ID)
+	r.setWriteOptions(q)
+	r.obj = role
+	rtt, resp, err := requireOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	wm := &WriteMeta{RequestTime: rtt}
+	var out ACLRole
+	if err := decodeBody(resp, &out); err != nil {
+		return nil, nil, err
+	}
+
+	return &out, wm, nil
+}
+
+// RoleDelete deletes a role given its ID.
+func (a *ACL) RoleDelete(roleID string, q *WriteOptions) (*WriteMeta, error) {
+	r := a.c.newRequest("DELETE", "/v1/acl/role/"+roleID)
+	r.setWriteOptions(q)
+	rtt, resp, err := requireOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, err
+	}
+	resp.Body.Close()
+
+	wm := &WriteMeta{RequestTime: rtt}
+	return wm, nil
+}
+
+// RoleRead retrieves the role details (by ID). Returns nil if not found.
+func (a *ACL) RoleRead(roleID string, q *QueryOptions) (*ACLRole, *QueryMeta, error) {
+	r := a.c.newRequest("GET", "/v1/acl/role/"+roleID)
+	r.setQueryOptions(q)
+	found, rtt, resp, err := requireNotFoundOrOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	qm := &QueryMeta{}
+	parseQueryMeta(resp, qm)
+	qm.RequestTime = rtt
+
+	if !found {
+		return nil, qm, nil
+	}
+
+	var out ACLRole
+	if err := decodeBody(resp, &out); err != nil {
+		return nil, nil, err
+	}
+
+	return &out, qm, nil
+}
+
+// RoleReadByName retrieves the role details (by name). Returns nil if not found.
+func (a *ACL) RoleReadByName(roleName string, q *QueryOptions) (*ACLRole, *QueryMeta, error) {
+	r := a.c.newRequest("GET", "/v1/acl/role/name/"+url.QueryEscape(roleName))
+	r.setQueryOptions(q)
+	found, rtt, resp, err := requireNotFoundOrOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	qm := &QueryMeta{}
+	parseQueryMeta(resp, qm)
+	qm.RequestTime = rtt
+
+	if !found {
+		return nil, qm, nil
+	}
+
+	var out ACLRole
+	if err := decodeBody(resp, &out); err != nil {
+		return nil, nil, err
+	}
+
+	return &out, qm, nil
+}
+
+// RoleList retrieves a listing of all roles. The listing does not include some
+// metadata for the role as those should be retrieved by subsequent calls to
+// RoleRead.
+func (a *ACL) RoleList(q *QueryOptions) ([]*ACLRole, *QueryMeta, error) {
+	r := a.c.newRequest("GET", "/v1/acl/roles")
+	r.setQueryOptions(q)
+	rtt, resp, err := requireOK(a.c.doRequest(r))
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	qm := &QueryMeta{}
+	parseQueryMeta(resp, qm)
+	qm.RequestTime = rtt
+
+	var entries []*ACLRole
+	if err := decodeBody(resp, &entries); err != nil {
+		return nil, nil, err
+	}
+	return entries, qm, nil
+}

api/api.go

@@ -897,10 +897,7 @@ func requireOK(d time.Duration, resp *http.Response, e error) (time.Duration, *h
 		return d, nil, e
 	}
 	if resp.StatusCode != 200 {
-		var buf bytes.Buffer
-		io.Copy(&buf, resp.Body)
-		resp.Body.Close()
-		return d, nil, fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, buf.Bytes())
+		return d, nil, generateUnexpectedResponseCodeError(resp)
 	}
 	return d, resp, nil
 }
@@ -912,3 +909,30 @@ func (req *request) filterQuery(filter string) {
 
 	req.params.Set("filter", filter)
 }
+
+// generateUnexpectedResponseCodeError consumes the rest of the body, closes
+// the body stream and generates an error indicating the status code was
+// unexpected.
+func generateUnexpectedResponseCodeError(resp *http.Response) error {
+	var buf bytes.Buffer
+	io.Copy(&buf, resp.Body)
+	resp.Body.Close()
+	return fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, buf.Bytes())
+}
+
+func requireNotFoundOrOK(d time.Duration, resp *http.Response, e error) (bool, time.Duration, *http.Response, error) {
+	if e != nil {
+		if resp != nil {
+			resp.Body.Close()
+		}
+		return false, d, nil, e
+	}
+	switch resp.StatusCode {
+	case 200:
+		return true, d, resp, nil
+	case 404:
+		return false, d, resp, nil
+	default:
+		return false, d, nil, generateUnexpectedResponseCodeError(resp)
+	}
+}

command/acl/acl_helpers.go

@@ -27,6 +27,10 @@ func PrintToken(token *api.ACLToken, ui cli.Ui, showMeta bool) {
 	for _, policy := range token.Policies {
 		ui.Info(fmt.Sprintf("   %s - %s", policy.ID, policy.Name))
 	}
+	ui.Info(fmt.Sprintf("Roles:"))
+	for _, role := range token.Roles {
+		ui.Info(fmt.Sprintf("   %s - %s", role.ID, role.Name))
+	}
 	ui.Info(fmt.Sprintf("Service Identities:"))
 	for _, svcid := range token.ServiceIdentities {
 		if len(svcid.Datacenters) > 0 {
@@ -59,6 +63,10 @@ func PrintTokenListEntry(token *api.ACLTokenListEntry, ui cli.Ui, showMeta bool)
 	for _, policy := range token.Policies {
 		ui.Info(fmt.Sprintf("   %s - %s", policy.ID, policy.Name))
 	}
+	ui.Info(fmt.Sprintf("Roles:"))
+	for _, role := range token.Roles {
+		ui.Info(fmt.Sprintf("   %s - %s", role.ID, role.Name))
+	}
 	ui.Info(fmt.Sprintf("Service Identities:"))
 	for _, svcid := range token.ServiceIdentities {
 		if len(svcid.Datacenters) > 0 {
@@ -95,6 +103,52 @@ func PrintPolicyListEntry(policy *api.ACLPolicyListEntry, ui cli.Ui, showMeta bo
 	}
 }
 
+func PrintRole(role *api.ACLRole, ui cli.Ui, showMeta bool) {
+	ui.Info(fmt.Sprintf("ID:           %s", role.ID))
+	ui.Info(fmt.Sprintf("Name:         %s", role.Name))
+	ui.Info(fmt.Sprintf("Description:  %s", role.Description))
+	if showMeta {
+		ui.Info(fmt.Sprintf("Hash:         %x", role.Hash))
+		ui.Info(fmt.Sprintf("Create Index: %d", role.CreateIndex))
+		ui.Info(fmt.Sprintf("Modify Index: %d", role.ModifyIndex))
+	}
+	ui.Info(fmt.Sprintf("Policies:"))
+	for _, policy := range role.Policies {
+		ui.Info(fmt.Sprintf("   %s - %s", policy.ID, policy.Name))
+	}
+	ui.Info(fmt.Sprintf("Service Identities:"))
+	for _, svcid := range role.ServiceIdentities {
+		if len(svcid.Datacenters) > 0 {
+			ui.Info(fmt.Sprintf("   %s (Datacenters: %s)", svcid.ServiceName, strings.Join(svcid.Datacenters, ", ")))
+		} else {
+			ui.Info(fmt.Sprintf("   %s (Datacenters: all)", svcid.ServiceName))
+		}
+	}
+}
+
+func PrintRoleListEntry(role *api.ACLRole, ui cli.Ui, showMeta bool) {
+	ui.Info(fmt.Sprintf("%s:", role.Name))
+	ui.Info(fmt.Sprintf("   ID:           %s", role.ID))
+	ui.Info(fmt.Sprintf("   Description:  %s", role.Description))
+	if showMeta {
+		ui.Info(fmt.Sprintf("   Hash:         %x", role.Hash))
+		ui.Info(fmt.Sprintf("   Create Index: %d", role.CreateIndex))
+		ui.Info(fmt.Sprintf("   Modify Index: %d", role.ModifyIndex))
+	}
+	ui.Info(fmt.Sprintf("   Policies:"))
+	for _, policy := range role.Policies {
+		ui.Info(fmt.Sprintf("      %s - %s", policy.ID, policy.Name))
+	}
+	ui.Info(fmt.Sprintf("   Service Identities:"))
+	for _, svcid := range role.ServiceIdentities {
+		if len(svcid.Datacenters) > 0 {
+			ui.Info(fmt.Sprintf("      %s (Datacenters: %s)", svcid.ServiceName, strings.Join(svcid.Datacenters, ", ")))
+		} else {
+			ui.Info(fmt.Sprintf("      %s (Datacenters: all)", svcid.ServiceName))
+		}
+	}
+}
+
 func GetTokenIDFromPartial(client *api.Client, partialID string) (string, error) {
 	if partialID == "anonymous" {
 		return structs.ACLTokenAnonymousID, nil
@@ -208,6 +262,53 @@ func GetRulesFromLegacyToken(client *api.Client, tokenID string, isSecret bool)
 	return token.Rules, nil
 }
 
+func GetRoleIDFromPartial(client *api.Client, partialID string) (string, error) {
+	// the full UUID string was given
+	if len(partialID) == 36 {
+		return partialID, nil
+	}
+
+	roles, _, err := client.ACL().RoleList(nil)
+	if err != nil {
+		return "", err
+	}
+
+	roleID := ""
+	for _, role := range roles {
+		if strings.HasPrefix(role.ID, partialID) {
+			if roleID != "" {
+				return "", fmt.Errorf("Partial role ID is not unique")
+			}
+			roleID = role.ID
+		}
+	}
+
+	if roleID == "" {
+		return "", fmt.Errorf("No such role ID with prefix: %s", partialID)
+	}
+
+	return roleID, nil
+}
+
+func GetRoleIDByName(client *api.Client, name string) (string, error) {
+	if name == "" {
+		return "", fmt.Errorf("No name specified")
+	}
+
+	roles, _, err := client.ACL().RoleList(nil)
+	if err != nil {
+		return "", err
+	}
+
+	for _, role := range roles {
+		if role.Name == name {
+			return role.ID, nil
+		}
+	}
+
+	return "", fmt.Errorf("No such role with name %s", name)
+}
+
 func ExtractServiceIdentities(serviceIdents []string) ([]*api.ACLServiceIdentity, error) {
 	var out []*api.ACLServiceIdentity
 	for _, svcidRaw := range serviceIdents {

command/acl/role/create/role_create.go

@@ -0,0 +1,134 @@
+package rolecreate
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/command/acl"
+	aclhelpers "github.com/hashicorp/consul/command/acl"
+	"github.com/hashicorp/consul/command/flags"
+	"github.com/mitchellh/cli"
+)
+
+func New(ui cli.Ui) *cmd {
+	c := &cmd{UI: ui}
+	c.init()
+	return c
+}
+
+type cmd struct {
+	UI    cli.Ui
+	flags *flag.FlagSet
+	http  *flags.HTTPFlags
+	help  string
+
+	name          string
+	description   string
+	policyIDs     []string
+	policyNames   []string
+	serviceIdents []string
+
+	showMeta bool
+}
+
+func (c *cmd) init() {
+	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
+	c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that role metadata such "+
+		"as the content hash and raft indices should be shown for each entry")
+	c.flags.StringVar(&c.name, "name", "", "The new role's name. This flag is required.")
+	c.flags.StringVar(&c.description, "description", "", "A description of the role")
+	c.flags.Var((*flags.AppendSliceValue)(&c.policyIDs), "policy-id", "ID of a "+
+		"policy to use for this role. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.policyNames), "policy-name", "Name of a "+
+		"policy to use for this role. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.serviceIdents), "service-identity", "Name of a "+
+		"service identity to use for this role. May be specified multiple times. Format is "+
+		"the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...")
+	c.http = &flags.HTTPFlags{}
+	flags.Merge(c.flags, c.http.ClientFlags())
+	flags.Merge(c.flags, c.http.ServerFlags())
+	c.help = flags.Usage(help, c.flags)
+}
+
+func (c *cmd) Run(args []string) int {
+	if err := c.flags.Parse(args); err != nil {
+		return 1
+	}
+
+	if c.name == "" {
+		c.UI.Error(fmt.Sprintf("Missing require '-name' flag"))
+		c.UI.Error(c.Help())
+		return 1
+	}
+
+	if len(c.policyNames) == 0 && len(c.policyIDs) == 0 && len(c.serviceIdents) == 0 {
+		c.UI.Error(fmt.Sprintf("Cannot create a role without specifying -policy-name, -policy-id, or -service-identity at least once"))
+		return 1
+	}
+
+	client, err := c.http.APIClient()
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err))
+		return 1
+	}
+
+	newRole := &api.ACLRole{
+		Name:        c.name,
+		Description: c.description,
+	}
+
+	for _, policyName := range c.policyNames {
+		// We could resolve names to IDs here but there isn't any reason why its would be better
+		// than allowing the agent to do it.
+		newRole.Policies = append(newRole.Policies, &api.ACLRolePolicyLink{Name: policyName})
+	}
+
+	for _, policyID := range c.policyIDs {
+		policyID, err := acl.GetPolicyIDFromPartial(client, policyID)
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("Error resolving policy ID %s: %v", policyID, err))
+			return 1
+		}
+		newRole.Policies = append(newRole.Policies, &api.ACLRolePolicyLink{ID: policyID})
+	}
+
+	parsedServiceIdents, err := acl.ExtractServiceIdentities(c.serviceIdents)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+	newRole.ServiceIdentities = parsedServiceIdents
+
+	role, _, err := client.ACL().RoleCreate(newRole, nil)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Failed to create new role: %v", err))
+		return 1
+	}
+
+	aclhelpers.PrintRole(role, c.UI, c.showMeta)
+	return 0
+}
+
+func (c *cmd) Synopsis() string {
+	return synopsis
+}
+
+func (c *cmd) Help() string {
+	return flags.Usage(c.help, nil)
+}
+
+const synopsis = "Create an ACL Role"
+
+const help = `
+Usage: consul acl role create -name NAME [options]
+
+    Create a new role:
+
+        $ consul acl role create -name "new-role" \
+                                 -description "This is an example role" \
+                                 -policy-id b52fc3de-5 \
+                                 -policy-name "acl-replication" \
+                                 -service-identity "web" \
+                                 -service-identity "db:east,west"
+`

command/acl/role/create/role_create_test.go

@@ -0,0 +1,116 @@
+package rolecreate
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/consul/agent"
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/logger"
+	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/testrpc"
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRoleCreateCommand_noTabs(t *testing.T) {
+	t.Parallel()
+
+	if strings.ContainsRune(New(cli.NewMockUi()).Help(), '\t') {
+		t.Fatal("help has tabs")
+	}
+}
+
+func TestRoleCreateCommand(t *testing.T) {
+	t.Parallel()
+
+	testDir := testutil.TempDir(t, "acl")
+	defer os.RemoveAll(testDir)
+
+	a := agent.NewTestAgent(t, t.Name(), `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			master = "root"
+		}
+	}`)
+
+	a.Agent.LogWriter = logger.NewLogWriter(512)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	ui := cli.NewMockUi()
+	cmd := New(ui)
+
+	// Create a policy
+	client := a.Client()
+
+	policy, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+
+	// create with policy by name
+	{
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-name=role-with-policy-by-name",
+			"-description=test-role",
+			"-policy-name=" + policy.Name,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+	}
+
+	// create with policy by id
+	{
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-name=role-with-policy-by-id",
+			"-description=test-role",
+			"-policy-id=" + policy.ID,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+	}
+
+	// create with service identity
+	{
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-name=role-with-service-identity",
+			"-description=test-role",
+			"-service-identity=web",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+	}
+
+	// create with service identity scoped to 2 DCs
+	{
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-name=role-with-service-identity-in-2-dcs",
+			"-description=test-role",
+			"-service-identity=db:abc,xyz",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+	}
+}

command/acl/role/delete/role_delete.go

@@ -0,0 +1,91 @@
+package roledelete
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/hashicorp/consul/command/acl"
+	"github.com/hashicorp/consul/command/flags"
+	"github.com/mitchellh/cli"
+)
+
+func New(ui cli.Ui) *cmd {
+	c := &cmd{UI: ui}
+	c.init()
+	return c
+}
+
+type cmd struct {
+	UI    cli.Ui
+	flags *flag.FlagSet
+	http  *flags.HTTPFlags
+	help  string
+
+	roleID string
+}
+
+func (c *cmd) init() {
+	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
+	c.flags.StringVar(&c.roleID, "id", "", "The ID of the role to delete. "+
+		"It may be specified as a unique ID prefix but will error if the prefix "+
+		"matches multiple role IDs")
+	c.http = &flags.HTTPFlags{}
+	flags.Merge(c.flags, c.http.ClientFlags())
+	flags.Merge(c.flags, c.http.ServerFlags())
+	c.help = flags.Usage(help, c.flags)
+}
+
+func (c *cmd) Run(args []string) int {
+	if err := c.flags.Parse(args); err != nil {
+		return 1
+	}
+
+	if c.roleID == "" {
+		c.UI.Error(fmt.Sprintf("Must specify the -id parameter"))
+		return 1
+	}
+
+	client, err := c.http.APIClient()
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err))
+		return 1
+	}
+
+	roleID, err := acl.GetRoleIDFromPartial(client, c.roleID)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error determining role ID: %v", err))
+		return 1
+	}
+
+	if _, err := client.ACL().RoleDelete(roleID, nil); err != nil {
+		c.UI.Error(fmt.Sprintf("Error deleting role %q: %v", roleID, err))
+		return 1
+	}
+
+	c.UI.Info(fmt.Sprintf("Role %q deleted successfully", roleID))
+	return 0
+}
+
+func (c *cmd) Synopsis() string {
+	return synopsis
+}
+
+func (c *cmd) Help() string {
+	return flags.Usage(c.help, nil)
+}
+
+const synopsis = "Delete an ACL Role"
+const help = `
+Usage: consul acl role delete [options] -id ROLE
+
+    Deletes an ACL role by providing the ID or a unique ID prefix.
+
+    Delete by prefix:
+
+        $ consul acl role delete -id b6b85
+
+    Delete by full ID:
+
+        $ consul acl role delete -id b6b856da-5193-4e78-845a-7d61ca8371ba
+
+`

command/acl/role/delete/role_delete_test.go

@@ -0,0 +1,141 @@
+package roledelete
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/consul/agent"
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/logger"
+	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/testrpc"
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRoleDeleteCommand_noTabs(t *testing.T) {
+	t.Parallel()
+
+	if strings.ContainsRune(New(cli.NewMockUi()).Help(), '\t') {
+		t.Fatal("help has tabs")
+	}
+}
+
+func TestRoleDeleteCommand(t *testing.T) {
+	t.Parallel()
+
+	testDir := testutil.TempDir(t, "acl")
+	defer os.RemoveAll(testDir)
+
+	a := agent.NewTestAgent(t, t.Name(), `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			master = "root"
+		}
+	}`)
+
+	a.Agent.LogWriter = logger.NewLogWriter(512)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	client := a.Client()
+
+	t.Run("id required", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 1)
+		require.Contains(t, ui.ErrorWriter.String(), "Must specify the -id parameter")
+	})
+
+	t.Run("delete works", func(t *testing.T) {
+		// Create a role
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{
+				Name: "test-role-for-id-delete",
+				ServiceIdentities: []*api.ACLServiceIdentity{
+					&api.ACLServiceIdentity{
+						ServiceName: "fake",
+					},
+				},
+			},
+			&api.WriteOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-id=" + role.ID,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+
+		output := ui.OutputWriter.String()
+		require.Contains(t, output, fmt.Sprintf("deleted successfully"))
+		require.Contains(t, output, role.ID)
+
+		role, _, err = client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.Nil(t, role)
+	})
+
+	t.Run("delete works via prefixes", func(t *testing.T) {
+		// Create a role
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{
+				Name: "test-role-for-id-prefix-delete",
+				ServiceIdentities: []*api.ACLServiceIdentity{
+					&api.ACLServiceIdentity{
+						ServiceName: "fake",
+					},
+				},
+			},
+			&api.WriteOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-id=" + role.ID[0:5],
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+
+		output := ui.OutputWriter.String()
+		require.Contains(t, output, fmt.Sprintf("deleted successfully"))
+		require.Contains(t, output, role.ID)
+
+		role, _, err = client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.Nil(t, role)
+	})
+}

command/acl/role/list/role_list.go

@@ -0,0 +1,79 @@
+package rolelist
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/hashicorp/consul/command/acl"
+	"github.com/hashicorp/consul/command/flags"
+	"github.com/mitchellh/cli"
+)
+
+func New(ui cli.Ui) *cmd {
+	c := &cmd{UI: ui}
+	c.init()
+	return c
+}
+
+type cmd struct {
+	UI    cli.Ui
+	flags *flag.FlagSet
+	http  *flags.HTTPFlags
+	help  string
+
+	showMeta bool
+}
+
+func (c *cmd) init() {
+	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
+	c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that policy metadata such "+
+		"as the content hash and raft indices should be shown for each entry")
+
+	c.http = &flags.HTTPFlags{}
+	flags.Merge(c.flags, c.http.ClientFlags())
+	flags.Merge(c.flags, c.http.ServerFlags())
+	c.help = flags.Usage(help, c.flags)
+}
+
+func (c *cmd) Run(args []string) int {
+	if err := c.flags.Parse(args); err != nil {
+		return 1
+	}
+
+	client, err := c.http.APIClient()
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err))
+		return 1
+	}
+
+	roles, _, err := client.ACL().RoleList(nil)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Failed to retrieve the role list: %v", err))
+		return 1
+	}
+
+	for _, role := range roles {
+		acl.PrintRoleListEntry(role, c.UI, c.showMeta)
+	}
+
+	return 0
+}
+
+func (c *cmd) Synopsis() string {
+	return synopsis
+}
+
+func (c *cmd) Help() string {
+	return flags.Usage(c.help, nil)
+}
+
+const synopsis = "Lists ACL Roles"
+const help = `
+Usage: consul acl role list [options]
+
+    Lists all the ACL roles.
+
+    Example:
+
+        $ consul acl role list
+`

command/acl/role/list/role_list_test.go

@@ -0,0 +1,83 @@
+package rolelist
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/consul/agent"
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/logger"
+	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/testrpc"
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRoleListCommand_noTabs(t *testing.T) {
+	t.Parallel()
+
+	if strings.ContainsRune(New(cli.NewMockUi()).Help(), '\t') {
+		t.Fatal("help has tabs")
+	}
+}
+
+func TestRoleListCommand(t *testing.T) {
+	t.Parallel()
+	require := require.New(t)
+
+	testDir := testutil.TempDir(t, "acl")
+	defer os.RemoveAll(testDir)
+
+	a := agent.NewTestAgent(t, t.Name(), `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			master = "root"
+		}
+	}`)
+
+	a.Agent.LogWriter = logger.NewLogWriter(512)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	ui := cli.NewMockUi()
+	cmd := New(ui)
+
+	var roleIDs []string
+
+	// Create a couple roles to list
+	client := a.Client()
+	svcids := []*api.ACLServiceIdentity{
+		&api.ACLServiceIdentity{ServiceName: "fake"},
+	}
+	for i := 0; i < 5; i++ {
+		name := fmt.Sprintf("test-role-%d", i)
+
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{Name: name, ServiceIdentities: svcids},
+			&api.WriteOptions{Token: "root"},
+		)
+		roleIDs = append(roleIDs, role.ID)
+
+		require.NoError(err)
+	}
+
+	args := []string{
+		"-http-addr=" + a.HTTPAddr(),
+		"-token=root",
+	}
+
+	code := cmd.Run(args)
+	require.Equal(code, 0)
+	require.Empty(ui.ErrorWriter.String())
+	output := ui.OutputWriter.String()
+
+	for i, v := range roleIDs {
+		require.Contains(output, fmt.Sprintf("test-role-%d", i))
+		require.Contains(output, v)
+	}
+}

command/acl/role/read/role_read.go

@@ -0,0 +1,115 @@
+package roleread
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/command/acl"
+	"github.com/hashicorp/consul/command/flags"
+	"github.com/mitchellh/cli"
+)
+
+func New(ui cli.Ui) *cmd {
+	c := &cmd{UI: ui}
+	c.init()
+	return c
+}
+
+type cmd struct {
+	UI    cli.Ui
+	flags *flag.FlagSet
+	http  *flags.HTTPFlags
+	help  string
+
+	roleID   string
+	roleName string
+	showMeta bool
+}
+
+func (c *cmd) init() {
+	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
+	c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that role metadata such "+
+		"as the content hash and raft indices should be shown for each entry")
+	c.flags.StringVar(&c.roleID, "id", "", "The ID of the role to read. "+
+		"It may be specified as a unique ID prefix but will error if the prefix "+
+		"matches multiple policy IDs")
+	c.flags.StringVar(&c.roleName, "name", "", "The name of the role to read.")
+	c.http = &flags.HTTPFlags{}
+	flags.Merge(c.flags, c.http.ClientFlags())
+	flags.Merge(c.flags, c.http.ServerFlags())
+	c.help = flags.Usage(help, c.flags)
+}
+
+func (c *cmd) Run(args []string) int {
+	if err := c.flags.Parse(args); err != nil {
+		return 1
+	}
+
+	if c.roleID == "" && c.roleName == "" {
+		c.UI.Error(fmt.Sprintf("Must specify either the -id or -name parameters"))
+		return 1
+	}
+
+	client, err := c.http.APIClient()
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err))
+		return 1
+	}
+
+	var role *api.ACLRole
+
+	if c.roleID != "" {
+		roleID, err := acl.GetRoleIDFromPartial(client, c.roleID)
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("Error determining role ID: %v", err))
+			return 1
+		}
+		role, _, err = client.ACL().RoleRead(roleID, nil)
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("Error reading role %q: %v", roleID, err))
+			return 1
+		} else if role == nil {
+			c.UI.Error(fmt.Sprintf("Role not found with ID %q", roleID))
+			return 1
+		}
+
+	} else {
+		role, _, err = client.ACL().RoleReadByName(c.roleName, nil)
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("Error reading role %q: %v", c.roleName, err))
+			return 1
+		} else if role == nil {
+			c.UI.Error(fmt.Sprintf("Role not found with name %q", c.roleName))
+			return 1
+		}
+	}
+
+	acl.PrintRole(role, c.UI, c.showMeta)
+	return 0
+}
+
+func (c *cmd) Synopsis() string {
+	return synopsis
+}
+
+func (c *cmd) Help() string {
+	return flags.Usage(c.help, nil)
+}
+
+const synopsis = "Read an ACL Role"
+const help = `
+Usage: consul acl role read [options] ROLE
+
+    This command will retrieve and print out the details
+    of a single role.
+
+    Read:
+
+        $ consul acl role read -id fdabbcb5-9de5-4b1a-961f-77214ae88cba
+
+    Read by name:
+
+        $ consul acl role read -name my-policy
+
+`

command/acl/role/read/role_read_test.go

@@ -0,0 +1,194 @@
+package roleread
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/consul/agent"
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/logger"
+	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/testrpc"
+	"github.com/hashicorp/go-uuid"
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRoleReadCommand_noTabs(t *testing.T) {
+	t.Parallel()
+
+	if strings.ContainsRune(New(cli.NewMockUi()).Help(), '\t') {
+		t.Fatal("help has tabs")
+	}
+}
+
+func TestRoleReadCommand(t *testing.T) {
+	t.Parallel()
+
+	testDir := testutil.TempDir(t, "acl")
+	defer os.RemoveAll(testDir)
+
+	a := agent.NewTestAgent(t, t.Name(), `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			master = "root"
+		}
+	}`)
+
+	a.Agent.LogWriter = logger.NewLogWriter(512)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	client := a.Client()
+
+	t.Run("id or name required", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 1)
+		require.Contains(t, ui.ErrorWriter.String(), "Must specify either the -id or -name parameters")
+	})
+
+	t.Run("read by id not found", func(t *testing.T) {
+		fakeID, err := uuid.GenerateUUID()
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-id=" + fakeID,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 1)
+		require.Contains(t, ui.ErrorWriter.String(), "Role not found with ID")
+	})
+
+	t.Run("read by name not found", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-name=blah",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 1)
+		require.Contains(t, ui.ErrorWriter.String(), "Role not found with name")
+	})
+
+	t.Run("read by id", func(t *testing.T) {
+		// create a role
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{
+				Name: "test-role-by-id",
+				ServiceIdentities: []*api.ACLServiceIdentity{
+					&api.ACLServiceIdentity{
+						ServiceName: "fake",
+					},
+				},
+			},
+			&api.WriteOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-id=" + role.ID,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+
+		output := ui.OutputWriter.String()
+		require.Contains(t, output, fmt.Sprintf("test-role"))
+		require.Contains(t, output, role.ID)
+	})
+
+	t.Run("read by id prefix", func(t *testing.T) {
+		// create a role
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{
+				Name: "test-role-by-id-prefix",
+				ServiceIdentities: []*api.ACLServiceIdentity{
+					&api.ACLServiceIdentity{
+						ServiceName: "fake",
+					},
+				},
+			},
+			&api.WriteOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-id=" + role.ID[0:5],
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+
+		output := ui.OutputWriter.String()
+		require.Contains(t, output, fmt.Sprintf("test-role"))
+		require.Contains(t, output, role.ID)
+	})
+
+	t.Run("read by name", func(t *testing.T) {
+		// create a role
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{
+				Name: "test-role-by-name",
+				ServiceIdentities: []*api.ACLServiceIdentity{
+					&api.ACLServiceIdentity{
+						ServiceName: "fake",
+					},
+				},
+			},
+			&api.WriteOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-token=root",
+			"-name=" + role.Name,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0)
+		require.Empty(t, ui.ErrorWriter.String())
+
+		output := ui.OutputWriter.String()
+		require.Contains(t, output, fmt.Sprintf("test-role"))
+		require.Contains(t, output, role.ID)
+	})
+}

command/acl/role/role.go

@@ -0,0 +1,56 @@
+package role
+
+import (
+	"github.com/hashicorp/consul/command/flags"
+	"github.com/mitchellh/cli"
+)
+
+func New() *cmd {
+	return &cmd{}
+}
+
+type cmd struct{}
+
+func (c *cmd) Run(args []string) int {
+	return cli.RunResultHelp
+}
+
+func (c *cmd) Synopsis() string {
+	return synopsis
+}
+
+func (c *cmd) Help() string {
+	return flags.Usage(help, nil)
+}
+
+const synopsis = "Manage Consul's ACL Roles"
+const help = `
+Usage: consul acl role <subcommand> [options] [args]
+
+  This command has subcommands for managing Consul's ACL Roles.
+  Here are some simple examples, and more detailed examples are available
+  in the subcommands or the documentation.
+
+  Create a new ACL Role:
+
+      $ consul acl role create -name "new-role" \
+                               -description "This is an example role" \
+                               -policy-id 06acc965
+  List all roles:
+
+      $ consul acl role list
+
+  Update a role:
+
+      $ consul acl role update -name "other-role" -datacenter "dc1"
+
+  Read a role:
+
+    $ consul acl role read -id 0479e93e-091c-4475-9b06-79a004765c24
+
+  Delete a role
+
+    $ consul acl role delete -name "my-role"
+
+  For more examples, ask for subcommand help or view the documentation.
+`

command/acl/role/update/role_update.go

@@ -0,0 +1,225 @@
+package roleupdate
+
+import (
+	"flag"
+	"fmt"
+
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/command/acl"
+	"github.com/hashicorp/consul/command/flags"
+	"github.com/mitchellh/cli"
+)
+
+func New(ui cli.Ui) *cmd {
+	c := &cmd{UI: ui}
+	c.init()
+	return c
+}
+
+type cmd struct {
+	UI    cli.Ui
+	flags *flag.FlagSet
+	http  *flags.HTTPFlags
+	help  string
+
+	roleID        string
+	name          string
+	description   string
+	policyIDs     []string
+	policyNames   []string
+	serviceIdents []string
+
+	noMerge  bool
+	showMeta bool
+}
+
+func (c *cmd) init() {
+	c.flags = flag.NewFlagSet("", flag.ContinueOnError)
+	c.flags.BoolVar(&c.showMeta, "meta", false, "Indicates that role metadata such "+
+		"as the content hash and raft indices should be shown for each entry")
+	c.flags.StringVar(&c.roleID, "id", "", "The ID of the role to update. "+
+		"It may be specified as a unique ID prefix but will error if the prefix "+
+		"matches multiple role IDs")
+	c.flags.StringVar(&c.name, "name", "", "The role name.")
+	c.flags.StringVar(&c.description, "description", "", "A description of the role")
+	c.flags.Var((*flags.AppendSliceValue)(&c.policyIDs), "policy-id", "ID of a "+
+		"policy to use for this role. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.policyNames), "policy-name", "Name of a "+
+		"policy to use for this role. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.serviceIdents), "service-identity", "Name of a "+
+		"service identity to use for this role. May be specified multiple times. Format is "+
+		"the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...")
+	c.flags.BoolVar(&c.noMerge, "no-merge", false, "Do not merge the current role "+
+		"information with what is provided to the command. Instead overwrite all fields "+
+		"with the exception of the role ID which is immutable.")
+
+	c.http = &flags.HTTPFlags{}
+	flags.Merge(c.flags, c.http.ClientFlags())
+	flags.Merge(c.flags, c.http.ServerFlags())
+	c.help = flags.Usage(help, c.flags)
+}
+
+func (c *cmd) Run(args []string) int {
+	if err := c.flags.Parse(args); err != nil {
+		return 1
+	}
+
+	if c.roleID == "" {
+		c.UI.Error(fmt.Sprintf("Cannot update a role without specifying the -id parameter"))
+		return 1
+	}
+
+	client, err := c.http.APIClient()
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err))
+		return 1
+	}
+
+	roleID, err := acl.GetRoleIDFromPartial(client, c.roleID)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error determining role ID: %v", err))
+		return 1
+	}
+
+	parsedServiceIdents, err := acl.ExtractServiceIdentities(c.serviceIdents)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
+	// Read the current role in both cases so we can fail better if not found.
+	currentRole, _, err := client.ACL().RoleRead(roleID, nil)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error when retrieving current role: %v", err))
+		return 1
+	} else if currentRole == nil {
+		c.UI.Error(fmt.Sprintf("Role not found with ID %q", roleID))
+		return 1
+	}
+
+	var role *api.ACLRole
+	if c.noMerge {
+		role = &api.ACLRole{
+			ID:                c.roleID,
+			Name:              c.name,
+			Description:       c.description,
+			ServiceIdentities: parsedServiceIdents,
+		}
+
+		for _, policyName := range c.policyNames {
+			// We could resolve names to IDs here but there isn't any reason
+			// why its would be better than allowing the agent to do it.
+			role.Policies = append(role.Policies, &api.ACLRolePolicyLink{Name: policyName})
+		}
+
+		for _, policyID := range c.policyIDs {
+			policyID, err := acl.GetPolicyIDFromPartial(client, policyID)
+			if err != nil {
+				c.UI.Error(fmt.Sprintf("Error resolving policy ID %s: %v", policyID, err))
+				return 1
+			}
+			role.Policies = append(role.Policies, &api.ACLRolePolicyLink{ID: policyID})
+		}
+	} else {
+		role = currentRole
+
+		if c.name != "" {
+			role.Name = c.name
+		}
+		if c.description != "" {
+			role.Description = c.description
+		}
+
+		for _, policyName := range c.policyNames {
+			found := false
+			for _, link := range role.Policies {
+				if link.Name == policyName {
+					found = true
+					break
+				}
+			}
+
+			if !found {
+				// We could resolve names to IDs here but there isn't any
+				// reason why its would be better than allowing the agent to do
+				// it.
+				role.Policies = append(role.Policies, &api.ACLRolePolicyLink{Name: policyName})
+			}
+		}
+
+		for _, policyID := range c.policyIDs {
+			policyID, err := acl.GetPolicyIDFromPartial(client, policyID)
+			if err != nil {
+				c.UI.Error(fmt.Sprintf("Error resolving policy ID %s: %v", policyID, err))
+				return 1
+			}
+			found := false
+
+			for _, link := range role.Policies {
+				if link.ID == policyID {
+					found = true
+					break
+				}
+			}
+
+			if !found {
+				role.Policies = append(role.Policies, &api.ACLRolePolicyLink{ID: policyID})
+			}
+		}
+
+		for _, svcid := range parsedServiceIdents {
+			found := -1
+			for i, link := range role.ServiceIdentities {
+				if link.ServiceName == svcid.ServiceName {
+					found = i
+					break
+				}
+			}
+
+			if found != -1 {
+				role.ServiceIdentities[found] = svcid
+			} else {
+				role.ServiceIdentities = append(role.ServiceIdentities, svcid)
+			}
+		}
+	}
+
+	role, _, err = client.ACL().RoleUpdate(role, nil)
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Error updating role %q: %v", roleID, err))
+		return 1
+	}
+
+	c.UI.Info(fmt.Sprintf("Role updated successfully"))
+	acl.PrintRole(role, c.UI, c.showMeta)
+	return 0
+}
+
+func (c *cmd) Synopsis() string {
+	return synopsis
+}
+
+func (c *cmd) Help() string {
+	return flags.Usage(c.help, nil)
+}
+
+const synopsis = "Update an ACL Role"
+const help = `
+Usage: consul acl role update [options]
+
+  Updates a role. By default it will merge the role information with its
+  current state so that you do not have to provide all parameters. This
+  behavior can be disabled by passing -no-merge.
+
+  Rename the Role:
+
+          $ consul acl role update -id abcd -name "better-name"
+
+  Update all editable fields of the role:
+
+          $ consul acl role update -id abcd \
+                                   -name "better-name" \
+                                   -description "replication" \
+                                   -policy-name "token-replication" \
+                                   -service-identity "web"
+`

command/acl/role/update/role_update_test.go

@@ -0,0 +1,398 @@
+package roleupdate
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/hashicorp/consul/agent"
+	"github.com/hashicorp/consul/api"
+	"github.com/hashicorp/consul/logger"
+	"github.com/hashicorp/consul/sdk/testutil"
+	"github.com/hashicorp/consul/testrpc"
+	"github.com/mitchellh/cli"
+	"github.com/stretchr/testify/require"
+
+	uuid "github.com/hashicorp/go-uuid"
+)
+
+func TestRoleUpdateCommand_noTabs(t *testing.T) {
+	t.Parallel()
+
+	if strings.ContainsRune(New(cli.NewMockUi()).Help(), '\t') {
+		t.Fatal("help has tabs")
+	}
+}
+
+func TestRoleUpdateCommand(t *testing.T) {
+	t.Parallel()
+
+	testDir := testutil.TempDir(t, "acl")
+	defer os.RemoveAll(testDir)
+
+	a := agent.NewTestAgent(t, t.Name(), `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			master = "root"
+		}
+	}`)
+
+	a.Agent.LogWriter = logger.NewLogWriter(512)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	client := a.Client()
+
+	// Create 2 policies
+	policy1, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy1"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+	policy2, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy2"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+
+	// create a role
+	role, _, err := client.ACL().RoleCreate(
+		&api.ACLRole{
+			Name: "test-role",
+			ServiceIdentities: []*api.ACLServiceIdentity{
+				&api.ACLServiceIdentity{
+					ServiceName: "fake",
+				},
+			},
+		},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+
+	t.Run("update a role that does not exist", func(t *testing.T) {
+		fakeID, err := uuid.GenerateUUID()
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + fakeID,
+			"-token=root",
+			"-policy-name=" + policy1.Name,
+			"-description=test role edited",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 1)
+		require.Contains(t, ui.ErrorWriter.String(), "Role not found with ID")
+	})
+
+	t.Run("update with policy by name", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-token=root",
+			"-policy-name=" + policy1.Name,
+			"-description=test role edited",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "test role edited", role.Description)
+		require.Len(t, role.Policies, 1)
+		require.Len(t, role.ServiceIdentities, 1)
+	})
+
+	t.Run("update with policy by id", func(t *testing.T) {
+		// also update with no description shouldn't delete the current
+		// description
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-token=root",
+			"-policy-id=" + policy2.ID,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "test role edited", role.Description)
+		require.Len(t, role.Policies, 2)
+		require.Len(t, role.ServiceIdentities, 1)
+	})
+
+	t.Run("update with service identity", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-token=root",
+			"-service-identity=web",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "test role edited", role.Description)
+		require.Len(t, role.Policies, 2)
+		require.Len(t, role.ServiceIdentities, 2)
+	})
+
+	t.Run("update with service identity scoped to 2 DCs", func(t *testing.T) {
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-token=root",
+			"-service-identity=db:abc,xyz",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "test role edited", role.Description)
+		require.Len(t, role.Policies, 2)
+		require.Len(t, role.ServiceIdentities, 3)
+	})
+}
+
+func TestRoleUpdateCommand_noMerge(t *testing.T) {
+	t.Parallel()
+
+	testDir := testutil.TempDir(t, "acl")
+	defer os.RemoveAll(testDir)
+
+	a := agent.NewTestAgent(t, t.Name(), `
+	primary_datacenter = "dc1"
+	acl {
+		enabled = true
+		tokens {
+			master = "root"
+		}
+	}`)
+
+	a.Agent.LogWriter = logger.NewLogWriter(512)
+
+	defer a.Shutdown()
+	testrpc.WaitForLeader(t, a.RPC, "dc1")
+
+	client := a.Client()
+
+	// Create 3 policies
+	policy1, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy1"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+	policy2, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy2"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+	policy3, _, err := client.ACL().PolicyCreate(
+		&api.ACLPolicy{Name: "test-policy3"},
+		&api.WriteOptions{Token: "root"},
+	)
+	require.NoError(t, err)
+
+	// create a role
+	createRole := func(t *testing.T) *api.ACLRole {
+		roleUnq, err := uuid.GenerateUUID()
+		require.NoError(t, err)
+
+		role, _, err := client.ACL().RoleCreate(
+			&api.ACLRole{
+				Name:        "test-role-" + roleUnq,
+				Description: "original description",
+				ServiceIdentities: []*api.ACLServiceIdentity{
+					&api.ACLServiceIdentity{
+						ServiceName: "fake",
+					},
+				},
+				Policies: []*api.ACLRolePolicyLink{
+					&api.ACLRolePolicyLink{
+						ID: policy3.ID,
+					},
+				},
+			},
+			&api.WriteOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		return role
+	}
+
+	t.Run("update a role that does not exist", func(t *testing.T) {
+		fakeID, err := uuid.GenerateUUID()
+		require.NoError(t, err)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + fakeID,
+			"-token=root",
+			"-policy-name=" + policy1.Name,
+			"-no-merge",
+			"-description=test role edited",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 1)
+		require.Contains(t, ui.ErrorWriter.String(), "Role not found with ID")
+	})
+
+	t.Run("update with policy by name", func(t *testing.T) {
+		role := createRole(t)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-name=" + role.Name,
+			"-token=root",
+			"-no-merge",
+			"-policy-name=" + policy1.Name,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "", role.Description)
+		require.Len(t, role.Policies, 1)
+		require.Len(t, role.ServiceIdentities, 0)
+	})
+
+	t.Run("update with policy by id", func(t *testing.T) {
+		role := createRole(t)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-name=" + role.Name,
+			"-token=root",
+			"-no-merge",
+			"-policy-id=" + policy2.ID,
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "", role.Description)
+		require.Len(t, role.Policies, 1)
+		require.Len(t, role.ServiceIdentities, 0)
+	})
+
+	t.Run("update with service identity", func(t *testing.T) {
+		role := createRole(t)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-name=" + role.Name,
+			"-token=root",
+			"-no-merge",
+			"-service-identity=web",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "", role.Description)
+		require.Len(t, role.Policies, 0)
+		require.Len(t, role.ServiceIdentities, 1)
+	})
+
+	t.Run("update with service identity scoped to 2 DCs", func(t *testing.T) {
+		role := createRole(t)
+
+		ui := cli.NewMockUi()
+		cmd := New(ui)
+		args := []string{
+			"-http-addr=" + a.HTTPAddr(),
+			"-id=" + role.ID,
+			"-name=" + role.Name,
+			"-token=root",
+			"-no-merge",
+			"-service-identity=db:abc,xyz",
+		}
+
+		code := cmd.Run(args)
+		require.Equal(t, code, 0, "err: %s", ui.ErrorWriter.String())
+		require.Empty(t, ui.ErrorWriter.String())
+
+		role, _, err := client.ACL().RoleRead(
+			role.ID,
+			&api.QueryOptions{Token: "root"},
+		)
+		require.NoError(t, err)
+		require.NotNil(t, role)
+		require.Equal(t, "", role.Description)
+		require.Len(t, role.Policies, 0)
+		require.Len(t, role.ServiceIdentities, 1)
+	})
+}

command/acl/token/create/token_create.go

@@ -25,6 +25,8 @@ type cmd struct {
 
 	policyIDs     []string
 	policyNames   []string
+	roleIDs       []string
+	roleNames     []string
 	serviceIdents []string
 	expirationTTL time.Duration
 	description   string
@@ -42,6 +44,10 @@ func (c *cmd) init() {
 		"policy to use for this token. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.policyNames), "policy-name", "Name of a "+
 		"policy to use for this token. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.roleIDs), "role-id", "ID of a "+
+		"role to use for this token. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.roleNames), "role-name", "Name of a "+
+		"role to use for this token. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.serviceIdents), "service-identity", "Name of a "+
 		"service identity to use for this token. May be specified multiple times. Format is "+
 		"the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...")
@@ -59,8 +65,9 @@ func (c *cmd) Run(args []string) int {
 	}
 
 	if len(c.policyNames) == 0 && len(c.policyIDs) == 0 &&
+		len(c.roleNames) == 0 && len(c.roleIDs) == 0 &&
 		len(c.serviceIdents) == 0 {
-		c.UI.Error(fmt.Sprintf("Cannot create a token without specifying -policy-name, -policy-id, or -service-identity at least once"))
+		c.UI.Error(fmt.Sprintf("Cannot create a token without specifying -policy-name, -policy-id, -role-name, -role-id, or -service-identity at least once"))
 		return 1
 	}
 
@@ -100,6 +107,21 @@ func (c *cmd) Run(args []string) int {
 		newToken.Policies = append(newToken.Policies, &api.ACLTokenPolicyLink{ID: policyID})
 	}
 
+	for _, roleName := range c.roleNames {
+		// We could resolve names to IDs here but there isn't any reason why its would be better
+		// than allowing the agent to do it.
+		newToken.Roles = append(newToken.Roles, &api.ACLTokenRoleLink{Name: roleName})
+	}
+
+	for _, roleID := range c.roleIDs {
+		roleID, err := acl.GetRoleIDFromPartial(client, roleID)
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("Error resolving role ID %s: %v", roleID, err))
+			return 1
+		}
+		newToken.Roles = append(newToken.Roles, &api.ACLTokenRoleLink{ID: roleID})
+	}
+
 	token, _, err := client.ACL().TokenCreate(newToken, nil)
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("Failed to create new token: %v", err))
@@ -130,8 +152,9 @@ Usage: consul acl token create [options]
 
           $ consul acl token create -description "Replication token" \
                                     -policy-id b52fc3de-5 \
-                                    -policy-name "acl-replication"
                                     -policy-name "acl-replication" \
+                                    -role-id c630d4ef-6 \
+                                    -role-name "db-updater" \
                                     -service-identity "web" \
                                     -service-identity "db:east,west"
 `

command/acl/token/update/token_update.go

@@ -25,9 +25,12 @@ type cmd struct {
 	tokenID            string
 	policyIDs          []string
 	policyNames        []string
+	roleIDs            []string
+	roleNames          []string
 	serviceIdents      []string
 	description        string
 	mergePolicies      bool
+	mergeRoles         bool
 	mergeServiceIdents bool
 	showMeta           bool
 	upgradeLegacy      bool
@@ -39,6 +42,8 @@ func (c *cmd) init() {
 		"as the content hash and raft indices should be shown for each entry")
 	c.flags.BoolVar(&c.mergePolicies, "merge-policies", false, "Merge the new policies "+
 		"with the existing policies")
+	c.flags.BoolVar(&c.mergeRoles, "merge-roles", false, "Merge the new roles "+
+		"with the existing roles")
 	c.flags.BoolVar(&c.mergeServiceIdents, "merge-service-identities", false, "Merge the new service identities "+
 		"with the existing service identities")
 	c.flags.StringVar(&c.tokenID, "id", "", "The Accessor ID of the token to read. "+
@@ -49,6 +54,10 @@ func (c *cmd) init() {
 		"policy to use for this token. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.policyNames), "policy-name", "Name of a "+
 		"policy to use for this token. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.roleIDs), "role-id", "ID of a "+
+		"role to use for this token. May be specified multiple times")
+	c.flags.Var((*flags.AppendSliceValue)(&c.roleNames), "role-name", "Name of a "+
+		"role to use for this token. May be specified multiple times")
 	c.flags.Var((*flags.AppendSliceValue)(&c.serviceIdents), "service-identity", "Name of a "+
 		"service identity to use for this token. May be specified multiple times. Format is "+
 		"the SERVICENAME or SERVICENAME:DATACENTER1,DATACENTER2,...")
@@ -175,6 +184,61 @@ func (c *cmd) Run(args []string) int {
 		}
 	}
 
+	if c.mergeRoles {
+		for _, roleName := range c.roleNames {
+			found := false
+			for _, link := range token.Roles {
+				if link.Name == roleName {
+					found = true
+					break
+				}
+			}
+
+			if !found {
+				// We could resolve names to IDs here but there isn't any reason why its would be better
+				// than allowing the agent to do it.
+				token.Roles = append(token.Roles, &api.ACLTokenRoleLink{Name: roleName})
+			}
+		}
+
+		for _, roleID := range c.roleIDs {
+			roleID, err := acl.GetRoleIDFromPartial(client, roleID)
+			if err != nil {
+				c.UI.Error(fmt.Sprintf("Error resolving role ID %s: %v", roleID, err))
+				return 1
+			}
+			found := false
+
+			for _, link := range token.Roles {
+				if link.ID == roleID {
+					found = true
+					break
+				}
+			}
+
+			if !found {
+				token.Roles = append(token.Roles, &api.ACLTokenRoleLink{Name: roleID})
+			}
+		}
+	} else {
+		token.Roles = nil
+
+		for _, roleName := range c.roleNames {
+			// We could resolve names to IDs here but there isn't any reason why its would be better
+			// than allowing the agent to do it.
+			token.Roles = append(token.Roles, &api.ACLTokenRoleLink{Name: roleName})
+		}
+
+		for _, roleID := range c.roleIDs {
+			roleID, err := acl.GetRoleIDFromPartial(client, roleID)
+			if err != nil {
+				c.UI.Error(fmt.Sprintf("Error resolving role ID %s: %v", roleID, err))
+				return 1
+			}
+			token.Roles = append(token.Roles, &api.ACLTokenRoleLink{ID: roleID})
+		}
+	}
+
 	if c.mergeServiceIdents {
 		for _, svcid := range parsedServiceIdents {
 			found := -1
@@ -229,5 +293,6 @@ Usage: consul acl token update [options]
 
         $ consul acl token update -id abcd \
                                   -description "replication" \
-                                  -policy-name "token-replication"
+                                  -policy-name "token-replication" \
+                                  -role-name "db-updater"
 `

command/commands_oss.go

@@ -10,6 +10,12 @@ import (
 	aclplist "github.com/hashicorp/consul/command/acl/policy/list"
 	aclpread "github.com/hashicorp/consul/command/acl/policy/read"
 	aclpupdate "github.com/hashicorp/consul/command/acl/policy/update"
+	aclrole "github.com/hashicorp/consul/command/acl/role"
+	aclrcreate "github.com/hashicorp/consul/command/acl/role/create"
+	aclrdelete "github.com/hashicorp/consul/command/acl/role/delete"
+	aclrlist "github.com/hashicorp/consul/command/acl/role/list"
+	aclrread "github.com/hashicorp/consul/command/acl/role/read"
+	aclrupdate "github.com/hashicorp/consul/command/acl/role/update"
 	aclrules "github.com/hashicorp/consul/command/acl/rules"
 	acltoken "github.com/hashicorp/consul/command/acl/token"
 	acltclone "github.com/hashicorp/consul/command/acl/token/clone"
@@ -106,6 +112,12 @@ func init() {
 	Register("acl token read", func(ui cli.Ui) (cli.Command, error) { return acltread.New(ui), nil })
 	Register("acl token update", func(ui cli.Ui) (cli.Command, error) { return acltupdate.New(ui), nil })
 	Register("acl token delete", func(ui cli.Ui) (cli.Command, error) { return acltdelete.New(ui), nil })
+	Register("acl role", func(cli.Ui) (cli.Command, error) { return aclrole.New(), nil })
+	Register("acl role create", func(ui cli.Ui) (cli.Command, error) { return aclrcreate.New(ui), nil })
+	Register("acl role list", func(ui cli.Ui) (cli.Command, error) { return aclrlist.New(ui), nil })
+	Register("acl role read", func(ui cli.Ui) (cli.Command, error) { return aclrread.New(ui), nil })
+	Register("acl role update", func(ui cli.Ui) (cli.Command, error) { return aclrupdate.New(ui), nil })
+	Register("acl role delete", func(ui cli.Ui) (cli.Command, error) { return aclrdelete.New(ui), nil })
 	Register("agent", func(ui cli.Ui) (cli.Command, error) {
 		return agent.New(ui, rev, ver, verPre, verHuman, make(chan struct{})), nil
 	})