Skip to content

fix: land the stranded dependency-update branch (MAPCO-11478) - #4

Merged
NivGreenstein merged 7 commits into
masterfrom
fix/land-ci-dependency-updates
Aug 25, 2026
Merged

fix: land the stranded dependency-update branch (MAPCO-11478)#4
NivGreenstein merged 7 commits into
masterfrom
fix/land-ci-dependency-updates

Conversation

@NivGreenstein

@NivGreenstein NivGreenstein commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Closes MAPCO-11478.

What this does

Lands the single real commit from the stranded chore/ci-security-updates branch, by cherry-pick, not merge.

That branch read as four commits ahead, but the content of the three older ones was already on master (verified: tms/errors.go is byte-identical between master and 5eb6988b). Only 87144cdf was new. A plain merge would have replayed the other three.

Commit What
fix: make pull request workflow pass The cherry-pick. Dependency bumps + vendor/, the Coveralls empty-token guard, the CITE port, go: true on both lambda jobs, and brittle MVT byte-size assertions replaced with decoded-tile assertions.
fix(devcontainer): track the go.mod toolchain and the shigola rename Required by the cherry-pick: the image pinned Go 1.26.2 and go.mod moves to 1.26.6.
fix(devcontainer): resolve compose paths from the project directory The devcontainer has been unbuildable since the rebrand.
ci: make the CITE port genuinely overridable SHIGOLA_PORT was referenced but never defined.
docs: record why the coverage guard and the compose file order exist Both are load-bearing and neither is self-evident.
fix(build): track the go.mod toolchain in the image and lambda builds Two more hardcoded Go pins; one broke docker build, the other would have broken the next release.

2055 files, but 2048 are vendor/ — the mechanical go mod vendor result of the go.mod bump. The seven non-vendor files in that commit are the whole of the human change. go.mod, go.sum and vendor/ must move as one unit or every -mod vendor build breaks.

Acceptance criteria

Criterion Status
Single real commit applied by cherry-pick, not merge; already-merged commits do not reappear
The test job passes on master ✅ green on CI
The vulnerability job passes on master ✅ green on CI
The coverage upload skips cleanly when no token is available ✅ verified under act — logs Skipping Coveralls upload: no GitHub token is available, job succeeds
The conformance workflow still passes ✅ green on CI
The stranded branch is deleted once its content is on master ⏳ after merge

Three fixes beyond the cherry-pick

The CITE port override never worked. 87144cdf passes TEGOLA_PORT: ${{ env.SHIGOLA_PORT }} to two steps, but SHIGOLA_PORT is defined nowhere in the repo. As a GitHub expression it renders to the empty string, so the override was unreachable — it only behaved because run.sh reads ${TEGOLA_PORT:-8081}, and :- treats empty as unset.

Worth recording the fix that doesn't work, since review caught it: a workflow-level env: SHIGOLA_PORT: '8081' default. Workflow-level env takes precedence over the runner's process environment, so SHIGOLA_PORT=9081 act … still gets the literal — it moves the hardcode rather than removing it. What landed instead: both consumers resolve the variable from the environment with the same shell expansion, and the two per-step rename blocks are gone. run.sh now reads SHIGOLA_PORT falling back to the pre-rename TEGOLA_PORT — the SHIGOLA_*-over-TEGOLA_* precedence internal/env.Getenv already uses. Verified: unset → 8081, SHIGOLA_PORT=9081 → 9081, TEGOLA_PORT=7081 → 7081, both → 9081.

Three files hardcoded the Go version. go.mod sets no toolchain directive, so a lower Go fails with go.mod requires go >= 1.26.6 rather than fetching one, and the build image sets GOTOOLCHAIN=local:

  • .devcontainer/Dockerfilegolang:1.26.2-bookworm
  • Dockerfile:30golang:1.26.2-alpine3.23, which broke docker build
  • .github/actions/amazon-linux-build-action/DockerfileGOLANG_VERSION=1.26.2, which had not fired yet but feeds both lambda jobs in on_release_publish.yml and would have broken the next release

That last one verifies a pinned checksum, so the hash moved with it: 708effb7…feef89, taken from the go.dev release feed and confirmed against the downloaded tarball.

Nothing ties these three to go.mod, so the next Go bump breaks them again. A toolchain directive would make them self-healing but changes module semantics for consumers — its own ticket.

The devcontainer was unbuildable. Beyond the Go pin: context: .. and ..:/workspace/shigola resolved one level above the repo — the build died with lstat .../shigola/.devcontainer: no such file or directory, and the mount put both repos at /workspace/shigola, so go.mod was not found. Relative paths resolve against the compose project directory (the first -f file's directory), which is the repo root in every invocation including devcontainer.json's own ordering. Also workspaceFolder: /workspace/shigola vs working_dir: /workspace/tegola, a rebrand leftover.

The rename half overlaps MAPCO-11504. Happy to drop it if you would rather keep that ticket whole.

Review findings deferred, not fixed

A two-axis review flagged these. All are inherited from 87144cdf or pre-existing:

  • assertMVTForLayers is byte-identical in provider/hana/hana_test.go and provider/postgis/postgis_internal_test.go. Different packages, so it cannot be shared locally — internal/ttools is the natural home. It also asserts layer order, which MVT does not guarantee.
  • on_release_publish.yml's go: falsego: true (both lambda jobs) is real — the unconditional Set shigola version step runs go run, so those jobs died with go: command not found — but it is uncommented and unmentioned in the cherry-picked commit body.
  • govulncheck@latest is unpinned, so that job can flip without a commit. MAPCO-11499.
  • f7955b0d bundles five unrelated changes. Inherited; it is precisely why the follow-up commits are separate.

Verification

CI is green on all workflows. Locally, in the devcontainer (Go 1.26.6, PostGIS + Redis as compose siblings), both CGO modes:

  • gofmt -s clean, go build -mod vendor ./... OK, go vet clean on cache/ and tms/
  • docker build completes and the binary runs with cgo and all providers linked in
  • Coveralls guard exercised under act with an empty token, since secrets.GITHUB_TOKEN is always injected on GitHub-hosted runners and the guard can never fire there

One note for anyone developing on Apple Silicon: TestValidateTileInGrid/a_zoom_beyond_the_scheme's_matrices and TestTile/a_matrix_the_scheme_does_not_have fail on arm64 while passing on CI's amd64 runners. They fail identically on master and on the stranded branch tip, and both date from the initial commit cd2f2488, so this is pre-existing and unrelated to this PR — but it is worth a look, since a local go test ./... on a Mac is red before you change anything. TestPGXOnNotice also fails in a devcontainer because it hardcodes 127.0.0.1:5432; that one is a sandbox artifact and passes when postgis shares the network namespace.

🤖 Generated with Claude Code

NivGreenstein and others added 5 commits August 25, 2026 09:40
Update vulnerable dependencies and make the local CI workflow resilient to missing credentials and occupied ports. Replace brittle MVT byte-size checks with decoded tile assertions.
The image pinned golang:1.26.2-bookworm to match go.mod. The preceding commit
moves go.mod to go 1.26.6 and there is no toolchain directive, so the container
could no longer build the module.

The workspace path was also still tegola: devcontainer.json declared
workspaceFolder /workspace/shigola while the compose file mounted the repo at
/workspace/tegola and both it and the Dockerfile set that as the working
directory. Plain docker compose worked, but the devcontainer flow opened into a
path that did not exist.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
context: .. and the workspace volume ..:/workspace/shigola both resolved one
level above the repo. The build failed with "lstat .../shigola/.devcontainer:
no such file or directory", and the mount put the directory holding both
shigola and shigola-docs at /workspace/shigola, so go.mod was not found.

Putting this file first in the -f list fixes those but then resolves the root
compose file's ./testdata mount under .devcontainer/, so neither ordering
worked. Relative paths resolve against the project directory -- the first -f
file's directory -- which is the repo root in both invocations, including
devcontainer.json's list where ../docker-compose.yml comes first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
87144cd passed TEGOLA_PORT: ${{ env.SHIGOLA_PORT }} to the two suite steps,
but SHIGOLA_PORT was defined nowhere, so the expression rendered to the empty
string. It only behaved because run.sh reads ${TEGOLA_PORT:-8081}, and :-
treats empty as unset.

Declaring a workflow-level env: SHIGOLA_PORT default does not fix it: that
takes precedence over the runner's process environment, so SHIGOLA_PORT=9081
act ... would still get the literal. It moves the hardcode rather than removing
it.

Instead let both consumers resolve the variable from the environment with the
same shell expansion, and drop the two per-step rename blocks. run.sh now reads
SHIGOLA_PORT falling back to the pre-rename TEGOLA_PORT -- the SHIGOLA_*-over-
TEGOLA_* precedence internal/env.Getenv already uses -- so setting the variable
works whether the suite is driven by the workflow or invoked directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are load-bearing and neither is self-evident. The Coveralls guard never
fires on GitHub-hosted runs, because secrets.GITHUB_TOKEN is always injected
there, so nothing in the file said who it was for. The dockerComposeFile order
is what makes the sibling compose file's relative build context and workspace
mount resolve against the repo root.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Coverage Report for CI Build 9918

Warning

No base build found for commit 66b602b on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 47.662%

Details

  • Patch coverage: Could not be determined — this PR's diff is too large for GitHub to return (406 error at GitHub).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 20163
Covered Lines: 9610
Line Coverage: 47.66%
Coverage Strength: 205.92 hits per line

💛 - Coveralls

NivGreenstein and others added 2 commits August 25, 2026 10:12
go.mod moved to go 1.26.6 and sets no toolchain directive, so a lower Go fails
with "go.mod requires go >= 1.26.6" rather than fetching one -- and the build
image sets GOTOOLCHAIN=local. The release Dockerfile pinned golang:1.26.2 and
broke the Docker build; the amazon-linux action pinned the same version and
would have broken both lambda jobs on the next release.

The action verifies a pinned checksum, so the tarball hash moves with it:
708effb7...feef89, from https://go.dev/dl and confirmed against the downloaded
tarball.

Three files now hardcode this version and nothing ties them to go.mod. A
toolchain directive would make them self-healing, but it changes module
semantics for consumers, so that belongs in its own ticket.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The workflow's serve step resolved ${SHIGOLA_PORT:-8081} while run.sh resolved
${SHIGOLA_PORT:-${TEGOLA_PORT:-8081}}. Setting only the legacy TEGOLA_PORT --
which run.sh still honours -- bound the server on 8081 while the suite polled
the other port, and the only symptom was the readiness poll timing out after
60s. Give both sides the same expression.

internal/env.Getenv warns when it falls back to a TEGOLA_ name, so run.sh now
warns too; a silent fallback is worst in exactly the place someone is still
using the old name.

Also finish the rename in three comment lines the earlier pass missed, point
each of the three hardcoded Go versions at the other two, and end the compose
context comment on what is true now rather than on the rejected alternative.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@NivGreenstein
NivGreenstein force-pushed the fix/land-ci-dependency-updates branch from e9cb03a to 041059a Compare August 25, 2026 07:33
@NivGreenstein
NivGreenstein merged commit e792b91 into master Aug 25, 2026
33 checks passed
@NivGreenstein
NivGreenstein deleted the fix/land-ci-dependency-updates branch August 25, 2026 07:42
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.

1 participant