Skip to content

fix(cli)!: wheels deploy requires a bind address on an accessory port - #4078

Merged
bpamiri merged 2 commits into
developfrom
peter/deploy-accessory-port-bind
Oct 2, 2026
Merged

bpamiri merged 2 commits into
developfrom
peter/deploy-accessory-port-bind

Conversation

@bpamiri

@bpamiri bpamiri commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #4067. Breaking (approved by the user for 4.2.0).

Problem

An accessory port: went to docker run --publish unchanged. A bare port: 5432 published on a random host port, and "5432:5432" on every interface (0.0.0.0), with no warning.

Change

  • Validator.$validateAccessoryPort() requires a bind address, matching Kamal main (basecamp/kamal@00cb4cf1d, unreleased; released Kamal ≤ 2.12.0 still passes ports through). Accepted shapes:

    • "<ipv4>:<host port>:<container port>", where the IPv4 address must be four 0–255 octets;
    • "[<ipv6>]:<host port>:<container port>";
    • in both, each port may be a range (8000-8010), and the value may end in /tcp, /udp or /sctp.

    Anything else, including trailing text after the ports, a hostname instead of an IP, 5432 or 5432:5432, fails with an error in the same style as Kamal's:
    accessory db: port "5432" must name a bind address, e.g. "127.0.0.1:5432:5432" to keep it on that host, "10.0.0.20:5432:5432" for a private network address, or "0.0.0.0:5432:5432" to publish it on every interface

  • AccessoryCommands.$portArgs() quotes the published value (shellEscape) like the other docker arguments.

  • Docs:

    • the CLI's wheels deploy docs accessories text and the v4-2-0 accessories guide and config reference describe the rule;
    • the 4.1-to-4.2 upgrade page gets a "Breaking: an accessory port: must name a bind address" bullet;
    • a changed changelog fragment marked Breaking.

Tests

New cli/lucli/tests/specs/deploy/config/AccessoryPortBindSpec.cfc (6 specs):

  • accepted: IPv4, 0.0.0.0, bracketed IPv6, /tcp, ranges, /udp, /sctp, and an accessory with no port:
  • rejected: 5432 (number), "5432", "5432:5432", localhost:…, [nope]:…, 999.1.1.1:…, …/tcp;id, … -v /:/x, …:abc, …/http
  • the message names the accessory and shows the three bound examples for its port

Updated to the bound form: the with-accessories.yml and full.yml fixtures, DeployFollowupsSpec's inline config, and the AccessorySpec / AccessoryCommandsSpec expectations (--publish '127.0.0.1:5432:5432').

RED on develop: the two rejection specs failed (a bare port was accepted). GREEN: PORT=9512 bash tools/test-cli-local.sh gave 2072 pass, 0 fail, 0 error; tools/test-local.sh cli gave 231 passed; a real pnpm build of web/sites/guides exits 0 with the new text on all three pages. Freeze-check clean (no frozen files touched).

Rule #9: no open community PR references #4067.

🤖 Generated with Claude Code

An accessory port: went to docker run --publish as written, so a bare 5432
published on a random host port and 5432:5432 on every interface, with no
warning. Validator.$validateAccessoryPort() now requires
"<ipv4>:<host>:<container>" or "[<ipv6>]:<host>:<container>" (ports or
ranges, optional /tcp, /udp or /sctp), as Kamal main does since
basecamp/kamal@00cb4cf1d, and fails with a message naming the accessory and
bound examples for its port. The --publish value is now quoted like the
other docker arguments. Fixtures, the CLI's accessories doc, the v4-2-0
accessories guide, config reference and 4.1-to-4.2 upgrade page use or
describe the bound form.

BREAKING CHANGE: a bare accessory port (5432) or host:container pair
(5432:5432) now fails validation; give it a bind address.

Fixes #4067

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@github-actions github-actions Bot added the docs label Oct 2, 2026
@bpamiri

bpamiri commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ 266001f: APPROVE

Checked against #4067. The breaking change is user-approved and labelled in the changelog and on the 4.2 upgrade page.

  • Validator. $validateAccessoryPort() runs per accessory. A non-string value fails, and an empty one is skipped.
    • The IPv4 branch needs at least three : fields, a real dotted quad ($isIPv4 checks each octet ≤ 255), and then exactly :<port|range>:<port|range>[/tcp|udp|sctp] anchored at both ends, so a trailing extra field fails.
    • The IPv6 branch needs a […] made of hex digits, colons and dots containing at least one :, followed by the same port tail.
    • A YAML-numeric port: 5432 stringifies and fails with a message that names the accessory and offers the 127.0.0.1, private-address and 0.0.0.0 forms with that container port. The value is trimmed, so the CFML $-before-trailing-LF quirk can't let a value through.
    • The specs cover IPv4 and IPv6 with protocols, ranges plus udp and sctp, a rejected trailing suffix, bare and host:container forms, a hostname bind and the message.
  • Execution. --publish is now shellEscaped, matching the other docker arguments (feat(cli): wheels deploy mounts a top-level volumes list on app containers #4048).
  • Docs. The v4-2-0 accessories caution, the config reference, the CLI's own docs/accessories.md and the fixtures all move to the bound form, and the same-host <service>-<name> guidance is kept.

Non-blocking nits:

  • The docblock says "optionally with "/tcp" or "/udp"", but the pattern (correctly) also accepts /sctp.
  • The config reference says "the same rule Kamal applies". It's the rule on Kamal main (basecamp/kamal@00cb4cf1d), which isn't in a release yet, and released Kamal 2.x passes port through as-is. "As Kamal main requires" would be exact, and it matters for anyone comparing against the Kamal they actually run.

Freeze-check: clean. CI was still pending when I reviewed.

…ay Kamal main

The complexity gate failed: Validator.validate() reached 31 with the new
port check. The per-accessory loop body (name, image, port, volume host
paths, hosts) is now the public $validateAccessory() helper. Review nits
folded in: the port docblock lists /sctp, and the config reference and
changelog say the bind-address rule is Kamal main's (unreleased), not
released Kamal's.

Refs #4067

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

rev1-r3 review @ 2f2a254: APPROVE

The delta since 266001ffd is a behaviour-preserving extraction plus both nits. validate()'s per-accessory loop body moves unchanged into public $validateAccessory(acc, accName, filePath): $validateName still runs before the isStruct early return, and the image, port, volume and directory, and host checks keep their order and labels. That should bring validate() back under the complexity gate. The docblock now lists /sctp, and the changelog and config reference say Kamal main (unreleased). Freeze-check: clean. The Complexity gate job was still pending when I reviewed, and it's the check this delta exists to fix, so confirm it's green before merging.

@bpamiri
bpamiri merged commit 2ad9259 into develop Oct 2, 2026
24 checks passed
@bpamiri
bpamiri deleted the peter/deploy-accessory-port-bind branch October 2, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wheels deploy: a bare accessory port: publishes on a random host port; require a bind address

1 participant