-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Changes from 18 commits
80cbb5b
432eea0
0c23ec0
c293aa5
e7941a0
cde330c
12d2d10
303cb7e
8b8ac53
7ca0270
c2ef535
daa317f
2b0778c
48c0ebc
2d0a396
d543203
343ee82
6449e13
8e38971
adb0f98
1445e2e
5eeefae
c29d7a6
2a38371
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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'; | ||
|
@@ -179,6 +180,42 @@ 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. You must provide `vpc` when using this prop. | ||
* | ||
* @default - the Vpc default strategy if not specified | ||
*/ | ||
readonly vpcSubnets?: ec2.SubnetSelection; | ||
|
||
/** | ||
* Whether or not to allow placing a canary to be run in only private subnets. By default, the Synthetics runtime | ||
* needs access to the S3 and CloudWatch APIs, which will fail in a private subnet without internet access enabled. | ||
* | ||
* You can bypass this check if you've enabled VPC Endpoints for these two APIs in your VPC or have an Internet Gateway setup. | ||
* | ||
* You must provide `vpc` when using this prop. | ||
* | ||
* @default false | ||
*/ | ||
readonly allowPrivateSubnet?: boolean; | ||
|
||
/** | ||
* The list of security groups to associate with the canary's network interfaces. You must provide `vpc` when using this prop. | ||
* | ||
* @default - If the canary is placed within a VPC and a security group is | ||
* not specified a dedicated security group will be created for this canary. | ||
*/ | ||
readonly securityGroups?: ec2.ISecurityGroup[]; | ||
} | ||
|
||
/** | ||
|
@@ -229,7 +266,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), | ||
|
@@ -242,6 +279,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; | ||
|
@@ -289,7 +327,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({ | ||
|
@@ -318,11 +358,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, | ||
}); | ||
} | ||
|
||
|
@@ -352,6 +400,15 @@ 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 | ||
*/ | ||
|
@@ -362,12 +419,40 @@ export class Canary extends cdk.Resource { | |
}; | ||
} | ||
|
||
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) { | ||
if (props.vpcSubnets != null || props.allowPrivateSubnet != null || props.securityGroups != null) { | ||
throw new Error("You must provide the 'vpc' prop when using VPC-related properties."); | ||
} | ||
|
||
return undefined; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
if (subnetIds.length < 1) { | ||
throw new Error('No matching subnets found in the VPC.'); | ||
} | ||
|
||
if (subnetIds.every(id => props.vpc!.isolatedSubnets.some(subnet => subnet.subnetId === id)) && !props.allowPrivateSubnet) { | ||
throw new Error("A canary in a vpc must have access to CloudWatch and S3. Your vpc must either have internet access or private vpc endpoints with 'allowPrivateSubnet=true'.\nSee https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/CloudWatch_Synthetics_Canaries_VPC.html"); | ||
} | ||
|
||
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), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should pull the list of security groups from securityGroupIds: Lazy.listValue({ produce: () => this.connections.securityGroups.map(sg => sg.securityGroupId) }), There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @corymhall Done! Though used |
||
}; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed. I think it should be fine to just add this to the documentation in the readme and the
vpcSubnets
docstring. Users will specify if they want to place it in isolated subnets via thevpcSubnets
prop, asking again here feels like a "are you sure you know what you are doing?" and I don't think that is needed.