Skip to content

Commit 4e51089

Browse files
committed
Verify the geo base on pull requests instead of publishing from CI
The workflow both verified the geo base and published it to Docker Hub. The publishing half needed credentials the repository does not hold, and it made one image the only one CI ships, ahead of any decision about the rest. Publishing is now a maintainer's act, done out of band. What remains is the check that nothing else performs: the gdal and pdal pins in pyproject.toml are compiled against the libraries this image provides, and no workflow builds the application image, where the equivalent guard lives. A mismatch breaks every backend build, so the check runs on pull requests, where it can still stop one, rather than after the merge. Nothing privileged is left in the workflow, so the ref guard goes with the push step, and fork pull requests can run it safely.
1 parent 510a930 commit 4e51089

3 files changed

Lines changed: 53 additions & 65 deletions

File tree

.github/workflows/geo-base.yml

Lines changed: 46 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,77 @@
1-
name: Build geo base image
1+
name: Verify geo base image
22

33
# Builds the native geospatial stack (GDAL, PDAL, PROJ, GEOS, Untwine) that
4-
# backend.dockerfile builds on top of. This takes upwards of an hour, so it runs
5-
# only when geobase.dockerfile changes or when triggered by hand -- the
6-
# application image build does not compile any of it.
4+
# backend.dockerfile builds on top of, then checks the built image against the
5+
# declarations in backend/pyproject.toml and smoke-tests both trees it ships.
6+
# This takes upwards of an hour, so it runs only when one of the inputs below
7+
# changes -- the application image build does not compile any of it.
78
#
8-
# Pushing to main only builds and smoke-tests; it does not publish. Publishing is a
9-
# deliberate act: run this workflow from the Actions tab against main with push set
10-
# to true. Dispatching from any other ref refuses to publish -- the tag is mutable
11-
# and every backend build consumes it, so anything published here reaches production
12-
# images immediately.
9+
# This workflow does not publish. gdslab/d2s-geo-base:latest is pushed by hand by
10+
# a maintainer: the tag is mutable and every backend build consumes it, so
11+
# anything published reaches production images immediately, and that is not a
12+
# decision to automate for one image ahead of a strategy for the rest.
1313
#
14-
# The geospatial declarations in backend/pyproject.toml describe the same libraries
15-
# this image provides, so republishing after a version change and updating those
16-
# declarations belong in the same pull request.
14+
# It runs on pull requests because that is where the answer is worth having. The
15+
# geospatial declarations in backend/pyproject.toml describe the same libraries
16+
# this image provides, since the Python bindings are compiled against them, and a
17+
# mismatch breaks every backend build. Nothing else checks that: no workflow here
18+
# builds the application image, where the equivalent guard lives. Learning about a
19+
# bad pin after the merge is learning too late.
1720

1821
on:
19-
workflow_dispatch:
20-
inputs:
21-
push:
22-
description: Push to Docker Hub after the smoke test passes
23-
type: boolean
24-
default: false
25-
push:
26-
branches: [main]
22+
pull_request:
2723
paths:
2824
- backend/geobase.dockerfile
2925
# geobase.dockerfile.dockerignore decides what the build can see,
3026
# fetch-source.sh is copied into it, and verify_geo_stack.sh is the smoke test
3127
# below -- each one changes what this workflow produces or what it proves.
32-
# Without them a change that quietly defeated the checksum guard would reach
33-
# main unbuilt and unverified. These are literal paths rather than prefixes:
28+
# Without them a change that quietly defeated the checksum guard would merge
29+
# unbuilt and unverified. These are literal paths rather than prefixes:
3430
# geobase.dockerfile does not match geobase.dockerfile.dockerignore.
3531
- backend/geobase.dockerfile.dockerignore
3632
- backend/scripts/fetch-source.sh
3733
- backend/scripts/verify_geo_stack.sh
3834
- backend/scripts/check_geo_pins.sh
3935
# pyproject.toml is not a build input; it is what the declaration check below
4036
# compares the built image against, so a pin-only change has to run this too.
41-
# Nothing builds the application image in CI, so without this entry a pin moved
42-
# on its own reaches main with nothing having checked it.
4337
- backend/pyproject.toml
4438
- .github/workflows/geo-base.yml
39+
# For verifying the stack outside a pull request -- checking a rebuild of a
40+
# published image, or confirming main is sound before pushing the tag by hand.
41+
workflow_dispatch:
42+
43+
permissions:
44+
contents: read
45+
46+
# An hour of compiling per push to a pull request branch is worth spending once.
47+
# A new commit makes the run in flight answer a question nobody is asking any more.
48+
concurrency:
49+
group: ${{ github.workflow }}-${{ github.ref }}
50+
cancel-in-progress: true
4551

4652
env:
53+
# Local to the runner. Nothing here pushes it; the name matches what
54+
# backend.dockerfile expects so the smoke tests exercise the real thing.
4755
IMAGE: gdslab/d2s-geo-base:latest
4856

4957
jobs:
50-
build:
58+
verify:
5159
runs-on: ubuntu-latest
5260
timeout-minutes: 180
5361
steps:
54-
# workflow_dispatch can be run from any ref, and the tag this publishes is
55-
# mutable and consumed by every backend build. Dispatching from a feature
56-
# branch with push set would ship unmerged GDAL/PDAL libraries into production
57-
# images. This fails rather than skipping the push, so a run that cannot
58-
# publish says so instead of finishing green and looking like it did.
59-
- name: Refuse to publish from a non-main ref
60-
if: inputs.push && github.ref != 'refs/heads/main'
61-
run: |
62-
echo "::error::Refusing to publish ${{ env.IMAGE }} from ${{ github.ref }}." \
63-
"Merge to main and dispatch from there."
64-
exit 1
65-
6662
- uses: actions/checkout@v4
6763

6864
- uses: docker/setup-buildx-action@v3
6965

70-
# mode=max is deliberate. Publishing is two steps -- a push to main builds and
71-
# verifies, then a dispatch from main publishes -- and only a full layer cache
72-
# makes that second run anything other than another hour of compiling. mode=min
73-
# would export just the final stage, which is a COPY on top of ubuntu, so the
74-
# expensive build stage would not be cached at all. Nothing else in this repo
75-
# uses the Actions cache, so the 10 GB budget is not contended.
66+
# Cache scope on a pull request is the pull request, so the first run here
67+
# compiles everything and later pushes to the same branch reuse it. That is
68+
# the case worth optimising: reviewing a geo change usually means several
69+
# commits, and only the first should cost an hour.
70+
#
71+
# mode=max is what makes that true. mode=min exports only the final stage,
72+
# which is a COPY on top of ubuntu, so the expensive build stage would not be
73+
# cached at all. Nothing else in this repo uses the Actions cache, so the
74+
# 10 GB budget is not contended.
7675
- name: Build
7776
uses: docker/build-push-action@v6
7877
with:
@@ -86,8 +85,8 @@ jobs:
8685
# The application build refuses to compile bindings against a geo base whose
8786
# versions disagree with pyproject.toml -- but nothing builds the application
8887
# image in CI, so that guard never runs here. Without this step, bumping
89-
# GDAL_VERSION and forgetting the pin passes every check above, publishes the
90-
# mutable tag, and breaks every backend build from then on.
88+
# GDAL_VERSION and forgetting the pin passes every check above and breaks
89+
# every backend build from the merge onwards.
9190
#
9291
# Same script the application build runs, so the two cannot disagree about what
9392
# the invariant is. It takes seconds; it runs before the smoke tests so a
@@ -112,8 +111,8 @@ jobs:
112111
# and the application's build stage compiles against. It is not what ships:
113112
# backend.dockerfile copies /opt/geo-runtime, stripped and pruned, into the
114113
# final image. Checking only the first would let a bad strip or an over-broad
115-
# rm -rf in the prune step reach production undetected. GEO_PREFIX has to move
116-
# with PATH, or the linkage sweep would read the tree that was already checked.
114+
# rm -rf in the prune step through undetected. GEO_PREFIX has to move with
115+
# PATH, or the linkage sweep would read the tree that was already checked.
117116
- name: Smoke test (runtime tree)
118117
run: |
119118
docker run --rm \
@@ -127,14 +126,3 @@ jobs:
127126
-v "$PWD/backend/scripts:/scripts:ro" \
128127
-v "$PWD/backend/app/tests/data:/app/app/tests/data:ro" \
129128
${{ env.IMAGE }} bash /scripts/verify_geo_stack.sh --cli-only
130-
131-
- name: Log in to Docker Hub
132-
if: inputs.push && github.ref == 'refs/heads/main'
133-
uses: docker/login-action@v3
134-
with:
135-
username: ${{ secrets.DOCKERHUB_USERNAME }}
136-
password: ${{ secrets.DOCKERHUB_TOKEN }}
137-
138-
- name: Push
139-
if: inputs.push && github.ref == 'refs/heads/main'
140-
run: docker push ${{ env.IMAGE }}

backend/backend.dockerfile

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ COPY pyproject.toml uv.lock ./
7575
# link bindings and library at different versions without complaint.
7676
#
7777
# The comparison lives in scripts/check_geo_pins.sh rather than inline here, because
78-
# .github/workflows/geo-base.yml runs the same check against the image it is about to
79-
# publish. Two copies would drift, and this is the one invariant the whole split
80-
# between the two images rests on.
78+
# .github/workflows/geo-base.yml runs the same check against the geo base it builds.
79+
# Two copies would drift, and this is the one invariant the whole split between the
80+
# two images rests on.
8181
COPY --chmod=0755 scripts/check_geo_pins.sh /usr/local/bin/check-geo-pins
8282
RUN check-geo-pins /opt/geo/VERSIONS pyproject.toml
8383

backend/scripts/check_geo_pins.sh

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
# check_geo_pins.sh <VERSIONS-file> <pyproject.toml>
77
#
88
# Two callers, one copy. backend.dockerfile runs it against the image it is about to
9-
# compile bindings against; .github/workflows/geo-base.yml runs it against the image it
10-
# is about to publish. Those are the two directions the coupling can break:
9+
# compile bindings against; .github/workflows/geo-base.yml runs it against a geo base
10+
# built from the pull request. Those are the two directions the coupling can break:
1111
#
1212
# - a pin moves without the image -- caught at application build time
13-
# - the image moves without a pin -- caught in CI, before a mutable tag is published
14-
# and every later backend build starts failing
13+
# - the image moves without a pin -- caught on the pull request, before the change
14+
# merges and every later backend build starts failing
1515
#
1616
# Keeping the comparison in one file is the point. Two copies of it would drift, and the
1717
# drift would stay invisible until a build broke somewhere unrelated to the change.

0 commit comments

Comments
 (0)