rpc: allow using both gob/protobuf for structs

As follow-up to the introduction of the protobufs for HCLSpec, we
introduce a new environment variable and code to use those structures,
so we don't use gob for serialising HCLSpecs.

This should make the plugins and packer able to transmit data
over-the-wire without using gob for the most part (the communicators
still use it, and will probably need some work to replace).

.github/workflows/go-test.yml

@@ -41,6 +41,9 @@ jobs:
         run: |
           mkdir -p ${{ env.TEST_RESULTS_PATH }}/packer-plugin-sdk
 
+      - name: Install buf
+        uses: bufbuild/[email protected]
+
       - name: Run gofmt
         run: |
             make fmt-check

Makefile

@@ -59,7 +59,7 @@ generate: install-gen-deps ## Generate dynamically generated code
 	@find ./ -type f | xargs grep -l '^// Code generated' | xargs rm -f
 	PROJECT_ROOT="$(CURDIR)" go generate ./...
 	go fmt bootcommand/boot_command.go
-# 	go run ./cmd/generate-fixer-deprecations
+	buf generate
 
 generate-check: generate ## Check go code generation is on par
 	@echo "==> Checking that auto-generated code is not changed..."

buf.gen.yaml

@@ -0,0 +1,7 @@
+version: v2
+plugins:
+- remote: buf.build/protocolbuffers/go:v1.28.1
+  out: rpc
+  opt: paths=source_relative
+inputs:
+- proto_file: rpc/hcl_spec.proto

plugin/set.go

@@ -33,6 +33,7 @@ type Set struct {
 	version        string
 	sdkVersion     string
 	apiVersion     string
+	useProto       bool
 	Builders       map[string]packersdk.Builder
 	PostProcessors map[string]packersdk.PostProcessor
 	Provisioners   map[string]packersdk.Provisioner
@@ -115,11 +116,42 @@ func (i *Set) Run() error {
 	return i.RunCommand(args...)
 }
 
+// parseProtobufFlag walks over the args to find if `--protobuf` is set.
+//
+// It then returns the args without it for the commands to process them.
+func (i *Set) parseProtobufFlag(args ...string) []string {
+	protobufPos := -1
+	for i, arg := range args {
+		if arg == "--protobuf" {
+			protobufPos = i
+			break
+		}
+	}
+
+	if protobufPos == -1 {
+		return args
+	}
+
+	i.useProto = true
+
+	if protobufPos == 0 {
+		return args[1:]
+	}
+
+	if protobufPos == len(args)-1 {
+		return args[:len(args)-1]
+	}
+
+	return append(args[:protobufPos], args[protobufPos+1:]...)
+}
+
 func (i *Set) RunCommand(args ...string) error {
 	if len(args) < 1 {
 		return fmt.Errorf("needs at least one argument")
 	}
 
+	args = i.parseProtobufFlag(args...)
+
 	switch args[0] {
 	case "describe":
 		return i.jsonDescribe(os.Stdout)
@@ -139,6 +171,7 @@ func (i *Set) start(kind, name string) error {
 	if err != nil {
 		return err
 	}
+	server.UseProto = i.useProto
 
 	log.Printf("[TRACE] starting %s %s", kind, name)
 

rpc/client.go

@@ -21,6 +21,9 @@ type Client struct {
 	mux      *muxBroker
 	client   *rpc.Client
 	closeMux bool
+	// UseProto makes it so that clients started from this will use
+	// protobuf/msgpack for serialisation instead of gob
+	UseProto bool
 }
 
 func NewClient(rwc io.ReadWriteCloser) (*Client, error) {
@@ -76,6 +79,13 @@ func (c *Client) Artifact() packer.Artifact {
 		commonClient: commonClient{
 			endpoint: DefaultArtifactEndpoint,
 			client:   c.client,
+			// Setting useProto to false is essentially a noop for
+			// this type of client since they don't exchange cty
+			// values, and there's no HCLSpec object tied to this.
+			//
+			// For documentation purposes though, we keep it visible
+			// in order to change this later if it becomes relevant.
+			useProto: false,
 		},
 	}
 }
@@ -86,6 +96,13 @@ func (c *Client) Build() packer.Build {
 			endpoint: DefaultBuildEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			// Setting useProto to false is essentially a noop for
+			// this type of client since they don't exchange cty
+			// values, and there's no HCLSpec object tied to this.
+			//
+			// For documentation purposes though, we keep it visible
+			// in order to change this later if it becomes relevant.
+			useProto: false,
 		},
 	}
 }
@@ -96,6 +113,7 @@ func (c *Client) Builder() packer.Builder {
 			endpoint: DefaultBuilderEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			useProto: c.UseProto,
 		},
 	}
 }
@@ -106,6 +124,13 @@ func (c *Client) Communicator() packer.Communicator {
 			endpoint: DefaultCommunicatorEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			// Setting useProto to false is essentially a noop for
+			// this type of client since they don't exchange cty
+			// values, and there's no HCLSpec object tied to this.
+			//
+			// For documentation purposes though, we keep it visible
+			// in order to change this later if it becomes relevant.
+			useProto: false,
 		},
 	}
 }
@@ -116,6 +141,13 @@ func (c *Client) Hook() packer.Hook {
 			endpoint: DefaultHookEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			// Setting useProto to false is essentially a noop for
+			// this type of client since they don't exchange cty
+			// values, and there's no HCLSpec object tied to this.
+			//
+			// For documentation purposes though, we keep it visible
+			// in order to change this later if it becomes relevant.
+			useProto: false,
 		},
 	}
 }
@@ -126,6 +158,7 @@ func (c *Client) PostProcessor() packer.PostProcessor {
 			endpoint: DefaultPostProcessorEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			useProto: c.UseProto,
 		},
 	}
 }
@@ -136,6 +169,7 @@ func (c *Client) Provisioner() packer.Provisioner {
 			endpoint: DefaultProvisionerEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			useProto: c.UseProto,
 		},
 	}
 }
@@ -146,6 +180,7 @@ func (c *Client) Datasource() packer.Datasource {
 			endpoint: DefaultDatasourceEndpoint,
 			client:   c.client,
 			mux:      c.mux,
+			useProto: c.UseProto,
 		},
 	}
 }
@@ -155,6 +190,13 @@ func (c *Client) Ui() packer.Ui {
 		commonClient: commonClient{
 			endpoint: DefaultUiEndpoint,
 			client:   c.client,
+			// Setting useProto to false is essentially a noop for
+			// this type of client since they don't exchange cty
+			// values, and there's no HCLSpec object tied to this.
+			//
+			// For documentation purposes though, we keep it visible
+			// in order to change this later if it becomes relevant.
+			useProto: false,
 		},
 		endpoint: DefaultUiEndpoint,
 	}

rpc/common.go

@@ -7,10 +7,13 @@ import (
 	"bytes"
 	"encoding/gob"
 	"fmt"
+	"log"
 	"net/rpc"
+	"reflect"
 
 	"github.com/hashicorp/hcl/v2/hcldec"
 	"github.com/zclconf/go-cty/cty"
+	"google.golang.org/protobuf/proto"
 )
 
 // commonClient allows to rpc call funcs that can be defined on the different
@@ -22,6 +25,12 @@ type commonClient struct {
 	endpoint string
 	client   *rpc.Client
 	mux      *muxBroker
+
+	// useProto lets us determine whether or not we should use protobuf for serialising
+	// data over RPC instead of gob.
+	//
+	// This is controlled by Packer using the `--use-proto` flag on plugin commands.
+	useProto bool
 }
 
 type commonServer struct {
@@ -30,6 +39,12 @@ type commonServer struct {
 	selfConfigurable interface {
 		ConfigSpec() hcldec.ObjectSpec
 	}
+
+	// useProto lets us determine whether or not we should use protobuf for serialising
+	// data over RPC instead of gob.
+	//
+	// This is controlled by Packer using the `--use-proto` flag on plugin commands.
+	useProto bool
 }
 
 type ConfigSpecResponse struct {
@@ -48,21 +63,91 @@ func (p *commonClient) ConfigSpec() hcldec.ObjectSpec {
 		panic(err.Error())
 	}
 
-	res := hcldec.ObjectSpec{}
-	err := gob.NewDecoder(bytes.NewReader(resp.ConfigSpec)).Decode(&res)
+	// Legacy: this will need to be removed when we discontinue gob-encoding
+	//
+	// This is required for backwards compatibility for now, but using
+	// gob to encode the spec objects will fail against the upstream cty
+	// library, since they removed support for it.
+	//
+	// This will be a breaking change, as older plugins won't be able to
+	// communicate with Packer any longer.
+	if !p.useProto {
+		log.Printf("[DEBUG] - common: receiving ConfigSpec as gob")
+		res := hcldec.ObjectSpec{}
+		err := gob.NewDecoder(bytes.NewReader(resp.ConfigSpec)).Decode(&res)
+		if err != nil {
+			panic(fmt.Errorf("failed to decode HCL spec from gob: %s", err))
+		}
+		return res
+	}
+
+	log.Printf("[DEBUG] - common: receiving ConfigSpec as protobuf")
+	spec, err := protobufToHCL2Spec(resp.ConfigSpec)
 	if err != nil {
-		panic("ici:" + err.Error())
+		panic(err)
 	}
-	return res
+
+	return spec
 }
 
 func (s *commonServer) ConfigSpec(_ interface{}, reply *ConfigSpecResponse) error {
 	spec := s.selfConfigurable.ConfigSpec()
-	b := bytes.NewBuffer(nil)
-	err := gob.NewEncoder(b).Encode(spec)
-	reply.ConfigSpec = b.Bytes()
 
-	return err
+	if !s.useProto {
+		log.Printf("[DEBUG] - common: sending ConfigSpec as gob")
+		b := &bytes.Buffer{}
+		err := gob.NewEncoder(b).Encode(spec)
+		if err != nil {
+			return fmt.Errorf("failed to encode spec from gob: %s", err)
+		}
+		reply.ConfigSpec = b.Bytes()
+
+		return nil
+	}
+
+	log.Printf("[DEBUG] - common: sending ConfigSpec as protobuf")
+	rawBytes, err := hcl2SpecToProtobuf(spec)
+	if err != nil {
+		return fmt.Errorf("failed to encode HCL spec from protobuf: %s", err)
+	}
+	reply.ConfigSpec = rawBytes
+
+	return nil
+}
+
+// hcl2SpecToProtobuf converts a hcldec.ObjectSpec to a protobuf-serialised
+// byte array so it can then be used to send to a Plugin/Packer.
+func hcl2SpecToProtobuf(spec hcldec.ObjectSpec) ([]byte, error) {
+	ret, err := ToProto(spec)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert hcldec.Spec to hclspec.Spec: %s", err)
+	}
+	rawBytes, err := proto.Marshal(ret)
+	if err != nil {
+		return nil, fmt.Errorf("failed to serialise hclspec.Spec to protobuf: %s", err)
+	}
+
+	return rawBytes, nil
+}
+
+// protobufToHCL2Spec converts a protobuf-encoded spec to a usable hcldec.Spec.
+func protobufToHCL2Spec(serData []byte) (hcldec.ObjectSpec, error) {
+	confSpec := &Spec{}
+	err := proto.Unmarshal(serData, confSpec)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal hclspec.Spec from raw protobuf: %q", err)
+	}
+	spec, err := confSpec.FromProto()
+	if err != nil {
+		return nil, fmt.Errorf("failed to decode HCL spec: %q", err)
+	}
+
+	obj, ok := spec.(*hcldec.ObjectSpec)
+	if !ok {
+		return nil, fmt.Errorf("decoded HCL spec is not an object spec: %s", reflect.TypeOf(spec).String())
+	}
+
+	return *obj, nil
 }
 
 func init() {