diff options
author | Ryan Brown <sb@ryansb.com> | 2016-08-30 10:24:00 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-08-30 10:24:00 -0400 |
commit | 0c37949941146e9b2a435009011ab5165f8da23d (patch) | |
tree | 7c01aa2d82511a8848b3f275483045bd6b03e828 | |
parent | 978716cf4c177f09f1e07d29d103fed27575846d (diff) | |
download | ansible-modules-core-0c37949941146e9b2a435009011ab5165f8da23d.tar.gz |
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
-rw-r--r-- | cloud/amazon/iam_policy.py | 23 |
1 files changed, 11 insertions, 12 deletions
diff --git a/cloud/amazon/iam_policy.py b/cloud/amazon/iam_policy.py index ce0c06a8..e1cc6b30 100644 --- a/cloud/amazon/iam_policy.py +++ b/cloud/amazon/iam_policy.py @@ -139,7 +139,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 @@ -147,13 +147,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': @@ -191,18 +191,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': @@ -236,20 +236,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': |