From bbddd27f0dfffe6623763afe2c02c876ba925a7c Mon Sep 17 00:00:00 2001
From: Martin Helmich <martin@helmich.me>
Date: Sun, 3 Feb 2019 19:01:19 +0100
Subject: [PATCH] client-go: Dynamic local port not accessible when
 port-forwarding

When setting up a port forwarding with the client-go library (using the
`k8s.io/client-go/tools/portforward.PortForwarder`) with a non-defined local
port (i.e. passing `:80` as `ports` parameter to `portforward.New(...)`), a
local port will be assigned dynamically.

Currently, the local port will be _always_ 0 if it was not specified initially.
This is because the assigned local port is only set on a _copy_ of the actual
`ForwardedPort` type that is obtained in a `range` loop. This PR changes this
behaviour to set the local port at the correct instance by passing a pointer
instead of a copy to the relevant functions.
---
 .../tools/portforward/portforward.go          |  5 +-
 .../tools/portforward/portforward_test.go     | 76 +++++++++++++++++++
 2 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/staging/src/k8s.io/client-go/tools/portforward/portforward.go b/staging/src/k8s.io/client-go/tools/portforward/portforward.go
index 0e9b369a98..dc047030c0 100644
--- a/staging/src/k8s.io/client-go/tools/portforward/portforward.go
+++ b/staging/src/k8s.io/client-go/tools/portforward/portforward.go
@@ -197,8 +197,9 @@ func (pf *PortForwarder) forward() error {
 	var err error
 
 	listenSuccess := false
-	for _, port := range pf.ports {
-		err = pf.listenOnPort(&port)
+	for i := range pf.ports {
+		port := &pf.ports[i]
+		err = pf.listenOnPort(port)
 		switch {
 		case err == nil:
 			listenSuccess = true
diff --git a/staging/src/k8s.io/client-go/tools/portforward/portforward_test.go b/staging/src/k8s.io/client-go/tools/portforward/portforward_test.go
index ff2401e76b..da1dfcc0f9 100644
--- a/staging/src/k8s.io/client-go/tools/portforward/portforward_test.go
+++ b/staging/src/k8s.io/client-go/tools/portforward/portforward_test.go
@@ -18,11 +18,13 @@ package portforward
 
 import (
 	"net"
+	"net/http"
 	"os"
 	"reflect"
 	"sort"
 	"strings"
 	"testing"
+	"time"
 
 	"k8s.io/apimachinery/pkg/util/httpstream"
 )
@@ -39,6 +41,37 @@ func (d *fakeDialer) Dial(protocols ...string) (httpstream.Connection, string, e
 	return d.conn, d.negotiatedProtocol, d.err
 }
 
+type fakeConnection struct {
+	closed    bool
+	closeChan chan bool
+}
+
+func newFakeConnection() httpstream.Connection {
+	return &fakeConnection{
+		closeChan: make(chan bool),
+	}
+}
+
+func (c *fakeConnection) CreateStream(headers http.Header) (httpstream.Stream, error) {
+	return nil, nil
+}
+
+func (c *fakeConnection) Close() error {
+	if !c.closed {
+		c.closed = true
+		close(c.closeChan)
+	}
+	return nil
+}
+
+func (c *fakeConnection) CloseChan() <-chan bool {
+	return c.closeChan
+}
+
+func (c *fakeConnection) SetIdleTimeout(timeout time.Duration) {
+	// no-op
+}
+
 func TestParsePortsAndNew(t *testing.T) {
 	tests := []struct {
 		input                   []string
@@ -254,3 +287,46 @@ func TestGetListener(t *testing.T) {
 
 	}
 }
+
+func TestGetPortsReturnsDynamicallyAssignedLocalPort(t *testing.T) {
+	dialer := &fakeDialer{
+		conn: newFakeConnection(),
+	}
+
+	stopChan := make(chan struct{})
+	readyChan := make(chan struct{})
+	errChan := make(chan error)
+
+	defer func() {
+		close(stopChan)
+
+		forwardErr := <-errChan
+		if forwardErr != nil {
+			t.Fatalf("ForwardPorts returned error: %s", forwardErr)
+		}
+	}()
+
+	pf, err := New(dialer, []string{":5000"}, stopChan, readyChan, os.Stdout, os.Stderr)
+
+	if err != nil {
+		t.Fatalf("error while calling New: %s", err)
+	}
+
+	go func() {
+		errChan <- pf.ForwardPorts()
+		close(errChan)
+	}()
+
+	<-pf.Ready
+
+	ports, err := pf.GetPorts()
+
+	if len(ports) != 1 {
+		t.Fatalf("expected 1 port, got %d", len(ports))
+	}
+
+	port := ports[0]
+	if port.Local == 0 {
+		t.Fatalf("local port is 0, expected != 0")
+	}
+}