From 127565faf152c9517342929fb311d7ca93639018 Mon Sep 17 00:00:00 2001 From: Christian Heike Date: Wed, 9 Jun 2021 07:36:05 +0200 Subject: [PATCH 1/3] Fix: provided request UID in response --- pkg/codec/deserializer_test.go | 12 ++++++--- pkg/webhook/facade/api.go | 4 +-- pkg/webhook/facade/v1_admission_review.go | 11 ++++---- .../facade/v1_admission_review_test.go | 25 ++++++++++++++++--- .../facade/v1beta1_admission_review.go | 11 ++++---- .../facade/v1beta1_admission_review_test.go | 25 ++++++++++++++++--- testing/admission_test/v1.go | 2 ++ testing/admission_test/v1beta1.go | 2 ++ 8 files changed, 67 insertions(+), 25 deletions(-) diff --git a/pkg/codec/deserializer_test.go b/pkg/codec/deserializer_test.go index 05800a9..4f46ee6 100644 --- a/pkg/codec/deserializer_test.go +++ b/pkg/codec/deserializer_test.go @@ -2,6 +2,7 @@ package codec_test import ( "encoding/json" + admissionv1 "k8s.io/api/admission/v1" "reflect" "github.com/dbsystel/kewl/pkg/panicutils" @@ -38,24 +39,29 @@ var _ = Describe("Deserializer", func() { BeforeEach(func() { sut = codec.NewDeserializer(runtime.NewScheme()) }) - It("should throw an error if object is unknown", func() { + It("should return an error if object is unknown", func() { deserialize, err := sut.Deserialize(schema.GroupVersionKind{Group: "meh", Version: "meh", Kind: "meh"}, nil) Expect(deserialize).To(BeNil()) Expect(err).To(HaveOccurred()) }) - It("should throw an error if the object is known, but cannot be deserialized", func() { + It("should return an error if the object is known, but cannot be deserialized", func() { Expect(sut.Register(&corev1Extension{})).To(Not(HaveOccurred())) deserialize, err := sut.Deserialize(gvk(corev1.SchemeGroupVersion, corev1.Pod{}), []byte("meh")) Expect(deserialize).To(BeNil()) Expect(err).To(HaveOccurred()) }) - It("should throw an error when the scheme extension is nil", func() { + It("should return an error when the scheme extension is nil", func() { Expect(sut.Register(nil)).To(HaveOccurred()) }) It("should deserialize an object correctly", func() { Expect(sut.Register(&corev1Extension{})).To(Not(HaveOccurred())) Expect(sut.Deserialize(gvk(corev1.SchemeGroupVersion, corev1.Pod{}), marshalJSON(&corev1.Pod{}))).Should(Not(BeNil())) }) + It("should return an error in case an invalid object was provided", func() { + Expect(sut.Register(&corev1Extension{})).To(Not(HaveOccurred())) + _, err := sut.Deserialize(gvk(corev1.SchemeGroupVersion, admissionv1.AdmissionReview{}), marshalJSON(&corev1.Pod{})) + Expect(err).To(HaveOccurred()) + }) It("should propagate not registered correctly", func() { Expect(sut.Register(&corev1Extension{})).To(Not(HaveOccurred())) _, err := sut.Deserialize(schema.GroupVersionKind{}, marshalJSON(&corev1.Pod{})) diff --git a/pkg/webhook/facade/api.go b/pkg/webhook/facade/api.go index 3ce2bca..cde2442 100644 --- a/pkg/webhook/facade/api.go +++ b/pkg/webhook/facade/api.go @@ -82,10 +82,10 @@ func AdmissionReviewFrom(bytes []byte) (AdmissionReview, error) { return nil, fmt.Errorf("%v - %v: %v", InvalidAdmissionReviewMsg, "invalid object GroupVersionKind", kind) } if kind.Group == v1.SchemeGroupVersion.Group && kind.Version == v1.SchemeGroupVersion.Version { - return v1AdmissionReviewFromBytes(bytes) + return V1AdmissionReviewFromBytes(bytes) } if kind.Group == v1beta1.SchemeGroupVersion.Group && kind.Version == v1beta1.SchemeGroupVersion.Version { - return v1beta1AdmissionReviewFromBytes(bytes) + return V1beta1AdmissionReviewFromBytes(bytes) } return nil, fmt.Errorf("could not create facade for: %v", kind) } diff --git a/pkg/webhook/facade/v1_admission_review.go b/pkg/webhook/facade/v1_admission_review.go index c03a2b5..264e3fd 100644 --- a/pkg/webhook/facade/v1_admission_review.go +++ b/pkg/webhook/facade/v1_admission_review.go @@ -12,7 +12,7 @@ import ( var v1PatchTypeJSONPatch = v1.PatchTypeJSONPatch -func v1AdmissionReviewFromBytes(bytes []byte) (AdmissionReview, error) { +func V1AdmissionReviewFromBytes(bytes []byte) (AdmissionReview, error) { target := &v1.AdmissionReview{} if err := json.Unmarshal(bytes, target); err != nil { return nil, errors.Wrap(err, "got an admission review v1, but could not serialize it") @@ -78,16 +78,15 @@ func (v *v1AdmissionReviewRequest) Resource() metav1.GroupVersionResource { return v.target.Resource } -func (v v1AdmissionReviewRequest) Version() string { - return v1.SchemeGroupVersion.Version -} - // Response decorator functions var _ AdmissionResponse = &v1AdmissionReview{} func (v *v1AdmissionReview) withResponse(handler func(response *v1.AdmissionResponse)) { if v.target.Response == nil { - v.target.Response = &v1.AdmissionResponse{} + if v.target.Request == nil { + return + } + v.target.Response = &v1.AdmissionResponse{UID: v.target.Request.UID} } handler(v.target.Response) } diff --git a/pkg/webhook/facade/v1_admission_review_test.go b/pkg/webhook/facade/v1_admission_review_test.go index 9602d46..e79186f 100644 --- a/pkg/webhook/facade/v1_admission_review_test.go +++ b/pkg/webhook/facade/v1_admission_review_test.go @@ -2,7 +2,6 @@ package facade_test import ( - "github.com/dbsystel/kewl/pkg/panicutils" "github.com/dbsystel/kewl/pkg/webhook/facade" "github.com/dbsystel/kewl/testing/admission_test" "github.com/dbsystel/kewl/testing/validation_test" @@ -13,15 +12,24 @@ import ( ) var _ = Describe("v1AdmissionReview test", func() { + var review *admission_test.V1AdmissionReview var sut facade.AdmissionReview BeforeEach(func() { - review, err := facade.AdmissionReviewFrom(admission_test.V1ValidPod().MustMarshal()) - panicutils.PanicIfError(err) - sut = review + review = admission_test.V1ValidPod() + var err error + sut, err = facade.AdmissionReviewFrom(review.MustMarshal()) + Expect(err).NotTo(HaveOccurred()) }) v1AdmissionReview := func() *v1.AdmissionReview { return K8sAdmissionReview(sut, &v1.AdmissionReview{}).(*v1.AdmissionReview) } + It("should return decode err", func() { + _, err := facade.V1AdmissionReviewFromBytes([]byte("bla")) + Expect(err).To(HaveOccurred()) + }) + It("should return the version correctly", func() { + Expect(sut.Version()).To(Equal(v1.SchemeGroupVersion.Version)) + }) It("should facade the request correctly", func() { expected := admission_test.V1ValidPod().Request Expect(sut.Request().Kind()).To(BeEquivalentTo(expected.Kind)) @@ -36,6 +44,7 @@ var _ = Describe("v1AdmissionReview test", func() { sut.Response().Allow() result := v1AdmissionReview() Expect(result.Response).NotTo(BeNil()) + Expect(result.Response.UID).To(Equal(review.Request.UID)) Expect(result.Response.Allowed).To(BeTrue()) Expect(sut.Response().ResponseType()).To(Equal(facade.AdmissionAllowed)) }) @@ -52,6 +61,7 @@ var _ = Describe("v1AdmissionReview test", func() { sut.Response().Deny(status) result := v1AdmissionReview() Expect(result.Response).NotTo(BeNil()) + Expect(result.Response.UID).To(Equal(review.Request.UID)) Expect(result.Response.Allowed).To(BeFalse()) Expect(result.Response.Result).To(BeEquivalentTo(status)) Expect(sut.Response().ResponseType()).To(Equal(facade.AdmissionDenied)) @@ -60,6 +70,7 @@ var _ = Describe("v1AdmissionReview test", func() { sut.Response().PatchJSON([]byte("{}")) result := v1AdmissionReview() Expect(result.Response).NotTo(BeNil()) + Expect(result.Response.UID).To(Equal(review.Request.UID)) Expect(result.Response.Allowed).To(BeTrue()) Expect(result.Response.PatchType).NotTo(BeNil()) Expect(result.Response.Patch).NotTo(BeEmpty()) @@ -74,5 +85,11 @@ var _ = Describe("v1AdmissionReview test", func() { sut.Response().Deny(nil) Expect(sut.Response().IsSet()).To(BeTrue()) }) + It("should not handle response if request is nil", func() { + Expect(sut.Response().IsSet()).To(BeFalse()) + sut.ClearRequest() + sut.Response().Allow() + Expect(sut.Response().IsSet()).To(BeFalse()) + }) }) }) diff --git a/pkg/webhook/facade/v1beta1_admission_review.go b/pkg/webhook/facade/v1beta1_admission_review.go index 23859e3..af8f873 100644 --- a/pkg/webhook/facade/v1beta1_admission_review.go +++ b/pkg/webhook/facade/v1beta1_admission_review.go @@ -12,7 +12,7 @@ import ( var v1beta1PatchTypeJSONPatch = v1beta1.PatchTypeJSONPatch -func v1beta1AdmissionReviewFromBytes(bytes []byte) (AdmissionReview, error) { +func V1beta1AdmissionReviewFromBytes(bytes []byte) (AdmissionReview, error) { target := &v1beta1.AdmissionReview{} if err := json.Unmarshal(bytes, target); err != nil { return nil, errors.Wrap(err, "got an admission review v1beta1, but could not serialize it") @@ -78,16 +78,15 @@ func (v *v1beta1AdmissionReviewRequest) Resource() metav1.GroupVersionResource { return v.target.Resource } -func (v v1beta1AdmissionReviewRequest) Version() string { - return v1beta1.SchemeGroupVersion.Version -} - // Response decorator functions var _ AdmissionResponse = &v1beta1AdmissionReview{} func (v *v1beta1AdmissionReview) withResponse(handler func(response *v1beta1.AdmissionResponse)) { if v.target.Response == nil { - v.target.Response = &v1beta1.AdmissionResponse{} + if v.target.Request == nil { + return + } + v.target.Response = &v1beta1.AdmissionResponse{UID: v.target.Request.UID} } handler(v.target.Response) } diff --git a/pkg/webhook/facade/v1beta1_admission_review_test.go b/pkg/webhook/facade/v1beta1_admission_review_test.go index 6358b4f..fdf622b 100644 --- a/pkg/webhook/facade/v1beta1_admission_review_test.go +++ b/pkg/webhook/facade/v1beta1_admission_review_test.go @@ -2,7 +2,6 @@ package facade_test import ( - "github.com/dbsystel/kewl/pkg/panicutils" "github.com/dbsystel/kewl/pkg/webhook/facade" "github.com/dbsystel/kewl/testing/admission_test" "github.com/dbsystel/kewl/testing/validation_test" @@ -13,15 +12,24 @@ import ( ) var _ = Describe("v1beta1AdmissionReview test", func() { + var review *admission_test.V1Beta1AdmissionReview var sut facade.AdmissionReview BeforeEach(func() { - review, err := facade.AdmissionReviewFrom(admission_test.V1Beta1ValidPod().MustMarshal()) - panicutils.PanicIfError(err) - sut = review + review = admission_test.V1Beta1ValidPod() + var err error + sut, err = facade.AdmissionReviewFrom(review.MustMarshal()) + Expect(err).NotTo(HaveOccurred()) }) v1beta1AdmissionReview := func() *v1beta1.AdmissionReview { return K8sAdmissionReview(sut, &v1beta1.AdmissionReview{}).(*v1beta1.AdmissionReview) } + It("should return decode err", func() { + _, err := facade.V1beta1AdmissionReviewFromBytes([]byte("bla")) + Expect(err).To(HaveOccurred()) + }) + It("should return the version correctly", func() { + Expect(sut.Version()).To(Equal(v1beta1.SchemeGroupVersion.Version)) + }) It("should facade the request correctly", func() { expected := admission_test.V1Beta1ValidPod().Request Expect(sut.Request().Kind()).To(BeEquivalentTo(expected.Kind)) @@ -36,6 +44,7 @@ var _ = Describe("v1beta1AdmissionReview test", func() { sut.Response().Allow() result := v1beta1AdmissionReview() Expect(result.Response).NotTo(BeNil()) + Expect(result.Response.UID).To(Equal(review.Request.UID)) Expect(result.Response.Allowed).To(BeTrue()) Expect(sut.Response().ResponseType()).To(Equal(facade.AdmissionAllowed)) }) @@ -52,6 +61,7 @@ var _ = Describe("v1beta1AdmissionReview test", func() { sut.Response().Deny(status) result := v1beta1AdmissionReview() Expect(result.Response).NotTo(BeNil()) + Expect(result.Response.UID).To(Equal(review.Request.UID)) Expect(result.Response.Allowed).To(BeFalse()) Expect(result.Response.Result).To(BeEquivalentTo(status)) Expect(sut.Response().ResponseType()).To(Equal(facade.AdmissionDenied)) @@ -60,6 +70,7 @@ var _ = Describe("v1beta1AdmissionReview test", func() { sut.Response().PatchJSON([]byte("{}")) result := v1beta1AdmissionReview() Expect(result.Response).NotTo(BeNil()) + Expect(result.Response.UID).To(Equal(review.Request.UID)) Expect(result.Response.Allowed).To(BeTrue()) Expect(result.Response.PatchType).NotTo(BeNil()) Expect(result.Response.Patch).NotTo(BeEmpty()) @@ -74,5 +85,11 @@ var _ = Describe("v1beta1AdmissionReview test", func() { sut.Response().Deny(nil) Expect(sut.Response().IsSet()).To(BeTrue()) }) + It("should not handle response if request is nil", func() { + Expect(sut.Response().IsSet()).To(BeFalse()) + sut.ClearRequest() + sut.Response().Allow() + Expect(sut.Response().IsSet()).To(BeFalse()) + }) }) }) diff --git a/testing/admission_test/v1.go b/testing/admission_test/v1.go index 683ae86..bbdd22a 100644 --- a/testing/admission_test/v1.go +++ b/testing/admission_test/v1.go @@ -7,6 +7,7 @@ import ( v1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/uuid" ) // V1TypeMeta is the metav1.TypeMeta for v1.AdmissionReview @@ -31,6 +32,7 @@ func NewV1Review(obj, oldObj testing.Reviewable) func() *V1AdmissionReview { return &V1AdmissionReview{ TypeMeta: V1TypeMeta, Request: &v1.AdmissionRequest{ + UID: uuid.NewUUID(), Kind: metav1.GroupVersionKind{Group: kind.Group, Version: kind.Version, Kind: kind.Kind}, Resource: metav1.GroupVersionResource{ Group: kind.Group, diff --git a/testing/admission_test/v1beta1.go b/testing/admission_test/v1beta1.go index 362c293..9c42ceb 100644 --- a/testing/admission_test/v1beta1.go +++ b/testing/admission_test/v1beta1.go @@ -6,6 +6,7 @@ import ( "github.com/dbsystel/kewl/testing/json_test" "k8s.io/api/admission/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/uuid" ) // V1Beta1TypeMeta is the metav1.TypeMeta for v1beta1.AdmissionReview @@ -29,6 +30,7 @@ func NewV1Beta1Review(obj, oldObj testing.Reviewable) func() *V1Beta1AdmissionRe return &V1Beta1AdmissionReview{ TypeMeta: V1Beta1TypeMeta, Request: &v1beta1.AdmissionRequest{ + UID: uuid.NewUUID(), Kind: metav1.GroupVersionKind{Group: kind.Group, Version: kind.Version, Kind: kind.Kind}, Resource: metav1.GroupVersionResource{ Group: kind.Group, From 565d615437cfa2a09bedf8b3ea091681081b7228 Mon Sep 17 00:00:00 2001 From: Christian Heike Date: Wed, 9 Jun 2021 07:58:50 +0200 Subject: [PATCH 2/3] Get rid of the tar errs extracting the pkgs in golangci action --- .github/workflows/go.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 94445ce..368ea61 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -26,7 +26,9 @@ jobs: with: version: v1.40 only-new-issues: true - + skip-go-installation: true + skip-pkg-cache: true + - name: Build run: go build -v ./pkg/... From 6de4c7bb19af9574ed7e2a377e07cbebb40552c6 Mon Sep 17 00:00:00 2001 From: Christian Heike Date: Wed, 9 Jun 2021 07:59:21 +0200 Subject: [PATCH 3/3] Fixed linter errs --- pkg/codec/deserializer_test.go | 3 ++- testing/admission_test/v1.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/codec/deserializer_test.go b/pkg/codec/deserializer_test.go index 4f46ee6..ddd8736 100644 --- a/pkg/codec/deserializer_test.go +++ b/pkg/codec/deserializer_test.go @@ -2,9 +2,10 @@ package codec_test import ( "encoding/json" - admissionv1 "k8s.io/api/admission/v1" "reflect" + admissionv1 "k8s.io/api/admission/v1" + "github.com/dbsystel/kewl/pkg/panicutils" "github.com/dbsystel/kewl/pkg/codec" diff --git a/testing/admission_test/v1.go b/testing/admission_test/v1.go index bbdd22a..d565508 100644 --- a/testing/admission_test/v1.go +++ b/testing/admission_test/v1.go @@ -32,7 +32,7 @@ func NewV1Review(obj, oldObj testing.Reviewable) func() *V1AdmissionReview { return &V1AdmissionReview{ TypeMeta: V1TypeMeta, Request: &v1.AdmissionRequest{ - UID: uuid.NewUUID(), + UID: uuid.NewUUID(), Kind: metav1.GroupVersionKind{Group: kind.Group, Version: kind.Version, Kind: kind.Kind}, Resource: metav1.GroupVersionResource{ Group: kind.Group,