diff --git a/changelogs/fragments/1093-iam_policy-update-docs-and-add-required_if.yml b/changelogs/fragments/1093-iam_policy-update-docs-and-add-required_if.yml new file mode 100644 index 00000000000..0bac9667140 --- /dev/null +++ b/changelogs/fragments/1093-iam_policy-update-docs-and-add-required_if.yml @@ -0,0 +1,4 @@ +minor_changes: + - iam_policy - update broken examples and add RETURN section to documentation; add extra integration tests for idempotency check mode runs (https://github.com/ansible-collections/community.aws/pull/1093). +bugfixes: + - iam_policy - require one of `policy_document` and `policy_json` when state is present to prevent MalformedPolicyDocumentException from being thrown (https://github.com/ansible-collections/community.aws/pull/1093). diff --git a/plugins/modules/iam_policy.py b/plugins/modules/iam_policy.py index 570c37efa1b..8989255d3c1 100644 --- a/plugins/modules/iam_policy.py +++ b/plugins/modules/iam_policy.py @@ -82,8 +82,7 @@ # Advanced example, create two new groups and add a READ-ONLY policy to both # groups. - name: Create Two Groups, Mario and Luigi - community.aws.iam: - iam_type: group + community.aws.iam_group: name: "{{ item }}" state: present loop: @@ -94,9 +93,9 @@ - name: Apply READ-ONLY policy to new groups that have been recently created community.aws.iam_policy: iam_type: group - iam_name: "{{ item.created_group.group_name }}" + iam_name: "{{ item.iam_group.group.group_name }}" policy_name: "READ-ONLY" - policy_document: readonlypolicy.json + policy_json: "{{ lookup('template', 'readonly.json.j2') }}" state: present loop: "{{ new_groups.results }}" @@ -107,12 +106,20 @@ iam_name: "{{ item.user }}" policy_name: "s3_limited_access_{{ item.prefix }}" state: present - policy_json: " {{ lookup( 'template', 's3_policy.json.j2') }} " + policy_json: "{{ lookup('template', 's3_policy.json.j2') }}" loop: - user: s3_user prefix: s3_user_prefix ''' +RETURN = ''' +policies: + description: A list of names of the inline policies embedded in the specified IAM resource (user, group, or role). + returned: always + type: list + elements: str +''' + import json try: @@ -120,9 +127,10 @@ except ImportError: pass -from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule -from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_policies, AWSRetry from ansible.module_utils.six import string_types +from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry +from ansible_collections.amazon.aws.plugins.module_utils.ec2 import compare_policies class PolicyError(Exception): @@ -174,9 +182,9 @@ def delete(self): self.changed = False return - self.changed = True if not self.check_mode: self._delete(self.name, self.policy_name) + self.changed = True def get_policy_text(self): try: @@ -298,8 +306,16 @@ def main(): skip_duplicates=dict(type='bool', default=None, required=False) ) mutually_exclusive = [['policy_document', 'policy_json']] - - module = AnsibleAWSModule(argument_spec=argument_spec, mutually_exclusive=mutually_exclusive, supports_check_mode=True) + required_if = [ + ('state', 'present', ('policy_document', 'policy_json'), True), + ] + + module = AnsibleAWSModule( + argument_spec=argument_spec, + mutually_exclusive=mutually_exclusive, + required_if=required_if, + supports_check_mode=True + ) skip_duplicates = module.params.get('skip_duplicates') diff --git a/tests/integration/targets/iam_policy/tasks/object.yml b/tests/integration/targets/iam_policy/tasks/object.yml index 2007da1164e..f13d5d5d425 100644 --- a/tests/integration/targets/iam_policy/tasks/object.yml +++ b/tests/integration/targets/iam_policy/tasks/object.yml @@ -30,6 +30,22 @@ - iam_policy_info is succeeded # ============================================================ + - name: 'Invalid creation of policy for {{ iam_type }} - missing required parameters' + iam_policy: + state: present + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_a }}' + skip_duplicates: yes + register: result + ignore_errors: yes + + - name: Assert task failed with correct error message + assert: + that: + - result.failed + - "'state is present but any of the following are missing: policy_document, policy_json' in result.msg" + - name: 'Create policy using document for {{ iam_type }} (check mode)' check_mode: yes iam_policy: @@ -75,6 +91,22 @@ - iam_policy_info.policies[0].policy_name == iam_policy_name_a - '"Id" not in iam_policy_info.policies[0].policy_document' + - name: 'Create policy using document for {{ iam_type }} (idempotency - check mode)' + iam_policy: + state: present + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_a }}' + policy_document: '{{ tmpdir.path }}/no_access.json' + skip_duplicates: yes + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Create policy using document for {{ iam_type }} (idempotency)' iam_policy: state: present @@ -208,6 +240,22 @@ - iam_policy_info.policies[0].policy_name == iam_policy_name_b - '"Id" not in iam_policy_info.policies[0].policy_document' + - name: 'Create policy using document for {{ iam_type }} (idempotency - check mode) (skip_duplicates = no)' + iam_policy: + state: present + skip_duplicates: no + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_b }}' + policy_document: '{{ tmpdir.path }}/no_access.json' + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Create policy using document for {{ iam_type }} (idempotency) (skip_duplicates = no)' iam_policy: state: present @@ -294,6 +342,22 @@ - iam_policy_info.policies[0].policy_name == iam_policy_name_c - iam_policy_info.policies[0].policy_document.Id == 'MyId' + - name: 'Create policy using json for {{ iam_type }} (idempotency - check mode)' + iam_policy: + state: present + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_c }}' + policy_json: '{{ lookup("file", "{{ tmpdir.path }}/no_access_with_id.json") }}' + skip_duplicates: yes + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Create policy using json for {{ iam_type }} (idempotency)' iam_policy: state: present @@ -340,7 +404,7 @@ policy_name: '{{ iam_policy_name_d }}' register: iam_policy_info - - name: 'Assert policy would be added for {{ iam_type }}' + - name: 'Assert policy would not be added for {{ iam_type }}' assert: that: - result is not changed @@ -436,6 +500,22 @@ - iam_policy_info.policies[0].policy_name == iam_policy_name_d - iam_policy_info.policies[0].policy_document.Id == 'MyId' + - name: 'Create policy using json for {{ iam_type }} (idempotency - check mode) (skip_duplicates = no)' + iam_policy: + state: present + skip_duplicates: no + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_d }}' + policy_json: '{{ lookup("file", "{{ tmpdir.path }}/no_access_with_id.json") }}' + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Create policy using json for {{ iam_type }} (idempotency) (skip_duplicates = no)' iam_policy: state: present @@ -506,7 +586,7 @@ policy_name: '{{ iam_policy_name_a }}' register: iam_policy_info - - name: 'Assert policy would be added for {{ iam_type }}' + - name: 'Assert policy would not be added for {{ iam_type }}' assert: that: - result is not changed @@ -587,6 +667,22 @@ - result[iam_object_key] == iam_name - iam_policy_info.policies[0].policy_document.Id == 'MyId' + - name: 'Update policy using document for {{ iam_type }} (idempotency - check mode) (skip_duplicates = no)' + iam_policy: + state: present + skip_duplicates: no + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_a }}' + policy_document: '{{ tmpdir.path }}/no_access_with_id.json' + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Update policy using document for {{ iam_type }} (idempotency) (skip_duplicates = no)' iam_policy: state: present @@ -655,7 +751,7 @@ policy_name: '{{ iam_policy_name_c }}' register: iam_policy_info - - name: 'Assert policy would be added for {{ iam_type }}' + - name: 'Assert policy would not be added for {{ iam_type }}' assert: that: - result is not changed @@ -731,6 +827,22 @@ - result[iam_object_key] == iam_name - '"Id" not in iam_policy_info.policies[0].policy_document' + - name: 'Update policy using json for {{ iam_type }} (idempotency - check mode) (skip_duplicates = no)' + iam_policy: + state: present + skip_duplicates: no + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_c }}' + policy_json: '{{ lookup("file", "{{ tmpdir.path }}/no_access.json") }}' + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Update policy using json for {{ iam_type }} (idempotency) (skip_duplicates = no)' iam_policy: state: present @@ -824,6 +936,21 @@ - result[iam_object_key] == iam_name - iam_policy_info.policies[0].policy_document.Id == 'MyOtherId' + - name: 'Update policy using document for {{ iam_type }} (idempotency - check mode)' + iam_policy: + state: present + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_b }}' + policy_document: '{{ tmpdir.path }}/no_access_with_second_id.json' + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Update policy using document for {{ iam_type }} (idempotency)' iam_policy: state: present @@ -916,6 +1043,22 @@ - result[iam_object_key] == iam_name - iam_policy_info.policies[0].policy_document.Id == 'MyOtherId' + - name: 'Update policy using json for {{ iam_type }} (idempotency - check mode)' + iam_policy: + state: present + skip_duplicates: no + iam_type: '{{ iam_type }}' + iam_name: '{{ iam_name }}' + policy_name: '{{ iam_policy_name_d }}' + policy_json: '{{ lookup("file", "{{ tmpdir.path }}/no_access_with_second_id.json") }}' + register: result + check_mode: yes + + - name: 'Assert no change would occur' + assert: + that: + - result is not changed + - name: 'Update policy using json for {{ iam_type }} (idempotency)' iam_policy: state: present