Skip to content

Commit e646ad5

Browse files
tmokmsstmokmss
authored
fix(ec2): Cannot deploy VPC flow log with other resources that requires bucket policies (#23889)
Closes #18985. The problem is described on the issue. In short, when we enable VPC Flow log, it tries to create a bucket policy for the target S3 bucket. That's why a deployment fails if there is a bucket policy defined in a CFn template and the policy is created AFTER a flow log is enabled, which cannot replace the existing policy created by the flow log. To avoid the error, this PR adds explicit dependencies for a VPC flow log resource: * dependency 1: Flow log must be created after a corresponding bucket policy is created by CFn * dependency 2: Flow log must be deleted before a corresponding `autoDeleteObjects` custom resource removed (i.e. deleting all the objects in the bucket). Dependency 2 is actually not related to the original issue, but I'd like to add this because I saw the error relating this on the integration tests. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7945fa6 commit e646ad5

File tree

56 files changed

+2034
-4960
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+2034
-4960
lines changed

Diff for: ‎packages/@aws-cdk/aws-ec2/lib/vpc-flow-logs.ts

+13-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as iam from '@aws-cdk/aws-iam';
22
import * as logs from '@aws-cdk/aws-logs';
33
import * as s3 from '@aws-cdk/aws-s3';
4-
import { IResource, PhysicalName, RemovalPolicy, Resource, FeatureFlags, Stack } from '@aws-cdk/core';
4+
import { IResource, PhysicalName, RemovalPolicy, Resource, FeatureFlags, Stack, CfnResource } from '@aws-cdk/core';
55
import { S3_CREATE_DEFAULT_LOGGING_POLICY } from '@aws-cdk/cx-api';
66
import { Construct } from 'constructs';
77
import { CfnFlowLog } from './ec2.generated';
@@ -252,7 +252,6 @@ class S3Destination extends FlowLogDestination {
252252
encryption: s3.BucketEncryption.UNENCRYPTED,
253253
removalPolicy: RemovalPolicy.RETAIN,
254254
});
255-
256255
} else {
257256
s3Bucket = this.props.s3Bucket;
258257
}
@@ -690,6 +689,18 @@ export class FlowLog extends FlowLogBase {
690689
logDestination,
691690
});
692691

692+
// VPC service implicitly tries to create a bucket policy when adding a vpc flow log.
693+
// To avoid the race condition, we add an explicit dependency here.
694+
if (this.bucket?.policy?.node.defaultChild instanceof CfnResource) {
695+
flowLog.addDependency(this.bucket?.policy.node.defaultChild);
696+
}
697+
698+
// we must remove a flow log configuration first before deleting objects.
699+
const deleteObjects = this.bucket?.node.tryFindChild('AutoDeleteObjectsCustomResource')?.node.defaultChild;
700+
if (deleteObjects instanceof CfnResource) {
701+
flowLog.addDependency(deleteObjects);
702+
}
703+
693704
this.flowLogId = flowLog.ref;
694705
this.node.defaultChild = flowLog;
695706
}

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/FlowLogsDefaultTestDeployAssert6AFD1854.assets.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "21.0.0",
2+
"version": "29.0.0",
33
"files": {
44
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
55
"source": {

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/FlowLogsTestStack.assets.json

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "21.0.0",
2+
"version": "29.0.0",
33
"files": {
44
"33e2651435a0d472a75c1e033c9832b21321d9e56711926b04c5705e5f63874c": {
55
"source": {
@@ -14,15 +14,15 @@
1414
}
1515
}
1616
},
17-
"9dcef326beebc49accefb3f0f234ec72b4de2a2aa5f1dc4ed26408fcc22c1dd7": {
17+
"8161ff5519d2aef653bfa7866cd056f5a3feaccfbd55708667998493d1311898": {
1818
"source": {
1919
"path": "FlowLogsTestStack.template.json",
2020
"packaging": "file"
2121
},
2222
"destinations": {
2323
"current_account-current_region": {
2424
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
25-
"objectKey": "9dcef326beebc49accefb3f0f234ec72b4de2a2aa5f1dc4ed26408fcc22c1dd7.json",
25+
"objectKey": "8161ff5519d2aef653bfa7866cd056f5a3feaccfbd55708667998493d1311898.json",
2626
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
2727
}
2828
}

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/FlowLogsTestStack.template.json

+5-1
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,11 @@
421421
}
422422
],
423423
"TrafficType": "ALL"
424-
}
424+
},
425+
"DependsOn": [
426+
"BucketAutoDeleteObjectsCustomResourceBAFD23C2",
427+
"BucketPolicyE9A3008A"
428+
]
425429
},
426430
"FlowLogsCWIAMRole017AD736": {
427431
"Type": "AWS::IAM::Role",

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/cdk.out

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"version":"21.0.0"}
1+
{"version":"29.0.0"}

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/integ.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "21.0.0",
2+
"version": "29.0.0",
33
"testCases": {
44
"FlowLogs/DefaultTest": {
55
"stacks": [

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/manifest.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "21.0.0",
2+
"version": "29.0.0",
33
"artifacts": {
44
"FlowLogsTestStack.assets": {
55
"type": "cdk:asset-manifest",
@@ -17,7 +17,7 @@
1717
"validateOnSynth": false,
1818
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}",
1919
"cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}",
20-
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/9dcef326beebc49accefb3f0f234ec72b4de2a2aa5f1dc4ed26408fcc22c1dd7.json",
20+
"stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/8161ff5519d2aef653bfa7866cd056f5a3feaccfbd55708667998493d1311898.json",
2121
"requiresBootstrapStackVersion": 6,
2222
"bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version",
2323
"additionalDependencies": [

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-customformat.js.snapshot/tree.json

+12-4
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@
691691
}
692692
},
693693
"constructInfo": {
694-
"fqn": "@aws-cdk/aws-ec2.FlowLog",
694+
"fqn": "@aws-cdk/core.Resource",
695695
"version": "0.0.0"
696696
}
697697
}
@@ -709,6 +709,14 @@
709709
"id": "IAMRole",
710710
"path": "FlowLogsTestStack/FlowLogsCW/IAMRole",
711711
"children": {
712+
"ImportIAMRole": {
713+
"id": "ImportIAMRole",
714+
"path": "FlowLogsTestStack/FlowLogsCW/IAMRole/ImportIAMRole",
715+
"constructInfo": {
716+
"fqn": "@aws-cdk/core.Resource",
717+
"version": "0.0.0"
718+
}
719+
},
712720
"Resource": {
713721
"id": "Resource",
714722
"path": "FlowLogsTestStack/FlowLogsCW/IAMRole/Resource",
@@ -853,7 +861,7 @@
853861
}
854862
},
855863
"constructInfo": {
856-
"fqn": "@aws-cdk/aws-ec2.FlowLog",
864+
"fqn": "@aws-cdk/core.Resource",
857865
"version": "0.0.0"
858866
}
859867
},
@@ -1142,7 +1150,7 @@
11421150
"path": "FlowLogs/DefaultTest/Default",
11431151
"constructInfo": {
11441152
"fqn": "constructs.Construct",
1145-
"version": "10.1.154"
1153+
"version": "10.1.216"
11461154
}
11471155
},
11481156
"DeployAssert": {
@@ -1188,7 +1196,7 @@
11881196
"path": "Tree",
11891197
"constructInfo": {
11901198
"fqn": "constructs.Construct",
1191-
"version": "10.1.154"
1199+
"version": "10.1.216"
11921200
}
11931201
}
11941202
},

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-interval.js.snapshot/FlowLogsDefaultTestDeployAssert6AFD1854.assets.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"version": "21.0.0",
2+
"version": "29.0.0",
33
"files": {
44
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
55
"source": {

Diff for: ‎packages/@aws-cdk/aws-ec2/test/integ.vpc-flow-logs-interval.js.snapshot/FlowLogsFeatureFlag.assets.json

-19
This file was deleted.

0 commit comments

Comments
 (0)