From a058c31ead34ba4817a5eee20a54d3564705454d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 16 Mar 2021 16:39:22 -0400 Subject: [PATCH 1/5] state: use constants for acl-roles table and indexes --- agent/consul/state/acl.go | 4 ++-- agent/consul/state/acl_events.go | 2 +- agent/consul/state/acl_oss.go | 18 +++++++++--------- agent/consul/state/acl_test.go | 2 +- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 811bfe3ff7..bebe9996c1 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -237,7 +237,7 @@ func (s *Restore) ACLPolicy(policy *structs.ACLPolicy) error { // ACLRoles is used when saving a snapshot func (s *Snapshot) ACLRoles() (memdb.ResultIterator, error) { - iter, err := s.tx.Get("acl-roles", "id") + iter, err := s.tx.Get(tableACLRoles, indexID) if err != nil { return nil, err } @@ -1440,7 +1440,7 @@ func (s *Store) ACLRoleBatchGet(ws memdb.WatchSet, ids []string) (uint64, struct } } - idx := maxIndexTxn(tx, "acl-roles") + idx := maxIndexTxn(tx, tableACLRoles) return idx, roles, nil } diff --git a/agent/consul/state/acl_events.go b/agent/consul/state/acl_events.go index f6a959a972..aea61ae543 100644 --- a/agent/consul/state/acl_events.go +++ b/agent/consul/state/acl_events.go @@ -20,7 +20,7 @@ func aclChangeUnsubscribeEvent(tx ReadTxn, changes Changes) ([]stream.Event, err token := changeObject(change).(*structs.ACLToken) secretIDs = append(secretIDs, token.SecretID) - case "acl-roles": + case tableACLRoles: role := changeObject(change).(*structs.ACLRole) tokens, err := aclTokenListByRole(tx, role.ID, &role.EnterpriseMeta) if err != nil { diff --git a/agent/consul/state/acl_oss.go b/agent/consul/state/acl_oss.go index b4a63e6b50..f53c5bd507 100644 --- a/agent/consul/state/acl_oss.go +++ b/agent/consul/state/acl_oss.go @@ -144,48 +144,48 @@ func (s *Store) ACLTokenUpsertValidateEnterprise(token *structs.ACLToken, existi func aclRoleInsert(tx *txn, role *structs.ACLRole) error { // insert the role into memdb - if err := tx.Insert("acl-roles", role); err != nil { + if err := tx.Insert(tableACLRoles, role); err != nil { return fmt.Errorf("failed inserting acl role: %v", err) } // update the overall acl-roles index - if err := indexUpdateMaxTxn(tx, role.ModifyIndex, "acl-roles"); err != nil { + if err := indexUpdateMaxTxn(tx, role.ModifyIndex, tableACLRoles); err != nil { return fmt.Errorf("failed updating acl roles index: %v", err) } return nil } func aclRoleGetByID(tx ReadTxn, id string, _ *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { - return tx.FirstWatch("acl-roles", "id", id) + return tx.FirstWatch(tableACLRoles, indexID, id) } func aclRoleGetByName(tx ReadTxn, name string, _ *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { - return tx.FirstWatch("acl-roles", "name", name) + return tx.FirstWatch(tableACLRoles, indexName, name) } func aclRoleList(tx ReadTxn, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { - return tx.Get("acl-roles", "id") + return tx.Get(tableACLRoles, indexID) } func aclRoleListByPolicy(tx ReadTxn, policy string, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { - return tx.Get("acl-roles", "policies", policy) + return tx.Get(tableACLRoles, indexPolicies, policy) } func aclRoleDeleteWithRole(tx *txn, role *structs.ACLRole, idx uint64) error { // remove the role - if err := tx.Delete("acl-roles", role); err != nil { + if err := tx.Delete(tableACLRoles, role); err != nil { return fmt.Errorf("failed deleting acl role: %v", err) } // update the overall acl-roles index - if err := indexUpdateMaxTxn(tx, idx, "acl-roles"); err != nil { + if err := indexUpdateMaxTxn(tx, idx, tableACLRoles); err != nil { return fmt.Errorf("failed updating acl policies index: %v", err) } return nil } func aclRoleMaxIndex(tx ReadTxn, _ *structs.ACLRole, _ *structs.EnterpriseMeta) uint64 { - return maxIndexTxn(tx, "acl-roles") + return maxIndexTxn(tx, tableACLRoles) } func aclRoleUpsertValidateEnterprise(tx *txn, role *structs.ACLRole, existing *structs.ACLRole) error { diff --git a/agent/consul/state/acl_test.go b/agent/consul/state/acl_test.go index 3f3f21019c..71d7cdd213 100644 --- a/agent/consul/state/acl_test.go +++ b/agent/consul/state/acl_test.go @@ -4082,7 +4082,7 @@ func TestStateStore_ACLRoles_Snapshot_Restore(t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(2), idx) require.ElementsMatch(t, roles, res) - require.Equal(t, uint64(2), s.maxIndex("acl-roles")) + require.Equal(t, uint64(2), s.maxIndex(tableACLRoles)) }() } From d7f5094702a11886d92c1e9a4e085fd07e0efa5d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 16 Mar 2021 17:12:15 -0400 Subject: [PATCH 2/5] state: add indexer tests for acl-roles table --- agent/consul/state/acl_oss_test.go | 46 ++++++++++++++++++++++++++++++ agent/consul/state/schema_test.go | 1 + 2 files changed, 47 insertions(+) diff --git a/agent/consul/state/acl_oss_test.go b/agent/consul/state/acl_oss_test.go index fab7197819..6d4b094177 100644 --- a/agent/consul/state/acl_oss_test.go +++ b/agent/consul/state/acl_oss_test.go @@ -33,3 +33,49 @@ func testIndexerTableACLPolicies() map[string]indexerTestCase { }, } } + +func testIndexerTableACLRoles() map[string]indexerTestCase { + obj := &structs.ACLRole{ + ID: "123e4567-e89a-12d7-a456-426614174abc", + Name: "RoLe", + Policies: []structs.ACLRolePolicyLink{ + {ID: "PolicyId1"}, {ID: "PolicyId2"}, + }, + } + encodedID := []byte{0x12, 0x3e, 0x45, 0x67, 0xe8, 0x9a, 0x12, 0xd7, 0xa4, 0x56, 0x42, 0x66, 0x14, 0x17, 0x4a, 0xbc} + return map[string]indexerTestCase{ + indexID: { + read: indexValue{ + source: obj.ID, + expected: encodedID, + }, + write: indexValue{ + source: obj, + expected: encodedID, + }, + }, + indexName: { + read: indexValue{ + source: "RoLe", + expected: []byte("role\x00"), + }, + write: indexValue{ + source: obj, + expected: []byte("role\x00"), + }, + }, + indexPolicies: { + read: indexValue{ + source: "PolicyId1", + expected: []byte("PolicyId1\x00"), + }, + writeMulti: indexValueMulti{ + source: obj, + expected: [][]byte{ + []byte("PolicyId1\x00"), + []byte("PolicyId2\x00"), + }, + }, + }, + } +} diff --git a/agent/consul/state/schema_test.go b/agent/consul/state/schema_test.go index 0992ded938..366647791e 100644 --- a/agent/consul/state/schema_test.go +++ b/agent/consul/state/schema_test.go @@ -129,6 +129,7 @@ func TestNewDBSchema_Indexers(t *testing.T) { var testcases = map[string]func() map[string]indexerTestCase{ tableACLPolicies: testIndexerTableACLPolicies, + tableACLRoles: testIndexerTableACLRoles, tableChecks: testIndexerTableChecks, tableServices: testIndexerTableServices, tableNodes: testIndexerTableNodes, From 00b6f0b41abfd6fd0cb544834585749395fa37cb Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 17 Mar 2021 13:57:40 -0400 Subject: [PATCH 3/5] state: convert acl-roles.name index to the functional indexer pattern --- agent/consul/state/acl.go | 14 +++++++++-- agent/consul/state/acl_oss.go | 23 ++++++++++++------- agent/consul/state/acl_oss_test.go | 2 +- agent/consul/state/acl_schema.go | 7 +++--- .../testdata/TestStateStoreSchema.golden | 2 +- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index bebe9996c1..e618ea25fa 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -1371,7 +1371,8 @@ func aclRoleSetTxn(tx *txn, idx uint64, role *structs.ACLRole, allowMissing bool } // ensure the name is unique (cannot conflict with another role with a different ID) - _, nameMatch, err := aclRoleGetByName(tx, role.Name, &role.EnterpriseMeta) + q := Query{EnterpriseMeta: role.EnterpriseMeta, Value: role.Name} + nameMatch, err := tx.First(tableACLRoles, indexName, q) if err != nil { return fmt.Errorf("failed acl role lookup: %v", err) } @@ -1424,6 +1425,15 @@ func (s *Store) ACLRoleGetByName(ws memdb.WatchSet, name string, entMeta *struct return s.aclRoleGet(ws, name, aclRoleGetByName, entMeta) } +func aclRoleGetByName(tx ReadTxn, name string, entMeta *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { + // TODO: accept non-pointer value + if entMeta == nil { + entMeta = structs.DefaultEnterpriseMeta() + } + q := Query{EnterpriseMeta: *entMeta, Value: name} + return tx.FirstWatch(tableACLRoles, indexName, q) +} + func (s *Store) ACLRoleBatchGet(ws memdb.WatchSet, ids []string) (uint64, structs.ACLRoles, error) { tx := s.db.Txn(false) defer tx.Abort() @@ -1488,7 +1498,7 @@ func (s *Store) ACLRoleList(ws memdb.WatchSet, policy string, entMeta *structs.E if policy != "" { iter, err = aclRoleListByPolicy(tx, policy, entMeta) } else { - iter, err = aclRoleList(tx, entMeta) + iter, err = tx.Get(tableACLRoles, indexName+"_prefix", entMeta) } if err != nil { diff --git a/agent/consul/state/acl_oss.go b/agent/consul/state/acl_oss.go index f53c5bd507..92abc2eae5 100644 --- a/agent/consul/state/acl_oss.go +++ b/agent/consul/state/acl_oss.go @@ -38,6 +38,21 @@ func indexNameFromACLPolicy(raw interface{}) ([]byte, error) { return b.Bytes(), nil } +func indexNameFromACLRole(raw interface{}) ([]byte, error) { + p, ok := raw.(*structs.ACLRole) + if !ok { + return nil, fmt.Errorf("unexpected type %T for structs.ACLRole index", raw) + } + + if p.Name == "" { + return nil, errMissingValueForIndex + } + + var b indexBuilder + b.String(strings.ToLower(p.Name)) + return b.Bytes(), nil +} + func aclPolicyGetByID(tx ReadTxn, id string, _ *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { return tx.FirstWatch(tableACLPolicies, indexID, id) } @@ -159,14 +174,6 @@ func aclRoleGetByID(tx ReadTxn, id string, _ *structs.EnterpriseMeta) (<-chan st return tx.FirstWatch(tableACLRoles, indexID, id) } -func aclRoleGetByName(tx ReadTxn, name string, _ *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { - return tx.FirstWatch(tableACLRoles, indexName, name) -} - -func aclRoleList(tx ReadTxn, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { - return tx.Get(tableACLRoles, indexID) -} - func aclRoleListByPolicy(tx ReadTxn, policy string, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { return tx.Get(tableACLRoles, indexPolicies, policy) } diff --git a/agent/consul/state/acl_oss_test.go b/agent/consul/state/acl_oss_test.go index 6d4b094177..39cb3846fb 100644 --- a/agent/consul/state/acl_oss_test.go +++ b/agent/consul/state/acl_oss_test.go @@ -56,7 +56,7 @@ func testIndexerTableACLRoles() map[string]indexerTestCase { }, indexName: { read: indexValue{ - source: "RoLe", + source: Query{Value: "RoLe"}, expected: []byte("role\x00"), }, write: indexValue{ diff --git a/agent/consul/state/acl_schema.go b/agent/consul/state/acl_schema.go index 0126b5b90b..c692596bca 100644 --- a/agent/consul/state/acl_schema.go +++ b/agent/consul/state/acl_schema.go @@ -151,9 +151,10 @@ func rolesTableSchema() *memdb.TableSchema { Name: indexName, AllowMissing: false, Unique: true, - Indexer: &memdb.StringFieldIndex{ - Field: "Name", - Lowercase: true, + Indexer: indexerSingleWithPrefix{ + readIndex: readIndex(indexFromQuery), + writeIndex: writeIndex(indexNameFromACLRole), + prefixIndex: prefixIndex(prefixIndexFromQuery), }, }, indexPolicies: { diff --git a/agent/consul/state/testdata/TestStateStoreSchema.golden b/agent/consul/state/testdata/TestStateStoreSchema.golden index 4a6b752557..227f947e26 100644 --- a/agent/consul/state/testdata/TestStateStoreSchema.golden +++ b/agent/consul/state/testdata/TestStateStoreSchema.golden @@ -18,7 +18,7 @@ table=acl-roles index=id unique indexer=github.com/hashicorp/go-memdb.UUIDFieldIndex Field=ID index=name unique - indexer=github.com/hashicorp/go-memdb.StringFieldIndex Field=Name Lowercase=true + 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 From 43df402e512029897675950b2067f25b61d3d22f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 19 Mar 2021 16:07:55 -0400 Subject: [PATCH 4/5] 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 From 6324f372413b6a47a5ee582ddf92e32935daef5d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 19 Mar 2021 19:35:16 -0400 Subject: [PATCH 5/5] state: use uuid for acl-roles.policies index Previously we were encoding the UUID as a string, but the index it references uses a UUID so this index can also use an encoded UUID to save a bit of memory. --- agent/consul/state/acl_oss.go | 6 ++- agent/consul/state/acl_oss_test.go | 17 ++++---- agent/consul/state/acl_schema.go | 2 +- agent/consul/state/catalog.go | 7 ---- agent/consul/state/indexer.go | 7 ++++ agent/consul/state/query.go | 42 +++++++++++++++++++ agent/consul/state/query_oss.go | 8 ++++ .../testdata/TestStateStoreSchema.golden | 2 +- 8 files changed, 73 insertions(+), 18 deletions(-) create mode 100644 agent/consul/state/query.go diff --git a/agent/consul/state/acl_oss.go b/agent/consul/state/acl_oss.go index e658a5e624..f0ca3b3ea5 100644 --- a/agent/consul/state/acl_oss.go +++ b/agent/consul/state/acl_oss.go @@ -66,7 +66,11 @@ func multiIndexPolicyFromACLRole(raw interface{}) ([][]byte, error) { vals := make([][]byte, 0, count) for _, link := range role.Policies { - vals = append(vals, []byte(strings.ToLower(link.ID)+"\x00")) + v, err := uuidStringToBytes(link.ID) + if err != nil { + return nil, err + } + vals = append(vals, v) } return vals, nil diff --git a/agent/consul/state/acl_oss_test.go b/agent/consul/state/acl_oss_test.go index 89a504986a..f7a882dd97 100644 --- a/agent/consul/state/acl_oss_test.go +++ b/agent/consul/state/acl_oss_test.go @@ -35,14 +35,18 @@ func testIndexerTableACLPolicies() map[string]indexerTestCase { } func testIndexerTableACLRoles() map[string]indexerTestCase { + policyID1 := "123e4567-e89a-12d7-a456-426614174001" + policyID2 := "123e4567-e89a-12d7-a456-426614174002" obj := &structs.ACLRole{ ID: "123e4567-e89a-12d7-a456-426614174abc", Name: "RoLe", Policies: []structs.ACLRolePolicyLink{ - {ID: "PolicyId1"}, {ID: "PolicyId2"}, + {ID: policyID1}, {ID: policyID2}, }, } encodedID := []byte{0x12, 0x3e, 0x45, 0x67, 0xe8, 0x9a, 0x12, 0xd7, 0xa4, 0x56, 0x42, 0x66, 0x14, 0x17, 0x4a, 0xbc} + encodedPID1 := []byte{0x12, 0x3e, 0x45, 0x67, 0xe8, 0x9a, 0x12, 0xd7, 0xa4, 0x56, 0x42, 0x66, 0x14, 0x17, 0x40, 0x01} + encodedPID2 := []byte{0x12, 0x3e, 0x45, 0x67, 0xe8, 0x9a, 0x12, 0xd7, 0xa4, 0x56, 0x42, 0x66, 0x14, 0x17, 0x40, 0x02} return map[string]indexerTestCase{ indexID: { read: indexValue{ @@ -66,15 +70,12 @@ func testIndexerTableACLRoles() map[string]indexerTestCase { }, indexPolicies: { read: indexValue{ - source: Query{Value: "PolicyId1"}, - expected: []byte("policyid1\x00"), + source: Query{Value: policyID1}, + expected: encodedPID1, }, writeMulti: indexValueMulti{ - source: obj, - expected: [][]byte{ - []byte("policyid1\x00"), - []byte("policyid2\x00"), - }, + source: obj, + expected: [][]byte{encodedPID1, encodedPID2}, }, }, } diff --git a/agent/consul/state/acl_schema.go b/agent/consul/state/acl_schema.go index 463a8b9c3d..7a5fecd04d 100644 --- a/agent/consul/state/acl_schema.go +++ b/agent/consul/state/acl_schema.go @@ -163,7 +163,7 @@ func rolesTableSchema() *memdb.TableSchema { AllowMissing: true, Unique: false, Indexer: indexerMulti{ - readIndex: readIndex(indexFromQuery), + readIndex: readIndex(indexFromUUIDQuery), writeIndexMulti: writeIndexMulti(multiIndexPolicyFromACLRole), }, }, diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 0171d6b247..02b064ea93 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -28,13 +28,6 @@ const ( minUUIDLookupLen = 2 ) -// Query is a type used to query any single value index that may include an -// enterprise identifier. -type Query struct { - Value string - structs.EnterpriseMeta -} - func resizeNodeLookupKey(s string) string { l := len(s) diff --git a/agent/consul/state/indexer.go b/agent/consul/state/indexer.go index 66f1c5e8f0..d7727ed975 100644 --- a/agent/consul/state/indexer.go +++ b/agent/consul/state/indexer.go @@ -107,6 +107,13 @@ func (b *indexBuilder) String(v string) { (*bytes.Buffer)(b).WriteString(null) } +// Raw appends the bytes without a null terminator to the buffer. Raw should +// only be used when v has a fixed length, or when building the last segment of +// a prefix index. +func (b *indexBuilder) Raw(v []byte) { + (*bytes.Buffer)(b).Write(v) +} + func (b *indexBuilder) Bytes() []byte { return (*bytes.Buffer)(b).Bytes() } diff --git a/agent/consul/state/query.go b/agent/consul/state/query.go new file mode 100644 index 0000000000..f58734a803 --- /dev/null +++ b/agent/consul/state/query.go @@ -0,0 +1,42 @@ +package state + +import ( + "encoding/hex" + "fmt" + "strings" + + "github.com/hashicorp/consul/agent/structs" +) + +// Query is a type used to query any single value index that may include an +// enterprise identifier. +type Query struct { + Value string + structs.EnterpriseMeta +} + +// uuidStringToBytes is a modified version of memdb.UUIDFieldIndex.parseString +func uuidStringToBytes(uuid string) ([]byte, error) { + l := len(uuid) + if l != 36 { + return nil, fmt.Errorf("UUID must be 36 characters") + } + + hyphens := strings.Count(uuid, "-") + if hyphens > 4 { + return nil, fmt.Errorf(`UUID should have maximum of 4 "-"; got %d`, hyphens) + } + + // The sanitized length is the length of the original string without the "-". + sanitized := strings.Replace(uuid, "-", "", -1) + sanitizedLength := len(sanitized) + if sanitizedLength%2 != 0 { + return nil, fmt.Errorf("UUID (without hyphens) must be even length") + } + + dec, err := hex.DecodeString(sanitized) + if err != nil { + return nil, fmt.Errorf("invalid UUID: %w", err) + } + return dec, nil +} diff --git a/agent/consul/state/query_oss.go b/agent/consul/state/query_oss.go index 79f45095be..bd5fd72485 100644 --- a/agent/consul/state/query_oss.go +++ b/agent/consul/state/query_oss.go @@ -36,3 +36,11 @@ func prefixIndexFromQuery(arg interface{}) ([]byte, error) { return nil, fmt.Errorf("unexpected type %T for Query prefix index", arg) } + +func indexFromUUIDQuery(raw interface{}) ([]byte, error) { + q, ok := raw.(Query) + if !ok { + return nil, fmt.Errorf("unexpected type %T for UUIDQuery index", raw) + } + return uuidStringToBytes(q.Value) +} diff --git a/agent/consul/state/testdata/TestStateStoreSchema.golden b/agent/consul/state/testdata/TestStateStoreSchema.golden index cddd96f7f6..540576a52e 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.indexerMulti readIndex=github.com/hashicorp/consul/agent/consul/state.indexFromQuery writeIndexMulti=github.com/hashicorp/consul/agent/consul/state.multiIndexPolicyFromACLRole + indexer=github.com/hashicorp/consul/agent/consul/state.indexerMulti readIndex=github.com/hashicorp/consul/agent/consul/state.indexFromUUIDQuery writeIndexMulti=github.com/hashicorp/consul/agent/consul/state.multiIndexPolicyFromACLRole table=acl-tokens index=accessor unique allow-missing