8000 fix SNS subscribe idempotency (#9167) · codeperl/localstack@35e4d87 · GitHub
[go: up one dir, main page]
Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 35e4d87

Browse files
bentskubentsku
authored
fix SNS subscribe idempotency (localstack#9167)
1 parent ab92c16 commit 35e4d87

File tree

3 files changed

+55
-60
lines changed

3 files changed

+55
-60
lines changed

‎localstack/services/sns/provider.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -578,11 +578,14 @@ def subscribe(
578578
for existing_topic_subscription in store.topic_subscriptions.get(topic_arn, []):
579579
sub = store.subscriptions.get(existing_topic_subscription, {})
580580
if sub.get("Endpoint") == endpoint:
581-
for attr in sns_constants.VALID_SUBSCRIPTION_ATTR_NAME:
582-
if attributes and sub.get(attr) != attributes.get(attr):
583-
raise InvalidParameterException(
584-
"Invalid parameter: Attributes Reason: Subscription already exists with different attributes"
585-
)
581+
if attributes:
582+
# validate the subscription attributes aren't different
583+
for attr in sns_constants.VALID_SUBSCRIPTION_ATTR_NAME:
584+
# if a new attribute is present and different from an existent one, raise
585+
if (new_attr := attributes.get(attr)) and sub.get(attr) != new_attr:
586+
raise InvalidParameterException(
587+
"Invalid parameter: Attributes Reason: Subscription already exists with different attributes"
588+
)
586589

587590
return SubscribeResponse(SubscriptionArn=sub["SubscriptionArn"])
588591

‎tests/aws/services/sns/test_sns.py

Lines changed: 35 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -740,14 +740,23 @@ def test_subscribe_idempotency(
740740
queue_url = sqs_create_queue()
741741
queue_arn = sqs_get_queue_arn(queue_url)
742742

743-
subscribe_resp = aws_client.sns.subscribe(
744-
TopicArn=topic_arn,
745-
Protocol="sqs",
746-
Endpoint=queue_arn,
747-
Attributes={
743+
def subscribe_queue_to_topic(attributes: dict = None) -> dict:
744+
kwargs = {}
745+
if attributes is not None:
746+
kwargs["Attributes"] = attributes
747+
response = aws_client.sns.subscribe(
748+
TopicArn=topic_arn,
749+
Protocol="sqs",
750+
Endpoint=queue_arn,
751+
ReturnSubscriptionArn=True,
752+
**kwargs,
753+
)
754+
return response
755+
756+
subscribe_resp = subscribe_queue_to_topic(
757+
{
748758
"RawMessageDelivery": "true",
749-
},
750-
ReturnSubscriptionArn=True,
759+
}
751760
)
752761
snapshot.match("subscribe", subscribe_resp)
753762

@@ -756,61 +765,41 @@ def test_subscribe_idempotency(
756765
)
757766
snapshot.match("get-sub-attrs", get_attrs_resp)
758767

759-
subscribe_resp = aws_client.sns.subscribe(
760-
TopicArn=topic_arn,
761-
Protocol="sqs",
762-
Endpoint=queue_arn,
763-
Attributes={
768+
subscribe_resp = subscribe_queue_to_topic(
769+
{
770+
"RawMessageDelivery": "true",
771+
}
772+
)
773+
snapshot.match("subscribe-exact-same-raw", subscribe_resp)
774+
775+
subscribe_resp = subscribe_queue_to_topic(
776+
{
764777
"RawMessageDelivery": "true",
765778
"FilterPolicyScope": "MessageAttributes", # test if it also matches default values
766-
},
767-
ReturnSubscriptionArn=True,
779+
}
768780
)
781+
769782
snapshot.match("subscribe-idempotent", subscribe_resp)
770783

771784
# no attributes and empty attributes are working as well
772-
subscribe_resp = aws_client.sns.subscribe(
773-
TopicArn=topic_arn,
774-
Protocol="sqs",
775-
Endpoint=queue_arn,
776-
ReturnSubscriptionArn=True,
777-
)
785+
subscribe_resp = subscribe_queue_to_topic()
778786
snapshot.match("subscribe-idempotent-no-attributes", subscribe_resp)
779787

780-
subscribe_resp = aws_client.sns.subscribe(
781-
TopicArn=topic_arn,
782-
Protocol="sqs",
783-
Endpoint=queue_arn,
784-
ReturnSubscriptionArn=True,
785-
Attributes={},
786-
)
788+
subscribe_resp = subscribe_queue_to_topic({})
787789
snapshot.match("subscribe-idempotent-empty-attributes", subscribe_resp)
788790

791+
subscribe_resp = subscribe_queue_to_topic({"FilterPolicyScope": "MessageAttributes"})
792+
snapshot.match("subscribe-missing-attributes", subscribe_resp)
793+
789794
with pytest.raises(ClientError) as e:
790-
aws_client.sns.subscribe(
791-
TopicArn=topic_arn,
792-
Protocol="sqs",
793-
Endpoint=queue_arn,
794-
Attributes={
795+
subscribe_queue_to_topic(
796+
{
795797
"RawMessageDelivery": "false",
796798
"FilterPolicyScope": "MessageBody",
797-
},
798-
ReturnSubscriptionArn=True,
799+
}
799800
)
800801
snapshot.match("subscribe-diff-attributes", e.value.response)
801802

802-
with pytest.raises(ClientError) as e:
803-
aws_client.sns.subscribe(
804-
TopicArn=topic_arn,
805-
Protocol="sqs",
806-
Endpoint=queue_arn,
807-
Attributes={
808-
"RawMessageDelivery": "false",
809-
},
810-
ReturnSubscriptionArn=True,
811-
)
812-
snapshot.match("subscribe-missing-attributes", e.value.response)
813-
814803

815804
class TestSNSSubscriptionLambda:
816805
@markers.aws.validated

‎tests/aws/services/sns/test_sns.snapshot.json

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4166,7 +4166,7 @@
41664166
}
41674167
},
41684168
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_subscribe_idempotency": {
4169-
"recorded-date": "04-09-2023, 09:45:33",
4169+
"recorded-date": "15-09-2023, 17:29:11",
41704170
"recorded-content": {
41714171
"subscribe": {
41724172
"SubscriptionArn": "arn:aws:sns:<region>:111111111111:<resource:4>:<resource:1>",
@@ -4192,6 +4192,13 @@
41924192
"HTTPStatusCode": 200
41934193
}
41944194
},
4195+
"subscribe-exact-same-raw": {
4196+
"SubscriptionArn": "arn:aws:sns:<region>:111111111111:<resource:4>:<resource:1>",
4197+
"ResponseMetadata": {
4198+
"HTTPHeaders": {},
4199+
"HTTPStatusCode": 200
4200+
}
4201+
},
41954202
"subscribe-idempotent": {
41964203
"SubscriptionArn": "arn:aws:sns:<region>:111111111111:<resource:4>:<resource:1>",
41974204
"ResponseMetadata": {
@@ -4213,18 +4220,14 @@
42134220
"HTTPStatusCode": 200
42144221
}
42154222
},
4216-
"subscribe-diff-attributes": {
4217-
"Error": {
4218-
"Code": "InvalidParameter",
4219-
"Message": "Invalid parameter: Attributes Reason: Subscription already exists with different attributes",
4220-
"Type": "Sender"
4221-
},
4223+
"subscribe-missing-attributes": {
4224+
"SubscriptionArn": "arn:aws:sns:<region>:111111111111:<resource:4>:<resource:1>",
42224225
"ResponseMetadata": {
42234226
"HTTPHeaders": {},
4224-
"HTTPStatusCode": 400
4227+
"HTTPStatusCode": 200
42254228
}
42264229
},
4227-
"subscribe-missing-attributes": {
4230+
"subscribe-diff-attributes": {
42284231
"Error": {
42294232
"Code": "InvalidParameter",
42304233
"Message": "Invalid parameter: Attributes Reason: Subscription already exists with different attributes",

0 commit comments

Comments
 (0)
0