Unified remote port forwarding UX

[mjbvz]

Problem

Remote extensions current each implement their own version of port forwarding, including their own configuration and UI related to port forwarding (see the ssh and containers extensions for example). Even with just two extensions supporting port forwarding, the UX around port forwarding is not consistent and we have duplicated development effort. This problem will become worse as more extensions / scenarios add support for port forwarding

Additionally, VS Code has a built-in tunneling mechanism that can be used to implement a limited version of port forwarding even if the remote extension doesn't directly support it. There is currently no UI for managing these tunnels. Managing these will become important if we allow extensions to open new tunnels, as #81131 proposes

Proposal

Build a unified port forwarding user experience that the remote extensions can contribute to. This would act somewhat like the existing Remote Explorer contribution

High level UX Needs

• List ports currently being forwarded
  • Include human readable names
  • Maybe include type (through VS Code's built-in tunnel service, through remote, external, ...)
• Let user close one of these ports
• Let user open a new port forwarding

High level extension API needs

• A way for VS Code to call out to the remote extension to create a new port forwarding. We also need to be able to tear down this connection (probably just make the result of the api call disposable)
• A way for a remote extension to tell us about port forwardings managed outside of VS Code (see ssh's LocalFoward support for an example of this)
  • We likely also need a way so that the extension can signal that its external port forwardings have changed
• Possible: provide a list of candidate ports for forwarding but that require user action to establish? (see the containers port forwarding ux for an example of this)

/cc @Chuxel for general feedback, @roblourens and @kieferrm for ssh, and @chrmarti for containers

[chrmarti]

Sounds good! For containers it would make sense to also have the following:

• A way to show static forwardings that cannot be removed. (These have the same lifecycle as the container.)
• A way to show details for an active (or potential) forwarding:
  • That the port is declared in the Dockerfile (but not statically forwarded).
  • Which process (showing the command line) currently is listening on it.

Candidate ports would be good to have, so we can carry that feature forward. (We discover ports being listened on and also read a list of candidate ports from the container.)

[chrmarti]

/cc @alexr00

[Chuxel]

Sorry on the delayed response - been a crazy few days.

Yeah I think being able to establish a difference between static publishing/forwarding and the dynamic variation is important for two reasons:

1. Read-only - which applies both to SSH and Containers
2. Behavior differences - "Publishing" a port via docker does not "forward" 127.0.0.1 - your service needs to listen on all interfaces. The dynamic forwarding code will handle 127.0.0.1 as well.
   In the explorer, we may need to use slightly different UX to represent them as well given some of the confusion we've seen for containers.

I also agree on candidate ports, but we should also allow someone to just key in a port so they can do the forwarding before the server in question has been started. (I assume this is implied, but figured it would be good to call out.)

[chrmarti]

Similar to the UX requirements the API requirements for Remote-Containers are:

• Have a way to register existing, static port forwardings. (These are part of the container's configuration and should be used instead of an additional dynamic port forwarding.)
• Have a way to register candidate ports.
• Maybe have a way look up existing forwardings.

It would make sense to include a hostname/ip when specifying a target port. The same port number on different hostnames/ips can be listened on by different processes.

[chrmarti]

If that becomes too fine-grained for an API, we could also consider doing a smaller API that allows each extension to contribute a port-forwarding view, similar to the targets and details views.

[alexr00]

UI that we need to power:

Current proposal:

	export interface Tunnel extends Port, Disposable {
		localPort: number;
		localAddress: Uri;
		forwardMechanism?: string;
		closeable: boolean;
	}

	export interface Port {
		remotePort: number;
		localPort?: number;
		remoteAddress: Uri;
		localAddress?: Uri;
		name?: string;
		description?: string;
	}

	/**
	 * Used as part of the ResolverResult if the extension has any candidate, published, or forwarded ports.
	 */
	export interface PortInformation {
		/**
		 * Ports that are already immutably published. This is not the same as forwarding.
		 */
		published?: Port[];
	}

	export namespace workspace {
		/**
		 * Forwards a port.
		 * @param forward The `localPort` is a suggestion only. If that port is not available another will be chosen.
		 */
		export function forwardPort(forward: (Port & { closeable: boolean })): Thenable<Tunnel>;
	}

	export type ResolverResult = ResolvedAuthority & ResolvedOptions & PortInformation;

	export interface RemoteAuthorityResolver {
	        /*
		 * Can be optionally implemented if the extension can forward ports better than the core.
		 * When not implemented, the core will use its default forwarding logic.
		 * When implemented, the core will use this to forward ports.
		 */
		forwardPort?(remotePort: number, localPort?: number): Thenable<Port | undefined>;
	}

[chrmarti]

Sorry, I couldn't make the call. I have a few questions:

Why is the address in the (address, port) pairs a URI? Intuitively I would expect it to be a hostname or IP address.

Could you document forwardMechanism and closeable?

Would it make sense to make a tunnel object having two port objects instead of having a port object almost being a tunnel? That would correspond more to how I think about these.

[alexr00]

New current proposal with feedback from the API sync:

	export interface Tunnel extends TunnelDescriptor, Disposable {
		localAddress: Uri;
		remoteAddress: Uri;
	}

	export interface TunnelDescriptor {
		remotePort: number;
		localPort?: number;
		name?: string;
		closeable?: boolean;
	}

	/**
	 * Used as part of the ResolverResult if the extension has any candidate, published, or forwarded ports.
	 */
	export interface TunnelInformation {
		/**
		 * Ports that are already immutably published. This is not the same as forwarding.
		 */
		published?: TunnelDescriptor[];
		/**
		 *  Tunnels that should be established.
		 */
		forward?: TunnelDescriptor[];
	}

	export type ResolverResult = ResolvedAuthority & ResolvedOptions & TunnelInformation;

	export interface RemoteAuthorityResolver {
		resolve(authority: string, context: RemoteAuthorityResolverContext): ResolverResult | Thenable<ResolverResult>;
		/**
		 * An optional event for the resolver to signal that the tunnel information has changed.
		 * (ex. there are new ports that have been published or that should be forwarded.)
		 */
		onTunnelInformationChanged?: Event<TunnelInformation>;
		/**
		 * Can be optionally implemented if the extension can forward ports better than the core.
		 * When not implemented, the core will use its default forwarding logic.
		 * When implemented, the core will use this to forward ports.
		 */
		forwardPort?(tunnelDescriptor: TunnelDescriptor): Thenable<Tunnel | undefined>;
	}

Some feedback prefers the forward in the TunnelInformation + onTunnelInformationChanged changed event. This approach allows us to restrict explicit port forwarding to resolvers.

Another approach that other feedback favors is to have something like this:

	export namespace workspace {
		/**
		 * Forwards a port.
		 * @param forward The `localPort` is a suggestion only. If that port is not available another will be chosen.
		 */
		export function makeTunnel(forward: TunnelDescriptor): Thenable<Tunnel>;
	}

I made the address a URI so that it would be opened using our existing API. forwardMechanism is gone because we don't have a clear use for it now. closeable indicates whether that port is able to be closed. Ex: a published port isn't closeable. A port forwarded through the UI is closeable.

[chrmarti]

What would the URI scheme be for a binary protocol? tcp://?

Using only ports to describe a tunnel is imprecise because a server might not listen on all IP addresses of its host machine, but only on specific ones. It might make sense to include host with the port properties. Or: Use URIs everywhere because these include host and port (sometimes they just imply the port).

[alexr00]

        export interface TunnelOptions {
		remote: { port: number, host: string };
		localPort?: number;
		name?: string;
	}

	export interface Tunnel {
		remote: { port: number, host: string };
		localAddress: string;
		onDispose: Event<void>;
		dispose(): void;
	}

	/**
	 * Used as part of the ResolverResult if the extension has any candidate, published, or forwarded ports.
	 */
	export interface TunnelInformation {
		/**
		 * Tunnels that are detected by the extension. The remotePort is used for display purposes.
		 * The localAddress should be the complete local address(ex. localhost:1234) for connecting to the port. Tunnels provided through
		 * detected are read-only from the forwarded ports UI.
		 */
		detectedTunnels?: { remote: { port: number, host: string }, localAddress: string }[];
	}

	export type ResolverResult = ResolvedAuthority & ResolvedOptions & TunnelInformation;

	export interface RemoteAuthorityResolver {
		/**
		 * Can be optionally implemented if the extension can forward ports better than the core.
		 * When not implemented, the core will use its default forwarding logic.
		 * When implemented, the core will use this to forward ports.
		 */
		forwardPort?(tunnelOptions: TunnelOptions): Thenable<Tunnel> | undefined;
	}

	export namespace workspace {
		/**
		 * Forwards a port. Currently only works for a remote host of localhost.
		 * @param forward The `localPort` is a suggestion only. If that port is not available another will be chosen.
		 */
		export function makeTunnel(forward: TunnelOptions): Thenable<Tunnel>;
	}