Remove spurious `changed` state on iam_policy module (#4381)

Due to a mixup of the group/role/user and policy names, policies with
the same name as the group/role/user they are attached to would never be
updated after creation. To fix that, we needed two changes to the logic
of policy comparison:

- Compare the new policy name to *all* matching policies, not just the
  first in lexicographical order
- Compare the new policy name to the matching ones, not to the IAM
  object the policy is attached to
pull/18777/head
Ryan Brown 8 years ago committed by Matt Clay
parent 4d75f9b4bc
commit c85d854c84

@ -137,7 +137,7 @@ def user_action(module, iam, name, policy_name, skip, pdoc, state):
current_policies = [cp for cp in iam.get_all_user_policies(name).
list_user_policies_result.
policy_names]
pol = ""
matching_policies = []
for pol in current_policies:
'''
urllib is needed here because boto returns url encoded strings instead
@ -145,13 +145,13 @@ def user_action(module, iam, name, policy_name, skip, pdoc, state):
if urllib.unquote(iam.get_user_policy(name, pol).
get_user_policy_result.policy_document) == pdoc:
policy_match = True
break
matching_policies.append(pol)
if state == 'present':
# If policy document does not already exist (either it's changed
# or the policy is not present) or if we're not skipping dupes then
# make the put call. Note that the put call does a create or update.
if (not policy_match or not skip) and pol != name:
if not policy_match or (not skip and policy_name not in matching_policies):
changed = True
iam.put_user_policy(name, policy_name, pdoc)
elif state == 'absent':
@ -189,18 +189,18 @@ def role_action(module, iam, name, policy_name, skip, pdoc, state):
module.fail_json(msg=e.message)
try:
pol = ""
matching_policies = []
for pol in current_policies:
if urllib.unquote(iam.get_role_policy(name, pol).
get_role_policy_result.policy_document) == pdoc:
policy_match = True
break
matching_policies.append(pol)
if state == 'present':
# If policy document does not already exist (either it's changed
# or the policy is not present) or if we're not skipping dupes then
# make the put call. Note that the put call does a create or update.
if (not policy_match or not skip) and pol != name:
if not policy_match or (not skip and policy_name not in matching_policies):
changed = True
iam.put_role_policy(name, policy_name, pdoc)
elif state == 'absent':
@ -234,20 +234,19 @@ def group_action(module, iam, name, policy_name, skip, pdoc, state):
current_policies = [cp for cp in iam.get_all_group_policies(name).
list_group_policies_result.
policy_names]
pol = ""
matching_policies = []
for pol in current_policies:
if urllib.unquote(iam.get_group_policy(name, pol).
get_group_policy_result.policy_document) == pdoc:
policy_match = True
if policy_match:
msg=("The policy document you specified already exists "
"under the name %s." % pol)
break
matching_policies.append(pol)
msg=("The policy document you specified already exists "
"under the name %s." % pol)
if state == 'present':
# If policy document does not already exist (either it's changed
# or the policy is not present) or if we're not skipping dupes then
# make the put call. Note that the put call does a create or update.
if (not policy_match or not skip) and pol != name:
if not policy_match or (not skip and policy_name not in matching_policies):
changed = True
iam.put_group_policy(name, policy_name, pdoc)
elif state == 'absent':

Loading…
Cancel
Save