From c85d854c84677f7da3fd6f0e1de8d2ae4ed428e3 Mon Sep 17 00:00:00 2001 From: Ryan Brown Date: Tue, 30 Aug 2016 10:24:00 -0400 Subject: [PATCH] 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 --- .../modules/cloud/amazon/iam_policy.py | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/iam_policy.py b/lib/ansible/modules/cloud/amazon/iam_policy.py index 4ab1e0c75c3..37b413dd05f 100644 --- a/lib/ansible/modules/cloud/amazon/iam_policy.py +++ b/lib/ansible/modules/cloud/amazon/iam_policy.py @@ -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':