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

aws-ec2: automatic created policy for bucket already created with FlowLogDestination.toS3 conflicts with that added by CDK via bucket.enforceSSL since #18271 #18676

Closed
psoaresgit opened this issue Jan 27, 2022 · 3 comments · Fixed by #18770 or #18772
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. p0

Comments

@psoaresgit
Copy link

What is the problem?

The AWS::S3::BucketPolicy added by #18271 since v2.9 conflicts with existing AWSLogDeliveryWrite20150319 policies automatically applied if the construct was created and deployed by CDK versions < v2.9.

CloudFormation does not allow to merge or adopt or overwrite existing bucket policies and it errors with 'The bucket policy already exists on bucket ...'.

If the above mentioned equivalent policy had been created by the CDK/CloudFormation then the secure transport policy could be added to it, but given the given the chance that existing deployments do have this policy, and given buckets with that policy automatically applied and dedicated to VPC Flow logs which allow the log delivery service to put logs do not benefit very directly by restricting that delivery to secureTransport is true (it's an AWS service delivering the logs), it might be best to not add this policy and reverse the changes in #18271.

Reproduction Steps

    const flowLog = vpc.addFlowLog('FlowLogS3', {
      destination: ec2.FlowLogDestination.toS3()
    });

What did you expect to happen?

That the bucket policy construct/resource for log delivery had already been created by the CDK/CloudFormation.

What actually happened?

The bucket policy had been automatically created and it prevented the new construct/resource from being created.

CDK CLI Version

v2.9

Framework Version

No response

Node.js Version

v16.13

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response

@psoaresgit psoaresgit added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 27, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Jan 27, 2022
@psoaresgit psoaresgit changed the title aws-ec2: FlowLogDestination.toS3 created by automatically created S3 bucket policies/statements conflict with that by CDK/CloudFormation since v2.9 aws-ec2: FlowLogDestination.toS3 automatically created S3 bucket policies/statements conflict with that by CDK/CloudFormation since v2.9 Jan 27, 2022
@psoaresgit psoaresgit changed the title aws-ec2: FlowLogDestination.toS3 automatically created S3 bucket policies/statements conflict with that by CDK/CloudFormation since v2.9 aws-ec2: automatic created policy for bucket created with FlowLogDestination.toS3 conflicts with that added by CDK via bucket.enforceSSL since #18271 Jan 27, 2022
@psoaresgit psoaresgit changed the title aws-ec2: automatic created policy for bucket created with FlowLogDestination.toS3 conflicts with that added by CDK via bucket.enforceSSL since #18271 aws-ec2: automatic created policy for bucket already created with FlowLogDestination.toS3 conflicts with that added by CDK via bucket.enforceSSL since #18271 Jan 27, 2022
@ryparker ryparker added the p2 label Jan 31, 2022
@ryparker
Copy link
Contributor

ryparker commented Jan 31, 2022

This issue description is difficult to understand but I believe I get the gist of it.

It seems like...

  1. You've already deployed a custom version of the bucket policy using an older version of CDK (< v2.9).
  2. chore(ec2): enforceSSL on flowLog s3 bucket #18271 was merged.
  3. You are unable to update your stack with the latest CDK version because your custom policy conflicts with the new bucket policy added in chore(ec2): enforceSSL on flowLog s3 bucket #18271.

Is this accurate? Could you share any error logs? The reproducible code you provided is not enough to reproduce the issue, could you share a complete reproduction with steps?

@ryparker ryparker added the needs-reproduction This issue needs reproduction. label Jan 31, 2022
@psoaresgit
Copy link
Author

psoaresgit commented Jan 31, 2022

No, it's not a custom bucket policy - AWS puts the policy necessary if none is provided See the documentation link I provided. Any bucket created with the construct before the change in question to the CDK resulted in AWS configuring a bucket policy. Any update with the latest version of the CDK to a stack that created such a bucket will fail because CloudFormation will not replace the policy that was generated and configured by AWS with the one now being created in the CloudFormation stack.

@corymhall corymhall added p0 and removed p2 needs-reproduction This issue needs reproduction. needs-triage This issue or PR still needs to be triaged. labels Feb 1, 2022
eladb pushed a commit that referenced this issue Feb 1, 2022
eladb pushed a commit that referenced this issue Feb 1, 2022
…h instances and launch templates (#18326)"

Reverts commit 17dbe5f. in the meantime until we figure out the longer term solution.

Fixes #18676
@njlynch njlynch assigned eladb and unassigned njlynch Feb 1, 2022
@mergify mergify bot closed this as completed in #18772 Feb 1, 2022
mergify bot pushed a commit that referenced this issue Feb 1, 2022
… s3 loggingBucket (#18264)" (#18772)

#18271 resulted in the definition of a new bucket policy, which broke existing users that already had an implicit bucket policy created by AWS (see [docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/AWS-logs-and-resource-policy.html#AWS-logs-infrastructure-S3)).

Reverts commit ad7374a in the meantime until we figure out the longer term solution.

Fixes #18676

----

*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

github-actions bot commented Feb 1, 2022

⚠️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.

mergify bot pushed a commit that referenced this issue Feb 1, 2022
#18271 resulted in the definition of a new bucket policy, which broke existing users that already had an implicit bucket policy created by AWS (see [docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/AWS-logs-and-resource-policy.html#AWS-logs-infrastructure-S3)).

Reverts commit 0ed5e85 in the meantime until we figure out the longer term solution.

Fixes #18676


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
… s3 loggingBucket (aws#18264)" (aws#18772)

aws#18271 resulted in the definition of a new bucket policy, which broke existing users that already had an implicit bucket policy created by AWS (see [docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/AWS-logs-and-resource-policy.html#AWS-logs-infrastructure-S3)).

Reverts commit ad7374a in the meantime until we figure out the longer term solution.

Fixes aws#18676

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…s#18770)

aws#18271 resulted in the definition of a new bucket policy, which broke existing users that already had an implicit bucket policy created by AWS (see [docs](https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/AWS-logs-and-resource-policy.html#AWS-logs-infrastructure-S3)).

Reverts commit 0ed5e85 in the meantime until we figure out the longer term solution.

Fixes aws#18676


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment