Fix idempotency and logic to update existing aggregator (#645) (#1297)

[PR #645/77d5813a backport][stable-4] Fix idempotency and logic to update existing aggregator

This is a backport of PR #645 as merged into main (77d5813).
SUMMARY

describe_configuration_aggregators method returns output similar
to the following:
{
    "ConfigurationAggregators": [
        {
            "ConfigurationAggregatorName": "test-name",
            "ConfigurationAggregatorArn": "arn:aws:config:us-east-1:123412341234:config-aggregator/config-aggregator-xbxbvyq5",
            "OrganizationAggregationSource": {
                "RoleArn": "arn:aws:iam::123412341234:role/my-role",
                "AllAwsRegions": true
            },
            "CreationTime": 1619030767.047,
            "LastUpdatedTime": 1626463216.998
        }
    ]
}

As a result, lines 134-136 fail:
    del current_params['ConfigurationAggregatorArn']
    del current_params['CreationTime']
    del current_params['LastUpdatedTime']

as they try to delete attributes from the current_params as opposed
to current_params['ConfigurationAggregators'][0].
The error message is:
KeyError: 'ConfigurationAggregators'

Additionally, if no account_sources attribute is specified, the module
fails idempotency check, because in that case AccountAggregationSources
attribute is present in params, but not in current_params.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

aws_config_aggregator
ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell <None>

changelogs/fragments/645-aws_config_aggregator-fix-update-and-idempotency.yml

@@ -0,0 +1,3 @@
+bugfixes:
+  - aws_config_aggregator - Fix idempotency when ``account_sources`` parameter is not specified (https://github.com/ansible-collections/community.aws/pull/645).
+  - aws_config_aggregator - Fix `KeyError` when updating existing aggregator (https://github.com/ansible-collections/community.aws/pull/645).

plugins/modules/aws_config_aggregator.py

@@ -126,22 +126,25 @@ def create_resource(client, module, params, result):
 
 
 def update_resource(client, module, params, result):
+    result['changed'] = False
+
     current_params = client.describe_configuration_aggregators(
         ConfigurationAggregatorNames=[params['ConfigurationAggregatorName']]
-    )
+    )['ConfigurationAggregators'][0]
 
-    del current_params['ConfigurationAggregatorArn']
-    del current_params['CreationTime']
-    del current_params['LastUpdatedTime']
+    if params['AccountAggregationSources'] != current_params.get('AccountAggregationSources', []):
+        result['changed'] = True
+
+    if params['OrganizationAggregationSource'] != current_params.get('OrganizationAggregationSource', {}):
+        result['changed'] = True
 
-    if params != current_params['ConfigurationAggregators'][0]:
+    if result['changed']:
         try:
             client.put_configuration_aggregator(
                 ConfigurationAggregatorName=params['ConfigurationAggregatorName'],
                 AccountAggregationSources=params['AccountAggregationSources'],
                 OrganizationAggregationSource=params['OrganizationAggregationSource']
             )
-            result['changed'] = True
             result['aggregator'] = camel_dict_to_snake_dict(resource_exists(client, module, params))
             return result
         except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:

tests/integration/targets/aws_config/tasks/main.yaml

@@ -201,6 +201,36 @@
         that:
           - output.changed
 
+    - name: Create aws_config_aggregator
+      aws_config_aggregator:
+        name: random_name
+        state: present
+        account_sources: []
+        organization_source:
+          all_aws_regions: true
+          role_arn: "{{ config_iam_role.arn }}"
+      register: output
+
+    - name: assert success
+      assert:
+        that:
+           - output is changed
+
+    - name: Create aws_config_aggregator - idempotency
+      aws_config_aggregator:
+        name: random_name
+        state: present
+        account_sources: []
+        organization_source:
+          all_aws_regions: true
+          role_arn: "{{ config_iam_role.arn }}"
+      register: output
+
+    - name: assert not changed
+      assert:
+        that:
+           - output is not changed
+
     # ============================================================
     # Update testing
     # ============================================================
@@ -250,6 +280,40 @@
         that:
           - output.changed
 
+    - name: Update aws_config_aggregator
+      aws_config_aggregator:
+        name: random_name
+        state: present
+        account_sources: []
+        organization_source:
+          all_aws_regions: false
+          aws_regions:
+            - '{{ aws_region }}'
+          role_arn: "{{ config_iam_role.arn }}"
+      register: output
+
+    - name: assert success
+      assert:
+        that:
+           - output is changed
+
+    - name: Update aws_config_aggregator - idempotency
+      aws_config_aggregator:
+        name: random_name
+        state: present
+        account_sources: []
+        organization_source:
+          all_aws_regions: false
+          aws_regions:
+            - '{{ aws_region }}'
+          role_arn: "{{ config_iam_role.arn }}"
+      register: output
+
+    - name: assert success
+      assert:
+        that:
+           - output is not changed
+
     # ============================================================
     # Read testing
     # ============================================================
@@ -300,6 +364,14 @@
           - not output.changed
 
   always:
+
+    - name: delete aws_config_aggregator
+      aws_config_aggregator:
+        name: random_name
+        state: absent
+      register: output
+      ignore_errors: true
+
     # ============================================================
     # Destroy testing
     # ============================================================