Skip to content

Commit

Permalink
fix SNS RawMessageDelivery casing (#10575)
Browse files Browse the repository at this point in the history
  • Loading branch information
bentsku committed Mar 29, 2024
1 parent 0fea3e0 commit 712e677
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 21 deletions.
1 change: 0 additions & 1 deletion localstack/services/moto.py
Expand Up @@ -59,7 +59,6 @@ def call_moto_with_request(
:param context: the original request context
:param service_request: the dictionary containing the service request parameters
:param override_headers: whether to override headers that are also request parameters
:return: an ASF ServiceResponse (same as a service provider would return)
"""
local_context = create_aws_request_context(
Expand Down
61 changes: 47 additions & 14 deletions localstack/services/sns/provider.py
Expand Up @@ -29,8 +29,10 @@
PublishBatchResponse,
PublishBatchResultEntry,
PublishResponse,
SetSubscriptionAttributesInput,
SnsApi,
String,
SubscribeInput,
SubscribeResponse,
SubscriptionAttributesMap,
TagKeyList,
Expand All @@ -51,7 +53,7 @@
from localstack.constants import AWS_REGION_US_EAST_1, DEFAULT_AWS_ACCOUNT_ID
from localstack.http import Request, Response, Router, route
from localstack.services.edge import ROUTER
from localstack.services.moto import call_moto
from localstack.services.moto import call_moto, call_moto_with_request
from localstack.services.plugins import ServiceLifecycleHook
from localstack.services.sns import constants as sns_constants
from localstack.services.sns.certificate import SNS_SERVER_CERT
Expand Down Expand Up @@ -264,8 +266,15 @@ def set_subscription_attributes(
topic_arn=sub["TopicArn"],
endpoint=sub["Endpoint"],
)
if attribute_name == "RawMessageDelivery":
attribute_value = attribute_value.lower()
try:
call_moto(context)
request = SetSubscriptionAttributesInput(
SubscriptionArn=subscription_arn,
AttributeName=attribute_name,
AttributeValue=attribute_value,
)
call_moto_with_request(context, service_request=request)
except CommonServiceException as e:
# Moto errors don't send the "Type": "Sender" field in their SNS exception
if e.code == "InvalidParameter":
Expand Down Expand Up @@ -613,9 +622,25 @@ def subscribe(
attribute_value=attr_value,
topic_arn=topic_arn,
endpoint=endpoint,
is_subscribe_call=True,
)
if attributes and "RawMessageDelivery" in attributes:
# Moto does not lower case the value, so we need to override the request
attrs_copy = {
**attributes,
"RawMessageDelivery": attributes["RawMessageDelivery"].lower(),
}
request = SubscribeInput(
TopicArn=topic_arn,
Protocol=protocol,
Endpoint=endpoint,
Attributes=attrs_copy,
ReturnSubscriptionArn=return_subscription_arn,
)
moto_response = call_moto_with_request(context, service_request=request)
else:
moto_response = call_moto(context)

moto_response = call_moto(context)
subscription_arn = moto_response.get("SubscriptionArn")
parsed_topic_arn = parse_and_validate_topic_arn(topic_arn)

Expand Down Expand Up @@ -658,6 +683,8 @@ def subscribe(
store.subscription_filter_policy[subscription_arn] = (
json.loads(attributes["FilterPolicy"]) if attributes["FilterPolicy"] else None
)
if raw_msg_delivery := attributes.get("RawMessageDelivery"):
subscription["RawMessageDelivery"] = raw_msg_delivery.lower()

store.subscriptions[subscription_arn] = subscription

Expand Down Expand Up @@ -770,6 +797,7 @@ def validate_subscription_attribute(
attribute_value: str,
topic_arn: str,
endpoint: str,
is_subscribe_call: bool = False,
) -> None:
"""
Validate the subscription attribute to be set. See:
Expand All @@ -778,48 +806,53 @@ def validate_subscription_attribute(
:param attribute_value: the subscription attribute value
:param topic_arn: the topic_arn of the subscription, needed to know if it is FIFO
:param endpoint: the subscription endpoint (like an SQS queue ARN)
:param is_subscribe_call: the error message is different if called from Subscribe or SetSubscriptionAttributes
:raises InvalidParameterException
:return:
"""
error_prefix = (
"Invalid parameter: Attributes Reason: " if is_subscribe_call else "Invalid parameter: "
)
if attribute_name not in sns_constants.VALID_SUBSCRIPTION_ATTR_NAME:
raise InvalidParameterException("Invalid parameter: AttributeName")
raise InvalidParameterException(f"{error_prefix}AttributeName")

if attribute_name == "FilterPolicy":
try:
json.loads(attribute_value or "{}")
except json.JSONDecodeError:
raise InvalidParameterException(
"Invalid parameter: FilterPolicy: failed to parse JSON."
)
raise InvalidParameterException(f"{error_prefix}FilterPolicy: failed to parse JSON.")
elif attribute_name == "FilterPolicyScope":
if attribute_value not in ("MessageAttributes", "MessageBody"):
raise InvalidParameterException(
f"Invalid parameter: FilterPolicyScope: Invalid value [{attribute_value}]. Please use either MessageBody or MessageAttributes"
f"{error_prefix}FilterPolicyScope: Invalid value [{attribute_value}]. "
f"Please use either MessageBody or MessageAttributes"
)
elif attribute_name == "RawMessageDelivery":
# TODO: only for SQS and https(s) subs, + firehose
return
if attribute_value.lower() not in ("true", "false"):
raise InvalidParameterException(
f"{error_prefix}RawMessageDelivery: Invalid value [{attribute_value}]. "
f"Must be true or false."
)

elif attribute_name == "RedrivePolicy":
try:
dlq_target_arn = json.loads(attribute_value).get("deadLetterTargetArn", "")
except json.JSONDecodeError:
raise InvalidParameterException(
"Invalid parameter: RedrivePolicy: failed to parse JSON."
)
raise InvalidParameterException(f"{error_prefix}RedrivePolicy: failed to parse JSON.")
try:
parsed_arn = parse_arn(dlq_target_arn)
except InvalidArnException:
raise InvalidParameterException(
"Invalid parameter: RedrivePolicy: deadLetterTargetArn is an invalid arn"
f"{error_prefix}RedrivePolicy: deadLetterTargetArn is an invalid arn"
)

if topic_arn.endswith(".fifo"):
if endpoint.endswith(".fifo") and (
not parsed_arn["resource"].endswith(".fifo") or "sqs" not in parsed_arn["service"]
):
raise InvalidParameterException(
"Invalid parameter: RedrivePolicy: must use a FIFO queue as DLQ for a FIFO Subscription to a FIFO Topic."
f"{error_prefix}RedrivePolicy: must use a FIFO queue as DLQ for a FIFO Subscription to a FIFO Topic."
)


Expand Down
32 changes: 31 additions & 1 deletion tests/aws/services/sns/test_sns.py
Expand Up @@ -580,12 +580,26 @@ def test_create_subscriptions_with_attributes(
queue_url = sqs_create_queue()
queue_arn = sqs_get_queue_arn(queue_url)

with pytest.raises(ClientError) as e:
sns_subscription(
TopicArn=topic_arn,
Protocol="sqs",
Endpoint=queue_arn,
Attributes={
"RawMessageDelivery": "wrongvalue", # set an weird case value, SNS will lower it
"FilterPolicyScope": "MessageBody",
"FilterPolicy": "",
},
ReturnSubscriptionArn=True,
)
snapshot.match("subscribe-wrong-attr", e.value.response)

subscribe_resp = sns_subscription(
TopicArn=topic_arn,
Protocol="sqs",
Endpoint=queue_arn,
Attributes={
"RawMessageDelivery": "true",
"RawMessageDelivery": "TrUe", # set an weird case value, SNS will lower it
"FilterPolicyScope": "MessageBody",
"FilterPolicy": "",
},
Expand Down Expand Up @@ -682,6 +696,22 @@ def test_validate_set_sub_attributes(
)
snapshot.match("fake-attribute", e.value.response)

with pytest.raises(ClientError) as e:
aws_client.sns.set_subscription_attributes(
SubscriptionArn=sub_arn,
AttributeName="RawMessageDelivery",
AttributeValue="test-ValUe",
)
snapshot.match("raw-message-wrong-value", e.value.response)

with pytest.raises(ClientError) as e:
aws_client.sns.set_subscription_attributes(
SubscriptionArn=sub_arn,
AttributeName="RawMessageDelivery",
AttributeValue="",
)
snapshot.match("raw-message-empty-value", e.value.response)

with pytest.raises(ClientError) as e:
aws_client.sns.set_subscription_attributes(
SubscriptionArn=sub_arn,
Expand Down
39 changes: 36 additions & 3 deletions tests/aws/services/sns/test_sns.snapshot.json
Expand Up @@ -653,8 +653,19 @@
}
},
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_create_subscriptions_with_attributes": {
"recorded-date": "24-08-2023, 23:27:53",
"recorded-date": "29-03-2024, 19:44:43",
"recorded-content": {
"subscribe-wrong-attr": {
"Error": {
"Code": "InvalidParameter",
"Message": "Invalid parameter: Attributes Reason: RawMessageDelivery: Invalid value [wrongvalue]. Must be true or false.",
"Type": "Sender"
},
"ResponseMetadata": {
"HTTPHeaders": {},
"HTTPStatusCode": 400
}
},
"subscribe": {
"SubscriptionArn": "arn:aws:sns:<region>:111111111111:<resource:4>:<resource:1>",
"ResponseMetadata": {
Expand Down Expand Up @@ -755,7 +766,7 @@
}
},
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_validate_set_sub_attributes": {
"recorded-date": "24-08-2023, 23:27:58",
"recorded-date": "29-03-2024, 19:30:24",
"recorded-content": {
"fake-attribute": {
"Error": {
Expand All @@ -768,6 +779,28 @@
"HTTPStatusCode": 400
}
},
"raw-message-wrong-value": {
"Error": {
"Code": "InvalidParameter",
"Message": "Invalid parameter: RawMessageDelivery: Invalid value [test-ValUe]. Must be true or false.",
"Type": "Sender"
},
"ResponseMetadata": {
"HTTPHeaders": {},
"HTTPStatusCode": 400
}
},
"raw-message-empty-value": {
"Error": {
"Code": "InvalidParameter",
"Message": "Invalid parameter: RawMessageDelivery: Invalid value []. Must be true or false.",
"Type": "Sender"
},
"ResponseMetadata": {
"HTTPHeaders": {},
"HTTPStatusCode": 400
}
},
"fake-arn-redrive-policy": {
"Error": {
"Code": "InvalidParameter",
Expand All @@ -782,7 +815,7 @@
"invalid-json-redrive-policy": {
"Error": {
"Code": "InvalidParameter",
"Message": "Invalid parameter: RedrivePolicy: failed to parse JSON. Unexpected character ('i' (code 105)): was expecting double-quote to start field name\n at [Source: java.io.StringReader@5498a819; line: 1, column: 3]",
"Message": "Invalid parameter: RedrivePolicy: failed to parse JSON. Unexpected character ('i' (code 105)): was expecting double-quote to start field name\n at [Source: java.io.StringReader@469cc9aa; line: 1, column: 3]",
"Type": "Sender"
},
"ResponseMetadata": {
Expand Down
4 changes: 2 additions & 2 deletions tests/aws/services/sns/test_sns.validation.json
Expand Up @@ -78,7 +78,7 @@
"last_validated_date": "2023-08-24T22:20:07+00:00"
},
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_create_subscriptions_with_attributes": {
"last_validated_date": "2023-08-24T21:27:53+00:00"
"last_validated_date": "2024-03-29T19:44:42+00:00"
},
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_list_subscriptions": {
"last_validated_date": "2023-08-25T14:23:53+00:00"
Expand All @@ -105,7 +105,7 @@
"last_validated_date": "2023-10-20T10:52:36+00:00"
},
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionCrud::test_validate_set_sub_attributes": {
"last_validated_date": "2023-08-24T21:27:58+00:00"
"last_validated_date": "2024-03-29T19:30:23+00:00"
},
"tests/aws/services/sns/test_sns.py::TestSNSSubscriptionHttp::test_subscribe_external_http_endpoint[False]": {
"last_validated_date": "2023-10-11T22:47:29+00:00"
Expand Down

0 comments on commit 712e677

Please sign in to comment.