API should either reject, de-duplicate, or warn duplicate exposed-ports

thaJeztah commented 1 year ago

Description

When creating a container using the CLI, duplicate exposed ports are de-duplicated;

docker create --name foo --expose 3306 --expose 3306/tcp alpine
a21cc9b32c1c4183c3c0ac2e409818a2961b86c267fef4c83a292f274e3a93af

docker container inspect --format '{{json .Config.ExposedPorts}}' foo | jq .
{
  "3306/tcp": {}
}

When looking at API call that's logged in the daemon debug logs;

DEBU[2023-03-22T20:14:11.522833800Z] Calling POST /v1.42/containers/create?name=foo
DEBU[2023-03-22T20:14:11.522943008Z] form data: {"AttachStderr":true,"AttachStdin":false,"AttachStdout":true,"Cmd":null,"Domainname":"","Entrypoint":null,"Env":null,"ExposedPorts":{"3306/tcp":{}},"HostConfig":{"AutoRemove":false,"Binds":null,"BlkioDeviceReadBps":[],"BlkioDeviceReadIOps":[],"BlkioDeviceWriteBps":[],"BlkioDeviceWriteIOps":[],"BlkioWeight":0,"BlkioWeightDevice":[],"CapAdd":null,"CapDrop":null,"Cgroup":"","CgroupParent":"","CgroupnsMode":"","ConsoleSize":[60,216],"ContainerIDFile":"","CpuCount":0,"CpuPercent":0,"CpuPeriod":0,"CpuQuota":0,"CpuRealtimePeriod":0,"CpuRealtimeRuntime":0,"CpuShares":0,"CpusetCpus":"","CpusetMems":"","DeviceCgroupRules":null,"DeviceRequests":null,"Devices":[],"Dns":[],"DnsOptions":[],"DnsSearch":[],"ExtraHosts":null,"GroupAdd":null,"IOMaximumBandwidth":0,"IOMaximumIOps":0,"IpcMode":"","Isolation":"","Links":null,"LogConfig":{"Config":{},"Type":""},"MaskedPaths":null,"Memory":0,"MemoryReservation":0,"MemorySwap":0,"MemorySwappiness":-1,"NanoCpus":0,"NetworkMode":"default","OomKillDisable":false,"OomScoreAdj":0,"PidMode":"","PidsLimit":0,"PortBindings":{},"Privileged":false,"PublishAllPorts":false,"ReadonlyPaths":null,"ReadonlyRootfs":false,"RestartPolicy":{"MaximumRetryCount":0,"Name":"no"},"SecurityOpt":null,"ShmSize":0,"UTSMode":"","Ulimits":null,"UsernsMode":"","VolumeDriver":"","VolumesFrom":null},"Hostname":"","Image":"alpine","Labels":{},"NetworkingConfig":{"EndpointsConfig":{}},"OnBuild":null,"OpenStdin":false,"StdinOnce":false,"Tty":false,"User":"","Volumes":{},"WorkingDir":""}

That contains "ExposedPorts":{"3306/tcp":{}}.

Looks like docker build also de-duplicates;

docker build -t bla -<<'EOF'
FROM alpine
EXPOSE 3306 3306/tcp
EOF

docker create --name bla bla
docker container inspect --format '{{json .Config.ExposedPorts}}' bla | jq .
{
  "3306/tcp": {}
}

docker image inspect  --format '{{json .Config.ExposedPorts}}' bla | jq .
{
  "3306/tcp": {}
}

However, when directly using the API, the duplicate ports are neither rejected, nor normalized;

curl -v \
  --unix-socket /var/run/docker.sock \
  "http://localhost/v1.42/containers/create?name=foobar" \
  -H "Content-Type: application/json" \
  -d '{"Image":"alpine:latest", "ExposedPorts":{"3306/tcp":{},"3306":{}}}'

docker container inspect --format '{{json .Config.ExposedPorts}}' foobar | jq .
{
  "3306": {},
  "3306/tcp": {}
}

Reproduce

See above

Expected behavior

We should either;

Reject duplicate ports
Accept duplicate ports, and perform normalizing + de-duplicating on the daemon side
Same as above, but return a Warning so that the duplicate is surfaced on the CLI

We likely cannot do 1. as doing so would be a breaking change, and could potentially break existing use-cases / clients.

docker version

Client:
 Version:           23.0.1
 API version:       1.42
 Go version:        go1.19.5
 Git commit:        a5ee5b1
 Built:             Thu Feb  9 19:45:39 2023
 OS/Arch:           linux/arm64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          23.0.1
  API version:      1.42 (minimum version 1.12)
  Go version:       go1.19.5
  Git commit:       bc3805a
  Built:            Thu Feb  9 19:48:15 2023
  OS/Arch:          linux/arm64
  Experimental:     false
 containerd:
  Version:          v1.6.16
  GitCommit:        31aa4358a36870b21a992d3ad2bef29e1d693bec
 runc:
  Version:          1.1.4
  GitCommit:        v1.1.4-0-g5fd4c4d
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

docker info

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc.)
    Version:  v0.10.4
    Path:     /usr/libexec/docker/cli-plugins/docker-buildx
  compose: Docker Compose (Docker Inc.)
    Version:  v2.16.0
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 3
  Running: 0
  Paused: 0
  Stopped: 3
 Images: 2
 Server Version: 23.0.1
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 31aa4358a36870b21a992d3ad2bef29e1d693bec
 runc version: v1.1.4-0-g5fd4c4d
 init version: de40ad0
 Security Options:
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 5.15.49-linuxkit
 Operating System: Alpine Linux v3.17
 OSType: linux
 Architecture: aarch64
 CPUs: 5
 Total Memory: 7.667GiB
 Name: bc12e4b49d96
 ID: 494d76fa-0417-47a4-b2ff-e197cc74ef2d
 Docker Root Dir: /var/lib/docker
 Debug Mode: true
  File Descriptors: 23
  Goroutines: 36
  System Time: 2023-03-22T20:39:05.956880547Z
  EventsListeners: 0
 Registry: https://index.docker.io/v1/
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false
 Product License: Community Engine

Additional Info

Reproduced on both 20.10 and 23.0, but likely has been around much longer.

milas commented 1 year ago

For what it's worth, here's the code in Compose v2 (modified July 2021) that populates ExposedPorts: https://github.com/docker/compose/blob/6bedc196cc74730522123e159d3e49f411b5c3cd/pkg/compose/create.go#L685-L696

So I think it's possible to have duplicates, e.g.

services:
  mariadb:
    image: mariadb
    expose:
      - 3306
    ports:
      - 3306:3306

In practice, I could not tell you how common that is, as I'm not really sure how expose is used in Compose YAML.

thaJeztah commented 1 year ago

@milas do you know why compose is merging exposed ports with Published ports? Those should be completely separate things?

thaJeztah commented 1 year ago

I see it was added in https://github.com/docker/compose/commit/94997be6331ab6c8aa53e3516f9408d81cee7b45, but that's probably ported from compose-cli.

Looks like it was;

Trying to think what exactly is done there; does that mean that any exposed port is automatically published (so --publish-all when using compose?

thaJeztah commented 1 year ago

Oh! I think I got it; it's the reverse; for a port to be published, it also has to be (implicitly) exposed.

As an example; here's creating an alpine container, which by itself does not have any ports "exposed" in its config;

docker create --name foo -p 80:80 alpine

But the port-mapping also set an explicit "expose";

DEBU[2023-03-23T09:19:41.786405173Z] Calling POST /v1.42/containers/create?name=foo
DEBU[2023-03-23T09:19:41.786524715Z] form data: {"AttachStderr":true,"AttachStdin":false,"AttachStdout":true,"Cmd":null,"Domainname":"","Entrypoint":null,"Env":null,"ExposedPorts":{"80/tcp":{}},"HostConfig":{"AutoRemove":false,"Binds":null,"BlkioDeviceReadBps":[],"BlkioDeviceReadIOps":[],"BlkioDeviceWriteBps":[],"BlkioDeviceWriteIOps":[],"BlkioWeight":0,"BlkioWeightDevice":[],"CapAdd":null,"CapDrop":null,"Cgroup":"","CgroupParent":"","CgroupnsMode":"","ConsoleSize":[60,216],"ContainerIDFile":"","CpuCount":0,"CpuPercent":0,"CpuPeriod":0,"CpuQuota":0,"CpuRealtimePeriod":0,"CpuRealtimeRuntime":0,"CpuShares":0,"CpusetCpus":"","CpusetMems":"","DeviceCgroupRules":null,"DeviceRequests":null,"Devices":[],"Dns":[],"DnsOptions":[],"DnsSearch":[],"ExtraHosts":null,"GroupAdd":null,"IOMaximumBandwidth":0,"IOMaximumIOps":0,"IpcMode":"","Isolation":"","Links":null,"LogConfig":{"Config":{},"Type":""},"MaskedPaths":null,"Memory":0,"MemoryReservation":0,"MemorySwap":0,"MemorySwappiness":-1,"NanoCpus":0,"NetworkMode":"default","OomKillDisable":false,"OomScoreAdj":0,"PidMode":"","PidsLimit":0,"PortBindings":{"80/tcp":[{"HostIp":"","HostPort":"80"}]},"Privileged":false,"PublishAllPorts":false,"ReadonlyPaths":null,"ReadonlyRootfs":false,"RestartPolicy":{"MaximumRetryCount":0,"Name":"no"},"SecurityOpt":null,"ShmSize":0,"UTSMode":"","Ulimits":null,"UsernsMode":"","VolumeDriver":"","VolumesFrom":null},"Hostname":"","Image":"alpine","Labels":{},"NetworkingConfig":{"EndpointsConfig":{}},"OnBuild":null,"OpenStdin":false,"StdinOnce":false,"Tty":false,"User":"","Volumes":{},"WorkingDir":""}

docker container inspect --format='{{ json .Config.ExposedPorts }}' foo
{"80/tcp":{}}

docker container inspect --format='{{ json .HostConfig.PortBindings }}' foo
{"80/tcp":[{"HostIp":"","HostPort":"80"}]}

sam-thibault commented 1 year ago

/cc @akerouanton

moby / moby