Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make config.Cmd and config.RunnerFunc mutually exclusive #272

Merged
merged 1 commit into from
Aug 25, 2023
Merged

Make config.Cmd and config.RunnerFunc mutually exclusive #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 118,19 @@ func (c *Client) NegotiatedVersion() int {
return c.negotiatedVersion
}

// ID returns a unique ID for the running plugin. By default this is the process
// ID (pid), but it could take other forms if RunnerFunc was provided.
func (c *Client) ID() string {
c.l.Lock()
defer c.l.Unlock()

if c.runner != nil {
return c.runner.ID()
}

return ""
}

// ClientConfig is the configuration used to initialize a new
// plugin client. After being used to initialize a plugin client,
// that configuration must not be modified again.
Expand Down Expand Up @@ -539,14 552,21 @@ func (c *Client) Start() (addr net.Addr, err error) {
// this in a {} for scoping reasons, and hopeful that the escape
// analysis will pop the stack here.
{
cmdSet := c.config.Cmd != nil
attachSet := c.config.Reattach != nil
secureSet := c.config.SecureConfig != nil
if cmdSet == attachSet {
return nil, fmt.Errorf("exactly one of Cmd or Reattach must be set")
var mutuallyExclusiveOptions int
if c.config.Cmd != nil {
mutuallyExclusiveOptions = 1
}
if c.config.Reattach != nil {
mutuallyExclusiveOptions = 1
}
if c.config.RunnerFunc != nil {
mutuallyExclusiveOptions = 1
}
if mutuallyExclusiveOptions != 1 {
return nil, fmt.Errorf("exactly one of Cmd, or Reattach, or RunnerFunc must be set")
}

if secureSet && attachSet {
if c.config.SecureConfig != nil && c.config.Reattach != nil {
return nil, ErrSecureConfigAndReattach
}
}
Expand Down Expand Up @@ -582,6 602,12 @@ func (c *Client) Start() (addr net.Addr, err error) {
}

cmd := c.config.Cmd
if cmd == nil {
// It's only possible to get here if RunnerFunc is non-nil, but we'll
// still use cmd as a spec to populate metadata for the external
// implementation to consume.
cmd = exec.Command("")
}
if !c.config.SkipHostEnv {
cmd.Env = append(cmd.Env, os.Environ()...)
}
Expand Down Expand Up @@ -731,7 757,7 @@ func (c *Client) Start() (addr net.Addr, err error) {
timeout := time.After(c.config.StartTimeout)

// Start looking for the address
c.logger.Debug("waiting for RPC address", "path", cmd.Path)
c.logger.Debug("waiting for RPC address", "plugin", runner.Name())
select {
case <-timeout:
err = errors.New("timeout while waiting for plugin to start")
Expand Down Expand Up @@ -939,6 965,9 @@ func (c *Client) checkProtoVersion(protoVersion string) (int, PluginSet, error)
//
// If this returns nil then the process hasn't been started yet. Please
// call Start or Client before calling this.
//
// Clients who specified a RunnerFunc will need to populate their own
// ReattachFunc in the returned ReattachConfig before it can be used.
func (c *Client) ReattachConfig() *ReattachConfig {
c.l.Lock()
defer c.l.Unlock()
Expand All @@ -956,11 985,16 @@ func (c *Client) ReattachConfig() *ReattachConfig {
return c.config.Reattach
}

return &ReattachConfig{
reattach := &ReattachConfig{
Protocol: c.protocol,
Addr: c.address,
Pid: c.config.Cmd.Process.Pid,
}

if c.config.Cmd != nil && c.config.Cmd.Process != nil {
reattach.Pid = c.config.Cmd.Process.Pid
}

return reattach
}

// Protocol returns the protocol of server on the remote end. This will
Expand Down
19 changes: 17 additions & 2 deletions client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 320,6 @@ func testClient_grpcSyncStdio(t *testing.T, useRunnerFunc bool) {

process := helperProcess("test-grpc")
cfg := &ClientConfig{
Cmd: process,
HandshakeConfig: testHandshake,
Plugins: testGRPCPluginMap,
AllowedProtocols: []Protocol{ProtocolGRPC},
Expand All @@ -330,8 329,11 @@ func testClient_grpcSyncStdio(t *testing.T, useRunnerFunc bool) {

if useRunnerFunc {
cfg.RunnerFunc = func(l hclog.Logger, cmd *exec.Cmd, _ string) (runner.Runner, error) {
return cmdrunner.NewCmdRunner(l, cmd)
process.Env = append(process.Env, cmd.Env...)
return cmdrunner.NewCmdRunner(l, process)
}
} else {
cfg.Cmd = process
}
c := NewClient(cfg)
defer c.Kill()
Expand Down Expand Up @@ -361,6 363,18 @@ func testClient_grpcSyncStdio(t *testing.T, useRunnerFunc bool) {
t.Fatalf("bad: %#v", raw)
}

// Check reattach config is sensible.
reattach := c.ReattachConfig()
if useRunnerFunc {
if reattach.Pid != 0 {
t.Fatal(reattach.Pid)
}
} else {
if reattach.Pid == 0 {
t.Fatal(reattach.Pid)
}
}

// Print the data
stdout := []byte("hello\nworld!")
stderr := []byte("and some error\n messages!")
Expand Down Expand Up @@ -870,6 884,7 @@ func TestClient_SkipHostEnv(t *testing.T) {
} {
t.Run(tc.helper, func(t *testing.T) {
process := helperProcess(tc.helper)
// Set env in the host process, which we'll look for in the plugin.
t.Setenv("PLUGIN_TEST_SKIP_HOST_ENV", "foo")
c := NewClient(&ClientConfig{
Cmd: process,
Expand Down