-
Notifications
You must be signed in to change notification settings - Fork 4.3k
feat: new resource interfaces shared by both L1s and L2s #35032
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
But
Not particularly attached to any name. I can change it to |
# Conflicts: # yarn.lock
…#2) (#35271) Implementing the interfaces from #35032 for L2s. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --------- Co-authored-by: Otavio Macedo <[email protected]>
…erfaces #3) (#35282) Using the updated interfaces from #35271. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --------- Co-authored-by: Rico Huijbers <[email protected]> Co-authored-by: Feng He <[email protected]> Co-authored-by: Kazuho Cryer-Shinozuka <[email protected]> Co-authored-by: Chooi Je Qin <[email protected]> Co-authored-by: claypack <[email protected]> Co-authored-by: Hung Tran <[email protected]> Co-authored-by: matoom-nomu <[email protected]>
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
This PR introduces a new set of minimal L1 interfaces to address resources in the CDK APIs.
New interfaces
They are named like
IBucketRef(andIInstanceRef,IKeyRef, etc), which is an interface similar toIBucketbut lives at the L1 level instead of the L2 level. It allows "pointing to a bucket" in a typed way. The L1CfnBucketimplementsIBucketRef, and the L2IBucketextendsIBucketRef.IBucketRefdoesn't have any of the additional handwritten conveniences afforded byIBucket, but because it doesn't involve any handwritten code this it to be implemented by the L1CfnBucket.We've then gone through all APIs where an
IBucketwas expected, and if that call site didn't use any of the facilities exposed byIBucketwe've replaced the type withIBucketRef.The result is that in those APIs, it is now possible to transparently pass both L1 and L2 resources, instead of only L2 resources. This gives you the freedom of picking between L1s and L2s as your need dictate.
If you are implementing IBucket yourself
If you are writing classes that implement
IBucket, you now have an additional implementation burden:You should copy this value from elsewhere, for example from
CfnBucketor aBucketyou wrap:Future work
This work is the first step in a series of changes that will make it easier for L1 and L2 classes to interoperate, and to progressively add enhancements to L1s in a way that doesn't require implementing full L2s. The goal is to make CDK more expressive with less upfront work.
Experimental
This work is considered experimental, and it may change; we may still add
{ account; region }to to theBucketReferencestruct (which is why you should copy it an underlying object, and not try to construct one yourself).We may also roll back this change entirely and remove the interfaces we introduced here, so don't start referencing
IBucketRefin your own code just yet.To limit the impact, these are introduced currently only for the following services: IAM, S3, EC2, KMS, CloudFront.
Feedback
If you have opinions on this change, don't hesitate to let us know via the issue tracker or via a Direct Message on Slack.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license