chore: jsdoc, improve error messages, add coverage

packages/aws-cdk-lib/aws-sns-subscriptions/lib/subscription.ts

@@ -39,18 +39,21 @@ export abstract class Subscription implements sns.ITopicSubscription {
   }
 
   /**
-   * TODO Doc
+   * Generates the principal to be used for a given subscription and the cross-region relation to the topic.
+   * Depending on the subscriber type and the region setup,
+   * this method will either return the default service principal (`sns.amazonaws.com`),
+   * return a service principal for the subscriber's or topic's opt-in region (`sns.<region>.amazonaws.com`),
+   * or throw an error.
    *
    * @see https://docs.aws.amazon.com/sns/latest/dg/sns-cross-region-delivery.html
    *
    * @param topic The topic to subscribe to
    * @throws Error if the queue and target are in different regions and both of those regions are opt-in
    * @throws Error if the queue and target are in different regions and if the subscriber is a Lambda function in an opt-in region
-   * @returns the grant principal
+   * @returns the generated grant principal for the topic
    */
   public generateGrantPrincipal(topic: sns.ITopic): iam.IPrincipal {
     const [subscriberRegion, topicRegion] = [this.subscriber, topic].map(({ stack }) => stack.region);
-    // TODO check resolved?
     if (subscriberRegion === topicRegion) {
       return new iam.ServicePrincipal('sns.amazonaws.com');
     };
@@ -61,18 +64,18 @@ export abstract class Subscription implements sns.ITopicSubscription {
     ));
 
     if (isSubscriberRegionOptIn === 'YES' && isTopicRegionOptIn === 'YES') {
-      throw new Error('Opt-in to opt-in cross region delivery is not supported');
+      throw new Error('Cross region delivery is not supported if both regions are opt-in');
     }
 
     if (isSubscriberRegionOptIn === 'YES') {
       if (lambda.Function.isFunction(this.subscriber)) {
-        throw new Error('Cross region delivery is not supported for Lambda functions');
+        throw new Error('Cross region delivery is not supported for Lambda functions belonging to an opt-in region');
       } else if (sqs.Queue.isQueue(this.subscriber)) {
         return new iam.ServicePrincipal(`sns.${subscriberRegion}.amazonaws.com`);
       } else {
         // All SQS queues and Lambda functions should be caught in the above checks
         // This is future proofing in case we implement other Subscription targets
-        throw new Error('Unknown subscriber type');
+        throw new Error(`Unknown subscriber type for resource "${this.subscriber.node.id}"`);
       }
     }
 

packages/aws-cdk-lib/aws-sns-subscriptions/test/subs.test.ts

@@ -3,7 +3,7 @@ import * as kms from '../../aws-kms';
 import * as lambda from '../../aws-lambda';
 import * as sns from '../../aws-sns';
 import * as sqs from '../../aws-sqs';
-import { App, CfnParameter, Duration, RemovalPolicy, Stack, Token } from '../../core';
+import { App, CfnParameter, Duration, RemovalPolicy, Resource, Stack, Token } from '../../core';
 import * as cxapi from '../../cx-api';
 import * as subs from '../lib';
 
@@ -852,7 +852,7 @@ describe('queue subscription, cross opt-in region', () => {
     const queue = new sqs.Queue(queueStack, 'MyQueue');
 
     expect(() => topic1.addSubscription(new subs.SqsSubscription(queue)))
-      .toThrow(/Opt-in to opt-in cross region delivery is not supported/);
+      .toThrow(/Cross region delivery is not supported if both regions are opt-in/);
   });
 });
 
@@ -1740,7 +1740,7 @@ describe('lambda subscription, cross opt-in region', () => {
     });
 
     expect(() => topic1.addSubscription(new subs.LambdaSubscription(func)))
-      .toThrow(/Cross region delivery is not supported for Lambda functions/);
+      .toThrow(/Cross region delivery is not supported for Lambda functions belonging to an opt-in region/);
   });
 
   test('throws if cross region and both regions are opt-in', () => {
@@ -1763,7 +1763,7 @@ describe('lambda subscription, cross opt-in region', () => {
     });
 
     expect(() => topic1.addSubscription(new subs.LambdaSubscription(func)))
-      .toThrow(/Opt-in to opt-in cross region delivery is not supported/);
+      .toThrow(/Cross region delivery is not supported if both regions are opt-in/);
   });
 });
 
@@ -2330,3 +2330,23 @@ test('sms subscription with unresolved', () => {
     },
   });
 });
+
+test('generateGrantPrincipal throws if subsriber is not recognized as a supported type', () => {
+  class DummyQueue extends Resource {
+  }
+
+  const app = new App();
+  const topicStack = new Stack(app, 'TopicStack');
+  const queueStack = new Stack(app, 'QueueStack', {
+    env: { region: optInRegion2 },
+  });
+
+  const topic1 = new sns.Topic(topicStack, 'Topic', {
+    topicName: 'topicName',
+    displayName: 'displayName',
+  });
+  const queue = new DummyQueue(queueStack, 'MyDummyQueue');
+
+  expect(() => topic1.addSubscription(new subs.SqsSubscription(queue as sqs.Queue)))
+    .toThrow(/Unknown subscriber type for resource "MyDummyQueue"/);
+});