Skip to content

Commit

Permalink
Merge entities during unseal only on the primary (#6075)
Browse files Browse the repository at this point in the history 
* Merge entities during unseal only on the primary

* Add another guard check

* Add perf standby to the check

* Make primary to not differ from case-insensitivity status w.r.t secondaries

* Ensure mutual exclusivity between loading and invalidations

* Both primary and secondaries won't persist during startup and invalidations

* Allow primary to persist when loading case sensitively

* Using core.perfStandby

* Add a tweak in core for testing

* Address review feedback

* update memdb but not storage in secondaries

* Wire all the things directly do mergeEntity

* Fix persist behavior

* Address review feedback
  • Loading branch information
@vishalnayak
vishalnayak authored Feb 8, 2019
1 parent 06fffd7 commit 0ce70b8
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 80 deletions.
4 changes: 4 additions & 0 deletions vault/core.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,10 @@ type Core struct {
// Can be toggled atomically to cause the core to never try to become
// active, or give up active as soon as it gets it
neverBecomeActive *uint32

// loadCaseSensitiveIdentityStore enforces the loading of identity store
// artifacts in a case sensitive manner. To be used only in testing.
loadCaseSensitiveIdentityStore bool
}

// CoreConfig is used to parameterize a core
Expand Down
42 changes: 24 additions & 18 deletions vault/identity_store_entities.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/golang/protobuf/ptypes"
"github.com/hashicorp/errwrap"
memdb "github.com/hashicorp/go-memdb"
"github.com/hashicorp/vault/helper/consts"
"github.com/hashicorp/vault/helper/identity"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/helper/storagepacker"
Expand Down Expand Up @@ -155,7 +156,7 @@ func (i *IdentityStore) pathEntityMergeID() framework.OperationFunc {
return nil, err
}

userErr, intErr := i.mergeEntity(ctx, txn, toEntity, fromEntityIDs, force, true, false)
userErr, intErr := i.mergeEntity(ctx, txn, toEntity, fromEntityIDs, force, true, false, true)
if userErr != nil {
return logical.ErrorResponse(userErr.Error()), nil
}
Expand Down Expand Up @@ -604,7 +605,7 @@ func (i *IdentityStore) handlePathEntityListCommon(ctx context.Context, req *log
return logical.ListResponseWithInfo(keys, entityInfo), nil
}

func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntity *identity.Entity, fromEntityIDs []string, force, grabLock, mergePolicies bool) (error, error) {
func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntity *identity.Entity, fromEntityIDs []string, force, grabLock, mergePolicies, persist bool) (error, error) {
if grabLock {
i.lock.Lock()
defer i.lock.Unlock()
Expand Down Expand Up @@ -651,6 +652,7 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
}
}

isPerfSecondaryOrStandby := i.core.ReplicationState().HasState(consts.ReplicationPerformanceSecondary) || i.core.perfStandby
for _, fromEntityID := range fromEntityIDs {
if fromEntityID == toEntity.ID {
return errors.New("to_entity_id should not be present in from_entity_ids"), nil
Expand Down Expand Up @@ -704,10 +706,12 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
return nil, err
}

// Delete the entity which we are merging from in storage
err = i.entityPacker.DeleteItem(fromEntity.ID)
if err != nil {
return nil, err
if persist && !isPerfSecondaryOrStandby {
// Delete the entity which we are merging from in storage
err = i.entityPacker.DeleteItem(fromEntity.ID)
if err != nil {
return nil, err
}
}
}

Expand All @@ -717,19 +721,21 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
return nil, err
}

// Persist the entity which we are merging to
toEntityAsAny, err := ptypes.MarshalAny(toEntity)
if err != nil {
return nil, err
}
item := &storagepacker.Item{
ID: toEntity.ID,
Message: toEntityAsAny,
}
if persist && !isPerfSecondaryOrStandby {
// Persist the entity which we are merging to
toEntityAsAny, err := ptypes.MarshalAny(toEntity)
if err != nil {
return nil, err
}
item := &storagepacker.Item{
ID: toEntity.ID,
Message: toEntityAsAny,
}

err = i.entityPacker.PutItem(item)
if err != nil {
return nil, err
err = i.entityPacker.PutItem(item)
if err != nil {
return nil, err
}
}

return nil, nil
Expand Down
152 changes: 113 additions & 39 deletions vault/identity_store_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,98 @@ import (
"github.com/hashicorp/vault/logical"
)

func TestIdentityStore_UnsealingWhenConflictingAliasNames(t *testing.T) {
err := AddTestCredentialBackend("github", credGithub.Factory)
if err != nil {
t.Fatalf("err: %s", err)
}

c, unsealKey, root := TestCoreUnsealed(t)

meGH := &MountEntry{
Table: credentialTableType,
Path: "github/",
Type: "github",
Description: "github auth",
}

err = c.enableCredential(namespace.RootContext(nil), meGH)
if err != nil {
t.Fatal(err)
}

alias := &identity.Alias{
ID: "alias1",
CanonicalID: "entity1",
MountType: "github",
MountAccessor: meGH.Accessor,
Name: "githubuser",
}
entity := &identity.Entity{
ID: "entity1",
Name: "name1",
Policies: []string{"foo", "bar"},
Aliases: []*identity.Alias{
alias,
},
NamespaceID: namespace.RootNamespaceID,
}
entity.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity.ID)

err = c.identityStore.upsertEntity(namespace.RootContext(nil), entity, nil, true)
if err != nil {
t.Fatal(err)
}

alias2 := &identity.Alias{
ID: "alias2",
CanonicalID: "entity2",
MountType: "github",
MountAccessor: meGH.Accessor,
Name: "GITHUBUSER",
}
entity2 := &identity.Entity{
ID: "entity2",
Name: "name2",
Policies: []string{"foo", "bar"},
Aliases: []*identity.Alias{
alias2,
},
NamespaceID: namespace.RootNamespaceID,
}
entity2.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity2.ID)

// Persist the second entity directly without the regular flow. This will skip
// merging of these enties.
entity2Any, err := ptypes.MarshalAny(entity2)
if err != nil {
t.Fatal(err)
}
item := &storagepacker.Item{
ID: entity2.ID,
Message: entity2Any,
}
if err = c.identityStore.entityPacker.PutItem(item); err != nil {
t.Fatal(err)
}

// Seal and ensure that unseal works
if err = c.Seal(root); err != nil {
t.Fatal(err)
}

var unsealed bool
for i := 0; i < 3; i++ {
unsealed, err = c.Unseal(unsealKey[i])
if err != nil {
t.Fatal(err)
}
}
if !unsealed {
t.Fatal("still sealed")
}
}

func TestIdentityStore_EntityIDPassthrough(t *testing.T) {
// Enable GitHub auth and initialize
ctx := namespace.RootContext(nil)
Expand Down Expand Up @@ -381,7 +473,7 @@ func TestIdentityStore_MergeConflictingAliases(t *testing.T) {
t.Fatalf("err: %s", err)
}

c, unsealKey, root := TestCoreUnsealed(t)
c, _, _ := TestCoreUnsealed(t)

meGH := &MountEntry{
Table: credentialTableType,
Expand Down Expand Up @@ -409,54 +501,36 @@ func TestIdentityStore_MergeConflictingAliases(t *testing.T) {
Aliases: []*identity.Alias{
alias,
},
NamespaceID: namespace.RootNamespaceID,
}
entity.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity.ID)
// Now add the alias to two entities, skipping all existing checking by
// writing directly
entityAny, err := ptypes.MarshalAny(entity)
err = c.identityStore.upsertEntity(namespace.RootContext(nil), entity, nil, true)
if err != nil {
t.Fatal(err)
}
item := &storagepacker.Item{
ID: entity.ID,
Message: entityAny,
}
if err = c.identityStore.entityPacker.PutItem(item); err != nil {
t.Fatal(err)
}

entity.ID = "entity2"
entity.Name = "name2"
entity.Policies = []string{"bar", "baz"}
alias.ID = "alias2"
alias.CanonicalID = "entity2"
entity.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity.ID)
entityAny, err = ptypes.MarshalAny(entity)
if err != nil {
t.Fatal(err)
}
item = &storagepacker.Item{
ID: entity.ID,
Message: entityAny,
alias2 := &identity.Alias{
ID: "alias2",
CanonicalID: "entity2",
MountType: "github",
MountAccessor: meGH.Accessor,
Name: "githubuser",
}
if err = c.identityStore.entityPacker.PutItem(item); err != nil {
t.Fatal(err)
entity2 := &identity.Entity{
ID: "entity2",
Name: "name2",
Policies: []string{"bar", "baz"},
Aliases: []*identity.Alias{
alias2,
},
NamespaceID: namespace.RootNamespaceID,
}

// Seal and unseal. If things are broken, we will now fail to unseal.
if err = c.Seal(root); err != nil {
t.Fatal(err)
}
entity2.BucketKeyHash = c.identityStore.entityPacker.BucketKeyHashByItemID(entity2.ID)

var unsealed bool
for i := 0; i < 3; i++ {
unsealed, err = c.Unseal(unsealKey[i])
if err != nil {
t.Fatal(err)
}
}
if !unsealed {
t.Fatal("still sealed")
err = c.identityStore.upsertEntity(namespace.RootContext(nil), entity2, nil, true)
if err != nil {
t.Fatal(err)
}

newEntity, err := c.identityStore.CreateOrFetchEntity(namespace.RootContext(nil), &logical.Alias{
Expand Down
55 changes: 32 additions & 23 deletions vault/identity_store_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ var (
errDuplicateIdentityName = errors.New("duplicate identity name")
)

func (c *Core) SetLoadCaseSensitiveIdentityStore(caseSensitive bool) {
c.loadCaseSensitiveIdentityStore = caseSensitive
}

func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
if c.identityStore == nil {
c.logger.Warn("identity store is not setup, skipping loading")
Expand All @@ -38,14 +42,16 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
return c.identityStore.loadGroups(ctx)
}

// Load everything when memdb is set to operate on lower cased names
err := loadFunc(ctx)
switch {
case err == nil:
// If it succeeds, all is well
return nil
case err != nil && !errwrap.Contains(err, errDuplicateIdentityName.Error()):
return err
if !c.loadCaseSensitiveIdentityStore {
// Load everything when memdb is set to operate on lower cased names
err := loadFunc(ctx)
switch {
case err == nil:
// If it succeeds, all is well
return nil
case err != nil && !errwrap.Contains(err, errDuplicateIdentityName.Error()):
return err
}
}

c.identityStore.logger.Warn("enabling case sensitive identity names")
Expand All @@ -56,8 +62,7 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
// Swap the memdb instance by the one which operates on case sensitive
// names, hence obviating the need to unload anything that's already
// loaded.
err = c.identityStore.resetDB(ctx)
if err != nil {
if err := c.identityStore.resetDB(ctx); err != nil {
return err
}

Expand Down Expand Up @@ -334,13 +339,15 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
fallthrough
default:
i.logger.Warn("alias is already tied to a different entity; these entities are being merged", "alias_id", alias.ID, "other_entity_id", aliasByFactors.CanonicalID, "entity_aliases", entity.Aliases, "alias_by_factors", aliasByFactors)
respErr, intErr := i.mergeEntity(ctx, txn, entity, []string{aliasByFactors.CanonicalID}, true, false, true)

respErr, intErr := i.mergeEntity(ctx, txn, entity, []string{aliasByFactors.CanonicalID}, true, false, true, persist)
switch {
case respErr != nil:
return respErr
case intErr != nil:
return intErr
}

// The entity and aliases will be loaded into memdb and persisted
// as a result of the merge so we are done here
return nil
Expand All @@ -363,23 +370,25 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
}

// If previous entity is set, update it in MemDB and persist it
if previousEntity != nil && persist {
if previousEntity != nil {
err = i.MemDBUpsertEntityInTxn(txn, previousEntity)
if err != nil {
return err
}

// Persist the previous entity object
marshaledPreviousEntity, err := ptypes.MarshalAny(previousEntity)
if err != nil {
return err
}
err = i.entityPacker.PutItem(&storagepacker.Item{
ID: previousEntity.ID,
Message: marshaledPreviousEntity,
})
if err != nil {
return err
if persist {
// Persist the previous entity object
marshaledPreviousEntity, err := ptypes.MarshalAny(previousEntity)
if err != nil {
return err
}
err = i.entityPacker.PutItem(&storagepacker.Item{
ID: previousEntity.ID,
Message: marshaledPreviousEntity,
})
if err != nil {
return err
}
}
}

Expand Down

0 comments on commit 0ce70b8

Please sign in to comment.