plugin: Serve via tf5server/tf6server rather than go-plugin directly (#…

…857)

Providers will automatically receive upstream enhancements to provider servers, including protocol logging. Lower level dependencies in this SDK, such as gRPC and go-plugin versions, can instead be managed upstream, which will ensure consistency across the SDKs.

.changelog/857.txt

@@ -0,0 +1,11 @@
+```release-note:note
+plugin: The `Debug` function has been deprecated in preference of setting the `Debug` field in the `ServeOpts` passed into the `Serve` function.
+```
+
+```release-note:enhancement
+plugin: Increased maximum gRPC send and receive message size limit to 256MB
+```
+
+```release-note:enhancement
+plugin: Added support for writing protocol data to disk by setting `TF_LOG_SDK_PROTO_DATA_DIR` environment variable
+```

.github/dependabot.yml

@@ -5,3 +5,8 @@ updates:
     schedule:
       interval: "daily"
     open-pull-requests-limit: 20
+    ignore:
+      # go-hclog should only be updated via terraform-plugin-log
+      - dependency-name: "github.com/hashicorp/go-hclog"
+      # go-plugin should only be updated via terraform-plugin-go
+      - dependency-name: "github.com/hashicorp/go-plugin"

go.mod

@@ -34,5 +34,4 @@ require (
 	golang.org/x/tools v0.0.0-20200713011307-fd294ab11aed
 	google.golang.org/appengine v1.6.6 // indirect
 	google.golang.org/genproto v0.0.0-20200711021454-869866162049 // indirect
-	google.golang.org/grpc v1.44.0
 )

plugin/debug.go

@@ -2,13 +2,7 @@ package plugin
 
 import (
 	"context"
-	"encoding/json"
 	"errors"
-	"fmt"
-	"os"
-	"os/signal"
-	"runtime"
-	"strings"
 	"time"
 
 	"github.com/hashicorp/go-plugin"
@@ -37,6 +31,10 @@ func DebugServe(ctx context.Context, opts *ServeOpts) (ReattachConfig, <-chan st
 	reattachCh := make(chan *plugin.ReattachConfig)
 	closeCh := make(chan struct{})
 
+	if opts == nil {
+		return ReattachConfig{}, closeCh, errors.New("ServeOpts must be passed in with at least GRPCProviderFunc, GRPCProviderV6Func, or ProviderFunc")
+	}
+
 	opts.TestConfig = &plugin.ServeTestConfig{
 		Context:          ctx,
 		ReattachConfigCh: reattachCh,
@@ -71,48 +69,20 @@ func DebugServe(ctx context.Context, opts *ServeOpts) (ReattachConfig, <-chan st
 // Debug starts a debug server and controls its lifecycle, printing the
 // information needed for Terraform to connect to the provider to stdout.
 // os.Interrupt will be captured and used to stop the server.
+//
+// Deprecated: Use the Serve function with the ServeOpts Debug field instead.
 func Debug(ctx context.Context, providerAddr string, opts *ServeOpts) error {
-	ctx, cancel := context.WithCancel(ctx)
-	// Ctrl-C will stop the server
-	sigCh := make(chan os.Signal, 1)
-	signal.Notify(sigCh, os.Interrupt)
-	defer func() {
-		signal.Stop(sigCh)
-		cancel()
-	}()
-	config, closeCh, err := DebugServe(ctx, opts)
-	if err != nil {
-		return fmt.Errorf("Error launching debug server: %w", err)
-	}
-	go func() {
-		select {
-		case <-sigCh:
-			cancel()
-		case <-ctx.Done():
-		}
-	}()
-	reattachBytes, err := json.Marshal(map[string]ReattachConfig{
-		providerAddr: config,
-	})
-	if err != nil {
-		return fmt.Errorf("Error building reattach string: %w", err)
+	if opts == nil {
+		return errors.New("ServeOpts must be passed in with at least GRPCProviderFunc, GRPCProviderV6Func, or ProviderFunc")
 	}
 
-	reattachStr := string(reattachBytes)
-
-	fmt.Printf("Provider started, to attach Terraform set the TF_REATTACH_PROVIDERS env var:\n\n")
-	switch runtime.GOOS {
-	case "windows":
-		fmt.Printf("\tCommand Prompt:\tset \"TF_REATTACH_PROVIDERS=%s\"\n", reattachStr)
-		fmt.Printf("\tPowerShell:\t$env:TF_REATTACH_PROVIDERS='%s'\n", strings.ReplaceAll(reattachStr, `'`, `''`))
-	case "linux", "darwin":
-		fmt.Printf("\tTF_REATTACH_PROVIDERS='%s'\n", strings.ReplaceAll(reattachStr, `'`, `'"'"'`))
-	default:
-		fmt.Println(reattachStr)
+	opts.Debug = true
+
+	if opts.ProviderAddr == "" {
+		opts.ProviderAddr = providerAddr
 	}
-	fmt.Println("")
 
-	// wait for the server to be done
-	<-closeCh
+	Serve(opts)
+
 	return nil
 }

plugin/serve.go

@@ -1,12 +1,12 @@
 package plugin
 
 import (
+	"errors"
 	"log"
 
 	hclog "github.com/hashicorp/go-hclog"
 	"github.com/hashicorp/go-plugin"
 	testing "github.com/mitchellh/go-testing-interface"
-	"google.golang.org/grpc"
 
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server"
@@ -18,10 +18,16 @@ import (
 const (
 	// The constants below are the names of the plugins that can be dispensed
 	// from the plugin server.
+	//
+	// Deprecated: This is no longer used, but left for backwards compatibility
+	// since it is exported. It will be removed in the next major version.
 	ProviderPluginName = "provider"
 )
 
 // Handshake is the HandshakeConfig used to configure clients and servers.
+//
+// Deprecated: This is no longer used, but left for backwards compatibility
+// since it is exported. It will be removed in the next major version.
 var Handshake = plugin.HandshakeConfig{
 	// The magic cookie values should NEVER be changed.
 	MagicCookieKey:   "TF_PLUGIN_MAGIC_COOKIE",
@@ -45,11 +51,20 @@ type ServeOpts struct {
 	// Logger is the logger that go-plugin will use.
 	Logger hclog.Logger
 
+	// Debug starts a debug server and controls its lifecycle, printing the
+	// information needed for Terraform to connect to the provider to stdout.
+	// os.Interrupt will be captured and used to stop the server.
+	//
+	// This option cannot be combined with TestConfig.
+	Debug bool
+
 	// TestConfig should only be set when the provider is being tested; it
 	// will opt out of go-plugin's lifecycle management and other features,
 	// and will use the supplied configuration options to control the
 	// plugin's lifecycle and communicate connection information. See the
 	// go-plugin GoDoc for more information.
+	//
+	// This option cannot be combined with Debug.
 	TestConfig *plugin.ServeTestConfig
 
 	// Set NoLogOutputOverride to not override the log output with an hclog
@@ -69,6 +84,11 @@ type ServeOpts struct {
 // Serve serves a plugin. This function never returns and should be the final
 // function called in the main function of the plugin.
 func Serve(opts *ServeOpts) {
+	if opts.Debug && opts.TestConfig != nil {
+		log.Printf("[ERROR] Error starting provider: cannot set both Debug and TestConfig")
+		return
+	}
+
 	if !opts.NoLogOutputOverride {
 		// In order to allow go-plugin to correctly pass log-levels through to
 		// terraform, we need to use an hclog.Logger with JSON output. We can
@@ -84,65 +104,126 @@ func Serve(opts *ServeOpts) {
 		log.SetOutput(logger.StandardWriter(&hclog.StandardLoggerOptions{InferLevels: true}))
 	}
 
-	// since the plugins may not yet be aware of the new protocol, we
-	// automatically wrap the plugins in the grpc shims.
-	if opts.GRPCProviderFunc == nil && opts.ProviderFunc != nil {
+	if opts.ProviderAddr == "" {
+		opts.ProviderAddr = "provider"
+	}
+
+	var err error
+
+	switch {
+	case opts.ProviderFunc != nil && opts.GRPCProviderFunc == nil:
 		opts.GRPCProviderFunc = func() tfprotov5.ProviderServer {
 			return schema.NewGRPCProviderServer(opts.ProviderFunc())
 		}
+		err = tf5serverServe(opts)
+	case opts.GRPCProviderFunc != nil:
+		err = tf5serverServe(opts)
+	case opts.GRPCProviderV6Func != nil:
+		err = tf6serverServe(opts)
+	default:
+		err = errors.New("no provider server defined in ServeOpts")
 	}
 
-	serveConfig := plugin.ServeConfig{
-		HandshakeConfig: Handshake,
-		GRPCServer: func(opts []grpc.ServerOption) *grpc.Server {
-			return grpc.NewServer(opts...)
-		},
-		Logger: opts.Logger,
-		Test:   opts.TestConfig,
+	if err != nil {
+		log.Printf("[ERROR] Error starting provider: %s", err)
 	}
+}
 
-	// assume we have either a v5 or a v6 provider
-	if opts.GRPCProviderFunc != nil {
-		provider := opts.GRPCProviderFunc()
-		addr := opts.ProviderAddr
-		if addr == "" {
-			addr = "provider"
-		}
-		serveConfig.VersionedPlugins = map[int]plugin.PluginSet{
-			5: {
-				ProviderPluginName: &tf5server.GRPCProviderPlugin{
-					GRPCProvider: func() tfprotov5.ProviderServer {
-						return provider
-					},
-					Name: addr,
-				},
-			},
-		}
-		if opts.UseTFLogSink != nil {
-			serveConfig.VersionedPlugins[5][ProviderPluginName].(*tf5server.GRPCProviderPlugin).Opts = append(serveConfig.VersionedPlugins[5][ProviderPluginName].(*tf5server.GRPCProviderPlugin).Opts, tf5server.WithLoggingSink(opts.UseTFLogSink))
-		}
+func tf5serverServe(opts *ServeOpts) error {
+	var tf5serveOpts []tf5server.ServeOpt
 
-	} else if opts.GRPCProviderV6Func != nil {
-		provider := opts.GRPCProviderV6Func()
-		addr := opts.ProviderAddr
-		if addr == "" {
-			addr = "provider"
-		}
-		serveConfig.VersionedPlugins = map[int]plugin.PluginSet{
-			6: {
-				ProviderPluginName: &tf6server.GRPCProviderPlugin{
-					GRPCProvider: func() tfprotov6.ProviderServer {
-						return provider
-					},
-					Name: addr,
-				},
-			},
-		}
-		if opts.UseTFLogSink != nil {
-			serveConfig.VersionedPlugins[6][ProviderPluginName].(*tf6server.GRPCProviderPlugin).Opts = append(serveConfig.VersionedPlugins[6][ProviderPluginName].(*tf6server.GRPCProviderPlugin).Opts, tf6server.WithLoggingSink(opts.UseTFLogSink))
-		}
+	if opts.Debug {
+		tf5serveOpts = append(tf5serveOpts, tf5server.WithManagedDebug())
+	}
+
+	if opts.Logger != nil {
+		tf5serveOpts = append(tf5serveOpts, tf5server.WithGoPluginLogger(opts.Logger))
+	}
+
+	if opts.TestConfig != nil {
+		// Convert send-only channels to bi-directional channels to appease
+		// the compiler. WithDebug is errantly defined to require
+		// bi-directional when send-only is actually needed, which may be
+		// fixed in the future so the opts.TestConfig channels can be passed
+		// through directly.
+		closeCh := make(chan struct{})
+		reattachConfigCh := make(chan *plugin.ReattachConfig)
+
+		go func() {
+			// Always forward close channel receive, since its signaling that
+			// the channel is closed.
+			val := <-closeCh
+			opts.TestConfig.CloseCh <- val
+		}()
+
+		go func() {
+			val, ok := <-reattachConfigCh
+
+			if ok {
+				opts.TestConfig.ReattachConfigCh <- val
+			}
+		}()
+
+		tf5serveOpts = append(tf5serveOpts, tf5server.WithDebug(
+			opts.TestConfig.Context,
+			reattachConfigCh,
+			closeCh),
+		)
+	}
+
+	if opts.UseTFLogSink != nil {
+		tf5serveOpts = append(tf5serveOpts, tf5server.WithLoggingSink(opts.UseTFLogSink))
+	}
+
+	return tf5server.Serve(opts.ProviderAddr, opts.GRPCProviderFunc, tf5serveOpts...)
+}
+
+func tf6serverServe(opts *ServeOpts) error {
+	var tf6serveOpts []tf6server.ServeOpt
+
+	if opts.Debug {
+		tf6serveOpts = append(tf6serveOpts, tf6server.WithManagedDebug())
+	}
+
+	if opts.Logger != nil {
+		tf6serveOpts = append(tf6serveOpts, tf6server.WithGoPluginLogger(opts.Logger))
+	}
+
+	if opts.TestConfig != nil {
+		// Convert send-only channels to bi-directional channels to appease
+		// the compiler. WithDebug is errantly defined to require
+		// bi-directional when send-only is actually needed, which may be
+		// fixed in the future so the opts.TestConfig channels can be passed
+		// through directly.
+		closeCh := make(chan struct{})
+		reattachConfigCh := make(chan *plugin.ReattachConfig)
+
+		go func() {
+			val, ok := <-closeCh
+
+			if ok {
+				opts.TestConfig.CloseCh <- val
+			}
+		}()
+
+		go func() {
+			val, ok := <-reattachConfigCh
+
+			if ok {
+				opts.TestConfig.ReattachConfigCh <- val
+			}
+		}()
+
+		tf6serveOpts = append(tf6serveOpts, tf6server.WithDebug(
+			opts.TestConfig.Context,
+			reattachConfigCh,
+			closeCh),
+		)
+	}
 
+	if opts.UseTFLogSink != nil {
+		tf6serveOpts = append(tf6serveOpts, tf6server.WithLoggingSink(opts.UseTFLogSink))
 	}
 
-	plugin.Serve(&serveConfig)
+	return tf6server.Serve(opts.ProviderAddr, opts.GRPCProviderV6Func, tf6serveOpts...)
 }