Skip to content

run our docker ci on gha runners directly #17442

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

Open
wants to merge 36 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
b7dd6e8
run our docker ci on gha runners directly
ewdurbin Jan 17, 2025
d29ccec
Merge branch 'main' into testing_gha_runners
ewdurbin Jan 17, 2025
ec51e8e
Merge branch 'main' into testing_gha_runners
ewdurbin Jan 18, 2025
982e544
Merge branch 'main' into testing_gha_runners
ewdurbin Jan 18, 2025
2113620
Merge branch 'main' into testing_gha_runners
ewdurbin Jan 18, 2025
06975e6
Merge branch 'main' into testing_gha_runners
ewdurbin Jan 21, 2025
9a599e0
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 5, 2025
d278fc6
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 6, 2025
ed3d594
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 13, 2025
7732972
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 18, 2025
238674d
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 20, 2025
6899045
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 21, 2025
c1ea5d7
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 23, 2025
4aba426
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 24, 2025
e709ff0
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 26, 2025
9a4efa3
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 27, 2025
81da1c5
Merge branch 'main' into testing_gha_runners
ewdurbin Feb 28, 2025
61cd4bb
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 2, 2025
a01b85b
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 12, 2025
343c2e4
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 14, 2025
0bcef8b
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 18, 2025
bafae25
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 18, 2025
4b5a227
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 18, 2025
8487add
Merge branch 'main' into testing_gha_runners
ewdurbin Mar 18, 2025
a9e6ff6
Merge branch 'main' into testing_gha_runners
ewdurbin Apr 2, 2025
860cfb2
Merge branch 'main' into testing_gha_runners
ewdurbin Apr 2, 2025
c9bf802
Merge branch 'main' into testing_gha_runners
ewdurbin Apr 9, 2025
6199059
Merge branch 'main' into testing_gha_runners
ewdurbin Apr 18, 2025
bdf6654
Merge branch 'main' into testing_gha_runners
ewdurbin Apr 18, 2025
e2ba2d3
Merge branch 'main' into testing_gha_runners
ewdurbin May 5, 2025
354cdcb
Merge branch 'main' into testing_gha_runners
ewdurbin May 7, 2025
e01cc94
Merge branch 'main' into testing_gha_runners
ewdurbin May 9, 2025
8a1642e
chore: pin hashes
miketheman May 9, 2025
9410d55
chore: ignore cache poisoning as the artifact is not reused
miketheman May 9, 2025
f326430
nit: use non-legacy syntax
miketheman May 9, 2025
eceaa17
Merge branch 'main' into testing_gha_runners
ewdurbin May 9, 2025
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
65 changes: 46 additions & 19 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,57 @@ concurrency:
cancel-in-progress: true
jobs:
build:
if: github.repository == 'pypi/warehouse'
runs-on: depot-ubuntu-24.04-arm
runs-on: ubuntu-24.04-arm
outputs:
buildId: ${{ steps.build.outputs.build-id}}
buildId: ${{ github.run_id }}
permissions:
id-token: write
packages: write
steps:
- name: Check out repository
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
with:
persist-credentials: false
- name: Set up Depot CLI
uses: depot/setup-action@b0b1ea4f69e92ebf5dea3f8713a1b0c37b2126a5 # v1.6.0
- name: Build image
id: build
uses: depot/build-push-action@636daae76684e38c301daa0c5eca1c095b24e780 # v1.14.0
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@b5ca514318bd6ebac0fb2aedd5d36ec1b5c232a2 # v3.10.0
- name: Cache
uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3 # zizmor: ignore[cache-poisoning]
id: cache
with:
save: true
path: |
var-cache-apt
var-lib-apt
root-cache-pip
root-npm
key: cache-${{ hashFiles('Dockerfile') }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
key: cache-${{ hashFiles('Dockerfile') }}
key: cache-${{ hashFiles('Dockerfile') }}
lookup-only: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }}

https://github.com/actions/cache/blob/5a3ec84eff668545956fd18022155c47e93e2684/README.md?plain=1#L78

- name: inject cache into docker
uses: reproducible-containers/buildkit-cache-dance@653a570f730e3b9460adc576db523788ba59a0d7 # v3.2.0
with:
cache-map: |
{
"var-cache-apt": "/var/cache/apt",
"var-lib-apt": "/var/lib/apt",
"root-cache-pip": "/root/.cache/pip",
"root-npm": "/root/.npm"
}
skip-extraction: ${{ steps.cache.outputs.cache-hit }}
- name: Login To GHCR
uses: docker/login-action@74a5d142397b4f367a81961eba4e8cd7edddf772 # v3.4.0
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Build and push
uses: docker/build-push-action@14487ce63c7a62a4a324b0bfb37086795e31c6c1 # v6.16.0
with:
context: .
cache-from: type=gha
cache-to: type=gha,mode=max
Copy link
Member

Choose a reason for hiding this comment

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

I think we need something like:

Suggested change
cache-to: type=gha,mode=max
cache-to: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository && '' || 'type=gha,mode=max' }}

Otherwise action runs on PRs from untrusted forks can overwrite this cache as well.

(cc @woodruffw, should zizmor flag this?)

Copy link
Member

Choose a reason for hiding this comment

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

Huh yeah, it should -- I'm actually confused as to why it didn't, since I have a coordinate for the push: true part of this:

https://github.com/woodruffw/zizmor/blob/9790cec2727f04fee3cbaef1725f910c114e343e/src/audit/cache_poisoning.rs#L183-L186

Copy link
Member

Choose a reason for hiding this comment

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

Oh never mind, I see why it didn't: it considers docker/build-push-action a "publish sink" but not a "cache source" at the moment, since I'm only detecting push: true and not the cache-to: ... field.

This is a shortcoming in the current audit, I'll file an issue 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Awesome, thanks! Just to confirm, this suggestion resolves the issue, correct?

Copy link
Member

Choose a reason for hiding this comment

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

I think so, although I'm murky on whether doing the same for cache-from is also needed: that's a sink rather than a source so I think it's okay, but I might be missing something.

Copy link
Member

Choose a reason for hiding this comment

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

I think we specifically want cache-from to always be permitted here, so PRs can take advantage of our central cache to speed up builds (but can't write to it).

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense!

push: true
build-args: |
DEVEL=yes
CI=yes
tags: pypi/warehouse:ci-${{ github.run_id }}
tags: |
ghcr.io/pypi/warehouse:ci-${{ github.run_id }}
test:
# Time out if our test suite has gotten hung
timeout-minutes: 15
Expand All @@ -45,10 +74,8 @@ jobs:
include:
- name: Tests
command: bin/tests --postgresql-host postgres
runs_on: depot-ubuntu-24.04-arm-4
- name: Lint
command: bin/lint
runs_on: depot-ubuntu-24.04-arm-4
- name: User Documentation
command: bin/user-docs
- name: Developer Documentation
Expand All @@ -59,13 +86,13 @@ jobs:
command: bin/licenses
- name: Translations
command: bin/translations
runs-on: ${{ (matrix.runs_on != null) && matrix.runs_on || 'depot-ubuntu-24.04-arm' }}
runs-on: ubuntu-24.04-arm
container:
image: registry.depot.dev/rltf7cln5v:${{ needs.build.outputs.buildId }}
image: ghcr.io/pypi/warehouse:ci-${{ needs.build.outputs.buildId }}
env:
BILLING_BACKEND: warehouse.subscriptions.services.MockStripeBillingService api_base=http://stripe:12111 api_version=2020-08-27
permissions:
id-token: write
packages: read
services:
postgres:
image: ${{ (matrix.name == 'Tests') && 'postgres:16.1' || '' }}
Expand Down Expand Up @@ -103,12 +130,12 @@ jobs:
check_db:
name: Check Database Consistency
needs: build
runs-on: depot-ubuntu-24.04-arm
runs-on: ubuntu-24.04-arm
continue-on-error: true
container:
image: registry.depot.dev/rltf7cln5v:${{ needs.build.outputs.buildId }}
image: ghcr.io/pypi/warehouse:ci-${{ needs.build.outputs.buildId }}
permissions:
id-token: write
packages: read
services:
postgres:
image: postgres:16.1
Expand Down
4 changes: 2 additions & 2 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ RUN --mount=type=cache,target=/root/.cache/pip \
FROM python:${PYTHON_IMAGE_VERSION}

# Setup some basic environment variables that are ~never going to change.
ENV PYTHONUNBUFFERED 1
ENV PYTHONPATH /opt/warehouse/src/
ENV PYTHONUNBUFFERED=1
ENV PYTHONPATH=/opt/warehouse/src/
ENV PATH="/opt/warehouse/bin:${PATH}"

WORKDIR /opt/warehouse/src/
Expand Down