Merge branch 'master' into image-lambda-metadata

mergify[bot] · web-flow · commit 531b7d97b4de · 2021-11-15T17:24:57.000Z
diff --git a/packages/@aws-cdk/aws-ec2/lib/nat.ts b/packages/@aws-cdk/aws-ec2/lib/nat.ts
@@ -233,10 +233,8 @@ class NatGatewayProvider extends NatProvider {
     // Create the NAT gateways
     let i = 0;
     for (const sub of options.natSubnets) {
-      const gateway = sub.addNatGateway();
-      if (this.props.eipAllocationIds) {
-        gateway.allocationId = pickN(i, this.props.eipAllocationIds);
-      }
+      const eipAllocationId = this.props.eipAllocationIds ? pickN(i, this.props.eipAllocationIds) : undefined;
+      const gateway = sub.addNatGateway(eipAllocationId);
       this.gateways.add(sub.availabilityZone, gateway.ref);
       i++;
     }
diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts
@@ -1840,11 +1840,11 @@ export class PublicSubnet extends Subnet implements IPublicSubnet {
    * Also adds the EIP for the managed NAT.
    * @returns A ref to the the NAT Gateway ID
    */
-  public addNatGateway() {
+  public addNatGateway(eipAllocationId?: string) {
     // Create a NAT Gateway in this public subnet
     const ngw = new CfnNatGateway(this, 'NATGateway', {
       subnetId: this.subnetId,
-      allocationId: new CfnEIP(this, 'EIP', {
+      allocationId: eipAllocationId ?? new CfnEIP(this, 'EIP', {
         domain: 'vpc',
       }).attrAllocationId,
     });
diff --git a/packages/@aws-cdk/aws-ec2/test/vpc.test.ts b/packages/@aws-cdk/aws-ec2/test/vpc.test.ts
@@ -585,6 +585,31 @@ describe('vpc', () => {
 
     });
 
+    test('EIP passed with NAT gateway does not create duplicate EIP', () => {
+      const stack = getTestStack();
+      new Vpc(stack, 'VPC', {
+        cidr: '10.0.0.0/16',
+        subnetConfiguration: [
+          {
+            cidrMask: 24,
+            name: 'ingress',
+            subnetType: SubnetType.PUBLIC,
+          },
+          {
+            cidrMask: 24,
+            name: 'application',
+            subnetType: SubnetType.PRIVATE_WITH_NAT,
+          },
+        ],
+        natGatewayProvider: NatProvider.gateway({ eipAllocationIds: ['b'] }),
+        natGateways: 1,
+      });
+      expect(stack).toCountResources('AWS::EC2::EIP', 0);
+      expect(stack).toHaveResource('AWS::EC2::NatGateway', {
+        AllocationId: 'b',
+      });
+    });
+
     test('with mis-matched nat and subnet configs it throws', () => {
       const stack = getTestStack();
       expect(() => new Vpc(stack, 'VPC', {
diff --git a/packages/@aws-cdk/aws-lambda-event-sources/lib/kafka.ts b/packages/@aws-cdk/aws-lambda-event-sources/lib/kafka.ts
@@ -3,7 +3,7 @@ import { ISecurityGroup, IVpc, SubnetSelection } from '@aws-cdk/aws-ec2';
 import * as iam from '@aws-cdk/aws-iam';
 import * as lambda from '@aws-cdk/aws-lambda';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
-import { Stack } from '@aws-cdk/core';
+import { Stack, Names } from '@aws-cdk/core';
 import { StreamEventSource, StreamEventSourceProps } from './stream';
 
 // keep this import separate from other imports to reduce chance for merge conflicts with v2-main
@@ -101,15 +101,16 @@ export interface SelfManagedKafkaEventSourceProps extends KafkaEventSourceProps
 export class ManagedKafkaEventSource extends StreamEventSource {
   // This is to work around JSII inheritance problems
   private innerProps: ManagedKafkaEventSourceProps;
+  private _eventSourceMappingId?: string = undefined;
 
   constructor(props: ManagedKafkaEventSourceProps) {
     super(props);
     this.innerProps = props;
   }
 
   public bind(target: lambda.IFunction) {
-    target.addEventSourceMapping(
-      `KafkaEventSource:${this.innerProps.clusterArn}${this.innerProps.topic}`,
+    const eventSourceMapping = target.addEventSourceMapping(
+      `KafkaEventSource:${Names.nodeUniqueId(target.node)}${this.innerProps.topic}`,
       this.enrichMappingOptions({
         eventSourceArn: this.innerProps.clusterArn,
         startingPosition: this.innerProps.startingPosition,
@@ -118,6 +119,8 @@ export class ManagedKafkaEventSource extends StreamEventSource {
       }),
     );
 
+    this._eventSourceMappingId = eventSourceMapping.eventSourceMappingId;
+
     if (this.innerProps.secret !== undefined) {
       this.innerProps.secret.grantRead(target);
     }
@@ -146,6 +149,16 @@ export class ManagedKafkaEventSource extends StreamEventSource {
       ? undefined
       : sourceAccessConfigurations;
   }
+
+  /**
+  * The identifier for this EventSourceMapping
+  */
+  public get eventSourceMappingId(): string {
+    if (!this._eventSourceMappingId) {
+      throw new Error('KafkaEventSource is not yet bound to an event source mapping');
+    }
+    return this._eventSourceMappingId;
+  }
 }
 
 /**
diff --git a/packages/@aws-cdk/aws-lambda-event-sources/test/kafka.test.ts b/packages/@aws-cdk/aws-lambda-event-sources/test/kafka.test.ts
@@ -488,5 +488,24 @@ describe('KafkaEventSource', () => {
         ]),
       });
     });
+
+    test('ManagedKafkaEventSource name conforms to construct id rules', () => {
+      // GIVEN
+      const stack = new cdk.Stack();
+      const fn = new TestFunction(stack, 'Fn');
+      const clusterArn = 'some-arn';
+      const kafkaTopic = 'some-topic';
+
+      const mskEventMapping = new sources.ManagedKafkaEventSource(
+        {
+          clusterArn,
+          topic: kafkaTopic,
+          startingPosition: lambda.StartingPosition.TRIM_HORIZON,
+        });
+
+      // WHEN
+      fn.addEventSource(mskEventMapping);
+      expect(mskEventMapping.eventSourceMappingId).toBeDefined();
+    });
   });
 });
