sns_topic module always deletes and replaces SMS subscriptions #453

overhacked · 2021-03-03T16:32:10Z

SUMMARY

The sns_topic module always deletes and replaces SMS subscriptions, because the _canonicalize_endpoint() function strips all non-numeric characters. A leading + character is added by AWS when a new subscription is created, so the sns_topic module never sees its previously-created subscriptions.

ISSUE TYPE

Bug Report

COMPONENT NAME

sns_topic

ANSIBLE VERSION

ansible 2.9.15
  config file = None
  configured module search path = ['$HOME/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = $HOME/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible
  executable location = $HOME/Dev/ansible-site/.venv/bin/ansible
  python version = 3.7.9 (default, Feb  1 2021, 15:19:48) [Clang 10.0.1 ([email protected]:llvm/llvm-project.git llvmorg-10.0.1-0-gef32c611a

STEPS TO REPRODUCE

Run the following playbook
Confirm the SNS subscription
Run the playbook again

---
- hosts: localhost
  gather_facts: no
  vars:
    aws_access_key_id: KEYID
    aws_secret_access_key: SECRET
    aws_region: REGION
    # Must include leading plus and country code (+1 for North America)
    mobile_number: YOUR_VALID_MOBILE

  tasks:
    - sns_topic:
      aws_access_key: '{{ aws_access_key_id }}'
      aws_secret_key:  '{{ aws_secret_access_key }}'
      region: '{{ aws_region }}'
      name: Test_Topic
      state: present
      display_name: Test Topic
      subscriptions:
      - protocol: sms
        endpoint: '{{ mobile_number }}'

EXPECTED RESULTS

On the first run, the task result is changed. On the second run, the task result is ok.

ACTUAL RESULTS

The task result is changed on both runs.

Verbatim command output

$ ansible-playbook -vvvv sns_test.yml
ansible-playbook 2.9.15
  config file = HOMEDIR/Dev/ansible-site/ansible.cfg
  configured module search path = ['HOMEDIR/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = HOMEDIR/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible
  executable location = HOMEDIR/Dev/ansible-site/.venv/bin/ansible-playbook
  python version = 3.7.9 (default, Feb  1 2021, 15:19:48) [Clang 10.0.1 ([email protected]:llvm/llvm-project.git llvmorg-10.0.1-0-gef32c611a
Using HOMEDIR/Dev/ansible-site/ansible.cfg as config file
setting up inventory plugins
Parsed HOMEDIR/Dev/ansible-site/inventories/zee/hosts.yml inventory source with yaml plugin
Loading callback plugin default of type stdout, v2.0 from HOMEDIR/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible/plugins/callback/default.py

PLAYBOOK: sns_test.yml ********************************************************************************************************************************
Positional arguments: sns_test.yml
verbosity: 4
connection: smart
timeout: 10
become_method: sudo
tags: ('all',)
inventory: ('HOMEDIR/Dev/ansible-site/inventories/DIRNAME',)
forks: 5
1 plays in sns_test.yml

PLAY [aws_client] *************************************************************************************************************************************
META: ran handlers
Trying secret ScriptVaultSecret(filename='HOMEDIR/bin/get_ansible_vault_password.sh') for vault_id=default
Trying secret ScriptVaultSecret(filename='HOMEDIR/bin/get_ansible_vault_password.sh') for vault_id=default

TASK [sns_topic] **************************************************************************************************************************************
task path: HOMEDIR/Dev/ansible-site/sns_test.yml:6
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: USERNAME
<localhost> EXEC /bin/sh -c 'echo ~USERNAME && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo HOMEDIR/.ansible/tmp `"&& mkdir "` echo HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004 `" && echo ansible-tmp-1614793718.438763-70642-181606092157004="` echo HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004 `" ) && sleep 0'
Using module file HOMEDIR/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible/modules/cloud/amazon/sns_topic.py
<localhost> PUT HOMEDIR/.ansible/tmp/ansible-local-70581b86ao8uh/tmp1hmf83g4 TO HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004/AnsiballZ_sns_topic.py
<localhost> EXEC /bin/sh -c 'chmod u+x HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004/ HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004/AnsiballZ_sns_topic.py && sleep 0'
<localhost> EXEC /bin/sh -c 'HOMEDIR/Dev/ansible-site/.venv/bin/python3.7m HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004/AnsiballZ_sns_topic.py && sleep 0'
<localhost> EXEC /bin/sh -c 'rm -f -r HOMEDIR/.ansible/tmp/ansible-tmp-1614793718.438763-70642-181606092157004/ > /dev/null 2>&1 && sleep 0'
changed: [aws_client] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "aws_access_key": "AKIAJA6ZHTNW5FLJYE3Q",
            "aws_secret_key": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "debug_botocore_endpoint_logs": false,
            "delivery_policy": null,
            "display_name": "Test Topic",
            "ec2_url": null,
            "name": "Test_Topic",
            "policy": null,
            "profile": null,
            "purge_subscriptions": true,
            "region": "us-east-1",
            "security_token": null,
            "state": "present",
            "subscriptions": [
                {
                    "endpoint": "+1VALIDMOBILE",
                    "protocol": "sms"
                }
            ],
            "validate_certs": true
        }
    },
    "sns_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic",
    "sns_topic": {
        "attributes_set": [
            "display_name"
        ],
        "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": "Test Topic",
        "name": "Test_Topic",
        "owner": "AWS_ACCOUNT_ID",
        "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:us-east-1:AWS_ACCOUNT_ID:Test_Topic\",\"Condition\":{\"StringEquals\":{\"AWS:SourceOwner\":\"AWS_ACCOUNT_ID\"}}}]}",
        "state": "present",
        "subscriptions": [
            {
                "endpoint": "+1VALIDMOBILE",
                "owner": "AWS_ACCOUNT_ID",
                "protocol": "sms",
                "subscription_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic:19322f05-0265-479b-84c8-e5669a24ac0c",
                "topic_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic"
            }
        ],
        "subscriptions_added": [
            [
                "sms",
                "1VALIDMOBILE"
            ]
        ],
        "subscriptions_confirmed": "0",
        "subscriptions_deleted": "0",
        "subscriptions_existing": [],
        "subscriptions_new": [
            {
                "endpoint": "+1VALIDMOBILE",
                "protocol": "sms"
            }
        ],
        "subscriptions_pending": "0",
        "subscriptions_purge": true,
        "topic_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic",
        "topic_created": false,
        "topic_deleted": false
    }
}
META: ran handlers
META: ran handlers

PLAY RECAP ********************************************************************************************************************************************
aws_client                 : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0


$ # Confirmed subscription in AWS console

$ ansible-playbook -vvvv sns_test.yml
ansible-playbook 2.9.15
  config file = HOMEDIR/Dev/ansible-site/ansible.cfg
  configured module search path = ['HOMEDIR/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = HOMEDIR/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible
  executable location = HOMEDIR/Dev/ansible-site/.venv/bin/ansible-playbook
  python version = 3.7.9 (default, Feb  1 2021, 15:19:48) [Clang 10.0.1 ([email protected]:llvm/llvm-project.git llvmorg-10.0.1-0-gef32c611a
Using HOMEDIR/Dev/ansible-site/ansible.cfg as config file
setting up inventory plugins
Parsed HOMEDIR/Dev/ansible-site/inventories/zee/hosts.yml inventory source with yaml plugin
Loading callback plugin default of type stdout, v2.0 from HOMEDIR/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible/plugins/callback/default.py

PLAYBOOK: sns_test.yml ********************************************************************************************************************************
Positional arguments: sns_test.yml
verbosity: 4
connection: smart
timeout: 10
become_method: sudo
tags: ('all',)
inventory: ('HOMEDIR/Dev/ansible-site/inventories/zee',)
forks: 5
1 plays in sns_test.yml

PLAY [aws_client] *************************************************************************************************************************************
META: ran handlers
Trying secret ScriptVaultSecret(filename='HOMEDIR/.homesick/repos/dotfiles/home/bin/get_ansible_vault_password.sh') for vault_id=default
Trying secret ScriptVaultSecret(filename='HOMEDIR/.homesick/repos/dotfiles/home/bin/get_ansible_vault_password.sh') for vault_id=default

TASK [sns_topic] **************************************************************************************************************************************
task path: HOMEDIR/Dev/ansible-site/sns_test.yml:6
<localhost> ESTABLISH LOCAL CONNECTION FOR USER: USERNAME
<localhost> EXEC /bin/sh -c 'echo ~USERNAME && sleep 0'
<localhost> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo HOMEDIR/.ansible/tmp `"&& mkdir "` echo HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857 `" && echo ansible-tmp-1614793769.8479924-71437-25683009179857="` echo HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857 `" ) && sleep 0'
Using module file HOMEDIR/Dev/ansible-site/.venv/lib/python3.7/site-packages/ansible/modules/cloud/amazon/sns_topic.py
<localhost> PUT HOMEDIR/.ansible/tmp/ansible-local-71384z34ghj5q/tmp19on15xu TO HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857/AnsiballZ_sns_topic.py
<localhost> EXEC /bin/sh -c 'chmod u+x HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857/ HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857/AnsiballZ_sns_topic.py && sleep 0'
<localhost> EXEC /bin/sh -c 'HOMEDIR/Dev/ansible-site/.venv/bin/python3.7m HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857/AnsiballZ_sns_topic.py && sleep 0'
<localhost> EXEC /bin/sh -c 'rm -f -r HOMEDIR/.ansible/tmp/ansible-tmp-1614793769.8479924-71437-25683009179857/ > /dev/null 2>&1 && sleep 0'
changed: [aws_client] => {
    "changed": true,
    "invocation": {
        "module_args": {
            "aws_access_key": "ACCESS_KEY_ID",
            "aws_secret_key": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "debug_botocore_endpoint_logs": false,
            "delivery_policy": null,
            "display_name": "Test Topic",
            "ec2_url": null,
            "name": "Test_Topic",
            "policy": null,
            "profile": null,
            "purge_subscriptions": true,
            "region": "us-east-1",
            "security_token": null,
            "state": "present",
            "subscriptions": [
                {
                    "endpoint": "+1VALIDMOBILE",
                    "protocol": "sms"
                }
            ],
            "validate_certs": true
        }
    },
    "sns_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic",
    "sns_topic": {
        "attributes_set": [],
        "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": "Test Topic",
        "name": "Test_Topic",
        "owner": "AWS_ACCOUNT_ID",
        "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:us-east-1:AWS_ACCOUNT_ID:Test_Topic\",\"Condition\":{\"StringEquals\":{\"AWS:SourceOwner\":\"AWS_ACCOUNT_ID\"}}}]}",
        "state": "present",
        "subscriptions": [
            {
                "endpoint": "+1VALIDMOBILE",
                "owner": "AWS_ACCOUNT_ID",
                "protocol": "sms",
                "subscription_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic:8842de3c-25db-4f90-bc10-b7a51669ccc3",
                "topic_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic"
            }
        ],
        "subscriptions_added": [
            [
                "sms",
                "1VALIDMOBILE"
            ]
        ],
        "subscriptions_confirmed": "0",
        "subscriptions_deleted": "0",
        "subscriptions_existing": [],
        "subscriptions_new": [
            {
                "endpoint": "+1VALIDMOBILE",
                "protocol": "sms"
            }
        ],
        "subscriptions_pending": "0",
        "subscriptions_purge": true,
        "topic_arn": "arn:aws:sns:us-east-1:AWS_ACCOUNT_ID:Test_Topic",
        "topic_created": false,
        "topic_deleted": false
    }
}
META: ran handlers
META: ran handlers

PLAY RECAP ********************************************************************************************************************************************
aws_client                 : ok=1    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

The text was updated successfully, but these errors were encountered:

ansibullbot · 2021-03-03T16:37:20Z

Files identified in the description:
None

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot · 2021-03-03T16:37:20Z

@overhacked: Greetings! Thanks for taking the time to open this issue. In order for the community to handle your issue effectively, we need a bit more information.

Here are the items we could not find in your description:

component name

Please set the description of this issue with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/ISSUE_TEMPLATE/bug_report.md

click here for bot help

ansibullbot · 2021-03-03T18:14:38Z

Files identified in the description:

[plugins/modules/sns_topic.py](https://github.com/['ansible-collections/amazon.aws', 'ansible-collections/community.aws', 'ansible-collections/community.vmware']/blob/main/plugins/modules/sns_topic.py)

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot · 2021-03-03T18:14:38Z

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

Adds `+` to the list of acceptable characters in an SMS endpoint. Closes ansible-collections#453.

…subscriptions (#454) * Update sns_topic.py Adds `+` to the list of acceptable characters in an SMS endpoint. Closes #453. * Add changelog fragment for #454 * sns_topic: comment explaining SMS canonicalization Add comment documenting to what standard SMS endpoint addresses (phone numbers) are canonicalized * sns_topic: fix changelog * Get quoting correct * Simplify message to leave details in PR description Co-authored-by: Mark Chappell <[email protected]> Co-authored-by: Mark Chappell <[email protected]>

…subscriptions (ansible-collections#454) * Update sns_topic.py Adds `+` to the list of acceptable characters in an SMS endpoint. Closes ansible-collections#453. * Add changelog fragment for ansible-collections#454 * sns_topic: comment explaining SMS canonicalization Add comment documenting to what standard SMS endpoint addresses (phone numbers) are canonicalized * sns_topic: fix changelog * Get quoting correct * Simplify message to leave details in PR description Co-authored-by: Mark Chappell <[email protected]> Co-authored-by: Mark Chappell <[email protected]>

…inimum we claim) (ansible-collections#453) ec2_vol tests set botocore requires to == rather than >= SUMMARY We currently use >= which generally means that the latest version of botocore will be pulled in. Given that we specify it's supposed to work with >= we should test the oldest possible version to ensure that we support at least that version. ISSUE TYPE Tests Pull Request COMPONENT NAME ec2_vol ADDITIONAL INFORMATION Depends-on: ansible-collections#460 Reviewed-by: None <None>

…nsible-collections#404) Add constraints.txt and requirements.txt for unit/integration tests SUMMARY Now that we state that we support specific minimum versions of the AWS SDKs, make sure we base our unit/integration tests against them such that modules need to explicitly test/request newer versions of the SDKs. ISSUE TYPE Feature Pull Request COMPONENT NAME tests/integration tests/unit ADDITIONAL INFORMATION Once merged into amazon.aws we should merge this into community.aws Depends-On: ansible-collections#453 Depends-On: ansible-collections#454 Depends-On: ansible-collections#450 Depends-On: ansible-collections#496 See also: ansible/ansible-zuul-jobs#991 Reviewed-by: Jill R <None> Reviewed-by: None <None>

ansibullbot added feature This issue/PR relates to a feature request needs_info This issue requires further information. Please answer any outstanding questions needs_template This issue/PR has an incomplete description. Please fill in the proposed template correctly needs_triage labels Mar 3, 2021

overhacked added a commit to overhacked/community.aws that referenced this issue Mar 3, 2021

Update sns_topic.py

519603c

Adds `+` to the list of acceptable characters in an SMS endpoint. Closes ansible-collections#453.

overhacked mentioned this issue Mar 3, 2021

sns_topic: Allow canonical international-format phone numbers in SMS subscriptions #454

Merged

ansibullbot added the has_pr label Mar 3, 2021

tremble closed this as completed in #454 Mar 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sns_topic module always deletes and replaces SMS subscriptions #453

sns_topic module always deletes and replaces SMS subscriptions #453

overhacked commented Mar 3, 2021 •

edited

Loading

ansibullbot commented Mar 3, 2021

ansibullbot commented Mar 3, 2021

ansibullbot commented Mar 3, 2021

ansibullbot commented Mar 3, 2021

sns_topic module always deletes and replaces SMS subscriptions #453

sns_topic module always deletes and replaces SMS subscriptions #453

Comments

overhacked commented Mar 3, 2021 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ANSIBLE VERSION

STEPS TO REPRODUCE

EXPECTED RESULTS

ACTUAL RESULTS

ansibullbot commented Mar 3, 2021

ansibullbot commented Mar 3, 2021

ansibullbot commented Mar 3, 2021

ansibullbot commented Mar 3, 2021

overhacked commented Mar 3, 2021 •

edited

Loading