Merge pull request #13801 from vvoland/work-shim2

core/runtime/v2: Preserve protobuf shim response bytes

core/runtime/v2/binary.go

@@ -118,8 +118,6 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", out, err)
 	}
-	response := bytes.TrimSpace(out)
-
 	onCloseWithShimLog := func() {
 		onClose()
 		cancelShimLog()
@@ -130,7 +128,7 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_
 		return nil, err
 	}
 
-	params, err := parseStartResponse(response)
+	params, err := parseStartResponse(out)
 	if err != nil {
 		return nil, err
 	}

core/runtime/v2/shim.go

@@ -17,6 +17,7 @@
 package v2
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
@@ -229,6 +230,7 @@ func parseStartResponse(response []byte) (*bootapi.BootstrapResult, error) {
 	}
 
 	// Fallback to legacy parsing for backward compatibility with legacy shims that return the address as a plain string or JSON.
+	response = bytes.TrimSpace(response)
 
 	var params client.BootstrapParams //nolint:staticcheck // Used for backward compatibility with legacy shims
 	if err := json.Unmarshal(response, &params); err != nil || params.Version < 2 {

core/runtime/v2/shim_test.go

@@ -23,29 +23,49 @@ import (
 	"testing"
 
 	bootapi "github.com/containerd/containerd/api/runtime/bootstrap/v1"
+	"github.com/containerd/containerd/v2/pkg/protobuf/proto"
 	"github.com/containerd/errdefs"
 	"github.com/stretchr/testify/require"
 )
 
 func TestParseStartResponse(t *testing.T) {
+	protobufResponse, err := proto.Marshal(&bootapi.BootstrapResult{
+		Version:  3,
+		Address:  "unix:///run/containerd/shim.sock",
+		Protocol: "ttrpc",
+		Metadata: map[string]string{"note": "line\n"},
+	})
+	require.NoError(t, err)
+	require.Equal(t, byte('\n'), protobufResponse[len(protobufResponse)-1])
+
 	testCases := []struct {
 		Name     string
-		Response string
+		Response []byte
 		Expected bootapi.BootstrapResult
 		Err      error
 	}{
 		{
-			Name:     "v2 shim",
-			Response: "/somedirectory/somesocket",
+			Name:     "v2 shim with trailing newline",
+			Response: []byte("/somedirectory/somesocket\n"),
 			Expected: bootapi.BootstrapResult{
 				Version:  2,
 				Address:  "/somedirectory/somesocket",
 				Protocol: "ttrpc",
 			},
 		},
+		{
+			Name:     "v3 shim protobuf",
+			Response: protobufResponse,
+			Expected: bootapi.BootstrapResult{
+				Version:  3,
+				Address:  "unix:///run/containerd/shim.sock",
+				Protocol: "ttrpc",
+				Metadata: map[string]string{"note": "line\n"},
+			},
+		},
 		{
 			Name:     "v2 shim using grpc",
-			Response: `{"version":2,"address":"/somedirectory/somesocket","protocol":"grpc"}`,
+			Response: []byte(`{"version":2,"address":"/somedirectory/somesocket","protocol":"grpc"}`),
 			Expected: bootapi.BootstrapResult{
 				Version:  2,
 				Address:  "/somedirectory/somesocket",
@@ -54,7 +74,7 @@ func TestParseStartResponse(t *testing.T) {
 		},
 		{
 			Name:     "v2 shim using ttrpc",
-			Response: `{"version":2,"address":"/somedirectory/somesocket","protocol":"ttrpc"}`,
+			Response: []byte(`{"version":2,"address":"/somedirectory/somesocket","protocol":"ttrpc"}`),
 			Expected: bootapi.BootstrapResult{
 				Version:  2,
 				Address:  "/somedirectory/somesocket",
@@ -63,7 +83,7 @@ func TestParseStartResponse(t *testing.T) {
 		},
 		{
 			Name:     "invalid shim v2 response",
-			Response: `{"address":"/somedirectory/somesocket","protocol":"ttrpc"}`,
+			Response: []byte(`{"address":"/somedirectory/somesocket","protocol":"ttrpc"}`),
 			Expected: bootapi.BootstrapResult{
 				Version:  2,
 				Address:  `{"address":"/somedirectory/somesocket","protocol":"ttrpc"}`,
@@ -72,7 +92,7 @@ func TestParseStartResponse(t *testing.T) {
 		},
 		{
 			Name:     "later unsupported shim",
-			Response: `{"Version": 4,"Address":"/somedirectory/somesocket","Protocol":"ttrpc"}`,
+			Response: []byte(`{"Version": 4,"Address":"/somedirectory/somesocket","Protocol":"ttrpc"}`),
 			Expected: bootapi.BootstrapResult{},
 			Err:      errdefs.ErrNotImplemented,
 		},
@@ -81,7 +101,7 @@ func TestParseStartResponse(t *testing.T) {
 	for i := range testCases {
 		tc := &testCases[i]
 		t.Run(tc.Name, func(t *testing.T) {
-			params, err := parseStartResponse([]byte(tc.Response))
+			params, err := parseStartResponse(tc.Response)
 			if err != nil {
 				if !errors.Is(err, tc.Err) {
 					t.Errorf("unexpected error: %v", err)
@@ -99,6 +119,7 @@ func TestParseStartResponse(t *testing.T) {
 			if params.Address != tc.Expected.Address {
 				t.Errorf("unexpected address %q, expected %q", params.Address, tc.Expected.Address)
 			}
+			require.Equal(t, tc.Expected.Metadata, params.Metadata)
 		})
 	}
 }