feat(internal/cmd): Add 'instance tunnel' command by craciunoiuc · Pull Request #248 · unikraft/cli

craciunoiuc · 2026-04-07T08:31:47Z

Nodes need this option --net-internal-traffic=vm in order to test it.

tunnel-3uhz1 │ [    0.141932] ERR:  [apptunlr] Target remote not in the same subnet.
tunnel-3uhz1 │ [    0.142826] ERR:  [apptunlr] Failed to hook TCP2TCP.

~~The Tunlr also needs updating to remove the check for inter-vm comms.~~
Done.

TODO: Update tests when platform is stable.

Blocked on: https://github.com/unikraft-cloud/platform/pull/705
Closes: TOOL-714

Copilot

Pull request overview

This PR adds a new unikraft instance tunnel subcommand intended to forward a local port to a non-public instance via an intermediate TLS tunnel service, enabling testing scenarios that require internal VM traffic.

Changes:

Adds InstancesTunnelCmd to the instances command group.
Implements target parsing, instance/IP resolution, tunnel-proxy instance creation, and local-to-remote relaying over TLS.
Adds control-channel heartbeats to keep the tunnel service connection alive.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jedevc · 2026-04-07T10:39:02Z

+	// exposedProxyPorts holds the port exposed by the tunnel service per target.
+	exposedProxyPorts []uint16
+	// portIterator tracks the next port offset when a single proxy port is given.
+	portIterator uint16


👀 why are these all on this struct?

The only things that should be here are the args that can be read by kong.

equivalent of passing them by arg everywhere

If I move to a separate package I can keep them there.

IMO, these kind of act like global variables. It's unclear where they get created / passed through. Passing them through as args would be better if we can.

If they grow to be very long, we can group them into structs.

sure, if i'm moving this to a separate package it will need them as arguments anyway

Ok, I simplified it as much as I could, let's see what Copilot says.

I think the exposed interface of the library is quite simple now:

New <- you create the instance RunProxy <- start the proxying instance TerminateProxy <- stop the proxying instance StartRelay <- block and send messages

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

craciunoiuc · 2026-04-07T16:42:47Z

I need to fix these + do one more pass so no rush

craciunoiuc · 2026-04-07T19:06:15Z

Should be as decent as it gets, definitely an upgrade compared to kraftkit

craciunoiuc · 2026-04-17T14:29:10Z

Have to rebase the code and to generate tests on Monday.

jedevc

Needs a rebase, and to get the tests passing 🎉

jedevc · 2026-05-01T10:22:10Z

+			cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+			defer cancel()
+			env.unikraftIgnoreErr(cleanupCtx, "instance", "delete", instanceName)
+		})


We should use the same test style as in the rest of the tests, so we get a resulting golden file!

For managing a tunnel in the background - we should probably add some sort of way to run it in the background using the golden setup.

Hmm, I don't have much context on that but I can try

jedevc · 2026-05-01T10:32:42Z

 }
+
+type InstancesTunnelCmd struct {
+	Targets []string `arg:"" name:"target" min:"1" help:"Forwarding target(s) in the form [LOCAL_PORT:]<INSTANCE>:DEST_PORT[/TYPE]."`


Use placeholder to show the form. See other examples of how this is done.

jedevc · 2026-05-01T10:33:36Z

+
+func (cmd *InstancesTunnelCmd) Run(ctx context.Context, stdio config.Stdio) error {
+	if len(cmd.TunnelProxyPorts) == 0 {
+		cmd.TunnelProxyPorts = []string{"4444"}


Can we not use the default tag?

What are you referring to? you mean for the images? you can just set a different image

I mean in the kong command struct.

Ah sorry, probably some old kraftkit copy paste

jedevc · 2026-05-01T10:35:28Z

+		for i, r := range resolved {
+			inst := r.(Instance)
+			if metro != "" && inst.key.Metro != "" && inst.key.Metro != metro {
+				return nil, "", fmt.Errorf("forwarding targets must be in the same metro: found %q and %q", metro, inst.key.Metro)


🤔 we could support instances across multiple metros. We would just need to create multiple tunnels. This is supported for the logs cmd, so makes sense to support it here.

Instead of resolving all the metros, we'd do a call more like CollectRefs, and use that to resolve all the ips, and metros, and then just do multiple tunnel creations!

Sounds crazy, but doable, the command has support for multiple tunnel connections

jedevc · 2026-05-01T10:38:03Z

+// NOTE(craciunoiuc): We do this here to have access to the Instances API
+func (cmd *InstancesTunnelCmd) tunnelLookupInstances(ctx context.Context, t *tunnel.Tunnel) ([]string, string, error) {


This function could very easily be added to the tunnel package actually.

Yes, we'd have to do the GetInstances request directly there, but then it's a reusable package, for anyone to use. Also, we already are doing CreateInstance requests in there, so.

Sure, was conflicted about that

jedevc · 2026-05-01T11:02:01Z

+func (r *tunnelRelay) listenLocal(ctx context.Context) (net.Listener, error) {
+	var lc net.ListenConfig
+	return lc.Listen(ctx, r.ctype+"4", r.lAddr)
+}


We can inline this right?

jedevc · 2026-05-01T11:03:27Z

+	defer rc.Close()
+	go func() { <-ctx.Done(); rc.Close() }()


Again, closed twice?

😭

I though I caught all the places where this happened

jedevc · 2026-05-01T11:07:12Z

+	parts := strings.SplitN(portsArg, "/", 3)
+	if len(parts) == 3 {
+		rest = parts[0] + "/" + parts[1]
+		ctype = parts[2]
+	} else if len(parts) == 2 {


👀 what? What is this case for? What if there are 4 /s here?

We could simplify this by just splitting at the last /.

I believe the format is now possibly metro/instance/connectionType

Comment is outdated needs an update

jedevc · 2026-05-01T11:07:33Z

+		if parseErr != nil {
+			return "", 0, 0, "", fmt.Errorf("%q is not a valid port number", segments[1])
+		}
+		return segments[0], uint16(rport64), uint16(rport64), ctype, nil


If no local port - shouldn't we select it to a random available port? (by setting it to 0)

I think it sets it to the default value?

It seems to be just using rport64 for both?

craciunoiuc · 2026-05-01T13:38:50Z

I'll rebase and fix afterwards

Signed-off-by: Cezar Craciunoiu <cezar@unikraft.io>

craciunoiuc requested a review from Copilot April 7, 2026 08:31

Copilot started reviewing on behalf of craciunoiuc April 7, 2026 08:32 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

craciunoiuc force-pushed the craciunoiuc/add-instance-tunnel branch 2 times, most recently from 49d2539 to 8ae136d Compare April 7, 2026 09:26

jedevc reviewed Apr 7, 2026

View reviewed changes

craciunoiuc force-pushed the craciunoiuc/add-instance-tunnel branch from 8ae136d to 5e8f3e5 Compare April 7, 2026 16:15

craciunoiuc requested a review from Copilot April 7, 2026 16:16

Copilot started reviewing on behalf of craciunoiuc April 7, 2026 16:17 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

craciunoiuc force-pushed the craciunoiuc/add-instance-tunnel branch from 5e8f3e5 to b8e8437 Compare April 7, 2026 19:02

craciunoiuc requested a review from jedevc April 7, 2026 19:05

craciunoiuc force-pushed the craciunoiuc/add-instance-tunnel branch from b8e8437 to a5b1451 Compare April 9, 2026 14:17

craciunoiuc force-pushed the craciunoiuc/add-instance-tunnel branch 2 times, most recently from dcaf1a6 to c7622e6 Compare April 20, 2026 09:02

jedevc reviewed May 1, 2026

View reviewed changes

craciunoiuc added 2 commits May 1, 2026 16:40

feat(internal/cmd): Add 'instance tunnel' command

d0c9694

Signed-off-by: Cezar Craciunoiu <cezar@unikraft.io>

feat(cmd): Add 'instance tunnel' tests and regenerate tests

41b74aa

Signed-off-by: Cezar Craciunoiu <cezar@unikraft.io>

craciunoiuc force-pushed the craciunoiuc/add-instance-tunnel branch from c7622e6 to 41b74aa Compare May 1, 2026 13:43

		// NOTE(craciunoiuc): We do this here to have access to the Instances API
		func (cmd InstancesTunnelCmd) tunnelLookupInstances(ctx context.Context, t tunnel.Tunnel) ([]string, string, error) {

Conversation

craciunoiuc commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

craciunoiuc Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

craciunoiuc commented Apr 7, 2026

Uh oh!

craciunoiuc commented Apr 7, 2026

Uh oh!

craciunoiuc commented Apr 17, 2026

Uh oh!

jedevc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

craciunoiuc commented Apr 7, 2026 •

edited

Loading

craciunoiuc Apr 7, 2026 •

edited

Loading