mirror of
https://github.com/mattermost/mattermost.git
synced 2026-11-02 20:31:01 -06:00
Jesse Hallam
8f6d17d875
* Tag the build server image on its Node version
mattermost-build-server was published as :${GO_VERSION}, but the image
bakes NODE_VERSION into PATH at build time. Since setup-buildenv skips
the build whenever `docker manifest inspect` finds that tag, bumping the
Dockerfile's Node changed nothing: CI kept pulling the existing image,
and the new Node only appeared once a master merge overwrote the tag in
place -- which also silently changes what release branches pinned to
that tag resolve to.
Tag on both instead, e.g. go1.26.7-node24.11.1. A Node bump now names a
tag nobody has published, so it gets built and published exactly once,
the same way a Go bump does. The "go" prefix keeps these distinct from
the bare Go versions published under the old scheme.
server/build/buildenv-tag.sh is the one place that composes the tag, and
it verifies both Dockerfiles agree on Node since one tag covers both
images. CI resolves it where it already resolved the Go version and
propagates BUILD_SERVER_TAG, so the buildenv actions just take a string.
That leaves nothing needing a threaded Go version except the
actions/setup-go in server-test-template, which reads server/.go-version
via go-version-file. Only two of that template's seven callers shard at
all, so the rest were passing a value nothing read.
Node also moves to 24.11.1 to match .nvmrc, which is what the webapp's
engines constraint has expected for a while.
The buildenv job in server-ci.yml is renamed from "go" to match
server-ci-weekly and server-ci-nightly-race, so its check name changes
from "Compute Go Version" to "Build Environment".
* Fail the step when buildenv-tag.sh fails
`echo BUILD_SERVER_TAG=$(./buildenv-tag.sh)` reports echo's exit status,
not the script's, so under `bash -e` a failing script left the step green
and wrote an empty value:
$ bash -e -c 'echo "TAG=$(false)"; echo continued'
TAG=
continued
That defeated the consistency check the script exists for: on drift it
exits 1, the step passed anyway, and setup-buildenv went on to inspect
`mattermost-build-server:` with no tag -- failing well away from the
cause. Capture into a variable first, which `bash -e` does catch.
Applies to all nine call sites, including the ones already quoted;
quoting doesn't help here.
* Rebuild the buildenv image when a version pin changes
buildenv-tag.sh reads server/.go-version and .nvmrc, so name them in the
paths that trigger a republish.
Redundant in practice -- the script requires the pins and the Dockerfiles
to agree, so a valid pin change always carries a Dockerfile change, which
already triggers this -- but it makes the dependency explicit and fails
here rather than in Server CI.
* Pass buildenv inputs via env, not expression interpolation
GitHub expands `${{ }}` into the script before bash runs it, so
`echo "BUILDENV_TAG=${{ inputs.build-server-tag }}"` executed whatever
the input contained. That was reachable: buildenv-tag.sh reads
ARG NODE_VERSION straight out of a Dockerfile, and a fork PR controls
that file, so `ARG NODE_VERSION=$(...)` would have landed in the tag and
then in this script.
Pass all three inputs through env instead, and reject a tag that isn't
usable as a docker tag anyway.
Also fixes the aggregate's error message, which still named the 'go' job
after it was renamed to 'buildenv'.
* Don't write a literal expression in a shell comment
GitHub parses ${{ ... }} anywhere in the template, including inside a
run: block's comment text, so an empty one failed the action manifest:
"An expression was expected".
* Drop the redundant tag validation
The env mapping is the fix: bash gets the input as a value, never as
script text, so there is nothing left to inject with. The regex only
restated what docker would reject anyway.
* Validate the tag before emitting it
* Pass buildenv values as step outputs, not GITHUB_ENV
2 lines
8 B
Plaintext
2 lines
8 B
Plaintext
24.11.1
|