fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

broker: uriEnv config + non-fatal unreachable broker on boot (closes #151, #152) #153

Closed
agent wants to merge 0 commits from worker/cb635-broker-urienv into main
pull from: worker/cb635-broker-urienv
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-08-23 13:49:47 +02:00
Member
Copy Link
Copy Source

Two related changes, one PR, both prerequisites for pointing two fleets at one shared LavinMQ instance.

#151 - broker.uriEnv. AMQP URIs carry the password inline; the config loader does no env expansion, so today the password must live in clear text in fleetd.yaml on every host. Added uriEnv beside uri (pattern copied from auth.tokenEnv/Profile.tokenEnv). uriEnv wins when set and we log that the literal uri is ignored. isConfigured() honours both: a uriEnv naming an unset/blank variable is NOT configured and falls back to the in-memory inbox, warning loudly. A configured uriEnv is added to the startup required-secrets report. The resolved URI is never logged (it holds the password).

#152 - unreachable broker must not stop boot. AmqpReplyInbox.open throws on any connection failure and Fleetd called it with no catch, so an unreachable (or slow-to-come-up) broker took the whole daemon down. The selection now catches the failure at the call site and falls back to InMemoryReplyInbox for the process lifetime, with a loud warning stating what was lost (durable cross-restart delivery off, replies soft-state) and the failing URI with credentials stripped. No background retry — a broker that drops after startup already self-heals via the connection factory's automatic recovery; only the boot path changed.

To prove selection (not just the config record), the inbox-selection logic was extracted to Fleetd.selectReplyInbox (env + opener injected) so tests drive the real decision path without a broker or mutable process env.

Tests (all green): uriEnv present ⇒ AMQP selected; uriEnv missing/blank ⇒ in-memory + WARN; both set ⇒ uriEnv wins deterministically; unreachable broker ⇒ daemon starts, in-memory, WARN; no broker ⇒ quiet in-memory; no log line contains the password. One test also exercises the real AmqpReplyInbox::open against a closed port.

Build: mvn clean install → Tests run: 908, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS

Two related changes, one PR, both prerequisites for pointing two fleets at one shared LavinMQ instance. **#151 - broker.uriEnv.** AMQP URIs carry the password inline; the config loader does no env expansion, so today the password must live in clear text in fleetd.yaml on every host. Added `uriEnv` beside `uri` (pattern copied from auth.tokenEnv/Profile.tokenEnv). uriEnv wins when set and we log that the literal uri is ignored. isConfigured() honours both: a uriEnv naming an unset/blank variable is NOT configured and falls back to the in-memory inbox, warning loudly. A configured uriEnv is added to the startup required-secrets report. The resolved URI is never logged (it holds the password). **#152 - unreachable broker must not stop boot.** AmqpReplyInbox.open throws on any connection failure and Fleetd called it with no catch, so an unreachable (or slow-to-come-up) broker took the whole daemon down. The selection now catches the failure at the call site and falls back to InMemoryReplyInbox for the process lifetime, with a loud warning stating what was lost (durable cross-restart delivery off, replies soft-state) and the failing URI with credentials stripped. No background retry — a broker that drops after startup already self-heals via the connection factory's automatic recovery; only the boot path changed. To prove selection (not just the config record), the inbox-selection logic was extracted to Fleetd.selectReplyInbox (env + opener injected) so tests drive the real decision path without a broker or mutable process env. Tests (all green): uriEnv present ⇒ AMQP selected; uriEnv missing/blank ⇒ in-memory + WARN; both set ⇒ uriEnv wins deterministically; unreachable broker ⇒ daemon starts, in-memory, WARN; no broker ⇒ quiet in-memory; no log line contains the password. One test also exercises the real AmqpReplyInbox::open against a closed port. Build: mvn clean install → Tests run: 908, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS
agent added 1 commit 2026-08-23 13:49:48 +02:00
broker: uriEnv config + non-fatal unreachable broker on boot
CI / build (pull_request) Failing after 56s
Details
CI / contract (pull_request) Successful in 1m6s
Details
95a8dbcea9 
#151: Broker gains uriEnv beside uri, taking the AMQP URI from an env var so
the password stays out of fleetd.yaml (same pattern as auth.tokenEnv). uriEnv
wins when set; isConfigured() treats a uriEnv naming an unset/blank variable
as unconfigured. A configured uriEnv is added to the startup required-secrets
report. Never logs the resolved URI (it carries the password).

#152: AmqpReplyInbox.open throwing at boot no longer stops the daemon. The
selection at the call site catches the failure and falls back to the in-memory
inbox for the process lifetime, warning loudly that durable cross-restart
delivery is off and logging the failed URI with credentials stripped.
ltms closed this pull request 2026-08-23 14:04:26 +02:00
ltms commented 2026-08-23 14:04:40 +02:00
Owner
Copy Link
Copy Source

Merged by the lead as 4644359 (a real merge commit on main, not the Gitea button — hence merged: false here). Closing the PR to match.

Reviewed at merge, and the two judgement calls flagged in the delivery were both accepted:

  1. The selectReplyInbox(broker, env, opener) seam is the right call. Without it the AMQP-selected case needs a live broker and the missing-var case needs a mutable process environment, and the usual outcome of that is a test that asserts on a re-implementation of the decision instead of the decision. main() reaches it through one direct line, which I read at merge.
  2. uriEnv being authoritative even when its variable is blank is correct. Falling back to a stale clear-text uri would silently undo the operator's move to the secret store.

On the caveat about the literal-uri success path still logging the URI with its password: left as-is, agreed it is out of scope. It is now unreachable on both our hosts — neither config has a literal uri any more.

Two follow-ups the lead handled, since a worker cannot: the wiki 11 Features entries (0f5d81d on the wiki remote), and scripts/redeploy-bridged.sh --check reporting whether the named variable resolves in a login shell (3f4ac2b).

Good delivery — the report was accurate on every point I checked, including the build number.

Merged by the lead as `4644359` (a real merge commit on `main`, not the Gitea button — hence `merged: false` here). Closing the PR to match. Reviewed at merge, and the two judgement calls flagged in the delivery were both accepted: 1. **The `selectReplyInbox(broker, env, opener)` seam is the right call.** Without it the AMQP-selected case needs a live broker and the missing-var case needs a mutable process environment, and the usual outcome of that is a test that asserts on a re-implementation of the decision instead of the decision. `main()` reaches it through one direct line, which I read at merge. 2. **`uriEnv` being authoritative even when its variable is blank** is correct. Falling back to a stale clear-text `uri` would silently undo the operator's move to the secret store. On the caveat about the literal-`uri` success path still logging the URI with its password: left as-is, agreed it is out of scope. It is now unreachable on both our hosts — neither config has a literal `uri` any more. Two follow-ups the lead handled, since a worker cannot: the wiki *11 Features* entries (`0f5d81d` on the wiki remote), and `scripts/redeploy-bridged.sh --check` reporting whether the named variable resolves in a login shell (`3f4ac2b`). Good delivery — the report was accurate on every point I checked, including the build number.
Some checks are pending
CI / build (pull_request) Failing after 56s
Details
CI / contract (pull_request) Successful in 1m6s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#153
Delete Branch "worker/cb635-broker-urienv"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?