From 6aa7ce268993bf1fdc8bba502d554431840d7ed0 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Mon, 31 Aug 2015 22:28:08 -0700 Subject: [PATCH] addressed comments. --- pkg/expapi/types.go | 1 - pkg/expapi/v1/types.go | 1 - pkg/master/master.go | 4 +- pkg/master/master_test.go | 57 ++++++-- pkg/registry/thirdpartyresourcedata/codec.go | 128 +++++++++++++++--- .../thirdpartyresourcedata/codec_test.go | 123 +++++++++++++++++ 6 files changed, 279 insertions(+), 35 deletions(-) create mode 100644 pkg/registry/thirdpartyresourcedata/codec_test.go diff --git a/pkg/expapi/types.go b/pkg/expapi/types.go index d826c60e04e..a9d84216443 100644 --- a/pkg/expapi/types.go +++ b/pkg/expapi/types.go @@ -271,7 +271,6 @@ type DeploymentList struct { Items []Deployment `json:"items"` } -<<<<<<< HEAD // DaemonSpec is the specification of a daemon. type DaemonSpec struct { // Selector is a label query over pods that are managed by the daemon. diff --git a/pkg/expapi/v1/types.go b/pkg/expapi/v1/types.go index 956e7334466..d2c53765e03 100644 --- a/pkg/expapi/v1/types.go +++ b/pkg/expapi/v1/types.go @@ -258,7 +258,6 @@ type DeploymentList struct { Items []Deployment `json:"items" description:"list of deployments"` } -<<<<<<< HEAD // DaemonSpec is the specification of a daemon. type DaemonSpec struct { // Selector is a label query over pods that are managed by the daemon. diff --git a/pkg/master/master.go b/pkg/master/master.go index 84fc0d7b84c..515c76873c4 100644 --- a/pkg/master/master.go +++ b/pkg/master/master.go @@ -799,11 +799,11 @@ func (m *Master) thirdpartyapi(group, kind, version string) *apiserver.APIGroupV return &apiserver.APIGroupVersion{ Root: apiRoot, - Creater: api.Scheme, + Creater: thirdpartyresourcedata.NewObjectCreator(version, api.Scheme), Convertor: api.Scheme, Typer: api.Scheme, - Mapper: thirdpartyresourcedata.NewMapper(explatest.RESTMapper, kind), + Mapper: thirdpartyresourcedata.NewMapper(explatest.RESTMapper, kind, version), Codec: explatest.Codec, Linker: explatest.SelfLinker, Storage: storage, diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go index dcc58bbd195..5934983c15f 100644 --- a/pkg/master/master_test.go +++ b/pkg/master/master_test.go @@ -83,6 +83,8 @@ func TestFindExternalAddress(t *testing.T) { } } +var versionsToTest = []string{"v1", "v3"} + type Foo struct { api.TypeMeta `json:",inline"` api.ObjectMeta `json:"metadata,omitempty" description:"standard object metadata"` @@ -98,7 +100,7 @@ type FooList struct { items []Foo `json:"items"` } -func initThirdParty(t *testing.T) (*tools.FakeEtcdClient, *httptest.Server) { +func initThirdParty(t *testing.T, version string) (*tools.FakeEtcdClient, *httptest.Server) { master := &Master{} api := &expapi.ThirdPartyResource{ ObjectMeta: api.ObjectMeta{ @@ -107,7 +109,7 @@ func initThirdParty(t *testing.T) (*tools.FakeEtcdClient, *httptest.Server) { Versions: []expapi.APIVersion{ { APIGroup: "group", - Name: "v3", + Name: version, }, }, } @@ -127,12 +129,18 @@ func initThirdParty(t *testing.T) (*tools.FakeEtcdClient, *httptest.Server) { } func TestInstallThirdPartyAPIList(t *testing.T) { - fakeClient, server := initThirdParty(t) + for _, version := range versionsToTest { + testInstallThirdPartyAPIListVersion(t, version) + } +} + +func testInstallThirdPartyAPIListVersion(t *testing.T, version string) { + fakeClient, server := initThirdParty(t, version) defer server.Close() fakeClient.ExpectNotFoundGet(etcdtest.PathPrefix() + "/ThirdPartyResourceData/company.com/foos/default") - resp, err := http.Get(server.URL + "/thirdparty/company.com/v1/namespaces/default/foos") + resp, err := http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos") if err != nil { t.Errorf("unexpected error: %v", err) return @@ -190,7 +198,13 @@ func decodeResponse(resp *http.Response, obj interface{}) error { } func TestInstallThirdPartyAPIGet(t *testing.T) { - fakeClient, server := initThirdParty(t) + for _, version := range versionsToTest { + testInstallThirdPartyAPIGetVersion(t, version) + } +} + +func testInstallThirdPartyAPIGetVersion(t *testing.T, version string) { + fakeClient, server := initThirdParty(t, version) defer server.Close() expectedObj := Foo{ @@ -198,7 +212,8 @@ func TestInstallThirdPartyAPIGet(t *testing.T) { Name: "test", }, TypeMeta: api.TypeMeta{ - Kind: "Foo", + Kind: "Foo", + APIVersion: version, }, SomeField: "test field", OtherField: 10, @@ -209,7 +224,7 @@ func TestInstallThirdPartyAPIGet(t *testing.T) { return } - resp, err := http.Get(server.URL + "/thirdparty/company.com/v1/namespaces/default/foos/test") + resp, err := http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") if err != nil { t.Errorf("unexpected error: %v", err) return @@ -229,7 +244,13 @@ func TestInstallThirdPartyAPIGet(t *testing.T) { } func TestInstallThirdPartyAPIPost(t *testing.T) { - fakeClient, server := initThirdParty(t) + for _, version := range versionsToTest { + testInstallThirdPartyAPIPostForVersion(t, version) + } +} + +func testInstallThirdPartyAPIPostForVersion(t *testing.T, version string) { + fakeClient, server := initThirdParty(t, version) defer server.Close() inputObj := Foo{ @@ -237,7 +258,8 @@ func TestInstallThirdPartyAPIPost(t *testing.T) { Name: "test", }, TypeMeta: api.TypeMeta{ - Kind: "Foo", + Kind: "Foo", + APIVersion: version, }, SomeField: "test field", OtherField: 10, @@ -248,7 +270,7 @@ func TestInstallThirdPartyAPIPost(t *testing.T) { return } - resp, err := http.Post(server.URL+"/thirdparty/company.com/v1/namespaces/default/foos", "application/json", bytes.NewBuffer(data)) + resp, err := http.Post(server.URL+"/thirdparty/company.com/"+version+"/namespaces/default/foos", "application/json", bytes.NewBuffer(data)) if err != nil { t.Errorf("unexpected error: %v", err) return @@ -270,6 +292,7 @@ func TestInstallThirdPartyAPIPost(t *testing.T) { etcdResp, err := fakeClient.Get(etcdtest.PathPrefix()+"/ThirdPartyResourceData/company.com/foos/default/test", false, false) if err != nil { t.Errorf("unexpected error: %v", err) + t.FailNow() } obj, err := explatest.Codec.Decode([]byte(etcdResp.Node.Value)) if err != nil { @@ -290,7 +313,13 @@ func TestInstallThirdPartyAPIPost(t *testing.T) { } func TestInstallThirdPartyAPIDelete(t *testing.T) { - fakeClient, server := initThirdParty(t) + for _, version := range versionsToTest { + testInstallThirdPartyAPIDeleteVersion(t, version) + } +} + +func testInstallThirdPartyAPIDeleteVersion(t *testing.T, version string) { + fakeClient, server := initThirdParty(t, version) defer server.Close() expectedObj := Foo{ @@ -309,7 +338,7 @@ func TestInstallThirdPartyAPIDelete(t *testing.T) { return } - resp, err := http.Get(server.URL + "/thirdparty/company.com/v1/namespaces/default/foos/test") + resp, err := http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") if err != nil { t.Errorf("unexpected error: %v", err) return @@ -328,7 +357,7 @@ func TestInstallThirdPartyAPIDelete(t *testing.T) { t.Errorf("expected:\n%v\nsaw:\n%v\n", expectedObj, item) } - resp, err = httpDelete(server.URL + "/thirdparty/company.com/v1/namespaces/default/foos/test") + resp, err = httpDelete(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") if err != nil { t.Errorf("unexpected error: %v", err) return @@ -338,7 +367,7 @@ func TestInstallThirdPartyAPIDelete(t *testing.T) { t.Errorf("unexpected status: %v", resp) } - resp, err = http.Get(server.URL + "/thirdparty/company.com/v1/namespaces/default/foos/test") + resp, err = http.Get(server.URL + "/thirdparty/company.com/" + version + "/namespaces/default/foos/test") if err != nil { t.Errorf("unexpected error: %v", err) return diff --git a/pkg/registry/thirdpartyresourcedata/codec.go b/pkg/registry/thirdpartyresourcedata/codec.go index 1d9dbc1b6d2..bc190a1b9fd 100644 --- a/pkg/registry/thirdpartyresourcedata/codec.go +++ b/pkg/registry/thirdpartyresourcedata/codec.go @@ -25,12 +25,14 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/meta" "k8s.io/kubernetes/pkg/expapi" + "k8s.io/kubernetes/pkg/expapi/latest" "k8s.io/kubernetes/pkg/runtime" ) type thirdPartyResourceDataMapper struct { - mapper meta.RESTMapper - kind string + mapper meta.RESTMapper + kind string + version string } func (t *thirdPartyResourceDataMapper) GroupForResource(resource string) (string, error) { @@ -38,7 +40,16 @@ func (t *thirdPartyResourceDataMapper) GroupForResource(resource string) (string } func (t *thirdPartyResourceDataMapper) RESTMapping(kind string, versions ...string) (*meta.RESTMapping, error) { - mapping, err := t.mapper.RESTMapping(kind, versions...) + if len(versions) != 1 { + return nil, fmt.Errorf("unexpected set of versions: %v", versions) + } + if versions[0] != t.version { + return nil, fmt.Errorf("unknown version %s expected %s", versions[0], t.version) + } + if kind != "ThirdPartyResourceData" { + return nil, fmt.Errorf("unknown kind %s expected %s", kind, t.kind) + } + mapping, err := t.mapper.RESTMapping("ThirdPartyResourceData", latest.Version) if err != nil { return nil, err } @@ -58,8 +69,12 @@ func (t *thirdPartyResourceDataMapper) VersionAndKindForResource(resource string return t.mapper.VersionAndKindForResource(resource) } -func NewMapper(mapper meta.RESTMapper, kind string) meta.RESTMapper { - return &thirdPartyResourceDataMapper{mapper, kind} +func NewMapper(mapper meta.RESTMapper, kind, version string) meta.RESTMapper { + return &thirdPartyResourceDataMapper{ + mapper: mapper, + kind: kind, + version: version, + } } type thirdPartyResourceDataCodec struct { @@ -81,9 +96,13 @@ func (t *thirdPartyResourceDataCodec) populate(objIn *expapi.ThirdPartyResourceD if !ok { return fmt.Errorf("unexpected object: %#v", obj) } + return t.populateFromObject(objIn, mapObj, data) +} + +func (t *thirdPartyResourceDataCodec) populateFromObject(objIn *expapi.ThirdPartyResourceData, mapObj map[string]interface{}, data []byte) error { kind, ok := mapObj["kind"].(string) if !ok { - return fmt.Errorf("unexpected object: %#v", obj) + return fmt.Errorf("unexpected object for kind: %#v", mapObj["kind"]) } if kind != t.kind { return fmt.Errorf("unexpected kind: %s, expected: %s", kind, t.kind) @@ -91,22 +110,40 @@ func (t *thirdPartyResourceDataCodec) populate(objIn *expapi.ThirdPartyResourceD metadata, ok := mapObj["metadata"].(map[string]interface{}) if !ok { - return fmt.Errorf("unexpected object: %#v", obj) + return fmt.Errorf("unexpected object for metadata: %#v", mapObj["metadata"]) + } + + if resourceVersion, ok := metadata["resourceVersion"]; ok { + resourceVersionStr, ok := resourceVersion.(string) + if !ok { + return fmt.Errorf("unexpected object for resourceVersion: %v", resourceVersion) + } + + objIn.ResourceVersion = resourceVersionStr } name, ok := metadata["name"].(string) if !ok { - return fmt.Errorf("unexpected object: %#v", obj) + return fmt.Errorf("unexpected object for name: %#v", metadata) } - - if resourceVersion, ok := metadata["resourceVersion"]; ok { - resourceVersionStr, ok := resourceVersion.(string) + + if labels, ok := metadata["labels"]; ok { + labelMap, ok := labels.(map[string]interface{}) if !ok { - return fmt.Errorf("unexpected object: %v", metadata["resourceVersion"]) + return fmt.Errorf("unexpected object for labels: %v", labelMap) + } + for key, value := range labelMap { + valueStr, ok := value.(string) + if !ok { + return fmt.Errorf("unexpected label: %v", value) + } + if objIn.Labels == nil { + objIn.Labels = map[string]string{} + } + objIn.Labels[key] = valueStr } - objIn.ResourceVersion = strconv.Atoi(resourceVersionStr) } - + objIn.Name = name objIn.Data = data return nil @@ -141,16 +178,50 @@ func (t *thirdPartyResourceDataCodec) DecodeInto(data []byte, obj runtime.Object return t.populate(thirdParty, data) } -func (t *thirdPartyResourceDataCodec) DecodeIntoWithSpecifiedVersionKind(data []byte, obj runtime.Object, kind, version string) error { +func (t *thirdPartyResourceDataCodec) DecodeIntoWithSpecifiedVersionKind(data []byte, obj runtime.Object, version, kind string) error { thirdParty, ok := obj.(*expapi.ThirdPartyResourceData) if !ok { return fmt.Errorf("unexpected object: %#v", obj) } + + if kind != "ThirdPartyResourceData" { + return fmt.Errorf("unexpeceted kind: %s", kind) + } + + var dataObj interface{} + if err := json.Unmarshal(data, &dataObj); err != nil { + return err + } + mapObj, ok := dataObj.(map[string]interface{}) + if !ok { + return fmt.Errorf("unexpcted object: %#v", dataObj) + } + if kindObj, found := mapObj["kind"]; !found { + mapObj["kind"] = kind + } else { + kindStr, ok := kindObj.(string) + if !ok { + return fmt.Errorf("unexpected object for 'kind': %v", kindObj) + } + if kindStr != t.kind { + return fmt.Errorf("kind doesn't match, expecting: %s, got %s", kind, kindStr) + } + } + if versionObj, found := mapObj["apiVersion"]; !found { + mapObj["apiVersion"] = version + } else { + versionStr, ok := versionObj.(string) + if !ok { + return fmt.Errorf("unexpected object for 'apiVersion': %v", versionObj) + } + if versionStr != version { + return fmt.Errorf("version doesn't match, expecting: %s, got %s", version, versionStr) + } + } + if err := t.populate(thirdParty, data); err != nil { return err } - thirdParty.Kind = kind - thirdParty.APIVersion = version return nil } @@ -178,3 +249,26 @@ func (t *thirdPartyResourceDataCodec) Encode(obj runtime.Object) (data []byte, e return nil, fmt.Errorf("unexpected object to encode: %#v", obj) } } + +func NewObjectCreator(version string, delegate runtime.ObjectCreater) runtime.ObjectCreater { + return &thirdPartyResourceDataCreator{version, delegate} +} + +type thirdPartyResourceDataCreator struct { + version string + delegate runtime.ObjectCreater +} + +func (t *thirdPartyResourceDataCreator) New(version, kind string) (out runtime.Object, err error) { + if t.version != version { + return nil, fmt.Errorf("unknown version %s for kind %s", version, kind) + } + switch kind { + case "ThirdPartyResourceData": + return &expapi.ThirdPartyResourceData{}, nil + case "ThirdPartyResourceDataList": + return &expapi.ThirdPartyResourceDataList{}, nil + default: + return t.delegate.New(latest.Version, kind) + } +} diff --git a/pkg/registry/thirdpartyresourcedata/codec_test.go b/pkg/registry/thirdpartyresourcedata/codec_test.go new file mode 100644 index 00000000000..30665cfd7c5 --- /dev/null +++ b/pkg/registry/thirdpartyresourcedata/codec_test.go @@ -0,0 +1,123 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package thirdpartyresourcedata + +import ( + "encoding/json" + "reflect" + "testing" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/expapi" +) + +type Foo struct { + api.TypeMeta `json:",inline"` + api.ObjectMeta `json:"metadata,omitempty" description:"standard object metadata"` + + SomeField string `json:"someField"` + OtherField int `json:"otherField"` +} + +type FooList struct { + api.TypeMeta `json:",inline"` + api.ListMeta `json:"metadata,omitempty" description:"standard list metadata; see http://docs.k8s.io/api-conventions.md#metadata"` + + items []Foo `json:"items"` +} + +func TestCodec(t *testing.T) { + tests := []struct { + obj *Foo + expectErr bool + name string + }{ + { + obj: &Foo{ObjectMeta: api.ObjectMeta{Name: "bar"}}, + expectErr: true, + name: "missing kind", + }, + { + obj: &Foo{ObjectMeta: api.ObjectMeta{Name: "bar"}, TypeMeta: api.TypeMeta{Kind: "Foo"}}, + name: "basic", + }, + { + obj: &Foo{ObjectMeta: api.ObjectMeta{Name: "bar", ResourceVersion: "baz"}, TypeMeta: api.TypeMeta{Kind: "Foo"}}, + name: "resource version", + }, + { + obj: &Foo{ + ObjectMeta: api.ObjectMeta{ + Name: "bar", + ResourceVersion: "baz", + Labels: map[string]string{"foo": "bar", "baz": "blah"}, + }, + TypeMeta: api.TypeMeta{Kind: "Foo"}, + }, + name: "labels", + }, + } + for _, test := range tests { + codec := thirdPartyResourceDataCodec{kind: "Foo"} + data, err := json.Marshal(test.obj) + if err != nil { + t.Errorf("[%s] unexpected error: %v", test.name, err) + continue + } + obj, err := codec.Decode(data) + if err != nil && !test.expectErr { + t.Errorf("[%s] unexpected error: %v", test.name, err) + continue + } + if test.expectErr { + if err == nil { + t.Errorf("[%s] unexpected non-error", test.name) + } + continue + } + rsrcObj, ok := obj.(*expapi.ThirdPartyResourceData) + if !ok { + t.Errorf("[%s] unexpected object: %v", test.name, obj) + continue + } + if !reflect.DeepEqual(rsrcObj.ObjectMeta, test.obj.ObjectMeta) { + t.Errorf("[%s]\nexpected\n%v\nsaw\n%v\n", test.name, rsrcObj.ObjectMeta, test.obj.ObjectMeta) + } + var output Foo + if err := json.Unmarshal(rsrcObj.Data, &output); err != nil { + t.Errorf("[%s] unexpected error: %v", test.name, err) + continue + } + if !reflect.DeepEqual(&output, test.obj) { + t.Errorf("[%s]\nexpected\n%v\nsaw\n%v\n", test.name, test.obj, &output) + } + + data, err = codec.Encode(rsrcObj) + if err != nil { + t.Errorf("[%s] unexpected error: %v", test.name, err) + } + + var output2 Foo + if err := json.Unmarshal(data, &output2); err != nil { + t.Errorf("[%s] unexpected error: %v", test.name, err) + continue + } + if !reflect.DeepEqual(&output2, test.obj) { + t.Errorf("[%s]\nexpected\n%v\nsaw\n%v\n", test.name, test.obj, &output2) + } + } +}