Always set ?namespace in query if specified

Revise our code to only call Request.Namespace() if a namespace
*should* be present.  For root scoped resources, namespace should
be ignored.  For namespaced resources, it is an error to have
Namespace=="".

cmd/integration/integration.go

@@ -279,18 +279,19 @@ func runReplicationControllerTest(c *client.Client) {
 	}
 
 	glog.Infof("Creating replication controllers")
-	if _, err := c.ReplicationControllers(api.NamespaceDefault).Create(&controller); err != nil {
+	updated, err := c.ReplicationControllers(api.NamespaceDefault).Create(&controller)
+	if err != nil {
 		glog.Fatalf("Unexpected error: %v", err)
 	}
 	glog.Infof("Done creating replication controllers")
 
 	// Give the controllers some time to actually create the pods
-	if err := wait.Poll(time.Second, time.Second*30, client.ControllerHasDesiredReplicas(c, &controller)); err != nil {
+	if err := wait.Poll(time.Second, time.Second*30, client.ControllerHasDesiredReplicas(c, updated)); err != nil {
 		glog.Fatalf("FAILED: pods never created %v", err)
 	}
 
 	// wait for minions to indicate they have info about the desired pods
-	pods, err := c.Pods(api.NamespaceDefault).List(labels.Set(controller.Spec.Selector).AsSelector())
+	pods, err := c.Pods(api.NamespaceDefault).List(labels.Set(updated.Spec.Selector).AsSelector())
 	if err != nil {
 		glog.Fatalf("FAILED: unable to get pods to list: %v", err)
 	}
@@ -523,7 +524,7 @@ func runMasterServiceTest(client *client.Client) {
 }
 
 func runServiceTest(client *client.Client) {
-	pod := api.Pod{
+	pod := &api.Pod{
 		ObjectMeta: api.ObjectMeta{
 			Name: "foo",
 			Labels: map[string]string{
@@ -548,14 +549,14 @@ func runServiceTest(client *client.Client) {
 			PodIP: "1.2.3.4",
 		},
 	}
-	_, err := client.Pods(api.NamespaceDefault).Create(&pod)
+	pod, err := client.Pods(api.NamespaceDefault).Create(pod)
 	if err != nil {
 		glog.Fatalf("Failed to create pod: %v, %v", pod, err)
 	}
 	if err := wait.Poll(time.Second, time.Second*20, podExists(client, pod.Namespace, pod.Name)); err != nil {
 		glog.Fatalf("FAILED: pod never started running %v", err)
 	}
-	svc1 := api.Service{
+	svc1 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "service1"},
 		Spec: api.ServiceSpec{
 			Selector: map[string]string{
@@ -566,7 +567,7 @@ func runServiceTest(client *client.Client) {
 			SessionAffinity: "None",
 		},
 	}
-	_, err = client.Services(api.NamespaceDefault).Create(&svc1)
+	svc1, err = client.Services(api.NamespaceDefault).Create(svc1)
 	if err != nil {
 		glog.Fatalf("Failed to create service: %v, %v", svc1, err)
 	}
@@ -574,7 +575,7 @@ func runServiceTest(client *client.Client) {
 		glog.Fatalf("FAILED: unexpected endpoints: %v", err)
 	}
 	// A second service with the same port.
-	svc2 := api.Service{
+	svc2 := &api.Service{
 		ObjectMeta: api.ObjectMeta{Name: "service2"},
 		Spec: api.ServiceSpec{
 			Selector: map[string]string{
@@ -585,7 +586,7 @@ func runServiceTest(client *client.Client) {
 			SessionAffinity: "None",
 		},
 	}
-	_, err = client.Services(api.NamespaceDefault).Create(&svc2)
+	svc2, err = client.Services(api.NamespaceDefault).Create(svc2)
 	if err != nil {
 		glog.Fatalf("Failed to create service: %v, %v", svc2, err)
 	}

pkg/client/cache/listwatch_test.go

@@ -42,7 +42,7 @@ func buildResourcePath(prefix, namespace, resource string) string {
 	base := path.Join("/api", testapi.Version(), prefix)
 	if len(namespace) > 0 {
 		if !(testapi.Version() == "v1beta1" || testapi.Version() == "v1beta2") {
-			base = path.Join(base, "ns", namespace)
+			base = path.Join(base, "namespaces", namespace)
 		}
 	}
 	return path.Join(base, resource)
@@ -58,11 +58,9 @@ func buildQueryValues(namespace string, query url.Values) url.Values {
 			}
 		}
 	}
-	if len(namespace) > 0 {
 	if testapi.Version() == "v1beta1" || testapi.Version() == "v1beta2" {
 		v.Set("namespace", namespace)
 	}
-	}
 	return v
 }
 

pkg/client/events.go

@@ -66,7 +66,7 @@ func (e *events) Create(event *api.Event) (*api.Event, error) {
 	}
 	result := &api.Event{}
 	err := e.client.Post().
-		Namespace(event.Namespace).
+		NamespaceIfScoped(event.Namespace, len(event.Namespace) > 0).
 		Resource("events").
 		Body(event).
 		Do().
@@ -85,7 +85,7 @@ func (e *events) Update(event *api.Event) (*api.Event, error) {
 	}
 	result := &api.Event{}
 	err := e.client.Put().
-		Namespace(event.Namespace).
+		NamespaceIfScoped(event.Namespace, len(event.Namespace) > 0).
 		Resource("events").
 		Name(event.Name).
 		Body(event).
@@ -98,7 +98,7 @@ func (e *events) Update(event *api.Event) (*api.Event, error) {
 func (e *events) List(label, field labels.Selector) (*api.EventList, error) {
 	result := &api.EventList{}
 	err := e.client.Get().
-		Namespace(e.namespace).
+		NamespaceIfScoped(e.namespace, len(e.namespace) > 0).
 		Resource("events").
 		SelectorParam("labels", label).
 		SelectorParam("fields", field).
@@ -115,7 +115,7 @@ func (e *events) Get(name string) (*api.Event, error) {
 
 	result := &api.Event{}
 	err := e.client.Get().
-		Namespace(e.namespace).
+		NamespaceIfScoped(e.namespace, len(e.namespace) > 0).
 		Resource("events").
 		Name(name).
 		Do().
@@ -127,7 +127,7 @@ func (e *events) Get(name string) (*api.Event, error) {
 func (e *events) Watch(label, field labels.Selector, resourceVersion string) (watch.Interface, error) {
 	return e.client.Get().
 		Prefix("watch").
-		Namespace(e.namespace).
+		NamespaceIfScoped(e.namespace, len(e.namespace) > 0).
 		Resource("events").
 		Param("resourceVersion", resourceVersion).
 		SelectorParam("labels", label).

pkg/client/events_test.go

@@ -62,7 +62,9 @@ func TestEventCreate(t *testing.T) {
 	}
 	timeStamp := util.Now()
 	event := &api.Event{
-		//namespace: namespace{"default"},
+		ObjectMeta: api.ObjectMeta{
+			Namespace: api.NamespaceDefault,
+		},
 		InvolvedObject: *objReference,
 		FirstTimestamp: timeStamp,
 		LastTimestamp:  timeStamp,
@@ -80,7 +82,7 @@ func TestEventCreate(t *testing.T) {
 	response, err := c.Setup().Events("").Create(event)
 
 	if err != nil {
-		t.Errorf("%#v should be nil.", err)
+		t.Fatalf("%v should be nil.", err)
 	}
 
 	if e, a := *objReference, response.InvolvedObject; !reflect.DeepEqual(e, a) {
@@ -99,6 +101,9 @@ func TestEventGet(t *testing.T) {
 	}
 	timeStamp := util.Now()
 	event := &api.Event{
+		ObjectMeta: api.ObjectMeta{
+			Namespace: "other",
+		},
 		InvolvedObject: *objReference,
 		FirstTimestamp: timeStamp,
 		LastTimestamp:  timeStamp,
@@ -116,7 +121,7 @@ func TestEventGet(t *testing.T) {
 	response, err := c.Setup().Events("").Get("1")
 
 	if err != nil {
-		t.Errorf("%#v should be nil.", err)
+		t.Fatalf("%v should be nil.", err)
 	}
 
 	if e, r := event.InvolvedObject, response.InvolvedObject; !reflect.DeepEqual(e, r) {

pkg/client/request.go

@@ -181,6 +181,14 @@ func (r *Request) Namespace(namespace string) *Request {
 	return r
 }
 
+// NamespaceIfScoped is a convenience function to set a namespace if scoped is true
+func (r *Request) NamespaceIfScoped(namespace string, scoped bool) *Request {
+	if scoped {
+		return r.Namespace(namespace)
+	}
+	return r
+}
+
 // AbsPath overwrites an existing path with the segments provided. Trailing slashes are preserved
 // when a single segment is passed.
 func (r *Request) AbsPath(segments ...string) *Request {
@@ -320,7 +328,7 @@ func (r *Request) finalURL() string {
 		query.Add(key, value)
 	}
 
-	if r.namespaceSet && r.namespaceInQuery && len(r.namespace) > 0 {
+	if r.namespaceSet && r.namespaceInQuery {
 		query.Add("namespace", r.namespace)
 	}
 
@@ -427,6 +435,14 @@ func (r *Request) Do() Result {
 			return Result{err: &RequestConstructionError{r.err}}
 		}
 
+		// TODO: added to catch programmer errors (invoking operations with an object with an empty namespace)
+		if (r.verb == "GET" || r.verb == "PUT" || r.verb == "DELETE") && r.namespaceSet && len(r.resourceName) > 0 && len(r.namespace) == 0 {
+			return Result{err: &RequestConstructionError{fmt.Errorf("an empty namespace may not be set when a resource name is provided")}}
+		}
+		if (r.verb == "POST") && r.namespaceSet && len(r.namespace) == 0 {
+			return Result{err: &RequestConstructionError{fmt.Errorf("an empty namespace may not be set during creation")}}
+		}
+
 		req, err := http.NewRequest(r.verb, r.finalURL(), r.body)
 		if err != nil {
 			return Result{err: &RequestConstructionError{err}}

pkg/config/config_test.go

@@ -56,17 +56,15 @@ func TestCreateObjects(t *testing.T) {
 	items := []runtime.Object{}
 
 	items = append(items, &api.Pod{
-		TypeMeta:   api.TypeMeta{APIVersion: "v1beta1", Kind: "Pod"},
+		ObjectMeta: api.ObjectMeta{Name: "test-pod", Namespace: "default"},
-		ObjectMeta: api.ObjectMeta{Name: "test-pod"},
 	})
 
 	items = append(items, &api.Service{
-		TypeMeta:   api.TypeMeta{APIVersion: "v1beta1", Kind: "Service"},
+		ObjectMeta: api.ObjectMeta{Name: "test-service", Namespace: "default"},
-		ObjectMeta: api.ObjectMeta{Name: "test-service"},
 	})
 
 	typer, mapper := getTyperAndMapper()
-	client, s := getFakeClient(t, []string{"/api/v1beta1/pods", "/api/v1beta1/services"})
+	client, s := getFakeClient(t, []string{"/api/v1beta1/pods?namespace=default", "/api/v1beta1/services?namespace=default"})
 
 	errs := CreateObjects(typer, mapper, client, items)
 	s.Close()

pkg/kubectl/resource/helper.go

@@ -35,6 +35,8 @@ type Helper struct {
 	// An interface for reading or writing the resource version of this
 	// type.
 	Versioner runtime.ResourceVersioner
+	// True if the resource type is scoped to namespaces
+	NamespaceScoped bool
 }
 
 // NewHelper creates a Helper from a ResourceMapping
@@ -44,12 +46,14 @@ func NewHelper(client RESTClient, mapping *meta.RESTMapping) *Helper {
 		Resource:   mapping.Resource,
 		Codec:      mapping.Codec,
 		Versioner:  mapping.MetadataAccessor,
+
+		NamespaceScoped: mapping.Scope.Name() == meta.RESTScopeNameNamespace,
 	}
 }
 
 func (m *Helper) Get(namespace, name string) (runtime.Object, error) {
 	return m.RESTClient.Get().
-		Namespace(namespace).
+		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		Name(name).
 		Do().
@@ -58,7 +62,7 @@ func (m *Helper) Get(namespace, name string) (runtime.Object, error) {
 
 func (m *Helper) List(namespace string, selector labels.Selector) (runtime.Object, error) {
 	return m.RESTClient.Get().
-		Namespace(namespace).
+		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		SelectorParam("labels", selector).
 		Do().
@@ -68,7 +72,7 @@ func (m *Helper) List(namespace string, selector labels.Selector) (runtime.Objec
 func (m *Helper) Watch(namespace, resourceVersion string, labelSelector, fieldSelector labels.Selector) (watch.Interface, error) {
 	return m.RESTClient.Get().
 		Prefix("watch").
-		Namespace(namespace).
+		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		Param("resourceVersion", resourceVersion).
 		SelectorParam("labels", labelSelector).
@@ -79,7 +83,7 @@ func (m *Helper) Watch(namespace, resourceVersion string, labelSelector, fieldSe
 func (m *Helper) WatchSingle(namespace, name, resourceVersion string) (watch.Interface, error) {
 	return m.RESTClient.Get().
 		Prefix("watch").
-		Namespace(namespace).
+		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		Name(name).
 		Param("resourceVersion", resourceVersion).
@@ -88,7 +92,7 @@ func (m *Helper) WatchSingle(namespace, name, resourceVersion string) (watch.Int
 
 func (m *Helper) Delete(namespace, name string) error {
 	return m.RESTClient.Delete().
-		Namespace(namespace).
+		NamespaceIfScoped(namespace, m.NamespaceScoped).
 		Resource(m.Resource).
 		Name(name).
 		Do().
@@ -100,14 +104,14 @@ func (m *Helper) Create(namespace string, modify bool, data []byte) (runtime.Obj
 		obj, err := m.Codec.Decode(data)
 		if err != nil {
 			// We don't know how to check a version on this object, but create it anyway
-			return createResource(m.RESTClient, m.Resource, namespace, data)
+			return m.createResource(m.RESTClient, m.Resource, namespace, data)
 		}
 
 		// Attempt to version the object based on client logic.
 		version, err := m.Versioner.ResourceVersion(obj)
 		if err != nil {
 			// We don't know how to clear the version on this object, so send it to the server as is
-			return createResource(m.RESTClient, m.Resource, namespace, data)
+			return m.createResource(m.RESTClient, m.Resource, namespace, data)
 		}
 		if version != "" {
 			if err := m.Versioner.SetResourceVersion(obj, ""); err != nil {
@@ -121,11 +125,11 @@ func (m *Helper) Create(namespace string, modify bool, data []byte) (runtime.Obj
 		}
 	}
 
-	return createResource(m.RESTClient, m.Resource, namespace, data)
+	return m.createResource(m.RESTClient, m.Resource, namespace, data)
 }
 
-func createResource(c RESTClient, resource, namespace string, data []byte) (runtime.Object, error) {
+func (m *Helper) createResource(c RESTClient, resource, namespace string, data []byte) (runtime.Object, error) {
-	return c.Post().Namespace(namespace).Resource(resource).Body(data).Do().Get()
+	return c.Post().NamespaceIfScoped(namespace, m.NamespaceScoped).Resource(resource).Body(data).Do().Get()
 }
 
 func (m *Helper) Update(namespace, name string, overwrite bool, data []byte) (runtime.Object, error) {
@@ -134,21 +138,21 @@ func (m *Helper) Update(namespace, name string, overwrite bool, data []byte) (ru
 	obj, err := m.Codec.Decode(data)
 	if err != nil {
 		// We don't know how to handle this object, but update it anyway
-		return updateResource(c, m.Resource, namespace, name, data)
+		return m.updateResource(c, m.Resource, namespace, name, data)
 	}
 
 	// Attempt to version the object based on client logic.
 	version, err := m.Versioner.ResourceVersion(obj)
 	if err != nil {
 		// We don't know how to version this object, so send it to the server as is
-		return updateResource(c, m.Resource, namespace, name, data)
+		return m.updateResource(c, m.Resource, namespace, name, data)
 	}
 	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()
 		if err != nil {
 			// The object does not exist, but we want it to be created
-			return updateResource(c, m.Resource, namespace, name, data)
+			return m.updateResource(c, m.Resource, namespace, name, data)
 		}
 		serverVersion, err := m.Versioner.ResourceVersion(serverObj)
 		if err != nil {
@@ -164,9 +168,9 @@ func (m *Helper) Update(namespace, name string, overwrite bool, data []byte) (ru
 		data = newData
 	}
 
-	return updateResource(c, m.Resource, namespace, name, data)
+	return m.updateResource(c, m.Resource, namespace, name, data)
 }
 
-func updateResource(c RESTClient, resource, namespace, name string, data []byte) (runtime.Object, error) {
+func (m *Helper) updateResource(c RESTClient, resource, namespace, name string, data []byte) (runtime.Object, error) {
-	return c.Put().Namespace(namespace).Resource(resource).Name(name).Body(data).Do().Get()
+	return c.Put().NamespaceIfScoped(namespace, m.NamespaceScoped).Resource(resource).Name(name).Body(data).Do().Get()
 }

pkg/kubectl/resource/helper_test.go

@@ -75,6 +75,10 @@ func TestHelperDelete(t *testing.T) {
 					return false
 				}
 				parts := splitPath(req.URL.Path)
+				if len(parts) < 3 {
+					t.Errorf("expected URL path to have 3 parts: %s", req.URL.Path)
+					return false
+				}
 				if parts[1] != "bar" {
 					t.Errorf("url doesn't contain namespace: %#v", req)
 					return false
@@ -95,6 +99,7 @@ func TestHelperDelete(t *testing.T) {
 		}
 		modifier := &Helper{
 			RESTClient:      client,
+			NamespaceScoped: true,
 		}
 		err := modifier.Delete("bar", "foo")
 		if (err != nil) != test.Err {
@@ -188,6 +193,7 @@ func TestHelperCreate(t *testing.T) {
 			RESTClient:      client,
 			Codec:           testapi.Codec(),
 			Versioner:       testapi.MetadataAccessor(),
+			NamespaceScoped: true,
 		}
 		data := []byte{}
 		if test.Object != nil {
@@ -268,6 +274,7 @@ func TestHelperGet(t *testing.T) {
 		}
 		modifier := &Helper{
 			RESTClient:      client,
+			NamespaceScoped: true,
 		}
 		obj, err := modifier.Get("bar", "foo")
 		if (err != nil) != test.Err {
@@ -338,6 +345,7 @@ func TestHelperList(t *testing.T) {
 		}
 		modifier := &Helper{
 			RESTClient:      client,
+			NamespaceScoped: true,
 		}
 		obj, err := modifier.List("bar", labels.SelectorFromSet(labels.Set{"foo": "baz"}))
 		if (err != nil) != test.Err {
@@ -443,6 +451,7 @@ func TestHelperUpdate(t *testing.T) {
 			RESTClient:      client,
 			Codec:           testapi.Codec(),
 			Versioner:       testapi.MetadataAccessor(),
+			NamespaceScoped: true,
 		}
 		data := []byte{}
 		if test.Object != nil {

pkg/service/endpoints_controller_test.go

@@ -260,7 +260,7 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAll(t *testing.T) {
 	serviceList := api.ServiceList{
 		Items: []api.Service{
 			{
-				ObjectMeta: api.ObjectMeta{Name: "foo"},
+				ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"},
 				Spec: api.ServiceSpec{
 					Selector: map[string]string{},
 				},
@@ -290,14 +290,14 @@ func TestSyncEndpointsItemsEmptySelectorSelectsAll(t *testing.T) {
 		},
 		Endpoints: []string{"1.2.3.4:8080"},
 	})
-	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo", "PUT", &data)
+	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo?namespace=other", "PUT", &data)
 }
 
 func TestSyncEndpointsItemsPreexisting(t *testing.T) {
 	serviceList := api.ServiceList{
 		Items: []api.Service{
 			{
-				ObjectMeta: api.ObjectMeta{Name: "foo"},
+				ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "bar"},
 				Spec: api.ServiceSpec{
 					Selector: map[string]string{
 						"foo": "bar",
@@ -329,14 +329,14 @@ func TestSyncEndpointsItemsPreexisting(t *testing.T) {
 		},
 		Endpoints: []string{"1.2.3.4:8080"},
 	})
-	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo", "PUT", &data)
+	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo?namespace=bar", "PUT", &data)
 }
 
 func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) {
 	serviceList := api.ServiceList{
 		Items: []api.Service{
 			{
-				ObjectMeta: api.ObjectMeta{Name: "foo"},
+				ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: api.NamespaceDefault},
 				Spec: api.ServiceSpec{
 					Selector: map[string]string{
 						"foo": "bar",
@@ -360,14 +360,14 @@ func TestSyncEndpointsItemsPreexistingIdentical(t *testing.T) {
 	if err := endpoints.SyncServiceEndpoints(); err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
-	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo", "GET", nil)
+	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints/foo?namespace=default", "GET", nil)
 }
 
 func TestSyncEndpointsItems(t *testing.T) {
 	serviceList := api.ServiceList{
 		Items: []api.Service{
 			{
-				ObjectMeta: api.ObjectMeta{Name: "foo"},
+				ObjectMeta: api.ObjectMeta{Name: "foo", Namespace: "other"},
 				Spec: api.ServiceSpec{
 					Selector: map[string]string{
 						"foo": "bar",
@@ -392,7 +392,7 @@ func TestSyncEndpointsItems(t *testing.T) {
 		},
 		Endpoints: []string{"1.2.3.4:8080"},
 	})
-	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints", "POST", &data)
+	endpointsHandler.ValidateRequest(t, "/api/"+testapi.Version()+"/endpoints?namespace=other", "POST", &data)
 }
 
 func TestSyncEndpointsPodError(t *testing.T) {

plugin/pkg/scheduler/factory/factory_test.go

@@ -277,7 +277,13 @@ func TestBind(t *testing.T) {
 	table := []struct {
 		binding *api.Binding
 	}{
-		{binding: &api.Binding{PodID: "foo", Host: "foohost.kubernetes.mydomain.com"}},
+		{binding: &api.Binding{
+			ObjectMeta: api.ObjectMeta{
+				Namespace: api.NamespaceDefault,
+			},
+			PodID: "foo",
+			Host:  "foohost.kubernetes.mydomain.com",
+		}},
 	}
 
 	for _, item := range table {
@@ -296,7 +302,7 @@ func TestBind(t *testing.T) {
 			continue
 		}
 		expectedBody := runtime.EncodeOrDie(testapi.Codec(), item.binding)
-		handler.ValidateRequest(t, "/api/"+testapi.Version()+"/bindings", "POST", &expectedBody)
+		handler.ValidateRequest(t, "/api/"+testapi.Version()+"/bindings?namespace=default", "POST", &expectedBody)
 	}
 }