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: remove the lambda layers used for custom resources underneath lambda functions #1731

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

moelasmar
Copy link
Contributor

@moelasmar moelasmar commented Sep 13, 2024

This change removed the Lambda layers we used to add the pgp and openssl binaries to the lambda functions that are used to implement the OpenPGPKeyPair, RsaPrivateKeySecret, CodeSigningCertificate, and CertificateSigningRequest constructs.

The change is to make these lambda functions to be of Image package type, so we can install the required binaries in the docker image, instead of maintaining the binaries in the github repo.

I followed this section https://github.com/cdklabs/aws-delivlib/blob/main/CONTRIBUTING.md#testing for testing these changes, and mainly the part of running yarn integ:update


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -142,7 +142,8 @@ export class TestStack extends Stack {
identity: 'aws-cdk-dev',
keySizeBits: 4_096,
pubKeyParameterName: `/${this.node.path}/CodeSign.pub`,
secretName: this.node.path + '/CodeSign',
// I am not able to immediately delete the existing secret, it needs to wait at least 7 days.
Copy link
Contributor

@comcalvi comcalvi Sep 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens to the old secret here? Are we not deleting it by changing the name?

Copy link
Contributor

@comcalvi comcalvi Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole chain of resource replacements is:

  1. We need to change the custom resource handler's packaging type from zip to Docker image, but Lambda doesn't allow that; we need to make a new Function, which we do by changing the logical ID.
  2. Changing the Custom Resource Service Token is similarly forbidden, so we must also create a new one; this is another logical ID change.
  3. Recreating the custom resource will cause it to create a new key, which will fail because the key with that name (this.node.path + '/CodeSign') already exists; so, we must change the key name.

@moelasmar I would think that step 3 would cause the secret to be deleted as well, because the custom resource should delete it on the DELETE event. Can you explain why it doesn't?

# Use a NodeJS 20.x runtime
FROM public.ecr.aws/lambda/nodejs:20-x86_64

# install openssel and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# install openssel and
# install openssel

@@ -0,0 +1,15 @@
FROM public.ecr.aws/lambda/nodejs:20-x86_64

# install openssel and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need all three of these Dockerfiles? They look like the same one each time, is there any harm in using one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed offline; can use an env var to change the one difference between them

@comcalvi
Copy link
Contributor

Broader context:

We used to store GPG and openssl in zip files, containing binaries that we have downloaded from an EC2 instance. When we discovered that these were out of date, we decided to store these through a Dockerfile. Lambda vends the Node runtime environments as docker images, which we can use to get the latest versions of these that Lambda supports.

Ideally, we'd rebuild this dockerfile every time we build, so that dependency upgrade PRs happen to fetch the latest versions of these for us.

moelasmar and others added 3 commits September 17, 2024 14:29
Signed-off-by: github-actions <github-actions@github.com>
Comment on lines -191 to -193
pipeline.autoBuild({
publicLogs: true,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the deletion here? We want this to be tested, no?

@mergify mergify bot merged commit 9af9a71 into main Sep 19, 2024
7 checks passed
@mergify mergify bot deleted the melasmar/remove_pgp_openssl_binaries branch September 19, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants