Remove gogo/protobuf and adjust types

kzys · Kirtana Ashok · commit 59550fac2f87 · 2023-01-18T12:58:03.000-08:00
This commit migrates containerd/protobuf from github.com/gogo/protobuf to google.golang.org/protobuf and adjust types. Proto-generated structs cannot be passed as values. Fixes containerd#6564. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
diff --git a/pkg/cri/server/container_stats_list.go b/pkg/cri/server/container_stats_list.go
@@ -36,7 +36,7 @@ func (c *criService) ListContainerStats(
 	if err != nil {
 		return nil, fmt.Errorf("failed to build metrics request: %w", err)
 	}
-	resp, err := c.client.TaskService().Metrics(ctx, &request)
+	resp, err := c.client.TaskService().Metrics(ctx, request)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch metrics for tasks: %w", err)
 	}
@@ -79,8 +79,8 @@ func (c *criService) normalizeContainerStatsFilter(filter *runtime.ContainerStat
 // the information in the stats request and the containerStore
 func (c *criService) buildTaskMetricsRequest(
 	r *runtime.ListContainerStatsRequest,
-) (tasks.MetricsRequest, []containerstore.Container, error) {
-	var req tasks.MetricsRequest
+) (*tasks.MetricsRequest, []containerstore.Container, error) {
+	req := &tasks.MetricsRequest{}
 	if r.GetFilter() == nil {
 		return req, c.containerStore.List(), nil
 	}
diff --git a/protobuf/any.go b/protobuf/any.go
@@ -17,28 +17,28 @@
 package protobuf
 
 import (
-	"github.com/containerd/containerd/protobuf/types"
 	"github.com/containerd/typeurl"
+	"google.golang.org/protobuf/types/known/anypb"
 )
 
 // FromAny converts typeurl.Any to github.com/containerd/containerd/protobuf/types.Any.
-func FromAny(from typeurl.Any) *types.Any {
+func FromAny(from typeurl.Any) *anypb.Any {
 	if from == nil {
 		return nil
 	}
 
-	if pbany, ok := from.(*types.Any); ok {
+	if pbany, ok := from.(*anypb.Any); ok {
 		return pbany
 	}
 
-	return &types.Any{
+	return &anypb.Any{
 		TypeUrl: from.GetTypeUrl(),
 		Value:   from.GetValue(),
 	}
 }
 
 // FromAny converts an arbitrary interface to github.com/containerd/containerd/protobuf/types.Any.
-func MarshalAnyToProto(from interface{}) (*types.Any, error) {
+func MarshalAnyToProto(from interface{}) (*anypb.Any, error) {
 	any, err := typeurl.MarshalAny(from)
 	if err != nil {
 		return nil, err
diff --git a/protobuf/proto/proto.go b/protobuf/proto/proto.go
@@ -18,13 +18,13 @@
 package proto
 
 import (
-	gogo "github.com/gogo/protobuf/proto"
+	google "google.golang.org/protobuf/proto"
 )
 
-func Marshal(input gogo.Message) ([]byte, error) {
-	return gogo.Marshal(input)
+func Marshal(input google.Message) ([]byte, error) {
+	return google.Marshal(input)
 }
 
-func Unmarshal(input []byte, output gogo.Message) error {
-	return gogo.Unmarshal(input, output)
+func Unmarshal(input []byte, output google.Message) error {
+	return google.Unmarshal(input, output)
 }
diff --git a/protobuf/timestamp.go b/protobuf/timestamp.go
@@ -19,30 +19,18 @@ package protobuf
 import (
 	"time"
 
-	"github.com/gogo/protobuf/types"
+	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
 // Once we migrate off from gogo/protobuf, we can use the function below, which don't return any errors.
 // https://github.com/protocolbuffers/protobuf-go/blob/v1.28.0/types/known/timestamppb/timestamp.pb.go#L200-L208
 
 // ToTimestamp creates protobuf's Timestamp from time.Time.
-func ToTimestamp(from time.Time) *types.Timestamp {
-	pt, err := types.TimestampProto(from)
-	if err != nil {
-		panic(err)
-	}
-	return pt
+func ToTimestamp(from time.Time) *timestamppb.Timestamp {
+	return timestamppb.New(from)
 }
 
 // FromTimestamp creates time.Time from protobuf's Timestamp.
-func FromTimestamp(from *types.Timestamp) time.Time {
-	if from == nil {
-		// Return time.Time's zero value as like timestamppb.
-		return time.Time{}.UTC()
-	}
-	tt, err := types.TimestampFromProto(from)
-	if err != nil {
-		panic(err)
-	}
-	return tt
+func FromTimestamp(from *timestamppb.Timestamp) time.Time {
+	return from.AsTime()
 }
diff --git a/protobuf/types/types.go b/protobuf/types/types.go
@@ -18,9 +18,11 @@
 package types
 
 import (
-	gogo "github.com/gogo/protobuf/types"
+	"google.golang.org/genproto/protobuf/field_mask"
+	"google.golang.org/protobuf/types/known/anypb"
+	"google.golang.org/protobuf/types/known/emptypb"
 )
 
-type Empty = gogo.Empty
-type Any = gogo.Any
-type FieldMask = gogo.FieldMask
+type Empty = emptypb.Empty
+type Any = anypb.Any
+type FieldMask = field_mask.FieldMask
diff --git a/runtime/v1/shim/service.go b/runtime/v1/shim/service.go
@@ -449,13 +449,13 @@ func (s *Service) Checkpoint(ctx context.Context, r *shimapi.CheckpointTaskReque
 	if err != nil {
 		return nil, err
 	}
-	var options runctypes.CheckpointOptions
+	var options *runctypes.CheckpointOptions
 	if r.Options != nil {
 		v, err := typeurl.UnmarshalAny(r.Options)
 		if err != nil {
 			return nil, err
 		}
-		options = *v.(*runctypes.CheckpointOptions)
+		options = v.(*runctypes.CheckpointOptions)
 	}
 	if err := p.(*process.Init).Checkpoint(ctx, &process.CheckpointConfig{
 		Path:                     r.Path,
@@ -644,13 +644,13 @@ func getTopic(ctx context.Context, e interface{}) string {
 }
 
 func newInit(ctx context.Context, path, workDir, runtimeRoot, namespace string, systemdCgroup bool, platform stdio.Platform, r *process.CreateConfig, rootfs string) (*process.Init, error) {
-	var options runctypes.CreateOptions
+	options := &runctypes.CreateOptions{}
 	if r.Options != nil {
 		v, err := typeurl.UnmarshalAny(r.Options)
 		if err != nil {
 			return nil, err
 		}
-		options = *v.(*runctypes.CreateOptions)
+		options = v.(*runctypes.CreateOptions)
 	}
 
 	runtime := process.NewRunc(runtimeRoot, path, namespace, r.Runtime, systemdCgroup)
diff --git a/runtime/v2/runc/container.go b/runtime/v2/runc/container.go
@@ -48,14 +48,14 @@ func NewContainer(ctx context.Context, platform stdio.Platform, r *task.CreateTa
 		return nil, fmt.Errorf("create namespace: %w", err)
 	}
 
-	var opts options.Options
+	opts := &options.Options{}
 	if r.Options.GetValue() != nil {
 		v, err := typeurl.UnmarshalAny(r.Options)
 		if err != nil {
 			return nil, err
 		}
 		if v != nil {
-			opts = *v.(*options.Options)
+			opts = v.(*options.Options)
 		}
 	}
 
@@ -123,7 +123,7 @@ func NewContainer(ctx context.Context, platform stdio.Platform, r *task.CreateTa
 		ns,
 		platform,
 		config,
-		&opts,
+		opts,
 		rootfs,
 	)
 	if err != nil {
@@ -188,7 +188,7 @@ func ReadOptions(path string) (*options.Options, error) {
 }
 
 // WriteOptions writes the options information into the path
-func WriteOptions(path string, opts options.Options) error {
+func WriteOptions(path string, opts *options.Options) error {
 	data, err := json.Marshal(opts)
 	if err != nil {
 		return err
@@ -467,13 +467,13 @@ func (c *Container) Checkpoint(ctx context.Context, r *task.CheckpointTaskReques
 	if err != nil {
 		return err
 	}
-	var opts options.CheckpointOptions
+	var opts *options.CheckpointOptions
 	if r.Options != nil {
 		v, err := typeurl.UnmarshalAny(r.Options)
 		if err != nil {
 			return err
 		}
-		opts = *v.(*options.CheckpointOptions)
+		opts = v.(*options.CheckpointOptions)
 	}
 	return p.(*process.Init).Checkpoint(ctx, &process.CheckpointConfig{
 		Path:                     r.Path,
diff --git a/services/introspection/local.go b/services/introspection/local.go
@@ -29,8 +29,9 @@ import (
 	"github.com/containerd/containerd/plugin"
 	ptypes "github.com/containerd/containerd/protobuf/types"
 	"github.com/containerd/containerd/services"
-	"github.com/gogo/googleapis/google/rpc"
 	"github.com/google/uuid"
+	"google.golang.org/genproto/googleapis/rpc/code"
+	rpc "google.golang.org/genproto/googleapis/rpc/status"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/status"
 )
@@ -55,7 +56,7 @@ type Local struct {
 	mu          sync.Mutex
 	root        string
 	plugins     *plugin.Set
-	pluginCache []api.Plugin
+	pluginCache []*api.Plugin
 }
 
 var _ = (api.IntrospectionClient)(&Local{})
@@ -79,7 +80,7 @@ func (l *Local) Plugins(ctx context.Context, req *api.PluginsRequest, _ ...grpc.
 	for _, p := range allPlugins {
 		p := p
 		if filter.Match(adaptPlugin(p)) {
-			plugins = append(plugins, &p)
+			plugins = append(plugins, p)
 		}
 	}
 
@@ -88,7 +89,7 @@ func (l *Local) Plugins(ctx context.Context, req *api.PluginsRequest, _ ...grpc.
 	}, nil
 }
 
-func (l *Local) getPlugins() []api.Plugin {
+func (l *Local) getPlugins() []*api.Plugin {
 	l.mu.Lock()
 	defer l.mu.Unlock()
 	plugins := l.plugins.GetAll()
@@ -148,7 +149,7 @@ func (l *Local) uuidPath() string {
 }
 
 func adaptPlugin(o interface{}) filters.Adaptor {
-	obj := o.(api.Plugin)
+	obj := o.(*api.Plugin)
 	return filters.AdapterFunc(func(fieldpath []string) (string, bool) {
 		if len(fieldpath) == 0 {
 			return "", false
@@ -174,8 +175,8 @@ func adaptPlugin(o interface{}) filters.Adaptor {
 	})
 }
 
-func pluginsToPB(plugins []*plugin.Plugin) []api.Plugin {
-	var pluginsPB []api.Plugin
+func pluginsToPB(plugins []*plugin.Plugin) []*api.Plugin {
+	var pluginsPB []*api.Plugin
 	for _, p := range plugins {
 		var platforms []*types.Platform
 		for _, p := range p.Meta.Platforms {
@@ -209,13 +210,13 @@ func pluginsToPB(plugins []*plugin.Plugin) []api.Plugin {
 				}
 			} else {
 				initErr = &rpc.Status{
-					Code:    int32(rpc.UNKNOWN),
+					Code:    int32(code.Code_UNKNOWN),
 					Message: err.Error(),
 				}
 			}
 		}
 
-		pluginsPB = append(pluginsPB, api.Plugin{
+		pluginsPB = append(pluginsPB, &api.Plugin{
 			Type:         p.Registration.Type.String(),
 			ID:           p.Registration.ID,
 			Requires:     requires,

Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ func (c *criService) ListContainerStats(`
`36`	`36`	`if err != nil {`
`37`	`37`	`return nil, fmt.Errorf("failed to build metrics request: %w", err)`
`38`	`38`	`}`
`39`		`- resp, err := c.client.TaskService().Metrics(ctx, &request)`
	`39`	`+ resp, err := c.client.TaskService().Metrics(ctx, request)`
`40`	`40`	`if err != nil {`
`41`	`41`	`return nil, fmt.Errorf("failed to fetch metrics for tasks: %w", err)`
`42`	`42`	`}`
`@@ -79,8 +79,8 @@ func (c criService) normalizeContainerStatsFilter(filter runtime.ContainerStat`
`79`	`79`	`// the information in the stats request and the containerStore`
`80`	`80`	`func (c *criService) buildTaskMetricsRequest(`
`81`	`81`	`r *runtime.ListContainerStatsRequest,`
`82`		`-) (tasks.MetricsRequest, []containerstore.Container, error) {`
`83`		`- var req tasks.MetricsRequest`
	`82`	`+) (*tasks.MetricsRequest, []containerstore.Container, error) {`
	`83`	`+ req := &tasks.MetricsRequest{}`
`84`	`84`	`if r.GetFilter() == nil {`
`85`	`85`	`return req, c.containerStore.List(), nil`
`86`	`86`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,28 +17,28 @@`
`17`	`17`	`package protobuf`
`18`	`18`
`19`	`19`	`import (`
`20`		`- "github.com/containerd/containerd/protobuf/types"`
`21`	`20`	`"github.com/containerd/typeurl"`
	`21`	`+ "google.golang.org/protobuf/types/known/anypb"`
`22`	`22`	`)`
`23`	`23`
`24`	`24`	`// FromAny converts typeurl.Any to github.com/containerd/containerd/protobuf/types.Any.`
`25`		`-func FromAny(from typeurl.Any) *types.Any {`
	`25`	`+func FromAny(from typeurl.Any) *anypb.Any {`
`26`	`26`	`if from == nil {`
`27`	`27`	`return nil`
`28`	`28`	`}`
`29`	`29`
`30`		`- if pbany, ok := from.(*types.Any); ok {`
	`30`	`+ if pbany, ok := from.(*anypb.Any); ok {`
`31`	`31`	`return pbany`
`32`	`32`	`}`
`33`	`33`
`34`		`- return &types.Any{`
	`34`	`+ return &anypb.Any{`
`35`	`35`	`TypeUrl: from.GetTypeUrl(),`
`36`	`36`	`Value: from.GetValue(),`
`37`	`37`	`}`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`// FromAny converts an arbitrary interface to github.com/containerd/containerd/protobuf/types.Any.`
`41`		`-func MarshalAnyToProto(from interface{}) (*types.Any, error) {`
	`41`	`+func MarshalAnyToProto(from interface{}) (*anypb.Any, error) {`
`42`	`42`	`any, err := typeurl.MarshalAny(from)`
`43`	`43`	`if err != nil {`
`44`	`44`	`return nil, err`
Original file line number	Diff line number	Diff line change
`@@ -18,13 +18,13 @@`
`18`	`18`	`package proto`
`19`	`19`
`20`	`20`	`import (`
`21`		`- gogo "github.com/gogo/protobuf/proto"`
	`21`	`+ google "google.golang.org/protobuf/proto"`
`22`	`22`	`)`
`23`	`23`
`24`		`-func Marshal(input gogo.Message) ([]byte, error) {`
`25`		`- return gogo.Marshal(input)`
	`24`	`+func Marshal(input google.Message) ([]byte, error) {`
	`25`	`+ return google.Marshal(input)`
`26`	`26`	`}`
`27`	`27`
`28`		`-func Unmarshal(input []byte, output gogo.Message) error {`
`29`		`- return gogo.Unmarshal(input, output)`
	`28`	`+func Unmarshal(input []byte, output google.Message) error {`
	`29`	`+ return google.Unmarshal(input, output)`
`30`	`30`	`}`
Original file line number	Diff line number	Diff line change
`@@ -449,13 +449,13 @@ func (s Service) Checkpoint(ctx context.Context, r shimapi.CheckpointTaskReque`
`449`	`449`	`if err != nil {`
`450`	`450`	`return nil, err`
`451`	`451`	`}`
`452`		`- var options runctypes.CheckpointOptions`
	`452`	`+ var options *runctypes.CheckpointOptions`
`453`	`453`	`if r.Options != nil {`
`454`	`454`	`v, err := typeurl.UnmarshalAny(r.Options)`
`455`	`455`	`if err != nil {`
`456`	`456`	`return nil, err`
`457`	`457`	`}`
`458`		`- options = v.(runctypes.CheckpointOptions)`
	`458`	`+ options = v.(*runctypes.CheckpointOptions)`
`459`	`459`	`}`
`460`	`460`	`if err := p.(*process.Init).Checkpoint(ctx, &process.CheckpointConfig{`
`461`	`461`	`Path: r.Path,`
`@@ -644,13 +644,13 @@ func getTopic(ctx context.Context, e interface{}) string {`
`644`	`644`	`}`
`645`	`645`
`646`	`646`	`func newInit(ctx context.Context, path, workDir, runtimeRoot, namespace string, systemdCgroup bool, platform stdio.Platform, r process.CreateConfig, rootfs string) (process.Init, error) {`
`647`		`- var options runctypes.CreateOptions`
	`647`	`+ options := &runctypes.CreateOptions{}`
`648`	`648`	`if r.Options != nil {`
`649`	`649`	`v, err := typeurl.UnmarshalAny(r.Options)`
`650`	`650`	`if err != nil {`
`651`	`651`	`return nil, err`
`652`	`652`	`}`
`653`		`- options = v.(runctypes.CreateOptions)`
	`653`	`+ options = v.(*runctypes.CreateOptions)`
`654`	`654`	`}`
`655`	`655`
`656`	`656`	`runtime := process.NewRunc(runtimeRoot, path, namespace, r.Runtime, systemdCgroup)`
Original file line number	Diff line number	Diff line change
`@@ -48,14 +48,14 @@ func NewContainer(ctx context.Context, platform stdio.Platform, r *task.CreateTa`
`48`	`48`	`return nil, fmt.Errorf("create namespace: %w", err)`
`49`	`49`	`}`
`50`	`50`
`51`		`- var opts options.Options`
	`51`	`+ opts := &options.Options{}`
`52`	`52`	`if r.Options.GetValue() != nil {`
`53`	`53`	`v, err := typeurl.UnmarshalAny(r.Options)`
`54`	`54`	`if err != nil {`
`55`	`55`	`return nil, err`
`56`	`56`	`}`
`57`	`57`	`if v != nil {`
`58`		`- opts = v.(options.Options)`
	`58`	`+ opts = v.(*options.Options)`
`59`	`59`	`}`
`60`	`60`	`}`
`61`	`61`
`@@ -123,7 +123,7 @@ func NewContainer(ctx context.Context, platform stdio.Platform, r *task.CreateTa`
`123`	`123`	`ns,`
`124`	`124`	`platform,`
`125`	`125`	`config,`
`126`		`- &opts,`
	`126`	`+ opts,`
`127`	`127`	`rootfs,`
`128`	`128`	`)`
`129`	`129`	`if err != nil {`
`@@ -188,7 +188,7 @@ func ReadOptions(path string) (*options.Options, error) {`
`188`	`188`	`}`
`189`	`189`
`190`	`190`	`// WriteOptions writes the options information into the path`
`191`		`-func WriteOptions(path string, opts options.Options) error {`
	`191`	`+func WriteOptions(path string, opts *options.Options) error {`
`192`	`192`	`data, err := json.Marshal(opts)`
`193`	`193`	`if err != nil {`
`194`	`194`	`return err`
`@@ -467,13 +467,13 @@ func (c Container) Checkpoint(ctx context.Context, r task.CheckpointTaskReques`
`467`	`467`	`if err != nil {`
`468`	`468`	`return err`
`469`	`469`	`}`
`470`		`- var opts options.CheckpointOptions`
	`470`	`+ var opts *options.CheckpointOptions`
`471`	`471`	`if r.Options != nil {`
`472`	`472`	`v, err := typeurl.UnmarshalAny(r.Options)`
`473`	`473`	`if err != nil {`
`474`	`474`	`return err`
`475`	`475`	`}`
`476`		`- opts = v.(options.CheckpointOptions)`
	`476`	`+ opts = v.(*options.CheckpointOptions)`
`477`	`477`	`}`
`478`	`478`	`return p.(*process.Init).Checkpoint(ctx, &process.CheckpointConfig{`
`479`	`479`	`Path: r.Path,`