feat: support different architectures when copying images #570
Conversation
Head branch was pushed to by a user without write access
|
Great PR @uncledru. Even I am looking for this feature. |
|
It will be great if @wchaws can look into this |
|
also tagging @rix0rrr |
|
Hi, awesome and useful feature, please make it happen soon. 😄 Thanks. |
|
Is there an update on this? |
| @@ -90,7 +90,7 @@ | |||
| ], | |||
| "main": "lib/index.js", | |||
| "license": "Apache-2.0", | |||
| "version": "0.0.0", | |||
| "version": "3.0.0", | |||
There was a problem hiding this comment.
please add a new test case using and asserting the new feature, keeping the old test case as it was.
If the old test case also needs to be changed, please explain why.
| @@ -184,3 +214,13 @@ func GetSecret(secretId string) (secret string, err error) { | |||
| } | |||
| return *resp.SecretString, nil | |||
| } | |||
|
|
|||
| func IsValidGOARCH(arch string) bool { | |||
There was a problem hiding this comment.
what is this file doing? Why was it added?
There was a problem hiding this comment.
This is good. Some housekeeping comments.
We should also include OSChoice and VariantChoice as well.
https://github.com/containers/image/blob/v5.21.0/types/types.go#L579-L583
| @@ -31,6 +31,11 @@ export interface ECRDeploymentProps { | |||
| */ | |||
| readonly dest: IImageName; | |||
|
|
|||
| /** | |||
| * The architecture of the docker images to copy. | |||
There was a problem hiding this comment.
| * The architecture of the docker images to copy. | |
| * The architecture of the docker images to copy. | |
| * | |
| * @default "x86" |
| @@ -192,6 +197,7 @@ export class ECRDeployment extends Construct { | |||
| SrcCreds: props.src.creds, | |||
| DestImage: props.dest.uri, | |||
| DestCreds: props.dest.creds, | |||
| Architecture: props.architecture, | |||
There was a problem hiding this comment.
I reckon we should fix this and move away from defaulting to the lambda architecture to explicitly defining a default here.
Fixes #568
Currently we can only copy images for the
amd64architecture. copy supportsArchitectureChoicewhich is leveraged in this PR. If noArchitectureis provided the current behavior applies (uses the platform architecture, which is x86).