identity: Introduce ConflictResolver interface (#29187)

This PR introduces a new interface for conflict resolution of duplicate
Identity artifacts. The initial implementation just reorganizes the code
to use the interface with no behavior change.

The interface is intended to provide a minimal touchpoint for
implementing new conflict resolution behavior. Since those changes will
also introduce significant testcases, the aim here is to merge the new
interface and ensure the current code works as intended (according to
existing tests).
This commit is contained in:
Mike Palmiotto 2024-12-16 10:46:36 -05:00 committed by GitHub
parent 2076decac0
commit 84dac8486a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 110 additions and 17 deletions

View file

@ -0,0 +1,85 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1
package vault
import (
"context"
"errors"
log "github.com/hashicorp/go-hclog"
"github.com/hashicorp/vault/helper/identity"
)
var errDuplicateIdentityName = errors.New("duplicate identity name")
// ConflictResolver defines the interface for resolving conflicts between
// entities, groups, and aliases. All methods should implement a check for
// existing=nil. This is an intentional design choice to allow the caller to
// search for extra information if necessary.
type ConflictResolver interface {
ResolveEntities(ctx context.Context, existing, duplicate *identity.Entity) error
ResolveGroups(ctx context.Context, existing, duplicate *identity.Group) error
ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error
}
// The errorResolver is a ConflictResolver that logs a warning message when a
// pre-existing Identity artifact is found with the same factors as a new one.
type errorResolver struct {
logger log.Logger
}
// ResolveEntities logs a warning message when a pre-existing Entity is found
// and returns a duplicate name error, which should be handled by the caller by
// putting the system in case-sensitive mode.
func (r *errorResolver) ResolveEntities(ctx context.Context, existing, duplicate *identity.Entity) error {
if existing == nil {
return nil
}
r.logger.Warn(errDuplicateIdentityName.Error(),
"entity_name", duplicate.Name,
"duplicate_of_name", existing.Name,
"duplicate_of_id", existing.ID,
"action", "merge the duplicate entities into one")
return errDuplicateIdentityName
}
// ResolveGroups logs a warning message when a pre-existing Group is found and
// returns a duplicate name error, which should be handled by the caller by
// putting the system in case-sensitive mode.
func (r *errorResolver) ResolveGroups(ctx context.Context, existing, duplicate *identity.Group) error {
if existing == nil {
return nil
}
r.logger.Warn(errDuplicateIdentityName.Error(),
"group_name", duplicate.Name,
"duplicate_of_name", existing.Name,
"duplicate_of_id", existing.ID,
"action", "merge the contents of duplicated groups into one and delete the other")
return errDuplicateIdentityName
}
// ResolveAliases logs a warning message when a pre-existing Alias is found and
// returns a duplicate name error, which should be handled by the caller by
// putting the system in case-sensitive mode.
func (r *errorResolver) ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error {
if existing == nil {
return nil
}
r.logger.Warn(errDuplicateIdentityName.Error(),
"alias_name", duplicate.Name,
"mount_accessor", duplicate.MountAccessor,
"local", duplicate.Local,
"entity_name", parent.Name,
"alias_canonical_id", duplicate.CanonicalID,
"duplicate_of_name", existing.Name,
"duplicate_of_canonical_id", existing.CanonicalID,
"action", "merge the canonical entity IDs into one")
return errDuplicateIdentityName
}

View file

@ -110,6 +110,8 @@ type IdentityStore struct {
// aliasLocks is used to protect modifications to alias entries based on the uniqueness factor
// which is name + accessor
aliasLocks []*locksutil.LockEntry
conflictResolver ConflictResolver
}
type groupDiff struct {

View file

@ -28,9 +28,8 @@ import (
)
var (
errDuplicateIdentityName = errors.New("duplicate identity name")
errCycleDetectedPrefix = "cyclic relationship detected for member group ID"
tmpSuffix = ".tmp"
errCycleDetectedPrefix = "cyclic relationship detected for member group ID"
tmpSuffix = ".tmp"
)
func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
@ -39,6 +38,12 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error {
return nil
}
// Resolve all conflicts by logging a warning and returning
// errDuplicateIdentityName. The error will flip the identityStore into
// case-sensitive mode by switching the underlying schema to one with a
// relaxed lowerCase constraint and reload all artifacts into MemDB.
c.identityStore.conflictResolver = &errorResolver{c.identityStore.logger}
loadFunc := func(context.Context) error {
if err := c.identityStore.loadEntities(ctx); err != nil {
return err
@ -141,11 +146,8 @@ func (i *IdentityStore) loadGroups(ctx context.Context) error {
if err != nil {
return err
}
if groupByName != nil {
i.logger.Warn(errDuplicateIdentityName.Error(), "group_name", group.Name, "conflicting_group_name", groupByName.Name, "action", "merge the contents of duplicated groups into one and delete the other")
if !i.disableLowerCasedNames {
return errDuplicateIdentityName
}
if err := i.conflictResolver.ResolveGroups(ctx, groupByName, group); err != nil && !i.disableLowerCasedNames {
return err
}
if i.logger.IsDebug() {
@ -468,11 +470,8 @@ LOOP:
if err != nil {
return nil
}
if entityByName != nil {
i.logger.Warn(errDuplicateIdentityName.Error(), "entity_name", entity.Name, "conflicting_entity_name", entityByName.Name, "action", "merge the duplicate entities into one")
if !i.disableLowerCasedNames {
return errDuplicateIdentityName
}
if err := i.conflictResolver.ResolveEntities(ctx, entityByName, entity); err != nil && !i.disableLowerCasedNames {
return err
}
mountAccessors := getAccessorsOnDuplicateAliases(entity.Aliases)
@ -622,7 +621,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)
// Though this is technically a conflict that should be resolved by the
// ConflictResolver implementation, the behavior here is a bit nuanced.
// Rather than introduce a behavior change, handle this case directly as
// before by merging.
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.mergeEntityAsPartOfUpsert(ctx, txn, entity, aliasByFactors.CanonicalID, persist)
switch {
@ -638,9 +645,8 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
}
if strutil.StrListContains(aliasFactors, i.sanitizeName(alias.Name)+alias.MountAccessor) {
i.logger.Warn(errDuplicateIdentityName.Error(), "alias_name", alias.Name, "mount_accessor", alias.MountAccessor, "local", alias.Local, "entity_name", entity.Name, "action", "delete one of the duplicate aliases")
if !i.disableLowerCasedNames {
return errDuplicateIdentityName
if err := i.conflictResolver.ResolveAliases(ctx, entity, aliasByFactors, alias); err != nil && !i.disableLowerCasedNames {
return err
}
}