Skip to content

Commit 76b1a2f

Browse files
deansheatherdeansheather
committed
Move bidiPipe to vpn package and make public
Also removes --log-file functionality for now.
1 parent c4f30eb commit 76b1a2f

File tree

3 files changed

+76
-89
lines changed

3 files changed

+76
-89
lines changed

‎cli/vpndaemon_windows.go

Lines changed: 5 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@
33
package cli
44

55
import (
6-
"io"
7-
"os"
8-
9-
"cdr.dev/slog"
106
"golang.org/x/xerrors"
117

8+
"cdr.dev/slog"
129
"cdr.dev/slog/sloggers/sloghuman"
1310
"github.com/coder/coder/v2/vpn"
1411
"github.com/coder/serpent"
@@ -18,7 +15,6 @@ func (r *RootCmd) vpnDaemonRun() *serpent.Command {
1815
var (
1916
rpcReadHandleInt int64
2017
rpcWriteHandleInt int64
21-
logPath string
2218
)
2319

2420
cmd := &serpent.Command{
@@ -42,16 +38,10 @@ func (r *RootCmd) vpnDaemonRun() *serpent.Command {
4238
Value: serpent.Int64Of(&rpcWriteHandleInt),
4339
Required: true,
4440
},
45-
{
46-
Flag: "log-path",
47-
Env: "CODER_VPN_DAEMON_LOG_PATH",
48-
Description: "The path to the log file to write to.",
49-
Value: serpent.StringOf(&logPath),
50-
Required: false, // logs will also be written to stderr
51-
},
5241
},
5342
Handler: func(inv *serpent.Invocation) error {
5443
ctx := inv.Context()
44+
logger := inv.Logger.AppendSinks(sloghuman.Sink(inv.Stderr)).Leveled(slog.LevelDebug)
5545

5646
if rpcReadHandleInt < 0 || rpcWriteHandleInt < 0 {
5747
return xerrors.Errorf("rpc-read-handle (%v) and rpc-write-handle (%v) must be positive", rpcReadHandleInt, rpcWriteHandleInt)
@@ -60,18 +50,10 @@ func (r *RootCmd) vpnDaemonRun() *serpent.Command {
6050
return xerrors.Errorf("rpc-read-handle (%v) and rpc-write-handle (%v) must be different", rpcReadHandleInt, rpcWriteHandleInt)
6151
}
6252

63-
logger := inv.Logger.AppendSinks(sloghuman.Sink(inv.Stderr)).Leveled(slog.LevelDebug)
64-
if logPath != "" {
65-
f, err := os.Create(logPath)
66-
if err != nil {
67-
return xerrors.Errorf("create log file: %w", err)
68-
}
69-
defer f.Close()
70-
logger = logger.AppendSinks(sloghuman.Sink(f))
71-
}
72-
53+
// We don't need to worry about duplicating the handles on Windows,
54+
// which is different from Unix.
7355
logger.Info(ctx, "opening bidirectional RPC pipe", slog.F("rpc_read_handle", rpcReadHandleInt), slog.F("rpc_write_handle", rpcWriteHandleInt))
74-
pipe, err := newBidiPipe(uintptr(rpcReadHandleInt), uintptr(rpcWriteHandleInt))
56+
pipe, err := vpn.NewBidirectionalPipe(uintptr(rpcReadHandleInt), uintptr(rpcWriteHandleInt))
7557
if err != nil {
7658
return xerrors.Errorf("create bidirectional RPC pipe: %w", err)
7759
}
@@ -91,58 +73,3 @@ func (r *RootCmd) vpnDaemonRun() *serpent.Command {
9173

9274
return cmd
9375
}
94-
95-
type bidiPipe struct {
96-
read *os.File
97-
write *os.File
98-
}
99-
100-
var _ io.ReadWriteCloser = bidiPipe{}
101-
102-
func newBidiPipe(readHandle, writeHandle uintptr) (bidiPipe, error) {
103-
read := os.NewFile(readHandle, "rpc_read")
104-
_, err := read.Stat()
105-
if err != nil {
106-
return bidiPipe{}, xerrors.Errorf("stat rpc_read pipe (handle=%v): %w", readHandle, err)
107-
}
108-
write := os.NewFile(writeHandle, "rpc_write")
109-
_, err = write.Stat()
110-
if err != nil {
111-
return bidiPipe{}, xerrors.Errorf("stat rpc_write pipe (handle=%v): %w", writeHandle, err)
112-
}
113-
return bidiPipe{
114-
read: read,
115-
write: write,
116-
}, nil
117-
}
118-
119-
// Read implements io.Reader. Data is read from the read pipe.
120-
func (b bidiPipe) Read(p []byte) (int, error) {
121-
n, err := b.read.Read(p)
122-
if err != nil {
123-
return n, xerrors.Errorf("read from rpc_read pipe (handle=%v): %w", b.read.Fd(), err)
124-
}
125-
return n, nil
126-
}
127-
128-
// Write implements io.Writer. Data is written to the write pipe.
129-
func (b bidiPipe) Write(p []byte) (n int, err error) {
130-
n, err = b.write.Write(p)
131-
if err != nil {
132-
return n, xerrors.Errorf("write to rpc_write pipe (handle=%v): %w", b.write.Fd(), err)
133-
}
134-
return n, nil
135-
}
136-
137-
// Close implements io.Closer. Both the read and write pipes are closed.
138-
func (b bidiPipe) Close() error {
139-
err := b.read.Close()
140-
if err != nil {
141-
return xerrors.Errorf("close rpc_read pipe (handle=%v): %w", b.read.Fd(), err)
142-
}
143-
err = b.write.Close()
144-
if err != nil {
145-
return xerrors.Errorf("close rpc_write pipe (handle=%v): %w", b.write.Fd(), err)
146-
}
147-
return nil
148-
}

‎cli/vpndaemon_windows_test.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package cli_test
55
import (
66
"fmt"
77
"os"
8-
"path/filepath"
98
"testing"
109

1110
"github.com/stretchr/testify/require"
@@ -64,7 +63,7 @@ func TestVPNDaemonRun(t *testing.T) {
6463
}
6564
})
6665

67-
t.Run("LogFile", func(t *testing.T) {
66+
t.Run("StartsTunnel", func(t *testing.T) {
6867
t.Parallel()
6968

7069
r1, w1, err := os.Pipe()
@@ -76,11 +75,8 @@ func TestVPNDaemonRun(t *testing.T) {
7675
defer r2.Close()
7776
defer w2.Close()
7877

79-
logDir := t.TempDir()
80-
logPath := filepath.Join(logDir, "coder-daemon.log")
81-
8278
ctx := testutil.Context(t, testutil.WaitLong)
83-
inv, _ := clitest.New(t, "vpn-daemon", "run", "--rpc-read-handle", fmt.Sprint(r1.Fd()), "--rpc-write-handle", fmt.Sprint(w2.Fd()), "--log-path", logPath)
79+
inv, _ := clitest.New(t, "vpn-daemon", "run", "--rpc-read-handle", fmt.Sprint(r1.Fd()), "--rpc-write-handle", fmt.Sprint(w2.Fd()))
8480
waiter := clitest.StartWithWaiter(t, inv.WithContext(ctx))
8581

8682
// Send garbage which should cause the handshake to fail and the daemon
@@ -90,11 +86,6 @@ func TestVPNDaemonRun(t *testing.T) {
9086
waiter.Cancel()
9187
err = waiter.Wait()
9288
require.ErrorContains(t, err, "handshake failed")
93-
94-
// Check that the log file was created and is not empty.
95-
stat, err := os.Stat(logPath)
96-
require.NoError(t, err)
97-
require.Greater(t, stat.Size(), int64(0))
9889
})
9990

10091
// TODO: once the VPN tunnel functionality is implemented, add tests that

‎vpn/pipe.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package vpn
2+
3+
import (
4+
"io"
5+
"os"
6+
7+
"github.com/hashicorp/go-multierror"
8+
"golang.org/x/xerrors"
9+
)
10+
11+
// BidirectionalPipe combines a pair of files that can be used for bidirectional
12+
// communication.
13+
type BidirectionalPipe struct {
14+
read *os.File
15+
write *os.File
16+
}
17+
18+
var _ io.ReadWriteCloser = BidirectionalPipe{}
19+
20+
// NewBidirectionalPipe creates a new BidirectionalPipe from the given file
21+
// descriptors.
22+
func NewBidirectionalPipe(readFd, writeFd uintptr) (BidirectionalPipe, error) {
23+
read := os.NewFile(readFd, "pipe_read")
24+
_, err := read.Stat()
25+
if err != nil {
26+
return BidirectionalPipe{}, xerrors.Errorf("stat pipe_read (fd=%v): %w", readFd, err)
27+
}
28+
write := os.NewFile(writeFd, "pipe_write")
29+
_, err = write.Stat()
30+
if err != nil {
31+
return BidirectionalPipe{}, xerrors.Errorf("stat pipe_write (fd=%v): %w", writeFd, err)
32+
}
33+
return BidirectionalPipe{
34+
read: read,
35+
write: write,
36+
}, nil
37+
}
38+
39+
// Read implements io.Reader. Data is read from the read pipe.
40+
func (b BidirectionalPipe) Read(p []byte) (int, error) {
41+
n, err := b.read.Read(p)
42+
if err != nil {
43+
return n, xerrors.Errorf("read from pipe_read (fd=%v): %w", b.read.Fd(), err)
44+
}
45+
return n, nil
46+
}
47+
48+
// Write implements io.Writer. Data is written to the write pipe.
49+
func (b BidirectionalPipe) Write(p []byte) (n int, err error) {
50+
n, err = b.write.Write(p)
51+
if err != nil {
52+
return n, xerrors.Errorf("write to pipe_write (fd=%v): %w", b.write.Fd(), err)
53+
}
54+
return n, nil
55+
}
56+
57+
// Close implements io.Closer. Both the read and write pipes are closed.
58+
func (b BidirectionalPipe) Close() error {
59+
var err error
60+
rErr := b.read.Close()
61+
if rErr != nil {
62+
err = multierror.Append(err, xerrors.Errorf("close pipe_read (fd=%v): %w", b.read.Fd(), rErr))
63+
}
64+
wErr := b.write.Close()
65+
if err != nil {
66+
err = multierror.Append(err, xerrors.Errorf("close pipe_write (fd=%v): %w", b.write.Fd(), wErr))
67+
}
68+
return err
69+
}

0 commit comments

Comments
 (0)