Update ugorji/go/codec to v1.2.4

[angdraug]

This is a continuation of hashicorp/packer#10146 that was submitted before the rpc code was moved from packer to packer-plugin-sdk.

When running Packer built with modern[0] version of ugorji using a plugin built from source and therefore with the 2015 version currently pinned in Packer's go.mod, I hit ugorji/go#269 when my plugin tries to pipe into a remote "cat > '%s'":

Build 'base-20201021-5f90bdf5-490e-582b-fb1d-b5d4315f5eaa' errored after 25 seconds 772 milliseconds: msgpack decode error [pos 274]: invalid length of bytes for decoding time - expecting 4 or 8 or 12, got 15

[0] As in, newer than ugorji/go@5a66da2 from 2017, e.g. Packer as packaged in Debian.

The same pipe works fine when either running Packer built from source (with the same pin to old ugorji), or forcing the plugin to build with the modern ugorji:

replace github.com/hashicorp/packer => github.com/angdraug/packer v1.6.4-ugorji-go-v1.1.13

Bringing ugorji in Packer to the current version will leap over this and other incompatibilities that have built up in ugorji over the years, will make custom plugins work out of the box with the Debian build of Packer, and will allow plugin writers to use modules that depend on newer versions of ugorji.

[azr]

Hello there, thanks for opening again 🙂 . This looks fine by me; I did some testing and some acceptance testing too.

I think we should merge this now before Packer 1.7.0 is released, since we bumped the API MAJOR version.
We should also probably do much more testing; and some reading of the diff for the lib.
This dependency could become a difficult tech debt; and even if we introduce a bug by merging this; the lib seems well maintained so that we could PR our way out of a bug.

I think that the whole Packer team needs to agree with this though !

For context: the codec is what marshals a go struct into a string and back. In core to plugins communications, it's used there:

packer-plugin-sdk/rpc/client.go

Lines 40 to 56 in f0882e2

	func newClientWithMux(mux *muxBroker, streamId uint32) (*Client, error) {
	clientConn, err := mux.Dial(streamId)
	if err != nil {
	return nil, err
	}
	h := &codec.MsgpackHandle{
	WriteExt: true,
	}
	clientCodec := codec.GoRpc.ClientCodec(clientConn, h)
	return &Client{
	mux: mux,
	client: rpc.NewClientWithCodec(clientCodec),
	closeMux: false,
	}, nil
	}

and there:

packer-plugin-sdk/rpc/server.go

Lines 143 to 160 in f0882e2

	// ServeConn serves a single connection over the RPC server. It is up
	// to the caller to obtain a proper io.ReadWriteCloser.
	func (s *PluginServer) Serve() {
	// Accept a connection on stream ID 0, which is always used for
	// normal client to server connections.
	stream, err := s.mux.Accept(s.streamId)
	if err != nil {
	log.Printf("[ERR] Error retrieving stream for serving: %s", err)
	return
	}
	defer stream.Close()
	h := &codec.MsgpackHandle{
	WriteExt: true,
	}
	rpcCodec := codec.GoRpc.ServerCodec(stream, h)
	s.server.ServeCodec(rpcCodec)
	}

So it's sort of simple, but we can't break these all the time 🙂 .

[azr]

Sorry, a Github 500 caused a dupe review, and I can't delete this one.

[azr]

Got confirmation during our weekly meeting !