Merge branch 'master' into huijbers/always-bootstrap

.github/workflows/issue-label-assign.yml

@@ -126,6 +126,7 @@ jobs:
            {"area":"@aws-cdk/aws-imagebuilder","keywords":["aws-imagebuilder","imagebuilder"],"labels":["@aws-cdk/aws-imagebuilder"],"assignees":["skinny85"]},
            {"area":"@aws-cdk/aws-inspector","keywords":["aws-inspector","inspector"],"labels":["@aws-cdk/aws-inspector"],"assignees":["skinny85"]},
            {"area":"@aws-cdk/aws-iot","keywords":["internet-of-things","aws-iot","iot"],"labels":["@aws-cdk/aws-iot"],"assignees":["skinny85"]},
+           {"area":"@aws-cdk/aws-iot-actions","keywords":["aws-iot-actions","iot-actions",],"labels":["@aws-cdk/aws-iot-actions"],"assignees":["skinny85"]},
            {"area":"@aws-cdk/aws-iot1click","keywords":["aws-iot1click","iot1click"],"labels":["@aws-cdk/aws-iot1click"],"assignees":["skinny85"]},
            {"area":"@aws-cdk/aws-iotanalytics","keywords":["aws-iotanalytics","iotanalytics"],"labels":["@aws-cdk/aws-iotanalytics"],"assignees":["skinny85"]},
            {"area":"@aws-cdk/aws-iotevents","keywords":["aws-iotevents","iotevents"],"labels":["@aws-cdk/aws-iotevents"],"assignees":["skinny85"]},

packages/@aws-cdk/aws-ec2/lib/security-group.ts

@@ -68,6 +68,8 @@ abstract class SecurityGroupBase extends Resource implements ISecurityGroup {
   public readonly connections: Connections = new Connections({ securityGroups: [this] });
   public readonly defaultPort?: Port;
 
+  private peerAsTokenCount: number = 0;
+
   constructor(scope: Construct, id: string, props?: ResourceProps) {
     super(scope, id, props);
 
@@ -83,7 +85,7 @@ abstract class SecurityGroupBase extends Resource implements ISecurityGroup {
       description = `from ${peer.uniqueId}:${connection}`;
     }
 
-    const [scope, id] = determineRuleScope(this, peer, connection, 'from', remoteRule);
+    const [scope, id] = this.determineRuleScope(peer, connection, 'from', remoteRule);
 
     // Skip duplicates
     if (scope.node.tryFindChild(id) === undefined) {
@@ -101,7 +103,7 @@ abstract class SecurityGroupBase extends Resource implements ISecurityGroup {
       description = `to ${peer.uniqueId}:${connection}`;
     }
 
-    const [scope, id] = determineRuleScope(this, peer, connection, 'to', remoteRule);
+    const [scope, id] = this.determineRuleScope(peer, connection, 'to', remoteRule);
 
     // Skip duplicates
     if (scope.node.tryFindChild(id) === undefined) {
@@ -121,75 +123,82 @@ abstract class SecurityGroupBase extends Resource implements ISecurityGroup {
   public toEgressRuleConfig(): any {
     return { destinationSecurityGroupId: this.securityGroupId };
   }
-}
 
-/**
- * Determine where to parent a new ingress/egress rule
- *
- * A SecurityGroup rule is parented under the group it's related to, UNLESS
- * we're in a cross-stack scenario with another Security Group. In that case,
- * we respect the 'remoteRule' flag and will parent under the other security
- * group.
- *
- * This is necessary to avoid cyclic dependencies between stacks, since both
- * ingress and egress rules will reference both security groups, and a naive
- * parenting will lead to the following situation:
- *
- *   ╔════════════════════╗         ╔════════════════════╗
- *   ║  ┌───────────┐     ║         ║    ┌───────────┐   ║
- *   ║  │  GroupA   │◀────╬─┐   ┌───╬───▶│  GroupB   │   ║
- *   ║  └───────────┘     ║ │   │   ║    └───────────┘   ║
- *   ║        ▲           ║ │   │   ║          ▲         ║
- *   ║        │           ║ │   │   ║          │         ║
- *   ║        │           ║ │   │   ║          │         ║
- *   ║  ┌───────────┐     ║ └───┼───╬────┌───────────┐   ║
- *   ║  │  EgressA  │─────╬─────┘   ║    │ IngressB  │   ║
- *   ║  └───────────┘     ║         ║    └───────────┘   ║
- *   ║                    ║         ║                    ║
- *   ╚════════════════════╝         ╚════════════════════╝
- *
- * By having the ability to switch the parent, we avoid the cyclic reference by
- * keeping all rules in a single stack.
- *
- * If this happens, we also have to change the construct ID, because
- * otherwise we might have two objects with the same ID if we have
- * multiple reversed security group relationships.
- *
- *   ╔═══════════════════════════════════╗
- *   ║┌───────────┐                      ║
- *   ║│  GroupB   │                      ║
- *   ║└───────────┘                      ║
- *   ║      ▲                            ║
- *   ║      │              ┌───────────┐ ║
- *   ║      ├────"from A"──│ IngressB  │ ║
- *   ║      │              └───────────┘ ║
- *   ║      │              ┌───────────┐ ║
- *   ║      ├─────"to B"───│  EgressA  │ ║
- *   ║      │              └───────────┘ ║
- *   ║      │              ┌───────────┐ ║
- *   ║      └─────"to B"───│  EgressC  │ ║  <-- oops
- *   ║                     └───────────┘ ║
- *   ╚═══════════════════════════════════╝
- */
-function determineRuleScope(
-  group: SecurityGroupBase,
-  peer: IPeer,
-  connection: Port,
-  fromTo: 'from' | 'to',
-  remoteRule?: boolean): [SecurityGroupBase, string] {
-
-  if (remoteRule && SecurityGroupBase.isSecurityGroup(peer) && differentStacks(group, peer)) {
-    // Reversed
-    const reversedFromTo = fromTo === 'from' ? 'to' : 'from';
-    return [peer, `${group.uniqueId}:${connection} ${reversedFromTo}`];
-  } else {
-    // Regular (do old ID escaping to in order to not disturb existing deployments)
-    return [group, `${fromTo} ${renderPeer(peer)}:${connection}`.replace('/', '_')];
+  /**
+   * Determine where to parent a new ingress/egress rule
+   *
+   * A SecurityGroup rule is parented under the group it's related to, UNLESS
+   * we're in a cross-stack scenario with another Security Group. In that case,
+   * we respect the 'remoteRule' flag and will parent under the other security
+   * group.
+   *
+   * This is necessary to avoid cyclic dependencies between stacks, since both
+   * ingress and egress rules will reference both security groups, and a naive
+   * parenting will lead to the following situation:
+   *
+   *   ╔════════════════════╗         ╔════════════════════╗
+   *   ║  ┌───────────┐     ║         ║    ┌───────────┐   ║
+   *   ║  │  GroupA   │◀────╬─┐   ┌───╬───▶│  GroupB   │   ║
+   *   ║  └───────────┘     ║ │   │   ║    └───────────┘   ║
+   *   ║        ▲           ║ │   │   ║          ▲         ║
+   *   ║        │           ║ │   │   ║          │         ║
+   *   ║        │           ║ │   │   ║          │         ║
+   *   ║  ┌───────────┐     ║ └───┼───╬────┌───────────┐   ║
+   *   ║  │  EgressA  │─────╬─────┘   ║    │ IngressB  │   ║
+   *   ║  └───────────┘     ║         ║    └───────────┘   ║
+   *   ║                    ║         ║                    ║
+   *   ╚════════════════════╝         ╚════════════════════╝
+   *
+   * By having the ability to switch the parent, we avoid the cyclic reference by
+   * keeping all rules in a single stack.
+   *
+   * If this happens, we also have to change the construct ID, because
+   * otherwise we might have two objects with the same ID if we have
+   * multiple reversed security group relationships.
+   *
+   *   ╔═══════════════════════════════════╗
+   *   ║┌───────────┐                      ║
+   *   ║│  GroupB   │                      ║
+   *   ║└───────────┘                      ║
+   *   ║      ▲                            ║
+   *   ║      │              ┌───────────┐ ║
+   *   ║      ├────"from A"──│ IngressB  │ ║
+   *   ║      │              └───────────┘ ║
+   *   ║      │              ┌───────────┐ ║
+   *   ║      ├─────"to B"───│  EgressA  │ ║
+   *   ║      │              └───────────┘ ║
+   *   ║      │              ┌───────────┐ ║
+   *   ║      └─────"to B"───│  EgressC  │ ║  <-- oops
+   *   ║                     └───────────┘ ║
+   *   ╚═══════════════════════════════════╝
+   */
+
+  protected determineRuleScope(
+    peer: IPeer,
+    connection: Port,
+    fromTo: 'from' | 'to',
+    remoteRule?: boolean): [SecurityGroupBase, string] {
+
+    if (remoteRule && SecurityGroupBase.isSecurityGroup(peer) && differentStacks(this, peer)) {
+      // Reversed
+      const reversedFromTo = fromTo === 'from' ? 'to' : 'from';
+      return [peer, `${this.uniqueId}:${connection} ${reversedFromTo}`];
+    } else {
+      // Regular (do old ID escaping to in order to not disturb existing deployments)
+      return [this, `${fromTo} ${this.renderPeer(peer)}:${connection}`.replace('/', '_')];
+    }
   }
-}
 
-function renderPeer(peer: IPeer) {
-  return Token.isUnresolved(peer.uniqueId) ? '{IndirectPeer}' : peer.uniqueId;
+  private renderPeer(peer: IPeer) {
+    if (Token.isUnresolved(peer.uniqueId)) {
+      // Need to return a unique value each time a peer
+      // is an unresolved token, else the duplicate skipper
+      // in `sg.addXxxRule` can detect unique rules as duplicates
+      return this.peerAsTokenCount++ ? `'{IndirectPeer${this.peerAsTokenCount}}'` : '{IndirectPeer}';
+    } else {
+      return peer.uniqueId;
+    }
+  }
 }
 
 function differentStacks(group1: SecurityGroupBase, group2: SecurityGroupBase) {
@@ -565,13 +574,12 @@ export class SecurityGroup extends SecurityGroupBase {
    */
   private removeNoTrafficRule() {
     if (this.disableInlineRules) {
-      const [scope, id] = determineRuleScope(
-        this,
+      const [scope, id] = this.determineRuleScope(
         NO_TRAFFIC_PEER,
         NO_TRAFFIC_PORT,
         'to',
-        false);
-
+        false,
+      );
       scope.node.tryRemoveChild(id);
     } else {
       const i = this.directEgressRules.findIndex(r => egressRulesEqual(r, MATCH_NO_TRAFFIC));

packages/@aws-cdk/aws-ec2/test/security-group.test.ts

@@ -208,6 +208,30 @@ describe('security group', () => {
 
   });
 
+  test('can add multiple rules using tokens on same security group', () => {
+    // GIVEN
+    const stack = new Stack(undefined, 'TestStack', { env: { account: '12345678', region: 'dummy' } });
+    const vpc = new Vpc(stack, 'VPC');
+    const sg = new SecurityGroup(stack, 'SG', { vpc });
+
+    const p1 = Lazy.string({ produce: () => 'dummyid1' });
+    const p2 = Lazy.string({ produce: () => 'dummyid2' });
+    const peer1 = Peer.prefixList(p1);
+    const peer2 = Peer.prefixList(p2);
+
+    // WHEN
+    sg.addIngressRule(peer1, Port.tcp(5432), 'Rule 1');
+    sg.addIngressRule(peer2, Port.tcp(5432), 'Rule 2');
+
+    // THEN -- no crash
+    expect(stack).toHaveResourceLike('AWS::EC2::SecurityGroupIngress', {
+      Description: 'Rule 1',
+    });
+    expect(stack).toHaveResourceLike('AWS::EC2::SecurityGroupIngress', {
+      Description: 'Rule 2',
+    });
+  });
+
   test('if tokens are used in ports, `canInlineRule` should be false to avoid cycles', () => {
     // GIVEN
     const p1 = Lazy.number({ produce: () => 80 });

packages/@aws-cdk/aws-iot-actions/README.md

@@ -48,3 +48,21 @@ new iot.TopicRule(this, 'TopicRule', {
   actions: [new actions.LambdaFunctionAction(func)],
 });
 ```
+
+## Put logs to CloudWatch Logs
+
+The code snippet below creates an AWS IoT Rule that put logs to CloudWatch Logs
+when it is triggered.
+
+```ts
+import * as iot from '@aws-cdk/aws-iot';
+import * as actions from '@aws-cdk/aws-iot-actions';
+import * as logs from '@aws-cdk/aws-logs';
+
+const logGroup = new logs.LogGroup(this, 'MyLogGroup');
+
+new iot.TopicRule(this, 'TopicRule', {
+  sql: iot.IotSql.fromStringAsVer20160323("SELECT topic(2) as device_id FROM 'device/+/data'"),
+  actions: [new actions.CloudWatchLogsAction(logGroup)],
+});
+```

packages/@aws-cdk/aws-iot-actions/lib/cloudwatch-logs-action.ts

@@ -0,0 +1,49 @@
+import * as iam from '@aws-cdk/aws-iam';
+import * as iot from '@aws-cdk/aws-iot';
+import * as logs from '@aws-cdk/aws-logs';
+import { singletonActionRole } from './private/role';
+
+/**
+ * Configuration properties of an action for CloudWatch Logs.
+ */
+export interface CloudWatchLogsActionProps {
+  /**
+   * The IAM role that allows access to the CloudWatch log group.
+   *
+   * @default a new role will be created
+   */
+  readonly role?: iam.IRole;
+}
+
+/**
+ * The action to send data to Amazon CloudWatch Logs
+ */
+export class CloudWatchLogsAction implements iot.IAction {
+  private readonly role?: iam.IRole;
+
+  /**
+   * @param logGroup The CloudWatch log group to which the action sends data
+   * @param props Optional properties to not use default
+   */
+  constructor(
+    private readonly logGroup: logs.ILogGroup,
+    props: CloudWatchLogsActionProps = {},
+  ) {
+    this.role = props.role;
+  }
+
+  bind(rule: iot.ITopicRule): iot.ActionConfig {
+    const role = this.role ?? singletonActionRole(rule);
+    this.logGroup.grantWrite(role);
+    this.logGroup.grant(role, 'logs:DescribeLogStreams');
+
+    return {
+      configuration: {
+        cloudwatchLogs: {
+          logGroupName: this.logGroup.logGroupName,
+          roleArn: role.roleArn,
+        },
+      },
+    };
+  }
+}

packages/@aws-cdk/aws-iot-actions/lib/index.ts

@@ -1 +1,2 @@
+export * from './cloudwatch-logs-action';
 export * from './lambda-function-action';

packages/@aws-cdk/aws-iot-actions/lib/private/role.ts

@@ -0,0 +1,27 @@
+import * as iam from '@aws-cdk/aws-iam';
+import { IConstruct, PhysicalName } from '@aws-cdk/core';
+
+// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
+// eslint-disable-next-line no-duplicate-imports, import/order
+import { Construct } from '@aws-cdk/core';
+
+/**
+ * Obtain the Role for the TopicRule
+ *
+ * If a role already exists, it will be returned. This ensures that if a rule have multiple
+ * actions, they will share a role.
+ * @internal
+ */
+export function singletonActionRole(scope: IConstruct): iam.IRole {
+  const id = 'TopicRuleActionRole';
+  const existing = scope.node.tryFindChild(id) as iam.IRole;
+  if (existing) {
+    return existing;
+  };
+
+  const role = new iam.Role(scope as Construct, id, {
+    roleName: PhysicalName.GENERATE_IF_NEEDED,
+    assumedBy: new iam.ServicePrincipal('iot.amazonaws.com'),
+  });
+  return role;
+}

packages/@aws-cdk/aws-iot-actions/package.json

@@ -82,6 +82,7 @@
     "@aws-cdk/aws-iam": "0.0.0",
     "@aws-cdk/aws-iot": "0.0.0",
     "@aws-cdk/aws-lambda": "0.0.0",
+    "@aws-cdk/aws-logs": "0.0.0",
     "@aws-cdk/core": "0.0.0",
     "constructs": "^3.3.69"
   },
@@ -90,6 +91,7 @@
     "@aws-cdk/aws-iam": "0.0.0",
     "@aws-cdk/aws-iot": "0.0.0",
     "@aws-cdk/aws-lambda": "0.0.0",
+    "@aws-cdk/aws-logs": "0.0.0",
     "@aws-cdk/core": "0.0.0",
     "constructs": "^3.3.69"
   },