aws-cdk
aws-cdk copied to clipboard
feat(ecr): adding ECR pull through cache rule
Fixes #20464
With this PR I tried to implement the pull through cache rules from ECR. (See here: https://docs.aws.amazon.com/AmazonECR/latest/userguide/pull-through-cache.html)
I was not sure if this needs a L2 Construct but I thought it would be good, because it is then shown in the CDK reference documentation. L1 Constructs are, in my experience, a bit more hidden. I'm still not sure if we should expose it to the user, because the list of things to consider when creating such a rule is quite long. I've added a hint in the docs and decided against copying all bullet points to the docs.
Some open questions:
- Should there be a method which automatically grants the
ecr:CreatePullThroughCacheRule(and others) on aGrantee? - Should there be more documentation? I've copied the text from the original AWS docs and I'm not sure if this is enough.
I would appreciate your feedback! ❤️
All Submissions:
- [x] Have you followed the guidelines in our Contributing guide?
Adding new Unconventional Dependencies:
- [ ] This PR adds new unconventional dependencies following the process described here
New Features
- [x] Have you added the new feature to an integration test?
- [x] Did you use
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?
- [x] Did you use
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
@corymhall I added a section in the readme for the usage of the repository created by the pull through cache rule. I took the example from this re:Invent talk actually: AWS re:Invent 2021 - Up-level your container image security with the latest from Amazon ECR.
For the access restriction I thought it would be good to have a function where I can put in some principals and repositories and the registry policy is generated out of that. However I'm not sure if that is too 'restrictive' (a user could want to generate a whole statement by her/hisself). And I'm not sure if IPrincipal is the best interface there, because it has to be a user/root principal, otherwise the registry policy won't deploy. (I have learned it the hard way while executing the integration tests). Do you know how I can restrict this programmatically?
For the upstream urls I decided to use an enum, because it felt naturally to me. And it divides the 'hard-coded' upstream url and the customizable repository prefix in two parts :)
Next step would be to add more tests and iterate on the restrictAcess functionality, because right now it does not feel very CDK-ish. I thought about using grant.. but I don't know if this is the cleanest way either.
If you have any feedback on the current changes I would be very happy :) Thank you as always for the valuable feedback!
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: b0e0eef013dbece66da5fff0513b865b79353424
- Result: SUCCEEDED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
This PR has been in the CHANGES REQUESTED 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.
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.