From eb6769ccc6c73e9115392aa46c4b5b146c193cb5 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 16 Mar 2021 15:29:30 -0400 Subject: [PATCH] state: convert acl-policies table to new pattern --- agent/consul/state/acl.go | 20 ++++++---- agent/consul/state/acl_oss.go | 24 ++++++++---- agent/consul/state/acl_oss_test.go | 2 +- agent/consul/state/acl_schema.go | 8 ++-- agent/consul/state/catalog_oss.go | 15 -------- agent/consul/state/query_oss.go | 38 +++++++++++++++++++ .../testdata/TestStateStoreSchema.golden | 2 +- 7 files changed, 73 insertions(+), 36 deletions(-) create mode 100644 agent/consul/state/query_oss.go diff --git a/agent/consul/state/acl.go b/agent/consul/state/acl.go index 5401d733e3..811bfe3ff7 100644 --- a/agent/consul/state/acl.go +++ b/agent/consul/state/acl.go @@ -228,11 +228,7 @@ func (s *Restore) ACLToken(token *structs.ACLToken) error { // ACLPolicies is used when saving a snapshot func (s *Snapshot) ACLPolicies() (memdb.ResultIterator, error) { - iter, err := s.tx.Get(tableACLPolicies, indexID) - if err != nil { - return nil, err - } - return iter, nil + return s.tx.Get(tableACLPolicies, indexID) } func (s *Restore) ACLPolicy(policy *structs.ACLPolicy) error { @@ -1162,7 +1158,8 @@ func aclPolicySetTxn(tx *txn, idx uint64, policy *structs.ACLPolicy) error { } // ensure the name is unique (cannot conflict with another policy with a different ID) - _, nameMatch, err := aclPolicyGetByName(tx, policy.Name, &policy.EnterpriseMeta) + q := Query{Value: policy.Name, EnterpriseMeta: policy.EnterpriseMeta} + nameMatch, err := tx.First(tableACLPolicies, indexName, q) if err != nil { return err } @@ -1195,6 +1192,15 @@ func (s *Store) ACLPolicyGetByName(ws memdb.WatchSet, name string, entMeta *stru return s.aclPolicyGet(ws, name, aclPolicyGetByName, entMeta) } +func aclPolicyGetByName(tx ReadTxn, name string, entMeta *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { + // todo: accept non-pointer value + if entMeta == nil { + entMeta = structs.DefaultEnterpriseMeta() + } + q := Query{Value: name, EnterpriseMeta: *entMeta} + return tx.FirstWatch(tableACLPolicies, indexName, q) +} + func (s *Store) ACLPolicyBatchGet(ws memdb.WatchSet, ids []string) (uint64, structs.ACLPolicies, error) { tx := s.db.Txn(false) defer tx.Abort() @@ -1252,7 +1258,7 @@ func (s *Store) ACLPolicyList(ws memdb.WatchSet, entMeta *structs.EnterpriseMeta tx := s.db.Txn(false) defer tx.Abort() - iter, err := aclPolicyList(tx, entMeta) + iter, err := tx.Get(tableACLPolicies, indexName+"_prefix", entMeta) if err != nil { return 0, nil, fmt.Errorf("failed acl policy lookup: %v", err) } diff --git a/agent/consul/state/acl_oss.go b/agent/consul/state/acl_oss.go index 730cd0892b..b4a63e6b50 100644 --- a/agent/consul/state/acl_oss.go +++ b/agent/consul/state/acl_oss.go @@ -4,6 +4,7 @@ package state import ( "fmt" + "strings" memdb "github.com/hashicorp/go-memdb" @@ -22,18 +23,25 @@ func aclPolicyInsert(tx *txn, policy *structs.ACLPolicy) error { return nil } +func indexNameFromACLPolicy(raw interface{}) ([]byte, error) { + p, ok := raw.(*structs.ACLPolicy) + if !ok { + return nil, fmt.Errorf("unexpected type %T for structs.ACLPolicy 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) } -func aclPolicyGetByName(tx ReadTxn, name string, _ *structs.EnterpriseMeta) (<-chan struct{}, interface{}, error) { - return tx.FirstWatch(tableACLPolicies, indexName, name) -} - -func aclPolicyList(tx ReadTxn, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { - return tx.Get(tableACLPolicies, indexID) -} - func aclPolicyDeleteWithPolicy(tx *txn, policy *structs.ACLPolicy, idx uint64) error { // remove the policy if err := tx.Delete(tableACLPolicies, policy); err != nil { diff --git a/agent/consul/state/acl_oss_test.go b/agent/consul/state/acl_oss_test.go index 2979e13b69..fab7197819 100644 --- a/agent/consul/state/acl_oss_test.go +++ b/agent/consul/state/acl_oss_test.go @@ -23,7 +23,7 @@ func testIndexerTableACLPolicies() map[string]indexerTestCase { }, indexName: { read: indexValue{ - source: []interface{}{"PolicyName"}, + source: Query{Value: "PolicyName"}, expected: []byte("policyname\x00"), }, write: indexValue{ diff --git a/agent/consul/state/acl_schema.go b/agent/consul/state/acl_schema.go index 88cd1b4d61..0126b5b90b 100644 --- a/agent/consul/state/acl_schema.go +++ b/agent/consul/state/acl_schema.go @@ -125,10 +125,10 @@ func policiesTableSchema() *memdb.TableSchema { Name: indexName, AllowMissing: false, Unique: true, - Indexer: &memdb.StringFieldIndex{ - Field: "Name", - // TODO (ACL-V2) - should we coerce to lowercase? - Lowercase: true, + Indexer: indexerSingleWithPrefix{ + readIndex: readIndex(indexFromQuery), + writeIndex: writeIndex(indexNameFromACLPolicy), + prefixIndex: prefixIndex(prefixIndexFromQuery), }, }, }, diff --git a/agent/consul/state/catalog_oss.go b/agent/consul/state/catalog_oss.go index 547ea89b11..090422603c 100644 --- a/agent/consul/state/catalog_oss.go +++ b/agent/consul/state/catalog_oss.go @@ -103,21 +103,6 @@ func indexFromServiceNode(raw interface{}) ([]byte, error) { return b.Bytes(), nil } -func prefixIndexFromQuery(arg interface{}) ([]byte, error) { - var b indexBuilder - switch v := arg.(type) { - case *structs.EnterpriseMeta: - return nil, nil - case structs.EnterpriseMeta: - return nil, nil - case Query: - b.String(strings.ToLower(v.Value)) - return b.Bytes(), nil - } - - return nil, fmt.Errorf("unexpected type %T for NodeServiceQuery prefix index", arg) -} - func serviceIndexName(name string, _ *structs.EnterpriseMeta) string { return fmt.Sprintf("service.%s", name) } diff --git a/agent/consul/state/query_oss.go b/agent/consul/state/query_oss.go new file mode 100644 index 0000000000..79f45095be --- /dev/null +++ b/agent/consul/state/query_oss.go @@ -0,0 +1,38 @@ +// +build !consulent + +package state + +import ( + "fmt" + "strings" + + "github.com/hashicorp/consul/agent/structs" +) + +// indexFromQuery builds an index key where Query.Value is lowercase, and is +// a required value. +func indexFromQuery(arg interface{}) ([]byte, error) { + q, ok := arg.(Query) + if !ok { + return nil, fmt.Errorf("unexpected type %T for Query index", arg) + } + + var b indexBuilder + b.String(strings.ToLower(q.Value)) + return b.Bytes(), nil +} + +func prefixIndexFromQuery(arg interface{}) ([]byte, error) { + var b indexBuilder + switch v := arg.(type) { + case *structs.EnterpriseMeta: + return nil, nil + case structs.EnterpriseMeta: + return nil, nil + case Query: + b.String(strings.ToLower(v.Value)) + return b.Bytes(), nil + } + + return nil, fmt.Errorf("unexpected type %T for Query prefix index", arg) +} diff --git a/agent/consul/state/testdata/TestStateStoreSchema.golden b/agent/consul/state/testdata/TestStateStoreSchema.golden index 7f74557800..4a6b752557 100644 --- a/agent/consul/state/testdata/TestStateStoreSchema.golden +++ b/agent/consul/state/testdata/TestStateStoreSchema.golden @@ -12,7 +12,7 @@ table=acl-policies 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.indexNameFromACLPolicy prefixIndex=github.com/hashicorp/consul/agent/consul/state.prefixIndexFromQuery table=acl-roles index=id unique