Repository navigation
ops: bundled Mosquitto opt-in, normalised DISABLE_MOSQUITTO, pprof loopback default (#441) - #444
Merged
Merged
Conversation
… pprof
Red on origin/master:
- test-fixtures/disable-mosquitto-cases.json: the one DISABLE_MOSQUITTO
table (unset/empty, true/1/yes/on, false/0/no/off in any case and with
surrounding whitespace, junk) that every reader is held to.
- test-issue-441-disable-mosquitto.js: drives the shipped
docker/entrypoint-go.sh and manage.sh over that table (selected
supervisord config, broker-check mode, the compose file dc_prod runs,
junk warning, no launcher-mismatch warning), plus the wizard default and
.env.example.
- cmd/ingestor/broker_mode_test.go: ResolveBrokerMode over the same table;
junk is a warning that names the default; recognised values are quiet.
- cmd/server/compose_optin_441_test.go: every app compose file renders
DISABLE_MOSQUITTO=true when unset; the staging files publish pprof on
${PPROF_BIND:-127.0.0.1} with the in-container listener pinned to
0.0.0.0; files that publish nothing do not widen it; docs upgrade note.
- internal/pprofbind: loopback default, wildcard override (shared and per
role), URL names localhost for an unspecified host.
- The #372 tests and the non-root entrypoint harnesses move to the new
defaults.
Relates to #441
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… loopback (#441) Owner decisions on the #431 "Left for the owner" items: 1. DISABLE_MOSQUITTO defaults to true in docker-compose.yml, docker-compose.example.yml, the image entrypoint, manage.sh (and its setup wizard, which now defaults to "no") and .env.example. 2. The entrypoint, manage.sh (parse_disable_mosquitto) and the ingestor (ResolveBrokerMode) read it the same way: trimmed, case-insensitive, true/1/yes/on and false/0/no/off; unset/empty/anything else is the default, and junk is reported. The launcher-mismatch diagnostics are removed; broker-check prints the effective mode. 3. pprofbind defaults the server and ingestor listeners to 127.0.0.1 (PPROF_HOST=0.0.0.0 or * widens them). The staging compose files publish 6060/6061 on ${PPROF_BIND:-127.0.0.1} and pin the in-container listener to 0.0.0.0, since docker's proxy cannot reach a loopback listener; PPROF_BIND=0.0.0.0 restores remote access. Ports stay distinct. 4. MQTT_SOURCE_STRICT unchanged (off). docs/deployment.md gains an "Upgrading past #441" note with the release text and the SSH tunnel; DEPLOY.md, README and .env.example follow. Relates to #441 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
Rapport — CS-pve-agent1 PR#444 #441 — head 32e2202Status: Implemented; all four #441 decisions have red-before/green-after tests, 27/27 mutants killed, Evidence tags: [T] = I ran it and saw the result. [A] = analysis of code or docs, not executed. [K] = known from CI or GitHub metadata. Branch Requirements → test → mutant"Red before" means the test fails with this branch's tests and
27 of 27 mutants were killed and none survived [T]. #372 tests whose expectations moved with the owner decisions: 5 JS cases and
|
| Compose file | Master vs head |
|---|---|
docker-compose.yml (bare compose) |
Identical. Only noise differs: the embedded DNS resolver's random port and the status sample age. Same supervisord-no-mosquitto.conf, same [config] internal MQTT broker disabled (DISABLE_MOSQUITTO=true) line, the ingestor connects and subscribes to the external broker, healthz 200 |
docker-compose.no-mosquitto.yml (what manage.sh runs for true) |
Identical, same noise only |
docker-compose.staging.yml |
Broker side identical. pprof differs by design (item 3): host publish 0.0.0.0+[::] → 127.0.0.1 on 6060/6061, and the container env gains PPROF_HOST=0.0.0.0. The in-container listener is unchanged (:::6060, :::6061) |
The intended changes, observed in the real images [T]
DISABLE_MOSQUITTO in the busybox entrypoint (DISABLE_CADDY=true, mosquitto process present or not):
| Value | master | head |
|---|---|---|
unset, '' |
runs | off |
false, False, ' off ', 0, no |
runs | runs |
true |
off | off |
TRUE, YES, 1, on |
runs (the old gap) | off |
ture, y |
runs | off + "not a recognised value" line |
pprof:
| Setup | Result |
|---|---|
Staging compose (head), PPROF_BIND unset |
docker port 127.0.0.1:6060/6061; 200 on loopback, refused via the host's external address |
Staging compose (head), PPROF_BIND=0.0.0.0 |
0.0.0.0:6060/6061; 200 via the external address |
Container with no published port, ENABLE_PPROF=true |
master :::6060 (reachable from another container); head 127.0.0.1:6060, 200 inside, refused from another container; with PPROF_HOST=0.0.0.0 back to :::6060, reachable |
Bare corescope-server, ENABLE_PPROF=true |
127.0.0.1:6060; external address refused. With PPROF_HOST=0.0.0.0: *:6060, 200 via the external address |
Suites (local, head) [T]
cd cmd/server && go vet ./... && go test ./...→ okcd cmd/ingestor && go vet ./... && go test ./...→ okcd internal/pprofbind && go vet ./... && go test -race ./...→ oksh test-all.sh→ 246 passed, 0 failed (incl. the newtest-issue-441-disable-mosquitto.js, 11/11, andtest-issue-372-broker-check.js, 23/23)node test-frontend-helpers.js→ 713 passed, 0 failed- E2E: a local Go server on
e2e-fixture.dbprepared as in CI (freshen, inline seed SQL,corescope-migrate, seeds 2073/199/245), started withENABLE_PPROF=true.test-e2e-playwright.js→ 151/154 passed, 3 skipped, 0 failed./api/healthz,/api/health,/api/stats,/api/mqtt/status,/api/nodes,/api/observers,/api/packets→ 200. The server was stopped by the pid holding the port. No frontend file changed, so no other E2E is affected bash scripts/check-xss-sinks.sh --diff origin/master→ "no public/**/*.{js,html} changes to scan"shellcheckon both changed non-root harnesses: clean. Ondocker/entrypoint-go.sh, only the existing SC1091 info. Onmanage.sh: 17 findings on master and 17 on head, none newscripts/check-dockerfile-internal-pkgs.sh→ ok;gofmt -lclean on every touched Go file
Always-checks [T]
cmd/serverchanges only a test file (compose_optin_441_test.go), so it stays read-only, and the invariant test passes in the suite- 0 new
map[string]interface{}outside tests (git diff origin/master -- '*.go' ':!*_test.go') - No
public/**change, so no colours - Fork guards unchanged: 9 in
deploy.yml, 1 inrelease-fast-path.yml; no.github/change - No closing keywords in title, body or commits; the body starts with
Relates to #441
CI per job (run 38520401105, attempt 1) [K]
| Job | Result |
|---|---|
| ✅ Go Build & Test | pass (16m), including bash scripts/test-nonroot-entrypoint.sh in the real Alpine base (ok 260 - off after on, clean volume: shipped config, i.e. an unset value now selects the no-mosquitto config under busybox) and test-issue-441-disable-mosquitto.js 11/11 |
| 🎭 Playwright E2E Tests (1/3) | pass (9m15s) |
| 🎭 Playwright E2E Tests (2/3) | pass (8m49s) |
| 🎭 Playwright E2E Tests (3/3) | pass (9m37s) |
| 📊 Verify E2E Shards & Coverage | pass |
| 🧱 Validate Docker Image | pass |
| 🏗️ Build & Publish Docker Image (gate) | pass |
| 📦 Release Artifacts / 🚀 Deploy Staging / 📝 Publish Badges | skipped (fork guards, as expected) |
The known flake (#271) did not fire, so nothing was rerun.
Remaining / for the owner
- Staging pprof narrows on the next pull. The staging compose files hard-code
ENABLE_PPROF=true, so a host profiled remotely needsPPROF_BIND=0.0.0.0in its.envbefore it pulls this, or an SSH tunnel. I did not touch, read or change any deployment [A]. PPROF_BINDvsPPROF_HOST(design choice).PPROF_BINDrestores remote access wherever pprof is published (the staging files), as ops: make the bundled Mosquitto opt-in, normalise DISABLE_MOSQUITTO, pprof loopback by default (#431 owner decisions) #441 asks. For a bare binary, adocker run -p …:6060, or reach from another container, the knob isPPROF_HOST=0.0.0.0. I did not makepprofbindreadPPROF_BIND, because a value meant for the publish would then bind the container's loopback and kill the published port (the ops(runtime): make bundled MQTT and pprof exposure explicit settings (#372) #431 separation). The PR description explains this; a reviewer may prefer otherwise.PPROF_BIND=0.0.0.0publishes IPv4 only (the ops(runtime): harden bundled MQTT and opt-in pprof defaults #372 finding). The old dual-stack publish has no one-line restore under a:-127.0.0.1default, and the docs say so. I did not test an IPv6 publish value.y/nwere accepted bymanage.sh(andnby the ingestor) before; they are now junk → defaulttrue, reported. This follows ops: make the bundled Mosquitto opt-in, normalise DISABLE_MOSQUITTO, pprof loopback by default (#431 owner decisions) #441's value list literally.docs/specs/deployment-simplification.mdstill shows the historical${DISABLE_MOSQUITTO:-false}. I left it alone as a dated spec, not operator docs.scripts/test-docker-nonroot.sh(local-only, needs a built image) was updated in step with the CI harness but not run here; it was shellchecked [T].- No staging or production access, no server API key, no upstream writes.
Collaborator
Author
Review — CS-MacBook PR#444 — head 32e2202Dom: APPROVE with nits. One #441 item is implemented differently from the issue's wording and needs the owner's explicit sign-off (F1). Evidence tags: [T] = I ran it and saw the result. [A] = analysis of code or docs, not executed. [K] = known from CI or GitHub metadata. Independent, read-only review. I checked Findings
No blocking correctness, security or scope problems found. #441, item by item
Local Docker:
|
| Compose file | Master vs head |
|---|---|
docker-compose.yml |
Identical apart from noise: the embedded DNS resolver port, MQTT client ID and connect timestamp. Both log [config] internal MQTT broker disabled (DISABLE_MOSQUITTO=true), both connect and subscribe to the external broker, healthz 200 |
docker-compose.no-mosquitto.yml |
Identical, same noise only |
docker-compose.staging.yml |
Broker side identical. The intended pprof difference only: PPROF_HOST "" → 0.0.0.0, and the publish gains host_ip: 127.0.0.1. The in-container sockets are the same (Go's 0.0.0.0 is the same dual-stack wildcard as :6060) |
pprof reachability [T]. "LAN" means curl against the host's own LAN address:
| Setup | Published | Loopback | LAN | Another container | Host -p mapping |
|---|---|---|---|---|---|
| master staging compose | 0.0.0.0 + [::] |
200 | 200 | — | — |
head staging compose, PPROF_BIND unset |
127.0.0.1 |
200 | refused | — | — |
head staging compose, PPROF_BIND=0.0.0.0 |
0.0.0.0 |
200 | 200 | — | — |
head staging compose, PPROF_BIND=127.0.0.1 |
127.0.0.1 |
200 | refused | — | — |
master image, ENABLE_PPROF=true |
— | — | — | 200 | 200 |
| head image, default | — | — | — | refused | refused |
head image, PPROF_BIND=0.0.0.0 |
— | — | — | refused (F1) | refused (F1) |
head image, PPROF_HOST=0.0.0.0 |
— | — | — | 200 | 200 |
Tests and mutants
On the merged tree (= head tree), against origin/master bcf263e1 [T]:
cd cmd/server && go test ./...→ okcd cmd/ingestor && go test -race ./...→ ok (430 s)cd internal/pprofbind && go test -race -count=1 ./...→ oksh test-all.sh→ 246 passed, 0 failed, includingtest-issue-441-disable-mosquitto.jsandtest-issue-372-broker-check.jsnode test-frontend-helpers.js→ 713 passed, 0 failed
Red before: I ran head's test files against master runtime files [T]:
- JS 441: 0/11
cmd/server: 6 compose/env/docs tests failpprofbind: 7 tests failcmd/ingestor: does not compile
E2E: a local Go server on e2e-fixture.db, prepared as in CI (freshen, the inline seed SQL from deploy.yml, corescope-migrate, seeds 2073, 199 and 245), ENABLE_PPROF=true, port 139xx [T].
test-e2e-playwright.jsstops on its fail-fast at "Version info lives on Perf dashboard, not in navbar" (#navStatswait timeout). The same test fails identically against a master-built server on the same fixture, so it is local to this machine and not caused by this PR.- A scratch copy with fail-fast removed gives 150/154 passed, 3 skipped, 1 failed (the same one) on both master and head, with the identical failure set.
- No
public/**file changed. CI ran all three Playwright shards green on head [K]. - The servers were stopped by the pid holding the port, not
$!.
My own mutants: 7 of 7 killed [T]. Each was applied to a fresh copy of head and run against the relevant suite. All were distinct from the author's 27.
| # | Mutant | Killed by |
|---|---|---|
| X1 | entrypoint: junk value → MOSQUITTO_DISABLED=false |
JS 441 entrypoint table row "ture" |
| X2 | pprofbind: PPROF_HOST=0.0.0.0 silently rewritten to loopback |
TestWildcardHostRestoresEveryInterface, TestPerRoleHostOverridesSharedHost + 2 |
| X3 | docker-compose.staging.no-mosquitto.yml drops PPROF_HOST=0.0.0.0 |
TestStagingComposes_InContainerPprofListenerStaysReachable + 1 |
| X4 | manage.sh parser: no trailing-whitespace trim | JS 441 " false " / " true " via proc rows |
| X5 | ingestor: off removed from the false set |
TestResolveBrokerMode_FollowsTheSharedTable + 1 |
| X6 | manage.sh mosquitto_disabled: junk counts as "enabled" (= true instead of != false) |
JS 441 broker-check / dc_prod rows for "ture" |
| X7 | pprofbind: per-role PPROF_HOST_<ROLE> ignored |
6 pprofbind tests |
Always-checks
- Scope [A]: no scope creep found. Every change follows from ops: make the bundled Mosquitto opt-in, normalise DISABLE_MOSQUITTO, pprof loopback by default (#431 owner decisions) #441: the wizard default is a "script default", and the docs, README and DEPLOY text are aligned.
MQTT_SOURCE_STRICTis unchanged. No new config knob:PPROF_BINDandPPROF_HOSTalready existed. The only new product behaviour beyond the issue's literal text is F1, and it is disclosed. cmd/serverread-only [T]: only a new test file (compose_optin_441_test.go). The read-only invariant test passes in the suite.- No new
map[string]interface{}outside tests [T]: 0 added lines. - No hardcoded colours, XSS gate clean [T]: no
public/**change;scripts/check-xss-sinks.sh --diff origin/masterreports nothing to scan. - Fork guards [T]: unchanged at 9 (
deploy.yml) and 1 (release-fast-path.yml); no.github/change. - No closing keywords [T]: none in commits or body. The body starts with
Relates to #441. - Commit author and committer [T]: both commits (
5d3cde4f,32e22025) are authored and committed bydborup <kontakt@meshview.dk>. - CI per job, run
38520401105attempt 1 [K]:- Go Build & Test, all three Playwright shards, Verify E2E Shards & Coverage, Validate Docker Image and Build & Publish (gate): all pass.
- Release, Deploy Staging and Badges: skipped by the fork guards.
- Known flakes flaky: #226 Hash Stats adopters sort E2E — server sorts multiByteNodes on packets only, ties come out in random order #256 and flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267 did not fire, and no reruns were needed.
Not verified
- An IPv6
PPROF_BINDvalue, and dual-stack publishing viaPPROF_BIND(the docs say it is IPv4-only). - The
manage.shsetup wizard interactively. Only the test harness's stubbed run. scripts/test-docker-nonroot.sh(local-only). I did not run it.scripts/test-nonroot-entrypoint.shin the real Alpine base ran in CI only [K].- The full DISABLE_MOSQUITTO value table inside the real busybox image. I covered it through the JS harness on the shipped entrypoint under macOS
sh[T]; the author's report has the busybox table. - An end-to-end upgrade of an install that relies on the bundled broker with the variable unset. Covered only by the tests and the release-note text [A].
- amd64 images (CI only) [K].
- No staging or production host was accessed. I did not read or use the server API key.
dborup
marked this pull request as ready for review
October 14, 2026 07:58
dborup-agent
pushed a commit
that referenced
this pull request
Oct 14, 2026
A new func Test... in cmd/server no longer needs a hand-written manifest entry. Manifest v2 pins only the slowest entries to shards 1..N-1 (go test -run); shard N is the catch-all and runs everything else with go test -skip of the pinned names. The five #444 tests, and any later ones, run there automatically. The guard now checks that every active Test, Fuzz seed target and Example is selected by exactly one shard, and the aggregator checks that the receipts together ran each active entry once. Stale pins select nothing and are only reported. Patterns over 96 KiB are refused (Linux MAX_ARG_STRLEN is 128 KiB per argv string); with 2/9 pins they are under 0.5 KiB. scripts/rebalance-go-server-shards.js regenerates the manifest from CI go test -json artifacts. The checked-in weights come from the GitHub runners of run 38529224242 attempts 1 and 2 (not local arm64): 2 pins on shard 1, 9 on shard 2, 2,483 entries in the catch-all, projected 319 / 325 / 299 test seconds. AGENTS.md documents when and how to rebalance, and the go-test summary prints the per-shard test seconds. Co-Authored-By: Claude Opus 5.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.
Relates to #441
Implements the owner decisions recorded in #441 on the "Left for the owner" items of #431 (#372).
Plan
DISABLE_MOSQUITTO=true:docker-compose.yml,docker-compose.example.yml, the image entrypoint (so a baredocker runas well),manage.sh(dc_prod/dc_staging, the port preflight and the setup wizard, whose prompt now defaults to "no"), and.env.example. The no-mosquitto and staging files already rendertrue.docker/entrypoint-go.sh,manage.sh(parse_disable_mosquitto) and the ingestor (ResolveBrokerMode) read the value with the same table: trimmed, case-insensitive,true/1/yes/on→ broker off,false/0/no/off→ broker on, unset/empty → the default (off), anything else → the default plus a warning. The table is a shared fixture (test-fixtures/disable-mosquitto-cases.json) that both the JS and the Go tests load, so the three implementations cannot drift apart silently. The launcher-mismatch diagnostics from ops(runtime): make bundled MQTT and pprof exposure explicit settings (#372) #431 are gone on both sides;broker-checkreports the effective mode instead (Broker mode: external (DISABLE_MOSQUITTO=yes, read as true)).internal/pprofbinddefaults the server (6060) and ingestor (6061) listeners to127.0.0.1;PPROF_HOST=0.0.0.0(or*) widens them. The staging compose files publish the ports on${PPROF_BIND:-127.0.0.1}and pin the in-container listener toPPROF_HOST=0.0.0.0, because docker's port proxy cannot reach a loopback listener inside the container. Without that pin,PPROF_BIND=0.0.0.0would publish a dead port. Ports stay distinct. The SSH tunnel is documented.MQTT_SOURCE_STRICTstays off by default. No code change; the existing guard tests plus two mutants pin it.Docs: a new "Upgrading past #441" section in
docs/deployment.md(the release note below, at more length). The #372 migration table, the env-var references indocs/deployment.mdandDEPLOY.md, the MQTT setup and troubleshooting sections, and the READMEdocker runexample (now-e DISABLE_MOSQUITTO=false) are updated to match.Design choice to review:
PPROF_BINDvsPPROF_HOST#441 says "the existing
PPROF_BINDsetting restores remote access".PPROF_BINDis the compose-level publish interface, and #431 deliberately kept it separate from the in-processPPROF_HOST. I kept that separation, so:PPROF_BIND=0.0.0.0alone restores remote access, as ops: make the bundled Mosquitto opt-in, normalise DISABLE_MOSQUITTO, pprof loopback by default (#431 owner decisions) #441 asks.docker run -p …:6060:6060, or another container on the network), the restore knob isPPROF_HOST=0.0.0.0. Makingpprofbindalso readPPROF_BINDwould re-conflate the two layers: aPPROF_BIND=127.0.0.1that reached a container would bind the container's loopback and silently kill the published port.Release note
Deployments with
DISABLE_MOSQUITTO=true: identical before and after (local Docker)Images built from
origin/master(bcf263e1) and from this branch. Each compose file was run withDISABLE_MOSQUITTO=truein.envand a stand-in external broker on a localmeshcore-net. Compared: rendereddocker compose config,docker port, supervisord config and processes, in-container listening sockets, container environment, entrypoint and ingestor log lines (MQTT connect/subscribe),/api/healthzand/api/mqtt/status.docker-compose.yml(bare compose)docker-compose.no-mosquitto.yml(whatmanage.shruns fortrue)docker-compose.staging.yml0.0.0.0+[::]to127.0.0.1on6060/6061, and the container env gainsPPROF_HOST=0.0.0.0. The in-container listener is unchanged (:::6060/:::6061)Heads-up for the owner: the staging compose files hard-code
ENABLE_PPROF=true. A staging host profiled remotely needsPPROF_BIND=0.0.0.0in its.envbefore it pulls this, or it will need an SSH tunnel. Nothing was changed on any deployment, and no staging or production host was accessed.Tests
Every item has a test that is red on
origin/masterand green here, plus mutants. The full table is in the report comment. In short:test-issue-441-disable-mosquitto.js(new, intest-all.sh): drives the shipped entrypoint (absolute paths redirected into a temp dir, stub supervisord) andmanage.sh(stubdocker) over every row of the shared table, via.envand via the process environment. It checks the selected supervisord config,broker-check's mode, the compose filedc_prodruns, the junk warning, the absence of any mismatch warning, the wizard default and.env.example.cmd/ingestor/broker_mode_test.go: the same table forResolveBrokerMode, plus the junk diagnostic and the absence of mismatch diagnostics.cmd/server/compose_optin_441_test.go(new): the opt-in default in every app compose file, the staging pprof publish on loopback, the in-container wildcard where pprof is published, no widening where it is not, and the docs upgrade note.internal/pprofbind: the loopback default, the wildcard override (shared and per role), distinct ports, and the URL.test-issue-372-broker-check.js,compose_bind_372_test.go,pprofbind_test.go) and in the two non-root entrypoint harnesses, which now ask for the bundled broker explicitly where they need it.Checks: no
cmd/serverruntime change (only a test file), so it stays read-only. No newmap[string]interface{}, nopublic/**change (so no colours, andcheck-xss-sinks.sh --diff origin/masterhas nothing to scan). Fork guards unchanged: 9 indeploy.yml, 1 inrelease-fast-path.yml.🤖 Generated with Claude Code