Skip to content

feat(compose): dedicated network for the provider relay link - #14275

Open
ndeloof wants to merge 1 commit into
docker:mainfrom
ndeloof:relay-dedicated-network
Open

ndeloof wants to merge 1 commit into
docker:mainfrom
ndeloof:relay-dedicated-network

Conversation

@ndeloof

@ndeloof ndeloof commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Compose now gives each locally-bound provider service its own dedicated bridge network for the relay link, instead of relying on the provider guessing a bind address that happens to work.

Context

When a provider service publishes an endpoint that runs on the provider's own host (not a remote resource), Compose deploys a relay container in front of it so dependents keep connecting to the compose-native <service>:<port> address. That relay has to reach the endpoint the provider bound — but on a standalone Linux engine, no address is both relay-reachable and off the LAN by default: host.docker.internal resolves to a project bridge's gateway, which every other container on that bridge can also reach, and the wildcard exposes the port on every host interface. get-relay-info already let the provider ask Compose which gateway to bind, but the answer pointed at one of the dependents' own networks — shared infrastructure the provider's endpoint had no business sitting on.

What the PR brings

  • A get-relay-info request now provisions (or reuses) a dedicated, internal:true bridge network scoped to that one provider service — joined by the relay container and nothing else — and answers with its name and gateway.
  • Sending get-relay-info is itself the provider's declaration that it binds locally: a provider backing a remote resource (an Amazon RDS instance, say) never sends it, so no network is ever created on its behalf. The network is created lazily, from the request, never speculatively from a published endpoint alone.
  • The relay container joins this network in addition to the dependents' networks it already joined, so the local endpoint stays reachable only from the relay — not from the LAN, and not from any other project service.
  • The dedicated network is torn down with the relay when the service stops publishing endpoints, and by down, including the "no compose file" down --project-name path where per-service provider metadata can't be reconstructed from container labels alone.
sequenceDiagram
    participant Compose
    participant Provider
    participant net as relay-link network<br/>(internal, per-service)
    participant relay as relay container

    rect rgb(235, 245, 255)
    note over Compose,net: Provider running its service locally
    Compose->>Provider: compose up --project-name=xx "database"
    Provider->>Compose: json { "type": "get-relay-info" }
    Compose->>net: create (or reuse) the dedicated<br/>relay-link network for "database"
    Compose--)Provider: json {"networks":[{"name":"myproject_database_relay",<br/>"gateway":"172.20.0.1"}]}
    Provider->>Provider: bind local endpoint to 172.20.0.1
    Provider--)Compose: json { "type": "publish-endpoint",<br/>"message": "80=172.20.0.1:49152" }
    Compose->>relay: deploy, join dependents' networks<br/>AND the relay-link network
    end
Loading
sequenceDiagram
    participant Compose
    participant Provider
    participant resource as remote resource<br/>(e.g. Amazon RDS)
    participant relay as relay container

    rect rgb(255, 245, 235)
    note over Compose,resource: Provider backing a remote resource
    Compose->>Provider: compose up --project-name=xx "database"
    Provider->>resource: provision
    note over Provider: no local endpoint to bind:<br/>get-relay-info is never sent
    Provider--)Compose: json { "type": "publish-endpoint",<br/>"message": "80=resource.example.com:5432" }
    Compose->>relay: deploy, join dependents' networks only<br/>(no relay-link network created)
    end
Loading

The two flows share the same relay deployment and publish-endpoint contract; only whether get-relay-info is ever sent decides whether a relay-link network exists at all.

Compose now creates a dedicated, isolated network for the link between a relay container and a provider service that binds its endpoint locally, instead of exposing that endpoint on a network shared with dependent services.

🤖 Generated with Claude Code

@ndeloof
ndeloof requested review from a team as code owners September 30, 2026 16:02
@ndeloof
ndeloof requested a review from glours September 30, 2026 16:02

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

Comment thread pkg/compose/relay.go
Comment thread pkg/compose/relay.go
Comment thread pkg/compose/down.go

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review covers only the commits pushed since 4b8105ea10ec.

Assessment: 🟡 NEEDS ATTENTION

Two medium-confidence findings in the new race-condition mitigations added to ensureRelayLinkNetworksDown. The relayInfo name-guard and the ensureRelayLinkNetwork NotFound switch look correct.

Comment thread pkg/compose/relay.go
if len(inspected.Network.Containers) > 0 {
// the daemon's async disconnect of the just-removed relay
// container hasn't caught up yet; a later down retries
logrus.Warnf("relay link network %s is still in use, skipping removal", n.Name)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] ensureRelayLinkNetworksDown skips network removal with no in-process retry when containers are still attached

When inspected.Network.Containers is non-empty, the new code logs a warning and continues, deferring cleanup to a future down invocation. This is safe for the expected transient case — the daemon's async disconnect of the just-removed relay container — but the loop has no bounded retry or polling timeout to confirm the disconnect actually completes. If any container (not necessarily the relay) remains attached to the internal:true relay-link network — even due to a daemon bug or a mistaken docker network connect — every subsequent down call will silently skip removal with the same warning and no escalation path. The user sees repeated warnings but the orphaned network is never cleaned up automatically.

The comment says "a later down retries," which is accurate for normal operation, but there is no mechanism inside the function to distinguish a transient disconnect lag (milliseconds) from a stuck container (permanent), so the skip is open-ended rather than bounded.

Consider replacing the skip with a short poll (e.g. 3 attempts × 500 ms) for the container list to drain before giving up, or at minimum changing the log from Warn to a more prominent message that includes actionable guidance (docker network disconnect <network> <container>).

Confidence Score
🟡 moderate 62/100

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not adding a bounded retry/poll here: this mirrors removeNetwork's existing skip-and-warn behavior for regular project networks exactly (also open-ended, also deferred to the next down), so it's consistent with the codebase's existing convention rather than a new gap this PR introduces. A container that never disconnects (daemon bug, stray manual network connect) is an operational anomaly outside what either code path currently handles — happy to revisit if that's a real risk worth carrying in this PR, deferring to @ndeloof on whether it's worth the added complexity here.

Comment thread pkg/compose/relay.go Outdated
@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.91729% with 44 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/compose/relay.go 66.15% 25 Missing and 19 partials ⚠️

📢 Thoughts on this report? Let us know!

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

This review covers only the commits pushed since 0e2129a6a289d85124a5ff05866656262910452b.

The incremental diff is focused and correct. ensureRelayLinkNetworksDown now accumulates errors with errors.Join instead of returning early, so a transient NetworkInspect failure on one relay-link network no longer blocks cleanup of the remaining networks. Key correctness points:

  • errors.Join(nil) / errors.Join() returns nil, so the no-error case is handled correctly — no spurious errors.
  • The continue after appending an inspect error correctly skips the NetworkRemove call for the failed network, preserving the safe-by-default posture.
  • The new test TestEnsureRelayLinkNetworksDownContinuesPastInspectError exercises the exact failure path: net-1 inspect fails (accumulated), net-2 inspect succeeds and is removed, the single returned op() surfaces the inspect error. The assertion len(ops) == 1 is correct because ensureRelayLinkNetworksDown always returns a one-element []downOp slice.

No bugs found in the added code.

@ndeloof
ndeloof force-pushed the relay-dedicated-network branch from 822c706 to 989e55d Compare October 1, 2026 13:44
@ndeloof
ndeloof requested a review from docker-agent October 1, 2026 13:44

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

Lower-confidence findings (not posted inline)

  • [low] pkg/compose/relay.go:197 — relayLinkNetworkName format {project}_{service}_relay can collide with a user-declared network key {service}_relay (confidence: weak 30/100)

Comment thread pkg/compose/relay.go Outdated
Comment thread pkg/compose/relay.go Outdated
@ndeloof
ndeloof requested a review from docker-agent October 1, 2026 13:59

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review covers only the commits pushed since 989e55d6a3df.

Assessment: 🟢 APPROVE

The two changes in this incremental diff are both correct:

  1. ensureRelayLinkNetwork conflict path — After NetworkCreate returns errdefs.IsConflict, the code now re-validates ownership via findRelayLinkNetwork (label-filtered) before proceeding. If no labeled relay network is found, it fails with a descriptive error rather than silently adopting an unrelated same-named network. The if/else chain is logically sound: the shadowed err inside the := is idiomatic Go, and the error message correctly uses the in-scope name variable.

  2. removeRelayLinkNetwork IsConflict tolerance — Tolerate errdefs.IsConflict on NetworkRemove with warn+skip, matching ensureRelayLinkNetworksDown's existing behavior. The Docker daemon disconnects containers asynchronously, so a brief conflict window after ContainerRemove is expected and transient. Any non-transient orphan would be caught by a subsequent down call via ensureRelayLinkNetworksDown, which lists by the same labels. This is consistent with the codebase-level convention used by removeNetwork for regular project networks.

@ndeloof
ndeloof force-pushed the relay-dedicated-network branch from e601e7a to 659a85b Compare October 1, 2026 15:01
@ndeloof
ndeloof requested a review from docker-agent October 1, 2026 15:01
get-relay-info answers pointed a locally-bound provider at one of the
dependents' own networks: shared infrastructure every other service on
it could also reach, and no more isolated than any project bridge.

Provision a dedicated, internal:true bridge network scoped to that one
provider service instead, created only when the provider itself sends
get-relay-info -- the provider's own declaration that it binds locally,
never for a provider backing a remote resource. The relay container
joins it alongside the dependents' networks it already joins; it is
torn down with the relay on endpoint changes and on down, including
the no-compose-file down path.

The network's lifecycle is hardened against the races a shared,
project-scoped resource runs into:

- a NetworkCreate conflict on the deterministic name re-checks by
  label (not just by name) before adopting the existing network, so a
  genuine name clash with an unrelated, user-declared network fails
  loudly instead of being silently adopted into the relay's isolation
  boundary;
- a NotFound gateway lookup after losing that conflict falls back to
  an unresolved answer instead of propagating the error;
- removeRelayLinkNetwork and ensureRelayLinkNetworksDown both tolerate
  a NetworkRemove conflict from the daemon's asynchronous endpoint
  disconnect, which can race the relay container's own removal;
- ensureRelayLinkNetworksDown accumulates failures with errors.Join
  across every relay-link network of a project instead of aborting on
  the first one, and skips (warn-only) a network that still has active
  endpoints, mirroring removeNetwork's existing tolerance for regular
  project networks.

docs/extension.md documents that the "desktop" network name in the
get-relay-info answer (under Docker Desktop) is an opaque placeholder,
never a real Docker network a provider can inspect or connect to.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof
ndeloof force-pushed the relay-dedicated-network branch from 659a85b to 4dbd41a Compare October 1, 2026 15:35
@ndeloof
ndeloof requested review from docker-agent and removed request for docker-agent October 1, 2026 15:50

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

This branch has not been deployed

No deployments
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.

2 participants