ec2_instance don't change termination protection in check mode (ansib…

…le-collections#505) * ec2_instance don't change termination protection in check mode Fixes: ansible/ansible/issues/67716 Extend termination protection tests * Set the path for the aws CLI tool - setting ansible_python_interpreter updates the python search path but not the shell search path * changelog Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis · Apr 3, 2021 · b216f8d · b216f8d
1 parent 60d09d0
commit b216f8d
Show file tree

Hide file tree

Showing 5 changed files with 120 additions and 7 deletions.
diff --git a/changelogs/fragments/505-ec2_instance-terminate_protection.yml b/changelogs/fragments/505-ec2_instance-terminate_protection.yml
@@ -0,0 +1,2 @@
+bugfixes:
+- ec2_instance - ensure that termination protection isn't modified when using check_mode (https://github.com/ansible/ansible/issues/67716).
diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py
@@ -1512,6 +1512,7 @@ def change_instance_state(filters, desired_state, ec2=None):
     unchanged = set()
     failure_reason = ""
 
+    # TODO: better check_moding in here https://github.com/ansible-collections/community.aws/issues/16
     for inst in instances:
         try:
             if desired_state == 'TERMINATED':
@@ -1588,7 +1589,8 @@ def handle_existing(existing_matches, changed, ec2, state):
         )
     changes = diff_instance_and_params(existing_matches[0], module.params)
     for c in changes:
-        ec2.modify_instance_attribute(aws_retry=True, **c)
+        if not module.check_mode:
+            ec2.modify_instance_attribute(aws_retry=True, **c)
     changed |= bool(changes)
     changed |= add_or_update_instance_profile(existing_matches[0], module.params.get('instance_role'))
     changed |= change_network_attachments(existing_matches[0], module.params, ec2)

diff --git a/tests/integration/targets/ec2_instance/inventory b/tests/integration/targets/ec2_instance/inventory
@@ -8,7 +8,7 @@ default_vpc_tests
 external_resource_attach
 instance_no_wait
 iam_instance_role
-termination_protection
+termination_protection_wrapper
 tags_and_vpc_settings
 checkmode_tests
 security_group

diff --git a/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/termination_protection.yml b/tests/integration/targets/ec2_instance/roles/ec2_instance/tasks/termination_protection.yml
@@ -1,5 +1,4 @@
 - block:
-
     - name: Create instance with termination protection (check mode)
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
@@ -35,13 +34,34 @@
         wait: yes
       register: create_instance_results
 
+    - set_fact:
+        instance_id: '{{ create_instance_results.instances[0].instance_id }}'
+
     - name: Check return values of the create instance task
       assert:
         that:
           - "{{ create_instance_results.instances | length }} > 0"
           - "'{{ create_instance_results.instances.0.state.name }}' == 'running'"
           - "'{{ create_instance_results.spec.DisableApiTermination }}'"
 
+    - name: Get info on termination protection
+      command: '{{ aws_cli }} ec2 describe-instance-attribute --attribute disableApiTermination --instance-id {{ instance_id }}'
+      environment:
+        AWS_ACCESS_KEY_ID: "{{ aws_access_key }}"
+        AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}"
+        AWS_SESSION_TOKEN: "{{ security_token | default('') }}"
+        AWS_DEFAULT_REGION: "{{ aws_region }}"
+      register: instance_termination_check
+
+    - name: convert it to an object
+      set_fact:
+        instance_termination_status: "{{ instance_termination_check.stdout | from_json }}"
+
+    - name: Assert termination protection status did not change in check_mode
+      assert:
+        that:
+          - instance_termination_status.DisableApiTermination.Value == true
+
     - name: Create instance with termination protection (check mode) (idempotent)
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
@@ -90,13 +110,46 @@
       failed_when: "'Unable to terminate instances' not in terminate_instance_results.msg"
       register: terminate_instance_results
 
-    # https://github.com/ansible/ansible/issues/67716
-    # Updates to termination protection in check mode has a bug (listed above)
+    - name: Set termination protection to false (check_mode)
+      ec2_instance:
+        name: "{{ resource_prefix }}-termination-protection"
+        image_id: "{{ ec2_ami_image }}"
+        tags:
+          TestId: "{{ resource_prefix }}"
+        termination_protection: false
+        instance_type: "{{ ec2_instance_type }}"
+        vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
+      check_mode: True
+      register: set_termination_protectioncheck_mode_results
+
+    - name: Check return value
+      assert:
+        that:
+          - "{{ set_termination_protectioncheck_mode_results.changed }}"
+
+    - name: Get info on termination protection
+      command: '{{ aws_cli }} ec2 describe-instance-attribute --attribute disableApiTermination --instance-id {{ instance_id }}'
+      environment:
+        AWS_ACCESS_KEY_ID: "{{ aws_access_key }}"
+        AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}"
+        AWS_SESSION_TOKEN: "{{ security_token | default('') }}"
+        AWS_DEFAULT_REGION: "{{ aws_region }}"
+      register: instance_termination_check
+
+    - name: convert it to an object
+      set_fact:
+        instance_termination_status: "{{ instance_termination_check.stdout | from_json }}"
+
+    - assert:
+        that:
+          - instance_termination_status.DisableApiTermination.Value == true
 
     - name: Set termination protection to false
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
         image_id: "{{ ec2_ami_image }}"
+        tags:
+          TestId: "{{ resource_prefix }}"
         termination_protection: false
         instance_type: "{{ ec2_instance_type }}"
         vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
@@ -105,13 +158,31 @@
     - name: Check return value
       assert:
         that:
-          - "{{ set_termination_protection_results.changed }}"
-          - "{{ not set_termination_protection_results.changes[0].DisableApiTermination.Value }}"
+          - set_termination_protection_results.changed
+
+    - name: Get info on termination protection
+      command: '{{ aws_cli }} ec2 describe-instance-attribute --attribute disableApiTermination --instance-id {{ instance_id }}'
+      environment:
+        AWS_ACCESS_KEY_ID: "{{ aws_access_key }}"
+        AWS_SECRET_ACCESS_KEY: "{{ aws_secret_key }}"
+        AWS_SESSION_TOKEN: "{{ security_token | default('') }}"
+        AWS_DEFAULT_REGION: "{{ aws_region }}"
+      register: instance_termination_check
+
+    - name: convert it to an object
+      set_fact:
+        instance_termination_status: "{{ instance_termination_check.stdout | from_json }}"
+
+    - assert:
+        that:
+          - instance_termination_status.DisableApiTermination.Value == false
 
     - name: Set termination protection to false (idempotent)
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
         image_id: "{{ ec2_ami_image }}"
+        tags:
+          TestId: "{{ resource_prefix }}"
         termination_protection: false
         instance_type: "{{ ec2_instance_type }}"
         vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
@@ -126,6 +197,8 @@
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
         image_id: "{{ ec2_ami_image }}"
+        tags:
+          TestId: "{{ resource_prefix }}"
         termination_protection: true
         instance_type: "{{ ec2_instance_type }}"
         vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
@@ -141,6 +214,8 @@
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
         image_id: "{{ ec2_ami_image }}"
+        tags:
+          TestId: "{{ resource_prefix }}"
         termination_protection: true
         instance_type: "{{ ec2_instance_type }}"
         vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"
@@ -155,6 +230,8 @@
       ec2_instance:
         name: "{{ resource_prefix }}-termination-protection"
         image_id: "{{ ec2_ami_image }}"
+        tags:
+          TestId: "{{ resource_prefix }}"
         termination_protection: false
         instance_type: "{{ ec2_instance_type }}"
         vpc_subnet_id: "{{ testing_subnet_b.subnet.id }}"

diff --git a/...egration/targets/ec2_instance/roles/ec2_instance/tasks/termination_protection_wrapper.yml b/...egration/targets/ec2_instance/roles/ec2_instance/tasks/termination_protection_wrapper.yml
@@ -0,0 +1,32 @@
+---
+- include_role:
+    name: 'setup_remote_tmp_dir'
+
+- set_fact:
+    virtualenv: "{{ remote_tmp_dir }}/virtualenv"
+    virtualenv_command: "{{ ansible_python_interpreter }} -m virtualenv"
+
+- set_fact:
+    virtualenv_interpreter: "{{ virtualenv }}/bin/python"
+    aws_cli: "{{ virtualenv }}/bin/aws"
+
+- pip:
+    name: "virtualenv"
+
+- pip:
+    name:
+      - awscli<=1.18.159
+      - botocore<1.19.0,>=1.13.3
+      - boto3 >= 1.9.250, <= 1.15.18
+      - coverage<5
+    virtualenv: "{{ virtualenv }}"
+    virtualenv_command: "{{ virtualenv_command }}"
+    virtualenv_site_packages: no
+
+- include_tasks: termination_protection.yml
+  vars:
+    ansible_python_interpreter: "{{ virtualenv_interpreter }}"
+
+- file:
+    state: absent
+    path: "{{ virtualenv }}"
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		bugfixes:
		- ec2_instance - ensure that termination protection isn't modified when using check_mode (https://github.com/ansible/ansible/issues/67716).