ci: re-enable the server Docker image build on stable releases - #1208
Open
Dumbris wants to merge 3 commits into
Open
ci: re-enable the server Docker image build on stable releases#1208Dumbris wants to merge 3 commits into
Dumbris wants to merge 3 commits into
Conversation
The `build-docker` job has been gated behind `if: false` since the Teams work; the server edition it builds now ships, so nothing is holding it back. Re-enable it and make `release` depend on it, so a stable tag never publishes binaries without a matching container image. Deviations from the issue's proposal, and why: - The guard is `startsWith(github.ref, 'refs/tags/v') && !contains(github.ref_name, '-')`, not the bare tag check. release.yml fires on every `v*` tag including RCs, and the job pushes `:latest` unconditionally — an RC tag must not move that pointer. This matches the guard `build`/`release` already use. - The image is now multi-arch (`linux/amd64,linux/arm64`). The release already ships an arm64 Linux binary, and container users on Raspberry Pi / Graviton would otherwise be stuck building it themselves — the exact problem the issue reports. The Dockerfile cross-compiles from `$BUILDPLATFORM` via `$TARGETARCH`, so this is a second native build rather than QEMU emulation. - Added a build-only `docker-image` job to PR Build. Now that `release` hard-depends on `build-docker`, a Dockerfile regression would otherwise first surface at tag time and block the release; this catches it on the PR instead, and smoke-tests the entrypoint. Verified locally: both the single-arch and the multi-arch build succeed, the amd64 image reports `(server) linux/amd64`, `/healthz` returns 200, and `/ui/` serves the real embedded Vue bundle (not the fallback stub). Closes #1171 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
7c394f7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d0d43f34.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://claude-github-issue-1171-62d.mcpproxy-docs.pages.dev |
cmd/release-gate's TestPublishJobsGatedOnQAGate caught this on the first CI run: the audit skips jobs that can never run (`if: false`), so un-gating build-docker made it visible as a publisher for the first time — and its `needs: [build]` closure did not reach the qa-gate job. A tag whose release qualification gate failed would still have pushed an image to GHCR, `:latest` pointer included. `needs: [build, qa-gate]` closes it. No cycle: release -> build-docker -> qa-gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 33874436167 --repo smart-mcp-proxy/mcpproxy-go
|
Two things a deep audit of the server edition turned up that bear directly on this PR: 1. Flipping `if: false` is not sufficient to close #1171. A GHCR package first created by GITHUB_TOKEN is PRIVATE by default, and this job had no labels and no visibility step. An anonymous registry probe today returns 403 for mcpproxy-server — identical to the control probe for a name that does not exist — while all five scanner sidecars return 200. So after the first stable tag, the `docker run` command this PR adds to the install docs would fail with `denied` for the person who filed the issue. `org.opencontainers.image.source` links the package to this repo, which is a prerequisite for the package page and README, but it does NOT make it public — that is a one-time manual flip in the org's package settings after the first push. The comment says so, so the next person does not have to rediscover it. 2. The install docs now warn about the data-dir traps. `MCPPROXY_DATA` and a bare `--data-dir` both silently wipe the persisted config on every boot (the override is applied after config resolution, so the config in the data dir is never read and is then overwritten with a fresh default, rotating the API key). `MCPPROXY_DATA_DIR`, documented elsewhere as the preferred form, has no implementation at all. Fixing the loader ordering is a separate change; documenting the trap is not. Also clarified that state dies on container REPLACEMENT, not restart — verified: `docker restart` on the same container reads back the same key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1171
build-dockerhas been gated behindif: falsesince the Teams work. The server edition it builds now ships, so this re-enables the job and makesreleasedepend on it — a stable tag no longer publishes binaries without a matching container image.What changed
.github/workflows/release.ymlbuild-dockerun-gated;platforms: linux/amd64,linux/arm64;releasenowneeds: build-dockerDockerfile$BUILDPLATFORMvia$TARGETOS/$TARGETARCH(no QEMU).github/workflows/pr-build.ymldocker-imagejob + entrypoint smoke testdocs/getting-started/installation.mddocs/mcp-registry-publishing.mdif: false" noteDeviations from the issue's proposal
startsWith(github.ref, 'refs/tags/v'). release.yml fires on everyv*tag including RCs, and the job pushes:latestunconditionally — an RC must not move that pointer. This matches the guardbuild/releasealready use. (Belt-and-suspenders:buildis stable-only too, so an RC would skip-cascade anyway.)releasehard-depends onbuild-docker, a Dockerfile regression would first surface at tag time and block a release. The new job catches it on the PR instead.Verification (local)
docker build— succeeds.docker buildx build --platform linux/amd64,linux/arm64— succeeds; log shows[linux/arm64->amd64 builder], i.e. cross-compiled, no QEMU.--platform linux/amd64:mcpproxy version→MCPProxy v0.0.0-multi (server) linux/amd64;/healthz→ 200;/ui/serves the real embedded Vue bundle (asset 200, 437 KB), not the fallback stub.X-API-Key, 200 with;MCPPROXY_API_KEYenv override works; state lands in/root/.mcpproxy(config.db, index.bleve, mcp_config.json).--entrypoint mcpproxy— verified that without it, theversionarg is swallowed by theserveentrypoint and the container just runs the server.actionlintfinding count is byte-identical before/after on both workflows (54 and 10, all pre-existing).Defect caught by CI (fixed in the second commit)
The first CI run failed
cmd/release-gate'sTestPublishJobsGatedOnQAGate, and it was a genuine hole in the issue's proposed change — not a flake:Fixed with
needs: [build, qa-gate]onbuild-docker. No cycle (release→build-docker→qa-gate). The guard test now passes; it demonstrably bites, since it is what found this.