Merge pull request #10200 from caesarxuchao/resthandler-validate-version

verify and default APIVersion in createHandler, verify APIVersion in UpdateResource

pkg/api/meta/restmapper_test.go

@@ -37,6 +37,10 @@ func (fakeCodec) DecodeInto([]byte, runtime.Object) error {
 	return nil
 }
 
+func (fakeCodec) DecodeIntoWithSpecifiedVersionKind([]byte, runtime.Object, string, string) error {
+	return nil
+}
+
 type fakeConvertor struct{}
 
 func (fakeConvertor) Convert(in, out interface{}) error {

pkg/apiserver/apiserver_test.go

@@ -101,6 +101,7 @@ func addTestTypes() {
 		ResourceVersion string `json:"resourceVersion,omitempty"`
 	}
 	api.Scheme.AddKnownTypes(testVersion, &Simple{}, &SimpleList{}, &api.Status{}, &ListOptions{}, &api.DeleteOptions{}, &SimpleGetOptions{}, &SimpleRoot{})
+	api.Scheme.AddKnownTypes(testVersion, &api.Pod{})
 }
 
 func addNewTestTypes() {
@@ -697,10 +698,12 @@ func TestUnimplementedRESTStorage(t *testing.T) {
 		response, err := client.Do(request)
 		if err != nil {
 			t.Fatalf("unexpected error: %v", err)
-			continue
 		}
 		defer response.Body.Close()
-		data, _ := ioutil.ReadAll(response.Body)
+		data, err := ioutil.ReadAll(response.Body)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
 		if response.StatusCode != v.ErrCode {
 			t.Errorf("%s: expected %d for %s, Got %s", k, v.ErrCode, v.Method, string(data))
 			continue
@@ -843,7 +846,11 @@ func TestList(t *testing.T) {
 		}
 		if resp.StatusCode != http.StatusOK {
 			t.Errorf("%d: unexpected status: %d, Expected: %d, %#v", i, resp.StatusCode, http.StatusOK, resp)
-			body, _ := ioutil.ReadAll(resp.Body)
+			body, err := ioutil.ReadAll(resp.Body)
+			if err != nil {
+				t.Errorf("%d: unexpected error: %v", i, err)
+				continue
+			}
 			t.Logf("%d: body: %s", i, string(body))
 			continue
 		}
@@ -907,7 +914,10 @@ func TestNonEmptyList(t *testing.T) {
 
 	if resp.StatusCode != http.StatusOK {
 		t.Errorf("Unexpected status: %d, Expected: %d, %#v", resp.StatusCode, http.StatusOK, resp)
-		body, _ := ioutil.ReadAll(resp.Body)
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
 		t.Logf("Data: %s", string(body))
 	}
 
@@ -955,7 +965,10 @@ func TestSelfLinkSkipsEmptyName(t *testing.T) {
 
 	if resp.StatusCode != http.StatusOK {
 		t.Errorf("Unexpected status: %d, Expected: %d, %#v", resp.StatusCode, http.StatusOK, resp)
-		body, _ := ioutil.ReadAll(resp.Body)
+		body, err := ioutil.ReadAll(resp.Body)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
 		t.Logf("Data: %s", string(body))
 	}
 	var listOut SimpleList
@@ -1050,7 +1063,10 @@ func TestGetBinary(t *testing.T) {
 	server := httptest.NewServer(handle(map[string]rest.Storage{"simple": &simpleStorage}))
 	defer server.Close()
 
-	req, _ := http.NewRequest("GET", server.URL+"/api/version/namespaces/default/simple/binary", nil)
+	req, err := http.NewRequest("GET", server.URL+"/api/version/namespaces/default/simple/binary", nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
 	req.Header.Add("Accept", "text/other, */*")
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
@@ -1495,7 +1511,10 @@ func TestDeleteWithOptions(t *testing.T) {
 	}
 	if res.StatusCode != http.StatusOK {
 		t.Errorf("unexpected response: %s %#v", request.URL, res)
-		s, _ := ioutil.ReadAll(res.Body)
+		s, err := ioutil.ReadAll(res.Body)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
 		t.Logf(string(s))
 	}
 	if simpleStorage.deleted != ID {
@@ -1933,7 +1952,10 @@ func TestCreateNotFound(t *testing.T) {
 	client := http.Client{}
 
 	simple := &Simple{Other: "foo"}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/simple", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -1956,7 +1978,10 @@ func TestCreateChecksDecode(t *testing.T) {
 	client := http.Client{}
 
 	simple := &api.Pod{}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/simple", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -1968,7 +1993,10 @@ func TestCreateChecksDecode(t *testing.T) {
 	if response.StatusCode != http.StatusBadRequest {
 		t.Errorf("Unexpected response %#v", response)
 	}
-	if b, _ := ioutil.ReadAll(response.Body); !strings.Contains(string(b), "must be of type Simple") {
+	b, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	} else if !strings.Contains(string(b), "cannot be handled as a Simple") {
 		t.Errorf("unexpected response: %s", string(b))
 	}
 }
@@ -2059,7 +2087,10 @@ func TestCreateWithName(t *testing.T) {
 	client := http.Client{}
 
 	simple := &Simple{Other: "foo"}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/simple/"+pathName+"/sub", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -2083,7 +2114,10 @@ func TestUpdateChecksDecode(t *testing.T) {
 	client := http.Client{}
 
 	simple := &api.Pod{}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("PUT", server.URL+"/api/version/namespaces/default/simple/bar", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -2095,7 +2129,10 @@ func TestUpdateChecksDecode(t *testing.T) {
 	if response.StatusCode != http.StatusBadRequest {
 		t.Errorf("Unexpected response %#v", response)
 	}
-	if b, _ := ioutil.ReadAll(response.Body); !strings.Contains(string(b), "must be of type Simple") {
+	b, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	} else if !strings.Contains(string(b), "cannot be handled as a Simple") {
 		t.Errorf("unexpected response: %s", string(b))
 	}
 }
@@ -2153,7 +2190,10 @@ func TestCreate(t *testing.T) {
 	simple := &Simple{
 		Other: "bar",
 	}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/default/foo", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -2209,7 +2249,10 @@ func TestCreateInNamespace(t *testing.T) {
 	simple := &Simple{
 		Other: "bar",
 	}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/other/foo", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -2265,7 +2308,10 @@ func TestCreateInvokesAdmissionControl(t *testing.T) {
 	simple := &Simple{
 		Other: "bar",
 	}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	request, err := http.NewRequest("POST", server.URL+"/api/version/namespaces/other/foo", bytes.NewBuffer(data))
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
@@ -2388,7 +2434,10 @@ func TestCreateTimeout(t *testing.T) {
 	defer server.Close()
 
 	simple := &Simple{Other: "foo"}
-	data, _ := codec.Encode(simple)
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	itemOut := expectApiStatus(t, "POST", server.URL+"/api/version/namespaces/default/foo?timeout=4ms", data, apierrs.StatusServerTimeout)
 	if itemOut.Status != api.StatusFailure || itemOut.Reason != api.StatusReasonTimeout {
 		t.Errorf("Unexpected status %#v", itemOut)
@@ -2468,3 +2517,99 @@ func TestCORSAllowedOrigins(t *testing.T) {
 		}
 	}
 }
+
+func TestCreateChecksAPIVersion(t *testing.T) {
+	handler := handle(map[string]rest.Storage{"simple": &SimpleRESTStorage{}})
+	server := httptest.NewServer(handler)
+	defer server.Close()
+	client := http.Client{}
+
+	simple := &Simple{}
+	//using newCodec and send the request to testVersion URL shall cause a discrepancy in apiVersion
+	data, err := newCodec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	request, err := http.NewRequest("POST", server.URL+"/api/"+testVersion+"/namespaces/default/simple", bytes.NewBuffer(data))
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	response, err := client.Do(request)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if response.StatusCode != http.StatusBadRequest {
+		t.Errorf("Unexpected response %#v", response)
+	}
+	b, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	} else if !strings.Contains(string(b), "does not match the specified apiVersion") {
+		t.Errorf("unexpected response: %s", string(b))
+	}
+}
+
+func TestCreateDefaultsAPIVersion(t *testing.T) {
+	handler := handle(map[string]rest.Storage{"simple": &SimpleRESTStorage{}})
+	server := httptest.NewServer(handler)
+	defer server.Close()
+	client := http.Client{}
+
+	simple := &Simple{}
+	data, err := codec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	m := make(map[string]interface{})
+	if err := json.Unmarshal(data, &m); err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	delete(m, "apiVersion")
+	data, err = json.Marshal(m)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	request, err := http.NewRequest("POST", server.URL+"/api/"+testVersion+"/namespaces/default/simple", bytes.NewBuffer(data))
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	response, err := client.Do(request)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if response.StatusCode != http.StatusCreated {
+		t.Errorf("unexpected status: %d, Expected: %d, %#v", response.StatusCode, http.StatusCreated, response)
+	}
+}
+
+func TestUpdateChecksAPIVersion(t *testing.T) {
+	handler := handle(map[string]rest.Storage{"simple": &SimpleRESTStorage{}})
+	server := httptest.NewServer(handler)
+	defer server.Close()
+	client := http.Client{}
+
+	simple := &Simple{ObjectMeta: api.ObjectMeta{Name: "bar"}}
+	data, err := newCodec.Encode(simple)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	request, err := http.NewRequest("PUT", server.URL+"/api/"+testVersion+"/namespaces/default/simple/bar", bytes.NewBuffer(data))
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	response, err := client.Do(request)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	if response.StatusCode != http.StatusBadRequest {
+		t.Errorf("Unexpected response %#v", response)
+	}
+	b, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	} else if !strings.Contains(string(b), "does not match the specified apiVersion") {
+		t.Errorf("unexpected response: %s", string(b))
+	}
+}

pkg/apiserver/resthandler.go

@@ -302,7 +302,7 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object
 		}
 
 		obj := r.New()
-		if err := scope.Codec.DecodeInto(body, obj); err != nil {
+		if err := scope.Codec.DecodeIntoWithSpecifiedVersionKind(body, obj, scope.APIVersion, scope.Kind); err != nil {
 			err = transformDecodeError(typer, err, obj, body)
 			errorJSON(err, scope.Codec, w)
 			return
@@ -469,7 +469,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType
 		}
 
 		obj := r.New()
-		if err := scope.Codec.DecodeInto(body, obj); err != nil {
+		if err := scope.Codec.DecodeIntoWithSpecifiedVersionKind(body, obj, scope.APIVersion, scope.Kind); err != nil {
 			err = transformDecodeError(typer, err, obj, body)
 			errorJSON(err, scope.Codec, w)
 			return

pkg/conversion/decode.go

@@ -80,6 +80,17 @@ func (s *Scheme) Decode(data []byte) (interface{}, error) {
 // If obj's version doesn't match that in data, an attempt will be made to convert
 // data into obj's version.
 func (s *Scheme) DecodeInto(data []byte, obj interface{}) error {
+	return s.DecodeIntoWithSpecifiedVersionKind(data, obj, "", "")
+}
+
+// DecodeIntoWithSpecifiedVersionKind compares the passed in specifiedVersion and
+// specifiedKind with data.Version and data.Kind, defaulting data.Version and
+// data.Kind to the specified value if they are empty, or generating an error if
+// data.Version and data.Kind are not empty and differ from the specified value.
+// The function then implements the functionality of DecodeInto.
+// If specifiedVersion and specifiedKind are empty, the function degenerates to
+// DecodeInto.
+func (s *Scheme) DecodeIntoWithSpecifiedVersionKind(data []byte, obj interface{}, specifiedVersion, specifiedKind string) error {
 	if len(data) == 0 {
 		return errors.New("empty input")
 	}
@@ -87,6 +98,19 @@ func (s *Scheme) DecodeInto(data []byte, obj interface{}) error {
 	if err != nil {
 		return err
 	}
+	if dataVersion == "" {
+		dataVersion = specifiedVersion
+	}
+	if dataKind == "" {
+		dataKind = specifiedKind
+	}
+	if len(specifiedVersion) > 0 && (dataVersion != specifiedVersion) {
+		return errors.New(fmt.Sprintf("The apiVersion in the data (%s) does not match the specified apiVersion(%s)", dataVersion, specifiedVersion))
+	}
+	if len(specifiedKind) > 0 && (dataKind != specifiedKind) {
+		return errors.New(fmt.Sprintf("The kind in the data (%s) does not match the specified kind(%s)", dataKind, specifiedKind))
+	}
+
 	objVersion, objKind, err := s.ObjectVersionAndKind(obj)
 	if err != nil {
 		return err

pkg/runtime/interfaces.go

@@ -38,6 +38,7 @@ type ObjectCodec interface {
 type Decoder interface {
 	Decode(data []byte) (Object, error)
 	DecodeInto(data []byte, obj Object) error
+	DecodeIntoWithSpecifiedVersionKind(data []byte, obj Object, kind, version string) error
 }
 
 // Encoder defines methods for serializing API objects into bytes

pkg/runtime/scheme.go

@@ -458,6 +458,10 @@ func (s *Scheme) DecodeInto(data []byte, obj Object) error {
 	return s.raw.DecodeInto(data, obj)
 }
 
+func (s *Scheme) DecodeIntoWithSpecifiedVersionKind(data []byte, obj Object, version, kind string) error {
+	return s.raw.DecodeIntoWithSpecifiedVersionKind(data, obj, version, kind)
+}
+
 // Copy does a deep copy of an API object.  Useful mostly for tests.
 func (s *Scheme) Copy(src Object) (Object, error) {
 	dst, err := s.raw.DeepCopy(src)

pkg/runtime/unstructured.go

@@ -74,6 +74,10 @@ func (unstructuredJSONScheme) DecodeInto(data []byte, obj Object) error {
 	return nil
 }
 
+func (unstructuredJSONScheme) DecodeIntoWithSpecifiedVersionKind(data []byte, obj Object, kind, version string) error {
+	return nil
+}
+
 func (unstructuredJSONScheme) DataVersionAndKind(data []byte) (version, kind string, err error) {
 	obj := TypeMeta{}
 	if err := json.Unmarshal(data, &obj); err != nil {

test/integration/auth_test.go

@@ -51,12 +51,6 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/test/integration/framework"
 )
 
-var nodeResourceName string
-
-func init() {
-	nodeResourceName = "nodes"
-}
-
 const (
 	AliceToken   string = "abc123" // username: alice.  Present in token file.
 	BobToken     string = "xyz987" // username: bob.  Present in token file.
@@ -86,7 +80,7 @@ func timeoutPath(resource, namespace, name string) string {
 var aPod string = `
 {
   "kind": "Pod",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a",
     "creationTimestamp": null%s
@@ -104,7 +98,7 @@ var aPod string = `
 var aRC string = `
 {
   "kind": "ReplicationController",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a",
     "labels": {
@@ -137,7 +131,7 @@ var aRC string = `
 var aService string = `
 {
   "kind": "Service",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a",
     "labels": {
@@ -161,7 +155,7 @@ var aService string = `
 var aNode string = `
 {
   "kind": "Node",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a"%s
   },
@@ -173,7 +167,7 @@ var aNode string = `
 var aEvent string = `
 {
   "kind": "Event",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a"%s
   },
@@ -189,7 +183,7 @@ var aEvent string = `
 var aBinding string = `
 {
   "kind": "Binding",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a"%s
   },
@@ -212,7 +206,7 @@ var emptyEndpoints string = `
 var aEndpoints string = `
 {
   "kind": "Endpoints",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "metadata": {
     "name": "a"%s
   },
@@ -237,7 +231,7 @@ var aEndpoints string = `
 var deleteNow string = `
 {
   "kind": "DeleteOptions",
-  "apiVersion": "v1",
+  "apiVersion": "` + testapi.Version() + `",
   "gracePeriodSeconds": null%s
 }
 `
@@ -337,11 +331,11 @@ func getTestRequests() []struct {
 		{"DELETE", timeoutPath("endpoints", api.NamespaceDefault, "a"), "", code200},
 
 		// Normal methods on minions
-		{"GET", path(nodeResourceName, "", ""), "", code200},
-		{"POST", timeoutPath(nodeResourceName, "", ""), aNode, code201},
-		{"PUT", timeoutPath(nodeResourceName, "", "a"), aNode, code200},
-		{"GET", path(nodeResourceName, "", "a"), "", code200},
-		{"DELETE", timeoutPath(nodeResourceName, "", "a"), "", code200},
+		{"GET", path("nodes", "", ""), "", code200},
+		{"POST", timeoutPath("nodes", "", ""), aNode, code201},
+		{"PUT", timeoutPath("nodes", "", "a"), aNode, code200},
+		{"GET", path("nodes", "", "a"), "", code200},
+		{"DELETE", timeoutPath("nodes", "", "a"), "", code200},
 
 		// Normal methods on events
 		{"GET", path("events", "", ""), "", code200},
@@ -367,8 +361,8 @@ func getTestRequests() []struct {
 		{"DELETE", timeoutPath("foo", api.NamespaceDefault, ""), "", code404},
 
 		// Special verbs on nodes
-		{"GET", pathWithPrefix("proxy", nodeResourceName, api.NamespaceDefault, "a"), "", code404},
-		{"GET", pathWithPrefix("redirect", nodeResourceName, api.NamespaceDefault, "a"), "", code404},
+		{"GET", pathWithPrefix("proxy", "nodes", api.NamespaceDefault, "a"), "", code404},
+		{"GET", pathWithPrefix("redirect", "nodes", api.NamespaceDefault, "a"), "", code404},
 		// TODO: test .../watch/..., which doesn't end before the test timeout.
 		// TODO: figure out how to create a minion so that it can successfully proxy/redirect.