Initial implementation #3

mcdonnnj · 2022-05-25T16:16:09Z

🗣 Description

This pull request is an initial implementation of a modern skeleton for creating AWS Lambdas using Python runtimes.

💭 Motivation and context

The existing cisagov/skeleton-aws-lambda project was built on treating a Lambda similar to how we treat Python packages using cisagov/skeleton-python-library. This project replaces it by treating Python Lambdas as application deployments that only contain handler logic. The bulk of the functionality in a Lambda would instead be handled by other projects that are included as dependencies.

🧪 Testing

Automated tests pass. I was able to run the Lambda locally using the directions in the README and successfully invoke it from the command line:

$ curl -s -XPOST "http://localhost:9000/2015-03-31/functions/function/invocations" -d '{"task": "cowsay", "character": "tux" , "contents": "Hello, World!"}' | jq -r '.message'
  _____________
| Hello, World! |
  =============
                  \
                   \
                    \
                     .--.
                    |o_o |
                    |:_/ |
                   //   \ \
                  (|     | )
                 /'\_   _/`\
                 \___)=(___/

$ curl -s -XPOST "http://localhost:9000/2015-03-31/functions/function/invocations" -d '{"task": "divide", "numerator": 1 , "denominator": 16}' | jq -r '.result'
0.0625

I also deployed a generated deployment package to AWS and tested that I could invoke it from the AWS CLI:

$ aws lambda invoke --cli-binary-format raw-in-base64-out --function-name example --payload '{"task": "cowsay", "character": "tux", "contents": "Hello from the AWS CLI!"}' response.json
$ jq -r '.message' < response.json 
  _______________________
| Hello from the AWS CLI! |
  =======================
                            \
                             \
                              \
                               .--.
                              |o_o |
                              |:_/ |
                             //   \ \
                            (|     | )
                           /'\_   _/`\
                           \___)=(___/

$ aws lambda invoke --cli-binary-format raw-in-base64-out --function-name example --payload '{"task": "divide", "numerator": 1, "denominator": 32}' response.json
$ jq -r '.result' < response.json 
0.03125

✅ Pre-approval checklist

This PR has an informative and human-readable title.
Changes are limited to a single goal - eschew scope creep!
All relevant type-of-change labels have been added.
I have read the CONTRIBUTING document.
These code changes follow cisagov code standards.
All relevant repo and/or project documentation has been updated
to reflect the changes in this PR.
Tests have been added and/or modified to cover the changes in this PR.
All new and existing tests pass.

✅ Post-merge checklist

Add a tag or create a release.

dav3r

This looks fantastic - great work! 🐮 💬
I noted a few minor items, but nothing that hinders my approval.

Dockerfile

src/build_artifact.sh

src/lambda_handler.py

jsf9k

Great work! I had a few suggestions for minor improvements, but I'm happy to approve even if you disagree with all of them.

.github/workflows/release.yml

.pre-commit-config.yaml

Dockerfile

README.md

pytest.ini

jsf9k

Approval intensifies

Add the Dockerfile and appropriate Docker Compose configuration for generating a deployment package.

This script creates a deployment package zipfile inside the built Docker image. The output directory should be mapped to the host filesystem to retrieve the artifact. This is automatically handled in the `docker-compose.yml` file.

Since we are using Pipfiles (and Pipfile.lock files) to manage project dependencies I have added pipenv to the development requirements in `requirements-dev.txt`.

Implement an example handler for this project to show how functionality could be handled.

The build process is updated so that a functional Lambda image is created as a base. The `docker-compose.yml` file was updated to provide a service to create a deployment package for use in AWS and a service to run the Lambda locally for testing.

This job will create Lambda deployment packages for each supported Python version and make them available as artifacts.

Update the README's introductory paragraph and add information about building deployment packages and running the Lambda locally.

Add a version file and appropriate scripts to manage and tag versions for this project.

Set up a barebones pytest configuration. It would be nice to test the handler but given how dependencies are managed this might make local testing a pain point.

This adds a workflow that is triggered on prerelease/release publishing. Right now it is set up to upload artifacts as Action artifacts due to the archival of the actions/upload-release-asset Action.

The vendor name pulled from cisagov/skeleton-aws-lambda did not include the update from cisagov/skeleton-docker. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

We prefer to use long flag names when available to improve the readability of command calls. This fixes some short flag name usage in the Dockerfile and build_artifact.sh script that was missed. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Using `%s - %s` was somewhat ambiguous so a more verbose error message will now be used. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Fix a missing period in a function comment and a typo in an error message. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Add an appropriate comment for the hook that runs against the `tests/` directory and correct the grammar of the comment for the hook that runs against everything but the `tests/` directory. Update the names for the two hook entries to mirror the comment changes. Co-authored-by: Shane Frasier <jeremy.frasier@trio.dhs.gov>

We prefer long flag names whenever possible and this adds a comment to explain the flags used to cover the one short flag we must use. Co-authored-by: Shane Frasier <jeremy.frasier@trio.dhs.gov>

Pull in changes made to cisagov/skeleton-docker around the authors label for consistency.

jsf9k

I have one question about something that puzzles me.

Dockerfile

jsf9k

Still looks good to me.

Fully divorce artifact creation from the Lambda image. We switch to mounting the build_artifact.sh into the container as part of the build_deployment_package service and move installing zip into that script. We also remove the BUILD_FILE_NAME environment variable from the Docker image and house it as part of the Docker Compose configuration. These changes better align the base image as a minimal AWS Lambda image suited for use on its own. Building a deployment artifact now leverages this image instead of being part of the image.

Now that pipenv has the --extra-pip-args option we can pass the options we need to the underlying pip calls. This allows us to use pipenv to directly install the dependencies instead of the previous hack of using a requirements.txt file generated from the Pipfile configuration.

Add a section about how to update Python dependencies for the Lambda to the README.

Update the Python dependencies for the project by running `pipenv lock` in each of the `src/py<Python version>` directories.

Update Action versions that were missed when Lineage updates were merged.

Add an attribution comment and a commented out ignore directive for a `github-actions` dependency that should be managed by this project. This ignore directive should be uncommented in downstream repositories.

Work toward a fancy automated release process can be pushed off until after the initial implementation is in place. This will allow appropriate testing of an otherwise functional implementation so it is appropriately isolated. In the interim releases can be managed by manually creating a release and uploading the artifacts generated from the merge commit's Actions run.

Pull in updates from cisagov/skeleton-packer around the regex used to find the old version.

dav3r

Strong work on this @mcdonnnj - well done! 👍

Part of this project's goals is to support the option of running a bespoke image as a Lambda and not just using deployment packages. Since we care about the created image due to this we should build it similar to how we would build other Docker images. Thus we switch to a multi- stage build so that the only things included in the final image are what is absolutely necessary to run the Lambda (per the dependency files). With this configuration we saw space savings of almost 10% by switching to a multi-stage build.

The default behavior of pipenv is to create a virtual environment to use for installing dependencies. Our configuration does not leverage this venv since we are simply using pipenv to manage dependencies and need all dependencies installed into the Lambda's task root directory. This default behavior can create issues with our configuration because if one of the core packages seeded into that venv (pip, setuptools, or wheel) is the same version as a dependency in the lockfile then it will not be installed into the Lambda's task root directory for later use..

Add the ability to change the image tag used for the Lambda image in the Docker Compose configuration. This is done using the LAMBDA_TAG environment variable with a fallback to the default value of latest.

mcdonnnj · 2022-12-01T16:48:13Z

Unless I discover a bug in testing this is the last time I re-request reviews for this I promise. I switched to a multi-stage build and in the process found a problem with using pipenv directly in this configuration. More information is in the commit messages if interested in context.

dav3r

Good late-breaking changes 👍 👍

jsf9k

I have one comment, which may be based on a misunderstanding of of a commit message.

Dockerfile

jsf9k

Solid

The need for this environment variable was removed in #3 but removing the creation of it was missed.

mcdonnnj added the improvement This issue or pull request will add or improve functionality, maintainability, or ease of use label May 25, 2022

mcdonnnj self-assigned this May 25, 2022

mcdonnnj marked this pull request as ready for review May 25, 2022 17:31

mcdonnnj requested review from dav3r, felddy and jsf9k as code owners May 25, 2022 17:31

dav3r approved these changes May 25, 2022

View reviewed changes

jsf9k approved these changes May 26, 2022

View reviewed changes

mcdonnnj force-pushed the first-commits branch 2 times, most recently from d57ee64 to a3ffd1f Compare May 27, 2022 21:02

jsf9k approved these changes May 31, 2022

View reviewed changes

mcdonnnj and others added 17 commits November 29, 2022 17:31

Add Docker scaffolding

836bfae

Add the Dockerfile and appropriate Docker Compose configuration for generating a deployment package.

Add the script to build a deployment package artifact

75e637e

This script creates a deployment package zipfile inside the built Docker image. The output directory should be mapped to the host filesystem to retrieve the artifact. This is automatically handled in the `docker-compose.yml` file.

Add Pipfiles for each supported Python version

d824f15

Since we are using Pipfiles (and Pipfile.lock files) to manage project dependencies I have added pipenv to the development requirements in `requirements-dev.txt`.

Add Lambda handler

109d223

Implement an example handler for this project to show how functionality could be handled.

Update the Lambda build process

7c5ea91

The build process is updated so that a functional Lambda image is created as a base. The `docker-compose.yml` file was updated to provide a service to create a deployment package for use in AWS and a service to run the Lambda locally for testing.

Add a build job to the GitHub Actions workflow

87eae5e

This job will create Lambda deployment packages for each supported Python version and make them available as artifacts.

Update project README

0b8c693

Update the README's introductory paragraph and add information about building deployment packages and running the Lambda locally.

Add version management

02afeb5

Add a version file and appropriate scripts to manage and tag versions for this project.

Add a barebones testing harness

7f3aa84

Set up a barebones pytest configuration. It would be nice to test the handler but given how dependencies are managed this might make local testing a pain point.

Add a CodeQL workflow

9c50c1f

Add a release workflow

a2a4383

This adds a workflow that is triggered on prerelease/release publishing. Right now it is set up to upload artifacts as Action artifacts due to the archival of the actions/upload-release-asset Action.

Fix vendor label in Dockerfile

80f0872

The vendor name pulled from cisagov/skeleton-aws-lambda did not include the update from cisagov/skeleton-docker. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Update to use long flag names

a4a5cde

We prefer to use long flag names when available to improve the readability of command calls. This fixes some short flag name usage in the Dockerfile and build_artifact.sh script that was missed. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Improve error messages for the divide task

b2014b6

Using `%s - %s` was somewhat ambiguous so a more verbose error message will now be used. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Fix language in lambda_handler.py

56b9ac9

Fix a missing period in a function comment and a typo in an error message. Co-authored-by: dav3r <david.redmin@trio.dhs.gov>

Update pytest.ini to use long flag names

9014297

We prefer long flag names whenever possible and this adds a comment to explain the flags used to cover the one short flag we must use. Co-authored-by: Shane Frasier <jeremy.frasier@trio.dhs.gov>

mcdonnnj force-pushed the first-commits branch from a3ffd1f to 9014297 Compare November 29, 2022 22:31

Update Dockerfile label information

6cb7864

Pull in changes made to cisagov/skeleton-docker around the authors label for consistency.

mcdonnnj mentioned this pull request Nov 30, 2022

Lineage pull request for: skeleton cisagov/publish-egress-ip-lambda#8

Merged

mcdonnnj requested review from dav3r and jsf9k November 30, 2022 20:34

jsf9k reviewed Nov 30, 2022

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

jsf9k approved these changes Nov 30, 2022

View reviewed changes

mcdonnnj added 6 commits November 30, 2022 16:11

Add Python dependency maintenance instructions

7d5c25e

Add a section about how to update Python dependencies for the Lambda to the README.

Update Python dependencies

2b5f9d7

Update the Python dependencies for the project by running `pipenv lock` in each of the `src/py<Python version>` directories.

Update missed GitHub Action versions

915eb71

Update Action versions that were missed when Lineage updates were merged.

Add commented out dependabot ignore information

6f98ec5

Add an attribution comment and a commented out ignore directive for a `github-actions` dependency that should be managed by this project. This ignore directive should be uncommented in downstream repositories.

mcdonnnj force-pushed the first-commits branch from 5b11f12 to 6f98ec5 Compare November 30, 2022 21:11

mcdonnnj added 2 commits November 30, 2022 16:41

Pull in updates to the bump_version.sh script

bde97e0

Pull in updates from cisagov/skeleton-packer around the regex used to find the old version.

dav3r approved these changes Nov 30, 2022

View reviewed changes

mcdonnnj added 3 commits December 1, 2022 04:03

Enable changing the image tag used with Docker Compose

a0036c7

Add the ability to change the image tag used for the Lambda image in the Docker Compose configuration. This is done using the LAMBDA_TAG environment variable with a fallback to the default value of latest.

mcdonnnj requested review from jsf9k and dav3r December 1, 2022 16:46

dav3r approved these changes Dec 1, 2022

View reviewed changes

jsf9k reviewed Dec 1, 2022

View reviewed changes

Dockerfile Show resolved Hide resolved

jsf9k approved these changes Dec 1, 2022

View reviewed changes

mcdonnnj merged commit 4ed8f0a into develop Dec 1, 2022

mcdonnnj deleted the first-commits branch December 1, 2022 19:16

mcdonnnj added a commit that referenced this pull request Jul 17, 2025

Remove unused ENV instruction

f7900c4

The need for this environment variable was removed in #3 but removing the creation of it was missed.

Initial implementation #3

Initial implementation #3

Uh oh!

Conversation

mcdonnnj commented May 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗣 Description

💭 Motivation and context

🧪 Testing

✅ Pre-approval checklist

✅ Post-merge checklist

Uh oh!

dav3r left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

dav3r left a comment

Choose a reason for hiding this comment

Uh oh!

mcdonnnj commented Dec 1, 2022

Uh oh!

dav3r left a comment

Choose a reason for hiding this comment

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsf9k left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcdonnnj commented May 25, 2022 •

edited

Loading