Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cleanup content-type protobuf constants #76824

Merged
merged 2 commits into from
Apr 24, 2019
Merged

Cleanup content-type protobuf constants #76824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1d2735
Collapse the protobuf extension file into codec factory
smarterclayton Apr 2, 2019
b48cc43
Cleanup content-type protobuf constants
wojtek-t Apr 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/api/testing/serialization_proto_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func TestProtobufRoundTrip(t *testing.T) {
func BenchmarkEncodeCodecProtobuf(b *testing.B) {
items := benchmarkItems(b)
width := len(items)
s := protobuf.NewSerializer(nil, nil, "application/arbitrary.content.type")
s := protobuf.NewSerializer(nil, nil)
b.ResetTimer()
for i := 0; i < b.N; i++ {
if _, err := runtime.Encode(s, &items[i%width]); err != nil {
Expand All @@ -142,7 +142,7 @@ func BenchmarkEncodeCodecFromInternalProtobuf(b *testing.B) {
b.Fatal(err)
}
}
s := protobuf.NewSerializer(nil, nil, "application/arbitrary.content.type")
s := protobuf.NewSerializer(nil, nil)
codec := legacyscheme.Codecs.EncoderForVersion(s, v1.SchemeGroupVersion)
b.ResetTimer()
for i := 0; i < b.N; i++ {
Expand Down Expand Up @@ -170,7 +170,7 @@ func BenchmarkEncodeProtobufGeneratedMarshal(b *testing.B) {
func BenchmarkDecodeCodecToInternalProtobuf(b *testing.B) {
items := benchmarkItems(b)
width := len(items)
s := protobuf.NewSerializer(legacyscheme.Scheme, legacyscheme.Scheme, "application/arbitrary.content.type")
s := protobuf.NewSerializer(legacyscheme.Scheme, legacyscheme.Scheme)
encoder := legacyscheme.Codecs.EncoderForVersion(s, v1.SchemeGroupVersion)
var encoded [][]byte
for i := range items {
Expand Down
4 changes: 2 additions & 2 deletions staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip/roundtrip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func roundTripToAllExternalVersions(t *testing.T, scheme *runtime.Scheme, codecF

// TODO remove this hack after we're past the intermediate steps
if !skipProtobuf && externalGVK.Group != "kubeadm.k8s.io" {
s := protobuf.NewSerializer(scheme, scheme, "application/arbitrary.content.type")
s := protobuf.NewSerializer(scheme, scheme)
protobufCodec := codecFactory.CodecForVersions(s, s, externalGVK.GroupVersion(), nil)
roundTrip(t, scheme, protobufCodec, object)
}
Expand Down Expand Up @@ -260,7 +260,7 @@ func roundTripOfExternalType(t *testing.T, scheme *runtime.Scheme, codecFactory

// TODO remove this hack after we're past the intermediate steps
if !skipProtobuf {
roundTrip(t, scheme, protobuf.NewSerializer(scheme, scheme, "application/protobuf"), object)
roundTrip(t, scheme, protobuf.NewSerializer(scheme, scheme), object)
}
}

Expand Down
1 change: 0 additions & 1 deletion staging/src/k8s.io/apimachinery/pkg/runtime/serializer/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ go_library(
srcs = [
"codec_factory.go",
"negotiated_codec.go",
"protobuf_extension.go",
],
importmap = "k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/runtime/serializer",
importpath = "k8s.io/apimachinery/pkg/runtime/serializer",
Expand Down
12 changes: 12 additions & 0 deletions staging/src/k8s.io/apimachinery/pkg/runtime/serializer/codec_factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer/json"
"k8s.io/apimachinery/pkg/runtime/serializer/protobuf"
"k8s.io/apimachinery/pkg/runtime/serializer/recognizer"
"k8s.io/apimachinery/pkg/runtime/serializer/versioning"
)
Expand Down Expand Up @@ -51,6 +52,8 @@ func newSerializersForScheme(scheme *runtime.Scheme, mf json.MetaFactory) []seri
jsonSerializer := json.NewSerializer(mf, scheme, scheme, false)
jsonPrettySerializer := json.NewSerializer(mf, scheme, scheme, true)
yamlSerializer := json.NewYAMLSerializer(mf, scheme, scheme)
serializer := protobuf.NewSerializer(scheme, scheme)
raw := protobuf.NewRawSerializer(scheme, scheme)

serializers := []serializerType{
{
Expand All @@ -71,6 +74,15 @@ func newSerializersForScheme(scheme *runtime.Scheme, mf json.MetaFactory) []seri
EncodesAsText: true,
Serializer: yamlSerializer,
},
{
AcceptContentTypes: []string{runtime.ContentTypeProtobuf},
ContentType: runtime.ContentTypeProtobuf,
FileExtensions: []string{"pb"},
Serializer: serializer,

Framer: protobuf.LengthDelimitedFramer,
StreamSerializer: raw,
},
}

for _, fn := range serializerExtensions {
Expand Down
32 changes: 14 additions & 18 deletions staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf/protobuf.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,18 @@ func IsNotMarshalable(err error) bool {
// as-is (any type info passed with the object will be used).
//
// This encoding scheme is experimental, and is subject to change at any time.
func NewSerializer(creater runtime.ObjectCreater, typer runtime.ObjectTyper, defaultContentType string) *Serializer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarterclayton is there a reason for this to vary (e.g. streaming), or is this safe to hard code?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't vary in the current codebase (we have also RawSerializer for protobuf for streaming purpose) - I don't see any reason for having that.
But I'm happy to hear @smarterclayton opinion too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content type used in Unknown is always content protobuf. So this should be safe if it's going into Unknown only.

func NewSerializer(creater runtime.ObjectCreater, typer runtime.ObjectTyper) *Serializer {
return &Serializer{
prefix: protoEncodingPrefix,
creater: creater,
typer: typer,
contentType: defaultContentType,
prefix: protoEncodingPrefix,
creater: creater,
typer: typer,
}
}

type Serializer struct {
prefix []byte
creater runtime.ObjectCreater
typer runtime.ObjectTyper
contentType string
prefix []byte
creater runtime.ObjectCreater
typer runtime.ObjectTyper
}

var _ runtime.Serializer = &Serializer{}
Expand Down Expand Up @@ -138,7 +136,7 @@ func (s *Serializer) Decode(originalData []byte, gvk *schema.GroupVersionKind, i
if intoUnknown, ok := into.(*runtime.Unknown); ok && intoUnknown != nil {
*intoUnknown = unk
if ok, _, _ := s.RecognizesData(bytes.NewBuffer(unk.Raw)); ok {
intoUnknown.ContentType = s.contentType
intoUnknown.ContentType = runtime.ContentTypeProtobuf
}
return intoUnknown, &actual, nil
}
Expand Down Expand Up @@ -303,20 +301,18 @@ func estimateUnknownSize(unk *runtime.Unknown, byteSize uint64) uint64 {
// encoded object, and thus is not self describing (callers must know what type is being described in order to decode).
//
// This encoding scheme is experimental, and is subject to change at any time.
func NewRawSerializer(creater runtime.ObjectCreater, typer runtime.ObjectTyper, defaultContentType string) *RawSerializer {
func NewRawSerializer(creater runtime.ObjectCreater, typer runtime.ObjectTyper) *RawSerializer {
return &RawSerializer{
creater: creater,
typer: typer,
contentType: defaultContentType,
creater: creater,
typer: typer,
}
}

// RawSerializer encodes and decodes objects without adding a runtime.Unknown wrapper (objects are encoded without identifying
// type).
type RawSerializer struct {
creater runtime.ObjectCreater
typer runtime.ObjectTyper
contentType string
creater runtime.ObjectCreater
typer runtime.ObjectTyper
}

var _ runtime.Serializer = &RawSerializer{}
Expand Down Expand Up @@ -358,7 +354,7 @@ func (s *RawSerializer) Decode(originalData []byte, gvk *schema.GroupVersionKind
if intoUnknown, ok := into.(*runtime.Unknown); ok && intoUnknown != nil {
intoUnknown.Raw = data
intoUnknown.ContentEncoding = ""
intoUnknown.ContentType = s.contentType
intoUnknown.ContentType = runtime.ContentTypeProtobuf
intoUnknown.SetGroupVersionKind(*actual)
return intoUnknown, actual, nil
}
Expand Down
48 changes: 0 additions & 48 deletions staging/src/k8s.io/apimachinery/pkg/runtime/serializer/protobuf_extension.go
View file
Open in desktop

This file was deleted.

10 changes: 5 additions & 5 deletions staging/src/k8s.io/apimachinery/pkg/test/runtime_serializer_protobuf_protobuf_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (d *testBufferedMarshalable) DeepCopyObject() runtime.Object {
}

func TestRecognize(t *testing.T) {
s := protobuf.NewSerializer(nil, nil, "application/protobuf")
s := protobuf.NewSerializer(nil, nil)
ignores := [][]byte{
nil,
{},
Expand Down Expand Up @@ -172,7 +172,7 @@ func TestEncode(t *testing.T) {
}

for i, test := range testCases {
s := protobuf.NewSerializer(nil, nil, "application/protobuf")
s := protobuf.NewSerializer(nil, nil)
data, err := runtime.Encode(s, test.obj)

switch {
Expand Down Expand Up @@ -251,15 +251,15 @@ func TestProtobufDecode(t *testing.T) {
Kind: "test",
},
// content type is set because the prefix matches the content
ContentType: "application/protobuf",
ContentType: runtime.ContentTypeProtobuf,
Raw: []byte{0x6b, 0x38, 0x73, 0x00, 0x01, 0x02, 0x03},
},
data: wire2,
},
}

for i, test := range testCases {
s := protobuf.NewSerializer(nil, nil, "application/protobuf")
s := protobuf.NewSerializer(nil, nil)
unk := &runtime.Unknown{}
err := runtime.DecodeInto(s, test.data, unk)

Expand Down Expand Up @@ -334,7 +334,7 @@ func TestDecodeObjects(t *testing.T) {
for i, test := range testCases {
scheme.AddKnownTypes(schema.GroupVersion{Version: "v1"}, &v1.Carp{})
require.NoError(t, v1.AddToScheme(scheme))
s := protobuf.NewSerializer(scheme, scheme, "application/protobuf")
s := protobuf.NewSerializer(scheme, scheme)
obj, err := runtime.Decode(s, test.data)

switch {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/apiserver/apiserver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,7 @@ func expectPartialObjectMetaEvents(t *testing.T, d *json.Decoder, values ...stri
func expectPartialObjectMetaEventsProtobuf(t *testing.T, r io.Reader, values ...string) {
scheme := runtime.NewScheme()
metav1.AddToGroupVersion(scheme, schema.GroupVersion{Version: "v1"})
rs := protobuf.NewRawSerializer(scheme, scheme, "application/vnd.kubernetes.protobuf")
rs := protobuf.NewRawSerializer(scheme, scheme)
d := streaming.NewDecoder(
protobuf.LengthDelimitedFramer.NewFrameReader(ioutil.NopCloser(r)),
rs,
Expand Down