From 74667b711106d354f45f5de28408b3604dd77ce6 Mon Sep 17 00:00:00 2001 From: DangPeng Liu Date: Thu, 18 Nov 2021 22:12:21 +0800 Subject: [PATCH] handle request error in webhook (#414) Signed-off-by: ldpliu --- pkg/webhook/clusterset/validatingWebhook.go | 70 +-------- pkg/webhook/serve/serve.go | 107 +++++++++++++ pkg/webhook/serve/serve_test.go | 86 +++++++++++ pkg/webhook/useridentity/mutatingWebhook.go | 86 ++--------- .../useridentity/mutatingWebhook_test.go | 146 ++++++++---------- 5 files changed, 271 insertions(+), 224 deletions(-) create mode 100644 pkg/webhook/serve/serve.go create mode 100644 pkg/webhook/serve/serve_test.go diff --git a/pkg/webhook/clusterset/validatingWebhook.go b/pkg/webhook/clusterset/validatingWebhook.go index 9994ef519..6c033b393 100644 --- a/pkg/webhook/clusterset/validatingWebhook.go +++ b/pkg/webhook/clusterset/validatingWebhook.go @@ -4,22 +4,18 @@ import ( "context" "encoding/json" "fmt" - "io" - "io/ioutil" "net/http" - "github.com/open-cluster-management/multicloud-operators-foundation/cmd/webhook/app/options" + serve "github.com/open-cluster-management/multicloud-operators-foundation/pkg/webhook/serve" hivev1 "github.com/openshift/hive/apis/hive/v1" hiveclient "github.com/openshift/hive/pkg/client/clientset/versioned" v1 "k8s.io/api/admission/v1" - authenticationv1 "k8s.io/api/authentication/v1" authorizationv1 "k8s.io/api/authorization/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes" - "k8s.io/klog/v2" clusterv1 "open-cluster-management.io/api/cluster/v1" ) @@ -28,68 +24,6 @@ type AdmissionHandler struct { KubeClient kubernetes.Interface } -// toAdmissionResponse is a helper function to create an AdmissionResponse -// with an embedded error -func toAdmissionResponse(err error) *v1.AdmissionResponse { - return &v1.AdmissionResponse{ - Result: &metav1.Status{ - Message: err.Error(), - }, - } -} - -// admitFunc is the type we use for all of our validators and mutators -type admitFunc func(request *v1.AdmissionRequest) *v1.AdmissionResponse - -// serve handles the http portion of a request prior to handing to an admit -// function -func (a *AdmissionHandler) serve(w io.Writer, r *http.Request, admit admitFunc) { - var body []byte - if r.Body != nil { - if data, err := ioutil.ReadAll(r.Body); err == nil { - body = data - } - } - // verify the content type is accurate - contentType := r.Header.Get("Content-Type") - if contentType != "application/json" { - klog.Errorf("contentType=%s, expect application/json", contentType) - return - } - - klog.V(2).Info(fmt.Sprintf("handling request: %s", body)) - - // The AdmissionReview that was sent to the webhook - requestedAdmissionReview := v1.AdmissionReview{} - - // The AdmissionReview that will be returned - responseAdmissionReview := v1.AdmissionReview{} - - deserializer := options.Codecs.UniversalDeserializer() - if _, _, err := deserializer.Decode(body, nil, &requestedAdmissionReview); err != nil { - klog.Error(err) - responseAdmissionReview.Response = toAdmissionResponse(err) - } else { - // pass to admitFunc - responseAdmissionReview.Response = admit(requestedAdmissionReview.Request) - } - - responseAdmissionReview.Kind = requestedAdmissionReview.Kind - responseAdmissionReview.APIVersion = requestedAdmissionReview.APIVersion - // Return the same UID - responseAdmissionReview.Response.UID = requestedAdmissionReview.Request.UID - - klog.V(2).Info(fmt.Sprintf("sending response: %+v", responseAdmissionReview)) - - respBytes, err := json.Marshal(responseAdmissionReview) - if err != nil { - klog.Error(err) - } - if _, err := w.Write(respBytes); err != nil { - klog.Error(err) - } -} - var managedClustersGVR = metav1.GroupVersionResource{ Group: "cluster.open-cluster-management.io", Version: "v1", @@ -337,5 +271,5 @@ func (a *AdmissionHandler) allowUpdateClusterSet(userInfo authenticationv1.UserI } func (a *AdmissionHandler) ServerValidateResource(w http.ResponseWriter, r *http.Request) { - a.serve(w, r, a.validateResource) + serve.Serve(w, r, a.validateResource) } diff --git a/pkg/webhook/serve/serve.go b/pkg/webhook/serve/serve.go new file mode 100644 index 000000000..c72e32223 --- /dev/null +++ b/pkg/webhook/serve/serve.go @@ -0,0 +1,107 @@ +package serve + +import ( + "bytes" + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + + "github.com/open-cluster-management/multicloud-operators-foundation/cmd/webhook/app/options" + v1 "k8s.io/api/admission/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/klog/v2" +) + +// toAdmissionResponse is a helper function to create an AdmissionResponse +// with an embedded error +func ToAdmissionResponse(err error) *v1.AdmissionResponse { + return &v1.AdmissionResponse{ + Result: &metav1.Status{ + Message: err.Error(), + Reason: metav1.StatusReasonBadRequest, + }, + } +} + +// admitFunc is the type we use for all of our validators and mutators +type admitFunc func(request *v1.AdmissionRequest) *v1.AdmissionResponse + +// serve handles the http portion of a request prior to handing to an admit +// function +func Serve(w http.ResponseWriter, r *http.Request, admit admitFunc) { + var body []byte + var errmsg string + // The AdmissionReview that was sent to the webhook + requestedAdmissionReview := v1.AdmissionReview{} + + // The AdmissionReview that will be returned + responseAdmissionReview := v1.AdmissionReview{} + + if r.Body == nil { + errmsg = "Request Body is null" + writerErrorResponse(errmsg, w) + return + } + data, err := ioutil.ReadAll(r.Body) + if err != nil { + errmsg = fmt.Sprintf("Can not read request body, err: %v", err) + writerErrorResponse(errmsg, w) + return + } + + body = data + + // verify the content type is accurate + contentType := r.Header.Get("Content-Type") + if contentType != "application/json" { + errmsg = fmt.Sprintf("contentType=%s, expect application/json", contentType) + writerErrorResponse(errmsg, w) + return + } + + klog.V(2).Info(fmt.Sprintf("handling request: %s", body)) + + deserializer := options.Codecs.UniversalDeserializer() + _, _, err = deserializer.Decode(body, nil, &requestedAdmissionReview) + if err != nil { + errmsg = fmt.Sprintf("Decode body error: %v", err) + writerErrorResponse(errmsg, w) + return + } else { + // pass to admitFunc + responseAdmissionReview.Response = admit(requestedAdmissionReview.Request) + } + + responseAdmissionReview.Kind = requestedAdmissionReview.Kind + responseAdmissionReview.APIVersion = requestedAdmissionReview.APIVersion + // Return the same UID + if requestedAdmissionReview.Request == nil { + errmsg = fmt.Sprintf("requestedAdmissionReview is nil") + writerErrorResponse(errmsg, w) + return + } + responseAdmissionReview.Response.UID = requestedAdmissionReview.Request.UID + + klog.V(2).Info(fmt.Sprintf("sending response: %+v", responseAdmissionReview)) + + respBytes, err := json.Marshal(responseAdmissionReview) + if err != nil { + errmsg = fmt.Sprintf("Decode responseAdmissionReview error: %v", err) + writerErrorResponse(errmsg, w) + return + } + _, err = w.Write(respBytes) + if err != nil { + errmsg = fmt.Sprintf("Write responsebyte error: %v", err) + writerErrorResponse(errmsg, w) + } + return +} + +func writerErrorResponse(errmsg string, httpWriter http.ResponseWriter) { + var buffer bytes.Buffer + buffer.WriteString(errmsg) + httpWriter.WriteHeader(http.StatusBadRequest) + httpWriter.Write(buffer.Bytes()) +} diff --git a/pkg/webhook/serve/serve_test.go b/pkg/webhook/serve/serve_test.go new file mode 100644 index 000000000..fa620e14a --- /dev/null +++ b/pkg/webhook/serve/serve_test.go @@ -0,0 +1,86 @@ +package serve + +import ( + "context" + "net/http" + "strconv" + "strings" + "testing" + + v1 "k8s.io/api/admission/v1" +) + +var statusCode = "statusCode" + +func fakeadmit(request *v1.AdmissionRequest) *v1.AdmissionResponse { + status := &v1.AdmissionResponse{ + Allowed: true, + } + return status +} +func TestServe(t *testing.T) { + cases := []struct { + name string + request *http.Request + responseWriter *fakeWriter + }{ + { + name: "nil body in request", + request: func() *http.Request { + r, _ := http.NewRequest(http.MethodOptions, "url", nil) + return r + }(), + + responseWriter: &fakeWriter{}, + }, + { + name: "error header format in request", + request: func() *http.Request { + r, _ := http.NewRequest(http.MethodOptions, "url", strings.NewReader("{\"foo\":\"bar\"}")) + return r + }(), + + responseWriter: &fakeWriter{}, + }, + { + name: "requestedAdmissionReview is not right", + request: func() *http.Request { + ctx := context.TODO() + r, _ := http.NewRequestWithContext(ctx, http.MethodHead, "url", strings.NewReader("{\"foo\":\"bar\"}")) + r.Header.Add("Content-Type", "application/json") + return r + }(), + responseWriter: &fakeWriter{}, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + Serve(c.responseWriter, c.request, fakeadmit) + if len(c.responseWriter.Header()[statusCode]) == 0 { + t.Errorf("response error:%v", c.responseWriter.Header()) + } + }) + } +} + +type fakeWriter struct { + header http.Header +} + +func (fw *fakeWriter) Header() http.Header { + if fw.header == nil { + fw.header = http.Header{} + } + return fw.header +} + +func (fw *fakeWriter) WriteHeader(status int) { + tempHead := make(map[string][]string) + tempHead[statusCode] = []string{strconv.Itoa(status)} + fw.header = tempHead +} + +func (fw *fakeWriter) Write(data []byte) (int, error) { + return len(data), nil +} diff --git a/pkg/webhook/useridentity/mutatingWebhook.go b/pkg/webhook/useridentity/mutatingWebhook.go index 789e286aa..27e5986f7 100644 --- a/pkg/webhook/useridentity/mutatingWebhook.go +++ b/pkg/webhook/useridentity/mutatingWebhook.go @@ -2,18 +2,14 @@ package useridentity import ( "encoding/json" - "fmt" - "io" - "io/ioutil" "net/http" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "github.com/mattbaird/jsonpatch" - "github.com/open-cluster-management/multicloud-operators-foundation/cmd/webhook/app/options" "github.com/open-cluster-management/multicloud-operators-foundation/pkg/utils" + serve "github.com/open-cluster-management/multicloud-operators-foundation/pkg/webhook/serve" v1 "k8s.io/api/admission/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" rbaclisters "k8s.io/client-go/listers/rbac/v1" "k8s.io/klog/v2" ) @@ -23,88 +19,26 @@ type AdmissionHandler struct { SkipOverwriteUserList []string } -// toAdmissionResponse is a helper function to create an AdmissionResponse -// with an embedded error -func toAdmissionResponse(err error) *v1.AdmissionResponse { - return &v1.AdmissionResponse{ - Result: &metav1.Status{ - Message: err.Error(), - }, - } -} - -// admitFunc is the type we use for all of our validators and mutators -type admitFunc func(v1.AdmissionReview) *v1.AdmissionResponse - -// serve handles the http portion of a request prior to handing to an admit -// function -func (a *AdmissionHandler) serve(w io.Writer, r *http.Request, admit admitFunc) { - var body []byte - if r.Body != nil { - if data, err := ioutil.ReadAll(r.Body); err == nil { - body = data - } - } - // verify the content type is accurate - contentType := r.Header.Get("Content-Type") - if contentType != "application/json" { - klog.Errorf("contentType=%s, expect application/json", contentType) - return - } - - klog.V(2).Info(fmt.Sprintf("handling request: %s", body)) - - // The AdmissionReview that was sent to the webhook - requestedAdmissionReview := v1.AdmissionReview{} - - // The AdmissionReview that will be returned - responseAdmissionReview := v1.AdmissionReview{} - - deserializer := options.Codecs.UniversalDeserializer() - if _, _, err := deserializer.Decode(body, nil, &requestedAdmissionReview); err != nil { - klog.Error(err) - responseAdmissionReview.Response = toAdmissionResponse(err) - } else { - // pass to admitFunc - responseAdmissionReview.Response = admit(requestedAdmissionReview) - } - - responseAdmissionReview.Kind = requestedAdmissionReview.Kind - responseAdmissionReview.APIVersion = requestedAdmissionReview.APIVersion - // Return the same UID - responseAdmissionReview.Response.UID = requestedAdmissionReview.Request.UID - - klog.V(2).Info(fmt.Sprintf("sending response: %+v", responseAdmissionReview)) - - respBytes, err := json.Marshal(responseAdmissionReview) - if err != nil { - klog.Error(err) - } - if _, err := w.Write(respBytes); err != nil { - klog.Error(err) - } -} - -func (a *AdmissionHandler) mutateResource(ar v1.AdmissionReview) *v1.AdmissionResponse { +func (a *AdmissionHandler) mutateResource(ar *v1.AdmissionRequest) *v1.AdmissionResponse { klog.V(4).Info("mutating custom resource") obj := unstructured.Unstructured{} - err := obj.UnmarshalJSON(ar.Request.Object.Raw) + err := obj.UnmarshalJSON(ar.Object.Raw) if err != nil { klog.Error(err) - return toAdmissionResponse(err) + return serve.ToAdmissionResponse(err) } annotations := obj.GetAnnotations() - if utils.ContainsString(a.SkipOverwriteUserList, ar.Request.UserInfo.Username) { - klog.V(4).Infof("Skip add user and group for resource: %+v, name: %+v", ar.Request.Resource.Resource, obj.GetName()) + if utils.ContainsString(a.SkipOverwriteUserList, ar.UserInfo.Username) { + klog.V(4).Infof("Skip add user and group for resource: %+v, name: %+v", ar.Resource.Resource, obj.GetName()) reviewResponse := v1.AdmissionResponse{} reviewResponse.Allowed = true return &reviewResponse } - resAnnotations := MergeUserIdentityToAnnotations(ar.Request.UserInfo, annotations, obj.GetNamespace(), a.Lister) + resAnnotations := MergeUserIdentityToAnnotations(ar.UserInfo, annotations, obj.GetNamespace(), a.Lister) obj.SetAnnotations(resAnnotations) reviewResponse := v1.AdmissionResponse{} @@ -115,7 +49,7 @@ func (a *AdmissionHandler) mutateResource(ar v1.AdmissionReview) *v1.AdmissionRe klog.Errorf("marshal json error: %+v", err) return nil } - res, err := jsonpatch.CreatePatch(ar.Request.Object.Raw, updatedObj) + res, err := jsonpatch.CreatePatch(ar.Object.Raw, updatedObj) if err != nil { klog.Errorf("Create patch error: %+v", err) return nil @@ -131,10 +65,10 @@ func (a *AdmissionHandler) mutateResource(ar v1.AdmissionReview) *v1.AdmissionRe pt := v1.PatchTypeJSONPatch reviewResponse.PatchType = &pt - klog.V(2).Infof("Successfully Added user and group for resource: %+v, name: %+v", ar.Request.Resource.Resource, obj.GetName()) + klog.V(2).Infof("Successfully Added user and group for resource: %+v, name: %+v", ar.Resource.Resource, obj.GetName()) return &reviewResponse } func (a *AdmissionHandler) ServeMutateResource(w http.ResponseWriter, r *http.Request) { - a.serve(w, r, a.mutateResource) + serve.Serve(w, r, a.mutateResource) } diff --git a/pkg/webhook/useridentity/mutatingWebhook_test.go b/pkg/webhook/useridentity/mutatingWebhook_test.go index 3253d67f4..9eaf1a58b 100644 --- a/pkg/webhook/useridentity/mutatingWebhook_test.go +++ b/pkg/webhook/useridentity/mutatingWebhook_test.go @@ -22,99 +22,85 @@ const ( appsubTest = `{"apiVersion":"apps.open-cluster-management.io/v1","kind": "Subscription","metadata": {"name": "git-sub","namespace": "parentsub","annotations": {"apps.open-cluster-management.io/cluster-admin": "true","apps.open-cluster-management.io/github-path": "test/e2e/github/nestedSubscription"}},"spec": {"channel": "ch-git/git","placement": {"local": "true"}}}` ) -func newAdmissionReview() *v1.AdmissionReview { - return &v1.AdmissionReview{ - TypeMeta: metav1.TypeMeta{ - Kind: "AdmissionReview", - APIVersion: "admission.k8s.io/v1", +func newAdmissionRequest() *v1.AdmissionRequest { + return &v1.AdmissionRequest{ + UID: "3f6d1d1f-61a0-49ea-b458-05454ba42ab6", + Kind: metav1.GroupVersionKind{ + Group: "apps.open-cluster-management.io", + Kind: "Channel", + Version: "v1", }, - Request: &v1.AdmissionRequest{ - UID: "3f6d1d1f-61a0-49ea-b458-05454ba42ab6", - Kind: metav1.GroupVersionKind{ - Group: "apps.open-cluster-management.io", - Kind: "Channel", - Version: "v1", - }, - Resource: metav1.GroupVersionResource{ - Group: "apps.open-cluster-management.io", - Version: "v1", - Resource: "channels", - }, - SubResource: "", - RequestKind: &metav1.GroupVersionKind{ - Group: "apps.open-cluster-management.io", - Kind: "Channel", - Version: "v1", - }, - RequestResource: &metav1.GroupVersionResource{ - Group: "apps.open-cluster-management.io", - Version: "v1", - Resource: "channels", - }, - RequestSubResource: "", - Name: "test", - Namespace: "default", - Operation: "CREATE", - UserInfo: authenticationv1.UserInfo{ - Username: "system:admin", - }, - Object: runtime.RawExtension{ - Raw: []byte(channelTest), - }, + Resource: metav1.GroupVersionResource{ + Group: "apps.open-cluster-management.io", + Version: "v1", + Resource: "channels", + }, + SubResource: "", + RequestKind: &metav1.GroupVersionKind{ + Group: "apps.open-cluster-management.io", + Kind: "Channel", + Version: "v1", + }, + RequestResource: &metav1.GroupVersionResource{ + Group: "apps.open-cluster-management.io", + Version: "v1", + Resource: "channels", + }, + RequestSubResource: "", + Name: "test", + Namespace: "default", + Operation: "CREATE", + UserInfo: authenticationv1.UserInfo{ + Username: "system:admin", + }, + Object: runtime.RawExtension{ + Raw: []byte(channelTest), }, - Response: nil, } } -func appSubAdmissionReview() *v1.AdmissionReview { - return &v1.AdmissionReview{ - TypeMeta: metav1.TypeMeta{ - Kind: "AdmissionReview", - APIVersion: "admission.k8s.io/v1", +func appSubAdmissionRequest() *v1.AdmissionRequest { + return &v1.AdmissionRequest{ + UID: "4d6d1d1f-61a0-49ea-b458-05454ba42ab6", + Kind: metav1.GroupVersionKind{ + Group: "apps.open-cluster-management.io", + Kind: "Subscription", + Version: "v1", + }, + Resource: metav1.GroupVersionResource{ + Group: "apps.open-cluster-management.io", + Version: "v1", + Resource: "subscriptions", + }, + SubResource: "", + RequestKind: &metav1.GroupVersionKind{ + Group: "apps.open-cluster-management.io", + Kind: "Subscription", + Version: "v1", + }, + RequestResource: &metav1.GroupVersionResource{ + Group: "apps.open-cluster-management.io", + Version: "v1", + Resource: "subscriptions", + }, + RequestSubResource: "", + Name: "subtest", + Namespace: "default", + Operation: "CREATE", + UserInfo: authenticationv1.UserInfo{ + Username: "system:serviceaccount:open-cluster-management-agent-addon:klusterlet-addon-appmgr", }, - Request: &v1.AdmissionRequest{ - UID: "4d6d1d1f-61a0-49ea-b458-05454ba42ab6", - Kind: metav1.GroupVersionKind{ - Group: "apps.open-cluster-management.io", - Kind: "Subscription", - Version: "v1", - }, - Resource: metav1.GroupVersionResource{ - Group: "apps.open-cluster-management.io", - Version: "v1", - Resource: "subscriptions", - }, - SubResource: "", - RequestKind: &metav1.GroupVersionKind{ - Group: "apps.open-cluster-management.io", - Kind: "Subscription", - Version: "v1", - }, - RequestResource: &metav1.GroupVersionResource{ - Group: "apps.open-cluster-management.io", - Version: "v1", - Resource: "subscriptions", - }, - RequestSubResource: "", - Name: "subtest", - Namespace: "default", - Operation: "CREATE", - UserInfo: authenticationv1.UserInfo{ - Username: "system:serviceaccount:open-cluster-management-agent-addon:klusterlet-addon-appmgr", - }, - Object: runtime.RawExtension{ - Raw: []byte(appsubTest), - }, + Object: runtime.RawExtension{ + Raw: []byte(appsubTest), }, - Response: nil, } } func TestMutateResource(t *testing.T) { adHandler := newAdmissionHandler() - rsp := adHandler.mutateResource(*newAdmissionReview()) + rsp := adHandler.mutateResource(newAdmissionRequest()) assert.True(t, true, len(rsp.Patch) != 0) - rsp = adHandler.mutateResource(*appSubAdmissionReview()) + rsp = adHandler.mutateResource(appSubAdmissionRequest()) assert.True(t, rsp.Patch == nil) }