From e66d1ce02dcf420daaf6b67b5b0169944ee5c925 Mon Sep 17 00:00:00 2001 From: Mengqi Yu Date: Fri, 14 Dec 2018 13:35:30 -0800 Subject: [PATCH] :warning: introduce PatchResponseFromRaw and drop PatchResponse --- example/mutatingwebhook.go | 12 +++++-- pkg/patch/doc.go | 33 ------------------- pkg/patch/patch.go | 45 -------------------------- pkg/webhook/admission/response.go | 12 ++++--- pkg/webhook/admission/response_test.go | 3 +- 5 files changed, 17 insertions(+), 88 deletions(-) delete mode 100644 pkg/patch/doc.go delete mode 100644 pkg/patch/patch.go diff --git a/example/mutatingwebhook.go b/example/mutatingwebhook.go index b781d3cee0..98b0323809 100644 --- a/example/mutatingwebhook.go +++ b/example/mutatingwebhook.go @@ -18,6 +18,7 @@ package main import ( "context" + "encoding/json" "net/http" corev1 "k8s.io/api/core/v1" @@ -44,13 +45,18 @@ func (a *podAnnotator) Handle(ctx context.Context, req types.Request) types.Resp if err != nil { return admission.ErrorResponse(http.StatusBadRequest, err) } - copy := pod.DeepCopy() - err = a.mutatePodsFn(ctx, copy) + err = a.mutatePodsFn(ctx, pod) if err != nil { return admission.ErrorResponse(http.StatusInternalServerError, err) } - return admission.PatchResponse(pod, copy) + + marshaledPod, err := json.Marshal(pod) + if err != nil { + return admission.ErrorResponse(http.StatusInternalServerError, err) + } + + return admission.PatchResponseFromRaw(req.AdmissionRequest.Object.Raw, marshaledPod) } // mutatePodsFn add an annotation to the given pod diff --git a/pkg/patch/doc.go b/pkg/patch/doc.go deleted file mode 100644 index 0e1d2a9f56..0000000000 --- a/pkg/patch/doc.go +++ /dev/null @@ -1,33 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -/* -Package patch provides method to calculate JSON patch between 2 k8s objects. - -Calculate JSON patch - - oldDeployment := appsv1.Deployment{ - // some fields - } - newDeployment := appsv1.Deployment{ - // some different fields - } - patch, err := NewJSONPatch(oldDeployment, newDeployment) - if err != nil { - // handle error - } -*/ -package patch diff --git a/pkg/patch/patch.go b/pkg/patch/patch.go deleted file mode 100644 index 5a93fa08fe..0000000000 --- a/pkg/patch/patch.go +++ /dev/null @@ -1,45 +0,0 @@ -/* -Copyright 2018 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package patch - -import ( - "encoding/json" - "fmt" - "reflect" - - "github.com/appscode/jsonpatch" - - "k8s.io/apimachinery/pkg/runtime" -) - -// NewJSONPatch calculates the JSON patch between original and current objects. -func NewJSONPatch(original, current runtime.Object) ([]jsonpatch.JsonPatchOperation, error) { - originalGVK := original.GetObjectKind().GroupVersionKind() - currentGVK := current.GetObjectKind().GroupVersionKind() - if !reflect.DeepEqual(originalGVK, currentGVK) { - return nil, fmt.Errorf("GroupVersionKind %#v is expected to match %#v", originalGVK, currentGVK) - } - ori, err := json.Marshal(original) - if err != nil { - return nil, err - } - cur, err := json.Marshal(current) - if err != nil { - return nil, err - } - return jsonpatch.CreatePatch(ori, cur) -} diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go index bf7c903baa..74f55a4ca3 100644 --- a/pkg/webhook/admission/response.go +++ b/pkg/webhook/admission/response.go @@ -19,10 +19,10 @@ package admission import ( "net/http" + "github.com/appscode/jsonpatch" + admissionv1beta1 "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/patch" "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) @@ -54,9 +54,11 @@ func ValidationResponse(allowed bool, reason string) types.Response { return resp } -// PatchResponse returns a new response with json patch. -func PatchResponse(original, current runtime.Object) types.Response { - patches, err := patch.NewJSONPatch(original, current) +// PatchResponseFromRaw takes 2 byte arrays and returns a new response with json patch. +// The original object should be passed in as raw bytes to avoid the roundtripping problem +// described in https://github.com/kubernetes-sigs/kubebuilder/issues/510. +func PatchResponseFromRaw(original, current []byte) types.Response { + patches, err := jsonpatch.CreatePatch(original, current) if err != nil { return ErrorResponse(http.StatusInternalServerError, err) } diff --git a/pkg/webhook/admission/response_test.go b/pkg/webhook/admission/response_test.go index 2c8434a041..fcbecce942 100644 --- a/pkg/webhook/admission/response_test.go +++ b/pkg/webhook/admission/response_test.go @@ -25,7 +25,6 @@ import ( . "github.com/onsi/gomega" admissionv1beta1 "k8s.io/api/admission/v1beta1" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" ) @@ -72,7 +71,7 @@ var _ = Describe("admission webhook response", func() { PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(), }, } - resp := PatchResponse(&corev1.Pod{}, &corev1.Pod{}) + resp := PatchResponseFromRaw([]byte(`{}`), []byte(`{}`)) Expect(resp).To(Equal(expected)) }) })