From 573396108a76cd2f7f6c1e3c651304a3f3e52fee Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 24 Mar 2021 13:22:54 -0400 Subject: [PATCH 1/3] 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. --- client.go | 8 +++++--- server.go | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/client.go b/client.go index 780a3121..29a79222 100644 --- a/client.go +++ b/client.go @@ -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 @@ -838,6 +839,7 @@ func (c *Client) reattach() (net.Addr, error) { // Default the protocol to net/rpc for backwards compatibility c.protocol = ProtocolNetRPC } + 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 diff --git a/server.go b/server.go index 80f0ac39..7a58cc39 100644 --- a/server.go +++ b/server.go @@ -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, } } From 096aa0ea2fb379207e74ae7669d38a17ac8fe272 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 31 Mar 2021 09:19:34 -0400 Subject: [PATCH 2/3] only set c.negotiatedVersion if we are in reattach test mode to reduce changes for existing callers --- client.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/client.go b/client.go index 29a79222..6b42975b 100644 --- a/client.go +++ b/client.go @@ -839,7 +839,10 @@ func (c *Client) reattach() (net.Addr, error) { // Default the protocol to net/rpc for backwards compatibility c.protocol = ProtocolNetRPC } - c.negotiatedVersion = c.config.Reattach.ProtocolVersion + + 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 From bc2dbfce56b47f933dcc9b5fa38b5175cebe5ded Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 31 Mar 2021 10:44:05 -0400 Subject: [PATCH 3/3] Add a test for correct protocol version in reattach config --- server_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server_test.go b/server_test.go index 099cfc76..5fec4502 100644 --- a/server_test.go +++ b/server_test.go @@ -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,