fix: Remove build golang lambda #535

wchaws · 2024-02-20T14:30:08Z

Fixes #222

Documentation
Remove skopeo and enable crane
Remove golang build
Include crane binary in release. Follow https://github.com/cdklabs/awscdk-asset-kubectl/tree/kubectl-v29/main
Python unit test
Test cases

Test Cases

Test project can be found at https://github.com/cdklabs/cdk-ecr-deployment/blob/dev/test/example.ecr-deployment.ts.
Instructions can be found at https://github.com/cdklabs/cdk-ecr-deployment/tree/dev?tab=readme-ov-file#sample-testexampleecr-deploymentts

1. Copy from local cdk docker image asset to ECR repo.

Prerequisite:

An existing ECR repo.

const image = new DockerImageAsset(this, 'CDKDockerImage', {
  directory: path.join(__dirname, 'docker'),
});

new ecrdeploy.ECRDeployment(this, 'DeployDockerImage1', {
  src: new ecrdeploy.DockerImageName(image.imageUri),
  dest: new ecrdeploy.DockerImageName(`${cdk.Aws.ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/my-nginx:latest`),
});

2. Copy from a ECR repo to another ECR repo.

Prerequisite:

Two existing ECR repo.

new ecrdeploy.ECRDeployment(this, 'DeployDockerImage1', {
  src: new ecrdeploy.DockerImageName(`${cdk.Aws.ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/AAAAlatest`),
  dest: new ecrdeploy.DockerImageName(`${cdk.Aws.ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/BBBB:latest`),
});

3. Copy from docker registry to ECR repo.

new ecrdeploy.ECRDeployment(this, 'DeployDockerImage2', {
  src: new ecrdeploy.DockerImageName('nginx:latest'),
  dest: new ecrdeploy.DockerImageName(`${cdk.Aws.ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/my-nginx2:latest`),
});

4. Copy from private docker registry to ECR repo.

Prerequisite:

An existing ECR repo.
A private docker registry and an access token for the registry.
Create a AWS Secrets Manager secret to store the access token. The format must be <username>:<password>.

Notice: The creds parameter can be the secret name or arn of the aws secrets manager in the same cdk stack region.

// Copy from private docker registry to ECR.
// The format of secret in aws secrets manager must be plain text! e.g. <username>:<password>
new ecrdeploy.ECRDeployment(this, 'DeployDockerImage3', {
  src: new ecrdeploy.DockerImageName('your-private-docker-registry/nginx:latest', 'username:password'),
  // src: new ecrdeploy.DockerImageName('your-private-docker-registry/nginx:latest', 'aws-secrets-manager-secret-name'),
  // src: new ecrdeploy.DockerImageName('your-private-docker-registry/nginx:latest', 'arn:aws:secretsmanager:us-west-2:000000000000:secret:id'),
  dest: new ecrdeploy.DockerImageName(`${cdk.Aws.ACCOUNT_ID}.dkr.ecr.us-west-2.amazonaws.com/my-nginx3:latest`),
}).addToPrincipalPolicy(new iam.PolicyStatement({
  effect: iam.Effect.ALLOW,
  actions: [
    'secretsmanager:GetSecretValue',
  ],
  resources: ['*'],
}));

5. Regional test

Select some regions to test if it's able to work

Signed-off-by: github-actions <[email protected]>

…nto dev

Signed-off-by: github-actions <[email protected]>

…nto dev

xazhao · 2024-08-13T23:18:44Z

Starting to look into this PR. Because I'm not familiar with this L2, could you please add some details to help me understand the change?

Reason of changes
What are changes

I see Python is added and Go is removed. Is there a reason for that? And are you removing all Go code from the repo?

xazhao · 2024-08-13T23:53:26Z

layer/build.sh

+ARCH=x86_64    # or arm64, x86_64, armv6, i386, s390x
+VERSION=v0.19.0
+
+curl -sL "https://github.com/google/go-containerregistry/releases/download/${VERSION}/go-containerregistry_${OS}_${ARCH}.tar.gz" > go-containerregistry.tar.gz


Does this script run on customer's machine?

Correct me if I'm wrong: this shell script will run whenever customers run synth().

Instead of downloading it every time when customers use the construct, can we do this part in our release process and publish the layer.zip. So they don't need to connect to Github which has availability risk.

rix0rrr

This PR is doing too much.

It's changing how the asset is built, as well as replacing the code inside the asset and changing functionality.

Those might all be valid changes, but I'd feel a lot better if they were submitted separately in different PRs.

rix0rrr · 2024-08-26T11:11:35Z

README.md

 ## Sample: [test/example.ecr-deployment.ts](./test/example.ecr-deployment.ts)

 ```shell
+# Generate crane lambda layer firstly.
+./layer/build.sh


This section needs some clarification that these instructions only apply if you're developing on the repository, not if you're a user of the construct.

README.md is typically consumer-targeted, and the section title says "Sample", so it should (mostly) only contain instructions for consumers.

These instructions would make more sense in CONTRIBUTING.md in a section title "How to run tests".

If you want to keep them here, clearly label the instructions as intended for developers, not for consumers.

rix0rrr · 2024-08-26T11:13:52Z

README.md

-To support a new docker image source(like docker tarball in s3), you need to implement [image transport interface](https://github.com/containers/image/blob/master/types/types.go). You could take a look at [docker-archive](https://github.com/containers/image/blob/ccb87a8d0f45cf28846e307eb0ec2b9d38a458c2/docker/archive/transport.go) transport for a good start.
-
-To test the `lambda` folder, `make test`.
+The underlying implementation depends on the https://github.com/google/go-containerregistry.


What does this mean?

rix0rrr · 2024-08-26T11:18:07Z

lambda/index.py

+import re
+import boto3
+import base64
+import subprocess
+import logging


I really like making 2 distinct changes in a single PR here.

The problem was: the Lambda code is downloaded at synth time.

The expected solution was: the Lambda code is built at release time and bundled into the NPM package.

You are doing that, and also replacing the code in the Lambda layer with a downloaded binary and a Python wrapper at the same time.

This seems to me like it doesn't appreciably simplify the solution to the immediate problem, as well as introduces another layer of risk that I'd rather avoid. The proposed layer code change should be submitted as a separate PR.

rix0rrr · 2024-08-26T11:26:03Z

src/index.ts

      resourceType: 'Custom::CDKBucketDeployment',
      properties: {
+        Time: Date.now().toString(),


This behavior can/should probably be triggered more intelligently. For example, by examining whether or not a tag is supplied, or adding a triggerEveryDeployment?: boolean/mutableSourceUri?: boolean property.

And in any case, functionality changes should not be part of the PR whose sole purpose it is to make the package behave more reliably.

rix0rrr · 2024-08-26T11:27:41Z

src/index.ts

-  /**
-   * Image to use to build Golang lambda for custom resource, if download fails or is not wanted.
-   *
-   * Might be needed for local build if all images need to come from own registry.
-   *
-   * Note that image should use yum as a package manager and have golang available.
-   *
-   * @default public.ecr.aws/sam/build-go1.x:latest
-   */
-  readonly buildImage?: string;


For maximum user impact, it would be better to leave this parameter in place, and to not bump the Major Version. That way, all users currently using the affected version can transparently upgrade to a fixed version. Ignore the property if supplied and report a warning.

Then, in a future Major Version you can remove this parameter, to have a correct API contract.

wchaws added 5 commits February 20, 2024 03:28

chore: try crane

f3f6aa2

chore: try crane wip

f13bbcf

chore: try crane wip

0eff792

chore: projen update

96c307b

chore: crane support read creds from secrets manager

8fd1025

cdklabs-automation enabled auto-merge February 20, 2024 14:30

chore: self mutation

7b43ef5

Signed-off-by: github-actions <[email protected]>

wchaws changed the title ~~Remove build golang lambda~~ BREAKING CHANGE!: Remove build golang lambda Feb 20, 2024

wchaws changed the title ~~BREAKING CHANGE!: Remove build golang lambda~~ fix: Remove build golang lambda Feb 20, 2024

wchaws changed the title ~~fix: Remove build golang lambda~~ fix: Remove build golang lambda WIP Feb 20, 2024

wchaws and others added 16 commits February 21, 2024 06:39

chore: remove golang build

a0f0d52

Merge branch 'dev' of https://github.com/cdklabs/cdk-ecr-deployment i…

43efd4f

…nto dev

fxi: build error

02d7f83

fix: build error

c4531a1

Merge branch 'main' into dev

f90e103

chore: remove build lambda

7c11e1b

chore: add unit test for python

af1c525

chore: enable lambda test

c8e6192

chore: self mutation

bf66e8e

Signed-off-by: github-actions <[email protected]>

chore: add python unit test

d8438ae

Merge branch 'dev' of https://github.com/cdklabs/cdk-ecr-deployment i…

5c63612

…nto dev

chore: move pyproject.toml -> lambda2/pyproject.toml

13462b1

Merge branch 'main' into dev

1939c90

chore: add more python ut

b1b8fc5

Merge branch 'dev' of https://github.com/cdklabs/cdk-ecr-deployment i…

8013833

…nto dev

chore: remove golang lambda

620fb87

wchaws force-pushed the dev branch from 096aa7d to 620fb87 Compare March 3, 2024 13:37

wchaws added 3 commits March 3, 2024 13:44

chore: add comments

20386dc

chore: update README.md

58a4878

chore: update REAMDE.md

963dedf

wchaws changed the title ~~fix: Remove build golang lambda WIP~~ fix: Remove build golang lambda Mar 3, 2024

wchaws requested review from kaizencc and scanlonp March 3, 2024 14:32

wchaws added 4 commits March 3, 2024 14:33

chore: update README

1614181

chore: update README

8e62c49

Merge branch 'main' into dev

7b80c20

Merge branch 'main' into dev

24865dd

wchaws self-assigned this Mar 16, 2024

wchaws added 3 commits March 19, 2024 13:11

Merge branch 'main' into dev

e58665a

Merge branch 'main' into dev

aee18b7

Merge branch 'main' into dev

b6d829c

wchaws requested a review from pahud June 13, 2024 05:52

wchaws and others added 3 commits June 14, 2024 11:27

Merge branch 'main' into dev

5d7249c

chore: set major version to 4

0f0ff9e

Merge branch 'main' of github.com:cdklabs/cdk-ecr-deployment into dev

41431b3

xazhao reviewed Aug 13, 2024

View reviewed changes

rix0rrr requested changes Aug 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Remove build golang lambda #535

fix: Remove build golang lambda #535

wchaws commented Feb 20, 2024 •

edited

Loading

xazhao commented Aug 13, 2024

xazhao Aug 13, 2024

xazhao Aug 14, 2024

rix0rrr left a comment

rix0rrr Aug 26, 2024

rix0rrr Aug 26, 2024

rix0rrr Aug 26, 2024

rix0rrr Aug 26, 2024

rix0rrr Aug 26, 2024

fix: Remove build golang lambda #535

Are you sure you want to change the base?

fix: Remove build golang lambda #535

Conversation

wchaws commented Feb 20, 2024 • edited Loading

Test Cases

1. Copy from local cdk docker image asset to ECR repo.

2. Copy from a ECR repo to another ECR repo.

3. Copy from docker registry to ECR repo.

4. Copy from private docker registry to ECR repo.

5. Regional test

xazhao commented Aug 13, 2024

xazhao Aug 13, 2024

Choose a reason for hiding this comment

xazhao Aug 14, 2024

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

rix0rrr Aug 26, 2024

Choose a reason for hiding this comment

rix0rrr Aug 26, 2024

Choose a reason for hiding this comment

rix0rrr Aug 26, 2024

Choose a reason for hiding this comment

rix0rrr Aug 26, 2024

Choose a reason for hiding this comment

rix0rrr Aug 26, 2024

Choose a reason for hiding this comment

wchaws commented Feb 20, 2024 •

edited

Loading