Skip to content

fix(config): treat IPv4-mapped IPv6 as the address it maps to - #18

Draft
Ladas wants to merge 1 commit into
praxis-proxy:mainfrom
Ladas:fix/host-port-binding-followup
Draft

fix(config): treat IPv4-mapped IPv6 as the address it maps to#18
Ladas wants to merge 1 commit into
praxis-proxy:mainfrom
Ladas:fix/host-port-binding-followup

Conversation

@Ladas

@Ladas Ladas commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Why

Follow-ups to #16, found by a review pass after it merged. #16 landed at the
commit that was pushed, so these never made it in. Nothing here is urgent; two
of the four are statements #16 introduced that are simply not true.

The one behaviour change

The host-binding registry compares bind addresses as IpAddr, which
distinguishes them by variant. ::ffff:127.0.0.1 and 127.0.0.1 name the same
interface to the container runtime, but never compared equal.

Measured on main versus this branch, same config, two mappings on host port
8080 written in the two notations:

main        valid
this PR     error: cluster "c1": duplicate host port binding ::ffff:127.0.0.1:8080/tcp

And the collision is real, not theoretical — probed against Docker directly:

docker run -p 127.0.0.1:18099:80 …          bound
docker run -p [::ffff:127.0.0.1]:18099:80 … REFUSED, already allocated

So main accepts a config that then fails at forge up with the opaque "port
is already allocated" the registry exists to prevent. canonical_ip collapses
the mapped form before comparison.

Two tests: the mapped pair now conflicts, and 127.0.0.1 against ::1 still
passes, because those genuinely are two addresses.

Honest scope note: this needs someone to both set bindAddress and use the
IPv4-mapped notation, which is rare. It is cheap insurance with tests, not a
fire.

Two comments that are wrong

  • PortMapping::protocol is documented as "tcp or udp". Since feat: add extraPortMappings support to ClusterSpec #16 the field is
    read by two validators that accept different values: cluster mappings take
    tcp, udp or sctp case-insensitively, matching KIND; service ports take
    tcp only. The doc now says so, and the two protocol checks cross-reference
    each other so the divergence is visible where it is enforced.
  • generate_kind_config claims "KIND only supports port mappings on
    control-plane nodes". KIND accepts extraPortMappings on any node. Using the
    first control-plane is our choice, and a sound one — kube-proxy makes a
    NodePort reachable through every node, and publishing the same host port from
    several nodes would collide — but it is a design decision, not a constraint
    imposed by KIND.

One deduplication

The bind-address parse check was written twice, once for clusters and once for
services, differing only in the error prefix. Folded into one
check_bind_address taking the PortOwner that already renders both prefixes.
Drop this commit if you would rather keep the diff to defects only.

Test plan

  • cargo clippy --all-targets -- -D warnings clean
  • cargo fmt --check, cargo doc --no-deps clean
  • 409 lib tests + 51 integration tests pass
  • the two new tests fail on main and pass here
  • manual: colliding pair rejected, 127.0.0.1/::1 pair still accepted

Follow-ups to praxis-proxy#16, found by review after it merged.

The host-binding registry compares bind addresses as `IpAddr`, which
distinguishes an address by variant. `::ffff:127.0.0.1` and `127.0.0.1`
name the same interface to the container runtime but never compared
equal, so a cluster mapping and a service port using the two notations
for one interface passed validation and then collided at `forge up` with
the opaque "port is already allocated" the registry exists to prevent.
`canonical_ip` collapses the mapped form before comparison. Two tests
cover it: the mapped pair now conflicts, and `::1` against `127.0.0.1`
still passes, because those are genuinely two addresses.

Also:

- The bind-address parse check was written twice, once for clusters and
  once for services, differing only in the error prefix. Folded into one
  `check_bind_address` taking a `PortOwner`, which already renders both
  prefixes.
- `PortMapping::protocol` was documented as "tcp or udp". The field is
  shared by two validators that accept different values: cluster
  mappings take tcp, udp or sctp case-insensitively, matching KIND, and
  service ports take tcp only. The doc now says so, and the two protocol
  checks explain why they differ.
- `generate_kind_config` claimed KIND only supports port mappings on
  control-plane nodes. KIND accepts the field on any node; one node is
  enough because kube-proxy makes a NodePort reachable through every
  node, and publishing the same host port from several would collide.

Signed-off-by: Ladislav Smola <lsmola@redhat.com>
@Ladas
Ladas force-pushed the fix/host-port-binding-followup branch from 494a5dd to eb5a2a5 Compare September 7, 2026 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant