rds_instance: fix 'typeError'when tagging gp3 db (#1437)

rds_instance: fix 'typeError'when tagging gp3 db

SUMMARY

This PR fixes bug causing error when managing tags on a gp3 rds database instance, caused by a conditional statement comparing allocated_storage even when it is not provided.
currently code returns TypeError: '>=' not supported between instances of 'NoneType' and 'int'.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

rds_instance
ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell
(cherry picked from commit f63d191)

changelogs/fragments/1437-rds_instance-gp3-tagging-bugfix.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- rds_instance - Fixed ``TypeError`` when tagging RDS DB with storage type ``gp3`` (https://github.com/ansible-collections/amazon.aws/pull/1437).

plugins/modules/rds_instance.py

@@ -1031,20 +1031,25 @@ def get_options_with_changing_values(client, module, parameters):
         new_allocated_storage = module.params.get('allocated_storage')
         current_allocated_storage = instance.get('PendingModifiedValues', {}).get('AllocatedStorage', instance['AllocatedStorage'])
 
-        if current_allocated_storage != new_allocated_storage:
-            parameters['AllocatedStorage'] = new_allocated_storage
-
-        if new_allocated_storage >= 400:
-            if new_iops < 12000:
-                module.fail_json(msg='IOPS must be at least 12000 when the allocated storage is larger than or equal to 400 GB.')
+        if new_allocated_storage:
+            if current_allocated_storage != new_allocated_storage:
+                parameters["AllocatedStorage"] = new_allocated_storage
+
+            if new_allocated_storage >= 400:
+                if new_iops < 12000:
+                    module.fail_json(
+                        msg="IOPS must be at least 12000 when the allocated storage is larger than or equal to 400 GB."
+                    )
 
-            if new_storage_throughput < 500 and GP3_THROUGHPUT:
-                module.fail_json(msg='Storage Throughput must be at least 500 when the allocated storage is larger than or equal to 400 GB.')
+                if new_storage_throughput < 500 and GP3_THROUGHPUT:
+                    module.fail_json(
+                        msg="Storage Throughput must be at least 500 when the allocated storage is larger than or equal to 400 GB."
+                    )
 
-            if current_iops != new_iops:
-                parameters['Iops'] = new_iops
-                # must be always specified when changing iops
-                parameters['AllocatedStorage'] = new_allocated_storage
+                if current_iops != new_iops:
+                    parameters["Iops"] = new_iops
+                    # must be always specified when changing iops
+                    parameters["AllocatedStorage"] = new_allocated_storage
 
     if parameters.get('NewDBInstanceIdentifier') and instance.get('PendingModifiedValues', {}).get('DBInstanceIdentifier'):
         if parameters['NewDBInstanceIdentifier'] == instance['PendingModifiedValues']['DBInstanceIdentifier'] and not apply_immediately:

tests/integration/targets/rds_instance_tagging/defaults/main.yml

@@ -1,4 +1,5 @@
 instance_id: ansible-test-{{ tiny_prefix }}
+instance_id_gp3: ansible-test-{{ tiny_prefix }}-gp3
 modified_instance_id: '{{ instance_id }}-updated'
 username: test
 password: test12345678

tests/integration/targets/rds_instance_tagging/tasks/main.yml

@@ -8,6 +8,9 @@
       security_token: '{{ security_token | default(omit) }}'
       region: '{{ aws_region }}'
   block:
+  - name: Test tagging db with storage type gp3
+    import_tasks: test_tagging_gp3.yml
+
   - name: Ensure the resource doesn't exist
     rds_instance:
       id: '{{ instance_id }}'

tests/integration/targets/rds_instance_tagging/tasks/test_tagging_gp3.yml

@@ -0,0 +1,190 @@
+- block:
+  - name: Ensure the resource doesn't exist
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: absent
+      skip_final_snapshot: true
+    register: result
+
+  - assert:
+      that:
+      - not result.changed
+    ignore_errors: yes
+
+    # Test invalid bad options
+  - name: Create a DB instance with an invalid engine
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: present
+      engine: thisisnotavalidengine
+      username: '{{ username }}'
+      password: '{{ password }}'
+      db_instance_class: '{{ db_instance_class }}'
+      allocated_storage: '{{ allocated_storage }}'
+    register: result
+    ignore_errors: true
+
+  - assert:
+      that:
+      - result.failed
+      - '"value of engine must be one of" in result.msg'
+
+    # Test creation, adding tags and enabling encryption
+  - name: Create a mariadb instance
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: present
+      engine: mariadb
+      username: '{{ username }}'
+      password: '{{ password }}'
+      db_instance_class: '{{ db_instance_class }}'
+      allocated_storage: '{{ allocated_storage }}'
+      storage_encrypted: true
+      tags:
+        Name: '{{ instance_id_gp3 }}'
+        Created_by: Ansible rds_instance tests
+    register: result
+
+  - assert:
+      that:
+      - result.changed
+      - result.db_instance_identifier == '{{ instance_id_gp3 }}'
+      - result.tags | length == 2
+      - result.tags.Name == '{{ instance_id_gp3 }}'
+      - result.tags.Created_by == 'Ansible rds_instance tests'
+      - result.kms_key_id
+      - result.storage_encrypted == true
+
+  - name: Test idempotency omitting tags - check_mode
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: present
+      engine: mariadb
+      username: '{{ username }}'
+      password: '{{ password }}'
+      db_instance_class: '{{ db_instance_class }}'
+      allocated_storage: '{{ allocated_storage }}'
+    register: result
+    check_mode: yes
+
+  - assert:
+      that:
+      - not result.changed
+
+  - name: Test idempotency omitting tags
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: present
+      engine: mariadb
+      username: '{{ username }}'
+      password: '{{ password }}'
+      db_instance_class: '{{ db_instance_class }}'
+      allocated_storage: '{{ allocated_storage }}'
+    register: result
+
+  - assert:
+      that:
+      - not result.changed
+      - result.db_instance_identifier == '{{ instance_id_gp3 }}'
+      - result.tags | length == 2
+
+  - name: Idempotence with minimal options
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: present
+    register: result
+
+  - assert:
+      that:
+      - not result.changed
+      - result.db_instance_identifier == '{{ instance_id_gp3 }}'
+      - result.tags | length == 2
+
+  - name: Test tags are not purged if purge_tags is False
+    rds_instance:
+      db_instance_identifier: '{{ instance_id_gp3 }}'
+      state: present
+      engine: mariadb
+      username: '{{ username }}'
+      password: '{{ password }}'
+      db_instance_class: '{{ db_instance_class }}'
+      allocated_storage: '{{ allocated_storage }}'
+      tags: {}
+      purge_tags: false
+    register: result
+
+  - assert:
+      that:
+      - not result.changed
+      - result.tags | length == 2
+
+  - name: Add a tag and remove a tag - check_mode
+    rds_instance:
+      db_instance_identifier: '{{ instance_id_gp3 }}'
+      state: present
+      tags:
+        Name: '{{ instance_id_gp3 }}-new'
+        Created_by: Ansible rds_instance tests
+      purge_tags: true
+    register: result
+    check_mode: yes
+
+  - assert:
+      that:
+      - result.changed
+
+  - name: Add a tag and remove a tag
+    rds_instance:
+      db_instance_identifier: '{{ instance_id_gp3 }}'
+      state: present
+      tags:
+        Name: '{{ instance_id_gp3 }}-new'
+        Created_by: Ansible rds_instance tests
+      purge_tags: true
+    register: result
+
+  - assert:
+      that:
+      - result.changed
+      - result.tags | length == 2
+      - result.tags.Name == '{{ instance_id_gp3 }}-new'
+
+  - name: Add a tag and remove a tag (idempotence) - check_mode
+    rds_instance:
+      db_instance_identifier: '{{ instance_id_gp3 }}'
+      state: present
+      tags:
+        Name: '{{ instance_id_gp3 }}-new'
+        Created_by: Ansible rds_instance tests
+      purge_tags: true
+    register: result
+    check_mode: yes
+
+  - assert:
+      that:
+      - not result.changed
+
+  - name: Add a tag and remove a tag (idempotence)
+    rds_instance:
+      db_instance_identifier: '{{ instance_id_gp3 }}'
+      state: present
+      tags:
+        Name: '{{ instance_id_gp3 }}-new'
+        Created_by: Ansible rds_instance tests
+      purge_tags: true
+    register: result
+
+  - assert:
+      that:
+      - not result.changed
+      - result.tags | length == 2
+      - result.tags.Name == '{{ instance_id_gp3 }}-new'
+
+  always:
+  - name: Remove DB instance
+    rds_instance:
+      id: '{{ instance_id_gp3 }}'
+      state: absent
+      skip_final_snapshot: true
+      wait: false
+    ignore_errors: yes