feat: Add credentials_arn to support ECR pull through cache
#30
Conversation
JoseAlvarezSonos
changed the title
|
+1 |
examples/complete/main.tf
Outdated
| @@ -112,6 +114,24 @@ data "aws_iam_policy_document" "registry" { | |||
| module.ecr.repository_arn, | |||
| ] | |||
| } | |||
|
|
|||
| dynamic "statement" { | |||
There was a problem hiding this comment.
Could you please rewrite this as two simple statement blocks instead of using dynamic in the example?
There was a problem hiding this comment.
Yes, refactored
examples/complete/main.tf
Outdated
| ecr_repository_prefix = "dockerhub" | ||
| upstream_registry_url = "registry-1.docker.io" | ||
| # Make sure to read https://docs.aws.amazon.com/AmazonECR/latest/userguide/pull-through-cache-creating-secret.html | ||
| credential_arn = "arn:aws:secretsmanager:us-east-1:123456789:secret:ecr-pullthroughcache/dockerhub" |
There was a problem hiding this comment.
Let's create the Secret Manager resource using this module and pass it as a reference here to show the completely working example.
There was a problem hiding this comment.
Yes, I created the resource and added the secret_string as variable, let me know if that's ok
examples/complete/main.tf
Outdated
| "ecr:CreateRepository", | ||
| "ecr:BatchImportUpstreamImage" | ||
| ] | ||
| resources = ["arn:aws:ecr:us-east-1:012345678901:repository/${statement.value}/*"] |
There was a problem hiding this comment.
012345678901 - should we replace this one with something else (more correct)?
There was a problem hiding this comment.
Good observation, I replaced it with data.aws_caller_identity.current.account_id
antonbabenko
changed the title
- Refactor the a dynamic block into a more verbose format. - Fixed all regions and accounts relying on data resources - Creating the Secret manager resources with the proper format included.
bryantbiggs
changed the title
credentials_arn to support ECR pull through cache
|
for the example? we don't usually enforce this for examples and just stick with the default commercial regions. the module source code we absolutely use the data sources to infer the correct partition, region, etc. |
|
@bryantbiggs I looked in the wrong place :) Approved now |
## [2.1.0](v2.0.0...v2.1.0) (2024-03-25) ### Features * Add `credentials_arn` to support ECR pull through cache ([#30](#30)) ([05e6fd0](05e6fd0))
|
This PR is included in version 2.1.0 🎉 |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Description
This feature PR adds the ability to pass a credentials_arn to the registry_pull_through_cache_rules variable, to allow to use this new field for pull through caches.
Motivation and Context
Since November 2023, ECR supports new upstream registries as pull through cache, including Dockerhub and GHCR. To use these new upstream registries, it's required to use credentials for the respective registries. AWS added a feature of providing a Secret Manager resource ARN to the ECR pull through cache rule to rely on this credentials. Along side this, the terraform-provider-aws added the same capabilities to the aws_ecr_pull_through_cache_rule resource.
Breaking Changes
This requires an update of the aws provider to >= 5.37 (previously >= 4.20). But there isn't any breaking change to update to this version.
How Has This Been Tested?
examples/*to demonstrate and validate my change(s)examples/*projectsWe are using our fork to make sure that it works and added all the required docs and examples to this repo as well.