fix: skip policy usage validation for cache update (#19008)

When updating the policy cache, we do not need to validate policy usage
as the policy has already been deleted by the node sending the
notification.
This commit is contained in:
Aditya Manthramurthy 2024-02-07 20:39:53 -08:00 committed by GitHub
parent 7e082f232e
commit e104b183d8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 42 additions and 35 deletions

View File

@ -1166,8 +1166,11 @@ func (store *IAMStoreSys) PolicyNotificationHandler(ctx context.Context, policy
return err return err
} }
// DeletePolicy - deletes policy from storage and cache. // DeletePolicy - deletes policy from storage and cache. When this called in
func (store *IAMStoreSys) DeletePolicy(ctx context.Context, policy string) error { // response to a notification (i.e. isFromNotification = true), it skips the
// validation of policy usage and the attempt to delete in the backend as well
// (as this is already done by the notifying node).
func (store *IAMStoreSys) DeletePolicy(ctx context.Context, policy string, isFromNotification bool) error {
if policy == "" { if policy == "" {
return errInvalidArgument return errInvalidArgument
} }
@ -1175,42 +1178,44 @@ func (store *IAMStoreSys) DeletePolicy(ctx context.Context, policy string) error
cache := store.lock() cache := store.lock()
defer store.unlock() defer store.unlock()
// Check if policy is mapped to any existing user or group. If so, we do not if !isFromNotification {
// allow deletion of the policy. If the policy is mapped to an STS account, // Check if policy is mapped to any existing user or group. If so, we do not
// we do allow deletion. // allow deletion of the policy. If the policy is mapped to an STS account,
users := []string{} // we do allow deletion.
groups := []string{} users := []string{}
for u, mp := range cache.iamUserPolicyMap { groups := []string{}
pset := mp.policySet() for u, mp := range cache.iamUserPolicyMap {
if store.getUsersSysType() == MinIOUsersSysType { pset := mp.policySet()
if _, ok := cache.iamUsersMap[u]; !ok { if store.getUsersSysType() == MinIOUsersSysType {
// This case can happen when a temporary account is if _, ok := cache.iamUsersMap[u]; !ok {
// deleted or expired - remove it from userPolicyMap. // This case can happen when a temporary account is
delete(cache.iamUserPolicyMap, u) // deleted or expired - remove it from userPolicyMap.
continue delete(cache.iamUserPolicyMap, u)
continue
}
}
if pset.Contains(policy) {
users = append(users, u)
} }
} }
if pset.Contains(policy) { for g, mp := range cache.iamGroupPolicyMap {
users = append(users, u) pset := mp.policySet()
if pset.Contains(policy) {
groups = append(groups, g)
}
} }
} if len(users) != 0 || len(groups) != 0 {
for g, mp := range cache.iamGroupPolicyMap { return errPolicyInUse
pset := mp.policySet()
if pset.Contains(policy) {
groups = append(groups, g)
} }
}
if len(users) != 0 || len(groups) != 0 {
return errPolicyInUse
}
err := store.deletePolicyDoc(ctx, policy) err := store.deletePolicyDoc(ctx, policy)
if errors.Is(err, errNoSuchPolicy) { if errors.Is(err, errNoSuchPolicy) {
// Ignore error if policy is already deleted. // Ignore error if policy is already deleted.
err = nil err = nil
} }
if err != nil { if err != nil {
return err return err
}
} }
delete(cache.iamPolicyDocsMap, policy) delete(cache.iamPolicyDocsMap, policy)

View File

@ -526,7 +526,9 @@ func (sys *IAMSys) GetRolePolicy(arnStr string) (arn.ARN, string, error) {
return roleArn, rolePolicy, nil return roleArn, rolePolicy, nil
} }
// DeletePolicy - deletes a canned policy from backend or etcd. // DeletePolicy - deletes a canned policy from backend. `notifyPeers` is true
// whenever this is called via the API. It is false when called via a
// notification from another peer. This is to avoid infinite loops.
func (sys *IAMSys) DeletePolicy(ctx context.Context, policyName string, notifyPeers bool) error { func (sys *IAMSys) DeletePolicy(ctx context.Context, policyName string, notifyPeers bool) error {
if !sys.Initialized() { if !sys.Initialized() {
return errServerNotInitialized return errServerNotInitialized
@ -540,7 +542,7 @@ func (sys *IAMSys) DeletePolicy(ctx context.Context, policyName string, notifyPe
} }
} }
err := sys.store.DeletePolicy(ctx, policyName) err := sys.store.DeletePolicy(ctx, policyName, !notifyPeers)
if err != nil { if err != nil {
return err return err
} }