fix(ecr): allow creating repository uri to use tokens like cfn params #32053
Conversation
github-actions
bot
added
bug
| this.tokenConditions[tagOrDigest] = isInputDigestCondition; | ||
| } | ||
|
|
||
| const suffix = Fn.conditionIf(isInputDigestCondition.logicalId, `@${tagOrDigest}`, `:${tagOrDigest}`).toString(); |
There was a problem hiding this comment.
What is this line doing? What's the purpose of @${tagOrDigest} and :${tagOrDigest} being passed in?
There was a problem hiding this comment.
the condition I added checks if the passed Token contains a Tag or Image Digest, if it is a tag the repo uri should be in this format <ecr repo base uri>@<image digest value>, if it is a tag, so the format will be <ecr repo base uri>:<tag value>
There was a problem hiding this comment.
I tagged you in 2 examples for each use case in the integ testing template
There was a problem hiding this comment.
I can update the code as well to make it more clear
| "ImageUri": { | ||
| "Fn::Join": [ | ||
| "", | ||
| [ | ||
| { | ||
| "Fn::Select": [ | ||
| 4, | ||
| { | ||
| "Fn::Split": [ | ||
| ":", | ||
| { | ||
| "Fn::GetAtt": [ | ||
| "MyRepoF4F48043", | ||
| "Arn" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| ".dkr.ecr.", | ||
| { | ||
| "Fn::Select": [ | ||
| 3, | ||
| { | ||
| "Fn::Split": [ | ||
| ":", | ||
| { | ||
| "Fn::GetAtt": [ | ||
| "MyRepoF4F48043", | ||
| "Arn" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| ".", | ||
| { | ||
| "Ref": "AWS::URLSuffix" | ||
| }, | ||
| "/", | ||
| { | ||
| "Ref": "MyRepoF4F48043" | ||
| }, | ||
| "@sha256:55d873154f78cd9a13487c9539ff380f396b887f4a815d84fd698ed872c74749" |
There was a problem hiding this comment.
@paulhcsun .. this is an example for the image uri in the Image Digest use case
| "ImageUri": { | ||
| "Fn::Join": [ | ||
| "", | ||
| [ | ||
| { | ||
| "Fn::Select": [ | ||
| 4, | ||
| { | ||
| "Fn::Split": [ | ||
| ":", | ||
| { | ||
| "Fn::GetAtt": [ | ||
| "MyRepoF4F48043", | ||
| "Arn" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| ".dkr.ecr.", | ||
| { | ||
| "Fn::Select": [ | ||
| 3, | ||
| { | ||
| "Fn::Split": [ | ||
| ":", | ||
| { | ||
| "Fn::GetAtt": [ | ||
| "MyRepoF4F48043", | ||
| "Arn" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| }, | ||
| ".", | ||
| { | ||
| "Ref": "AWS::URLSuffix" | ||
| }, | ||
| "/", | ||
| { | ||
| "Ref": "MyRepoF4F48043" | ||
| }, | ||
| ":testTag" | ||
| ] |
There was a problem hiding this comment.
@paulhcsun .. and this is an example for the image uri in the tag use case
paulhcsun
changed the title
…to melasmar/ecr-allow-tokens-repo-uri
aws-cdk-automation
added
the
pr/needs-maintainer-review
|
@Mergifyio update |
☑️ Nothing to do
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
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. |
Issue # (if applicable)
Closes #31860.
Reason for this change
Currently customers can pass one property
tagOrDigestand if the customers pass a CFN parameter, CDK could not know if it is a tag or digest, and so the generated URI is not correct.Now the same parameter can supports Tokens, and it will generate a CFN condition to check if the value of this token is digest or tag, and then update the uri based on the condition output.
Description of changes
Check if the input is a Token, and so instead of determining if its value is a tag or digest in synth time, we create a CFN condition to do this check in CFN, and then determine how to build the repo uri.
Description of how you validated changes
Added unit test cases, and integration test cases with assertions.
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license