iam_role - delete inline policies, stabilize for migration to amazon.…

…aws (#1054) iam_role - delete inline policies, stabilize for migration to amazon.aws SUMMARY Stabilize for migration to amazon.aws delete inline policies before deleting the role removed global vars and refactored function definitions added some extra integration tests for check mode ISSUE TYPE Feature Pull Request COMPONENT NAME iam_role Reviewed-by: Markus Bergholz <git@osuv.de> Reviewed-by: Joseph Torcasso <None> Reviewed-by: Alina Buzachis <None> Reviewed-by: Jill R <None> (cherry picked from commit ce41867)
ansible-collections · Apr 13, 2022 · 5f770ae · 5f770ae
1 parent 6f0f283
commit 5f770ae
Show file tree

Hide file tree

Showing 12 changed files with 386 additions and 180 deletions.
diff --git a/changelogs/fragments/1054-iam_role-delete-inline-policies-and-refactor.yml b/changelogs/fragments/1054-iam_role-delete-inline-policies-and-refactor.yml
@@ -0,0 +1,3 @@
+minor_changes:
+  - iam_role - delete inline policies prior to deleting role (https://github.com/ansible-collections/community.aws/pull/1054).
+  - iam_role - remove global vars and refactor accordingly (https://github.com/ansible-collections/community.aws/pull/1054).
diff --git a/plugins/modules/iam_role.py b/plugins/modules/iam_role.py
diff --git a/tests/integration/targets/iam_role/tasks/boundary_policy.yml b/tests/integration/targets/iam_role/tasks/boundary_policy.yml
@@ -34,6 +34,18 @@
       - iam_role is changed
       - iam_role.iam_role.role_name == test_role
 
+- name: "Configure Boundary Policy (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    create_instance_profile: no
+    boundary: "{{ boundary_policy }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Configure Boundary Policy (no change)"
   iam_role:
     name: "{{ test_role }}"

diff --git a/tests/integration/targets/iam_role/tasks/complex_role_creation.yml b/tests/integration/targets/iam_role/tasks/complex_role_creation.yml
@@ -60,6 +60,27 @@
       - '"create_date" in iam_role.iam_role'
       - '"role_id" in iam_role.iam_role'
 
+- name: "Complex IAM role (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    assume_role_policy_document: '{{ lookup("file", "deny-assume.json") }}'
+    boundary: "{{ boundary_policy }}"
+    create_instance_profile: no
+    description: "Ansible Test Role {{ resource_prefix }}"
+    managed_policy:
+      - "{{ safe_managed_policy }}"
+      - "{{ custom_policy_name }}"
+    max_session_duration: 43200
+    path: "{{ test_path }}"
+    tags:
+      TagA: "ValueA"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Complex IAM role (no change)"
   iam_role:
     name: "{{ test_role }}"

diff --git a/tests/integration/targets/iam_role/tasks/creation_deletion.yml b/tests/integration/targets/iam_role/tasks/creation_deletion.yml
@@ -77,8 +77,8 @@
   iam_role:
     name: "{{ test_role }}"
     create_instance_profile: no
-  check_mode: yes
   register: iam_role
+  check_mode: yes
 
 - assert:
     that:
@@ -114,6 +114,17 @@
       - '"create_date" in iam_role.iam_role'
       - '"role_id" in iam_role.iam_role'
 
+- name: "Minimal IAM Role without instance profile (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    create_instance_profile: no
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Minimal IAM Role without instance profile (no change)"
   iam_role:
     name: "{{ test_role }}"
@@ -170,6 +181,8 @@
       - role_info is succeeded
       - role_info.iam_roles | length == 0
 
+# ------------------------------------------------------------------------------------------
+
 # (with path)
 - name: "Minimal IAM Role with path (CHECK MODE)"
   iam_role:
@@ -203,6 +216,17 @@
       - '"create_date" in iam_role.iam_role'
       - '"role_id" in iam_role.iam_role'
 
+- name: "Minimal IAM Role with path (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    path: "{{ test_path }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Minimal IAM Role with path (no change)"
   iam_role:
     name: "{{ test_role }}"
@@ -292,7 +316,20 @@
       - role_info is succeeded
       - role_info.iam_roles | length == 0
 
+# ------------------------------------------------------------------------------------------
+
 # (with Instance profile)
+- name: "Minimal IAM Role with instance profile - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    create_instance_profile: yes
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is changed
+
 - name: "Minimal IAM Role with instance profile"
   iam_role:
     name: "{{ test_role }}"
@@ -314,6 +351,17 @@
       - '"create_date" in iam_role.iam_role'
       - '"role_id" in iam_role.iam_role'
 
+- name: "Minimal IAM Role wth instance profile (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    create_instance_profile: yes
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Minimal IAM Role wth instance profile (no change)"
   iam_role:
     name: "{{ test_role }}"
@@ -350,4 +398,4 @@
       - '"permissions_boundary" not in role_info.iam_roles[0]'
       - role_info.iam_roles[0].role_id == iam_role.iam_role.role_id
       - role_info.iam_roles[0].role_name == test_role
-      - role_info.iam_roles[0].tags | length == 0
+      - role_info.iam_roles[0].tags | length == 0
diff --git a/tests/integration/targets/iam_role/tasks/description_update.yml b/tests/integration/targets/iam_role/tasks/description_update.yml
@@ -22,6 +22,17 @@
       - iam_role.iam_role.role_name == test_role
       - iam_role.iam_role.description == 'Ansible Test Role {{ resource_prefix }}'
 
+- name: "Add Description (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    description: "Ansible Test Role {{ resource_prefix }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Add Description (no change)"
   iam_role:
     name: "{{ test_role }}"
@@ -61,6 +72,8 @@
       - role_info.iam_roles[0].role_name == test_role
       - role_info.iam_roles[0].tags | length == 0
 
+# ------------------------------------------------------------------------------------------
+
 - name: "Update Description (CHECK MODE)"
   iam_role:
     name: "{{ test_role }}"
@@ -84,6 +97,17 @@
       - iam_role.iam_role.role_name == test_role
       - iam_role.iam_role.description == 'Ansible Test Role (updated) {{ resource_prefix }}'
 
+- name: "Update Description (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    description: "Ansible Test Role (updated) {{ resource_prefix }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Update Description (no change)"
   iam_role:
     name: "{{ test_role }}"

diff --git a/tests/integration/targets/iam_role/tasks/inline_policy_update.yml b/tests/integration/targets/iam_role/tasks/inline_policy_update.yml
@@ -46,18 +46,3 @@
       - role_info.iam_roles[0].tags | length == 1
       - '"TagB" in role_info.iam_roles[0].tags'
       - role_info.iam_roles[0].tags.TagB == "ValueB"
-
-# XXX iam_role fails to remove inline policies before deleting the role
-- name: "Detach inline policy a"
-  iam_policy:
-    state: absent
-    iam_type: "role"
-    iam_name: "{{ test_role }}"
-    policy_name: "inline-policy-a"
-
-- name: "Detach inline policy b"
-  iam_policy:
-    state: absent
-    iam_type: "role"
-    iam_name: "{{ test_role }}"
-    policy_name: "inline-policy-b"
diff --git a/tests/integration/targets/iam_role/tasks/main.yml b/tests/integration/targets/iam_role/tasks/main.yml
@@ -18,9 +18,7 @@
 # - There are some known timing issues with boto3 returning before actions
 #   complete in the case of problems with "changed" status it's worth enabling
 #   the standard_pauses and paranoid_pauses options as a first step in debugging
-#
-# Possible Bugs:
-# - Fails to delete role if inline policies not removed first
+
 
 - name: "Setup AWS connection info"
   module_defaults:
@@ -93,23 +91,6 @@
     # ===================================================================
     # Cleanup
 
-    # XXX iam_role fails to remove inline policies before deleting the role
-    - name: "Detach inline policy a"
-      iam_policy:
-        state: absent
-        iam_type: "role"
-        iam_name: "{{ test_role }}"
-        policy_name: "inline-policy-a"
-      ignore_errors: true
-
-    - name: "Detach inline policy b"
-      iam_policy:
-        state: absent
-        iam_type: "role"
-        iam_name: "{{ test_role }}"
-        policy_name: "inline-policy-b"
-      ignore_errors: true
-
     - name: "Remove IAM Role"
       iam_role:
         state: absent

diff --git a/tests/integration/targets/iam_role/tasks/max_session_update.yml b/tests/integration/targets/iam_role/tasks/max_session_update.yml
@@ -31,7 +31,17 @@
 - assert:
     that:
       - iam_role is not changed
-      - iam_role.iam_role.role_name == test_role
+
+- name: "Update Max Session Duration (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    max_session_duration: 43200
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
 
 - name: "iam_role_info after updating Max Session Duration"
   iam_role_info:

diff --git a/tests/integration/targets/iam_role/tasks/policy_update.yml b/tests/integration/targets/iam_role/tasks/policy_update.yml
@@ -25,6 +25,19 @@
       - iam_role is changed
       - iam_role.iam_role.role_name == test_role
 
+- name: "Add Managed Policy (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    purge_policies: no
+    managed_policy:
+      - "{{ safe_managed_policy }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Add Managed Policy (no change)"
   iam_role:
     name: "{{ test_role }}"
@@ -69,6 +82,8 @@
       - '"TagB" in role_info.iam_roles[0].tags'
       - role_info.iam_roles[0].tags.TagB == "ValueB"
 
+# ------------------------------------------------------------------------------------------
+
 - name: "Update Managed Policy without purge (CHECK MODE)"
   iam_role:
     name: "{{ test_role }}"
@@ -95,6 +110,19 @@
       - iam_role is changed
       - iam_role.iam_role.role_name == test_role
 
+- name: "Update Managed Policy without purge (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    purge_policies: no
+    managed_policy:
+      - "{{ custom_policy_name }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Update Managed Policy without purge (no change)"
   iam_role:
     name: "{{ test_role }}"
@@ -139,6 +167,8 @@
       - '"TagB" in role_info.iam_roles[0].tags'
       - role_info.iam_roles[0].tags.TagB == "ValueB"
 
+# ------------------------------------------------------------------------------------------
+
 # Managed Policies are purged by default
 - name: "Update Managed Policy with purge (CHECK MODE)"
   iam_role:
@@ -164,6 +194,18 @@
       - iam_role is changed
       - iam_role.iam_role.role_name == test_role
 
+- name: "Update Managed Policy with purge (no change) - check mode"
+  iam_role:
+    name: "{{ test_role }}"
+    managed_policy:
+      - "{{ custom_policy_name }}"
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Update Managed Policy with purge (no change)"
   iam_role:
     name: "{{ test_role }}"

diff --git a/tests/integration/targets/iam_role/tasks/role_removal.yml b/tests/integration/targets/iam_role/tasks/role_removal.yml
@@ -41,6 +41,18 @@
       - role_info is succeeded
       - role_info.iam_roles | length == 0
 
+- name: "Remove IAM Role (should be gone already) - check mode"
+  iam_role:
+    state: absent
+    name: "{{ test_role }}"
+    delete_instance_profile: yes
+  register: iam_role
+  check_mode: yes
+
+- assert:
+    that:
+      - iam_role is not changed
+
 - name: "Remove IAM Role (should be gone already)"
   iam_role:
     state: absent