search

kubernetes / kubectl

Issue tracker and mirror of kubectl code
Apache License 2.0
2.89k stars 924 forks source link

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

Open ringerc opened 1 year ago

ringerc commented 1 year ago

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
ringerc commented 1 year ago

Any thoughts on this proposal?

mpuckett159 commented 1 year ago

/triage accepted I would suggest for this that it starts off as a plugin to prototype the functionality out and gauge user interest. The idea seems sound.

ringerc commented 10 months ago

Thanks @mpuckett159 . I have a branch for it that I can probably turn into a plugin later. I'll keep an eye to see if others turn up here too.

ringerc commented 8 months ago

The branch is https://github.com/ringerc/kubernetes_patches/tree/kubectl-port-forward-output-file

skissane-medallia commented 2 months ago

@ringerc I have the same problem as you do.... having this accepted into kubectl would make my life easier... or if it were available as a plugin, that would be the next best thing...

I tried synching your branch with latest kubernetes master, and I discovered it has some merge conflicts due to recent changes in port-forward command code, most of which are due to https://github.com/kubernetes/enhancements/issues/4006 – unfortunately I don't understand this code well enough to feel confident in trying to fix the conflicts (I guess if I spent enough time studying it I probably could), maybe you might have the time to try that

ringerc commented 1 month ago

I landed up getting quite bogged in this, IIRC because the go code that kubectl uses to set up port-forwards assumes that the same port is selected for ipv4 and ipv6. I don't recall the details now as I had to pause this work.

Having it as a plugin wasn't worth it for me; it's no better than needing an ugly wrapper script to parse the kubectl output, as is presently done.