feat(cloudfront): implement OriginAccessControl L2 construct

[AMZN-hgoffin]

THIS IS DRAFT STATUS / WORK IN PROGRESS - creating for discussion purposes

This is a L2 construct for OriginAccessControl, and the ability to specify OAC instead of OAI when using CloudFrontWebDistribution or Distribution constructs.

This does NOT yet automatically apply bucket policy or KMS key policy. It may be impossible to do so due circular dependency involving the distribution ID, which is only known after creation of the Distribution but must be specified on the resource policy for any buckets and attached KMS keys that are input properties for the Distribution.

I believe that this circular dependency can be "fixed" with CustomResource construct whose implementation is trusted by the KMS Key to modify the Key's resource policy, and which then adds the cloudfront distribution ID to the key policy at the end of the stack. But that sure is an ugly hack, and I haven't ruled out everything else yet.

Closes #21771.

---

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

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

[AMZN-hgoffin]

There are a few things going on in this pull request:

• full OAC L2 support, including support in CloudFrontWebDistribution and Distribution/S3Origin constructs.
• make S3Origin behavior more explicit. new autoResourcePolicy parameter controls whether or not S3Origin will modify bucket policies.
• support KMS and cross-stack bucket imports when using S3Origin with OAC (using a custom resource hack).
• explicitly fail when autoResourcePolicy cannot be fulfilled due to imported or cross-stack buckets, except where this would break backwards compatibility (so, no changes to behavior when using OAI)

Overall this should make the behavior far less surprising, and it solves the circular-dependency issues with KMS keys. In cases where OAC is enabled but the bucket or key policies cannot be configured due to circular dependencies, stack synthesis will fail with an error messages, and it is up to the user to deploy with autoResourcePolicy: false and then manually fix up permissions afterwards.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 014087f
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[AMZN-hgoffin]

This got derailed by the mainline rename of every single file into the monolithic library, so I'm going to have to step away for a few days until I have enough free time to get my bearings again. I'm doing this largely for personal experience - I'm not sure if this will even be accepted, or just be a discussion point - but I do think that my latest commit series here "does something interesting". It's something the CDK can do that raw CloudFormation cannot do - fixing dependency loops by using CustomResource lambdas to defer only the conflicting parts of resource definitions.

The idea of a "deferred policy addition" is really intriguing as a way to break circular dependencies (like the Origin KMS Key <-> Distribution ID loop) by allowing resources to be deployed in a "not working" state and then patched up at the end when all of the output properties are known. Having an automated way to do that in the CDK (which I've called "DistributionPolicySetter" here but could be refactored into something like a standalone "DeferredPolicyUpdate" L2 construct) would be really cool.

[AMZN-hgoffin]

I suppose the best thing to do here would be to port the L2 constructs, minus all of the deferred policy setting stuff, and just get that functionality working with the new autoResourcePolicy flag but none of the custom resource stuff. That will allow same-stack deployment of a non-KMS bucket and a cloudfront distribution, and introduce the new policy flags which make it easier to reason about resource policies and what needs to be manually edited.

I am starting to like the idea of a DeferredPolicySetter construct as a separate, opt-in L2 construct which can be explicitly (not magically) added to resolve the circular dependencies which are inherent to the design of Origin Access Control - but based on lines-of-code and files-touched, that is a significant undertaking and should definitely split out for my next swing at this problem.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[otaviomacedo]

Hi, @AMZN-hgoffin. For large changes like this, especially when they involve new L2s, we ask contributors to write an RFC first, showing the API, as well as explaining the reasons for doing this, the trade-offs etc. It may seem more work, but it actually saves time and improves the quality in the long run.

[AMZN-hgoffin]

Hi, @AMZN-hgoffin. For large changes like this, especially when they involve new L2s, we ask contributors to write an RFC first, showing the API, as well as explaining the reasons for doing this, the trade-offs etc. It may seem more work, but it actually saves time and improves the quality in the long

Sure, that’s fine, happy to run through that next. I find that it helps to sketch out code and look at the usage before writing the document. I didn’t realize that draft changes triggered a review request or I would have left this in my own repo :)

I’ll open two RFCs as soon as I have time, one for the uncontroversial OAC support and one for the L2 helper that works around the circular dependency for KMS keys.

[otaviomacedo]

That's fine. I'm just letting you know as soon as possible so don't you spend too much time on this PR before we review the RFC. But, by all means, keep it open as draft. You might even reference specific parts of it in the RFC.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

[aws-cdk-automation]

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

[aws-cdk-automation]

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

[BerndWessels]

@AMZN-hgoffin hi, we are desperately trying to fix the cyclical dependency as well as getting OAC properly implemented. Have you had any success with this?