fix(s3): move notification destinations into their own module (#2659)

In accordance with new guidelines, we're centralizing cross-service
integrations into their own package. In this case, centralizing S3
Notification Destinations into `@aws-cdk/aws-s3-notifications`.

Fixes #2445.

BREAKING CHANGE: using a Topic, Queue or Lambda as bucket notification
destination now requires an integration object from the
`@aws-cdk/aws-s3-notifications` package.

packages/@aws-cdk/assert/jest.ts

@@ -1,11 +1,15 @@
 import { Stack } from "@aws-cdk/cdk";
 import { SynthesizedStack } from "@aws-cdk/cx-api";
 import { HaveResourceAssertion, ResourcePart } from "./lib/assertions/have-resource";
+import { MatchStyle, matchTemplate } from "./lib/assertions/match-template";
 import { expect as ourExpect } from './lib/expect';
 
 declare global {
   namespace jest {
     interface Matchers<R> {
+      toMatchTemplate(template: any,
+                      matchStyle?: MatchStyle): R;
+
       toHaveResource(resourceType: string,
                      properties?: any,
                      comparison?: ResourcePart): R;
@@ -18,6 +22,27 @@ declare global {
 }
 
 expect.extend({
+  toMatchTemplate(
+    actual: SynthesizedStack | Stack,
+    template: any,
+    matchStyle?: MatchStyle) {
+
+    const assertion = matchTemplate(template, matchStyle);
+    const inspector = ourExpect(actual);
+    const pass = assertion.assertUsing(inspector);
+    if (pass) {
+      return {
+        pass,
+        message: () => `Not ` + assertion.description
+      };
+    } else {
+      return {
+        pass,
+        message: () => assertion.description
+      };
+    }
+  },
+
   toHaveResource(
       actual: SynthesizedStack | Stack,
       resourceType: string,

packages/@aws-cdk/assert/lib/synth-utils.ts

@@ -14,4 +14,24 @@ export class SynthUtils {
     const synth = new Synthesizer();
     return synth.synthesize(stack, options);
   }
+
+  public static subset(stack: Stack, options: SubsetOptions): any {
+    const template = SynthUtils.toCloudFormation(stack);
+    if (template.Resources) {
+      for (const [key, resource] of Object.entries(template.Resources)) {
+        if (options.resourceTypes && !options.resourceTypes.includes((resource as any).Type)) {
+          delete template.Resources[key];
+        }
+      }
+    }
+
+    return template;
+  }
+}
+
+export interface SubsetOptions {
+  /**
+   * Match all resources of the given type
+   */
+  resourceTypes?: string[];
 }

packages/@aws-cdk/aws-iam/lib/policy-document.ts

@@ -343,4 +343,4 @@ export class PolicyStatement extends cdk.Token {
 export enum PolicyStatementEffect {
   Allow = 'Allow',
   Deny = 'Deny',
-}
+}

packages/@aws-cdk/aws-iam/lib/principals.ts

@@ -131,6 +131,13 @@ export interface ServicePrincipalOpts {
    * @default the current Stack's region.
    */
   readonly region?: string;
+
+  /**
+   * Additional conditions to add to the Service Principal
+   *
+   * @default - No conditions
+   */
+  readonly conditions?: { [key: string]: any };
 }
 
 /**
@@ -146,7 +153,7 @@ export class ServicePrincipal extends PrincipalBase {
       Service: [
         new ServicePrincipalToken(this.service, this.opts).toString()
       ]
-    });
+    }, this.opts.conditions);
   }
 
   public toString() {

packages/@aws-cdk/aws-lambda-event-sources/lib/s3.ts

@@ -1,5 +1,6 @@
 import lambda = require('@aws-cdk/aws-lambda');
 import s3 = require('@aws-cdk/aws-s3');
+import notifs = require('@aws-cdk/aws-s3-notifications');
 
 export interface S3EventSourceProps {
   /**
@@ -27,7 +28,7 @@ export class S3EventSource implements lambda.IEventSource {
   public bind(target: lambda.IFunction) {
     const filters = this.props.filters || [];
     for (const event of this.props.events) {
-      this.bucket.addEventNotification(event, target, ...filters);
+      this.bucket.addEventNotification(event, new notifs.LambdaDestination(target), ...filters);
     }
   }
 }

packages/@aws-cdk/aws-lambda-event-sources/package.json

@@ -69,6 +69,7 @@
     "@aws-cdk/aws-kinesis": "^0.32.0",
     "@aws-cdk/aws-lambda": "^0.32.0",
     "@aws-cdk/aws-s3": "^0.32.0",
+    "@aws-cdk/aws-s3-notifications": "^0.32.0",
     "@aws-cdk/aws-sns": "^0.32.0",
     "@aws-cdk/aws-sqs": "^0.32.0",
     "@aws-cdk/cdk": "^0.32.0"
@@ -82,6 +83,7 @@
     "@aws-cdk/aws-kinesis": "^0.32.0",
     "@aws-cdk/aws-lambda": "^0.32.0",
     "@aws-cdk/aws-s3": "^0.32.0",
+    "@aws-cdk/aws-s3-notifications": "^0.32.0",
     "@aws-cdk/aws-sns": "^0.32.0",
     "@aws-cdk/aws-sqs": "^0.32.0",
     "@aws-cdk/cdk": "^0.32.0"

packages/@aws-cdk/aws-lambda/lib/function-base.ts

@@ -2,16 +2,14 @@ import cloudwatch = require('@aws-cdk/aws-cloudwatch');
 import ec2 = require('@aws-cdk/aws-ec2');
 import iam = require('@aws-cdk/aws-iam');
 import logs = require('@aws-cdk/aws-logs');
-import s3n = require('@aws-cdk/aws-s3-notifications');
-import cdk = require('@aws-cdk/cdk');
 import { IResource, Resource } from '@aws-cdk/cdk';
 import { IEventSource } from './event-source';
 import { EventSourceMapping, EventSourceMappingOptions } from './event-source-mapping';
 import { CfnPermission } from './lambda.generated';
 import { Permission } from './permission';
 
 export interface IFunction extends IResource, logs.ILogSubscriptionDestination,
-  s3n.IBucketNotificationDestination, ec2.IConnectable, iam.IGrantable {
+  ec2.IConnectable, iam.IGrantable {
 
   /**
    * Logical ID of this Function.
@@ -270,31 +268,6 @@ export abstract class FunctionBase extends Resource implements IFunction  {
     return { arn: this.functionArn };
   }
 
-  /**
-   * Allows this Lambda to be used as a destination for bucket notifications.
-   * Use `bucket.onEvent(lambda)` to subscribe.
-   */
-  public asBucketNotificationDestination(bucketArn: string, bucketId: string): s3n.BucketNotificationDestinationProps {
-    const permissionId = `AllowBucketNotificationsFrom${bucketId}`;
-    if (!this.node.tryFindChild(permissionId)) {
-      this.addPermission(permissionId, {
-        sourceAccount: this.node.stack.accountId,
-        principal: new iam.ServicePrincipal('s3.amazonaws.com'),
-        sourceArn: bucketArn,
-      });
-    }
-
-    // if we have a permission resource for this relationship, add it as a dependency
-    // to the bucket notifications resource, so it will be created first.
-    const permission = this.node.tryFindChild(permissionId) as cdk.CfnResource;
-
-    return {
-      type: s3n.BucketNotificationDestinationType.Lambda,
-      arn: this.functionArn,
-      dependencies: permission ? [ permission ] : undefined
-    };
-  }
-
   /**
    * Adds an event source to this function.
    *

packages/@aws-cdk/aws-lambda/package.json

@@ -87,7 +87,6 @@
     "@aws-cdk/aws-iam": "^0.32.0",
     "@aws-cdk/aws-logs": "^0.32.0",
     "@aws-cdk/aws-s3": "^0.32.0",
-    "@aws-cdk/aws-s3-notifications": "^0.32.0",
     "@aws-cdk/aws-sqs": "^0.32.0",
     "@aws-cdk/cdk": "^0.32.0",
     "@aws-cdk/cx-api": "^0.32.0"
@@ -101,7 +100,6 @@
     "@aws-cdk/aws-iam": "^0.32.0",
     "@aws-cdk/aws-logs": "^0.32.0",
     "@aws-cdk/aws-s3": "^0.32.0",
-    "@aws-cdk/aws-s3-notifications": "^0.32.0",
     "@aws-cdk/aws-sqs": "^0.32.0",
     "@aws-cdk/cdk": "^0.32.0",
     "@aws-cdk/cx-api": "^0.32.0"

packages/@aws-cdk/aws-s3-notifications/README.md

@@ -1,9 +1,4 @@
-## S3 Bucket Notifications API
+## S3 Bucket Notifications Destinations
 
-This module includes the API that constructs should implement in order to be
-able to be used as destinations for bucket notifications.
-
-To implement the `IBucketNotificationDestination`, a construct should implement
-a method `asBucketNotificationDestination(bucketArn, bucketId)` which registers
-this resource as a destination for bucket notifications _for the specified
-bucket_ and returns the ARN of the destination and it's type.
+This module includes integration classes for using Topics, Queues or Lambdas
+as S3 Notification Destinations.

packages/@aws-cdk/aws-s3-notifications/lib/index.ts

@@ -1 +1,3 @@
-export * from './destination';
+export * from './sqs';
+export * from './sns';
+export * from './lambda';

packages/@aws-cdk/aws-s3-notifications/lib/lambda.ts

@@ -0,0 +1,34 @@
+import iam = require('@aws-cdk/aws-iam');
+import lambda = require('@aws-cdk/aws-lambda');
+import s3 = require('@aws-cdk/aws-s3');
+import { CfnResource, Construct } from '@aws-cdk/cdk';
+
+/**
+ * Use a Lambda function as a bucket notification destination
+ */
+export class LambdaDestination implements s3.IBucketNotificationDestination {
+  constructor(private readonly fn: lambda.IFunction) {
+  }
+
+  public bind(_scope: Construct, bucket: s3.IBucket): s3.BucketNotificationDestinationProps {
+    const permissionId = `AllowBucketNotificationsFrom${bucket.node.uniqueId}`;
+
+    if (this.fn.node.tryFindChild(permissionId) === undefined) {
+      this.fn.addPermission(permissionId, {
+        sourceAccount: bucket.node.stack.accountId,
+        principal: new iam.ServicePrincipal('s3.amazonaws.com'),
+        sourceArn: bucket.bucketArn
+      });
+    }
+
+    // if we have a permission resource for this relationship, add it as a dependency
+    // to the bucket notifications resource, so it will be created first.
+    const permission = this.fn.node.findChild(permissionId) as CfnResource;
+
+    return {
+      type: s3.BucketNotificationDestinationType.Lambda,
+      arn: this.fn.functionArn,
+      dependencies: permission ? [ permission ] : undefined
+    };
+  }
+}

packages/@aws-cdk/aws-s3-notifications/lib/sns.ts

@@ -0,0 +1,26 @@
+import iam = require('@aws-cdk/aws-iam');
+import s3 = require('@aws-cdk/aws-s3');
+import sns = require('@aws-cdk/aws-sns');
+import { Construct } from '@aws-cdk/cdk';
+
+/**
+ * Use an SNS topic as a bucket notification destination
+ */
+export class SnsDestination implements s3.IBucketNotificationDestination {
+  constructor(private readonly topic: sns.ITopic) {
+  }
+
+  public bind(_scope: Construct, bucket: s3.IBucket): s3.BucketNotificationDestinationProps {
+    this.topic.addToResourcePolicy(new iam.PolicyStatement()
+      .addServicePrincipal('s3.amazonaws.com')
+      .addAction('sns:Publish')
+      .addResource(this.topic.topicArn)
+      .addCondition('ArnLike', { "aws:SourceArn": bucket.bucketArn }));
+
+    return {
+      arn: this.topic.topicArn,
+      type: s3.BucketNotificationDestinationType.Topic,
+      dependencies: [ this.topic ] // make sure the topic policy resource is created before the notification config
+    };
+  }
+}

packages/@aws-cdk/aws-s3-notifications/lib/sqs.ts

@@ -0,0 +1,41 @@
+import iam = require('@aws-cdk/aws-iam');
+import s3 = require('@aws-cdk/aws-s3');
+import sqs = require('@aws-cdk/aws-sqs');
+import { Construct } from '@aws-cdk/cdk';
+
+/**
+ * Use an SQS queue as a bucket notification destination
+ */
+export class SqsDestination implements s3.IBucketNotificationDestination {
+  constructor(private readonly queue: sqs.IQueue) {
+  }
+
+  /**
+   * Allows using SQS queues as destinations for bucket notifications.
+   * Use `bucket.onEvent(event, queue)` to subscribe.
+   */
+  public bind(_scope: Construct, bucket: s3.IBucket): s3.BucketNotificationDestinationProps {
+    this.queue.grantSendMessages(new iam.ServicePrincipal('s3.amazonaws.com', {
+      conditions: {
+        ArnLike: { 'aws:SourceArn': bucket.bucketArn }
+      }
+    }));
+
+    // if this queue is encrypted, we need to allow S3 to read messages since that's how
+    // it verifies that the notification destination configuration is valid.
+    if (this.queue.encryptionMasterKey) {
+      this.queue.encryptionMasterKey.addToResourcePolicy(new iam.PolicyStatement()
+        .addServicePrincipal('s3.amazonaws.com')
+        .addAction('kms:GenerateDataKey*')
+        .addAction('kms:Decrypt')
+        .addAllResources(), /* allowNoOp */ false);
+    }
+
+    return {
+      arn: this.queue.queueArn,
+      type: s3.BucketNotificationDestinationType.Queue,
+      dependencies: [ this.queue ]
+    };
+  }
+
+}