From 6324f372413b6a47a5ee582ddf92e32935daef5d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 19 Mar 2021 19:35:16 -0400 Subject: [PATCH] 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