port-forward: Support writing json port mapping to file or stdout [PATCH]

What would you like to be added

I'd like to be able to easily get a machine-parseable JSON mapping of local to remote port mappings from kubectl port-forward when using :remoteport-style mappings that assign dynamic local ports.

Proof of concept

Here's my working tree with the draft patch. Quickstart:

git clone \
    --reference=/path/to/your/existing/kubernetes/checkout \
    -b kubectl-port-forward-output-file \
    https://github.com/ringerc/kubernetes_patches  \
    kubectl-port-forward-output-file
cd kubectl-port-forward-output-file
make kubectl
./_output/local/go/bin/kubectl port-forward --quiet --ports-file=- [...usual...args...here...]

Why is this needed

Presently, it's very cumbersome to use kubectl port-forward in scripts and tools, especially with dynamic local-port bindings to avoid collisions. Parsing the human-readable port mapping output risks errors relating to message translation / localization / internationalization / locale configuration, and should never be encouraged.

Because port-forwarding requires a lot more logic than just the REST API request, it's not very convenient to do port-forwarding natively from various scripting languages even if they have kube API bindings, so shelling out to kubectl port-forward is often the preferred solution.

By having kubectl emit a machine-readable mapping of local to remote ports it is much easier to and more reliable to use kubectl in scripts.

Proposal details

To meet his need, I propose a new kubectl port-forward --ports-file={path|-} option that takes a file-path or the special value - to mean stdout. When specified, kubectl will emit a JSON array of local-to-remote port mappings represented as JSON objects. For example, the port-forward target pod/nginx :http might produce:

[{"LocalPort":33915,"LocalProtocol":"tcp4","LocalAddress":"127.0.0.1","RemotePort":80,"RemotePortName":"http"},{"LocalPort":33915,"LocalProtocol":"tcp6","LocalAddress":"::1","RemotePort":80,"RemotePortName":"http"}]

A compact one-line JSON representation will always be used to simplify line-buffered reading from scripting tools.

Pretty-printed, this will look like:

[
  {
    "LocalPort": 43309,
    "LocalProtocol": "tcp4",
    "LocalAddress": "127.0.0.1",
    "RemotePort": 80,
    "RemotePortName": "http"
  },
  {
    "LocalPort": 43309,
    "LocalProtocol": "tcp6",
    "LocalAddress": "::1",
    "RemotePort": 80,
    "RemotePortName": "http"
  }
]

This mapping is only emitted once all port-forwards are established, before kubectl port-forward begins waiting until an exit signal/error is received.

One entry is emitted for each (listen-address, port) pair. See notes below for why that's important. The order in which entries will be output is undefined. Applications can find the mapping they want using the RemotePortName field, which contains the original remote-side of the port mapping requested by the caller before any service-port mapping and named port resolution are performed.

kubectl port-forward will close the output file or file descriptor once the port mapping is written unless the output is stdout. This allows scripts invoking it to more easily detect when ports are mapped, especially when passing a pipe file descriptor as the output. /dev/fd/N paths are special-cased, and the corresponding file descriptor is used directly instead of opening the path as a file; this makes it very easy for scripts to pass a pipe then read until EoF.

The remote side of the reported mapping will always use numeric ports; port names are resolved to port numbers. It could be extended to also include the port name, but this may require changes to the client-go portforward code as well. If added, a new Remote key will be introduced; this will be set to the original remote-side input, a port name or number depending on how the caller specified the port.

Issues to consider

client-go's portforward produces unreliable local port mappings

The GetPorts() function in client-go/tools/portforward produces incorrect results if different local ports were assigned to different listen addresses. This can easily happen, even on the default localhost listen address, if a listen port is busy for IPv4 but not busy for IPv6 or vice versa. The function returns whichever port was assigned by the last successful listener created for that port.

To fix this, this patch adds a new GetPortMappings() that returns one entry per port per listening address. Each entry includes the local port, local listening address and local address scheme (protocol).

The original GetPorts() is not changed, because doing so would require adding fields to the public ForwardedPort struct and cause GetPorts() to return multiple entries for one port, which existing code may not expect.

File descriptor paths are special cased using Linux specific path notation

So that the invoking process receives an end-of-file when reading from a pipe file descriptor that it passes to kubectl port-forward, it is necessary to open the write end of the inherited pipe file descriptor with os.NewFile() rather than opening a /dev/fd/{n} path with os.OpenFile().

Accordingly, this patch special-cases Linux-style /dev/fd/{n} file descriptor paths and parses out the file descriptor number from the path, then opens that directly. This ensures that when kubectl port-forward Close()s the inherited fd and the fd is a pipe, the calling process receives an end-of-file on reads from the pipe.

The pfw.py script supplied below demonstrates this in action.

The /dev/fd/n notation for file descriptors is Linux specific. There is no widespread 'nix convention for file descriptor IDs like there is for the use of - for stdin where file path arguments are accepted. The use of Linux style /dev/fd/{n} notation is considered preferable to inventing a custom new syntax. It does mean that kubectl will not actually attempt to open the file-path specified in this case, but that's already the case for - and can just be documented as a special case.

Conflicts with existing stdout output when writing port-map to stdout

Unfortunately, kubectl currently writes its human-readable port-mapping to stdout not stderr, so simply writing the json port mapping to stdout produces unhelpful output:

$ ./_output/local/go/bin/kubectl port-forward --ports-file=- pod/nginx :http 2>/dev/null
Forwarding from 127.0.0.1:41897 -> 80
Forwarding from [::1]:41897 -> 80
[{"Local":41897,"Remote":80}]

This output is generated by client-go's portforward code and may be depended upon by other clients than kubectl. The destination stream can be controlled, but the messages are always output as individual lines of human-friendly text like this.

To handle this, I was originally thinking of adding a -f json option to kubectl port-forward, and an OutputPrinter option to the client-go PortForwarder, so the messages could be transformed by the caller. I could then get rid of the current logic that waits within kubectl itself for the forwards to be up. But this got a little ugly when I then wanted to enrich the port mapping output to show the original port name/number requested by the caller, before service port mapping and named port resolution. It also looked like it'd be difficult to do in a clean, extensible manner without a whole lot of boilerplate.

There doesn't seem to be generic support in kubectl for command output formatters (like -f json) either. kubectl get has its own printers/formatters to support -f json and most other commands don't seem to accept a formatter at all. So I'd have to add it to kubectl port-forward and modify all the existing message output sites. It'd be large, ugly and intrusive.

Also, it's desirable to be able to close a port-map file descriptor once port forwarding is established. This makes scripting much easier because you can wait for EoF to know the port mappings are up. But we don't want to close stdout after writing the mapping - that'd foreclose future enhancements anyone else might want to add. So even if some kind of -f json scheme was added to obtain the port mapping output over stdout, it'd still be good to have a separate argument for a port-mapping file or fd for scripting use.

I landed up keeping it simple and I added a --quiet option that suppresses the default Forwarded to ... messages on stdout, so port mappings can be obtained on stdout by explicitly requesting them with kubectl port-forward --v=10 --ports-file=- pod/nginx :http. It's a bit ugly; I'd welcome advice on cleaner methods that won't break BC for people, and won't require me to write large amounts of new output abstraction infrastructure for all of kubectl.

Tests

I had to make quite a few changes to the tests for port-forward to make this set of changes testable. I'd welcome advice on how to do that more cleanly. I expect I need to split them out from the functional changes before this can be considered as a potentially merge-able PR anyway.

Tests need updates to be compatible with recent fixes

Sample patch

Current patch can be found in branch https://github.com/ringerc/kubernetes_patches/tree/kubectl-port-forward-output-file

Github compare at https://github.com/kubernetes/kubernetes/compare/master...ringerc:kubernetes_patches:kubectl-port-forward-output-file?expand=0

Demo

$ cd kubernetes
$ git remote add ringerc https://github.com/ringerc/kubernetes_patches
$ git fetch ringerc
$ git checkout kubectl-port-forward-output-file
$ make -s kubectl
$ kind create cluster
$ cat > nginx.yaml <<'__END__'
apiVersion: v1
kind: Pod
metadata:
  name: nginx
spec:
  containers:
  - name: nginx
    image: nginx:1.14.2
    ports:
    - containerPort: 80
      name: http
__END__
$ kubectl apply -f nginx.yaml
pod/nginx created
$ kubectl wait --for=condition=Ready pod/nginx
pod/nginx condition met
$  ./_output/local/go/bin/kubectl port-forward --quiet --ports-file=- pod/nginx :http | jq '.'
[
  {
    "LocalPort": 43309,
    "LocalProtocol": "tcp4",
    "LocalAddress": "127.0.0.1",
    "RemotePort": 80,
    "RemotePortName": "http"
  },
  {
    "LocalPort": 43309,
    "LocalProtocol": "tcp6",
    "LocalAddress": "::1",
    "RemotePort": 80,
    "RemotePortName": "http"
  }
]

or with Python, save this as pfw.py:

#!/usr/bin/env python3

import os
import sys
import subprocess
import json

def port_forward():
    (pfw_read, pfw_write) = os.pipe()
    cmd = [
            "./_output/local/go/bin/kubectl",
            "port-forward",
            #"--v=100",
            "--ports-file=/dev/fd/{}".format(pfw_write),
            "pod/nginx",
            ":http",
          ]
    print("Starting port-forward...", file=sys.stderr)
    pfw = subprocess.Popen(cmd,
            pass_fds=[pfw_write],
            stdout=subprocess.DEVNULL,
            )
    # Close the write end of the pipe after it is inherited by the child
    # process. This ensures that we will get EoF when read()ing from the read
    # end of the pipe.
    os.fdopen(pfw_write).close()
    # We can read() until EoF because kubectl port-forward will close the
    # writer end of the pipe once the mapping is written. This means we don't
    # have to rely on line-buffered reads.
    with os.fdopen(pfw_read, encoding=sys.getdefaultencoding()) as portmap_fd:
        print("Waiting for port-forward to be ready...", file=sys.stderr)
        portmap = json.loads(portmap_fd.read())
        print("Port forwarding is ready, port mapping read", file=sys.stderr)
        return (pfw, portmap)

def main():
    pfw, portmap = port_forward()
    print("Mapped ports:", file=sys.stderr)
    for p in portmap:
        localAddr =  p['LocalAddress']
        if p['LocalProtocol'] == 'tcp6':
            localAddr = "[" + localAddr + "]"
        print(f"\t{localAddr}:{p['LocalPort']} => {p['RemotePort']} ({p['RemotePortName']})", file=sys.stderr)
    print("Did something with the forwarded port here")
    pfw.terminate()
    pfw.wait()

if __name__ == '__main__':
    main()                                                                                                                                                                                                                                    

$ python3 ./pfw.py
Starting port-forward...
Waiting for port-forward to be ready...
Port forwarding is ready, port mapping read
Mapped ports:
	127.0.0.1:38403 => 80 (http)
	[::1]:38403 => 80 (http)
Did something with the forwarded port here

Start with kubectl port-forward and client-go/tools/portforward, especially the GetPorts() behavior described in the issue, then review the referenced kubectl-port-forward-output-file patch and existing port-forward tests. Done means an agreed design for file or stdout JSON mappings, compatible handling of existing output, and updated tests covering the mapping and output behavior.