Skip to content

Commit

Permalink
add ProtocolVersion to ReattachConfig (#171)
Browse files Browse the repository at this point in the history
* add ProtocolVersion to ReattachConfig

A client connecting to a plugin process needs to know the resolved ProtocolVersion, so this PR adds that information to the returned reattach config. Prior to this, client.NegotiatedVersion() always returned 0.

* Add a test for correct protocol version in reattach config
  • Loading branch information
mildwonkey authored Mar 31, 2021
1 parent 0c19a13 commit 044aadd
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
11 changes: 8 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,10 @@ type ClientConfig struct {
// already-running plugin process. You can retrieve this information by
// calling ReattachConfig on Client.
type ReattachConfig struct {
Protocol Protocol
Addr net.Addr
Pid int
Protocol Protocol
ProtocolVersion int
Addr net.Addr
Pid int

// Test is set to true if this is reattaching to to a plugin in "test mode"
// (see ServeConfig.Test). In this mode, client.Kill will NOT kill the
Expand Down Expand Up @@ -839,6 +840,10 @@ func (c *Client) reattach() (net.Addr, error) {
c.protocol = ProtocolNetRPC
}

if c.config.Reattach.Test {
c.negotiatedVersion = c.config.Reattach.ProtocolVersion
}

// If we're in test mode, we do NOT set the process. This avoids the
// process being killed (the only purpose we have for c.process), since
// in test mode the process is responsible for exiting on its own.
Expand Down
9 changes: 5 additions & 4 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,10 +415,11 @@ func Serve(opts *ServeConfig) {
// quite ready if they connect immediately but the client should
// retry a few times.
ch <- &ReattachConfig{
Protocol: protoType,
Addr: listener.Addr(),
Pid: os.Getpid(),
Test: true,
Protocol: protoType,
ProtocolVersion: protoVersion,
Addr: listener.Addr(),
Pid: os.Getpid(),
Test: true,
}
}

Expand Down
5 changes: 5 additions & 0 deletions server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ func TestServer_testMode(t *testing.T) {
t.Fatal("config should not be nil")
}

// Check that the reattach config includes the negotiated protocol version
if config.ProtocolVersion != int(testHandshake.ProtocolVersion) {
t.Fatalf("wrong protocol version in reattach config. got %d, expected %d", config.ProtocolVersion, testHandshake.ProtocolVersion)
}

// Connect!
c := NewClient(&ClientConfig{
Cmd: nil,
Expand Down

0 comments on commit 044aadd

Please sign in to comment.