Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sns): allow tokens to be used in UrlSubscription #2938

Merged
merged 11 commits into from
Jul 4, 2019
Prev Previous commit
Next Next commit
Build fixes, default protocol, more tests, docs
spg committed Jun 21, 2019
commit d524d120d32e0061ca05e650701dfa56c05f8541
27 changes: 24 additions & 3 deletions packages/@aws-cdk/aws-sns-subscriptions/lib/url.ts
Original file line number Diff line number Diff line change
@@ -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!;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The “!” shouldn’t be required here. Try to use !props.protocol above

} 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,
43 changes: 41 additions & 2 deletions packages/@aws-cdk/aws-sns-subscriptions/test/subs.test.ts
Original file line number Diff line number Diff line change
@@ -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');