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

(ec2): Cannot deploy VPC flow log with other resources that requires bucket policies #18985

Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud @aws-cdk/aws-s3 Related to Amazon S3 bug This issue is a bug. p2

Comments

@tmokmss
Copy link
Contributor

tmokmss commented Feb 16, 2022

What is the problem?

Hi team, I found a somewhat confusing behavior about S3 bucket and VPC flow log as I wrote below.
I've already found a workaround and it is not critical. Could you take a look when you get a chance?

Reproduction Steps

Define stack by the below code and run npx cdk deploy Stack.

Note that to make sure a bucket policy resource to be created, autoDeleteObjects: true is used here.
But actually any resources that creates bucket policy will also be affected by this behavior, such as ALB access logging.

import * as cdk from 'aws-cdk-lib';
import { RemovalPolicy, Stack } from 'aws-cdk-lib';
import { FlowLogDestination, Vpc } from 'aws-cdk-lib/aws-ec2';
import { Bucket } from 'aws-cdk-lib/aws-s3';

const stack = new Stack(new cdk.App(), "Stack");

const vpc = new Vpc(stack, 'Vpc', {
  natGateways: 1,
});

const logBucket = new Bucket(stack, 'LogBucket', {
  autoDeleteObjects: true,
  removalPolicy: RemovalPolicy.DESTROY,
});

vpc.addFlowLog('FlowLogS3', {
  destination: FlowLogDestination.toS3(logBucket, 'vpcFlowLog'),
});

What did you expect to happen?

deployment successes.

What actually happened?

The below error happens during CFn deployment.

4:45:46 PM | CREATE_FAILED        | AWS::S3::BucketPolicy                 | LogBucketPolicy900DBE48
The bucket policy already exists on bucket stack-logbucketcc3b17e8-wmmxwdjw71d4.

CDK CLI Version

2.62.2

Framework Version

2.62.2

Node.js Version

16.13.1

OS

macOS

Language

Typescript

Language Version

No response

Other information

CFn FlowLog resource seems to implicitly create bucket policy if it doesn't exist on the target S3 bucket.
If there is another bucket policy definition in the CFn template and the vpc flow log resource is processed earlier than it, CFn fails to create the policy due to the error above.

As a workaround, you can explicitly set a dependency to make sure vpc flow log to be created after CFn creates the bucket policy resource.

vpc.node.findChild("FlowLogS3").node.findChild("FlowLog").node.addDependency(logBucket)

If CDK automatically added this dependency, it would help a lot.

@tmokmss tmokmss added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2022
@github-actions github-actions bot added the @aws-cdk/aws-s3 Related to Amazon S3 label Feb 16, 2022
@tmokmss tmokmss changed the title (s3): Cannot deploy VPC flow log and other resources that requires bucket policies (s3): Cannot deploy VPC flow log with other resources that requires bucket policies Feb 16, 2022
@tmokmss tmokmss changed the title (s3): Cannot deploy VPC flow log with other resources that requires bucket policies (ec2): Cannot deploy VPC flow log with other resources that requires bucket policies Feb 16, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Feb 16, 2022
@corymhall
Copy link
Contributor

This also relates to #18816

@corymhall corymhall added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 16, 2022
@otaviomacedo otaviomacedo removed their assignment Mar 4, 2022
@corymhall corymhall removed their assignment Mar 4, 2022
@tmokmss
Copy link
Contributor Author

tmokmss commented Aug 4, 2022

This issue still exists due to lack of the explicit dependency of VPC flow log on bucket's policy

@corymhall corymhall self-assigned this Aug 5, 2022
@khaitranhq
Copy link

Is there any update on this issue? @corymhall

@xhiroga
Copy link

xhiroga commented Dec 26, 2022

This PR #20765 append a new feature flag .

@MrArnoldPalmer
Copy link
Contributor

For those still having this issue make sure the @aws-cdk/aws-s3:createDefaultLoggingPolicy is enabled. New CDK apps will have this enabled by default but if you have an existing app you need to set it explicitly to get the fix.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@tmokmss
Copy link
Contributor Author

tmokmss commented Jan 28, 2023

@MrArnoldPalmer Hi, I confirmed this issues still exists even on CDK 2.62.2. This is because currently VPC service tries to create a bucket policy before CFn creates one when flow log is enabled.

To avoid this behavior, we must make sure that CFn creates a bucket policy before enabling vpc flow log. We need to set the explicit dependency between S3::BucketPolicy and EC2::FlowLog.

edit) I'll submit a PR for this.

@comcalvi
Copy link
Contributor

confirmed @tmokmss is correct, this issue has not been resolved yet

@mergify mergify bot closed this as completed in #23889 Jan 31, 2023
mergify bot pushed a commit that referenced this issue Jan 31, 2023
…es 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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@bestickley
Copy link

This issue is still not resolved for me on CDK v2.144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment