Skip to content

Automated Container Publishing to AWS #8

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.

Sign up for GitHub

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

Merged
cabutlermit merged 5 commits into from
Mar 6, 2026
Merged

Automated Container Publishing to AWS #8

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac64ead
Sets preferred container architecture
JPrevost Mar 5, 2026
745e498
Update Makefile with Dev Build Commands
cabutlermit Mar 5, 2026
6b991e3
Create/Update GitHub Actions Workflows
cabutlermit Mar 5, 2026
7af22d5
FIX: Update .gitignore
cabutlermit Mar 6, 2026
919ae66
FIX: Return CI to on.push
cabutlermit Mar 6, 2026
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 .aws-architecture
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
linux/arm64
9 changes: 8 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
name: CI
on: push
Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Why are we changing CI to only run on PRs? This doesn't feel related to the ECR work.

Copy link
Contributor Author

@cabutlermit cabutlermit Mar 6, 2026

Choose a reason for hiding this comment

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

This is a result of conversations a month or two ago when Graham & I were working on fixing up another repository. We both agreed that running CI on every single push before a PR was excessive. Once a PR is opened, CI will run on every subsequent push in the PR, which seemed fine.

But your question is more about scope creep... I'm happy to pull this out since it's not directly related to the publishing workflows (and then fix it at a more appropriate time).

Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Yeah, I don't like this change and would prefer to have it removed for now. I'm open to discussing further, but at this point I prefer to see CI turn green before opening a PR


on:
push:
paths-ignore:
- '.github/**'

permissions: read-all
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

overly broad permissions [zizmor:zizmor/excessive-permissions]


jobs:
test:
uses: mitlibraries/.github/.github/workflows/python-uv-shared-test.yml@main
Expand Down
60 changes: 60 additions & 0 deletions .github/workflows/dev-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
### This is the Terraform-generated dev-build.yml workflow for the ###
### timdex-semantic-builder-dev app repository. ###
### If this is a Lambda repo, uncomment the FUNCTION line at the end of ###
### the document. If the container requires any additional pre-build ###
### commands, uncomment and edit the PREBUILD line at the end of the ###
### document. ###

name: Dev Container Build and Deploy
on:
workflow_dispatch:
pull_request:
Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Slightly nervous having this automatically deploy on pull request could lead to race conditions as to what is in dev1 at any given time. I'm open to giving it a shot though and if we find it confusing we can switch to just workflow_dispatch: for dev1

Copy link
Contributor Author

@cabutlermit cabutlermit Mar 6, 2026

Choose a reason for hiding this comment

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

I hear your concern on this and would be happy to revisit in the future. So far, we haven't seen a problem with DataEng applications and this automatic deploy on PR, but that doesn't mean that this might not become a problem in the future.

Copy link
Member

@JPrevost JPrevost Mar 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think for this lambda it's probably fine.

Do we actually run deploys for PRs opened from renovate and the like or are we somehow limiting to human PRs? If it's just humans, I have no real worry. If bots can overwrite my dev1, it's eventually gonna happen :)

Copy link
Contributor Author

@cabutlermit cabutlermit Mar 6, 2026

Choose a reason for hiding this comment

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

The shared workflows (in the repository) have this conditional:

    # This line adds a check for the user which is requesting the PR. As long as its not dependabot, we go ahead and run it. 
    if: ${{ github.triggering_actor != 'dependabot[bot]' }}

So, it will not build on PRs from dependabot. I have not addressed Renovate skips yet!

Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Gotcha. Once I activate renovate on this I'll PR to that or open a ticket.

branches:
- main
paths-ignore:
- '.github/**'

permissions:
id-token: write
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

overly broad permissions [zizmor:zizmor/excessive-permissions]

contents: read

jobs:
prep:
name: Prep for Build
runs-on: ubuntu-latest
outputs:
cpuarch: ${{ steps.setarch.outputs.cpuarch }}
steps:
- name: Checkout
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

credential persistence through GitHub Actions artifacts [zizmor:zizmor/artipacked]

uses: actions/checkout@v5

- name: Set CPU Architecture
id: setarch
run: |
echo "### :abacus: Architecture Selection" >> $GITHUB_STEP_SUMMARY
if [[ -f .aws-architecture ]]; then
ARCH=$(cat .aws-architecture)
echo "\`$ARCH\` was read from \`.aws-architecture\` and passed to the deploy job." >> $GITHUB_STEP_SUMMARY
Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Ah, this may be why we generate a separate arch_tag file... we need different values in the docker tag than in this parameter here. (I have another comment elsewhere that I'm choosing to leave even though I think this explains it just so you can see what was confusing when reviewing the workflow)

Copy link
Contributor Author

@cabutlermit cabutlermit Mar 6, 2026

Choose a reason for hiding this comment

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

There is likely a better way to do this, but it does come down to the fact that we need two different formats to track the architecture. Annoying.

else
ARCH="linux/amd64"
echo "No \`.aws-architecture\` file, so default \`$ARCH\` was passed to the deploy job." >> $GITHUB_STEP_SUMMARY
fi
if [[ "$ARCH" != "linux/arm64" && "$ARCH" != "linux/amd64" ]]; then
echo "$ARCH is INVALID architecture!"
echo "$ARCH is INVALID architecture!" >> $GITHUB_STEP_SUMMARY
exit 1
fi
echo "cpuarch=$ARCH" >> $GITHUB_OUTPUT

deploy:
needs: prep
name: Dev Deploy
uses: mitlibraries/.github/.github/workflows/ecr-multi-arch-deploy-dev.yml@main
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

secrets unconditionally inherited by called workflow [zizmor:zizmor/secrets-inherit]

secrets: inherit
with:
AWS_REGION: "us-east-1"
GHA_ROLE: "timdex-semantic-builder-gha-dev"
ECR: "timdex-semantic-builder-dev"
CPU_ARCH: ${{ needs.prep.outputs.cpuarch }}
FUNCTION: "timdex-semantic-builder-dev"
# PREBUILD:
57 changes: 57 additions & 0 deletions .github/workflows/prod-deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
### This is the Terraform-generated prod-promote.yml workflow for the ###
### timdex-semantic-builder-prod repository. ###
### If this is a Lambda repo, uncomment the FUNCTION line at the end of ###
### the document. ###

name: Prod Container Promote
on:
workflow_dispatch:
release:
types: [published]
Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Non-blocking:

Longterm I'd like to talk about how we might have an emergency rollback option to get back to a clean deploy if something unexpected goes wrong. This may add some level of risk and uncertainty as to what is in a given tier at any time, but it adds a huge safety net that allow us to get back to a working state rapidly.

One way might be to have a way to deploy the previous-latest tag from ECR

Copy link
Contributor Author

@cabutlermit cabutlermit Mar 6, 2026

Choose a reason for hiding this comment

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

This will definitely take some thinking and conversation. Infra has ways to do this via the Console/CLI, but I'm pretty sure that the permissions we grant (in this case, to Timdex_Managers) don't provide rights to do this.


permissions:
id-token: write
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

overly broad permissions [zizmor:zizmor/excessive-permissions]

contents: read

jobs:
prep:
name: Prep for Promote
runs-on: ubuntu-latest
outputs:
cpuarch: ${{ steps.setarch.outputs.cpuarch }}
steps:
- name: Checkout
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

credential persistence through GitHub Actions artifacts [zizmor:zizmor/artipacked]

uses: actions/checkout@v5

- name: Set CPU Architecture
id: setarch
run: |
echo "### :abacus: Architecture Selection" >> $GITHUB_STEP_SUMMARY
if [[ -f .aws-architecture ]]; then
ARCH=$(cat .aws-architecture)
echo "\`$ARCH\` was read from \`.aws-architecture\` and passed to the deploy job." >> $GITHUB_STEP_SUMMARY
else
ARCH="linux/amd64"
echo "No \`.aws-architecture\` file, so default \`$ARCH\` was passed to the deploy job." >> $GITHUB_STEP_SUMMARY
fi
if [[ "$ARCH" != "linux/arm64" && "$ARCH" != "linux/amd64" ]]; then
echo "$ARCH is INVALID architecture!"
echo "$ARCH is INVALID architecture!" >> $GITHUB_STEP_SUMMARY
exit 1
fi
echo "cpuarch=$ARCH" >> $GITHUB_OUTPUT

deploy:
needs: prep
name: Deploy
uses: mitlibraries/.github/.github/workflows/ecr-multi-arch-promote-prod.yml@main
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

secrets unconditionally inherited by called workflow [zizmor:zizmor/secrets-inherit]

secrets: inherit
with:
AWS_REGION: "us-east-1"
GHA_ROLE_STAGE: timdex-semantic-builder-gha-stage
GHA_ROLE_PROD: timdex-semantic-builder-gha-prod
ECR_STAGE: "timdex-semantic-builder-stage"
ECR_PROD: "timdex-semantic-builder-prod"
CPU_ARCH: ${{ needs.prep.outputs.cpuarch }}
FUNCTION: "timdex-semantic-builder-prod"

60 changes: 60 additions & 0 deletions .github/workflows/stage-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
### This is the Terraform-generated stage-build.yml workflow for the ###
### timdex-semantic-builder-stage app repository. ###
### If this is a Lambda repo, uncomment the FUNCTION line at the end of ###
### the document. If the container requires any additional pre-build ###
### commands, uncomment and edit the PREBUILD line at the end of the ###
### document. ###

name: Stage Container Build and Deploy
on:
workflow_dispatch:
push:
branches:
- main
paths-ignore:
- '.github/**'

permissions:
id-token: write
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

overly broad permissions [zizmor:zizmor/excessive-permissions]

contents: read

jobs:
prep:
name: Prep for Build
runs-on: ubuntu-latest
outputs:
cpuarch: ${{ steps.setarch.outputs.cpuarch }}
steps:
- name: Checkout
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

credential persistence through GitHub Actions artifacts [zizmor:zizmor/artipacked]

uses: actions/checkout@v5

- name: Set CPU Architecture
id: setarch
run: |
echo "### :abacus: Architecture Selection" >> $GITHUB_STEP_SUMMARY
if [[ -f .aws-architecture ]]; then
ARCH=$(cat .aws-architecture)
echo "\`$ARCH\` was read from \`.aws-architecture\` and passed to the deploy job." >> $GITHUB_STEP_SUMMARY
else
ARCH="linux/amd64"
echo "No \`.aws-architecture\` file, so default \`$ARCH\` was passed to the deploy job." >> $GITHUB_STEP_SUMMARY
fi
if [[ "$ARCH" != "linux/arm64" && "$ARCH" != "linux/amd64" ]]; then
echo "$ARCH is INVALID architecture!"
echo "$ARCH is INVALID architecture!" >> $GITHUB_STEP_SUMMARY
exit 1
fi
echo "cpuarch=$ARCH" >> $GITHUB_OUTPUT

deploy:
needs: prep
name: Stage Deploy
uses: mitlibraries/.github/.github/workflows/ecr-multi-arch-deploy-stage.yml@main
Copy link

@qltysh qltysh bot Mar 5, 2026

Choose a reason for hiding this comment

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

secrets unconditionally inherited by called workflow [zizmor:zizmor/secrets-inherit]

secrets: inherit
with:
AWS_REGION: "us-east-1"
GHA_ROLE: "timdex-semantic-builder-gha-stage"
ECR: "timdex-semantic-builder-stage"
CPU_ARCH: ${{ needs.prep.outputs.cpuarch }}
FUNCTION: "timdex-semantic-builder-stage"
# PREBUILD:
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,6 @@ dmypy.json
# SAM
.aws-sam/
tests/sam/env.json

# For multi-arch deploys
.arch_tag
65 changes: 64 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
SHELL=/bin/bash
DATETIME:=$(shell date -u +%Y%m%dT%H%M%SZ)

### This is the Terraform-generated header for timdex-semantic-builder-dev. If
### this is a Lambda repo, uncomment the FUNCTION line below
### and review the other commented lines in the document.
ECR_NAME_DEV := timdex-semantic-builder-dev
ECR_URL_DEV := 222053980223.dkr.ecr.us-east-1.amazonaws.com/timdex-semantic-builder-dev
CPU_ARCH ?= $(shell cat .aws-architecture 2>/dev/null || echo "linux/amd64")
FUNCTION_DEV := timdex-semantic-builder-dev
### End of Terraform-generated header

help: # Preview Makefile commands
@awk 'BEGIN { FS = ":.*#"; print "Usage: make <target>\n\nTargets:" } \
/^[-_[:alpha:]]+:.?*#/ { printf " %-15s%s\n", $$1, $$2 }' $(MAKEFILE_LIST)

# ensure OS binaries aren't called if naming conflict with Make recipes
.PHONY: help venv install update test coveralls lint black mypy ruff safety lint-apply black-apply ruff-apply
.PHONY: help venv install update test coveralls lint black mypy ruff safety lint-apply black-apply ruff-apply check-arch dist-dev publish-dev update-lambda-dev docker-clean

##############################################
# Python Environment and Dependency commands
Expand Down Expand Up @@ -79,3 +88,57 @@ sam-http-ping: # Send curl command to SAM HTTP server
curl --location 'http://localhost:3000/myapp' \
--header 'Content-Type: application/json' \
--data '{"msg":"in a bottle"}'

####################################
# Deployment to ECR
####################################
### Terraform-generated Developer Deploy Commands for Dev environment ###
check-arch:
@ARCH_FILE=".aws-architecture"; \
if [[ "$(CPU_ARCH)" != "linux/amd64" && "$(CPU_ARCH)" != "linux/arm64" ]]; then \
echo "Invalid CPU_ARCH: $(CPU_ARCH)"; exit 1; \
fi; \
if [[ -f $$ARCH_FILE ]]; then \
echo "latest-$(shell echo $(CPU_ARCH) | cut -d'/' -f2)" > .arch_tag; \
else \
echo "latest" > .arch_tag; \
fi

dist-dev: check-arch ## Build docker container (intended for developer-based manual build)
@ARCH_TAG=$$(cat .arch_tag); \
docker buildx inspect $(ECR_NAME_DEV) >/dev/null 2>&1 || docker buildx create --name $(ECR_NAME_DEV) --use; \
docker buildx use $(ECR_NAME_DEV); \
docker buildx build --platform $(CPU_ARCH) \
--load \
--tag $(ECR_URL_DEV):$$ARCH_TAG \
--tag $(ECR_URL_DEV):make-$$ARCH_TAG \
--tag $(ECR_URL_DEV):make-$(shell git describe --always) \
--tag $(ECR_NAME_DEV):$$ARCH_TAG \
.

publish-dev: dist-dev ## Build, tag and push (intended for developer-based manual publish)
Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Non-blocking:

I wonder if we could get a permissions check. I intentionally ran this without credentials and was surprised to see it try to do a lot of work before failing due to lack of credentials. I quick fail would be nice longterm, but clearly not needed now as it is very minor.

Works with expected role.

Copy link
Contributor Author

@cabutlermit cabutlermit Mar 6, 2026

Choose a reason for hiding this comment

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

yeah... publish-dev starts by re-running dist-dev before attempting to connect to AWS. I'm 100% on board to rethinking these commands, but maybe not for this PR?

Copy link
Member

@JPrevost JPrevost Mar 6, 2026

Choose a reason for hiding this comment

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

Definitely not for this pr, and thanks for pointing out what was happening was the dist-dev. We probably don't need to rethink this at all with that in mind.

@ARCH_TAG=$$(cat .arch_tag); \
aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin $(ECR_URL_DEV); \
docker push $(ECR_URL_DEV):$$ARCH_TAG; \
docker push $(ECR_URL_DEV):make-$$ARCH_TAG; \
docker push $(ECR_URL_DEV):make-$(shell git describe --always); \
echo "Cleaning up dangling Docker images..."; \
docker image prune -f --filter "dangling=true"

## If this is a Lambda repo, uncomment the two lines below
update-lambda-dev: ## Updates the lambda with whatever is the most recent image in the ecr (intended for developer-based manual update)
@ARCH_TAG=$$(cat .arch_tag); \
aws lambda update-function-code \
--region us-east-1 \
--function-name $(FUNCTION_DEV) \
--image-uri $(ECR_URL_DEV):make-$$ARCH_TAG

docker-clean: ## Clean up Docker detritus generated by dist-dev and publish-dev
@ARCH_TAG=$$(cat .arch_tag); \
echo "Cleaning up Docker leftovers (containers, images, builders)"; \
docker rmi -f $(ECR_URL_DEV):$$ARCH_TAG; \
docker rmi -f $(ECR_URL_DEV):make-$$ARCH_TAG; \
docker rmi -f $(ECR_URL_DEV):make-$(shell git describe --always) || true; \
docker rmi -f $(ECR_NAME_DEV):$$ARCH_TAG || true; \
docker buildx rm $(ECR_NAME_DEV) || true
@rm -rf .arch_tag
Loading