Remove proto-based ServerV2 implementation of DeepCopy in favor of the

manual implementation to avoid issues with proto-based type merge
panics.

Fixes https://github.com/gravitational/teleport/issues/5691.

api/types/authority.go

@@ -24,7 +24,6 @@ import (
 	"github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/utils"
 
-	"github.com/gogo/protobuf/proto"
 	"github.com/gravitational/trace"
 )
 
@@ -505,23 +504,6 @@ func (r *Rotation) CheckAndSetDefaults() error {
 	return nil
 }
 
-// Merge overwrites r from src and
-// is part of support for cloning Server values
-// using proto.Clone.
-//
-// Note: this does not implement the full Merger interface,
-// specifically, it assumes that r is zero value.
-// See https://github.com/gogo/protobuf/blob/v1.3.1/proto/clone.go#L58-L60
-//
-// Implements proto.Merger
-func (r *Rotation) Merge(src proto.Message) {
-	s, ok := src.(*Rotation)
-	if !ok {
-		return
-	}
-	*r = *s
-}
-
 // GenerateSchedule generates schedule based on the time period, using
 // even time periods between rotation phases.
 func GenerateSchedule(now time.Time, gracePeriod time.Duration) (*RotationSchedule, error) {

api/types/resource.go

@@ -23,7 +23,6 @@ import (
 	"github.com/gravitational/teleport/api/defaults"
 	"github.com/gravitational/teleport/api/utils"
 
-	"github.com/gogo/protobuf/proto"
 	"github.com/gravitational/trace"
 )
 
@@ -205,36 +204,6 @@ func (m *Metadata) CheckAndSetDefaults() error {
 	return nil
 }
 
-// Merge overwrites r from src and
-// is part of support for cloning Server values
-// using proto.Clone.
-//
-// Note: this does not implement the full Merger interface,
-// specifically, it assumes that r is zero value.
-// See https://github.com/gogo/protobuf/blob/v1.3.1/proto/clone.go#L58-L60
-//
-// Implements proto.Merger
-func (m *Metadata) Merge(src proto.Message) {
-	metadata, ok := src.(*Metadata)
-	if !ok {
-		return
-	}
-	*m = *metadata
-	// Manually clone expiry timestamp as proto.Clone
-	// cannot cope with values that contain unexported
-	// attributes (as time.Time does)
-	if metadata.Expires != nil {
-		expires := *metadata.Expires
-		m.Expires = &expires
-	}
-	if len(metadata.Labels) != 0 {
-		m.Labels = make(map[string]string)
-		for k, v := range metadata.Labels {
-			m.Labels[k] = v
-		}
-	}
-}
-
 // LabelPattern is a regexp that describes a valid label key
 const LabelPattern = `^[a-zA-Z/.0-9_*-]+$`
 

api/types/server.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/gravitational/teleport/api/utils"
 
-	"github.com/gogo/protobuf/proto"
 	"github.com/gravitational/trace"
 )
 
@@ -335,7 +334,18 @@ func (s *ServerV2) CheckAndSetDefaults() error {
 
 // DeepCopy creates a clone of this server value
 func (s *ServerV2) DeepCopy() Server {
-	return proto.Clone(s).(*ServerV2)
+	newServer := *s
+	if s.Metadata.Expires != nil {
+		expires := *s.Metadata.Expires
+		newServer.Metadata.Expires = &expires
+	}
+	if len(s.Metadata.Labels) != 0 {
+		newServer.Metadata.Labels = make(map[string]string)
+		for k, v := range s.Metadata.Labels {
+			newServer.Metadata.Labels[k] = v
+		}
+	}
+	return &newServer
 }
 
 // CommandLabel is a label that has a value as a result of the

lib/services/server.go

@@ -393,47 +393,6 @@ func GetServerSchema() string {
 	return fmt.Sprintf(V2SchemaTemplate, MetadataSchema, fmt.Sprintf(ServerSpecV2Schema, RotationSchema), DefaultDefinitions)
 }
 
-// UnmarshalServerResource unmarshals role from JSON or YAML,
-// sets defaults and checks the schema
-func UnmarshalServerResource(data []byte, kind string, cfg *MarshalConfig) (Server, error) {
-	if len(data) == 0 {
-		return nil, trace.BadParameter("missing server data")
-	}
-
-	var h ResourceHeader
-	err := utils.FastUnmarshal(data, &h)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-
-	switch h.Version {
-	case V2:
-		var s ServerV2
-
-		if cfg.SkipValidation {
-			if err := utils.FastUnmarshal(data, &s); err != nil {
-				return nil, trace.BadParameter(err.Error())
-			}
-		} else {
-			if err := utils.UnmarshalWithSchema(GetServerSchema(), &s, data); err != nil {
-				return nil, trace.BadParameter(err.Error())
-			}
-		}
-		s.Kind = kind
-		if err := s.CheckAndSetDefaults(); err != nil {
-			return nil, trace.Wrap(err)
-		}
-		if cfg.ID != 0 {
-			s.SetResourceID(cfg.ID)
-		}
-		if !cfg.Expires.IsZero() {
-			s.SetExpiry(cfg.Expires)
-		}
-		return &s, nil
-	}
-	return nil, trace.BadParameter("server resource version %q is not supported", h.Version)
-}
-
 // UnmarshalServer unmarshals the Server resource from JSON.
 func UnmarshalServer(bytes []byte, kind string, opts ...MarshalOption) (Server, error) {
 	cfg, err := CollectOptions(opts)
@@ -473,6 +432,13 @@ func UnmarshalServer(bytes []byte, kind string, opts ...MarshalOption) (Server,
 		if !cfg.Expires.IsZero() {
 			s.SetExpiry(cfg.Expires)
 		}
+		if s.Metadata.Expires != nil {
+			utils.UTC(s.Metadata.Expires)
+		}
+		// Force the timestamps to UTC for consistency.
+		// See https://github.com/gogo/protobuf/issues/519 for details on issues this causes for proto.Clone
+		utils.UTC(&s.Spec.Rotation.Started)
+		utils.UTC(&s.Spec.Rotation.LastRotated)
 		return &s, nil
 	}
 	return nil, trace.BadParameter("server resource version %q is not supported", h.Version)

lib/services/servers_test.go

@@ -200,7 +200,7 @@ func TestUnmarshalServerResourceKubernetes(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			got, err := UnmarshalServerResource([]byte(tt.in), KindKubeService, &MarshalConfig{})
+			got, err := UnmarshalServer([]byte(tt.in), KindKubeService)
 			require.NoError(t, err)
 			require.Empty(t, cmp.Diff(got, tt.want))
 		})

lib/services/services_test.go

@@ -108,7 +108,7 @@ func (s *ServicesSuite) TestLabelKeyValidation(c *check.C) {
 func TestServerDeepCopy(t *testing.T) {
 	t.Parallel()
 	// setup
-	now := time.Date(1984, time.April, 4, 0, 0, 0, 0, time.UTC)
+	now := time.Date(1984, time.April, 4, 0, 0, 0, 0, time.Local)
 	expires := now.Add(1 * time.Hour)
 	srv := &ServerV2{
 		Kind:    KindNode,