Can't replace a generic resource that is cluster scoped

It should be allowed to invoke kubectl replace with a JSON file that has
no resource version set.  Namespaced resources were working correctly,
but cluster resources were silently failing to lookup the current state
of the object to get the resource version because we weren't using
NamespaceIfScoped(). Added a failing test.

hack/test-cmd.sh

@@ -494,6 +494,31 @@ runTests() {
   #cleaning
   rm /tmp/tmp-valid-pod.json
   
+  ## replace of a cluster scoped resource can succeed
+  # Pre-condition: a node exists
+  kubectl create -f - "${kube_flags[@]}" << __EOF__
+{
+  "kind": "Node",
+  "apiVersion": "v1",
+  "metadata": {
+    "name": "node-${version}-test"
+  }
+}
+__EOF__
+  kubectl replace -f - "${kube_flags[@]}" << __EOF__
+{
+  "kind": "Node",
+  "apiVersion": "v1",
+  "metadata": {
+    "name": "node-${version}-test",
+    "annotations": {"a":"b"}
+  }
+}
+__EOF__
+  # Post-condition: the node command succeeds
+  kube::test::get_object_assert "node node-${version}-test" "{{.metadata.annotations.a}}" 'b'
+  kubectl delete node node-${version}-test
+
   ## kubectl edit can update the image field of a POD. tmp-editor.sh is a fake editor
   echo -e '#!/bin/bash\nsed -i "s/nginx/gcr.io\/google_containers\/serve_hostname/g" $1' > /tmp/tmp-editor.sh
   chmod +x /tmp/tmp-editor.sh

pkg/kubectl/resource/helper.go

@@ -144,7 +144,7 @@ func (m *Helper) Replace(namespace, name string, overwrite bool, obj runtime.Obj
 	}
 	if version == "" && overwrite {
 		// Retrieve the current version of the object to overwrite the server object
-		serverObj, err := c.Get().Namespace(namespace).Resource(m.Resource).Name(name).Do().Get()
+		serverObj, err := c.Get().NamespaceIfScoped(namespace, m.NamespaceScoped).Resource(m.Resource).Name(name).Do().Get()
 		if err != nil {
 			// The object does not exist, but we want it to be created
 			return m.replaceResource(c, m.Resource, namespace, name, obj)

pkg/kubectl/resource/helper_test.go

@@ -357,40 +357,42 @@ func TestHelperList(t *testing.T) {
 }
 
 func TestHelperReplace(t *testing.T) {
-	expectPut := func(req *http.Request) bool {
+	expectPut := func(path string, req *http.Request) bool {
 		if req.Method != "PUT" {
 			t.Errorf("unexpected method: %#v", req)
 			return false
 		}
-		parts := splitPath(req.URL.Path)
+		if req.URL.Path != path {
-		if parts[1] != "bar" {
+			t.Errorf("unexpected url: %v", req.URL)
-			t.Errorf("url doesn't contain namespace: %#v", req.URL)
-			return false
-		}
-		if parts[2] != "foo" {
-			t.Errorf("url doesn't contain name: %#v", req)
 			return false
 		}
 		return true
 	}
 
 	tests := []struct {
-		Resp       *http.Response
+		Resp            *http.Response
-		HTTPClient *http.Client
+		HTTPClient      *http.Client
-		HttpErr    error
+		HttpErr         error
-		Overwrite  bool
+		Overwrite       bool
-		Object     runtime.Object
+		Object          runtime.Object
+		Namespace       string
+		NamespaceScoped bool
 
+		ExpectPath   string
 		ExpectObject runtime.Object
 		Err          bool
-		Req          func(*http.Request) bool
+		Req          func(string, *http.Request) bool
 	}{
 		{
-			HttpErr: errors.New("failure"),
+			Namespace:       "bar",
-			Err:     true,
+			NamespaceScoped: true,
+			HttpErr:         errors.New("failure"),
+			Err:             true,
 		},
 		{
-			Object: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}},
+			Namespace:       "bar",
+			NamespaceScoped: true,
+			Object:          &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}},
 			Resp: &http.Response{
 				StatusCode: http.StatusNotFound,
 				Body:       objBody(&unversioned.Status{Status: unversioned.StatusFailure}),
@@ -398,19 +400,26 @@ func TestHelperReplace(t *testing.T) {
 			Err: true,
 		},
 		{
-			Object:       &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}},
+			Namespace:       "bar",
-			ExpectObject: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}},
+			NamespaceScoped: true,
+			Object:          &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}},
+			ExpectPath:      "/namespaces/bar/foo",
+			ExpectObject:    &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}},
 			Resp: &http.Response{
 				StatusCode: http.StatusOK,
 				Body:       objBody(&unversioned.Status{Status: unversioned.StatusSuccess}),
 			},
 			Req: expectPut,
 		},
+		// namespace scoped resource
 		{
+			Namespace:       "bar",
+			NamespaceScoped: true,
 			Object: &api.Pod{
 				ObjectMeta: api.ObjectMeta{Name: "foo"},
 				Spec:       apitesting.DeepEqualSafePodSpec(),
 			},
+			ExpectPath: "/namespaces/bar/foo",
 			ExpectObject: &api.Pod{
 				ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"},
 				Spec:       apitesting.DeepEqualSafePodSpec(),
@@ -424,11 +433,32 @@ func TestHelperReplace(t *testing.T) {
 			}),
 			Req: expectPut,
 		},
+		// cluster scoped resource
 		{
-			Object:       &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"}},
+			Object: &api.Node{
-			ExpectObject: &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"}},
+				ObjectMeta: api.ObjectMeta{Name: "foo"},
-			Resp:         &http.Response{StatusCode: http.StatusOK, Body: objBody(&unversioned.Status{Status: unversioned.StatusSuccess})},
+			},
-			Req:          expectPut,
+			ExpectObject: &api.Node{
+				ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"},
+			},
+			Overwrite:  true,
+			ExpectPath: "/foo",
+			HTTPClient: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
+				if req.Method == "PUT" {
+					return &http.Response{StatusCode: http.StatusOK, Body: objBody(&unversioned.Status{Status: unversioned.StatusSuccess})}, nil
+				}
+				return &http.Response{StatusCode: http.StatusOK, Body: objBody(&api.Node{ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"}})}, nil
+			}),
+			Req: expectPut,
+		},
+		{
+			Namespace:       "bar",
+			NamespaceScoped: true,
+			Object:          &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"}},
+			ExpectPath:      "/namespaces/bar/foo",
+			ExpectObject:    &api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "10"}},
+			Resp:            &http.Response{StatusCode: http.StatusOK, Body: objBody(&unversioned.Status{Status: unversioned.StatusSuccess})},
+			Req:             expectPut,
 		},
 	}
 	for i, test := range tests {
@@ -441,23 +471,22 @@ func TestHelperReplace(t *testing.T) {
 		modifier := &Helper{
 			RESTClient:      client,
 			Versioner:       testapi.Default.MetadataAccessor(),
-			NamespaceScoped: true,
+			NamespaceScoped: test.NamespaceScoped,
 		}
-		_, err := modifier.Replace("bar", "foo", test.Overwrite, test.Object)
+		_, err := modifier.Replace(test.Namespace, "foo", test.Overwrite, test.Object)
 		if (err != nil) != test.Err {
 			t.Errorf("%d: unexpected error: %t %v", i, test.Err, err)
 		}
 		if err != nil {
 			continue
 		}
-		if test.Req != nil && !test.Req(client.Req) {
+		if test.Req != nil && !test.Req(test.ExpectPath, client.Req) {
 			t.Errorf("%d: unexpected request: %#v", i, client.Req)
 		}
 		body, err := ioutil.ReadAll(client.Req.Body)
 		if err != nil {
 			t.Fatalf("%d: unexpected error: %#v", i, err)
 		}
-		t.Logf("got body: %s", string(body))
 		expect := []byte{}
 		if test.ExpectObject != nil {
 			expect = []byte(runtime.EncodeOrDie(testapi.Default.Codec(), test.ExpectObject))