Skip to content

Commit

Permalink
feat: Add support to explicitly specify a CPU architecture for the to…
Browse files Browse the repository at this point in the history
…-be-copied image instead of relying on auto-detection (#961)

### What's the problem?

It's observed that when copying a multi-arch source image from location
A to B in ECR, we always end up with `amd64` in the destination image at
location B. The hypothesis is that the underlying lambda function used
to auto-detect the architecture is backed up by Dockerfile with [a
hard-coded
runtime.GOARCH](https://github.com/cdklabs/cdk-ecr-deployment/blob/dcd74cdfc394d6a43a5517cb5cce1843e1fe3111/lambda/Dockerfile#L11).

### Proposed Solution
Instead of solely relying on an auto-detection mechanism, this PR is
meant to add a feature to allow users to explicitly specify a CPU
architecture for the to-be-copied image, by triggering the underlying
logic
[here](https://github.com/containers/image/blob/5263462aa97ae23d3a620b4dcdbbc557665a42c8/internal/pkg/platform/platform_matcher.go#L164).
This will unblock the usage of Graviton instances for [Amazon
SageMaker](https://aws.amazon.com/sagemaker/) and other computing
platforms.

As a side note, I've run `npm run release` on my dev box to refresh
`API.md`

---------

Co-authored-by: Bo Xiong <[email protected]>
Co-authored-by: Rico Huijbers <[email protected]>
  • Loading branch information
3 people authored Dec 23, 2024
1 parent 093ad7b commit fbab401
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 8 deletions.
1 change: 1 addition & 0 deletions .gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const project = new CdklabsConstructLibrary({
repositoryUrl: 'https://github.com/cdklabs/cdk-ecr-deployment', /* The repository is the location where the actual code for your package lives. */
gitignore: [
'cdk.out/',
'lambda/out/bootstrap',
], /* Additional entries to .gitignore. */
npmignore: [
'/cdk.out',
Expand Down
20 changes: 20 additions & 0 deletions API.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions lambda/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ func handler(ctx context.Context, event cfn.Event) (physicalResourceID string, d
if err != nil {
return physicalResourceID, data, err
}
imageArch, err := getStrPropsDefault(event.ResourceProperties, IMAGE_ARCH, "")
if err != nil {
return physicalResourceID, data, err
}
srcCreds, err := getStrPropsDefault(event.ResourceProperties, SRC_CREDS, "")
if err != nil {
return physicalResourceID, data, err
Expand All @@ -71,7 +75,7 @@ func handler(ctx context.Context, event cfn.Event) (physicalResourceID string, d
return physicalResourceID, data, err
}

log.Printf("SrcImage: %v DestImage: %v", srcImage, destImage)
log.Printf("SrcImage: %v DestImage: %v ImageArch: %v", srcImage, destImage, imageArch)

srcRef, err := alltransports.ParseImageName(srcImage)
if err != nil {
Expand All @@ -82,13 +86,13 @@ func handler(ctx context.Context, event cfn.Event) (physicalResourceID string, d
return physicalResourceID, data, err
}

srcOpts := NewImageOpts(srcImage)
srcOpts := NewImageOpts(srcImage, imageArch)
srcOpts.SetCreds(srcCreds)
srcCtx, err := srcOpts.NewSystemContext()
if err != nil {
return physicalResourceID, data, err
}
destOpts := NewImageOpts(destImage)
destOpts := NewImageOpts(destImage, imageArch)
destOpts.SetCreds(destCreds)
destCtx, err := destOpts.NewSystemContext()
if err != nil {
Expand Down
13 changes: 11 additions & 2 deletions lambda/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ func TestMain(t *testing.T) {
destRef, err := alltransports.ParseImageName(destImage)
assert.NoError(t, err)

srcOpts := NewImageOpts(srcImage)
srcOpts := NewImageOpts(srcImage, "")
srcCtx, err := srcOpts.NewSystemContext()
assert.NoError(t, err)
destOpts := NewImageOpts(destImage)
destOpts := NewImageOpts(destImage, "")
destCtx, err := destOpts.NewSystemContext()
assert.NoError(t, err)

Expand All @@ -53,3 +53,12 @@ func TestMain(t *testing.T) {
})
assert.NoError(t, err)
}

func TestNewImageOpts(t *testing.T) {
srcOpts := NewImageOpts("s3://cdk-ecr-deployment/nginx.tar:nginx:latest", "arm64")
_, err := srcOpts.NewSystemContext()
assert.NoError(t, err)
destOpts := NewImageOpts("dir:/tmp/nginx.dir", "arm64")
_, err = destOpts.NewSystemContext()
assert.NoError(t, err)
}
9 changes: 6 additions & 3 deletions lambda/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
const (
SRC_IMAGE string = "SrcImage"
DEST_IMAGE string = "DestImage"
IMAGE_ARCH string = "ImageArch"
SRC_CREDS string = "SrcCreds"
DEST_CREDS string = "DestCreds"
)
Expand Down Expand Up @@ -86,14 +87,15 @@ type ImageOpts struct {
requireECRLogin bool
region string
creds string
arch string
}

func NewImageOpts(uri string) *ImageOpts {
func NewImageOpts(uri string, arch string) *ImageOpts {
requireECRLogin := strings.Contains(uri, "dkr.ecr")
if requireECRLogin {
return &ImageOpts{uri, requireECRLogin, GetECRRegion(uri), ""}
return &ImageOpts{uri, requireECRLogin, GetECRRegion(uri), "", arch}
} else {
return &ImageOpts{uri, requireECRLogin, "", ""}
return &ImageOpts{uri, requireECRLogin, "", "", arch}
}
}

Expand All @@ -109,6 +111,7 @@ func (s *ImageOpts) NewSystemContext() (*types.SystemContext, error) {
ctx := &types.SystemContext{
DockerRegistryUserAgent: "ecr-deployment",
DockerAuthConfig: &types.DockerAuthConfig{},
ArchitectureChoice: s.arch,
}

if s.creds != "" {
Expand Down
20 changes: 20 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ export interface ECRDeploymentProps {
*/
readonly dest: IImageName;

/**
* The image architecture to be copied.
*
* The 'amd64' architecture will be copied by default. Specify the
* architecture or architectures to copy here.
*
* It is currently not possible to copy more than one architecture
* at a time: the array you specify must contain exactly one string.
*
* @default ['amd64']
*/
readonly imageArch?: string[];

/**
* The amount of memory (in MiB) to allocate to the AWS Lambda function which
* replicates the files from the CDK bucket to the destination bucket.
Expand Down Expand Up @@ -199,14 +212,21 @@ export class ECRDeployment extends Construct {
resources: ['*'],
}));

if (props.imageArch && props.imageArch.length !== 1) {
throw new Error(`imageArch must contain exactly 1 element, got ${JSON.stringify(props.imageArch)}`);
}
const imageArch = props.imageArch ? props.imageArch[0] : '';

new CustomResource(this, 'CustomResource', {
serviceToken: this.handler.functionArn,
// This has been copy/pasted and is a pure lie, but changing it is going to change people's infra!! X(
resourceType: 'Custom::CDKBucketDeployment',
properties: {
SrcImage: props.src.uri,
SrcCreds: props.src.creds,
DestImage: props.dest.uri,
DestCreds: props.dest.creds,
...imageArch ? { ImageArch: imageArch } : {},
},
});
}
Expand Down
61 changes: 61 additions & 0 deletions test/ecr-deployment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Stack, App, aws_ecr as ecr, assertions } from 'aws-cdk-lib';
import { DockerImageName, ECRDeployment } from '../src';

// Yes, it's a lie. It's also the truth.
const CUSTOM_RESOURCE_TYPE = 'Custom::CDKBucketDeployment';

let app: App;
let stack: Stack;

const src = new DockerImageName('javacs3/javacs3:latest', 'dockerhub');
let dest: DockerImageName;
beforeEach(() => {
app = new App();
stack = new Stack(app, 'Stack');

const repo = new ecr.Repository(stack, 'Repo', {
repositoryName: 'repo',
});
dest = new DockerImageName(`${repo.repositoryUri}:copied`);

// Otherwise we do a Docker build :x
process.env.FORCE_PREBUILT_LAMBDA = 'true';
});

test('ImageArch is missing from custom resource if argument not specified', () => {
// WHEN
new ECRDeployment(stack, 'ECR', {
src,
dest,
});

// THEN
const template = assertions.Template.fromStack(stack);
template.hasResourceProperties(CUSTOM_RESOURCE_TYPE, {
ImageArch: assertions.Match.absent(),
});
});

test('ImageArch is in custom resource properties if specified', () => {
// WHEN
new ECRDeployment(stack, 'ECR', {
src,
dest,
imageArch: ['banana'],
});

// THEN
const template = assertions.Template.fromStack(stack);
template.hasResourceProperties(CUSTOM_RESOURCE_TYPE, {
ImageArch: 'banana',
});
});

test('Cannot specify more or fewer than 1 elements in imageArch', () => {
// WHEN
expect(() => new ECRDeployment(stack, 'ECR', {
src,
dest,
imageArch: ['banana', 'pear'],
})).toThrow(/imageArch must contain exactly 1 element/);
});
18 changes: 18 additions & 0 deletions test/example.ecr-deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,27 @@ class TestECRDeployment extends Stack {
dest: new ecrDeploy.DockerImageName(`${repo.repositoryUri}:latest`),
});

new ecrDeploy.ECRDeployment(this, 'DeployECRImage', {
src: new ecrDeploy.DockerImageName(image.imageUri),
dest: new ecrDeploy.DockerImageName(`${repo.repositoryUri}:latest`),
imageArch: ['arm64'],
});

new ecrDeploy.ECRDeployment(this, 'DeployDockerImage', {
src: new ecrDeploy.DockerImageName('javacs3/javacs3:latest', 'dockerhub'),
dest: new ecrDeploy.DockerImageName(`${repo.repositoryUri}:dockerhub`),
}).addToPrincipalPolicy(new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: [
'secretsmanager:GetSecretValue',
],
resources: ['*'],
}));

new ecrDeploy.ECRDeployment(this, 'DeployDockerImage', {
src: new ecrDeploy.DockerImageName('javacs3/javacs3:latest', 'dockerhub'),
dest: new ecrDeploy.DockerImageName(`${repo.repositoryUri}:dockerhub`),
imageArch: ['amd64'],
}).addToPrincipalPolicy(new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
actions: [
Expand Down

0 comments on commit fbab401

Please sign in to comment.