Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1515 +/- ##
==========================================
- Coverage 79.61% 79.13% -0.48%
==========================================
Files 305 307 +2
Lines 9790 9864 +74
==========================================
+ Hits 7794 7806 +12
- Misses 1996 2058 +62
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
200459d to
3e78fe2
Compare
jooola
left a comment
jooola
left a comment
There was a problem hiding this comment.
Looks good, we still need a few extra things and fix the few nits.
| var VNCCmd = base.Cmd{ | ||
| BaseCobraCommand: func(client hcapi2.Client) *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "vnc-port-foward [options] <server>", |
There was a problem hiding this comment.
| Use: "vnc-port-foward [options] <server>", | |
| Use: "vnc-port-forward [options] <server>", |
| cmd.Flags().IntP("port", "p", 5900, "Local port to listen on (0 picks a random free port)") | ||
| return cmd | ||
| }, | ||
| Experimental: base.ExperimentalWrapper("The vnc-port-foward command", "experimental", ""), |
There was a problem hiding this comment.
| Experimental: base.ExperimentalWrapper("The vnc-port-foward command", "experimental", ""), | |
| Experimental: base.ExperimentalWrapper("The vnc-port-forward command", "experimental", ""), |
| return fmt.Errorf("invalid port: %d", port) | ||
| } | ||
|
|
||
| server, _, err := s.Client().Server().Get(s, idOrName) |
There was a problem hiding this comment.
Should we check if the server is running and return a nice error message? What happens if the server is off or rebuilding?
| return err | ||
| } | ||
|
|
||
| cmd.Printf("Accepted connection from %s\n", conn.RemoteAddr()) |
There was a problem hiding this comment.
Shouldn't this be at the top of the function?
I understand this log message as: We accepted the connection of a VNC client on port 5900, and then start the process of request the console URL, and opening the websocket connection.
| if err := s.WaitForActions(ctx, cmd, result.Action); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
I wonder if we should only log a simple "waiting" message, instead of the full blown waiting with prorgess bar. WDYT?
| @@ -0,0 +1,115 @@ | |||
| package server | |||
There was a problem hiding this comment.
Should we rename this file to vnc_port_foward.go ?
Could we also have some test for this command?
| "github.com/hetznercloud/hcloud-go/v2/hcloud" | ||
| ) | ||
|
|
||
| var VNCCmd = base.Cmd{ |
There was a problem hiding this comment.
| var VNCCmd = base.Cmd{ | |
| var VNCPortForwardCmd = base.Cmd{ |
| func ProxyWebsocket(ctx context.Context, conn net.Conn, ws *websocket.Conn) error { | ||
| closeAll := func() { | ||
| _ = conn.Close() | ||
| _ = ws.Close() | ||
| } | ||
| stop := context.AfterFunc(ctx, closeAll) | ||
| defer stop() | ||
|
|
||
| errs := make(chan error, 2) | ||
| go func() { | ||
| _, err := io.Copy(ws, conn) | ||
| errs <- err | ||
| }() | ||
| go func() { | ||
| _, err := io.Copy(conn, ws) | ||
| errs <- err | ||
| }() | ||
|
|
||
| // close both connections if one side errors | ||
| err := <-errs | ||
| closeAll() | ||
| <-errs | ||
|
|
||
| if ctx.Err() != nil || errors.Is(err, net.ErrClosed) { | ||
| return nil | ||
| } | ||
| return err | ||
| } |
There was a problem hiding this comment.
Maybe some debug logs could be useful, in case something is not working as expected.
This PR adds a new
hcloud server vnccommand that requests a web console and then forwards the port locally. This makes it possible to use any VNC client.