Skip to content

Comments

Log rather than error if port mapping is overspecified#48575

Merged
robmry merged 2 commits intomoby:masterfrom
robmry:port_mapping_validation
Oct 17, 2024
Merged

Log rather than error if port mapping is overspecified#48575
robmry merged 2 commits intomoby:masterfrom
robmry:port_mapping_validation

Conversation

@robmry
Copy link
Contributor

@robmry robmry commented Oct 3, 2024

- What I did

Previously, if a port mapping specified a host IP or port that could not be used because the endpoint's network was in routed mode (so, there's no host binding), it'd be treated as an error.

However:

  • the selected gateway endpoint may change over time, as networks are connected and disconnected - so the binding may make sense for some other endpoint.
  • the validation was complicated, duplicated logic in order to fail early, and wasn't complete.

- How I did it

So, just log when fields are ignored, at the point where they're ignored.

Removed the old validation function and its test.

Give the port mapping code a context containing a logger that adds network and endpoint ids.

- How to verify it

Added unit tests for cases that were previously errors, checking the log output.

Example of the log message with extra context:

# DOCKER_USERLANDPROXY=false hack/make.sh run

# docker network create b4
# docker run --rm -ti --network b4 -p [::1]:8080:80 nginx

INFO[2024-10-03T14:15:19.298614757Z] Cannot map from IPv6 to an IPv4-only container because the userland proxy is disabled  eid=a8005de0bc9e294b83ae31aefdd17bb65e8055f75fd6a88eafe411557411228b mapping="[::1]:8080:80/tcp" nid=cbbe0a39ecb8522b791e0c44ae106820f3eea6f5404e5656557e9f138108bfc9 spanID=02e5a360305a8237 traceID=d2de070d083025c8a0bbdc7d7c6fe9aa

- Description for the changelog

Log rather than error if a port mapping includes a host IP address or port number that
cannot be used because NAT from the host is disabled. The unused fields may be needed
if the gateway endpoint changes when networks are connected or disconnected.

@robmry robmry force-pushed the port_mapping_validation branch from 6de9036 to a4c9c61 Compare October 3, 2024 14:25
Previously, if a port mapping specified a host IP or port that
could not be used because the endpoint's network was in routed
mode (so, there's no host binding), it'd be treated as an error.

However:
- the selected gateway endpoint may change over time, as networks
  are connected and disconnected - so the binding may make sense
  for some other endpoint.
- the validation was complicated, duplicated logic in order to
  fail early, and wasn't complete.

So, just log when fields are ignored, at the point where they're
ignored.

Signed-off-by: Rob Murray <[email protected]>
@robmry robmry force-pushed the port_mapping_validation branch from c4a4659 to 98efe66 Compare October 4, 2024 10:40
@robmry robmry marked this pull request as ready for review October 4, 2024 13:00
@robmry robmry requested a review from akerouanton October 4, 2024 13:00
Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants