sns_topic: Loop through defaultHealthyRetryPolicy attributes and cast them as integers

[gregharvey]

SUMMARY

This is a simple patch that intercepts the defaultHealthyRetryPolicy attributes just before they are sent to the AWS API and ensures they are cast as integers, otherwise you get type errors if you try and set those attributes via a role in a dict (due to known limitations of jinja they get cast as strings by Ansible).

Fixes #713

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

sns_topic

ADDITIONAL INFORMATION

Because the AWS documentation for the create-topic method is virtually non-existent we've had to guess the attributes based on the UI, but as far as we can tell every attribute should be an integer within the defaultHealthyRetryPolicy dict except for backoffFunction, so we simply loop through the provided attributes casting as integer if it is not that attribute. Similarly, we have hard-coded the http key in the dict because, again as far as we can tell and without any real docs, there simply is no other option, so this is safe to do.

BEFORE
======

TASK [aws/aws_sns : Create an SNS topic and subscription.] *********************
task path: /home/controller/ce-provision/roles/aws/aws_sns/tasks/main.yml:2
The full traceback is:
Traceback (most recent call last):
  File "/tmp/ansible_community.aws.sns_topic_payload_f1uqbdm9/ansible_community.aws.sns_topic_payload.zip/ansible_collections/community/aws/plugins/modules/sns_topic.py", line 346, in _set_topic_attrs
  File "/usr/local/lib/python3.7/dist-packages/botocore/client.py", line 386, in _api_call
    return self._make_api_call(operation_name, kwargs)
  File "/usr/local/lib/python3.7/dist-packages/botocore/client.py", line 705, in _make_api_call
    raise error_class(parsed_response, operation_name)
botocore.errorfactory.InvalidParameterException: An error occurred (InvalidParameter) when calling the SetTopicAttributes operation: Invalid parameter: DeliveryPolicy: http.defaultHealthyRetryPolicy.minDelayTarget type mismatch, expecting integer, received a string
fatal: [localhost]: FAILED! => {
    "boto3_version": "1.17.98",
    "botocore_version": "1.20.98",
    "changed": false,
    "error": {
        "code": "InvalidParameter",
        "message": "Invalid parameter: DeliveryPolicy: http.defaultHealthyRetryPolicy.minDelayTarget type mismatch, expecting integer, received a string",
        "type": "Sender"
    },
    "invocation": {
        "module_args": {
            "aws_access_key": null,
            "aws_ca_bundle": null,
            "aws_config": null,
            "aws_secret_key": null,
            "debug_botocore_endpoint_logs": false,
            "delivery_policy": {
                "http": {
                    "defaultHealthyRetryPolicy": {
                        "backoffFunction": "linear",
                        "maxDelayTarget": "20",
                        "minDelayTarget": "20",
                        "numMaxDelayRetries": "0",
                        "numMinDelayRetries": "0",
                        "numNoDelayRetries": "0",
                        "numRetries": "3"
                    },
                    "disableSubscriptionOverrides": false
                }
            },
            "display_name": "",
            "ec2_url": null,
            "name": "Notify-CodeEnigma-Pushover",
            "policy": null,
            "profile": null,
            "purge_subscriptions": true,
            "region": null,
            "security_token": null,
            "state": "present",
            "subscriptions": [
                {
                    "endpoint": "alert@example.com",
                    "protocol": "email"
                }
            ],
            "validate_certs": true
        }
    },
    "msg": "Couldn't set topic delivery policy: An error occurred (InvalidParameter) when calling the SetTopicAttributes operation: Invalid parameter: DeliveryPolicy: http.defaultHealthyRetryPolicy.minDelayTarget type mismatch, expecting integer, received a string",
    "response_metadata": {
        "http_headers": {
            "content-length": "379",
            "content-type": "text/xml",
            "date": "Mon, 13 Sep 2021 12:54:00 GMT",
            "x-amzn-requestid": "264bf1a5-a080-5d81-84ae-a81ceca899d3"
        },
        "http_status_code": 400,
        "request_id": "264bf1a5-a080-5d81-84ae-a81ceca899d3",
        "retry_attempts": 0
    }
}


AFTER
=====

TASK [aws/aws_sns : Create an SNS topic and subscription.] *********************
task path: /home/controller/ce-provision/roles/aws/aws_sns/tasks/main.yml:2
changed: [localhost] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "aws_access_key": null,
            "aws_ca_bundle": null,
            "aws_config": null,
            "aws_secret_key": null,
            "debug_botocore_endpoint_logs": false,
            "delivery_policy": {
                "http": {
                    "defaultHealthyRetryPolicy": {
                        "backoffFunction": "linear",
                        "maxDelayTarget": 20,
                        "minDelayTarget": 20,
                        "numMaxDelayRetries": 0,
                        "numMinDelayRetries": 0,
                        "numNoDelayRetries": 0,
                        "numRetries": 3
                    },
                    "disableSubscriptionOverrides": false
                }
            },
            "display_name": "",
            "ec2_url": null,
            "name": "Notify-CodeEnigma-Pushover",
            "policy": null,
            "profile": null,
            "purge_subscriptions": true,
            "region": null,
            "security_token": null,
            "state": "present",
            "subscriptions": [
                {
                    "endpoint": "alert@example.com",
                    "protocol": "email"
                }
            ],
            "topic_type": "standard",
            "validate_certs": true
        }
    },
    "sns_arn": "arn:aws:sns:eu-west-1:XXXXXXXXXXXXXXXXX:Notify-CodeEnigma-Pushover",
    "sns_topic": {
        "attributes_set": [
            "delivery_policy"
        ],
        "check_mode": false,
        "delivery_policy": "{\"http\":{\"defaultHealthyRetryPolicy\":{\"minDelayTarget\":20,\"maxDelayTarget\":20,\"numRetries\":3,\"numMaxDelayRetries\":0,\"numNoDelayRetries\":0,\"numMinDelayRetries\":0,\"backoffFunction\":\"linear\"},\"disableSubscriptionOverrides\":false}}",
        "display_name": "",
        "name": "Notify-CodeEnigma-Pushover",
        "owner": "XXXXXXXXXXXXXXXXX",
        "policy": "{\"Version\":\"2008-10-17\",\"Id\":\"__default_policy_ID\",\"Statement\":[{\"Sid\":\"__default_statement_ID\",\"Effect\":\"Allow\",\"Principal\":{\"AWS\":\"*\"},\"Action\":[\"SNS:GetTopicAttributes\",\"SNS:SetTopicAttributes\",\"SNS:AddPermission\",\"SNS:RemovePermission\",\"SNS:DeleteTopic\",\"SNS:Subscribe\",\"SNS:ListSubscriptionsByTopic\",\"SNS:Publish\",\"SNS:Receive\"],\"Resource\":\"arn:aws:sns:eu-west-1:XXXXXXXXXXXXXXXXX:Notify-CodeEnigma-Pushover\",\"Condition\":{\"StringEquals\":{\"AWS:SourceOwner\":\"XXXXXXXXXXXXXXXXX\"}}}]}",
        "state": "present",
        "subscriptions": [
            {
                "endpoint": "alert@example.com",
                "owner": "XXXXXXXXXXXXXXXXX",
                "protocol": "email",
                "subscription_arn": "arn:aws:sns:eu-west-1:XXXXXXXXXXXXXXXXX:Notify-CodeEnigma-Pushover:a00d4a62-48e5-4a46-a3bd-913e64c79a9a",
                "topic_arn": "arn:aws:sns:eu-west-1:XXXXXXXXXXXXXXXXX:Notify-CodeEnigma-Pushover"
            }
        ],
        "subscriptions_added": [],
        "subscriptions_confirmed": "1",
        "subscriptions_deleted": "0",
        "subscriptions_existing": [],
        "subscriptions_new": [
            {
                "endpoint": "alert@example.com",
                "protocol": "email"
            }
        ],
        "subscriptions_pending": "0",
        "subscriptions_purge": true,
        "topic_arn": "arn:aws:sns:eu-west-1:XXXXXXXXXXXXXXXXX:Notify-CodeEnigma-Pushover",
        "topic_created": false,
        "topic_deleted": false,
        "topic_type": "standard"
    }
}

[ansibullbot]

cc @jillr @joelthompson @markuman @nand0p @s-hertel @tremble @wimnat
click here for bot help

[gregharvey]

Oops, indentations. I'm from a PHP background, I forgot to check if Python has different standards. 😃
Hopefully tests will be happier this time.

[alinabuzachis]

@gregharvey Thank you for taking time to work on this. Could you please add a changelog fragment https://cn-ansibledoc.readthedocs.io/zh_CN/latest/community/development_process.html#creating-a-changelog-fragment?

[gregharvey]

@alinabuzachis You're welcome, done - hope I did it right?

[tremble]

Hi @gregharvey,

Thanks for taking the time to contribute this fix. I tried updating our integration tests to catch this bug in future. Unfortunately, it looks like your approach results in problems with idempotency: because during the comparison the value is still a string, it sees a change from an integer (in the existing policy) to a string (in the new policy) and always returns "changed" if someone uses strings instead of integers in the policy. It would also throw exceptions if someone did something silly like passing "notanumber" instead of an integer, or simply didn't pass defaultHealthyRetryPolicy.

Rather than updating the value and throwing an ugly exception if someone tried to pass something other than a number it's better to properly define the delivery_policy option so that rather than it just being a free-form dictionary (associative array) we define what we shape we expect the dictionary to have.

Since this then involves updating the docs (which can be downright tedious), I've opened #739 which hopefully fixes your issue, as well as significantly improves the documentation.

[tremble]

Hi @gregharvey,

I very much appreciate that you took the time to contribute a fix here.

#739 has now been merged and should fix your issue. It should be available in the collection whenever the next release occurs.

Mark

[gregharvey]

That looks like it'll work well, yes, thank you! :-)