Skip to content

Conversation

PatMyron
Copy link
Contributor

@PatMyron PatMyron commented Jan 4, 2022

#18283

could pass another bucket, but automatically created buckets are convenient/popular, so worth improving defaults

https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.Distribution.html
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.CloudFrontWebDistribution.html
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html


# updated integ snapshots
packages/@aws-cdk/aws-cloudfront $ /workspace/aws-cdk/tools/\@aws-cdk/cdk-integ-tools/bin/cdk-integ integ.cloudfront-bucket-logging.js integ.distribution-extensive.js --dry-run

also: #18268, #18269, #18270, #18271


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@gitpod-io
Copy link

gitpod-io bot commented Jan 4, 2022

@github-actions github-actions bot added the @aws-cdk/aws-cloudfront Related to Amazon CloudFront label Jan 4, 2022
Copy link
Contributor

@njlynch njlynch left a comment

Choose a reason for hiding this comment

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

I see you've opened quite a lot of these.

I'm ever-so-slightly hesitant about the enforceSSL change. There is a chance -- remote as it may be -- that someone, somewhere has opened up their logs for access using HTTP, and that this will break them. I can't find a good reason why turning on S3-managed encryption by default would possibly break anyone. We could make the former change under a feature flag, but I feel that the risk of anyone doing that for any reason is so minimal that I'm happy to approve this (and the other in my queue) as-is, and we'll deal with the fall-out if and when it arises.

@mergify
Copy link
Contributor

mergify bot commented Jan 6, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 600cfa1
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit ad7374a into aws:master Jan 6, 2022
@mergify
Copy link
Contributor

mergify bot commented Jan 6, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@PatMyron PatMyron deleted the cloudfront branch January 6, 2022 22:19
eladb pushed a commit that referenced this pull request Feb 1, 2022
eladb pushed a commit that referenced this pull request Feb 1, 2022
mergify bot pushed a commit that referenced this pull request 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*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
…ngBucket (aws#18264)

could pass another bucket, but automatically created buckets are convenient/popular, so worth improving defaults

https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.Distribution.html
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.CloudFrontWebDistribution.html
https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.Bucket.html

---

```sh
# updated integ snapshots
packages/@aws-cdk/aws-cloudfront $ /workspace/aws-cdk/tools/\@aws-cdk/cdk-integ-tools/bin/cdk-integ integ.cloudfront-bucket-logging.js integ.distribution-extensive.js --dry-run
```

----

*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 pull request 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*
PatMyron added a commit to PatMyron/aws-cdk that referenced this pull request Feb 22, 2022
aws#18264 got reverted in:
aws#18772

because of the BucketPolicy, re-submitting the non-BucketPolicy half of that PR
mergify bot pushed a commit that referenced this pull request Mar 11, 2022
#18264 got reverted in #18772 because of the BucketPolicy, re-submitting the **_non-BucketPolicy half of that PR_**


----

*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
Labels
@aws-cdk/aws-cloudfront Related to Amazon CloudFront
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants