Merge pull request #3892 from thaJeztah/port_sort

container port: sort ports before printing

cli/command/container/port.go

@@ -4,12 +4,15 @@ import (
 	"context"
 	"fmt"
 	"net"
+	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/completion"
 	"github.com/docker/go-connections/nat"
+	"github.com/fvbommel/sortorder"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
@@ -43,6 +46,12 @@ func NewPortCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
+// runPort shows the port mapping for a given container. Optionally, it
+// allows showing the mapping for a specific (container)port and proto.
+//
+// TODO(thaJeztah): currently this defaults to show the TCP port if no
+// proto is specified. We should consider changing this to "any" protocol
+// for the given private port.
 func runPort(dockerCli command.Cli, opts *portOptions) error {
 	ctx := context.Background()
 
@@ -51,34 +60,36 @@ func runPort(dockerCli command.Cli, opts *portOptions) error {
 		return err
 	}
 
+	var out []string
 	if opts.port != "" {
-		port := opts.port
-		proto := "tcp"
-		parts := strings.SplitN(port, "/", 2)
-
-		if len(parts) == 2 && len(parts[1]) != 0 {
-			port = parts[0]
-			proto = parts[1]
+		port, proto, _ := strings.Cut(opts.port, "/")
+		if proto == "" {
+			proto = "tcp"
 		}
-		natPort := port + "/" + proto
-		newP, err := nat.NewPort(proto, port)
-		if err != nil {
-			return err
+		if _, err = strconv.ParseUint(port, 10, 16); err != nil {
+			return errors.Wrapf(err, "Error: invalid port (%s)", port)
+		}
+		frontends, exists := c.NetworkSettings.Ports[nat.Port(port+"/"+proto)]
+		if !exists || frontends == nil {
+			return errors.Errorf("Error: No public port '%s' published for %s", opts.port, opts.container)
 		}
-		if frontends, exists := c.NetworkSettings.Ports[newP]; exists && frontends != nil {
 		for _, frontend := range frontends {
-				fmt.Fprintln(dockerCli.Out(), net.JoinHostPort(frontend.HostIP, frontend.HostPort))
+			out = append(out, net.JoinHostPort(frontend.HostIP, frontend.HostPort))
 		}
-			return nil
-		}
-		return errors.Errorf("Error: No public port '%s' published for %s", natPort, opts.container)
-	}
-
+	} else {
 		for from, frontends := range c.NetworkSettings.Ports {
 			for _, frontend := range frontends {
-			fmt.Fprintf(dockerCli.Out(), "%s -> %s\n", from, net.JoinHostPort(frontend.HostIP, frontend.HostPort))
+				out = append(out, fmt.Sprintf("%s -> %s", from, net.JoinHostPort(frontend.HostIP, frontend.HostPort)))
 			}
 		}
+	}
+
+	if len(out) > 0 {
+		sort.Slice(out, func(i, j int) bool {
+			return sortorder.NaturalLess(out[i], out[j])
+		})
+		_, _ = fmt.Fprintln(dockerCli.Out(), strings.Join(out, "\n"))
+	}
 
 	return nil
 }

cli/command/container/port_test.go

@@ -15,18 +15,31 @@ func TestNewPortCommandOutput(t *testing.T) {
 	testCases := []struct {
 		name string
 		ips  []string
+		port string
 	}{
 		{
 			name: "container-port-ipv4",
 			ips:  []string{"0.0.0.0"},
+			port: "80",
 		},
 		{
 			name: "container-port-ipv6",
 			ips:  []string{"::"},
+			port: "80",
 		},
 		{
 			name: "container-port-ipv6-and-ipv4",
 			ips:  []string{"::", "0.0.0.0"},
+			port: "80",
+		},
+		{
+			name: "container-port-ipv6-and-ipv4-443-udp",
+			ips:  []string{"::", "0.0.0.0"},
+			port: "443/udp",
+		},
+		{
+			name: "container-port-all-ports",
+			ips:  []string{"::", "0.0.0.0"},
 		},
 	}
 	for _, tc := range testCases {
@@ -37,18 +50,26 @@ func TestNewPortCommandOutput(t *testing.T) {
 					ci := types.ContainerJSON{NetworkSettings: &types.NetworkSettings{}}
 					ci.NetworkSettings.Ports = nat.PortMap{
 						"80/tcp":  make([]nat.PortBinding, len(tc.ips)),
+						"443/tcp": make([]nat.PortBinding, len(tc.ips)),
+						"443/udp": make([]nat.PortBinding, len(tc.ips)),
 					}
 					for i, ip := range tc.ips {
 						ci.NetworkSettings.Ports["80/tcp"][i] = nat.PortBinding{
 							HostIP: ip, HostPort: "3456",
 						}
+						ci.NetworkSettings.Ports["443/tcp"][i] = nat.PortBinding{
+							HostIP: ip, HostPort: "4567",
+						}
+						ci.NetworkSettings.Ports["443/udp"][i] = nat.PortBinding{
+							HostIP: ip, HostPort: "5678",
+						}
 					}
 					return ci, nil
 				},
 			}, test.EnableContentTrust)
 			cmd := NewPortCommand(cli)
 			cmd.SetErr(io.Discard)
-			cmd.SetArgs([]string{"some_container", "80"})
+			cmd.SetArgs([]string{"some_container", tc.port})
 			err := cmd.Execute()
 			assert.NilError(t, err)
 			golden.Assert(t, cli.OutBuffer().String(), tc.name+".golden")

cli/command/container/testdata/container-port-all-ports.golden

@@ -0,0 +1,6 @@
+80/tcp -> 0.0.0.0:3456
+80/tcp -> [::]:3456
+443/tcp -> 0.0.0.0:4567
+443/tcp -> [::]:4567
+443/udp -> 0.0.0.0:5678
+443/udp -> [::]:5678

cli/command/container/testdata/container-port-ipv6-and-ipv4-443-udp.golden

@@ -0,0 +1,2 @@
+0.0.0.0:5678
+[::]:5678

cli/command/container/testdata/container-port-ipv6-and-ipv4.golden

@@ -1,2 +1,2 @@
-[::]:3456
 0.0.0.0:3456
+[::]:3456