Skip to content

Commit 4ee2010

Browse files
committed
Fixes address parsing in port-forward
The rules for address parsing are: * Explicitly specified addresses must bind successfully * `localhost` is pinned to `127.0.0.1` and `::1` and at least one of those must bind successfully This change also makes output of the command consistent between runs with the same arguments. Previously the command was using the range via map of addresses which sometimes was producing different output because the order of values is not guaranteed in Go.
1 parent b862590 commit 4ee2010

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

pkg/kubectl/cmd/portforward/portforward.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func NewCmdPortForward(f cmdutil.Factory, streams genericclioptions.IOStreams) *
120120
},
121121
}
122122
cmdutil.AddPodRunningTimeoutFlag(cmd, defaultPodPortForwardWaitTimeout)
123-
cmd.Flags().StringSliceVar(&opts.Address, "address", []string{"localhost"}, "Addresses to listen on (comma separated)")
123+
cmd.Flags().StringSliceVar(&opts.Address, "address", []string{"localhost"}, "Addresses to listen on (comma separated). Only accepts IP addresses or localhost as a value. When localhost is supplied, kubectl will try to bind on both 127.0.0.1 and ::1 and will fail if neither of these addresses are available to bind.")
124124
// TODO support UID
125125
return cmd
126126
}

staging/src/k8s.io/client-go/tools/portforward/portforward.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"io/ioutil"
2424
"net"
2525
"net/http"
26+
"sort"
2627
"strconv"
2728
"strings"
2829
"sync"
@@ -122,10 +123,14 @@ func parseAddresses(addressesToParse []string) ([]listenAddress, error) {
122123
parsed := make(map[string]listenAddress)
123124
for _, address := range addressesToParse {
124125
if address == "localhost" {
125-
ip := listenAddress{address: "127.0.0.1", protocol: "tcp4", failureMode: "all"}
126-
parsed[ip.address] = ip
127-
ip = listenAddress{address: "::1", protocol: "tcp6", failureMode: "all"}
128-
parsed[ip.address] = ip
126+
if _, exists := parsed["127.0.0.1"]; !exists {
127+
ip := listenAddress{address: "127.0.0.1", protocol: "tcp4", failureMode: "all"}
128+
parsed[ip.address] = ip
129+
}
130+
if _, exists := parsed["::1"]; !exists {
131+
ip := listenAddress{address: "::1", protocol: "tcp6", failureMode: "all"}
132+
parsed[ip.address] = ip
133+
}
129134
} else if net.ParseIP(address).To4() != nil {
130135
parsed[address] = listenAddress{address: address, protocol: "tcp4", failureMode: "any"}
131136
} else if net.ParseIP(address) != nil {
@@ -140,6 +145,9 @@ func parseAddresses(addressesToParse []string) ([]listenAddress, error) {
140145
addresses[id] = v
141146
id++
142147
}
148+
// Sort addresses before returning to get a stable order
149+
sort.Slice(addresses, func(i, j int) bool { return addresses[i].address < addresses[j].address })
150+
143151
return addresses, nil
144152
}
145153

staging/src/k8s.io/client-go/tools/portforward/portforward_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,62 @@ func TestParsePortsAndNew(t *testing.T) {
8383
{protocol: "tcp6", address: "::1", failureMode: "all"},
8484
},
8585
},
86+
{
87+
input: []string{"5000:5000"},
88+
addresses: []string{"localhost", "::1"},
89+
expectedPorts: []ForwardedPort{
90+
{5000, 5000},
91+
},
92+
expectedAddresses: []listenAddress{
93+
{protocol: "tcp4", address: "127.0.0.1", failureMode: "all"},
94+
{protocol: "tcp6", address: "::1", failureMode: "any"},
95+
},
96+
},
97+
{
98+
input: []string{"5000:5000"},
99+
addresses: []string{"localhost", "127.0.0.1", "::1"},
100+
expectedPorts: []ForwardedPort{
101+
{5000, 5000},
102+
},
103+
expectedAddresses: []listenAddress{
104+
{protocol: "tcp4", address: "127.0.0.1", failureMode: "any"},
105+
{protocol: "tcp6", address: "::1", failureMode: "any"},
106+
},
107+
},
108+
{
109+
input: []string{"5000:5000"},
110+
addresses: []string{"localhost", "127.0.0.1", "10.10.10.1"},
111+
expectedPorts: []ForwardedPort{
112+
{5000, 5000},
113+
},
114+
expectedAddresses: []listenAddress{
115+
{protocol: "tcp4", address: "127.0.0.1", failureMode: "any"},
116+
{protocol: "tcp6", address: "::1", failureMode: "all"},
117+
{protocol: "tcp4", address: "10.10.10.1", failureMode: "any"},
118+
},
119+
},
120+
{
121+
input: []string{"5000:5000"},
122+
addresses: []string{"127.0.0.1", "::1", "localhost"},
123+
expectedPorts: []ForwardedPort{
124+
{5000, 5000},
125+
},
126+
expectedAddresses: []listenAddress{
127+
{protocol: "tcp4", address: "127.0.0.1", failureMode: "any"},
128+
{protocol: "tcp6", address: "::1", failureMode: "any"},
129+
},
130+
},
131+
{
132+
input: []string{"5000:5000"},
133+
addresses: []string{"10.0.0.1", "127.0.0.1"},
134+
expectedPorts: []ForwardedPort{
135+
{5000, 5000},
136+
},
137+
expectedAddresses: []listenAddress{
138+
{protocol: "tcp4", address: "10.0.0.1", failureMode: "any"},
139+
{protocol: "tcp4", address: "127.0.0.1", failureMode: "any"},
140+
},
141+
},
86142
{
87143
input: []string{"5000", "5000:5000", "8888:5000", "5000:8888", ":5000", "0:5000"},
88144
addresses: []string{"127.0.0.1", "::1"},

0 commit comments

Comments
 (0)