diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go index 619e3ecf4b6d7..06ddad6d843cd 100644 --- a/cmd/integration/integration.go +++ b/cmd/integration/integration.go @@ -465,6 +465,81 @@ func runAtomicPutTest(c *client.Client) { glog.Info("Atomic PUTs work.") } +func runPatchTest(c *client.Client) { + name := "patchservice" + resource := "services" + var svc api.Service + err := c.Post().Resource(resource).Body( + &api.Service{ + TypeMeta: api.TypeMeta{ + APIVersion: latest.Version, + }, + ObjectMeta: api.ObjectMeta{ + Name: name, + Labels: map[string]string{ + "name": name, + }, + }, + Spec: api.ServiceSpec{ + Port: 12345, + // This is here because validation requires it. + Selector: map[string]string{ + "foo": "bar", + }, + Protocol: "TCP", + SessionAffinity: "None", + }, + }, + ).Do().Into(&svc) + if err != nil { + glog.Fatalf("Failed creating patchservice: %v", err) + } + if len(svc.Labels) != 1 { + glog.Fatalf("Original length does not equal one") + } + + // add label + _, err = c.Patch().Resource(resource).Name(name).Body([]byte("{\"labels\":{\"foo\":\"bar\"}}")).Do().Get() + if err != nil { + glog.Fatalf("Failed updating patchservice: %v", err) + } + err = c.Get().Resource(resource).Name(name).Do().Into(&svc) + if err != nil { + glog.Fatalf("Failed getting patchservice: %v", err) + } + if len(svc.Labels) != 2 || svc.Labels["foo"] != "bar" { + glog.Fatalf("Failed updating patchservice, labels are: %v", svc.Labels) + } + + // remove one label + _, err = c.Patch().Resource(resource).Name(name).Body([]byte("{\"labels\":{\"name\":null}}")).Do().Get() + if err != nil { + glog.Fatalf("Failed updating patchservice: %v", err) + } + err = c.Get().Resource(resource).Name(name).Do().Into(&svc) + if err != nil { + glog.Fatalf("Failed getting patchservice: %v", err) + } + if len(svc.Labels) != 1 || svc.Labels["foo"] != "bar" { + glog.Fatalf("Failed updating patchservice, labels are: %v", svc.Labels) + } + + // remove all labels + _, err = c.Patch().Resource(resource).Name(name).Body([]byte("{\"labels\":null}")).Do().Get() + if err != nil { + glog.Fatalf("Failed updating patchservice: %v", err) + } + err = c.Get().Resource(resource).Name(name).Do().Into(&svc) + if err != nil { + glog.Fatalf("Failed getting patchservice: %v", err) + } + if svc.Labels != nil { + glog.Fatalf("Failed remove all labels from patchservice: %v", svc.Labels) + } + + glog.Info("PATCHs work.") +} + func runMasterServiceTest(client *client.Client) { time.Sleep(12 * time.Second) var svcList api.ServiceList @@ -665,6 +740,7 @@ func main() { testFuncs := []testFunc{ runReplicationControllerTest, runAtomicPutTest, + runPatchTest, runServiceTest, runAPIVersionsTest, runMasterServiceTest, diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index c2e15b95043f3..2da563d95ff8c 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -132,6 +132,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage getter, isGetter := storage.(RESTGetter) deleter, isDeleter := storage.(RESTDeleter) updater, isUpdater := storage.(RESTUpdater) + patcher, isPatcher := storage.(RESTPatcher) _, isWatcher := storage.(ResourceWatcher) _, isRedirector := storage.(Redirector) @@ -167,6 +168,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter) actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater) + actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher) actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter) actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) actions = appendIf(actions, action{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) @@ -196,6 +198,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter) actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater) + actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher) actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter) actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) actions = appendIf(actions, action{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) @@ -229,6 +232,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage actions = appendIf(actions, action{"GET", itemPath, nameParams, namer}, isGetter) actions = appendIf(actions, action{"PUT", itemPath, nameParams, namer}, isUpdater) + actions = appendIf(actions, action{"PATCH", itemPath, nameParams, namer}, isPatcher) actions = appendIf(actions, action{"DELETE", itemPath, nameParams, namer}, isDeleter) actions = appendIf(actions, action{"WATCH", "watch/" + itemPath, nameParams, namer}, isWatcher) actions = appendIf(actions, action{"REDIRECT", "redirect/" + itemPath, nameParams, namer}, isRedirector) @@ -281,6 +285,16 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage Reads(versionedObject) addParams(route, action.Params) ws.Route(route) + case "PATCH": // Partially update a resource + route := ws.PATCH(action.Path).To(PatchResource(patcher, ctxFn, action.Namer, mapping.Codec, a.group.Typer, resource, admit)). + Filter(m). + Doc("partially update the specified " + kind). + // TODO: toggle patch strategy by content type + // Consumes("application/merge-patch+json", "application/json-patch+json"). + Operation("patch" + kind). + Reads(versionedObject) + addParams(route, action.Params) + ws.Route(route) case "POST": // Create a resource. route := ws.POST(action.Path).To(CreateResource(creater, ctxFn, action.Namer, mapping.Codec, a.group.Typer, resource, admit)). Filter(m). diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 07d50e3c8b39c..b486c836071fc 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -173,7 +173,8 @@ func handleInternal(storage map[string]RESTStorage, admissionControl admission.I type Simple struct { api.TypeMeta `json:",inline"` api.ObjectMeta `json:"metadata"` - Other string `json:"other,omitempty"` + Other string `json:"other,omitempty"` + Labels map[string]string `json:"labels,omitempty"` } func (*Simple) IsAnAPIObject() {} @@ -826,6 +827,70 @@ func TestDeleteMissing(t *testing.T) { } } +func TestPatch(t *testing.T) { + storage := map[string]RESTStorage{} + ID := "id" + item := &Simple{ + ObjectMeta: api.ObjectMeta{ + Name: ID, + Namespace: "", // update should allow the client to send an empty namespace + }, + Other: "bar", + } + simpleStorage := SimpleRESTStorage{item: *item} + storage["simple"] = &simpleStorage + selfLinker := &setTestSelfLinker{ + t: t, + expectedSet: "/api/version/simple/" + ID + "?namespace=default", + name: ID, + namespace: api.NamespaceDefault, + } + handler := handleLinker(storage, selfLinker) + server := httptest.NewServer(handler) + defer server.Close() + + client := http.Client{} + request, err := http.NewRequest("PATCH", server.URL+"/api/version/simple/"+ID, bytes.NewReader([]byte(`{"labels":{"foo":"bar"}}`))) + _, err = client.Do(request) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if simpleStorage.updated == nil || simpleStorage.updated.Labels["foo"] != "bar" { + t.Errorf("Unexpected update value %#v, expected %#v.", simpleStorage.updated, item) + } + if !selfLinker.called { + t.Errorf("Never set self link") + } +} + +func TestPatchRequiresMatchingName(t *testing.T) { + storage := map[string]RESTStorage{} + ID := "id" + item := &Simple{ + ObjectMeta: api.ObjectMeta{ + Name: ID, + Namespace: "", // update should allow the client to send an empty namespace + }, + Other: "bar", + } + simpleStorage := SimpleRESTStorage{item: *item} + storage["simple"] = &simpleStorage + handler := handle(storage) + server := httptest.NewServer(handler) + defer server.Close() + + client := http.Client{} + request, err := http.NewRequest("PATCH", server.URL+"/api/version/simple/"+ID, bytes.NewReader([]byte(`{"metadata":{"name":"idbar"}}`))) + 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) + } +} + func TestUpdate(t *testing.T) { storage := map[string]RESTStorage{} simpleStorage := SimpleRESTStorage{} diff --git a/pkg/apiserver/interfaces.go b/pkg/apiserver/interfaces.go index 233bd12236ed9..a216e04778a20 100644 --- a/pkg/apiserver/interfaces.go +++ b/pkg/apiserver/interfaces.go @@ -77,6 +77,11 @@ type RESTUpdater interface { Update(ctx api.Context, obj runtime.Object) (runtime.Object, bool, error) } +type RESTPatcher interface { + RESTGetter + RESTUpdater +} + // RESTResult indicates the result of a REST transformation. type RESTResult struct { // The result of this operation. May be nil if the operation has no meaningful diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index de4dff38c53c9..0c799a9cb2dc2 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -30,6 +30,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/emicklei/go-restful" + "github.com/evanphx/json-patch" "github.com/golang/glog" ) @@ -187,6 +188,83 @@ func CreateResource(r RESTCreater, ctxFn ContextFunc, namer ScopeNamer, codec ru } } +// PatchResource returns a function that will handle a resource patch +// TODO: Eventually PatchResource should just use AtomicUpdate and this routine should be a bit cleaner +func PatchResource(r RESTPatcher, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, typer runtime.ObjectTyper, resource string, admit admission.Interface) restful.RouteFunction { + return func(req *restful.Request, res *restful.Response) { + w := res.ResponseWriter + glog.Infof("hi") + + // TODO: we either want to remove timeout or document it (if we document, move timeout out of this function and declare it in api_installer) + timeout := parseTimeout(req.Request.URL.Query().Get("timeout")) + + namespace, name, err := namer.Name(req) + if err != nil { + notFound(w, req.Request) + return + } + + obj := r.New() + // PATCH requires same permission as UPDATE + err = admit.Admit(admission.NewAttributesRecord(obj, namespace, resource, "UPDATE")) + if err != nil { + errorJSON(err, codec, w) + return + } + + ctx := ctxFn(req) + ctx = api.WithNamespace(ctx, namespace) + + original, err := r.Get(ctx, name) + if err != nil { + errorJSON(err, codec, w) + return + } + + originalObjJs, err := codec.Encode(original) + if err != nil { + errorJSON(err, codec, w) + return + } + patchJs, err := readBody(req.Request) + if err != nil { + errorJSON(err, codec, w) + return + } + patchedObjJs, err := jsonpatch.MergePatch(originalObjJs, patchJs) + if err != nil { + errorJSON(err, codec, w) + return + } + + if err := codec.DecodeInto(patchedObjJs, obj); err != nil { + errorJSON(err, codec, w) + return + } + if err := checkName(obj, name, namespace, namer); err != nil { + errorJSON(err, codec, w) + return + } + + result, err := finishRequest(timeout, func() (runtime.Object, error) { + // update should never create as previous get would fail + obj, _, err := r.Update(ctx, obj) + return obj, err + }) + if err != nil { + errorJSON(err, codec, w) + return + } + + if err := setSelfLink(result, req, namer); err != nil { + errorJSON(err, codec, w) + return + } + + writeJSON(http.StatusOK, codec, result, w) + } +} + // UpdateResource returns a function that will handle a resource update func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, typer runtime.ObjectTyper, resource string, admit admission.Interface) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { @@ -216,18 +294,9 @@ func UpdateResource(r RESTUpdater, ctxFn ContextFunc, namer ScopeNamer, codec ru return } - // check the provided name against the request - if objNamespace, objName, err := namer.ObjectName(obj); err == nil { - if objName != name { - errorJSON(errors.NewBadRequest("the name of the object does not match the name on the URL"), codec, w) - return - } - if len(namespace) > 0 { - if len(objNamespace) > 0 && objNamespace != namespace { - errorJSON(errors.NewBadRequest("the namespace of the object does not match the namespace on the request"), codec, w) - return - } - } + if err := checkName(obj, name, namespace, namer); err != nil { + errorJSON(err, codec, w) + return } err = admit.Admit(admission.NewAttributesRecord(obj, namespace, resource, "UPDATE")) @@ -378,6 +447,21 @@ func setSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) err return namer.SetSelfLink(obj, newURL.String()) } +// checkName checks the provided name against the request +func checkName(obj runtime.Object, name, namespace string, namer ScopeNamer) error { + if objNamespace, objName, err := namer.ObjectName(obj); err == nil { + if objName != name { + return errors.NewBadRequest("the name of the object does not match the name on the URL") + } + if len(namespace) > 0 { + if len(objNamespace) > 0 && objNamespace != namespace { + return errors.NewBadRequest("the namespace of the object does not match the namespace on the request") + } + } + } + return nil +} + // setListSelfLink sets the self link of a list to the base URL, then sets the self links // on all child objects returned. func setListSelfLink(obj runtime.Object, req *restful.Request, namer ScopeNamer) error { diff --git a/pkg/client/restclient.go b/pkg/client/restclient.go index a481e83fabd81..d91f8528639bf 100644 --- a/pkg/client/restclient.go +++ b/pkg/client/restclient.go @@ -105,6 +105,11 @@ func (c *RESTClient) Put() *Request { return c.Verb("PUT") } +// Patch begins a PATCH request. Short for c.Verb("Patch"). +func (c *RESTClient) Patch() *Request { + return c.Verb("PATCH") +} + // Get begins a GET request. Short for c.Verb("GET"). func (c *RESTClient) Get() *Request { return c.Verb("GET") diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go index 2dd5f12c41f0a..d67c79a841dc5 100644 --- a/test/integration/auth_test.go +++ b/test/integration/auth_test.go @@ -202,12 +202,12 @@ func getTestRequests() []struct { {"PUT", "/api/v1beta1/pods/a" + timeoutFlag, aPod, code200}, {"GET", "/api/v1beta1/pods", "", code200}, {"GET", "/api/v1beta1/pods/a", "", code200}, + {"PATCH", "/api/v1beta1/pods/a", "{%v}", code200}, {"DELETE", "/api/v1beta1/pods/a" + timeoutFlag, "", code200}, // Non-standard methods (not expected to work, // but expected to pass/fail authorization prior to // failing validation. - {"PATCH", "/api/v1beta1/pods/a", "", code405}, {"OPTIONS", "/api/v1beta1/pods", "", code405}, {"OPTIONS", "/api/v1beta1/pods/a", "", code405}, {"HEAD", "/api/v1beta1/pods", "", code405},