Merge pull request #2864 from deads2k/deads-tighten-validation-on-bad-gets

tighten validation for client resource gets

pkg/client/client_test.go

@@ -38,6 +38,7 @@ import (
 
 // TODO: Move this to a common place, it's needed in multiple tests.
 const apiPath = "/api/v1beta1"
+const nameRequiredError = "name is required parameter to Get"
 
 type testRequest struct {
 	Method  string
@@ -255,6 +256,17 @@ func TestGetPod(t *testing.T) {
 	c.Validate(t, receivedPod, err)
 }
 
+func TestGetPodWithNoName(t *testing.T) {
+	ns := api.NamespaceDefault
+	c := &testClient{Error: true}
+	receivedPod, err := c.Setup().Pods(ns).Get("")
+	if (err != nil) && (err.Error() != nameRequiredError) {
+		t.Errorf("Expected error: %v, but got %v", nameRequiredError, err)
+	}
+
+	c.Validate(t, receivedPod, err)
+}
+
 func TestDeletePod(t *testing.T) {
 	c := &testClient{
 		Request:  testRequest{Method: "DELETE", Path: "/pods/foo"},
@@ -361,6 +373,17 @@ func TestGetController(t *testing.T) {
 	c.Validate(t, receivedController, err)
 }
 
+func TestGetControllerWithNoName(t *testing.T) {
+	ns := api.NamespaceDefault
+	c := &testClient{Error: true}
+	receivedPod, err := c.Setup().ReplicationControllers(ns).Get("")
+	if (err != nil) && (err.Error() != nameRequiredError) {
+		t.Errorf("Expected error: %v, but got %v", nameRequiredError, err)
+	}
+
+	c.Validate(t, receivedPod, err)
+}
+
 func TestUpdateController(t *testing.T) {
 	requestController := &api.ReplicationController{
 		ObjectMeta: api.ObjectMeta{Name: "foo", ResourceVersion: "1"},
@@ -502,6 +525,17 @@ func TestGetService(t *testing.T) {
 	c.Validate(t, response, err)
 }
 
+func TestGetServiceWithNoName(t *testing.T) {
+	ns := api.NamespaceDefault
+	c := &testClient{Error: true}
+	receivedPod, err := c.Setup().Services(ns).Get("")
+	if (err != nil) && (err.Error() != nameRequiredError) {
+		t.Errorf("Expected error: %v, but got %v", nameRequiredError, err)
+	}
+
+	c.Validate(t, receivedPod, err)
+}
+
 func TestCreateService(t *testing.T) {
 	c := &testClient{
 		Request:  testRequest{Method: "POST", Path: "/services", Body: &api.Service{ObjectMeta: api.ObjectMeta{Name: "service-1"}}},
@@ -557,6 +591,17 @@ func TestGetEndpoints(t *testing.T) {
 	c.Validate(t, response, err)
 }
 
+func TestGetEndpointWithNoName(t *testing.T) {
+	ns := api.NamespaceDefault
+	c := &testClient{Error: true}
+	receivedPod, err := c.Setup().Endpoints(ns).Get("")
+	if (err != nil) && (err.Error() != nameRequiredError) {
+		t.Errorf("Expected error: %v, but got %v", nameRequiredError, err)
+	}
+
+	c.Validate(t, receivedPod, err)
+}
+
 func TestGetServerVersion(t *testing.T) {
 	expect := version.Info{
 		Major:     "foo",
@@ -625,6 +670,16 @@ func TestGetMinion(t *testing.T) {
 	c.Validate(t, response, err)
 }
 
+func TestGetMinionWithNoName(t *testing.T) {
+	c := &testClient{Error: true}
+	receivedPod, err := c.Setup().Nodes().Get("")
+	if (err != nil) && (err.Error() != nameRequiredError) {
+		t.Errorf("Expected error: %v, but got %v", nameRequiredError, err)
+	}
+
+	c.Validate(t, receivedPod, err)
+}
+
 func TestCreateMinion(t *testing.T) {
 	requestMinion := &api.Node{
 		ObjectMeta: api.ObjectMeta{

pkg/client/endpoints.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -33,7 +34,7 @@ type EndpointsNamespacer interface {
 type EndpointsInterface interface {
 	Create(endpoints *api.Endpoints) (*api.Endpoints, error)
 	List(selector labels.Selector) (*api.EndpointsList, error)
-	Get(id string) (*api.Endpoints, error)
+	Get(name string) (*api.Endpoints, error)
 	Update(endpoints *api.Endpoints) (*api.Endpoints, error)
 	Watch(label, field labels.Selector, resourceVersion string) (watch.Interface, error)
 }
@@ -64,9 +65,13 @@ func (c *endpoints) List(selector labels.Selector) (result *api.EndpointsList, e
 }
 
 // Get returns information about the endpoints for a particular service.
-func (c *endpoints) Get(id string) (result *api.Endpoints, err error) {
+func (c *endpoints) Get(name string) (result *api.Endpoints, err error) {
+	if len(name) == 0 {
+		return nil, errors.New("name is required parameter to Get")
+	}
+
 	result = &api.Endpoints{}
-	err = c.r.Get().Namespace(c.ns).Path("endpoints").Path(id).Do().Into(result)
+	err = c.r.Get().Namespace(c.ns).Path("endpoints").Path(name).Do().Into(result)
 	return
 }
 

pkg/client/events.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -34,7 +35,7 @@ type EventNamespacer interface {
 type EventInterface interface {
 	Create(event *api.Event) (*api.Event, error)
 	List(label, field labels.Selector) (*api.EventList, error)
-	Get(id string) (*api.Event, error)
+	Get(name string) (*api.Event, error)
 	Watch(label, field labels.Selector, resourceVersion string) (watch.Interface, error)
 	// Search finds events about the specified object
 	Search(objOrRef runtime.Object) (*api.EventList, error)
@@ -86,11 +87,15 @@ func (e *events) List(label, field labels.Selector) (*api.EventList, error) {
 }
 
 // Get returns the given event, or an error.
-func (e *events) Get(id string) (*api.Event, error) {
+func (e *events) Get(name string) (*api.Event, error) {
+	if len(name) == 0 {
+		return nil, errors.New("name is required parameter to Get")
+	}
+
 	result := &api.Event{}
 	err := e.client.Get().
 		Path("events").
-		Path(id).
+		Path(name).
 		Namespace(e.namespace).
 		Do().
 		Into(result)

pkg/client/minions.go

@@ -16,17 +16,21 @@ limitations under the License.
 
 package client
 
-import "github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+import (
+	"errors"
+
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
+)
 
 type NodesInterface interface {
 	Nodes() NodeInterface
 }
 
 type NodeInterface interface {
-	Get(id string) (result *api.Node, err error)
+	Get(name string) (result *api.Node, err error)
 	Create(minion *api.Node) (*api.Node, error)
 	List() (*api.NodeList, error)
-	Delete(id string) error
+	Delete(name string) error
 }
 
 // nodes implements NodesInterface
@@ -63,13 +67,17 @@ func (c *nodes) List() (*api.NodeList, error) {
 }
 
 // Get gets an existing minion
-func (c *nodes) Get(id string) (*api.Node, error) {
+func (c *nodes) Get(name string) (*api.Node, error) {
+	if len(name) == 0 {
+		return nil, errors.New("name is required parameter to Get")
+	}
+
 	result := &api.Node{}
-	err := c.r.Get().Path(c.resourceName()).Path(id).Do().Into(result)
+	err := c.r.Get().Path(c.resourceName()).Path(name).Do().Into(result)
 	return result, err
 }
 
 // Delete deletes an existing minion.
-func (c *nodes) Delete(id string) error {
+func (c *nodes) Delete(name string) error {
-	return c.r.Delete().Path(c.resourceName()).Path(id).Do().Error()
+	return c.r.Delete().Path(c.resourceName()).Path(name).Do().Error()
 }

pkg/client/pods.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -60,6 +61,10 @@ func (c *pods) List(selector labels.Selector) (result *api.PodList, err error) {
 
 // GetPod takes the name of the pod, and returns the corresponding Pod object, and an error if it occurs
 func (c *pods) Get(name string) (result *api.Pod, err error) {
+	if len(name) == 0 {
+		return nil, errors.New("name is required parameter to Get")
+	}
+
 	result = &api.Pod{}
 	err = c.r.Get().Namespace(c.ns).Path("pods").Path(name).Do().Into(result)
 	return

pkg/client/replication_controllers.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -59,6 +60,10 @@ func (c *replicationControllers) List(selector labels.Selector) (result *api.Rep
 
 // Get returns information about a particular replication controller.
 func (c *replicationControllers) Get(name string) (result *api.ReplicationController, err error) {
+	if len(name) == 0 {
+		return nil, errors.New("name is required parameter to Get")
+	}
+
 	result = &api.ReplicationController{}
 	err = c.r.Get().Namespace(c.ns).Path("replicationControllers").Path(name).Do().Into(result)
 	return

pkg/client/services.go

@@ -17,6 +17,7 @@ limitations under the License.
 package client
 
 import (
+	"errors"
 	"fmt"
 
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
@@ -59,6 +60,10 @@ func (c *services) List(selector labels.Selector) (result *api.ServiceList, err
 
 // Get returns information about a particular service.
 func (c *services) Get(name string) (result *api.Service, err error) {
+	if len(name) == 0 {
+		return nil, errors.New("name is required parameter to Get")
+	}
+
 	result = &api.Service{}
 	err = c.r.Get().Namespace(c.ns).Path("services").Path(name).Do().Into(result)
 	return