From 1b2f72e4f39ceb36e4d8c9fd2a7fc946b72451b1 Mon Sep 17 00:00:00 2001 From: spg Date: Wed, 19 Jun 2019 15:13:01 -0700 Subject: [PATCH 1/7] Check if url is unresolved --- packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts | 14 +++++++------- .../aws-sns-subscriptions/test/subs.test.ts | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts index 5eb0029a93e48..222aa70421851 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts @@ -1,5 +1,5 @@ import sns = require('@aws-cdk/aws-sns'); -import { SubscriptionProps } from './subscription'; +import { Construct, Token } from '@aws-cdk/cdk'; /** * Options for URL subscriptions. @@ -23,17 +23,17 @@ export interface UrlSubscriptionProps extends SubscriptionProps { * @see https://docs.aws.amazon.com/sns/latest/dg/sns-http-https-endpoint-as-subscriber.html */ export class UrlSubscription implements sns.ITopicSubscription { - constructor(private readonly url: string, private readonly props: UrlSubscriptionProps = {}) { - if (!url.startsWith('http://') && !url.startsWith('https://')) { + constructor(private readonly url: string, private readonly protocol: sns.SubscriptionProtocol, private readonly props: UrlSubscriptionProps = {}) { + if (!Token.isUnresolved(url) && !url.startsWith('http://') && !url.startsWith('https://')) { throw new Error('URL must start with either http:// or https://'); } } - public bind(_topic: sns.ITopic): sns.TopicSubscriptionConfig { - return { - subscriberId: this.url, + public bind(scope: Construct, topic: sns.ITopic): void { + new sns.Subscription(scope, topic.node.uniqueId + 'Url', { + topic, endpoint: this.url, - protocol: this.url.startsWith('https:') ? sns.SubscriptionProtocol.HTTPS : sns.SubscriptionProtocol.HTTP, + protocol: this.protocol, rawMessageDelivery: this.props.rawMessageDelivery, filterPolicy: this.props.filterPolicy, }; diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts index 2dd8c907469e7..8e2f152b49f38 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts @@ -19,7 +19,7 @@ beforeEach(() => { }); test('url subscription', () => { - topic.addSubscription(new subs.UrlSubscription('https://foobar.com/')); + topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.Https)); expect(stack).toMatchTemplate({ "Resources": { @@ -45,7 +45,7 @@ test('url subscription', () => { }); test('url subscription (with raw delivery)', () => { - topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', { + topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.Https, { rawMessageDelivery: true })); From e48f892607158109841ab1781854fe73ce4073a2 Mon Sep 17 00:00:00 2001 From: spg Date: Fri, 21 Jun 2019 06:14:16 -0700 Subject: [PATCH 2/7] Change ID generation --- packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts index 222aa70421851..c3692a0a9852a 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts @@ -30,7 +30,7 @@ export class UrlSubscription implements sns.ITopicSubscription { } public bind(scope: Construct, topic: sns.ITopic): void { - new sns.Subscription(scope, topic.node.uniqueId + 'Url', { + new sns.Subscription(scope, Token.isUnresolved(this.url) ? 'UnresolvedUrl' : this.url, { topic, endpoint: this.url, protocol: this.protocol, From f27a446d97c501befb860a096f54a036cf65ca53 Mon Sep 17 00:00:00 2001 From: spg Date: Fri, 21 Jun 2019 08:42:21 -0700 Subject: [PATCH 3/7] Fix build --- packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts | 9 +++++---- .../@aws-cdk/aws-sns-subscriptions/test/subs.test.ts | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts index c3692a0a9852a..ae262154eac80 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts @@ -1,5 +1,6 @@ import sns = require('@aws-cdk/aws-sns'); -import { Construct, Token } from '@aws-cdk/cdk'; +import { Token } from '@aws-cdk/cdk'; +import { SubscriptionProps } from './subscription'; /** * Options for URL subscriptions. @@ -29,9 +30,9 @@ export class UrlSubscription implements sns.ITopicSubscription { } } - public bind(scope: Construct, topic: sns.ITopic): void { - new sns.Subscription(scope, Token.isUnresolved(this.url) ? 'UnresolvedUrl' : this.url, { - topic, + public bind(_topic: sns.ITopic): sns.TopicSubscriptionConfig { + return { + subscriberId: this.url, endpoint: this.url, protocol: this.protocol, rawMessageDelivery: this.props.rawMessageDelivery, diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts index 8e2f152b49f38..3fb629cf7fbd5 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts @@ -19,7 +19,7 @@ beforeEach(() => { }); test('url subscription', () => { - topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.Https)); + topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.HTTPS)); expect(stack).toMatchTemplate({ "Resources": { @@ -45,7 +45,7 @@ test('url subscription', () => { }); test('url subscription (with raw delivery)', () => { - topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.Https, { + topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.HTTPS, { rawMessageDelivery: true })); From d524d120d32e0061ca05e650701dfa56c05f8541 Mon Sep 17 00:00:00 2001 From: spg Date: Fri, 21 Jun 2019 09:14:48 -0700 Subject: [PATCH 4/7] Build fixes, default protocol, more tests, docs --- .../@aws-cdk/aws-sns-subscriptions/lib/url.ts | 27 ++++++++++-- .../aws-sns-subscriptions/test/subs.test.ts | 43 ++++++++++++++++++- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts index ae262154eac80..219e4d9f74e18 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts @@ -14,6 +14,13 @@ export interface UrlSubscriptionProps extends SubscriptionProps { * @default false */ readonly rawMessageDelivery?: boolean; + + /** + * The subscription's protocol. + * + * @default Protocol is derived from url + */ + readonly protocol?: sns.SubscriptionProtocol; } /** @@ -24,15 +31,29 @@ export interface UrlSubscriptionProps extends SubscriptionProps { * @see https://docs.aws.amazon.com/sns/latest/dg/sns-http-https-endpoint-as-subscriber.html */ export class UrlSubscription implements sns.ITopicSubscription { - constructor(private readonly url: string, private readonly protocol: sns.SubscriptionProtocol, private readonly props: UrlSubscriptionProps = {}) { - if (!Token.isUnresolved(url) && !url.startsWith('http://') && !url.startsWith('https://')) { + private readonly protocol: sns.SubscriptionProtocol; + private readonly unresolvedUrl: boolean; + + constructor(private readonly url: string, private readonly props: UrlSubscriptionProps = {}) { + this.unresolvedUrl = Token.isUnresolved(url); + if (!this.unresolvedUrl && !url.startsWith('http://') && !url.startsWith('https://')) { throw new Error('URL must start with either http:// or https://'); } + + if (this.unresolvedUrl && props.protocol === undefined) { + throw new Error('Must provide protocol if url is unresolved'); + } + + if (this.unresolvedUrl) { + this.protocol = props.protocol!; + } else { + this.protocol = this.url.startsWith('https:') ? sns.SubscriptionProtocol.HTTPS : sns.SubscriptionProtocol.HTTP; + } } public bind(_topic: sns.ITopic): sns.TopicSubscriptionConfig { return { - subscriberId: this.url, + subscriberId: this.unresolvedUrl ? 'UnresolvedUrl' : this.url, endpoint: this.url, protocol: this.protocol, rawMessageDelivery: this.props.rawMessageDelivery, diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts index 3fb629cf7fbd5..35a4c13db1c7f 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts @@ -2,6 +2,7 @@ import '@aws-cdk/assert/jest'; import lambda = require('@aws-cdk/aws-lambda'); import sns = require('@aws-cdk/aws-sns'); import sqs = require('@aws-cdk/aws-sqs'); +import cdk = require('@aws-cdk/cdk'); import { PhysicalName, Stack } from '@aws-cdk/cdk'; import subs = require('../lib'); @@ -19,7 +20,7 @@ beforeEach(() => { }); test('url subscription', () => { - topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.HTTPS)); + topic.addSubscription(new subs.UrlSubscription('https://foobar.com/')); expect(stack).toMatchTemplate({ "Resources": { @@ -45,7 +46,7 @@ test('url subscription', () => { }); test('url subscription (with raw delivery)', () => { - topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', sns.SubscriptionProtocol.HTTPS, { + topic.addSubscription(new subs.UrlSubscription('https://foobar.com/', { rawMessageDelivery: true })); @@ -71,6 +72,44 @@ test('url subscription (with raw delivery)', () => { }); }); +test('url subscription (unresolved url with protocol)', () => { + const secret = cdk.SecretValue.secretsManager('my-secret'); + const url = secret.toString(); + topic.addSubscription(new subs.UrlSubscription(url, {protocol: sns.SubscriptionProtocol.HTTPS})); + + expect(stack).toMatchTemplate({ + "Resources": { + "MyTopic86869434": { + "Type": "AWS::SNS::Topic", + "Properties": { + "DisplayName": "displayName", + "TopicName": "topicName" + } + }, + "MyTopicUnresolvedUrlBA127FB3": { + "Type": "AWS::SNS::Subscription", + "Properties": { + "Endpoint": "{{resolve:secretsmanager:my-secret:SecretString:::}}", + "Protocol": "https", + "TopicArn": { "Ref": "MyTopic86869434" }, + } + } + } + }); +}); + +test('url subscription (unknown protocol)', () => { + expect(() => topic.addSubscription(new subs.UrlSubscription('some-protocol://foobar.com/'))) + .toThrowError(/URL must start with either http:\/\/ or https:\/\//); +}); + +test('url subscription (unresolved url without protocol)', () => { + const secret = cdk.SecretValue.secretsManager('my-secret'); + const url = secret.toString(); + expect(() => topic.addSubscription(new subs.UrlSubscription(url))) + .toThrowError(/Must provide protocol if url is unresolved/); +}); + test('queue subscription', () => { const queue = new sqs.Queue(stack, 'MyQueue'); From 938bee400bbce9bca3b62991a48fbae36e34cab9 Mon Sep 17 00:00:00 2001 From: spg Date: Tue, 25 Jun 2019 13:31:37 -0700 Subject: [PATCH 5/7] Fix for 0.36.0 --- packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts | 2 +- packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts index 219e4d9f74e18..d70f6c17edd5e 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts @@ -1,5 +1,5 @@ import sns = require('@aws-cdk/aws-sns'); -import { Token } from '@aws-cdk/cdk'; +import { Token } from '@aws-cdk/core'; import { SubscriptionProps } from './subscription'; /** diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts index 53694e949125a..cf44331d27364 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts @@ -2,7 +2,7 @@ import '@aws-cdk/assert/jest'; import lambda = require('@aws-cdk/aws-lambda'); import sns = require('@aws-cdk/aws-sns'); import sqs = require('@aws-cdk/aws-sqs'); -import { Stack } from '@aws-cdk/core'; +import { Stack, SecretValue } from '@aws-cdk/core'; import subs = require('../lib'); // tslint:disable:object-literal-key-quotes @@ -72,7 +72,7 @@ test('url subscription (with raw delivery)', () => { }); test('url subscription (unresolved url with protocol)', () => { - const secret = cdk.SecretValue.secretsManager('my-secret'); + const secret = SecretValue.secretsManager('my-secret'); const url = secret.toString(); topic.addSubscription(new subs.UrlSubscription(url, {protocol: sns.SubscriptionProtocol.HTTPS})); @@ -103,7 +103,7 @@ test('url subscription (unknown protocol)', () => { }); test('url subscription (unresolved url without protocol)', () => { - const secret = cdk.SecretValue.secretsManager('my-secret'); + const secret = SecretValue.secretsManager('my-secret'); const url = secret.toString(); expect(() => topic.addSubscription(new subs.UrlSubscription(url))) .toThrowError(/Must provide protocol if url is unresolved/); From 20459e71ccc15b186d41e504f247e3e3502435fe Mon Sep 17 00:00:00 2001 From: spg Date: Wed, 26 Jun 2019 15:17:27 -0700 Subject: [PATCH 6/7] Reorder imports --- packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts index cf44331d27364..2d1d7216e96b3 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts @@ -2,7 +2,7 @@ import '@aws-cdk/assert/jest'; import lambda = require('@aws-cdk/aws-lambda'); import sns = require('@aws-cdk/aws-sns'); import sqs = require('@aws-cdk/aws-sqs'); -import { Stack, SecretValue } from '@aws-cdk/core'; +import { SecretValue, Stack } from '@aws-cdk/core'; import subs = require('../lib'); // tslint:disable:object-literal-key-quotes From 4904d1f5bd4a7b30009c895e0bcc33776403ca94 Mon Sep 17 00:00:00 2001 From: Elad Ben-Israel Date: Mon, 1 Jul 2019 11:48:46 +0300 Subject: [PATCH 7/7] Update url.ts --- packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts index d70f6c17edd5e..12f13512aa132 100644 --- a/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts +++ b/packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts @@ -18,7 +18,7 @@ export interface UrlSubscriptionProps extends SubscriptionProps { /** * The subscription's protocol. * - * @default Protocol is derived from url + * @default - Protocol is derived from url */ readonly protocol?: sns.SubscriptionProtocol; }