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

[spg]

Allows to pass in a dynamic reference as the url, like so:

myTopic.addSubscription(
  new UrlSubscription("{{resolve:secretsmanager:some-url}}", sns.SubscriptionProtocol.Https)
);

without having the CDK complain:

Error: URL must start with either http:// or https://
    at new UrlSubscription (/Users/spgingras/myProject/cdk-app/node_modules/@aws-cdk/aws-sns-subscriptions/lib/url.ts:28:13)
...

---

Pull Request Checklist

• Testing
  • Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
  • CLI change?: coordinate update of integration tests with team
  • cdk-init template change?: coordinated update of integration tests with team
• Docs
  • jsdocs: All public APIs documented
  • README: README and/or documentation topic updated
  • Design: For significant features, design document added to design folder
• Title and Description
  • Change type: title prefixed with fix, feat and module name in parens, which will appear in changelog
  • Title: use lower-case and doesn't end with a period
  • Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
  • Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
• Sensitive Modules (requires 2 PR approvers)
  • IAM Policy Document (in @aws-cdk/aws-iam)
  • EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
  • Grant APIs (only if not based on official documentation with a reference)

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

[rix0rrr]

You're right that this needs a fix, but I would like to see the fix done differently.

Can you change it to only check if (!cdk.Token.isUnresolved(url)) { ...do the check ... }.

You will be able to plug in SecretsManager values by going:

const url = secretsmanager.Secret.fromSecretAttributes(stack, 'Secret', { ... }).secretValue;

Or, if you really want to use the direct string construction, Token.asString('{{resolve:...}}').

https://github.com/awslabs/aws-cdk/blob/master/packages/%40aws-cdk/aws-secretsmanager/test/test.secret.ts#L299

[spg]

This avoids an error:

Error: Cannot use tokens in construct ID: ${Token[TOKEN.11]}

[rix0rrr]

Ah, so it does. But now you can only add one URL subscription for any given topic.

Can you use the following expression instead?

Token.isUnresolved(this.url) ? 'UnresolvedUrl' : this.url

So we use the literal one if available, and only fall back to the reuse-breaking identifier if the URL is unresolved?

[rix0rrr]

I'd rather have the protocol optionally inside the props, and derive it from the literal URL (if available).

Only force passing it if the URL is unresolved.

[spg]

@rix0rrr fixed in d524d12

[spg]

@rix0rrr not sure what is happening with CodeBuild

[eladb]

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