Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support to explicitly specify a CPU architecture for the to-be-copied image instead of relying on auto-detection #961

Merged
merged 3 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, "")
xiongbo-sjtu marked this conversation as resolved.
Show resolved Hide resolved
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
Loading