Skip to content

Commit

Permalink
Do not automatically decode runtime.RawExtension
Browse files Browse the repository at this point in the history
Make clients opt in to decoding objects that are stored
in the generic api.List object by invoking runtime.DecodeList()
with a set of schemes. Makes it easier to handle unknown
schema objects because decoding is in the control of the code.

Add runtime.Unstructured, which is a simple in memory
representation of an external object.
  • Loading branch information
smarterclayton committed Apr 29, 2015
1 parent a4316aa commit 12ba4e2
Show file tree
Hide file tree
Showing 20 changed files with 391 additions and 61 deletions.
6 changes: 4 additions & 2 deletions pkg/api/testing/fuzzer.go
Expand Up @@ -115,12 +115,14 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
},
func(j *api.List, c fuzz.Continue) {
c.FuzzNoCustom(j) // fuzz self without calling this function again
if j.Items == nil {
// TODO: uncomment when round trip starts from a versioned object
if false { //j.Items == nil {
j.Items = []runtime.Object{}
}
},
func(j *runtime.Object, c fuzz.Continue) {
if c.RandBool() {
// TODO: uncomment when round trip starts from a versioned object
if true { //c.RandBool() {
*j = &runtime.Unknown{
TypeMeta: runtime.TypeMeta{Kind: "Something", APIVersion: "unknown"},
RawJSON: []byte(`{"apiVersion":"unknown","kind":"Something","someKey":"someValue"}`),
Expand Down
33 changes: 15 additions & 18 deletions pkg/client/testclient/fixture.go
Expand Up @@ -81,7 +81,7 @@ func ObjectReaction(o ObjectRetriever, mapper meta.RESTMapper) ReactionFunc {

// AddObjectsFromPath loads the JSON or YAML file containing Kubernetes API resources
// and adds them to the provided ObjectRetriever.
func AddObjectsFromPath(path string, o ObjectRetriever) error {
func AddObjectsFromPath(path string, o ObjectRetriever, decoder runtime.Decoder) error {
data, err := ioutil.ReadFile(path)
if err != nil {
return err
Expand All @@ -90,7 +90,7 @@ func AddObjectsFromPath(path string, o ObjectRetriever) error {
if err != nil {
return err
}
obj, err := api.Codec.Decode(data)
obj, err := decoder.Decode(data)
if err != nil {
return err
}
Expand All @@ -103,17 +103,12 @@ func AddObjectsFromPath(path string, o ObjectRetriever) error {
type objects struct {
types map[string][]runtime.Object
last map[string]int
typer runtime.ObjectTyper
creater runtime.ObjectCreater
copier copier
scheme runtime.ObjectScheme
decoder runtime.ObjectDecoder
}

var _ ObjectRetriever = &objects{}

type copier interface {
Copy(obj runtime.Object) (runtime.Object, error)
}

// NewObjects implements the ObjectRetriever interface by introspecting the
// objects provided to Add() and returning them when the Kind method is invoked.
// If an api.List object is provided to Add(), each child item is added. If an
Expand All @@ -124,18 +119,17 @@ type copier interface {
// as a runtime.Object if Status == Success). If multiple PodLists are provided, they
// will be returned in order by the Kind call, and the last PodList will be reused for
// subsequent calls.
func NewObjects(scheme *runtime.Scheme) ObjectRetriever {
func NewObjects(scheme runtime.ObjectScheme, decoder runtime.ObjectDecoder) ObjectRetriever {
return objects{
types: make(map[string][]runtime.Object),
last: make(map[string]int),
typer: scheme,
creater: scheme,
copier: scheme,
scheme: scheme,
decoder: decoder,
}
}

func (o objects) Kind(kind, name string) (runtime.Object, error) {
empty, _ := o.creater.New("", kind)
empty, _ := o.scheme.New("", kind)
nilValue := reflect.Zero(reflect.TypeOf(empty)).Interface().(runtime.Object)

arr, ok := o.types[kind]
Expand All @@ -146,14 +140,14 @@ func (o objects) Kind(kind, name string) (runtime.Object, error) {
if !ok {
return empty, nil
}
out, err := o.creater.New("", kind)
out, err := o.scheme.New("", kind)
if err != nil {
return nilValue, err
}
if err := runtime.SetList(out, arr); err != nil {
return nilValue, err
}
if out, err = o.copier.Copy(out); err != nil {
if out, err = o.scheme.Copy(out); err != nil {
return nilValue, err
}
return out, nil
Expand All @@ -168,7 +162,7 @@ func (o objects) Kind(kind, name string) (runtime.Object, error) {
if index < 0 {
return nilValue, errors.NewNotFound(kind, name)
}
out, err := o.copier.Copy(arr[index])
out, err := o.scheme.Copy(arr[index])
if err != nil {
return nilValue, err
}
Expand All @@ -187,7 +181,7 @@ func (o objects) Kind(kind, name string) (runtime.Object, error) {
}

func (o objects) Add(obj runtime.Object) error {
_, kind, err := o.typer.ObjectVersionAndKind(obj)
_, kind, err := o.scheme.ObjectVersionAndKind(obj)
if err != nil {
return err
}
Expand All @@ -202,6 +196,9 @@ func (o objects) Add(obj runtime.Object) error {
if err != nil {
return err
}
if errs := runtime.DecodeList(list, o.decoder); len(errs) > 0 {
return errs[0]
}
for _, obj := range list {
if err := o.Add(obj); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/testclient/testclient.go
Expand Up @@ -27,7 +27,7 @@ import (

// NewSimpleFake returns a client that will respond with the provided objects
func NewSimpleFake(objects ...runtime.Object) *Fake {
o := NewObjects(api.Scheme)
o := NewObjects(api.Scheme, api.Scheme)
for _, obj := range objects {
if err := o.Add(obj); err != nil {
panic(err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/client/testclient/testclient_test.go
Expand Up @@ -27,8 +27,8 @@ import (
)

func TestNewClient(t *testing.T) {
o := NewObjects(api.Scheme)
if err := AddObjectsFromPath("../../../examples/guestbook/frontend-service.json", o); err != nil {
o := NewObjects(api.Scheme, api.Scheme)
if err := AddObjectsFromPath("../../../examples/guestbook/frontend-service.json", o, api.Scheme); err != nil {
t.Fatal(err)
}
client := &Fake{ReactFn: ObjectReaction(o, latest.RESTMapper)}
Expand All @@ -52,7 +52,7 @@ func TestNewClient(t *testing.T) {
}

func TestErrors(t *testing.T) {
o := NewObjects(api.Scheme)
o := NewObjects(api.Scheme, api.Scheme)
o.Add(&api.List{
Items: []runtime.Object{
// This first call to List will return this error
Expand Down
8 changes: 8 additions & 0 deletions pkg/conversion/error.go
Expand Up @@ -54,6 +54,10 @@ type missingKindErr struct {
data string
}

func NewMissingKindErr(data string) error {
return &missingKindErr{data}
}

func (k *missingKindErr) Error() string {
return fmt.Sprintf("Object 'Kind' is missing in '%s'", k.data)
}
Expand All @@ -70,6 +74,10 @@ type missingVersionErr struct {
data string
}

func NewMissingVersionErr(data string) error {
return &missingVersionErr{data}
}

func (k *missingVersionErr) Error() string {
return fmt.Sprintf("Object 'apiVersion' is missing in '%s'", k.data)
}
Expand Down
11 changes: 11 additions & 0 deletions pkg/conversion/scheme.go
Expand Up @@ -238,6 +238,17 @@ func (s *Scheme) AddDefaultingFuncs(defaultingFuncs ...interface{}) error {
return nil
}

// Recognizes returns true if the scheme is able to handle the provided version and kind
// of an object.
func (s *Scheme) Recognizes(version, kind string) bool {
m, ok := s.versionMap[version]
if !ok {
return false
}
_, ok = m[kind]
return ok
}

// RegisterInputDefaults sets the provided field mapping function and field matching
// as the defaults for the provided input type. The fn may be nil, in which case no
// mapping will happen by default. Use this method to register a mechanism for handling
Expand Down
11 changes: 11 additions & 0 deletions pkg/kubectl/cmd/get_test.go
Expand Up @@ -393,6 +393,17 @@ func TestGetMultipleTypeObjectsAsList(t *testing.T) {
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
list, err := runtime.ExtractList(out)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if errs := runtime.DecodeList(list, api.Scheme); len(errs) > 0 {
t.Fatalf("unexpected error: %v", errs)
}
if err := runtime.SetList(out, list); err != nil {
t.Fatalf("unexpected error: %v", err)
}

expected := &api.List{
Items: []runtime.Object{
&pods.Items[0],
Expand Down
6 changes: 6 additions & 0 deletions pkg/kubectl/resource/visitor.go
Expand Up @@ -347,6 +347,12 @@ func (v FlattenListVisitor) Visit(fn VisitorFunc) error {
if err != nil {
return fn(info)
}
if errs := runtime.DecodeList(items, struct {
runtime.ObjectTyper
runtime.Decoder
}{v.Mapper, info.Mapping.Codec}); len(errs) > 0 {
return errors.NewAggregate(errs)
}
for i := range items {
item, err := v.InfoForObject(items[i])
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/namespace/namespace_controller_test.go
Expand Up @@ -135,7 +135,7 @@ func TestSyncNamespaceThatIsActive(t *testing.T) {
}

func TestRunStop(t *testing.T) {
o := testclient.NewObjects(api.Scheme)
o := testclient.NewObjects(api.Scheme, api.Scheme)
client := &testclient.Fake{ReactFn: testclient.ObjectReaction(o, latest.RESTMapper)}
nsMgr := NewNamespaceManager(client, 1*time.Second)

Expand Down
8 changes: 4 additions & 4 deletions pkg/runtime/codec.go
Expand Up @@ -21,8 +21,8 @@ import (
)

// CodecFor returns a Codec that invokes Encode with the provided version.
func CodecFor(scheme *Scheme, version string) Codec {
return &codecWrapper{scheme, version}
func CodecFor(codec ObjectCodec, version string) Codec {
return &codecWrapper{codec, version}
}

// yamlCodec converts YAML passed to the Decoder methods to JSON.
Expand Down Expand Up @@ -69,11 +69,11 @@ func EncodeOrDie(codec Codec, obj Object) string {
// codecWrapper implements encoding to an alternative
// default version for a scheme.
type codecWrapper struct {
*Scheme
ObjectCodec
version string
}

// Encode implements Codec
func (c *codecWrapper) Encode(obj Object) ([]byte, error) {
return c.Scheme.EncodeToVersion(obj, c.version)
return c.EncodeToVersion(obj, c.version)
}
42 changes: 36 additions & 6 deletions pkg/runtime/embedded_test.go
Expand Up @@ -63,9 +63,22 @@ func TestDecodeEmptyRawExtensionAsObject(t *testing.T) {
s.AddKnownTypes("", &ObjectTest{})
s.AddKnownTypeWithName("v1test", "ObjectTest", &ObjectTestExternal{})

_, err := s.Decode([]byte(`{"kind":"ObjectTest","apiVersion":"v1test","items":[{}]}`))
if err == nil {
t.Fatalf("unexpected non-error")
obj, err := s.Decode([]byte(`{"kind":"ObjectTest","apiVersion":"v1test","items":[{}]}`))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
test := obj.(*ObjectTest)
if unk, ok := test.Items[0].(*runtime.Unknown); !ok || unk.Kind != "" || unk.APIVersion != "" || string(unk.RawJSON) != "{}" {
t.Fatalf("unexpected object: %#v", test.Items[0])
}

obj, err = s.Decode([]byte(`{"kind":"ObjectTest","apiVersion":"v1test","items":[{"kind":"Other","apiVersion":"v1"}]}`))
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
test = obj.(*ObjectTest)
if unk, ok := test.Items[0].(*runtime.Unknown); !ok || unk.Kind != "Other" || unk.APIVersion != "v1" || string(unk.RawJSON) != `{"kind":"Other","apiVersion":"v1"}` {
t.Fatalf("unexpected object: %#v", test.Items[0])
}
}

Expand Down Expand Up @@ -99,17 +112,34 @@ func TestArrayOfRuntimeObject(t *testing.T) {
if err := json.Unmarshal(wire, obj); err != nil {
t.Fatalf("unexpected error: %v", err)
}
t.Logf("exact wire is: %#v", string(obj.Items[0].RawJSON))
t.Logf("exact wire is: %s", string(obj.Items[0].RawJSON))

decoded, err := s.Decode(wire)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
list, err := runtime.ExtractList(decoded)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if errs := runtime.DecodeList(list, s); len(errs) > 0 {
t.Fatalf("unexpected error: %v", errs)
}

list2, err := runtime.ExtractList(list[3])
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if errs := runtime.DecodeList(list2, s); len(errs) > 0 {
t.Fatalf("unexpected error: %v", errs)
}
if err := runtime.SetList(list[3], list2); err != nil {
t.Fatalf("unexpected error: %v", err)
}

internal.Items[2].(*runtime.Unknown).Kind = "OtherTest"
internal.Items[2].(*runtime.Unknown).APIVersion = "unknown"
if e, a := internal, decoded; !reflect.DeepEqual(e, a) {
t.Log(string(decoded.(*ObjectTest).Items[2].(*runtime.Unknown).RawJSON))
if e, a := internal.Items, list; !reflect.DeepEqual(e, a) {
t.Errorf("mismatched decoded: %s", util.ObjectDiff(e, a))
}
}
Expand Down
40 changes: 38 additions & 2 deletions pkg/runtime/helper.go
Expand Up @@ -23,7 +23,7 @@ import (
"github.com/GoogleCloudPlatform/kubernetes/pkg/conversion"
)

// TODO: move me to pkg/api/meta
// IsListType returns true if the provided Object has a slice called Items
func IsListType(obj Object) bool {
_, err := GetItemsPtr(obj)
return err == nil
Expand All @@ -33,7 +33,6 @@ func IsListType(obj Object) bool {
// If 'list' doesn't have an Items member, it's not really a list type
// and an error will be returned.
// This function will either return a pointer to a slice, or an error, but not both.
// TODO: move me to pkg/api/meta
func GetItemsPtr(list Object) (interface{}, error) {
v, err := conversion.EnforcePtr(list)
if err != nil {
Expand Down Expand Up @@ -150,9 +149,36 @@ func FieldPtr(v reflect.Value, fieldName string, dest interface{}) error {
return fmt.Errorf("couldn't assign/convert %v to %v", field.Type(), v.Type())
}

// DecodeList alters the list in place, attempting to decode any objects found in
// the list that have the runtime.Unknown type. Any errors that occur are returned
// after the entire list is processed. Decoders are tried in order.
func DecodeList(objects []Object, decoders ...ObjectDecoder) []error {
errs := []error(nil)
for i, obj := range objects {
switch t := obj.(type) {
case *Unknown:
for _, decoder := range decoders {
if !decoder.Recognizes(t.APIVersion, t.Kind) {
continue
}
obj, err := decoder.Decode(t.RawJSON)
if err != nil {
errs = append(errs, err)
break
}
objects[i] = obj
break
}
}
}
return errs
}

// MultiObjectTyper returns the types of objects across multiple schemes in order.
type MultiObjectTyper []ObjectTyper

var _ ObjectTyper = MultiObjectTyper{}

func (m MultiObjectTyper) DataVersionAndKind(data []byte) (version, kind string, err error) {
for _, t := range m {
version, kind, err = t.DataVersionAndKind(data)
Expand All @@ -162,6 +188,7 @@ func (m MultiObjectTyper) DataVersionAndKind(data []byte) (version, kind string,
}
return
}

func (m MultiObjectTyper) ObjectVersionAndKind(obj Object) (version, kind string, err error) {
for _, t := range m {
version, kind, err = t.ObjectVersionAndKind(obj)
Expand All @@ -171,3 +198,12 @@ func (m MultiObjectTyper) ObjectVersionAndKind(obj Object) (version, kind string
}
return
}

func (m MultiObjectTyper) Recognizes(version, kind string) bool {
for _, t := range m {
if t.Recognizes(version, kind) {
return true
}
}
return false
}

0 comments on commit 12ba4e2

Please sign in to comment.