Prevent accidental setting of sync or timeout

pkg/apiserver/watch_test.go

@@ -146,7 +146,7 @@ func TestWatchParamParsing(t *testing.T) {
 	simpleStorage := &SimpleRESTStorage{}
 	handler := New(map[string]RESTStorage{
 		"foo": simpleStorage,
-	}, "/prefix/version")
+	}, codec, "/prefix/version")
 	server := httptest.NewServer(handler)
 
 	dest, _ := url.Parse(server.URL)

pkg/client/request.go

@@ -30,10 +30,15 @@ import (
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/api"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/labels"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/tools"
+	"github.com/GoogleCloudPlatform/kubernetes/pkg/util"
 	"github.com/GoogleCloudPlatform/kubernetes/pkg/watch"
 	"github.com/golang/glog"
 )
 
+// specialParams lists parameters that are handled specially and which users of Request
+// are therefore not allowed to set manually.
+var specialParams = util.NewStringSet("sync", "timeout")
+
 // Verb begins a request with a verb (GET, POST, PUT, DELETE)
 //
 // Example usage of Client's request building interface:
@@ -134,12 +139,12 @@ func (r *Request) ParseSelectorParam(paramName, item string) *Request {
 	if r.err != nil {
 		return r
 	}
-	if sel, err := labels.ParseSelector(item); err != nil {
+	sel, err := labels.ParseSelector(item)
+	if err != nil {
 		r.err = err
-	} else {
+		return r
-		r.params[paramName] = sel.String()
 	}
-	return r
+	return r.setParam(paramName, sel.String())
 }
 
 // SelectorParam adds the given selector as a query parameter with the name paramName.
@@ -147,8 +152,7 @@ func (r *Request) SelectorParam(paramName string, s labels.Selector) *Request {
 	if r.err != nil {
 		return r
 	}
-	r.params[paramName] = s.String()
+	return r.setParam(paramName, s.String())
-	return r
 }
 
 // UintParam creates a query parameter with the given value.
@@ -156,7 +160,15 @@ func (r *Request) UintParam(paramName string, u uint64) *Request {
 	if r.err != nil {
 		return r
 	}
-	r.params[paramName] = strconv.FormatUint(u, 10)
+	return r.setParam(paramName, strconv.FormatUint(u, 10))
+}
+
+func (r *Request) setParam(paramName, value string) *Request {
+	if specialParams.Has(paramName) {
+		r.err = fmt.Errorf("must set %v through the corresponding function, not directly.", paramName)
+		return r
+	}
+	r.params[paramName] = value
 	return r
 }
 
@@ -181,21 +193,23 @@ func (r *Request) Body(obj interface{}) *Request {
 	}
 	switch t := obj.(type) {
 	case string:
-		if data, err := ioutil.ReadFile(t); err != nil {
+		data, err := ioutil.ReadFile(t)
+		if err != nil {
 			r.err = err
-		} else {
+			return r
-			r.body = bytes.NewBuffer(data)
 		}
+		r.body = bytes.NewBuffer(data)
 	case []byte:
 		r.body = bytes.NewBuffer(t)
 	case io.Reader:
 		r.body = t
 	default:
-		if data, err := api.Encode(obj); err != nil {
+		data, err := api.Encode(obj)
+		if err != nil {
 			r.err = err
-		} else {
+			return r
-			r.body = bytes.NewBuffer(data)
 		}
+		r.body = bytes.NewBuffer(data)
 	}
 	return r
 }

pkg/client/request_test.go

@@ -264,6 +264,25 @@ func TestUintParam(t *testing.T) {
 	}
 }
 
+func TestUnacceptableParamNames(t *testing.T) {
+	table := []struct {
+		name          string
+		testVal       string
+		expectSuccess bool
+	}{
+		{"sync", "foo", false},
+		{"timeout", "42", false},
+	}
+
+	for _, item := range table {
+		c := New("", nil)
+		r := c.Get().setParam(item.name, item.testVal)
+		if e, a := item.expectSuccess, r.err == nil; e != a {
+			t.Errorf("expected %v, got %v (%v)", e, a, r.err)
+		}
+	}
+}
+
 func TestSetPollPeriod(t *testing.T) {
 	c := New("", nil)
 	r := c.Get()

pkg/controller/replication_controller.go

@@ -87,16 +87,16 @@ func MakeReplicationManager(kubeClient client.Interface) *ReplicationManager {
 // Run begins watching and syncing.
 func (rm *ReplicationManager) Run(period time.Duration) {
 	rm.syncTime = time.Tick(period)
-	index := uint64(0)
+	resourceVersion := uint64(0)
-	go util.Forever(func() { rm.watchControllers(&index) }, period)
+	go util.Forever(func() { rm.watchControllers(&resourceVersion) }, period)
 }
 
-// index is a pointer to the resource version to use/update.
+// resourceVersion is a pointer to the resource version to use/update.
-func (rm *ReplicationManager) watchControllers(index *uint64) {
+func (rm *ReplicationManager) watchControllers(resourceVersion *uint64) {
 	watching, err := rm.kubeClient.WatchReplicationControllers(
 		labels.Everything(),
 		labels.Everything(),
-		*index,
+		*resourceVersion,
 	)
 	if err != nil {
 		glog.Errorf("Unexpected failure to watch: %v", err)
@@ -120,7 +120,7 @@ func (rm *ReplicationManager) watchControllers(index *uint64) {
 				glog.Errorf("unexpected object: %#v", event.Object)
 			} else {
 				// If we get disconnected, start where we left off.
-				*index = rc.ResourceVersion + 1
+				*resourceVersion = rc.ResourceVersion + 1
 				rm.syncHandler(*rc)
 			}
 		}

pkg/controller/replication_controller_test.go

@@ -339,8 +339,8 @@ func TestWatchControllers(t *testing.T) {
 		return nil
 	}
 
-	index := uint64(0)
+	resourceVersion := uint64(0)
-	go manager.watchControllers(&index)
+	go manager.watchControllers(&resourceVersion)
 
 	// Test normal case
 	testControllerSpec.ID = "foo"