sns_topic ensure "changed" works as expected when managing delivery policies (#60944)
* sns_topic: (integration tests) Move the tests over to using module defaults * sns_topic: (integration tests) Add test for behaviour of changed when using delivery_policy * sns_topic: ensure "changed" behaves properly when managing delivery policies - a delivery_policy isn't an IAM policy, so compare_policies didn't cope with it - AWS automatically adds an additional option when you set an HTTP delivery policy * Parse the delivery policies so we can test the changes properly
This commit is contained in:
parent
9c6495d4d4
commit
021ba057ab
3 changed files with 104 additions and 34 deletions
|
@ -0,0 +1,3 @@
|
||||||
|
bugfixes:
|
||||||
|
- fixed issue with sns_topic's delivery_policy option resulting in changed
|
||||||
|
always being true
|
|
@ -217,6 +217,7 @@ sns_topic:
|
||||||
|
|
||||||
import json
|
import json
|
||||||
import re
|
import re
|
||||||
|
import copy
|
||||||
|
|
||||||
try:
|
try:
|
||||||
import botocore
|
import botocore
|
||||||
|
@ -298,6 +299,20 @@ class SnsTopicManager(object):
|
||||||
self.topic_arn = response['TopicArn']
|
self.topic_arn = response['TopicArn']
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
def _compare_delivery_policies(self, policy_a, policy_b):
|
||||||
|
_policy_a = copy.deepcopy(policy_a)
|
||||||
|
_policy_b = copy.deepcopy(policy_b)
|
||||||
|
# AWS automatically injects disableSubscriptionOverrides if you set an
|
||||||
|
# http policy
|
||||||
|
if 'http' in policy_a:
|
||||||
|
if 'disableSubscriptionOverrides' not in policy_a['http']:
|
||||||
|
_policy_a['http']['disableSubscriptionOverrides'] = False
|
||||||
|
if 'http' in policy_b:
|
||||||
|
if 'disableSubscriptionOverrides' not in policy_b['http']:
|
||||||
|
_policy_b['http']['disableSubscriptionOverrides'] = False
|
||||||
|
comparison = (_policy_a != _policy_b)
|
||||||
|
return comparison
|
||||||
|
|
||||||
def _set_topic_attrs(self):
|
def _set_topic_attrs(self):
|
||||||
changed = False
|
changed = False
|
||||||
try:
|
try:
|
||||||
|
@ -326,7 +341,7 @@ class SnsTopicManager(object):
|
||||||
self.module.fail_json_aws(e, msg="Couldn't set topic policy")
|
self.module.fail_json_aws(e, msg="Couldn't set topic policy")
|
||||||
|
|
||||||
if self.delivery_policy and ('DeliveryPolicy' not in topic_attributes or
|
if self.delivery_policy and ('DeliveryPolicy' not in topic_attributes or
|
||||||
compare_policies(self.delivery_policy, json.loads(topic_attributes['DeliveryPolicy']))):
|
self._compare_delivery_policies(self.delivery_policy, json.loads(topic_attributes['DeliveryPolicy']))):
|
||||||
changed = True
|
changed = True
|
||||||
self.attributes_set.append('delivery_policy')
|
self.attributes_set.append('delivery_policy')
|
||||||
if not self.check_mode:
|
if not self.check_mode:
|
||||||
|
|
|
@ -1,14 +1,10 @@
|
||||||
- block:
|
- module_defaults:
|
||||||
|
group/aws:
|
||||||
- name: set up AWS connection info
|
aws_secret_key: "{{ aws_secret_key }}"
|
||||||
set_fact:
|
aws_access_key: "{{ aws_access_key }}"
|
||||||
aws_connection_info: &aws_connection_info
|
security_token: "{{ security_token|default(omit) }}"
|
||||||
aws_secret_key: "{{ aws_secret_key|default() }}"
|
region: "{{ aws_region }}"
|
||||||
aws_access_key: "{{ aws_access_key|default() }}"
|
block:
|
||||||
security_token: "{{ security_token|default() }}"
|
|
||||||
region: "{{ aws_region|default() }}"
|
|
||||||
no_log: yes
|
|
||||||
|
|
||||||
# This should exist, but there's no expectation that the test user should be able to
|
# This should exist, but there's no expectation that the test user should be able to
|
||||||
# create/update this role, merely validate that it's there.
|
# create/update this role, merely validate that it's there.
|
||||||
# Use ansible -m iam_role -a 'name=ansible_lambda_role
|
# Use ansible -m iam_role -a 'name=ansible_lambda_role
|
||||||
|
@ -20,7 +16,6 @@
|
||||||
name: ansible_lambda_role
|
name: ansible_lambda_role
|
||||||
assume_role_policy_document: "{{ lookup('file', 'lambda-trust-policy.json', convert_data=False) }}"
|
assume_role_policy_document: "{{ lookup('file', 'lambda-trust-policy.json', convert_data=False) }}"
|
||||||
create_instance_profile: no
|
create_instance_profile: no
|
||||||
<<: *aws_connection_info
|
|
||||||
register: iam_role
|
register: iam_role
|
||||||
|
|
||||||
- name: pause if role was created
|
- name: pause if role was created
|
||||||
|
@ -35,7 +30,6 @@
|
||||||
iam_type: role
|
iam_type: role
|
||||||
policy_json: "{{ lookup('file', 'lambda-policy.json') }}"
|
policy_json: "{{ lookup('file', 'lambda-policy.json') }}"
|
||||||
state: present
|
state: present
|
||||||
<<: *aws_connection_info
|
|
||||||
register: iam_policy
|
register: iam_policy
|
||||||
|
|
||||||
- name: pause if policy was created
|
- name: pause if policy was created
|
||||||
|
@ -47,7 +41,6 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My topic name"
|
display_name: "My topic name"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_create
|
register: sns_topic_create
|
||||||
|
|
||||||
- name: assert that creation worked
|
- name: assert that creation worked
|
||||||
|
@ -63,7 +56,6 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My topic name"
|
display_name: "My topic name"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_no_change
|
register: sns_topic_no_change
|
||||||
|
|
||||||
- name: assert that recreation had no effect
|
- name: assert that recreation had no effect
|
||||||
|
@ -76,7 +68,6 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_update_name
|
register: sns_topic_update_name
|
||||||
|
|
||||||
- name: assert that updating name worked
|
- name: assert that updating name worked
|
||||||
|
@ -90,7 +81,6 @@
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
policy: "{{ lookup('template', 'initial-policy.json') }}"
|
policy: "{{ lookup('template', 'initial-policy.json') }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_add_policy
|
register: sns_topic_add_policy
|
||||||
|
|
||||||
- name: assert that adding policy worked
|
- name: assert that adding policy worked
|
||||||
|
@ -103,7 +93,6 @@
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
policy: "{{ lookup('template', 'initial-policy.json') }}"
|
policy: "{{ lookup('template', 'initial-policy.json') }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_rerun_policy
|
register: sns_topic_rerun_policy
|
||||||
|
|
||||||
- name: assert that rerunning policy had no effect
|
- name: assert that rerunning policy had no effect
|
||||||
|
@ -116,7 +105,6 @@
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
policy: "{{ lookup('template', 'updated-policy.json') }}"
|
policy: "{{ lookup('template', 'updated-policy.json') }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_update_policy
|
register: sns_topic_update_policy
|
||||||
|
|
||||||
- name: assert that updating policy worked
|
- name: assert that updating policy worked
|
||||||
|
@ -124,6 +112,84 @@
|
||||||
that:
|
that:
|
||||||
- sns_topic_update_policy.changed
|
- sns_topic_update_policy.changed
|
||||||
|
|
||||||
|
- name: add delivery policy
|
||||||
|
sns_topic:
|
||||||
|
name: "{{ sns_topic_topic_name }}"
|
||||||
|
display_name: "My new topic name"
|
||||||
|
delivery_policy:
|
||||||
|
http:
|
||||||
|
defaultHealthyRetryPolicy:
|
||||||
|
minDelayTarget: 20
|
||||||
|
maxDelayTarget: 20
|
||||||
|
numRetries: 3
|
||||||
|
numMaxDelayRetries: 0
|
||||||
|
numNoDelayRetries: 0
|
||||||
|
numMinDelayRetries: 0
|
||||||
|
backoffFunction: 'linear'
|
||||||
|
register: sns_topic_add_delivery_policy
|
||||||
|
|
||||||
|
- name: assert that adding delivery policy worked
|
||||||
|
vars:
|
||||||
|
delivery_policy: '{{ sns_topic_add_delivery_policy.sns_topic.delivery_policy | from_json }}'
|
||||||
|
assert:
|
||||||
|
that:
|
||||||
|
- sns_topic_add_delivery_policy.changed
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.minDelayTarget == 20
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.maxDelayTarget == 20
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.numRetries == 3
|
||||||
|
|
||||||
|
- name: rerun same delivery policy
|
||||||
|
sns_topic:
|
||||||
|
name: "{{ sns_topic_topic_name }}"
|
||||||
|
display_name: "My new topic name"
|
||||||
|
delivery_policy:
|
||||||
|
http:
|
||||||
|
defaultHealthyRetryPolicy:
|
||||||
|
minDelayTarget: 20
|
||||||
|
maxDelayTarget: 20
|
||||||
|
numRetries: 3
|
||||||
|
numMaxDelayRetries: 0
|
||||||
|
numNoDelayRetries: 0
|
||||||
|
numMinDelayRetries: 0
|
||||||
|
backoffFunction: 'linear'
|
||||||
|
register: sns_topic_rerun_delivery_policy
|
||||||
|
|
||||||
|
- name: assert that rerunning delivery_policy had no effect
|
||||||
|
vars:
|
||||||
|
delivery_policy: '{{ sns_topic_rerun_delivery_policy.sns_topic.delivery_policy | from_json }}'
|
||||||
|
assert:
|
||||||
|
that:
|
||||||
|
- not sns_topic_rerun_delivery_policy.changed
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.minDelayTarget == 20
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.maxDelayTarget == 20
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.numRetries == 3
|
||||||
|
|
||||||
|
- name: rerun a slightly different delivery policy
|
||||||
|
sns_topic:
|
||||||
|
name: "{{ sns_topic_topic_name }}"
|
||||||
|
display_name: "My new topic name"
|
||||||
|
delivery_policy:
|
||||||
|
http:
|
||||||
|
defaultHealthyRetryPolicy:
|
||||||
|
minDelayTarget: 40
|
||||||
|
maxDelayTarget: 40
|
||||||
|
numRetries: 6
|
||||||
|
numMaxDelayRetries: 0
|
||||||
|
numNoDelayRetries: 0
|
||||||
|
numMinDelayRetries: 0
|
||||||
|
backoffFunction: 'linear'
|
||||||
|
register: sns_topic_rerun_delivery_policy
|
||||||
|
|
||||||
|
- name: assert that rerunning delivery_policy worked
|
||||||
|
vars:
|
||||||
|
delivery_policy: '{{ sns_topic_rerun_delivery_policy.sns_topic.delivery_policy | from_json }}'
|
||||||
|
assert:
|
||||||
|
that:
|
||||||
|
- sns_topic_rerun_delivery_policy.changed
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.minDelayTarget == 40
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.maxDelayTarget == 40
|
||||||
|
- delivery_policy.http.defaultHealthyRetryPolicy.numRetries == 6
|
||||||
|
|
||||||
- name: create temp dir
|
- name: create temp dir
|
||||||
tempfile:
|
tempfile:
|
||||||
state: directory
|
state: directory
|
||||||
|
@ -143,7 +209,6 @@
|
||||||
runtime: 'python2.7'
|
runtime: 'python2.7'
|
||||||
role: ansible_lambda_role
|
role: ansible_lambda_role
|
||||||
handler: '{{ sns_topic_lambda_function }}.handler'
|
handler: '{{ sns_topic_lambda_function }}.handler'
|
||||||
<<: *aws_connection_info
|
|
||||||
register: lambda_result
|
register: lambda_result
|
||||||
|
|
||||||
- set_fact:
|
- set_fact:
|
||||||
|
@ -155,7 +220,6 @@
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
purge_subscriptions: no
|
purge_subscriptions: no
|
||||||
subscriptions: "{{ sns_topic_subscriptions }}"
|
subscriptions: "{{ sns_topic_subscriptions }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_subscribe
|
register: sns_topic_subscribe
|
||||||
|
|
||||||
- name: assert that subscribing worked
|
- name: assert that subscribing worked
|
||||||
|
@ -169,7 +233,6 @@
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
purge_subscriptions: no
|
purge_subscriptions: no
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_no_purge
|
register: sns_topic_no_purge
|
||||||
|
|
||||||
- name: assert that not purging subscriptions had no effect
|
- name: assert that not purging subscriptions had no effect
|
||||||
|
@ -183,7 +246,6 @@
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
display_name: "My new topic name"
|
display_name: "My new topic name"
|
||||||
purge_subscriptions: yes
|
purge_subscriptions: yes
|
||||||
<<: *aws_connection_info
|
|
||||||
register: sns_topic_purge
|
register: sns_topic_purge
|
||||||
|
|
||||||
- name: assert that purging subscriptions worked
|
- name: assert that purging subscriptions worked
|
||||||
|
@ -196,12 +258,10 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
state: absent
|
state: absent
|
||||||
<<: *aws_connection_info
|
|
||||||
|
|
||||||
- name: no-op with third party topic (effectively get existing subscriptions)
|
- name: no-op with third party topic (effectively get existing subscriptions)
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
register: third_party_topic
|
register: third_party_topic
|
||||||
|
|
||||||
|
@ -209,7 +269,6 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
subscriptions: "{{ sns_topic_subscriptions }}"
|
subscriptions: "{{ sns_topic_subscriptions }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
register: third_party_topic_subscribe
|
register: third_party_topic_subscribe
|
||||||
|
|
||||||
|
@ -224,7 +283,6 @@
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
display_name: "This should not work"
|
display_name: "This should not work"
|
||||||
subscriptions: "{{ sns_topic_subscriptions }}"
|
subscriptions: "{{ sns_topic_subscriptions }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
ignore_errors: yes
|
ignore_errors: yes
|
||||||
register: third_party_name_change
|
register: third_party_name_change
|
||||||
|
@ -238,7 +296,6 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
subscriptions: "{{ third_party_topic.sns_topic.subscriptions }}"
|
subscriptions: "{{ third_party_topic.sns_topic.subscriptions }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
register: third_party_unsubscribe
|
register: third_party_unsubscribe
|
||||||
|
|
||||||
|
@ -253,7 +310,6 @@
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
state: absent
|
state: absent
|
||||||
subscriptions: "{{ subscriptions }}"
|
subscriptions: "{{ subscriptions }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
ignore_errors: yes
|
ignore_errors: yes
|
||||||
register: third_party_deletion
|
register: third_party_deletion
|
||||||
|
@ -261,7 +317,6 @@
|
||||||
- name: no-op after third party deletion
|
- name: no-op after third party deletion
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
register: third_party_deletion_facts
|
register: third_party_deletion_facts
|
||||||
|
|
||||||
|
@ -281,7 +336,6 @@
|
||||||
sns_topic:
|
sns_topic:
|
||||||
name: "{{ sns_topic_topic_name }}"
|
name: "{{ sns_topic_topic_name }}"
|
||||||
state: absent
|
state: absent
|
||||||
<<: *aws_connection_info
|
|
||||||
ignore_errors: yes
|
ignore_errors: yes
|
||||||
|
|
||||||
- name: unsubscribe from third party topic
|
- name: unsubscribe from third party topic
|
||||||
|
@ -289,7 +343,6 @@
|
||||||
name: "{{ sns_topic_third_party_topic_arn }}"
|
name: "{{ sns_topic_third_party_topic_arn }}"
|
||||||
subscriptions: []
|
subscriptions: []
|
||||||
purge_subscriptions: yes
|
purge_subscriptions: yes
|
||||||
<<: *aws_connection_info
|
|
||||||
region: "{{ sns_topic_third_party_region }}"
|
region: "{{ sns_topic_third_party_region }}"
|
||||||
ignore_errors: yes
|
ignore_errors: yes
|
||||||
|
|
||||||
|
@ -297,7 +350,6 @@
|
||||||
lambda:
|
lambda:
|
||||||
name: '{{ sns_topic_lambda_name }}'
|
name: '{{ sns_topic_lambda_name }}'
|
||||||
state: absent
|
state: absent
|
||||||
<<: *aws_connection_info
|
|
||||||
ignore_errors: yes
|
ignore_errors: yes
|
||||||
|
|
||||||
- name: remove tempdir
|
- name: remove tempdir
|
||||||
|
|
Loading…
Reference in a new issue