luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Identity: Remove unused MemDB indexes and unused functions (#4817) 

* refactor delete utility

* refactor delete alias utility

* remove MemDBUpsertAlias

* Remove MemDBAliasByCanonicalID

* remove MemDBAliasesByMetadata

* remove MemDBDeleteAliasByID

* Remove MemDBUpsertEntity and MemDBEntityByNameInTxn

* Remove is.MemDBEntitiesByBucketEntryKeyHash

* Remove MemDBEntitiesByBucketEntryKeyHash and MemDBEntityByMergedEntityID

* Remove MemDBEntities

* Remove validateMemberGroupID

* Remove validateEntityID, validateGroupID, deleteAliasFromEntity

* Remove updateAliasInEntity

* Remove satisfiesMetadataFilters and UpsertGroup

* Remove MemDBUpsertGroup

* Remove deleteGroupByID

* Remove deleleGroupByName

* Remove MemDBDeleteGroupByNameInTxn

* Remove MemDBGroupsByPolicy and MemDBGroupsByPolicyInTxn

* Remove MemDBGroupIterator

* Remove MemDBGroupsByBucketEntryKeyHash

* Remove deleteGroupAlias

* Remove metadata index from entities table

* Remove unneeded indexes from entity alias and group alias schema

* Remove unneeded index from groups table schema

* Fix test

* s/entity/lockEntity

* Don't expose the memdb instance outside identity store

* More txn.Abort() corrections

* switch back to deferring abort calls
This commit is contained in:
Vishal Nayak 2018-06-24 07:45:53 -04:00 committed by GitHub
parent 619dd3c6ed
commit 57c7ecfcd4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 362 additions and 961 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
vault/identity_store.go
View File

@ -226,7 +226,7 @@ func (i *IdentityStore) Invalidate(ctx context.Context, key string) {
}
// Only update MemDB and don't touch the storage
err = i.upsertGroupInTxn(txn, group, false)
err = i.UpsertGroupInTxn(txn, group, false)
if err != nil {
i.logger.Error("failed to update group in MemDB", "error", err)
return

124
vault/identity_store_aliases.go
View File

@ -9,6 +9,8 @@ import (
"github.com/hashicorp/errwrap"
memdb "github.com/hashicorp/go-memdb"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/locksutil"
"github.com/hashicorp/vault/helper/storagepacker"
"github.com/hashicorp/vault/logical"
"github.com/hashicorp/vault/logical/framework"
)
@ -291,19 +293,31 @@ func (i *IdentityStore) handleAliasUpdateCommon(req *logical.Request, d *framewo
if entity != nil && entity.ID != existingEntity.ID {
// Alias should be transferred from 'existingEntity' to 'entity'
err = i.deleteAliasFromEntity(existingEntity, alias)
if err != nil {
return nil, err
for aliasIndex, item := range existingEntity.Aliases {
if item.ID == alias.ID {
entity.Aliases = append(existingEntity.Aliases[:aliasIndex], existingEntity.Aliases[aliasIndex+1:]...)
break
}
}
previousEntity = existingEntity
entity.Aliases = append(entity.Aliases, alias)
resp.AddWarning(fmt.Sprintf("alias is being transferred from entity %q to %q", existingEntity.ID, entity.ID))
} else {
// Update entity with modified alias
err = i.updateAliasInEntity(existingEntity, alias)
if err != nil {
return nil, err
aliasFound := false
for aliasIndex, item := range existingEntity.Aliases {
if item.ID == alias.ID {
aliasFound = true
existingEntity.Aliases[aliasIndex] = alias
break
}
}
if !aliasFound {
return nil, fmt.Errorf("alias does not exist in entity")
}
entity = existingEntity
}
}
@ -406,7 +420,103 @@ func (i *IdentityStore) pathAliasIDDelete() framework.OperationFunc {
return logical.ErrorResponse("missing alias ID"), nil
}
return nil, i.deleteAlias(aliasID)
// Fetch the alias using its ID
alias, err := i.MemDBAliasByID(aliasID, false, false)
if err != nil {
return nil, err
}
// If there is no alias for the ID, do nothing
if alias == nil {
return nil, nil
}
// Find the entity to which the alias is tied to
lockEntity, err := i.MemDBEntityByAliasID(alias.ID, false)
if err != nil {
return nil, err
}
// If there is no entity tied to a valid alias, something is wrong
if lockEntity == nil {
return nil, fmt.Errorf("alias not associated to an entity")
}
// Acquire the lock to modify the entity storage entry
lock := locksutil.LockForKey(i.entityLocks, lockEntity.ID)
lock.Lock()
defer lock.Unlock()
// Create a MemDB transaction to delete entity
txn := i.db.Txn(true)
defer txn.Abort()
// Fetch the alias again after acquiring the lock using the transaction
// created above
alias, err = i.MemDBAliasByIDInTxn(txn, aliasID, false, false)
if err != nil {
return nil, err
}
// If there is no alias for the ID, do nothing
if alias == nil {
return nil, nil
}
// Fetch the entity again after acquiring the lock using the transaction
// created above
entity, err := i.MemDBEntityByAliasIDInTxn(txn, alias.ID, true)
if err != nil {
return nil, err
}
// If there is no entity tied to a valid alias, something is wrong
if entity == nil {
return nil, fmt.Errorf("alias not associated to an entity")
}
// Lock switching should not end up in this code pointing to different
// entities
if lockEntity.ID != entity.ID {
return nil, fmt.Errorf("operating on an entity to which the lock doesn't belong to")
}
aliases := []*identity.Alias{
alias,
}
// Delete alias from the entity object
err = i.deleteAliasesInEntityInTxn(txn, entity, aliases)
if err != nil {
return nil, err
}
// Update the entity index in the entities table
err = i.MemDBUpsertEntityInTxn(txn, entity)
if err != nil {
return nil, err
}
// Persist the entity object
entityAsAny, err := ptypes.MarshalAny(entity)
if err != nil {
return nil, err
}
item := &storagepacker.Item{
ID: entity.ID,
Message: entityAsAny,
}
err = i.entityPacker.PutItem(item)
if err != nil {
return nil, err
}
// Committing the transaction *after* successfully updating entity in
// storage
txn.Commit()
return nil, nil
}
}

95
vault/identity_store_aliases_test.go
View File

@ -63,10 +63,13 @@ func TestIdentityStore_MemDBAliasIndexes(t *testing.T) {
entity.BucketKeyHash = is.entityPacker.BucketKeyHashByItemID(entity.ID)
err = is.MemDBUpsertEntity(entity)
txn := is.db.Txn(true)
defer txn.Abort()
err = is.MemDBUpsertEntityInTxn(txn, entity)
if err != nil {
t.Fatal(err)
}
txn.Commit()
alias := &identity.Alias{
CanonicalID: entity.ID,
@ -80,10 +83,13 @@ func TestIdentityStore_MemDBAliasIndexes(t *testing.T) {
},
}
err = is.MemDBUpsertAlias(alias, false)
txn = is.db.Txn(true)
defer txn.Abort()
err = is.MemDBUpsertAliasInTxn(txn, alias, false)
if err != nil {
t.Fatal(err)
}
txn.Commit()
aliasFetched, err := is.MemDBAliasByID("testaliasid", false, false)
if err != nil {
@ -94,15 +100,6 @@ func TestIdentityStore_MemDBAliasIndexes(t *testing.T) {
t.Fatalf("bad: mismatched aliases; expected: %#v\n actual: %#v\n", alias, aliasFetched)
}
aliasFetched, err = is.MemDBAliasByCanonicalID(entity.ID, false, false)
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(alias, aliasFetched) {
t.Fatalf("bad: mismatched aliases; expected: %#v\n actual: %#v\n", alias, aliasFetched)
}
aliasFetched, err = is.MemDBAliasByFactors(validateMountResp.MountAccessor, "testaliasname", false, false)
if err != nil {
t.Fatal(err)
@ -112,52 +109,6 @@ func TestIdentityStore_MemDBAliasIndexes(t *testing.T) {
t.Fatalf("bad: mismatched aliases; expected: %#v\n actual: %#v\n", alias, aliasFetched)
}
aliasesFetched, err := is.MemDBAliasesByMetadata(map[string]string{
"testkey1": "testmetadatavalue1",
}, false, false)
if err != nil {
t.Fatal(err)
}
if len(aliasesFetched) != 1 {
t.Fatalf("bad: length of aliases; expected: 1, actual: %d", len(aliasesFetched))
}
if !reflect.DeepEqual(alias, aliasesFetched[0]) {
t.Fatalf("bad: mismatched aliases; expected: %#v\n actual: %#v\n", alias, aliasFetched)
}
aliasesFetched, err = is.MemDBAliasesByMetadata(map[string]string{
"testkey2": "testmetadatavalue2",
}, false, false)
if err != nil {
t.Fatal(err)
}
if len(aliasesFetched) != 1 {
t.Fatalf("bad: length of aliases; expected: 1, actual: %d", len(aliasesFetched))
}
if !reflect.DeepEqual(alias, aliasesFetched[0]) {
t.Fatalf("bad: mismatched aliases; expected: %#v\n actual: %#v\n", alias, aliasFetched)
}
aliasesFetched, err = is.MemDBAliasesByMetadata(map[string]string{
"testkey1": "testmetadatavalue1",
"testkey2": "testmetadatavalue2",
}, false, false)
if err != nil {
t.Fatal(err)
}
if len(aliasesFetched) != 1 {
t.Fatalf("bad: length of aliases; expected: 1, actual: %d", len(aliasesFetched))
}
if !reflect.DeepEqual(alias, aliasesFetched[0]) {
t.Fatalf("bad: mismatched aliases; expected: %#v\n actual: %#v\n", alias, aliasFetched)
}
alias2 := &identity.Alias{
CanonicalID: entity.ID,
ID: "testaliasid2",
@ -170,37 +121,17 @@ func TestIdentityStore_MemDBAliasIndexes(t *testing.T) {
},
}
err = is.MemDBUpsertAlias(alias2, false)
txn = is.db.Txn(true)
defer txn.Abort()
err = is.MemDBUpsertAliasInTxn(txn, alias2, false)
if err != nil {
t.Fatal(err)
}
aliasesFetched, err = is.MemDBAliasesByMetadata(map[string]string{
"testkey1": "testmetadatavalue1",
}, false, false)
if err != nil {
t.Fatal(err)
}
if len(aliasesFetched) != 2 {
t.Fatalf("bad: length of aliases; expected: 2, actual: %d", len(aliasesFetched))
}
aliasesFetched, err = is.MemDBAliasesByMetadata(map[string]string{
"testkey3": "testmetadatavalue3",
}, false, false)
if err != nil {
t.Fatal(err)
}
if len(aliasesFetched) != 1 {
t.Fatalf("bad: length of aliases; expected: 1, actual: %d", len(aliasesFetched))
}
err = is.MemDBDeleteAliasByID("testaliasid", false)
err = is.MemDBDeleteAliasByIDInTxn(txn, "testaliasid", false)
if err != nil {
t.Fatal(err)
}
txn.Commit()
aliasFetched, err = is.MemDBAliasByID("testaliasid", false, false)
if err != nil {

62
vault/identity_store_entities.go
View File

@ -511,7 +511,60 @@ func (i *IdentityStore) pathEntityIDDelete() framework.OperationFunc {
return logical.ErrorResponse("missing entity id"), nil
}
return nil, i.deleteEntity(entityID)
// Since an entity ID is required to acquire the lock to modify the
// storage, fetch the entity without acquiring the lock
lockEntity, err := i.MemDBEntityByID(entityID, false)
if err != nil {
return nil, err
}
if lockEntity == nil {
return nil, nil
}
// Acquire the lock to modify the entity storage entry
lock := locksutil.LockForKey(i.entityLocks, lockEntity.ID)
lock.Lock()
defer lock.Unlock()
// Create a MemDB transaction to delete entity
txn := i.db.Txn(true)
defer txn.Abort()
// Fetch the entity using its ID
entity, err := i.MemDBEntityByIDInTxn(txn, entityID, true)
if err != nil {
return nil, err
}
// If there is no entity for the ID, do nothing
if entity == nil {
return nil, nil
}
// Delete all the aliases in the entity. This function will also remove
// the corresponding alias indexes too.
err = i.deleteAliasesInEntityInTxn(txn, entity, entity.Aliases)
if err != nil {
return nil, err
}
// Delete the entity using the same transaction
err = i.MemDBDeleteEntityByIDInTxn(txn, entity.ID)
if err != nil {
return nil, err
}
// Delete the entity from storage
err = i.entityPacker.DeleteItem(entity.ID)
if err != nil {
return nil, err
}
// Committing the transaction *after* successfully deleting entity
txn.Commit()
return nil, nil
}
}
@ -520,11 +573,16 @@ func (i *IdentityStore) pathEntityIDDelete() framework.OperationFunc {
func (i *IdentityStore) pathEntityIDList() framework.OperationFunc {
return func(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
ws := memdb.NewWatchSet()
iter, err := i.MemDBEntities(ws)
txn := i.db.Txn(false)
iter, err := txn.Get(entitiesTable, "id")
if err != nil {
return nil, errwrap.Wrapf("failed to fetch iterator for entities in memdb: {{err}}", err)
}
ws.Add(iter.WatchCh())
var entityIDs []string
entityInfo := map[string]interface{}{}

31
vault/identity_store_entities_test.go
View File

@ -222,11 +222,16 @@ func TestIdentityStore_MemDBImmutability(t *testing.T) {
entity.BucketKeyHash = is.entityPacker.BucketKeyHashByItemID(entity.ID)
err = is.MemDBUpsertEntity(entity)
txn := is.db.Txn(true)
defer txn.Abort()
err = is.MemDBUpsertEntityInTxn(txn, entity)
if err != nil {
t.Fatal(err)
}
txn.Commit()
entityFetched, err := is.MemDBEntityByID(entity.ID, true)
if err != nil {
t.Fatal(err)
@ -478,10 +483,13 @@ func TestIdentityStore_MemDBEntityIndexes(t *testing.T) {
entity.BucketKeyHash = is.entityPacker.BucketKeyHashByItemID(entity.ID)
err = is.MemDBUpsertEntity(entity)
txn := is.db.Txn(true)
defer txn.Abort()
err = is.MemDBUpsertEntityInTxn(txn, entity)
if err != nil {
t.Fatal(err)
}
txn.Commit()
// Fetch the entity using its ID
entityFetched, err := is.MemDBEntityByID(entity.ID, false)
@ -503,23 +511,8 @@ func TestIdentityStore_MemDBEntityIndexes(t *testing.T) {
t.Fatalf("entity mismatched entities; expected: %#v\n actual: %#v\n", entity, entityFetched)
}
// Fetch entities using the metadata
entitiesFetched, err := is.MemDBEntitiesByMetadata(map[string]string{
"someusefulkey": "someusefulvalue",
}, false)
if err != nil {
t.Fatal(err)
}
if len(entitiesFetched) != 1 {
t.Fatalf("bad: length of entities; expected: 1, actual: %d", len(entitiesFetched))
}
if !reflect.DeepEqual(entity, entitiesFetched[0]) {
t.Fatalf("entity mismatch; entity: %#v\n entitiesFetched[0]: %#v\n", entity, entitiesFetched[0])
}
entitiesFetched, err = is.MemDBEntitiesByBucketEntryKeyHash(entity.BucketKeyHash)
txn = is.db.Txn(false)
entitiesFetched, err := is.MemDBEntitiesByBucketEntryKeyHashInTxn(txn, entity.BucketKeyHash)
if err != nil {
t.Fatal(err)
}

2
command/identity_group_aliases_integ_test.go → vault/identity_store_ext_test.go
View File

@ -1,4 +1,4 @@
package command
package vault_test
import (
"testing"

43
vault/identity_store_group_aliases.go
View File

@ -253,7 +253,48 @@ func (i *IdentityStore) pathGroupAliasIDDelete() framework.OperationFunc {
return logical.ErrorResponse("missing group alias ID"), nil
}
return nil, i.deleteGroupAlias(groupAliasID)
i.groupLock.Lock()
defer i.groupLock.Unlock()
txn := i.db.Txn(true)
defer txn.Abort()
alias, err := i.MemDBAliasByIDInTxn(txn, groupAliasID, false, true)
if err != nil {
return nil, err
}
if alias == nil {
return nil, nil
}
group, err := i.MemDBGroupByAliasIDInTxn(txn, alias.ID, true)
if err != nil {
return nil, err
}
// If there is no group tied to a valid alias, something is wrong
if group == nil {
return nil, fmt.Errorf("alias not associated to a group")
}
// Delete group alias in memdb
err = i.MemDBDeleteAliasByIDInTxn(txn, group.Alias.ID, true)
if err != nil {
return nil, err
}
// Delete the alias
group.Alias = nil
err = i.UpsertGroupInTxn(txn, group, true)
if err != nil {
return nil, err
}
txn.Commit()
return nil, nil
}
}

8
vault/identity_store_group_aliases_test.go
View File

@ -163,15 +163,17 @@ func TestIdentityStore_GroupAliases_MemDBIndexes(t *testing.T) {
BucketKeyHash: i.groupPacker.BucketKeyHashByItemID("testgroupid"),
}
err = i.MemDBUpsertAlias(group.Alias, true)
txn := i.db.Txn(true)
defer txn.Abort()
err = i.MemDBUpsertAliasInTxn(txn, group.Alias, true)
if err != nil {
t.Fatal(err)
}
err = i.MemDBUpsertGroup(group)
err = i.MemDBUpsertGroupInTxn(txn, group)
if err != nil {
t.Fatal(err)
}
txn.Commit()
alias, err := i.MemDBAliasByID("testgroupaliasid", false, true)
if err != nil {

62
vault/identity_store_groups.go
View File

@ -301,10 +301,15 @@ func (i *IdentityStore) handleGroupReadCommon(group *identity.Group) (*logical.R
respData["alias"] = aliasMap
memberGroupIDs, err := i.memberGroupIDsByID(group.ID)
var memberGroupIDs []string
memberGroups, err := i.MemDBGroupsByParentGroupID(group.ID, false)
if err != nil {
return nil, err
}
for _, memberGroup := range memberGroups {
memberGroupIDs = append(memberGroupIDs, memberGroup.ID)
}
respData["member_group_ids"] = memberGroupIDs
return &logical.Response{
@ -318,7 +323,53 @@ func (i *IdentityStore) pathGroupIDDelete() framework.OperationFunc {
if groupID == "" {
return logical.ErrorResponse("empty group ID"), nil
}
return nil, i.deleteGroupByID(groupID)
if groupID == "" {
return nil, fmt.Errorf("missing group ID")
}
// Acquire the lock to modify the group storage entry
i.groupLock.Lock()
defer i.groupLock.Unlock()
// Create a MemDB transaction to delete group
txn := i.db.Txn(true)
defer txn.Abort()
group, err := i.MemDBGroupByIDInTxn(txn, groupID, false)
if err != nil {
return nil, err
}
// If there is no group for the ID, do nothing
if group == nil {
return nil, nil
}
// Delete group alias from memdb
if group.Type == groupTypeExternal && group.Alias != nil {
err = i.MemDBDeleteAliasByIDInTxn(txn, group.Alias.ID, true)
if err != nil {
return nil, err
}
}
// Delete the group using the same transaction
err = i.MemDBDeleteGroupByIDInTxn(txn, group.ID)
if err != nil {
return nil, err
}
// Delete the group from storage
err = i.groupPacker.DeleteItem(group.ID)
if err != nil {
return nil, err
}
// Committing the transaction *after* successfully deleting group
txn.Commit()
return nil, nil
}
}
@ -326,11 +377,16 @@ func (i *IdentityStore) pathGroupIDDelete() framework.OperationFunc {
func (i *IdentityStore) pathGroupIDList() framework.OperationFunc {
return func(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
ws := memdb.NewWatchSet()
iter, err := i.MemDBGroupIterator(ws)
txn := i.db.Txn(false)
iter, err := txn.Get(groupsTable, "id")
if err != nil {
return nil, errwrap.Wrapf("failed to fetch iterator for group in memdb: {{err}}", err)
}
ws.Add(iter.WatchCh())
var groupIDs []string
groupInfo := map[string]interface{}{}

28
vault/identity_store_groups_test.go
View File

@ -118,10 +118,13 @@ func TestIdentityStore_MemDBGroupIndexes(t *testing.T) {
}
// Insert it into memdb
err = i.MemDBUpsertGroup(group)
txn := i.db.Txn(true)
defer txn.Abort()
err = i.MemDBUpsertGroupInTxn(txn, group)
if err != nil {
t.Fatal(err)
}
txn.Commit()
// Insert another dummy group
group = &identity.Group{
@ -138,10 +141,14 @@ func TestIdentityStore_MemDBGroupIndexes(t *testing.T) {
}
// Insert it into memdb
err = i.MemDBUpsertGroup(group)
txn = i.db.Txn(true)
defer txn.Abort()
err = i.MemDBUpsertGroupInTxn(txn, group)
if err != nil {
t.Fatal(err)
}
txn.Commit()
var fetchedGroup *identity.Group
@ -181,23 +188,6 @@ func TestIdentityStore_MemDBGroupIndexes(t *testing.T) {
t.Fatalf("failed to fetch a indexed groups")
}
// Fetch groups based on policy name
fetchedGroups, err = i.MemDBGroupsByPolicy("testpolicy1", false)
if err != nil {
t.Fatal(err)
}
if len(fetchedGroups) != 1 || fetchedGroups[0].Name != "testgroupname" {
t.Fatalf("failed to fetch an indexed group")
}
fetchedGroups, err = i.MemDBGroupsByPolicy("testpolicy2", false)
if err != nil {
t.Fatal(err)
}
if len(fetchedGroups) != 2 {
t.Fatalf("failed to fetch indexed groups")
}
// Fetch groups based on member entity ID
fetchedGroups, err = i.MemDBGroupsByMemberEntityID("testentityid1", false, false)
if err != nil {

38
vault/identity_store_schema.go
View File

@ -47,13 +47,6 @@ func aliasesTableSchema() *memdb.TableSchema {
Field: "ID",
},
},
"canonical_id": &memdb.IndexSchema{
Name: "canonical_id",
Unique: false,
Indexer: &memdb.StringFieldIndex{
Field: "CanonicalID",
},
},
"factors": &memdb.IndexSchema{
Name: "factors",
Unique: true,
@ -68,14 +61,6 @@ func aliasesTableSchema() *memdb.TableSchema {
},
},
},
"metadata": &memdb.IndexSchema{
Name: "metadata",
Unique: false,
AllowMissing: true,
Indexer: &memdb.StringMapFieldIndex{
Field: "Metadata",
},
},
},
}
}
@ -98,14 +83,6 @@ func entitiesTableSchema() *memdb.TableSchema {
Field: "Name",
},
},
"metadata": &memdb.IndexSchema{
Name: "metadata",
Unique: false,
AllowMissing: true,
Indexer: &memdb.StringMapFieldIndex{
Field: "Metadata",
},
},
"merged_entity_ids": &memdb.IndexSchema{
Name: "merged_entity_ids",
Unique: true,
@ -160,14 +137,6 @@ func groupsTableSchema() *memdb.TableSchema {
Field: "ParentGroupIDs",
},
},
"policies": {
Name: "policies",
Unique: false,
AllowMissing: true,
Indexer: &memdb.StringSliceFieldIndex{
Field: "Policies",
},
},
"bucket_key_hash": &memdb.IndexSchema{
Name: "bucket_key_hash",
Unique: false,
@ -191,13 +160,6 @@ func groupAliasesTableSchema() *memdb.TableSchema {
Field: "ID",
},
},
"canonical_id": &memdb.IndexSchema{
Name: "canonical_id",
Unique: false,
Indexer: &memdb.StringFieldIndex{
Field: "CanonicalID",
},
},
"factors": &memdb.IndexSchema{
Name: "factors",
Unique: true,

828
vault/identity_store_util.go
View File

File diff suppressed because it is too large Load Diff