From 168173a3f11bbb6a01ab76f22a11fb961cbc8bbd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 15 Feb 2021 16:00:07 +0100 Subject: [PATCH] Use net.JoinHostPort() to fix formatting with IPv6 addresses Signed-off-by: Sebastiaan van Stijn --- cli/command/container/port.go | 5 +- cli/command/container/port_test.go | 57 +++++++++++++++++++ .../testdata/container-port-ipv4.golden | 1 + .../container-port-ipv6-and-ipv4.golden | 2 + .../testdata/container-port-ipv6.golden | 1 + opts/port.go | 5 +- opts/port_test.go | 44 ++++++++++++++ 7 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 cli/command/container/port_test.go create mode 100644 cli/command/container/testdata/container-port-ipv4.golden create mode 100644 cli/command/container/testdata/container-port-ipv6-and-ipv4.golden create mode 100644 cli/command/container/testdata/container-port-ipv6.golden diff --git a/cli/command/container/port.go b/cli/command/container/port.go index 83e16a98b7..dc769b40cf 100644 --- a/cli/command/container/port.go +++ b/cli/command/container/port.go @@ -3,6 +3,7 @@ package container import ( "context" "fmt" + "net" "strings" "github.com/docker/cli/cli" @@ -61,7 +62,7 @@ func runPort(dockerCli command.Cli, opts *portOptions) error { } if frontends, exists := c.NetworkSettings.Ports[newP]; exists && frontends != nil { for _, frontend := range frontends { - fmt.Fprintf(dockerCli.Out(), "%s:%s\n", frontend.HostIP, frontend.HostPort) + fmt.Fprintln(dockerCli.Out(), net.JoinHostPort(frontend.HostIP, frontend.HostPort)) } return nil } @@ -70,7 +71,7 @@ func runPort(dockerCli command.Cli, opts *portOptions) error { for from, frontends := range c.NetworkSettings.Ports { for _, frontend := range frontends { - fmt.Fprintf(dockerCli.Out(), "%s -> %s:%s\n", from, frontend.HostIP, frontend.HostPort) + fmt.Fprintf(dockerCli.Out(), "%s -> %s\n", from, net.JoinHostPort(frontend.HostIP, frontend.HostPort)) } } diff --git a/cli/command/container/port_test.go b/cli/command/container/port_test.go new file mode 100644 index 0000000000..5778feedc3 --- /dev/null +++ b/cli/command/container/port_test.go @@ -0,0 +1,57 @@ +package container + +import ( + "io/ioutil" + "testing" + + "github.com/docker/cli/internal/test" + "github.com/docker/docker/api/types" + "github.com/docker/go-connections/nat" + "gotest.tools/v3/assert" + "gotest.tools/v3/golden" +) + +func TestNewPortCommandOutput(t *testing.T) { + testCases := []struct { + name string + ips []string + }{ + { + name: "container-port-ipv4", + ips: []string{"0.0.0.0"}, + }, + { + name: "container-port-ipv6", + ips: []string{"::"}, + }, + { + name: "container-port-ipv6-and-ipv4", + ips: []string{"::", "0.0.0.0"}, + }, + } + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + cli := test.NewFakeCli(&fakeClient{ + inspectFunc: func(string) (types.ContainerJSON, error) { + ci := types.ContainerJSON{NetworkSettings: &types.NetworkSettings{}} + ci.NetworkSettings.Ports = nat.PortMap{ + "80/tcp": make([]nat.PortBinding, len(tc.ips)), + } + for i, ip := range tc.ips { + ci.NetworkSettings.Ports["80/tcp"][i] = nat.PortBinding{ + HostIP: ip, HostPort: "3456", + } + } + return ci, nil + }, + }, test.EnableContentTrust) + cmd := NewPortCommand(cli) + cmd.SetErr(ioutil.Discard) + cmd.SetArgs([]string{"some_container", "80"}) + err := cmd.Execute() + assert.NilError(t, err) + golden.Assert(t, cli.OutBuffer().String(), tc.name+".golden") + }) + } +} diff --git a/cli/command/container/testdata/container-port-ipv4.golden b/cli/command/container/testdata/container-port-ipv4.golden new file mode 100644 index 0000000000..f1811520cc --- /dev/null +++ b/cli/command/container/testdata/container-port-ipv4.golden @@ -0,0 +1 @@ +0.0.0.0:3456 diff --git a/cli/command/container/testdata/container-port-ipv6-and-ipv4.golden b/cli/command/container/testdata/container-port-ipv6-and-ipv4.golden new file mode 100644 index 0000000000..fdb6c56bb4 --- /dev/null +++ b/cli/command/container/testdata/container-port-ipv6-and-ipv4.golden @@ -0,0 +1,2 @@ +[::]:3456 +0.0.0.0:3456 diff --git a/cli/command/container/testdata/container-port-ipv6.golden b/cli/command/container/testdata/container-port-ipv6.golden new file mode 100644 index 0000000000..4bd36cd904 --- /dev/null +++ b/cli/command/container/testdata/container-port-ipv6.golden @@ -0,0 +1 @@ +[::]:3456 diff --git a/opts/port.go b/opts/port.go index f65367168d..e0c5389930 100644 --- a/opts/port.go +++ b/opts/port.go @@ -3,6 +3,7 @@ package opts import ( "encoding/csv" "fmt" + "net" "regexp" "strconv" "strings" @@ -148,8 +149,8 @@ func ConvertPortToPortConfig( ports := []swarm.PortConfig{} for _, binding := range portBindings[port] { - if binding.HostIP != "" && binding.HostIP != "0.0.0.0" { - logrus.Warnf("ignoring IP-address (%s:%s:%s) service will listen on '0.0.0.0'", binding.HostIP, binding.HostPort, port) + if p := net.ParseIP(binding.HostIP); p != nil && !p.IsUnspecified() { + logrus.Warnf("ignoring IP-address (%s:%s) service will listen on '0.0.0.0'", net.JoinHostPort(binding.HostIP, binding.HostPort), port) } startHostPort, endHostPort, err := nat.ParsePortRange(binding.HostPort) diff --git a/opts/port_test.go b/opts/port_test.go index e364c52b10..b02fb95237 100644 --- a/opts/port_test.go +++ b/opts/port_test.go @@ -1,9 +1,13 @@ package opts import ( + "bytes" + "os" "testing" "github.com/docker/docker/api/types/swarm" + "github.com/docker/go-connections/nat" + "github.com/sirupsen/logrus" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -316,6 +320,46 @@ func TestPortOptInvalidSimpleSyntax(t *testing.T) { } } +func TestConvertPortToPortConfigWithIP(t *testing.T) { + testCases := []struct { + value string + expectedWarning string + }{ + { + value: "0.0.0.0", + }, + { + value: "::", + }, + { + value: "192.168.1.5", + expectedWarning: `ignoring IP-address (192.168.1.5:2345:80/tcp) service will listen on '0.0.0.0'`, + }, + { + value: "::2", + expectedWarning: `ignoring IP-address ([::2]:2345:80/tcp) service will listen on '0.0.0.0'`, + }, + } + + var b bytes.Buffer + logrus.SetOutput(&b) + for _, tc := range testCases { + tc := tc + t.Run(tc.value, func(t *testing.T) { + _, err := ConvertPortToPortConfig("80/tcp", map[nat.Port][]nat.PortBinding{ + "80/tcp": {{HostIP: tc.value, HostPort: "2345"}}, + }) + assert.NilError(t, err) + if tc.expectedWarning == "" { + assert.Equal(t, b.String(), "") + } else { + assert.Assert(t, is.Contains(b.String(), tc.expectedWarning)) + } + }) + } + logrus.SetOutput(os.Stderr) +} + func assertContains(t *testing.T, portConfigs []swarm.PortConfig, expected swarm.PortConfig) { var contains = false for _, portConfig := range portConfigs {