Skip to content

Commit

Permalink
Merge branch 'master' into aws-iot-actions-logs
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Nov 2, 2021
2 parents 4cc2ee2 + 1e22189 commit 8265df4
Show file tree
Hide file tree
Showing 15 changed files with 591 additions and 79 deletions.
1 change: 1 addition & 0 deletions .github/workflows/issue-label-assign.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]},
Expand Down
152 changes: 80 additions & 72 deletions packages/@aws-cdk/aws-ec2/lib/security-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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));
Expand Down
24 changes: 24 additions & 0 deletions packages/@aws-cdk/aws-ec2/test/security-group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
39 changes: 37 additions & 2 deletions packages/@aws-cdk/aws-servicecatalog/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ enables organizations to create and manage catalogs of products for their end us
- [Granting access to a portfolio](#granting-access-to-a-portfolio)
- [Sharing a portfolio with another AWS account](#sharing-a-portfolio-with-another-aws-account)
- [Product](#product)
- [Creating a product from a local asset](#creating-a-product-from-local-asset)
- [Creating a product from a stack](#creating-a-product-from-a-stack)
- [Adding a product to a portfolio](#adding-a-product-to-a-portfolio)
- [TagOptions](#tag-options)
- [Constraints](#constraints)
Expand Down Expand Up @@ -125,10 +127,12 @@ const product = new servicecatalog.CloudFormationProduct(this, 'MyFirstProduct',
cloudFormationTemplate: servicecatalog.CloudFormationTemplate.fromUrl(
'https://raw.githubusercontent.com/awslabs/aws-cloudformation-templates/master/aws/services/ServiceCatalog/Product.yaml'),
},
]
],
});
```

### Creating a product from a local asset

A `CloudFormationProduct` can also be created using a Cloudformation template from an Asset.
Assets are files that are uploaded to an S3 Bucket before deployment.
`CloudFormationTemplate.fromAsset` can be utilized to create a Product by passing the path to a local template file on your disk:
Expand All @@ -149,7 +153,38 @@ const product = new servicecatalog.CloudFormationProduct(this, 'MyFirstProduct',
productVersionName: "v2",
cloudFormationTemplate: servicecatalog.CloudFormationTemplate.fromAsset(path.join(__dirname, 'development-environment.template.json')),
},
]
],
});
```

### Creating a product from a stack

You can define a service catalog `CloudFormationProduct` entirely within CDK using a service catalog `ProductStack`.
A separate child stack for your product is created and you can add resources like you would for any other CDK stack,
such as an S3 Bucket, IAM roles, and EC2 instances. This stack is passed in as a product version to your
product. This will not create a separate stack during deployment.

```ts
import * as s3 from '@aws-cdk/aws-s3';
import * as cdk from '@aws-cdk/core';

class S3BucketProduct extends servicecatalog.ProductStack {
constructor(scope: cdk.Construct, id: string) {
super(scope, id);

new s3.Bucket(this, 'BucketProduct');
}
}

const product = new servicecatalog.CloudFormationProduct(this, 'MyFirstProduct', {
productName: "My Product",
owner: "Product Owner",
productVersions: [
{
productVersionName: "v1",
cloudFormationTemplate: servicecatalog.CloudFormationTemplate.fromProductStack(new S3BucketProduct(this, 'S3BucketProduct')),
},
],
});
```

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as s3_assets from '@aws-cdk/aws-s3-assets';
import { hashValues } from './private/util';
import { ProductStack } from './product-stack';

// 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
Expand All @@ -26,6 +27,13 @@ export abstract class CloudFormationTemplate {
return new CloudFormationAssetTemplate(path, options);
}

/**
* Creates a product with the resources defined in the given product stack.
*/
public static fromProductStack(productStack: ProductStack): CloudFormationTemplate {
return new CloudFormationProductStackTemplate(productStack);
}

/**
* Called when the product is initialized to allow this object to bind
* to the stack, add resources and have fun.
Expand Down Expand Up @@ -88,3 +96,21 @@ class CloudFormationAssetTemplate extends CloudFormationTemplate {
};
}
}

/**
* Template from a CDK defined product stack.
*/
class CloudFormationProductStackTemplate extends CloudFormationTemplate {
/**
* @param stack A service catalog product stack.
*/
constructor(public readonly productStack: ProductStack) {
super();
}

public bind(_scope: Construct): CloudFormationTemplateConfig {
return {
httpUrl: this.productStack._getTemplateUrl(),
};
}
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-servicecatalog/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export * from './constraints';
export * from './cloudformation-template';
export * from './portfolio';
export * from './product';
export * from './product-stack';
export * from './tag-options';

// AWS::ServiceCatalog CloudFormation Resources:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import * as cdk from '@aws-cdk/core';

/**
* Deployment environment for an AWS Service Catalog product stack.
*
* Interoperates with the StackSynthesizer of the parent stack.
*/
export class ProductStackSynthesizer extends cdk.StackSynthesizer {
private stack?: cdk.Stack;

public bind(stack: cdk.Stack): void {
if (this.stack !== undefined) {
throw new Error('A Stack Synthesizer can only be bound once, create a new instance to use with a different Stack');
}
this.stack = stack;
}

public addFileAsset(_asset: cdk.FileAssetSource): cdk.FileAssetLocation {
throw new Error('Service Catalog Product Stacks cannot use Assets');
}

public addDockerImageAsset(_asset: cdk.DockerImageAssetSource): cdk.DockerImageAssetLocation {
throw new Error('Service Catalog Product Stacks cannot use Assets');
}

public synthesize(session: cdk.ISynthesisSession): void {
if (!this.stack) {
throw new Error('You must call bindStack() first');
}
// Synthesize the template, but don't emit as a cloud assembly artifact.
// It will be registered as an S3 asset of its parent instead.
this.synthesizeStackTemplate(this.stack, session);
}
}
Loading

0 comments on commit 8265df4

Please sign in to comment.