From 9c94d772fb708d401a9f27239b45d0c4fbf32cc9 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Fri, 2 Oct 2020 12:04:08 -0300 Subject: [PATCH] Add support for admission review v1beta1 --- internal/admission/controller/convert.go | 90 ++++++++++++++++ internal/admission/controller/main.go | 103 +++++++++++-------- internal/admission/controller/main_test.go | 29 ++++-- internal/admission/controller/server.go | 57 +++++----- internal/admission/controller/server_test.go | 93 ----------------- 5 files changed, 203 insertions(+), 169 deletions(-) create mode 100644 internal/admission/controller/convert.go delete mode 100644 internal/admission/controller/server_test.go diff --git a/internal/admission/controller/convert.go b/internal/admission/controller/convert.go new file mode 100644 index 000000000..2382badee --- /dev/null +++ b/internal/admission/controller/convert.go @@ -0,0 +1,90 @@ +/* +Copyright 2020 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 controller + +import ( + "unsafe" + + admissionv1 "k8s.io/api/admission/v1" + admissionv1beta1 "k8s.io/api/admission/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" +) + +// these conversions are copied from https://github.com/kubernetes/kubernetes/blob/4db3a096ce8ac730b2280494422e1c4cf5fe875e/pkg/apis/admission/v1beta1/zz_generated.conversion.go +// to avoid copying in kubernetes/kubernetes +// they are sightly modified to remove complexity + +func convertV1beta1AdmissionReviewToAdmissionAdmissionReview(in *admissionv1beta1.AdmissionReview, out *admissionv1.AdmissionReview) { + if in.Request != nil { + if out.Request == nil { + out.Request = &admissionv1.AdmissionRequest{} + } + in, out := &in.Request, &out.Request + *out = new(admissionv1.AdmissionRequest) + convertV1beta1AdmissionRequestToAdmissionAdmissionRequest(*in, *out) + } else { + out.Request = nil + } + out.Response = (*admissionv1.AdmissionResponse)(unsafe.Pointer(in.Response)) +} + +func convertV1beta1AdmissionRequestToAdmissionAdmissionRequest(in *admissionv1beta1.AdmissionRequest, out *admissionv1.AdmissionRequest) { + out.UID = types.UID(in.UID) + out.Kind = in.Kind + out.Resource = in.Resource + out.SubResource = in.SubResource + out.RequestKind = (*metav1.GroupVersionKind)(unsafe.Pointer(in.RequestKind)) + out.RequestResource = (*metav1.GroupVersionResource)(unsafe.Pointer(in.RequestResource)) + out.RequestSubResource = in.RequestSubResource + out.Name = in.Name + out.Namespace = in.Namespace + out.Operation = admissionv1.Operation(in.Operation) + out.Object = in.Object + out.OldObject = in.OldObject + out.Options = in.Options +} + +func convertAdmissionAdmissionReviewToV1beta1AdmissionReview(in *admissionv1.AdmissionReview, out *admissionv1beta1.AdmissionReview) { + if in.Request != nil { + if out.Request == nil { + out.Request = &admissionv1beta1.AdmissionRequest{} + } + in, out := &in.Request, &out.Request + *out = new(admissionv1beta1.AdmissionRequest) + convertAdmissionAdmissionRequestToV1beta1AdmissionRequest(*in, *out) + } else { + out.Request = nil + } + out.Response = (*admissionv1beta1.AdmissionResponse)(unsafe.Pointer(in.Response)) +} + +func convertAdmissionAdmissionRequestToV1beta1AdmissionRequest(in *admissionv1.AdmissionRequest, out *admissionv1beta1.AdmissionRequest) { + out.UID = types.UID(in.UID) + out.Kind = in.Kind + out.Resource = in.Resource + out.SubResource = in.SubResource + out.RequestKind = (*metav1.GroupVersionKind)(unsafe.Pointer(in.RequestKind)) + out.RequestResource = (*metav1.GroupVersionResource)(unsafe.Pointer(in.RequestResource)) + out.RequestSubResource = in.RequestSubResource + out.Name = in.Name + out.Namespace = in.Namespace + out.Operation = admissionv1beta1.Operation(in.Operation) + out.Object = in.Object + out.OldObject = in.OldObject + out.Options = in.Options +} diff --git a/internal/admission/controller/main.go b/internal/admission/controller/main.go index 7cb9c2ad5..445bb263c 100644 --- a/internal/admission/controller/main.go +++ b/internal/admission/controller/main.go @@ -18,13 +18,16 @@ package controller import ( "fmt" + "net/http" admissionv1 "k8s.io/api/admission/v1" + admissionv1beta1 "k8s.io/api/admission/v1beta1" networking "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/klog/v2" - - "k8s.io/ingress-nginx/internal/ingress/annotations/parser" ) // Checker must return an error if the ingress provided as argument @@ -56,62 +59,76 @@ var ( // HandleAdmission populates the admission Response // with Allowed=false if the Object is an ingress that would prevent nginx to reload the configuration // with Allowed=true otherwise -func (ia *IngressAdmission) HandleAdmission(ar *admissionv1.AdmissionReview) { - if ar.Request == nil { - ar.Response = &admissionv1.AdmissionResponse{ - Allowed: false, +func (ia *IngressAdmission) HandleAdmission(obj runtime.Object) (runtime.Object, error) { + outputVersion := admissionv1.SchemeGroupVersion + + review, isV1 := obj.(*admissionv1.AdmissionReview) + + status := &admissionv1.AdmissionResponse{} + status.UID = review.Request.UID + + if !isV1 { + outputVersion = admissionv1beta1.SchemeGroupVersion + reviewv1beta1, isv1beta1 := obj.(*admissionv1beta1.AdmissionReview) + if !isv1beta1 { + return nil, fmt.Errorf("request is not of type apiextensions v1 or v1beta1") } - return + review = &admissionv1.AdmissionReview{} + convertV1beta1AdmissionReviewToAdmissionAdmissionReview(reviewv1beta1, review) } - if ar.Request.Resource != networkingV1Beta1Resource && ar.Request.Resource != networkingV1Resource { - err := fmt.Errorf("rejecting admission review because the request does not contains an Ingress resource but %s with name %s in namespace %s", - ar.Request.Resource.String(), ar.Request.Name, ar.Request.Namespace) - ar.Response = &admissionv1.AdmissionResponse{ - UID: ar.Request.UID, - Allowed: false, - Result: &metav1.Status{Message: err.Error()}, - } - - return + if review.Request.Resource != networkingV1Beta1Resource && review.Request.Resource != networkingV1Resource { + return nil, fmt.Errorf("rejecting admission review because the request does not contains an Ingress resource but %s with name %s in namespace %s", + review.Request.Resource.String(), review.Request.Name, review.Request.Namespace) } ingress := networking.Ingress{} - deserializer := codecs.UniversalDeserializer() - if _, _, err := deserializer.Decode(ar.Request.Object.Raw, nil, &ingress); err != nil { - klog.ErrorS(err, "failed to decode ingress", "ingress", ar.Request.Name, "namespace", ar.Request.Namespace) - ar.Response = &admissionv1.AdmissionResponse{ - UID: ar.Request.UID, - Allowed: false, - - Result: &metav1.Status{Message: err.Error()}, - AuditAnnotations: map[string]string{ - parser.GetAnnotationWithPrefix("error"): err.Error(), - }, + codec := json.NewSerializerWithOptions(json.DefaultMetaFactory, scheme, scheme, json.SerializerOptions{ + Pretty: true, + }) + codec.Decode(review.Request.Object.Raw, nil, nil) + _, _, err := codec.Decode(review.Request.Object.Raw, nil, &ingress) + if err != nil { + klog.ErrorS(err, "failed to decode ingress") + status.Allowed = false + status.Result = &metav1.Status{ + Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, + Message: err.Error(), } - return + review.Response = status + return convertResponse(review, outputVersion), nil } if err := ia.Checker.CheckIngress(&ingress); err != nil { - klog.ErrorS(err, "failed to generate configuration for ingress", "ingress", ar.Request.Name, "namespace", ar.Request.Namespace) - ar.Response = &admissionv1.AdmissionResponse{ - UID: ar.Request.UID, - Allowed: false, - Result: &metav1.Status{Message: err.Error()}, - AuditAnnotations: map[string]string{ - parser.GetAnnotationWithPrefix("error"): err.Error(), - }, + klog.ErrorS(err, "invalid ingress configuration", "ingress", review.Request.Name, "namespace", review.Request.Namespace) + status.Allowed = false + status.Result = &metav1.Status{ + Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, + Message: err.Error(), } - return + review.Response = status + return convertResponse(review, outputVersion), nil } - klog.InfoS("successfully validated configuration, accepting", "ingress", ar.Request.Name, "namespace", ar.Request.Namespace) - ar.Response = &admissionv1.AdmissionResponse{ - UID: ar.Request.UID, - Allowed: true, - } + klog.InfoS("successfully validated configuration, accepting", "ingress", review.Request.Name, "namespace", review.Request.Namespace) + status.Allowed = true + review.Response = status + + return convertResponse(review, outputVersion), nil +} + +func convertResponse(review *admissionv1.AdmissionReview, outputVersion schema.GroupVersion) runtime.Object { + // reply v1 + if outputVersion.Version == admissionv1.SchemeGroupVersion.Version { + return review + } + + // reply v1beta1 + reviewv1beta1 := &admissionv1beta1.AdmissionReview{} + convertAdmissionAdmissionReviewToV1beta1AdmissionReview(review, reviewv1beta1) + return review } diff --git a/internal/admission/controller/main_test.go b/internal/admission/controller/main_test.go index 9cd94f43c..93fc3b271 100644 --- a/internal/admission/controller/main_test.go +++ b/internal/admission/controller/main_test.go @@ -23,6 +23,7 @@ import ( admissionv1 "k8s.io/api/admission/v1" networking "k8s.io/api/networking/v1beta1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/json" ) @@ -53,21 +54,33 @@ func TestHandleAdmission(t *testing.T) { adm := &IngressAdmission{ Checker: failTestChecker{t: t}, } - review := &admissionv1.AdmissionReview{ + + result, err := adm.HandleAdmission(&admissionv1.AdmissionReview{ Request: &admissionv1.AdmissionRequest{ Resource: v1.GroupVersionResource{Group: "", Version: "v1", Resource: "pod"}, }, - } - - adm.HandleAdmission(review) - if review.Response.Allowed { + }) + if err == nil { t.Fatalf("with a non ingress resource, the check should not pass") } - review.Request.Resource = v1.GroupVersionResource{Group: networking.GroupName, Version: "v1beta1", Resource: "ingresses"} - review.Request.Object.Raw = []byte{0xff} + result, err = adm.HandleAdmission(&admissionv1.AdmissionReview{ + Request: &admissionv1.AdmissionRequest{ + Resource: v1.GroupVersionResource{Group: networking.GroupName, Version: "v1beta1", Resource: "ingresses"}, + Object: runtime.RawExtension{ + Raw: []byte{0xff}, + }, + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + review, isV1 := (result).(*admissionv1.AdmissionReview) + if !isV1 { + t.Fatalf("expected AdmissionReview V1 object but %T returned", result) + } - adm.HandleAdmission(review) if review.Response.Allowed { t.Fatalf("when the request object is not decodable, the request should not be allowed") } diff --git a/internal/admission/controller/server.go b/internal/admission/controller/server.go index c9bcd4fb4..8c9235bdc 100644 --- a/internal/admission/controller/server.go +++ b/internal/admission/controller/server.go @@ -17,26 +17,29 @@ limitations under the License. package controller import ( - "io" "io/ioutil" "net/http" admissionv1 "k8s.io/api/admission/v1" + admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/serializer" - "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apimachinery/pkg/runtime/serializer/json" "k8s.io/klog/v2" ) var ( scheme = runtime.NewScheme() - codecs = serializer.NewCodecFactory(scheme) ) +func init() { + admissionv1beta1.AddToScheme(scheme) + admissionv1.AddToScheme(scheme) +} + // AdmissionController checks if an object // is allowed in the cluster type AdmissionController interface { - HandleAdmission(*admissionv1.AdmissionReview) + HandleAdmission(runtime.Object) (runtime.Object, error) } // AdmissionControllerServer implements an HTTP server @@ -44,7 +47,6 @@ type AdmissionController interface { // https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#validatingadmissionwebhook type AdmissionControllerServer struct { AdmissionController AdmissionController - Decoder runtime.Decoder } // NewAdmissionControllerServer instanciates an admission controller server with @@ -52,36 +54,41 @@ type AdmissionControllerServer struct { func NewAdmissionControllerServer(ac AdmissionController) *AdmissionControllerServer { return &AdmissionControllerServer{ AdmissionController: ac, - Decoder: codecs.UniversalDeserializer(), } } // ServeHTTP implements http.Server method -func (acs *AdmissionControllerServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { - review, err := parseAdmissionReview(acs.Decoder, r.Body) +func (acs *AdmissionControllerServer) ServeHTTP(w http.ResponseWriter, req *http.Request) { + defer req.Body.Close() + + data, err := ioutil.ReadAll(req.Body) if err != nil { - klog.ErrorS(err, "Unexpected error decoding request") + klog.ErrorS(err, "Failed to read request body") w.WriteHeader(http.StatusBadRequest) return } - acs.AdmissionController.HandleAdmission(review) - if err := writeAdmissionReview(w, review); err != nil { - klog.ErrorS(err, "Unexpected returning admission review") - } -} + codec := json.NewSerializerWithOptions(json.DefaultMetaFactory, scheme, scheme, json.SerializerOptions{ + Pretty: true, + }) -func parseAdmissionReview(decoder runtime.Decoder, r io.Reader) (*admissionv1.AdmissionReview, error) { - review := &admissionv1.AdmissionReview{} - data, err := ioutil.ReadAll(r) + obj, _, err := codec.Decode(data, nil, nil) if err != nil { - return nil, err + klog.ErrorS(err, "Failed to decode request body") + w.WriteHeader(http.StatusBadRequest) + return } - _, _, err = decoder.Decode(data, nil, review) - return review, err -} -func writeAdmissionReview(w io.Writer, ar *admissionv1.AdmissionReview) error { - e := json.NewEncoder(w) - return e.Encode(ar) + result, err := acs.AdmissionController.HandleAdmission(obj) + if err != nil { + klog.ErrorS(err, "failed to process webhook request") + w.WriteHeader(http.StatusInternalServerError) + return + } + + if err := codec.Encode(result, w); err != nil { + klog.ErrorS(err, "failed to encode response body") + w.WriteHeader(http.StatusInternalServerError) + return + } } diff --git a/internal/admission/controller/server_test.go b/internal/admission/controller/server_test.go deleted file mode 100644 index ad2ffa946..000000000 --- a/internal/admission/controller/server_test.go +++ /dev/null @@ -1,93 +0,0 @@ -/* -Copyright 2019 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 controller - -import ( - "bytes" - "fmt" - "net/http" - "net/http/httptest" - "strings" - "testing" - - admissionv1 "k8s.io/api/admission/v1" -) - -type testAdmissionHandler struct{} - -func (testAdmissionHandler) HandleAdmission(ar *admissionv1.AdmissionReview) { - ar.Response = &admissionv1.AdmissionResponse{ - Allowed: true, - } -} - -type errorReader struct{} - -func (errorReader) Read(p []byte) (n int, err error) { - return 0, fmt.Errorf("this is a test error") -} - -type errorWriter struct{} - -func (errorWriter) Write(p []byte) (n int, err error) { - return 0, fmt.Errorf("this is a test error") -} - -func (errorWriter) Header() http.Header { - return nil -} - -func (errorWriter) WriteHeader(statusCode int) {} - -func TestServer(t *testing.T) { - w := httptest.NewRecorder() - b := bytes.NewBuffer(nil) - writeAdmissionReview(b, &admissionv1.AdmissionReview{}) - - // Happy path - r := httptest.NewRequest("GET", "http://test.ns.svc", b) - NewAdmissionControllerServer(testAdmissionHandler{}).ServeHTTP(w, r) - ar, err := parseAdmissionReview(codecs.UniversalDeserializer(), w.Body) - if w.Code != http.StatusOK { - t.Errorf("when the admission review allows the request, the http status should be OK") - } - if err != nil { - t.Errorf("failed to parse admission response when the admission controller returns a value") - } - if !ar.Response.Allowed { - t.Errorf("when the admission review allows the request, the parsed body returns not allowed") - } - - // Ensure the code does not panic when failing to handle the request - NewAdmissionControllerServer(testAdmissionHandler{}).ServeHTTP(errorWriter{}, r) - - w = httptest.NewRecorder() - NewAdmissionControllerServer(testAdmissionHandler{}).ServeHTTP(w, httptest.NewRequest("GET", "http://test.ns.svc", strings.NewReader("invalid-json"))) - if w.Code != http.StatusBadRequest { - t.Errorf("when the server fails to read the request, the replied status should be bad request") - } -} - -func TestParseAdmissionReview(t *testing.T) { - ar, err := parseAdmissionReview(codecs.UniversalDeserializer(), errorReader{}) - if ar != nil { - t.Errorf("when reading from request fails, no AdmissionRewiew should be returned") - } - if err == nil { - t.Errorf("when reading from request fails, an error should be returned") - } -}