Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(synthetics): add vpc configuration #18447

Merged
merged 24 commits into from
Mar 15, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
80cbb5b
feat(aws-synthetics): complete runConfig and add vpcConfig
RichiCoder1 Jan 14, 2022
432eea0
update tests and integ commit
RichiCoder1 Jan 15, 2022
0c23ec0
remove cdk.out template stuff
RichiCoder1 Jan 16, 2022
c293aa5
Merge branch 'aws:master' into aws-synthetic-runconfig-and-vpc
RichiCoder1 Jan 16, 2022
e7941a0
Merge branch 'master' into aws-synthetic-runconfig-and-vpc
RichiCoder1 Feb 25, 2022
cde330c
remove runconfig changes, scope down to vpc
RichiCoder1 Feb 25, 2022
12d2d10
revert runconfig related canary changes
RichiCoder1 Feb 25, 2022
303cb7e
add readme docs for vpc
RichiCoder1 Feb 25, 2022
8b8ac53
fixup tests and docs
RichiCoder1 Feb 25, 2022
7ca0270
include integration fixes
RichiCoder1 Feb 25, 2022
c2ef535
fix docs rosetta compilation
RichiCoder1 Feb 25, 2022
daa317f
Apply suggestions from code review
RichiCoder1 Mar 4, 2022
2b0778c
fix docs?
RichiCoder1 Mar 5, 2022
48c0ebc
merge
RichiCoder1 Mar 5, 2022
2d0a396
address pr feedback around vpc and split integ tests
RichiCoder1 Mar 5, 2022
d543203
simplify subnet verification logic
RichiCoder1 Mar 5, 2022
343ee82
address pr comments and expandd unit tests
RichiCoder1 Mar 11, 2022
6449e13
Merge branch 'master' into aws-synthetic-runconfig-and-vpc
kaizencc Mar 11, 2022
8e38971
remove private subnet warning and add IConnectable
RichiCoder1 Mar 14, 2022
adb0f98
tweak to only init connections if vpc, like lambda
RichiCoder1 Mar 14, 2022
1445e2e
further tweak wording
RichiCoder1 Mar 14, 2022
5eeefae
make sg ids be evaluated at render time
RichiCoder1 Mar 15, 2022
c29d7a6
Merge branch 'master' into aws-synthetic-runconfig-and-vpc
kaizencc Mar 15, 2022
2a38371
Merge branch 'master' into aws-synthetic-runconfig-and-vpc
mergify[bot] Mar 15, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/@aws-cdk/aws-synthetics/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,24 @@ new synthetics.Canary(this, 'Bucket Canary', {
>
> See Synthetics [docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Synthetics_Canaries_WritingCanary.html).

### VPC
RichiCoder1 marked this conversation as resolved.
Show resolved Hide resolved

You can specify what VPC a canary executes in. This can allow for monitoring services that may be internal to a specific VPC. To place a canary within a VPC, you can specify the `vpc` property with the desired `VPC` to place then canary in. This will automatically attach the appropriate IAM permissions to attach to the VPC. This will also create a Security Group and attach to the default subnets for the VPC unless specified via `vpcSubnets` and `securityGroups`.
RichiCoder1 marked this conversation as resolved.
Show resolved Hide resolved

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

declare const vpc: ec2.IVpc;
new synthetics.Canary(this, 'Vpc Canary', {
test: synthetics.Test.custom({
code: synthetics.Code.fromAsset(path.join(__dirname, 'canary')),
handler: 'index.handler',
}),
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_PUPPETEER_3_3,
vpc,
});
```

### Alarms

You can configure a CloudWatch Alarm on a canary metric. Metrics are emitted by CloudWatch automatically and can be accessed by the following APIs:
Expand Down
75 changes: 68 additions & 7 deletions packages/@aws-cdk/aws-synthetics/lib/canary.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as crypto from 'crypto';
import { Metric, MetricOptions, MetricProps } from '@aws-cdk/aws-cloudwatch';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import * as cdk from '@aws-cdk/core';
Expand Down Expand Up @@ -179,6 +180,30 @@ export interface CanaryProps {
* @default - No environment variables.
*/
readonly environmentVariables?: { [key: string]: string };

/**
* The VPC where this canary is run.
*
* Specify this if the canary needs to access resources in a VPC.
*
* @default - Not in VPC
*/
readonly vpc?: ec2.IVpc;

/**
* Where to place the network interfaces within the VPC.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention that you cannot set this property if vpc is not set.

*
* @default - the Vpc default strategy if not specified
*/
readonly vpcSubnets?: ec2.SubnetSelection;

/**
* The list of security groups to associate with the canary's network interfaces.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also can't be set if vpc is not set, right? Needs some documentation.

*
* @default - If the function is placed within a VPC and a security group is
RichiCoder1 marked this conversation as resolved.
Show resolved Hide resolved
* not specified a dedicated security group will be created for this canary.
*/
readonly securityGroups?: ec2.ISecurityGroup[];
}

/**
Expand Down Expand Up @@ -229,7 +254,7 @@ export class Canary extends cdk.Resource {
enforceSSL: true,
});

this.role = props.role ?? this.createDefaultRole(props.artifactsBucketLocation?.prefix);
this.role = props.role ?? this.createDefaultRole(props);

const resource: CfnCanary = new CfnCanary(this, 'Resource', {
artifactS3Location: this.artifactsBucket.s3UrlForObject(props.artifactsBucketLocation?.prefix),
Expand All @@ -242,6 +267,7 @@ export class Canary extends cdk.Resource {
successRetentionPeriod: props.successRetentionPeriod?.toDays(),
code: this.createCode(props),
runConfig: this.createRunConfig(props),
vpcConfig: this.createVpcConfig(props),
});

this.canaryId = resource.attrId;
Expand Down Expand Up @@ -289,7 +315,9 @@ export class Canary extends cdk.Resource {
/**
* Returns a default role for the canary
*/
private createDefaultRole(prefix?: string): iam.IRole {
private createDefaultRole(props: CanaryProps): iam.IRole {
const prefix = props.artifactsBucketLocation?.prefix;

// Created role will need these policies to run the Canary.
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-synthetics-canary.html#cfn-synthetics-canary-executionrolearn
const policy = new iam.PolicyDocument({
Expand Down Expand Up @@ -318,11 +346,19 @@ export class Canary extends cdk.Resource {
],
});

const managedPolicies: iam.IManagedPolicy[] = [];

if (props.vpc) {
// Policy that will have ENI creation permissions
managedPolicies.push(iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaVPCAccessExecutionRole'));
}

return new iam.Role(this, 'ServiceRole', {
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
inlinePolicies: {
canaryPolicy: policy,
},
managedPolicies,
});
}

Expand Down Expand Up @@ -352,22 +388,47 @@ export class Canary extends cdk.Resource {
};
}

private createRunConfig(props: CanaryProps): CfnCanary.RunConfigProperty | undefined {
if (!props.environmentVariables) {
return undefined;
}
return {
environmentVariables: props.environmentVariables,
};
}

/**
* Returns a canary schedule object
*/
* Returns a canary schedule object
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably should go back to the way it was

private createSchedule(props: CanaryProps): CfnCanary.ScheduleProperty {
return {
durationInSeconds: String(`${props.timeToLive?.toSeconds() ?? 0}`),
expression: props.schedule?.expressionString ?? 'rate(5 minutes)',
};
}

private createRunConfig(props: CanaryProps): CfnCanary.RunConfigProperty | undefined {
if (!props.environmentVariables) {
private createVpcConfig(props: CanaryProps): CfnCanary.VPCConfigProperty | undefined {
RichiCoder1 marked this conversation as resolved.
Show resolved Hide resolved
if (!props.vpc) {
return undefined;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fail synth if a user tries to set vpc related options without setting vpc. Right now they look to be just ignored in that case, which might be cause for confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the two places I took for inspiration don't explicitly do this, but I think it's a good idea.

const { subnetIds } = props.vpc.selectSubnets(props.vpcSubnets);
let securityGroups: ec2.ISecurityGroup[];

if (props.securityGroups && props.securityGroups.length > 0) {
securityGroups = props.securityGroups;
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
} else {
const securityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', {
vpc: props.vpc,
description: 'Automatic security group for Canary ' + cdk.Names.uniqueId(this),
});
securityGroups = [securityGroup];
}

return {
environmentVariables: props.environmentVariables,
vpcId: props.vpc.vpcId,
subnetIds,
securityGroupIds: securityGroups.map(sg => sg.securityGroupId),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pull the list of security groups from this.connections since users will be able to modify this connections object.

securityGroupIds: Lazy.listValue({ produce: () => this.connections.securityGroups.map(sg => sg.securityGroupId) }),

Copy link
Contributor Author

@RichiCoder1 RichiCoder1 Mar 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@corymhall Done! Though used Lazy.list as I got a warning that listValue was deprecated and that seemed to be the replacement. Good to know too, I was wondering how this sort of lazy evaluation worked!

};
}

Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-synthetics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
},
"dependencies": {
"@aws-cdk/aws-cloudwatch": "0.0.0",
"@aws-cdk/aws-ec2": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
Expand All @@ -98,6 +99,7 @@
},
"peerDependencies": {
"@aws-cdk/aws-cloudwatch": "0.0.0",
"@aws-cdk/aws-ec2": "0.0.0",
"@aws-cdk/aws-iam": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-s3-assets": "0.0.0",
Expand Down
36 changes: 36 additions & 0 deletions packages/@aws-cdk/aws-synthetics/test/canary.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Match, Template } from '@aws-cdk/assertions';
import * as ec2 from '@aws-cdk/aws-ec2';
import * as iam from '@aws-cdk/aws-iam';
import * as s3 from '@aws-cdk/aws-s3';
import { Duration, Lazy, Stack } from '@aws-cdk/core';
Expand Down Expand Up @@ -441,6 +442,41 @@ test('can specify custom test', () => {
});
});

test('can specify vpc', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add the following unit tests:

  • use vpcSubnets
  • no securityGroups means one will be created for you
  • can specify a securityGroups correctly
  • vpc with no public security groups and no allowPrivateSubnets fails
  • vpc with no public security groups and allowPrivateSubnets does not fail

Please also throw all these unit tests under a

describe('canary in a vpc', () => {

// GIVEN
const stack = new Stack();
const vpc = new ec2.Vpc(stack, 'VPC', { maxAzs: 2 });

// WHEN
new synthetics.Canary(stack, 'Canary', {
test: synthetics.Test.custom({
handler: 'index.handler',
code: synthetics.Code.fromInline(`
exports.handler = async () => {
console.log(\'hello world\');
};`),
}),
runtime: synthetics.Runtime.SYNTHETICS_NODEJS_2_0,
vpc,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::Synthetics::Canary', {
Code: {
Handler: 'index.handler',
Script: `
exports.handler = async () => {
console.log(\'hello world\');
};`,
},
VPCConfig: {
VpcId: {
Ref: Match.anyValue(),
},
},
});
});

test('Role policy generated as expected', () => {
// GIVEN
const stack = new Stack();
Expand Down
Loading