Fix iam_password_policy integration tests (#60930)

* iam_password_policy: (integration tests) Use module defaults for AWS connection details

* iam_password_policy: (integration tests) Ensure the policy is removed when tests fail

* iam_password_policy: (integration tests) Add regression test for #59102

* iam_password_policy: Only return changed when the policy changes.

* iam_password_policy: PasswordReusePrevention must be omitted to remove/set to 0

* #60930 add changelog

* Update hacking AWS security policy to allow testing of Password Policy Management

changelogs/fragments/60930-iam_password_policy.yml

@@ -0,0 +1,3 @@
+bugfixes:
+- iam_password_policy now only returns changed when the policy changes
+- iam_password_policy no longer throws errors when you don't set pw_reuse_prevent

hacking/aws_config/testing_policies/security-policy.json

@@ -128,6 +128,16 @@
                 "iam:GetServerCertificate"
             ],
             "Resource": "*"
+        },
+        {
+            "Sid": "AllowAccessToManagePasswordPolicy",
+            "Effect": "Allow",
+            "Action": [
+                "iam:GetAccountPasswordPolicy",
+                "iam:DeleteAccountPasswordPolicy",
+                "iam:UpdateAccountPasswordPolicy"
+            ],
+            "Resource": "*"
         }
     ]
 }

lib/ansible/modules/cloud/amazon/iam_password_policy.py

@@ -109,7 +109,6 @@ from ansible.module_utils.ec2 import camel_dict_to_snake_dict, boto3_tag_list_to
 
 
 class IAMConnection(object):
-
     def __init__(self, module):
         try:
             self.connection = module.resource('iam')
@@ -117,6 +116,17 @@ class IAMConnection(object):
         except Exception as e:
             module.fail_json(msg="Failed to connect to AWS: %s" % str(e))
 
+    def policy_to_dict(self, policy):
+        policy_attributes = [
+            'allow_users_to_change_password', 'expire_passwords', 'hard_expiry',
+            'max_password_age', 'minimum_password_length', 'password_reuse_prevention',
+            'require_lowercase_characters', 'require_numbers', 'require_symbols', 'require_uppercase_characters'
+        ]
+        ret = {}
+        for attr in policy_attributes:
+            ret[attr] = getattr(policy, attr)
+        return ret
+
     def update_password_policy(self, module, policy):
         min_pw_length = module.params.get('min_pw_length')
         require_symbols = module.params.get('require_symbols')
@@ -135,18 +145,27 @@ class IAMConnection(object):
             RequireUppercaseCharacters=require_uppercase,
             RequireLowercaseCharacters=require_lowercase,
             AllowUsersToChangePassword=allow_pw_change,
-            PasswordReusePrevention=pw_reuse_prevent,
             HardExpiry=pw_expire
         )
+        if pw_reuse_prevent:
+            update_parameters.update(PasswordReusePrevention=pw_reuse_prevent)
         if pw_max_age:
             update_parameters.update(MaxPasswordAge=pw_max_age)
 
+        try:
+            original_policy = self.policy_to_dict(policy)
+        except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
+            original_policy = {}
+
         try:
             results = policy.update(**update_parameters)
             policy.reload()
+            updated_policy = self.policy_to_dict(policy)
         except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
             self.module.fail_json_aws(e, msg="Couldn't update IAM Password Policy")
-        return camel_dict_to_snake_dict(results)
+
+        changed = (original_policy != updated_policy)
+        return (changed, updated_policy, camel_dict_to_snake_dict(results))
 
     def delete_password_policy(self, policy):
         try:
@@ -182,8 +201,8 @@ def main():
     state = module.params.get('state')
 
     if state == 'present':
-        update_result = resource.update_password_policy(module, policy)
+        (changed, new_policy, update_result) = resource.update_password_policy(module, policy)
-        module.exit_json(changed=True, task_status={'IAM': update_result})
+        module.exit_json(changed=changed, task_status={'IAM': update_result}, policy=new_policy)
 
     if state == 'absent':
         delete_result = resource.delete_password_policy(policy)

test/integration/targets/iam_password_policy/tasks/main.yaml

@@ -1,90 +1,105 @@
-- name: set connection information for all tasks
+- module_defaults:
-  set_fact:
+    group/aws:
-    aws_connection_info: &aws_connection_info
       aws_access_key: "{{ aws_access_key }}"
       aws_secret_key: "{{ aws_secret_key }}"
-      security_token: "{{ security_token }}"
+      security_token: "{{ security_token | default(omit) }}"
       region: "{{ aws_region }}"
-  no_log: true
+  block:
+  - name: set iam password policy
+    iam_password_policy:
+      state: present
+      min_pw_length: 8
+      require_symbols: false
+      require_numbers: true
+      require_uppercase: true
+      require_lowercase: true
+      allow_pw_change: true
+      pw_max_age: 60
+      pw_reuse_prevent: 5
+      pw_expire: false
+    register: result
 
-- name: set iam password policy
+  - name: assert that changes were made
-  iam_password_policy:
+    assert:
-    <<: *aws_connection_info
+      that:
-    state: present
+        - result.changed
-    min_pw_length: 8
-    require_symbols: false
-    require_numbers: true
-    require_uppercase: true
-    require_lowercase: true
-    allow_pw_change: true
-    pw_max_age: 60
-    pw_reuse_prevent: 5
-    pw_expire: false
-  register: result
 
-- name: assert that changes were made
+  - name: verify iam password policy has been created
-  assert:
+    iam_password_policy:
-    that:
+      state: present
-      - result.changed
+      min_pw_length: 8
+      require_symbols: false
+      require_numbers: true
+      require_uppercase: true
+      require_lowercase: true
+      allow_pw_change: true
+      pw_max_age: 60
+      pw_reuse_prevent: 5
+      pw_expire: false
+    register: result
 
-- name: verify iam password policy has been created
+  - name: assert that no changes were made
-  iam_password_policy:
+    assert:
-    <<: *aws_connection_info
+      that:
-    state: present
+        - not result.changed
-    min_pw_length: 8
-    require_symbols: false
-    require_numbers: true
-    require_uppercase: true
-    require_lowercase: true
-    allow_pw_change: true
-    pw_max_age: 60
-    pw_reuse_prevent: 5
-    pw_expire: false
-  register: result
 
-- name: assert that no changes were made
+  - name: update iam password policy with different settings
-  assert:
+    iam_password_policy:
-    that:
+      state: present
-      - not result.changed
+      min_pw_length: 15
+      require_symbols: true
+      require_numbers: true
+      require_uppercase: true
+      require_lowercase: true
+      allow_pw_change: true
+      pw_max_age: 30
+      pw_reuse_prevent: 10
+      pw_expire: true
+    register: result
 
-- name: update iam password policy
+  - name: assert that updates were made
-  iam_password_policy:
+    assert:
-    <<: *aws_connection_info
+      that:
-    state: present
+        - result.changed
-    min_pw_length: 15
-    require_symbols: true
-    require_numbers: true
-    require_uppercase: true
-    require_lowercase: true
-    allow_pw_change: true
-    pw_max_age: 30
-    pw_reuse_prevent: 10
-    pw_expire: true
-  register: result
 
-- name: assert that updates were made
+  # Test for regression of #59102
-  assert:
+  - name: update iam password policy without expiry
-    that:
+    iam_password_policy:
-      - result.changed
+      state: present
+      min_pw_length: 15
+      require_symbols: true
+      require_numbers: true
+      require_uppercase: true
+      require_lowercase: true
+      allow_pw_change: true
+    register: result
 
-- name: remove iam password policy
+  - name: assert that changes were made
-  iam_password_policy:
+    assert:
-    <<: *aws_connection_info
+      that:
-    state: absent
+        - result.changed
-  register: result
 
-- name: assert password policy has been removed
+  - name: remove iam password policy
-  assert:
+    iam_password_policy:
-    that:
+      state: absent
-      - result.changed
+    register: result
 
-- name: verify password policy has been removed
+  - name: assert password policy has been removed
-  iam_password_policy:
+    assert:
-    <<: *aws_connection_info
+      that:
-    state: absent
+        - result.changed
-  register: result
 
-- name: assert no changes were made
+  - name: verify password policy has been removed
-  assert:
+    iam_password_policy:
-    that:
+      state: absent
-      - not result.changed
+    register: result
+
+  - name: assert no changes were made
+    assert:
+      that:
+        - not result.changed
+  always:
+  - name: remove iam password policy
+    iam_password_policy:
+      state: absent
+    register: result