From e801ab3749662986dff770ed15dd60247fb51f73 Mon Sep 17 00:00:00 2001 From: Laurence Warne Date: Sun, 7 Jul 2024 11:35:27 +0100 Subject: [PATCH 1/6] feat(sns): add delivery policy to sns subscriptions --- .../SnsToUrlStack.assets.json | 19 ++ .../SnsToUrlStack.template.json | 64 +++++ .../test/integ.sns-url.js.snapshot/cdk.out | 1 + ...efaultTestDeployAssert83AD650C.assets.json | 19 ++ ...aultTestDeployAssert83AD650C.template.json | 36 +++ .../test/integ.sns-url.js.snapshot/integ.json | 12 + .../integ.sns-url.js.snapshot/manifest.json | 119 +++++++++ .../test/integ.sns-url.js.snapshot/tree.json | 164 ++++++++++++ .../test/integ.sns-url.ts | 40 +++ .../aws-sns-subscriptions/README.md | 29 +++ .../aws-sns-subscriptions/lib/url.ts | 8 + .../aws-sns/lib/delivery-policy.ts | 100 ++++++++ packages/aws-cdk-lib/aws-sns/lib/index.ts | 1 + .../aws-cdk-lib/aws-sns/lib/subscription.ts | 66 +++++ .../aws-sns/test/subscription.test.ts | 240 ++++++++++++++++++ 15 files changed, 918 insertions(+) create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.assets.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.template.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdk.out create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.assets.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.template.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/integ.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/manifest.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/tree.json create mode 100644 packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts create mode 100644 packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.assets.json new file mode 100644 index 0000000000000..156438fef8333 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.assets.json @@ -0,0 +1,19 @@ +{ + "version": "36.0.0", + "files": { + "8a2787919c117033d0683a025f8e14a095ddca0e52f77112498af03dd6bd96ec": { + "source": { + "path": "SnsToUrlStack.template.json", + "packaging": "file" + }, + "destinations": { + "current_account-current_region": { + "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", + "objectKey": "8a2787919c117033d0683a025f8e14a095ddca0e52f77112498af03dd6bd96ec.json", + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" + } + } + } + }, + "dockerImages": {} +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.template.json new file mode 100644 index 0000000000000..56c33a78db24a --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/SnsToUrlStack.template.json @@ -0,0 +1,64 @@ +{ + "Resources": { + "MyTopic86869434": { + "Type": "AWS::SNS::Topic" + }, + "MyTopichttpsfoobarcomDEA92AB5": { + "Type": "AWS::SNS::Subscription", + "Properties": { + "DeliveryPolicy": { + "healthyRetryPolicy": { + "minDelayTarget": 20, + "maxDelayTarget": 21, + "numRetries": 10 + }, + "throttlePolicy": { + "maxReceivesPerSecond": 10 + }, + "requestPolicy": { + "headerContentType": "application/json" + } + }, + "Endpoint": "https://foobar.com/", + "Protocol": "https", + "TopicArn": { + "Ref": "MyTopic86869434" + } + } + } + }, + "Parameters": { + "BootstrapVersion": { + "Type": "AWS::SSM::Parameter::Value", + "Default": "/cdk-bootstrap/hnb659fds/version", + "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]" + } + }, + "Rules": { + "CheckBootstrapVersion": { + "Assertions": [ + { + "Assert": { + "Fn::Not": [ + { + "Fn::Contains": [ + [ + "1", + "2", + "3", + "4", + "5" + ], + { + "Ref": "BootstrapVersion" + } + ] + } + ] + }, + "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI." + } + ] + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdk.out b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdk.out new file mode 100644 index 0000000000000..1f0068d32659a --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdk.out @@ -0,0 +1 @@ +{"version":"36.0.0"} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.assets.json new file mode 100644 index 0000000000000..59e2cd2727a59 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.assets.json @@ -0,0 +1,19 @@ +{ + "version": "36.0.0", + "files": { + "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": { + "source": { + "path": "cdkintegDefaultTestDeployAssert83AD650C.template.json", + "packaging": "file" + }, + "destinations": { + "current_account-current_region": { + "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", + "objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json", + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" + } + } + } + }, + "dockerImages": {} +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.template.json new file mode 100644 index 0000000000000..ad9d0fb73d1dd --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/cdkintegDefaultTestDeployAssert83AD650C.template.json @@ -0,0 +1,36 @@ +{ + "Parameters": { + "BootstrapVersion": { + "Type": "AWS::SSM::Parameter::Value", + "Default": "/cdk-bootstrap/hnb659fds/version", + "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]" + } + }, + "Rules": { + "CheckBootstrapVersion": { + "Assertions": [ + { + "Assert": { + "Fn::Not": [ + { + "Fn::Contains": [ + [ + "1", + "2", + "3", + "4", + "5" + ], + { + "Ref": "BootstrapVersion" + } + ] + } + ] + }, + "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI." + } + ] + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/integ.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/integ.json new file mode 100644 index 0000000000000..cebe64cd92ad3 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/integ.json @@ -0,0 +1,12 @@ +{ + "version": "36.0.0", + "testCases": { + "cdk-integ/DefaultTest": { + "stacks": [ + "SnsToUrlStack" + ], + "assertionStack": "cdk-integ/DefaultTest/DeployAssert", + "assertionStackName": "cdkintegDefaultTestDeployAssert83AD650C" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/manifest.json new file mode 100644 index 0000000000000..172b65b13adae --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/manifest.json @@ -0,0 +1,119 @@ +{ + "version": "36.0.0", + "artifacts": { + "SnsToUrlStack.assets": { + "type": "cdk:asset-manifest", + "properties": { + "file": "SnsToUrlStack.assets.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "SnsToUrlStack": { + "type": "aws:cloudformation:stack", + "environment": "aws://unknown-account/unknown-region", + "properties": { + "templateFile": "SnsToUrlStack.template.json", + "terminationProtection": false, + "validateOnSynth": false, + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", + "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/8a2787919c117033d0683a025f8e14a095ddca0e52f77112498af03dd6bd96ec.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", + "additionalDependencies": [ + "SnsToUrlStack.assets" + ], + "lookupRole": { + "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}", + "requiresBootstrapStackVersion": 8, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "dependencies": [ + "SnsToUrlStack.assets" + ], + "metadata": { + "/SnsToUrlStack/MyTopic/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "MyTopic86869434" + } + ], + "/SnsToUrlStack/MyTopic/https:----foobar.com--/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "MyTopichttpsfoobarcomDEA92AB5" + } + ], + "/SnsToUrlStack/BootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "BootstrapVersion" + } + ], + "/SnsToUrlStack/CheckBootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "CheckBootstrapVersion" + } + ] + }, + "displayName": "SnsToUrlStack" + }, + "cdkintegDefaultTestDeployAssert83AD650C.assets": { + "type": "cdk:asset-manifest", + "properties": { + "file": "cdkintegDefaultTestDeployAssert83AD650C.assets.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "cdkintegDefaultTestDeployAssert83AD650C": { + "type": "aws:cloudformation:stack", + "environment": "aws://unknown-account/unknown-region", + "properties": { + "templateFile": "cdkintegDefaultTestDeployAssert83AD650C.template.json", + "terminationProtection": false, + "validateOnSynth": false, + "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", + "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json", + "requiresBootstrapStackVersion": 6, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", + "additionalDependencies": [ + "cdkintegDefaultTestDeployAssert83AD650C.assets" + ], + "lookupRole": { + "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}", + "requiresBootstrapStackVersion": 8, + "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version" + } + }, + "dependencies": [ + "cdkintegDefaultTestDeployAssert83AD650C.assets" + ], + "metadata": { + "/cdk-integ/DefaultTest/DeployAssert/BootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "BootstrapVersion" + } + ], + "/cdk-integ/DefaultTest/DeployAssert/CheckBootstrapVersion": [ + { + "type": "aws:cdk:logicalId", + "data": "CheckBootstrapVersion" + } + ] + }, + "displayName": "cdk-integ/DefaultTest/DeployAssert" + }, + "Tree": { + "type": "cdk:tree", + "properties": { + "file": "tree.json" + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/tree.json new file mode 100644 index 0000000000000..bcb578d89a04f --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.js.snapshot/tree.json @@ -0,0 +1,164 @@ +{ + "version": "tree-0.1", + "tree": { + "id": "App", + "path": "", + "children": { + "SnsToUrlStack": { + "id": "SnsToUrlStack", + "path": "SnsToUrlStack", + "children": { + "MyTopic": { + "id": "MyTopic", + "path": "SnsToUrlStack/MyTopic", + "children": { + "Resource": { + "id": "Resource", + "path": "SnsToUrlStack/MyTopic/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::SNS::Topic", + "aws:cdk:cloudformation:props": {} + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "https:----foobar.com--": { + "id": "https:----foobar.com--", + "path": "SnsToUrlStack/MyTopic/https:----foobar.com--", + "children": { + "Resource": { + "id": "Resource", + "path": "SnsToUrlStack/MyTopic/https:----foobar.com--/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::SNS::Subscription", + "aws:cdk:cloudformation:props": { + "deliveryPolicy": { + "healthyRetryPolicy": { + "minDelayTarget": 20, + "maxDelayTarget": 21, + "numRetries": 10 + }, + "throttlePolicy": { + "maxReceivesPerSecond": 10 + }, + "requestPolicy": { + "headerContentType": "application/json" + } + }, + "endpoint": "https://foobar.com/", + "protocol": "https", + "topicArn": { + "Ref": "MyTopic86869434" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "BootstrapVersion": { + "id": "BootstrapVersion", + "path": "SnsToUrlStack/BootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "CheckBootstrapVersion": { + "id": "CheckBootstrapVersion", + "path": "SnsToUrlStack/CheckBootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "cdk-integ": { + "id": "cdk-integ", + "path": "cdk-integ", + "children": { + "DefaultTest": { + "id": "DefaultTest", + "path": "cdk-integ/DefaultTest", + "children": { + "Default": { + "id": "Default", + "path": "cdk-integ/DefaultTest/Default", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "DeployAssert": { + "id": "DeployAssert", + "path": "cdk-integ/DefaultTest/DeployAssert", + "children": { + "BootstrapVersion": { + "id": "BootstrapVersion", + "path": "cdk-integ/DefaultTest/DeployAssert/BootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "CheckBootstrapVersion": { + "id": "CheckBootstrapVersion", + "path": "cdk-integ/DefaultTest/DeployAssert/CheckBootstrapVersion", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "Tree": { + "id": "Tree", + "path": "Tree", + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts new file mode 100644 index 0000000000000..7a3e65b93ef58 --- /dev/null +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts @@ -0,0 +1,40 @@ +import * as sns from 'aws-cdk-lib/aws-sns'; +import * as cdk from 'aws-cdk-lib'; +import * as subs from 'aws-cdk-lib/aws-sns-subscriptions'; +import * as integ from '@aws-cdk/integ-tests-alpha'; + +class SnsToUrlStack extends cdk.Stack { + topic: sns.Topic; + constructor(scope: cdk.App, id: string, props?: cdk.StackProps) { + super(scope, id, props); + this.topic = new sns.Topic(this, 'MyTopic'); + + this.topic.addSubscription( + new subs.UrlSubscription( + 'https://foobar.com/', + { + deliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 20, + maxDelayTarget: 21, + numRetries: 10, + }, + throttlePolicy: { + maxReceivesPerSecond: 10, + }, + requestPolicy: { + headerContentType: 'application/json', + }, + }, + }, + ), + ); + } +} + +const app = new cdk.App(); +const stack = new SnsToUrlStack(app, 'SnsToUrlStack'); + +new integ.IntegTest(app, 'cdk-integ', { + testCases: [stack], +}); diff --git a/packages/aws-cdk-lib/aws-sns-subscriptions/README.md b/packages/aws-cdk-lib/aws-sns-subscriptions/README.md index c7ab5ea003638..3e62619487e35 100644 --- a/packages/aws-cdk-lib/aws-sns-subscriptions/README.md +++ b/packages/aws-cdk-lib/aws-sns-subscriptions/README.md @@ -44,6 +44,35 @@ const url = new CfnParameter(this, 'url-param'); myTopic.addSubscription(new subscriptions.UrlSubscription(url.valueAsString)); ``` +The [delivery policy](https://docs.aws.amazon.com/sns/latest/dg/sns-message-delivery-retries.html) can also be set like so: + +```ts +const myTopic = new sns.Topic(this, 'MyTopic'); + +myTopic.addSubscription( + new subscriptions.UrlSubscription( + 'https://foobar.com/', + { + deliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 5, + maxDelayTarget: 10, + numRetries: 6, + backoffFunction: sns.BackoffFunction.EXPONENTIAL, + }, + throttlePolicy: { + maxReceivesPerSecond: 10, + }, + requestPolicy: { + headerContentType: 'application/json', + }, + }, + }, + ), +); +``` + + ### Amazon SQS Subscribe a queue to your topic: diff --git a/packages/aws-cdk-lib/aws-sns-subscriptions/lib/url.ts b/packages/aws-cdk-lib/aws-sns-subscriptions/lib/url.ts index 3eb582c1a8b7a..59bb26724f716 100644 --- a/packages/aws-cdk-lib/aws-sns-subscriptions/lib/url.ts +++ b/packages/aws-cdk-lib/aws-sns-subscriptions/lib/url.ts @@ -21,6 +21,13 @@ export interface UrlSubscriptionProps extends SubscriptionProps { * @default - Protocol is derived from url */ readonly protocol?: sns.SubscriptionProtocol; + + /** + * The delivery policy. + * + * @default - if the initial delivery of the message fails, three retries with a delay between failed attempts set at 20 seconds + */ + readonly deliveryPolicy?: sns.DeliveryPolicy; } /** @@ -63,6 +70,7 @@ export class UrlSubscription implements sns.ITopicSubscription { filterPolicy: this.props.filterPolicy, filterPolicyWithMessageBody: this.props.filterPolicyWithMessageBody, deadLetterQueue: this.props.deadLetterQueue, + deliveryPolicy: this.props.deliveryPolicy, }; } } diff --git a/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts b/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts new file mode 100644 index 0000000000000..1a46bac9f7c69 --- /dev/null +++ b/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts @@ -0,0 +1,100 @@ +/** + * Algorithms which can be used by SNS to calculate the delays associated with all of the retry attempts between the first and last retries in the backoff phase. + */ +export enum BackoffFunction { + /** Arithmetic. + */ + ARITHMETIC = 'ARITHMETIC', + /** Geometric. + */ + EXPONENTIAL = 'EXPONENTIAL', + /** Linear. + */ + GEOMETRIC = 'GEOMETRIC', + /** Linear. + */ + LINEAR = 'LINEAR', +} + +/** + * Options for customising AWS SNS HTTP/S delivery throttling. + */ +export interface ThrottlePolicy { + /** The maximum number of deliveries per second, per subscription. + * @default - no throttling + */ + readonly maxReceivesPerSecond?: number; +} + +/** + * Options for customising aspects of the content sent in AWS SNS HTTP/S requests. + */ +export interface RequestPolicy { + /** The content type of the notification being sent to HTTP/S endpoints. + * @default - text/plain; charset=UTF-8 + */ + readonly headerContentType?: string; +} + +/** + * Options for customising the retry policy of the delivery of SNS messages to HTTP/S endpoints. + */ +export interface HealthyRetryPolicy { + /** The minimum delay for a retry (in seconds). Must be at least 1 and not exceed `maxDelayTarget`. + * @default 20 + */ + readonly minDelayTarget: number; + /** The maximum delay for a retry (in seconds). Must be at least `minDelayTarget` and less than 3,600. + * @default 20 + */ + readonly maxDelayTarget: number; + + /** The total number of retries, including immediate, pre-backoff, backoff, and post-backoff retries. Must be greater than or equal to zero and not exceed 100. + * @default 3 + */ + readonly numRetries: number; + + /** The number of retries to be done immediately, with no delay between them. Must be zero or greater. + * @default 0 + */ + readonly numNoDelayRetries?: number; + + /** The number of retries in the pre-backoff phase, with the specified minimum delay between them. Must be zero or greater + * @default 0 + */ + readonly numMinDelayRetries?: number; + + /** The number of retries in the post-backoff phase, with the maximum delay between them. Must be zero or greater + * @default 0 + */ + readonly numMaxDelayRetries?: number; + + /** The model for backoff between retries. + * @default - linear + */ + readonly backoffFunction?: BackoffFunction; +} + +/** + * Options for customising the delivery of SNS messages to HTTP/S endpoints. + */ +export interface DeliveryPolicy { + + /** + * The retry policy of the delivery of SNS messages to HTTP/S endpoints. + * @default - Amazon SNS attempts up to three retries with a delay between failed attempts set at 20 seconds + */ + readonly healthyRetryPolicy?: HealthyRetryPolicy; + + /** + * The throttling policy of the delivery of SNS messages to HTTP/S endpoints. + * @default - No throttling + */ + readonly throttlePolicy?: ThrottlePolicy; + + /** + * The request of the content sent in AWS SNS HTTP/S requests. + * @default - The content type is set to 'text/plain; charset=UTF-8' + */ + readonly requestPolicy?: RequestPolicy; +} diff --git a/packages/aws-cdk-lib/aws-sns/lib/index.ts b/packages/aws-cdk-lib/aws-sns/lib/index.ts index cdcb67084921e..2ea429718b107 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/index.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/index.ts @@ -4,6 +4,7 @@ export * from './topic-base'; export * from './subscription'; export * from './subscriber'; export * from './subscription-filter'; +export * from './delivery-policy'; // AWS::SNS CloudFormation Resources: export * from './sns.generated'; diff --git a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts index d03b1d942df28..a273f884633b1 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts @@ -1,4 +1,5 @@ import { Construct } from 'constructs'; +import { DeliveryPolicy } from './delivery-policy'; import { CfnSubscription } from './sns.generated'; import { SubscriptionFilter } from './subscription-filter'; import { ITopic } from './topic-base'; @@ -67,6 +68,13 @@ export interface SubscriptionOptions { * @default - No subscription role is provided */ readonly subscriptionRoleArn?: string; + + /** + * The delivery policy. + * + * @default - if the initial delivery of the message fails, three retries with a delay between failed attempts set at 20 seconds + */ + readonly deliveryPolicy?: DeliveryPolicy; } /** * Properties for creating a new subscription @@ -136,6 +144,43 @@ export class Subscription extends Resource { throw new Error('Subscription role arn is required field for subscriptions with a firehose protocol.'); } + if (props.deliveryPolicy) { + if (props.protocol !== SubscriptionProtocol.HTTP && props.protocol !== SubscriptionProtocol.HTTPS) { + throw new Error('Delivery policy is only supported for HTTP and HTTPS subscriptions'); + } + const { healthyRetryPolicy, throttlePolicy } = props.deliveryPolicy; + if (healthyRetryPolicy) { + const delayTargetLimit = 3600; + if (healthyRetryPolicy.minDelayTarget < 1 || healthyRetryPolicy.minDelayTarget > delayTargetLimit) { + throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimit} inclusive`); + } + if (healthyRetryPolicy.maxDelayTarget < 1 || healthyRetryPolicy.maxDelayTarget > delayTargetLimit) { + throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimit} inclusive`); + } + if (healthyRetryPolicy.minDelayTarget > healthyRetryPolicy.maxDelayTarget) { + throw new Error('minDelayTarget must not exceed maxDelayTarget'); + } + const numRetriesLimit = 100; + if (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit) { + throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive`); + } + if (healthyRetryPolicy.numNoDelayRetries && healthyRetryPolicy.numNoDelayRetries < 0) { + throw new Error('numNoDelayRetries must be zero or greater'); + } + if (healthyRetryPolicy.numMinDelayRetries && healthyRetryPolicy.numMinDelayRetries < 0) { + throw new Error('numMinDelayRetries must be zero or greater'); + } + if (healthyRetryPolicy.numMaxDelayRetries && healthyRetryPolicy.numMaxDelayRetries < 0) { + throw new Error('numMaxDelayRetries must be zero or greater'); + } + } + if (throttlePolicy) { + if (throttlePolicy.maxReceivesPerSecond !== undefined && throttlePolicy.maxReceivesPerSecond < 1) { + throw new Error('maxReceivesPerSecond must be greater than zero'); + } + } + } + // Format filter policy const filterPolicy = this.filterPolicyWithMessageBody ? buildFilterPolicyWithMessageBody(this.filterPolicyWithMessageBody) @@ -152,10 +197,31 @@ export class Subscription extends Resource { region: props.region, redrivePolicy: this.buildDeadLetterConfig(this.deadLetterQueue), subscriptionRoleArn: props.subscriptionRoleArn, + deliveryPolicy: props.deliveryPolicy ? this.renderDeliveryPolicy(props.deliveryPolicy): undefined, }); } + private renderDeliveryPolicy(deliveryPolicy: DeliveryPolicy): any { + return { + healthyRetryPolicy: deliveryPolicy.healthyRetryPolicy ? { + minDelayTarget: deliveryPolicy.healthyRetryPolicy.minDelayTarget, + maxDelayTarget: deliveryPolicy.healthyRetryPolicy.maxDelayTarget, + numRetries: deliveryPolicy.healthyRetryPolicy.numRetries, + numNoDelayRetries: deliveryPolicy.healthyRetryPolicy.numNoDelayRetries, + numMinDelayRetries: deliveryPolicy.healthyRetryPolicy.numMinDelayRetries, + numMaxDelayRetries: deliveryPolicy.healthyRetryPolicy.numMaxDelayRetries, + backoffFunction: deliveryPolicy.healthyRetryPolicy.backoffFunction, + }: undefined, + throttlePolicy: deliveryPolicy.throttlePolicy ? { + maxReceivesPerSecond: deliveryPolicy.throttlePolicy.maxReceivesPerSecond, + }: undefined, + requestPolicy: deliveryPolicy.requestPolicy ? { + headerContentType: deliveryPolicy.requestPolicy.headerContentType, + }: undefined, + }; + } + private buildDeadLetterQueue(props: SubscriptionProps) { if (!props.deadLetterQueue) { return undefined; diff --git a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts index 193f530b27704..762fc43b53bef 100644 --- a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts @@ -264,6 +264,51 @@ describe('Subscription', () => { }); + test('with delivery policy', () => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + // WHEN + new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 5, + maxDelayTarget: 10, + numRetries: 6, + backoffFunction: sns.BackoffFunction.EXPONENTIAL, + }, + throttlePolicy: { + maxReceivesPerSecond: 10, + }, + requestPolicy: { + headerContentType: 'application/json', + }, + }, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::SNS::Subscription', { + DeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 5, + maxDelayTarget: 10, + numRetries: 6, + backoffFunction: sns.BackoffFunction.EXPONENTIAL, + }, + throttlePolicy: { + maxReceivesPerSecond: 10, + }, + requestPolicy: { + headerContentType: 'application/json', + }, + }, + }); + }); + test.each( [ SubscriptionProtocol.LAMBDA, @@ -357,4 +402,199 @@ describe('Subscription', () => { topic, })).toThrow(/Subscription role arn is required field for subscriptions with a firehose protocol./); }); + + test.each([ + sns.SubscriptionProtocol.APPLICATION, + sns.SubscriptionProtocol.EMAIL, + sns.SubscriptionProtocol.EMAIL_JSON, + sns.SubscriptionProtocol.FIREHOSE, + sns.SubscriptionProtocol.LAMBDA, + sns.SubscriptionProtocol.SMS, + sns.SubscriptionProtocol.SQS, + ])('throws an error when deliveryPolicy is specified with protocol %s', (protocol) => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + //THEN + expect(() => new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 11, + maxDelayTarget: 10, + numRetries: 6, + }, + }, + protocol: protocol, + subscriptionRoleArn: '???', + topic, + })).toThrow(/Delivery policy is only supported for HTTP and HTTPS subscriptions/); + }); + + test('throws an error when deliveryPolicy minDelayTarget exceeds maxDelayTarget', () => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + //THEN + expect(() => new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 11, + maxDelayTarget: 10, + numRetries: 6, + }, + }, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + })).toThrow(/minDelayTarget must not exceed maxDelayTarget/); + }); + + const delayTestCases = [ + { + prop: 'minDelayTarget', + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 0, + maxDelayTarget: 10, + numRetries: 6, + }, + }, + }, + { + prop: 'maxDelayTarget', + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 10, + maxDelayTarget: 0, + numRetries: 6, + }, + }, + }, + { + prop: 'minDelayTarget', + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 3601, + maxDelayTarget: 10, + numRetries: 6, + }, + }, + }, + { + prop: 'maxDelayTarget', + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 10, + maxDelayTarget: 3601, + numRetries: 6, + }, + }, + }, + ]; + + delayTestCases.forEach(({ prop, invalidDeliveryPolicy }) => { + const invalidValue = invalidDeliveryPolicy.healthyRetryPolicy[prop]; + test(`throws an error when ${prop} is ${invalidValue}`, () => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + //THEN + expect(() => new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: invalidDeliveryPolicy, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + })).toThrow(new RegExp(`${prop} must be between 1 and 3600 inclusive`)); + }); + }); + + test.each([-1, 101])('throws an error when deliveryPolicy numRetries is %d', (invalidValue: number) => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + //THEN + expect(() => new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 10, + maxDelayTarget: 10, + numRetries: invalidValue, + }, + }, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + })).toThrow(/numRetries must be between 0 and 100 inclusive/); + }); + + test.each([ + { + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 1, + maxDelayTarget: 10, + numRetries: 6, + numNoDelayRetries: -1, + }, + }, + prop: 'numNoDelayRetries', + }, + { + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 1, + maxDelayTarget: 10, + numRetries: 6, + numMinDelayRetries: -1, + }, + }, + prop: 'numMinDelayRetries', + }, + { + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 1, + maxDelayTarget: 10, + numRetries: 6, + numMaxDelayRetries: -1, + }, + }, + prop: 'numMaxDelayRetries', + }, + ])('throws an error when $prop < 0', ({ invalidDeliveryPolicy, prop }) => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + //THEN + expect(() => new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: invalidDeliveryPolicy, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + })).toThrow(new RegExp(`${prop} must be zero or greater`)); + }); + + test('throws an error when throttlePolicy < 1', () => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + //THEN + expect(() => new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: { + throttlePolicy: { + maxReceivesPerSecond: 0, + }, + }, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + })).toThrow(/maxReceivesPerSecond must be greater than zero/); + }); }); From 6b2326177bd39b5d6d3804d7d5be9fc624035ef3 Mon Sep 17 00:00:00 2001 From: Laurence Warne Date: Sat, 21 Sep 2024 10:40:01 +0100 Subject: [PATCH 2/6] Address comments --- .../test/integ.sns-url.ts | 4 +- .../aws-sns-subscriptions/README.md | 4 +- .../aws-sns/lib/delivery-policy.ts | 61 +++++++++---- .../aws-cdk-lib/aws-sns/lib/subscription.ts | 89 ++++++++++--------- .../aws-sns/test/subscription.test.ts | 48 +++++----- 5 files changed, 120 insertions(+), 86 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts index 7a3e65b93ef58..18f602207f2e8 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-sns-subscriptions/test/integ.sns-url.ts @@ -15,8 +15,8 @@ class SnsToUrlStack extends cdk.Stack { { deliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 20, - maxDelayTarget: 21, + minDelayTarget: cdk.Duration.seconds(20), + maxDelayTarget: cdk.Duration.seconds(21), numRetries: 10, }, throttlePolicy: { diff --git a/packages/aws-cdk-lib/aws-sns-subscriptions/README.md b/packages/aws-cdk-lib/aws-sns-subscriptions/README.md index 3e62619487e35..15ebca379d55d 100644 --- a/packages/aws-cdk-lib/aws-sns-subscriptions/README.md +++ b/packages/aws-cdk-lib/aws-sns-subscriptions/README.md @@ -55,8 +55,8 @@ myTopic.addSubscription( { deliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 5, - maxDelayTarget: 10, + minDelayTarget: Duration.seconds(5), + maxDelayTarget: Duration.seconds(10), numRetries: 6, backoffFunction: sns.BackoffFunction.EXPONENTIAL, }, diff --git a/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts b/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts index 1a46bac9f7c69..427449015ae45 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts @@ -1,17 +1,23 @@ +import { Duration } from '../../core'; + /** * Algorithms which can be used by SNS to calculate the delays associated with all of the retry attempts between the first and last retries in the backoff phase. */ export enum BackoffFunction { - /** Arithmetic. + /** + * Arithmetic, see {@link https://docs.aws.amazon.com/images/sns/latest/dg/images/backoff-graph.png|this image} for how this function compares to others */ ARITHMETIC = 'ARITHMETIC', - /** Geometric. + /** + * Exponential, see {@link https://docs.aws.amazon.com/images/sns/latest/dg/images/backoff-graph.png|this image} for how this function compares to others */ EXPONENTIAL = 'EXPONENTIAL', - /** Linear. + /** + * Geometric, see {@link https://docs.aws.amazon.com/images/sns/latest/dg/images/backoff-graph.png|this image} for how this function compares to others */ GEOMETRIC = 'GEOMETRIC', - /** Linear. + /** + * Linear, see {@link https://docs.aws.amazon.com/images/sns/latest/dg/images/backoff-graph.png|this image} for how this function compares to others */ LINEAR = 'LINEAR', } @@ -20,7 +26,9 @@ export enum BackoffFunction { * Options for customising AWS SNS HTTP/S delivery throttling. */ export interface ThrottlePolicy { - /** The maximum number of deliveries per second, per subscription. + /** + * The maximum number of deliveries per second, per subscription. + * * @default - no throttling */ readonly maxReceivesPerSecond?: number; @@ -30,7 +38,9 @@ export interface ThrottlePolicy { * Options for customising aspects of the content sent in AWS SNS HTTP/S requests. */ export interface RequestPolicy { - /** The content type of the notification being sent to HTTP/S endpoints. + /** + * The content type of the notification being sent to HTTP/S endpoints. + * * @default - text/plain; charset=UTF-8 */ readonly headerContentType?: string; @@ -40,36 +50,50 @@ export interface RequestPolicy { * Options for customising the retry policy of the delivery of SNS messages to HTTP/S endpoints. */ export interface HealthyRetryPolicy { - /** The minimum delay for a retry (in seconds). Must be at least 1 and not exceed `maxDelayTarget`. - * @default 20 + /** + * The minimum delay for a retry. Must be at least one second, not exceed `maxDelayTarget`, and correspond to a whole number of seconds. + * + * @default - 20 seconds */ - readonly minDelayTarget: number; - /** The maximum delay for a retry (in seconds). Must be at least `minDelayTarget` and less than 3,600. - * @default 20 + readonly minDelayTarget: Duration; + /** + * The maximum delay for a retry. Must be at least `minDelayTarget` less than 3,600 seconds, and correspond to a whole number of seconds, + * + * @default - 20 seconds */ - readonly maxDelayTarget: number; + readonly maxDelayTarget: Duration; - /** The total number of retries, including immediate, pre-backoff, backoff, and post-backoff retries. Must be greater than or equal to zero and not exceed 100. + /** + * The total number of retries, including immediate, pre-backoff, backoff, and post-backoff retries. Must be greater than or equal to zero and not exceed 100. + * * @default 3 */ readonly numRetries: number; - /** The number of retries to be done immediately, with no delay between them. Must be zero or greater. + /** + * The number of retries to be done immediately, with no delay between them. Must be zero or greater. + * * @default 0 */ readonly numNoDelayRetries?: number; - /** The number of retries in the pre-backoff phase, with the specified minimum delay between them. Must be zero or greater + /** + * The number of retries in the pre-backoff phase, with the specified minimum delay between them. Must be zero or greater + * * @default 0 */ readonly numMinDelayRetries?: number; - /** The number of retries in the post-backoff phase, with the maximum delay between them. Must be zero or greater + /** + * The number of retries in the post-backoff phase, with the maximum delay between them. Must be zero or greater + * * @default 0 */ readonly numMaxDelayRetries?: number; - /** The model for backoff between retries. + /** + * The model for backoff between retries. + * * @default - linear */ readonly backoffFunction?: BackoffFunction; @@ -82,18 +106,21 @@ export interface DeliveryPolicy { /** * The retry policy of the delivery of SNS messages to HTTP/S endpoints. + * * @default - Amazon SNS attempts up to three retries with a delay between failed attempts set at 20 seconds */ readonly healthyRetryPolicy?: HealthyRetryPolicy; /** * The throttling policy of the delivery of SNS messages to HTTP/S endpoints. + * * @default - No throttling */ readonly throttlePolicy?: ThrottlePolicy; /** * The request of the content sent in AWS SNS HTTP/S requests. + * * @default - The content type is set to 'text/plain; charset=UTF-8' */ readonly requestPolicy?: RequestPolicy; diff --git a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts index a273f884633b1..6d47881dc5fb5 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts @@ -144,43 +144,6 @@ export class Subscription extends Resource { throw new Error('Subscription role arn is required field for subscriptions with a firehose protocol.'); } - if (props.deliveryPolicy) { - if (props.protocol !== SubscriptionProtocol.HTTP && props.protocol !== SubscriptionProtocol.HTTPS) { - throw new Error('Delivery policy is only supported for HTTP and HTTPS subscriptions'); - } - const { healthyRetryPolicy, throttlePolicy } = props.deliveryPolicy; - if (healthyRetryPolicy) { - const delayTargetLimit = 3600; - if (healthyRetryPolicy.minDelayTarget < 1 || healthyRetryPolicy.minDelayTarget > delayTargetLimit) { - throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimit} inclusive`); - } - if (healthyRetryPolicy.maxDelayTarget < 1 || healthyRetryPolicy.maxDelayTarget > delayTargetLimit) { - throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimit} inclusive`); - } - if (healthyRetryPolicy.minDelayTarget > healthyRetryPolicy.maxDelayTarget) { - throw new Error('minDelayTarget must not exceed maxDelayTarget'); - } - const numRetriesLimit = 100; - if (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit) { - throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive`); - } - if (healthyRetryPolicy.numNoDelayRetries && healthyRetryPolicy.numNoDelayRetries < 0) { - throw new Error('numNoDelayRetries must be zero or greater'); - } - if (healthyRetryPolicy.numMinDelayRetries && healthyRetryPolicy.numMinDelayRetries < 0) { - throw new Error('numMinDelayRetries must be zero or greater'); - } - if (healthyRetryPolicy.numMaxDelayRetries && healthyRetryPolicy.numMaxDelayRetries < 0) { - throw new Error('numMaxDelayRetries must be zero or greater'); - } - } - if (throttlePolicy) { - if (throttlePolicy.maxReceivesPerSecond !== undefined && throttlePolicy.maxReceivesPerSecond < 1) { - throw new Error('maxReceivesPerSecond must be greater than zero'); - } - } - } - // Format filter policy const filterPolicy = this.filterPolicyWithMessageBody ? buildFilterPolicyWithMessageBody(this.filterPolicyWithMessageBody) @@ -197,16 +160,60 @@ export class Subscription extends Resource { region: props.region, redrivePolicy: this.buildDeadLetterConfig(this.deadLetterQueue), subscriptionRoleArn: props.subscriptionRoleArn, - deliveryPolicy: props.deliveryPolicy ? this.renderDeliveryPolicy(props.deliveryPolicy): undefined, + deliveryPolicy: props.deliveryPolicy ? this.renderDeliveryPolicy(props.deliveryPolicy, props.protocol): undefined, }); } - private renderDeliveryPolicy(deliveryPolicy: DeliveryPolicy): any { + private renderDeliveryPolicy(deliveryPolicy: DeliveryPolicy, protocol: SubscriptionProtocol): any { + if (protocol !== SubscriptionProtocol.HTTP && protocol !== SubscriptionProtocol.HTTPS) { + throw new Error(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`); + } + const { healthyRetryPolicy, throttlePolicy } = deliveryPolicy; + if (healthyRetryPolicy) { + const delayTargetLimitSecs = 3600; + const minDelayTarget = healthyRetryPolicy.minDelayTarget; + const maxDelayTarget = healthyRetryPolicy.maxDelayTarget; + if (minDelayTarget.toMilliseconds() % 1000 !== 0) { + throw new Error(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`); + } + if (maxDelayTarget.toMilliseconds() % 1000 !== 0) { + throw new Error(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`); + } + const minDelayTargetSecs = minDelayTarget.toSeconds(); + if (minDelayTargetSecs < 1 || minDelayTargetSecs > delayTargetLimitSecs) { + throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive`); + } + const maxDelayTargetSecs = maxDelayTarget.toSeconds(); + if (maxDelayTargetSecs < 1 || maxDelayTargetSecs > delayTargetLimitSecs) { + throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive`); + } + if (minDelayTargetSecs > maxDelayTargetSecs) { + throw new Error('minDelayTarget must not exceed maxDelayTarget'); + } + const numRetriesLimit = 100; + if (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit) { + throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive`); + } + if (healthyRetryPolicy.numNoDelayRetries && healthyRetryPolicy.numNoDelayRetries < 0) { + throw new Error('numNoDelayRetries must be zero or greater'); + } + if (healthyRetryPolicy.numMinDelayRetries && healthyRetryPolicy.numMinDelayRetries < 0) { + throw new Error('numMinDelayRetries must be zero or greater'); + } + if (healthyRetryPolicy.numMaxDelayRetries && healthyRetryPolicy.numMaxDelayRetries < 0) { + throw new Error('numMaxDelayRetries must be zero or greater'); + } + } + if (throttlePolicy) { + if (throttlePolicy.maxReceivesPerSecond !== undefined && throttlePolicy.maxReceivesPerSecond < 1) { + throw new Error('maxReceivesPerSecond must be greater than zero'); + } + } return { healthyRetryPolicy: deliveryPolicy.healthyRetryPolicy ? { - minDelayTarget: deliveryPolicy.healthyRetryPolicy.minDelayTarget, - maxDelayTarget: deliveryPolicy.healthyRetryPolicy.maxDelayTarget, + minDelayTarget: deliveryPolicy.healthyRetryPolicy.minDelayTarget.toSeconds(), + maxDelayTarget: deliveryPolicy.healthyRetryPolicy.maxDelayTarget.toSeconds(), numRetries: deliveryPolicy.healthyRetryPolicy.numRetries, numNoDelayRetries: deliveryPolicy.healthyRetryPolicy.numNoDelayRetries, numMinDelayRetries: deliveryPolicy.healthyRetryPolicy.numMinDelayRetries, diff --git a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts index 762fc43b53bef..2f73d73c64065 100644 --- a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts @@ -274,8 +274,8 @@ describe('Subscription', () => { endpoint: 'endpoint', deliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 5, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(5), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, backoffFunction: sns.BackoffFunction.EXPONENTIAL, }, @@ -421,15 +421,15 @@ describe('Subscription', () => { endpoint: 'endpoint', deliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 11, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(11), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, }, }, protocol: protocol, subscriptionRoleArn: '???', topic, - })).toThrow(/Delivery policy is only supported for HTTP and HTTPS subscriptions/); + })).toThrow(new RegExp(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`)); }); test('throws an error when deliveryPolicy minDelayTarget exceeds maxDelayTarget', () => { @@ -442,8 +442,8 @@ describe('Subscription', () => { endpoint: 'endpoint', deliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 11, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(11), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, }, }, @@ -457,8 +457,8 @@ describe('Subscription', () => { prop: 'minDelayTarget', invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 0, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(0), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, }, }, @@ -467,8 +467,8 @@ describe('Subscription', () => { prop: 'maxDelayTarget', invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 10, - maxDelayTarget: 0, + minDelayTarget: cdk.Duration.seconds(10), + maxDelayTarget: cdk.Duration.seconds(0), numRetries: 6, }, }, @@ -477,8 +477,8 @@ describe('Subscription', () => { prop: 'minDelayTarget', invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 3601, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(3601), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, }, }, @@ -487,8 +487,8 @@ describe('Subscription', () => { prop: 'maxDelayTarget', invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 10, - maxDelayTarget: 3601, + minDelayTarget: cdk.Duration.seconds(10), + maxDelayTarget: cdk.Duration.seconds(3601), numRetries: 6, }, }, @@ -508,7 +508,7 @@ describe('Subscription', () => { deliveryPolicy: invalidDeliveryPolicy, protocol: sns.SubscriptionProtocol.HTTPS, topic, - })).toThrow(new RegExp(`${prop} must be between 1 and 3600 inclusive`)); + })).toThrow(new RegExp(`${prop} must be between 1 and 3600 seconds inclusive`)); }); }); @@ -522,8 +522,8 @@ describe('Subscription', () => { endpoint: 'endpoint', deliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 10, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(10), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: invalidValue, }, }, @@ -536,8 +536,8 @@ describe('Subscription', () => { { invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 1, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(1), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, numNoDelayRetries: -1, }, @@ -547,8 +547,8 @@ describe('Subscription', () => { { invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 1, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(1), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, numMinDelayRetries: -1, }, @@ -558,8 +558,8 @@ describe('Subscription', () => { { invalidDeliveryPolicy: { healthyRetryPolicy: { - minDelayTarget: 1, - maxDelayTarget: 10, + minDelayTarget: cdk.Duration.seconds(1), + maxDelayTarget: cdk.Duration.seconds(10), numRetries: 6, numMaxDelayRetries: -1, }, From 2ea400695b81e6ebc61f7858d34358005ea8406d Mon Sep 17 00:00:00 2001 From: Laurence Warne Date: Sun, 22 Sep 2024 11:09:20 +0100 Subject: [PATCH 3/6] Add more validation --- .../aws-cdk-lib/aws-sns/lib/subscription.ts | 18 ++++---- .../aws-sns/test/subscription.test.ts | 45 +++++++++++++++++-- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts index 6d47881dc5fb5..36961145e58a5 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts @@ -195,19 +195,21 @@ export class Subscription extends Resource { if (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit) { throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive`); } - if (healthyRetryPolicy.numNoDelayRetries && healthyRetryPolicy.numNoDelayRetries < 0) { - throw new Error('numNoDelayRetries must be zero or greater'); + const { numNoDelayRetries, numMinDelayRetries, numMaxDelayRetries } = healthyRetryPolicy; + if (numNoDelayRetries && (numNoDelayRetries < 0 || !Number.isInteger(numNoDelayRetries))) { + throw new Error('numNoDelayRetries must be an integer zero or greater'); } - if (healthyRetryPolicy.numMinDelayRetries && healthyRetryPolicy.numMinDelayRetries < 0) { - throw new Error('numMinDelayRetries must be zero or greater'); + if (numMinDelayRetries && (numMinDelayRetries < 0 || !Number.isInteger(numMinDelayRetries))) { + throw new Error('numMinDelayRetries must be an integer zero or greater'); } - if (healthyRetryPolicy.numMaxDelayRetries && healthyRetryPolicy.numMaxDelayRetries < 0) { - throw new Error('numMaxDelayRetries must be zero or greater'); + if (numMaxDelayRetries && (numMaxDelayRetries < 0 || !Number.isInteger(numMaxDelayRetries))) { + throw new Error('numMaxDelayRetries must be an integer zero or greater'); } } if (throttlePolicy) { - if (throttlePolicy.maxReceivesPerSecond !== undefined && throttlePolicy.maxReceivesPerSecond < 1) { - throw new Error('maxReceivesPerSecond must be greater than zero'); + const maxReceivesPerSecond = throttlePolicy.maxReceivesPerSecond; + if (maxReceivesPerSecond !== undefined && (maxReceivesPerSecond < 1 || !Number.isInteger(maxReceivesPerSecond))) { + throw new Error('maxReceivesPerSecond must be an integer greater than zero'); } } return { diff --git a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts index 2f73d73c64065..32747b162bd73 100644 --- a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts @@ -543,6 +543,7 @@ describe('Subscription', () => { }, }, prop: 'numNoDelayRetries', + value: -1, }, { invalidDeliveryPolicy: { @@ -554,6 +555,7 @@ describe('Subscription', () => { }, }, prop: 'numMinDelayRetries', + value: -1, }, { invalidDeliveryPolicy: { @@ -565,8 +567,45 @@ describe('Subscription', () => { }, }, prop: 'numMaxDelayRetries', + value: -1, }, - ])('throws an error when $prop < 0', ({ invalidDeliveryPolicy, prop }) => { + { + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: cdk.Duration.seconds(1), + maxDelayTarget: cdk.Duration.seconds(10), + numRetries: 6, + numNoDelayRetries: 1.5, + }, + }, + prop: 'numNoDelayRetries', + value: 1.5, + }, + { + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: cdk.Duration.seconds(1), + maxDelayTarget: cdk.Duration.seconds(10), + numRetries: 6, + numMinDelayRetries: 1.5, + }, + }, + prop: 'numMinDelayRetries', + value: 1.5, + }, + { + invalidDeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: cdk.Duration.seconds(1), + maxDelayTarget: cdk.Duration.seconds(10), + numRetries: 6, + numMaxDelayRetries: 1.5, + }, + }, + prop: 'numMaxDelayRetries', + value: 1.5, + }, + ])('throws an error when $prop = $value', ({ invalidDeliveryPolicy, prop }) => { // GIVEN const stack = new cdk.Stack(); const topic = new sns.Topic(stack, 'Topic'); @@ -577,7 +616,7 @@ describe('Subscription', () => { deliveryPolicy: invalidDeliveryPolicy, protocol: sns.SubscriptionProtocol.HTTPS, topic, - })).toThrow(new RegExp(`${prop} must be zero or greater`)); + })).toThrow(new RegExp(`${prop} must be an integer zero or greater`)); }); test('throws an error when throttlePolicy < 1', () => { @@ -595,6 +634,6 @@ describe('Subscription', () => { }, protocol: sns.SubscriptionProtocol.HTTPS, topic, - })).toThrow(/maxReceivesPerSecond must be greater than zero/); + })).toThrow(/maxReceivesPerSecond must be an integer greater than zero/); }); }); From 9499d82c766dad0eec8cdb77951f77f2088df9a8 Mon Sep 17 00:00:00 2001 From: Laurence Warne Date: Mon, 23 Sep 2024 16:34:22 +0100 Subject: [PATCH 4/6] Address more comments --- .../aws-sns/lib/delivery-policy.ts | 6 +- .../aws-cdk-lib/aws-sns/lib/subscription.ts | 66 +++++++++++-------- .../aws-sns/test/subscription.test.ts | 30 +++++++++ 3 files changed, 70 insertions(+), 32 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts b/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts index 427449015ae45..cecc4b51b194c 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/delivery-policy.ts @@ -55,20 +55,20 @@ export interface HealthyRetryPolicy { * * @default - 20 seconds */ - readonly minDelayTarget: Duration; + readonly minDelayTarget?: Duration; /** * The maximum delay for a retry. Must be at least `minDelayTarget` less than 3,600 seconds, and correspond to a whole number of seconds, * * @default - 20 seconds */ - readonly maxDelayTarget: Duration; + readonly maxDelayTarget?: Duration; /** * The total number of retries, including immediate, pre-backoff, backoff, and post-backoff retries. Must be greater than or equal to zero and not exceed 100. * * @default 3 */ - readonly numRetries: number; + readonly numRetries?: number; /** * The number of retries to be done immediately, with no delay between them. Must be zero or greater. diff --git a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts index 36961145e58a5..aeb51fb1380d3 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts @@ -174,53 +174,61 @@ export class Subscription extends Resource { const delayTargetLimitSecs = 3600; const minDelayTarget = healthyRetryPolicy.minDelayTarget; const maxDelayTarget = healthyRetryPolicy.maxDelayTarget; - if (minDelayTarget.toMilliseconds() % 1000 !== 0) { - throw new Error(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`); + if (minDelayTarget !== undefined) { + if (minDelayTarget.toMilliseconds() % 1000 !== 0) { + throw new Error(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`); + } + const minDelayTargetSecs = minDelayTarget.toSeconds(); + if (minDelayTargetSecs < 1 || minDelayTargetSecs > delayTargetLimitSecs) { + throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${minDelayTargetSecs}s`); + } } - if (maxDelayTarget.toMilliseconds() % 1000 !== 0) { - throw new Error(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`); - } - const minDelayTargetSecs = minDelayTarget.toSeconds(); - if (minDelayTargetSecs < 1 || minDelayTargetSecs > delayTargetLimitSecs) { - throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive`); - } - const maxDelayTargetSecs = maxDelayTarget.toSeconds(); - if (maxDelayTargetSecs < 1 || maxDelayTargetSecs > delayTargetLimitSecs) { - throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive`); - } - if (minDelayTargetSecs > maxDelayTargetSecs) { - throw new Error('minDelayTarget must not exceed maxDelayTarget'); + if (maxDelayTarget !== undefined) { + if (maxDelayTarget.toMilliseconds() % 1000 !== 0) { + throw new Error(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`); + } + const maxDelayTargetSecs = maxDelayTarget.toSeconds(); + if (maxDelayTargetSecs < 1 || maxDelayTargetSecs > delayTargetLimitSecs) { + throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${maxDelayTargetSecs}s`); + } + if ((minDelayTarget !== undefined) && minDelayTarget.toSeconds() > maxDelayTargetSecs) { + throw new Error('minDelayTarget must not exceed maxDelayTarget'); + } } + const numRetriesLimit = 100; - if (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit) { - throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive`); + if (healthyRetryPolicy.numRetries && (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit)) { + throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive, got: ${healthyRetryPolicy.numRetries}`); } const { numNoDelayRetries, numMinDelayRetries, numMaxDelayRetries } = healthyRetryPolicy; if (numNoDelayRetries && (numNoDelayRetries < 0 || !Number.isInteger(numNoDelayRetries))) { - throw new Error('numNoDelayRetries must be an integer zero or greater'); + throw new Error(`numNoDelayRetries must be an integer zero or greater, got: ${numNoDelayRetries}`); } if (numMinDelayRetries && (numMinDelayRetries < 0 || !Number.isInteger(numMinDelayRetries))) { - throw new Error('numMinDelayRetries must be an integer zero or greater'); + throw new Error(`numMinDelayRetries must be an integer zero or greater, got: ${numMinDelayRetries}`); } if (numMaxDelayRetries && (numMaxDelayRetries < 0 || !Number.isInteger(numMaxDelayRetries))) { - throw new Error('numMaxDelayRetries must be an integer zero or greater'); + throw new Error(`numMaxDelayRetries must be an integer zero or greater, got: ${numMaxDelayRetries}`); } } if (throttlePolicy) { const maxReceivesPerSecond = throttlePolicy.maxReceivesPerSecond; if (maxReceivesPerSecond !== undefined && (maxReceivesPerSecond < 1 || !Number.isInteger(maxReceivesPerSecond))) { - throw new Error('maxReceivesPerSecond must be an integer greater than zero'); + throw new Error(`maxReceivesPerSecond must be an integer greater than zero, got: ${maxReceivesPerSecond}`); } } return { - healthyRetryPolicy: deliveryPolicy.healthyRetryPolicy ? { - minDelayTarget: deliveryPolicy.healthyRetryPolicy.minDelayTarget.toSeconds(), - maxDelayTarget: deliveryPolicy.healthyRetryPolicy.maxDelayTarget.toSeconds(), - numRetries: deliveryPolicy.healthyRetryPolicy.numRetries, - numNoDelayRetries: deliveryPolicy.healthyRetryPolicy.numNoDelayRetries, - numMinDelayRetries: deliveryPolicy.healthyRetryPolicy.numMinDelayRetries, - numMaxDelayRetries: deliveryPolicy.healthyRetryPolicy.numMaxDelayRetries, - backoffFunction: deliveryPolicy.healthyRetryPolicy.backoffFunction, + healthyRetryPolicy: healthyRetryPolicy ? { + // minDelayTarget, maxDelayTarget and numRetries are (empirically) mandatory when healthyRetryPolicy is specified, + // but for user-friendliness we allow them to be undefined and set them here instead. + // The defaults we use here are the same used in the event healthyRetryPolicy is not specified, see https://docs.aws.amazon.com/sns/latest/dg/creating-delivery-policy.html. + minDelayTarget: (healthyRetryPolicy.minDelayTarget === undefined)? 20 : healthyRetryPolicy.minDelayTarget.toSeconds(), + maxDelayTarget: (healthyRetryPolicy.maxDelayTarget === undefined)? 20 : healthyRetryPolicy.maxDelayTarget.toSeconds(), + numRetries: (healthyRetryPolicy.numRetries === undefined)? 3: healthyRetryPolicy.numRetries, + numNoDelayRetries: healthyRetryPolicy.numNoDelayRetries, + numMinDelayRetries: healthyRetryPolicy.numMinDelayRetries, + numMaxDelayRetries: healthyRetryPolicy.numMaxDelayRetries, + backoffFunction: healthyRetryPolicy.backoffFunction, }: undefined, throttlePolicy: deliveryPolicy.throttlePolicy ? { maxReceivesPerSecond: deliveryPolicy.throttlePolicy.maxReceivesPerSecond, diff --git a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts index 32747b162bd73..78facdd63dc11 100644 --- a/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts +++ b/packages/aws-cdk-lib/aws-sns/test/subscription.test.ts @@ -309,6 +309,36 @@ describe('Subscription', () => { }); }); + test('sets correct healthyRetryPolicy defaults for attributes required by Cloudformation', () => { + // GIVEN + const stack = new cdk.Stack(); + const topic = new sns.Topic(stack, 'Topic'); + + // WHEN + new sns.Subscription(stack, 'Subscription', { + endpoint: 'endpoint', + deliveryPolicy: { + healthyRetryPolicy: { + backoffFunction: sns.BackoffFunction.EXPONENTIAL, + }, + }, + protocol: sns.SubscriptionProtocol.HTTPS, + topic, + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::SNS::Subscription', { + DeliveryPolicy: { + healthyRetryPolicy: { + minDelayTarget: 20, + maxDelayTarget: 20, + numRetries: 3, + backoffFunction: sns.BackoffFunction.EXPONENTIAL, + }, + }, + }); + }); + test.each( [ SubscriptionProtocol.LAMBDA, From 50b161499ee06624c7eab4c82aed5bae56cc72f2 Mon Sep 17 00:00:00 2001 From: Laurence Warne Date: Wed, 25 Sep 2024 13:36:09 +0100 Subject: [PATCH 5/6] Tweak ternary operator spacing --- packages/aws-cdk-lib/aws-sns/lib/subscription.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts index aeb51fb1380d3..68e3f2fc542b4 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts @@ -222,9 +222,9 @@ export class Subscription extends Resource { // minDelayTarget, maxDelayTarget and numRetries are (empirically) mandatory when healthyRetryPolicy is specified, // but for user-friendliness we allow them to be undefined and set them here instead. // The defaults we use here are the same used in the event healthyRetryPolicy is not specified, see https://docs.aws.amazon.com/sns/latest/dg/creating-delivery-policy.html. - minDelayTarget: (healthyRetryPolicy.minDelayTarget === undefined)? 20 : healthyRetryPolicy.minDelayTarget.toSeconds(), - maxDelayTarget: (healthyRetryPolicy.maxDelayTarget === undefined)? 20 : healthyRetryPolicy.maxDelayTarget.toSeconds(), - numRetries: (healthyRetryPolicy.numRetries === undefined)? 3: healthyRetryPolicy.numRetries, + minDelayTarget: (healthyRetryPolicy.minDelayTarget === undefined) ? 20 : healthyRetryPolicy.minDelayTarget.toSeconds(), + maxDelayTarget: (healthyRetryPolicy.maxDelayTarget === undefined) ? 20 : healthyRetryPolicy.maxDelayTarget.toSeconds(), + numRetries: (healthyRetryPolicy.numRetries === undefined) ? 3: healthyRetryPolicy.numRetries, numNoDelayRetries: healthyRetryPolicy.numNoDelayRetries, numMinDelayRetries: healthyRetryPolicy.numMinDelayRetries, numMaxDelayRetries: healthyRetryPolicy.numMaxDelayRetries, From 442d8c9401dc7d4f2f20b0fd55ec6cf9bd20abf3 Mon Sep 17 00:00:00 2001 From: Laurence Warne Date: Mon, 14 Oct 2024 19:43:29 +0100 Subject: [PATCH 6/6] Tweak subscription protocol conditional --- packages/aws-cdk-lib/aws-sns/lib/subscription.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts index 68e3f2fc542b4..2b6cc1c073bc7 100644 --- a/packages/aws-cdk-lib/aws-sns/lib/subscription.ts +++ b/packages/aws-cdk-lib/aws-sns/lib/subscription.ts @@ -166,7 +166,7 @@ export class Subscription extends Resource { } private renderDeliveryPolicy(deliveryPolicy: DeliveryPolicy, protocol: SubscriptionProtocol): any { - if (protocol !== SubscriptionProtocol.HTTP && protocol !== SubscriptionProtocol.HTTPS) { + if (![SubscriptionProtocol.HTTP, SubscriptionProtocol.HTTPS].includes(protocol)) { throw new Error(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`); } const { healthyRetryPolicy, throttlePolicy } = deliveryPolicy;