Skip to content

Commit

Permalink
Do not fail mc-admin-policy-attach if policy already attached/detached
Browse files Browse the repository at this point in the history
Currently, attempts to attach a policy to a user who already has
the policy attached to them results in a 400.

This change is to handle cases where policy attach/detach operations
are automated using scripts/jobs. A re-run of the attach/detach
step should not result in the failure of the entire job.
  • Loading branch information
dhananjaykrutika committed Oct 8, 2024
1 parent 07eeb10 commit 6b716f8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
20 changes: 19 additions & 1 deletion cmd/admin-policy-attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ import (
"github.com/minio/mc/pkg/probe"
)

const (
errCodeChangeAlreadyApplied = "XMinioAdminPolicyChangeAlreadyApplied"
)

var adminAttachPolicyFlags = []cli.Flag{
cli.StringFlag{
Name: "user, u",
Expand Down Expand Up @@ -68,6 +72,17 @@ func mainAdminPolicyAttach(ctx *cli.Context) error {
return userAttachOrDetachPolicy(ctx, true)
}

func isCodeChangeAlreadyAppliedError(e error) bool {
switch v := e.(type) {
case madmin.ErrorResponse:
if v.Code == errCodeChangeAlreadyApplied {
return true
}
}

return false
}

func userAttachOrDetachPolicy(ctx *cli.Context, attach bool) error {
if len(ctx.Args()) < 2 {
showCommandHelpAndExit(ctx, 1) // last argument is exit code
Expand Down Expand Up @@ -97,7 +112,10 @@ func userAttachOrDetachPolicy(ctx *cli.Context, attach bool) error {
} else {
res, e = client.DetachPolicy(globalContext, req)
}
fatalIf(probe.NewError(e), "Unable to make user/group policy association")

if e != nil && !isCodeChangeAlreadyAppliedError(e) {
fatalIf(probe.NewError(e), "Unable to make user/group policy association")
}

var emptyResp madmin.PolicyAssociationResp
if res.UpdatedAt == emptyResp.UpdatedAt {
Expand Down
3 changes: 3 additions & 0 deletions functional-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,9 @@ function test_admin_users() {
# check that the user can write objects with readwrite policy
assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd admin policy attach "$SERVER_ALIAS" readwrite --user="${username}"

# verify that re-attaching an already attached policy to a user does not result in a failure.
assert_success "$start_time" "${FUNCNAME[0]}" mc_cmd admin policy attach "$SERVER_ALIAS" readwrite --user="${username}"

# Validate that the correct policy has been added to the user
"${MC_CMD[@]}" --json admin user list "${SERVER_ALIAS}" | jq -r '.policyName' | grep --quiet "^readwrite$"
rv=$?
Expand Down

0 comments on commit 6b716f8

Please sign in to comment.