From 43df402e512029897675950b2067f25b61d3d22f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 19 Mar 2021 16:07:55 -0400 Subject: [PATCH] state: convert acl-roles.policies index to new pattern --- agent/consul/state/acl.go | 86 +++++-------------- agent/consul/state/acl_events.go | 3 +- agent/consul/state/acl_oss.go | 23 ++++- agent/consul/state/acl_oss_test.go | 8 +- agent/consul/state/acl_schema.go | 5 +- .../testdata/TestStateStoreSchema.golden | 2 +- 6 files changed, 50 insertions(+), 77 deletions(-) diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index e618ea25fa..2ed6b4926a 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -113,57 +113,6 @@ func (s *TokenRolesIndex) PrefixFromArgs(args ...interface{}) ([]byte, error) { 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 } @@ -544,22 +493,21 @@ func fixupTokenRoleLinks(tx ReadTxn, original *structs.ACLToken) (*structs.ACLTo func resolveRolePolicyLinks(tx *txn, role *structs.ACLRole, allowMissing bool) error { for linkIndex, link := range role.Policies { - if link.ID != "" { - policy, err := getPolicyWithTxn(tx, nil, link.ID, aclPolicyGetByID, &role.EnterpriseMeta) - - 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 { + if link.ID == "" { return fmt.Errorf("Encountered a Role with policies linked by Name in the state store") } + + policy, err := getPolicyWithTxn(tx, nil, link.ID, aclPolicyGetByID, &role.EnterpriseMeta) + 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) + } } return nil } @@ -1495,8 +1443,14 @@ func (s *Store) ACLRoleList(ws memdb.WatchSet, policy string, entMeta *structs.E var iter memdb.ResultIterator var err error + // TODO: accept non-pointer value + if entMeta == nil { + entMeta = structs.DefaultEnterpriseMeta() + } + if policy != "" { - iter, err = aclRoleListByPolicy(tx, policy, entMeta) + q := Query{Value: policy, EnterpriseMeta: *entMeta} + iter, err = tx.Get(tableACLRoles, indexPolicies, q) } else { iter, err = tx.Get(tableACLRoles, indexName+"_prefix", entMeta) } diff --git a/agent/consul/state/acl_events.go b/agent/consul/state/acl_events.go index aea61ae543..4c49711c4c 100644 --- a/agent/consul/state/acl_events.go +++ b/agent/consul/state/acl_events.go @@ -36,7 +36,8 @@ func aclChangeUnsubscribeEvent(tx ReadTxn, changes Changes) ([]stream.Event, err } secretIDs = appendSecretIDsFromTokenIterator(secretIDs, tokens) - roles, err := aclRoleListByPolicy(tx, policy.ID, &policy.EnterpriseMeta) + q := Query{Value: policy.ID, EnterpriseMeta: policy.EnterpriseMeta} + roles, err := tx.Get(tableACLRoles, indexPolicies, q) if err != nil { return nil, err } diff --git a/agent/consul/state/acl_oss.go b/agent/consul/state/acl_oss.go index 92abc2eae5..e658a5e624 100644 --- a/agent/consul/state/acl_oss.go +++ b/agent/consul/state/acl_oss.go @@ -53,6 +53,25 @@ func indexNameFromACLRole(raw interface{}) ([]byte, error) { return b.Bytes(), nil } +func multiIndexPolicyFromACLRole(raw interface{}) ([][]byte, error) { + role, ok := raw.(*structs.ACLRole) + if !ok { + return nil, fmt.Errorf("unexpected type %T for structs.ACLRole index", raw) + } + + count := len(role.Policies) + if count == 0 { + return nil, errMissingValueForIndex + } + + vals := make([][]byte, 0, count) + for _, link := range role.Policies { + vals = append(vals, []byte(strings.ToLower(link.ID)+"\x00")) + } + + return vals, nil +} + func aclPolicyGetByID(tx ReadTxn, id string, _ *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { return tx.FirstWatch(tableACLPolicies, indexID, id) } @@ -174,10 +193,6 @@ func aclRoleGetByID(tx ReadTxn, id string, _ *structs.EnterpriseMeta) (<-chan st return tx.FirstWatch(tableACLRoles, indexID, id) } -func aclRoleListByPolicy(tx ReadTxn, policy string, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { - return tx.Get(tableACLRoles, indexPolicies, policy) -} - func aclRoleDeleteWithRole(tx *txn, role *structs.ACLRole, idx uint64) error { // remove the role if err := tx.Delete(tableACLRoles, role); err != nil { diff --git a/agent/consul/state/acl_oss_test.go b/agent/consul/state/acl_oss_test.go index 39cb3846fb..89a504986a 100644 --- a/agent/consul/state/acl_oss_test.go +++ b/agent/consul/state/acl_oss_test.go @@ -66,14 +66,14 @@ func testIndexerTableACLRoles() map[string]indexerTestCase { }, indexPolicies: { read: indexValue{ - source: "PolicyId1", - expected: []byte("PolicyId1\x00"), + source: Query{Value: "PolicyId1"}, + expected: []byte("policyid1\x00"), }, writeMulti: indexValueMulti{ source: obj, expected: [][]byte{ - []byte("PolicyId1\x00"), - []byte("PolicyId2\x00"), + []byte("policyid1\x00"), + []byte("policyid2\x00"), }, }, }, diff --git a/agent/consul/state/acl_schema.go b/agent/consul/state/acl_schema.go index c692596bca..463a8b9c3d 100644 --- a/agent/consul/state/acl_schema.go +++ b/agent/consul/state/acl_schema.go @@ -162,7 +162,10 @@ func rolesTableSchema() *memdb.TableSchema { // Need to allow missing for the anonymous token AllowMissing: true, Unique: false, - Indexer: &RolePoliciesIndex{}, + Indexer: indexerMulti{ + readIndex: readIndex(indexFromQuery), + writeIndexMulti: writeIndexMulti(multiIndexPolicyFromACLRole), + }, }, }, } diff --git a/agent/consul/state/testdata/TestStateStoreSchema.golden b/agent/consul/state/testdata/TestStateStoreSchema.golden index 227f947e26..cddd96f7f6 100644 --- a/agent/consul/state/testdata/TestStateStoreSchema.golden +++ b/agent/consul/state/testdata/TestStateStoreSchema.golden @@ -20,7 +20,7 @@ table=acl-roles index=name unique indexer=github.com/hashicorp/consul/agent/consul/state.indexerSingleWithPrefix readIndex=github.com/hashicorp/consul/agent/consul/state.indexFromQuery writeIndex=github.com/hashicorp/consul/agent/consul/state.indexNameFromACLRole prefixIndex=github.com/hashicorp/consul/agent/consul/state.prefixIndexFromQuery index=policies allow-missing - indexer=github.com/hashicorp/consul/agent/consul/state.RolePoliciesIndex + indexer=github.com/hashicorp/consul/agent/consul/state.indexerMulti readIndex=github.com/hashicorp/consul/agent/consul/state.indexFromQuery writeIndexMulti=github.com/hashicorp/consul/agent/consul/state.multiIndexPolicyFromACLRole table=acl-tokens index=accessor unique allow-missing