Skip to content

Commit

Permalink
Merge pull request #1284 from estroz/feature/wh-admissionreview-v1
Browse files Browse the repository at this point in the history 
⚠️ pkg/webhook/admission: upgrade v1beta1 admission types to v1
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Dec 3, 2020
2 parents f349d96 + 8161d1c commit 758d268
Show file tree
Hide file tree
Showing 12 changed files with 318 additions and 292 deletions.
419 changes: 217 additions & 202 deletions pkg/builder/webhook_test.go
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pkg/webhook/admission/decode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
admissionv1 "k8s.io/api/admission/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand All @@ -39,7 +39,7 @@ var _ = Describe("Admission Webhook Decoder", func() {
})

req := Request{
AdmissionRequest: admissionv1beta1.AdmissionRequest{
AdmissionRequest: admissionv1.AdmissionRequest{
Object: runtime.RawExtension{
Raw: []byte(`{
"apiVersion": "v1",
Expand Down
26 changes: 19 additions & 7 deletions pkg/webhook/admission/http.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"io/ioutil"
"net/http"

v1 "k8s.io/api/admission/v1"
"k8s.io/api/admission/v1beta1"
admissionv1beta1 "k8s.io/api/admission/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
Expand All @@ -35,7 +35,8 @@ var admissionScheme = runtime.NewScheme()
var admissionCodecs = serializer.NewCodecFactory(admissionScheme)

func init() {
utilruntime.Must(admissionv1beta1.AddToScheme(admissionScheme))
utilruntime.Must(v1.AddToScheme(admissionScheme))
utilruntime.Must(v1beta1.AddToScheme(admissionScheme))
}

var _ http.Handler = &Webhook{}
Expand Down Expand Up @@ -70,11 +71,15 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
return
}

// Both v1 and v1beta1 AdmissionReview types are exactly the same, so the v1beta1 type can
// be decoded into the v1 type. However the runtime codec's decoder guesses which type to
// decode into by type name if an Object's TypeMeta isn't set. By setting TypeMeta of an`
// unregistered type to the v1 GVK, the decoder will coerce a v1beta1 AdmissionReview to v1.
req := Request{}
ar := v1beta1.AdmissionReview{
// avoid an extra copy
Request: &req.AdmissionRequest,
}
ar := unversionedAdmissionReview{}
// avoid an extra copy
ar.Request = &req.AdmissionRequest
ar.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("AdmissionReview"))
if _, _, err := admissionCodecs.UniversalDeserializer().Decode(body, nil, &ar); err != nil {
wh.log.Error(err, "unable to decode the request")
reviewResponse = Errored(http.StatusBadRequest, err)
Expand All @@ -90,7 +95,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {

func (wh *Webhook) writeResponse(w io.Writer, response Response) {
encoder := json.NewEncoder(w)
responseAdmissionReview := v1beta1.AdmissionReview{
responseAdmissionReview := v1.AdmissionReview{
Response: &response.AdmissionResponse,
}
err := encoder.Encode(responseAdmissionReview)
Expand All @@ -107,3 +112,10 @@ func (wh *Webhook) writeResponse(w io.Writer, response Response) {
}
}
}

// unversionedAdmissionReview is used to decode both v1 and v1beta1 AdmissionReview types.
type unversionedAdmissionReview struct {
v1.AdmissionReview
}

var _ runtime.Object = &unversionedAdmissionReview{}
6 changes: 3 additions & 3 deletions pkg/webhook/admission/http_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
admissionv1 "k8s.io/api/admission/v1"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
)

var _ = Describe("Admission Webhooks", func() {
Expand Down Expand Up @@ -155,7 +155,7 @@ func (h *fakeHandler) Handle(ctx context.Context, req Request) Response {
if h.fn != nil {
return h.fn(ctx, req)
}
return Response{AdmissionResponse: admissionv1beta1.AdmissionResponse{
return Response{AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
}}
}
15 changes: 8 additions & 7 deletions pkg/webhook/admission/multi.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ import (
"fmt"
"net/http"

"gomodules.xyz/jsonpatch/v2"
admissionv1beta1 "k8s.io/api/admission/v1beta1"
jsonpatch "gomodules.xyz/jsonpatch/v2"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
)

Expand All @@ -37,10 +38,10 @@ func (hs multiMutating) Handle(ctx context.Context, req Request) Response {
if !resp.Allowed {
return resp
}
if resp.PatchType != nil && *resp.PatchType != admissionv1beta1.PatchTypeJSONPatch {
if resp.PatchType != nil && *resp.PatchType != admissionv1.PatchTypeJSONPatch {
return Errored(http.StatusInternalServerError,
fmt.Errorf("unexpected patch type returned by the handler: %v, only allow: %v",
resp.PatchType, admissionv1beta1.PatchTypeJSONPatch))
resp.PatchType, admissionv1.PatchTypeJSONPatch))
}
patches = append(patches, resp.Patches...)
}
Expand All @@ -50,13 +51,13 @@ func (hs multiMutating) Handle(ctx context.Context, req Request) Response {
return Errored(http.StatusBadRequest, fmt.Errorf("error when marshaling the patch: %w", err))
}
return Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
},
Patch: marshaledPatch,
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
},
}
}
Expand Down Expand Up @@ -94,7 +95,7 @@ func (hs multiValidating) Handle(ctx context.Context, req Request) Response {
}
}
return Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand Down
16 changes: 8 additions & 8 deletions pkg/webhook/admission/multi_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"gomodules.xyz/jsonpatch/v2"
admissionv1beta1 "k8s.io/api/admission/v1beta1"
jsonpatch "gomodules.xyz/jsonpatch/v2"
admissionv1 "k8s.io/api/admission/v1"
)

var _ = Describe("Multi-Handler Admission Webhooks", func() {
alwaysAllow := &fakeHandler{
fn: func(ctx context.Context, req Request) Response {
return Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
},
}
Expand All @@ -39,7 +39,7 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
alwaysDeny := &fakeHandler{
fn: func(ctx context.Context, req Request) Response {
return Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
},
}
Expand Down Expand Up @@ -82,9 +82,9 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
Value: "2",
},
},
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
},
}
},
Expand All @@ -99,9 +99,9 @@ var _ = Describe("Multi-Handler Admission Webhooks", func() {
Value: "world",
},
},
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
},
}
},
Expand Down
15 changes: 7 additions & 8 deletions pkg/webhook/admission/response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ package admission
import (
"net/http"

"gomodules.xyz/jsonpatch/v2"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
jsonpatch "gomodules.xyz/jsonpatch/v2"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -50,7 +49,7 @@ func Patched(reason string, patches ...jsonpatch.JsonPatchOperation) Response {
// Errored creates a new Response for error-handling a request.
func Errored(code int32, err error) Response {
return Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: code,
Expand All @@ -67,7 +66,7 @@ func ValidationResponse(allowed bool, reason string) Response {
code = http.StatusOK
}
resp := Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: allowed,
Result: &metav1.Status{
Code: int32(code),
Expand All @@ -90,17 +89,17 @@ func PatchResponseFromRaw(original, current []byte) Response {
}
return Response{
Patches: patches,
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
},
}
}

// validationResponseFromStatus returns a response for admitting a request with provided Status object.
func validationResponseFromStatus(allowed bool, status metav1.Status) Response {
resp := Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: allowed,
Result: &status,
},
Expand Down
34 changes: 17 additions & 17 deletions pkg/webhook/admission/response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import (

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"gomodules.xyz/jsonpatch/v2"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
jsonpatch "gomodules.xyz/jsonpatch/v2"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -33,7 +33,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should return an 'allowed' response", func() {
Expect(Allowed("")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -46,7 +46,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should populate a status with a reason when a reason is given", func() {
Expect(Allowed("acceptable")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -62,7 +62,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should return a 'not allowed' response", func() {
Expect(Denied("")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusForbidden,
Expand All @@ -75,7 +75,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should populate a status with a reason when a reason is given", func() {
Expect(Denied("UNACCEPTABLE!")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusForbidden,
Expand All @@ -102,7 +102,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should return an 'allowed' response with the given patches", func() {
Expect(Patched("", ops...)).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -115,7 +115,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should populate a status with a reason when a reason is given", func() {
Expect(Patched("some changes", ops...)).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -132,7 +132,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
It("should return a denied response with an error", func() {
err := errors.New("this is an error")
expected := Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusBadRequest,
Expand All @@ -150,7 +150,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
By("checking that a message is populated for 'allowed' responses")
Expect(ValidationResponse(true, "acceptable")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -163,7 +163,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
By("checking that a message is populated for 'denied' responses")
Expect(ValidationResponse(false, "UNACCEPTABLE!")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusForbidden,
Expand All @@ -178,7 +178,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
By("checking that it returns an 'allowed' response when allowed is true")
Expect(ValidationResponse(true, "")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -190,7 +190,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
By("checking that it returns an 'denied' response when allowed is false")
Expect(ValidationResponse(false, "")).To(Equal(
Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Code: http.StatusForbidden,
Expand All @@ -207,9 +207,9 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
Patches: []jsonpatch.JsonPatchOperation{
{Operation: "replace", Path: "/a", Value: "bar"},
},
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
PatchType: func() *admissionv1beta1.PatchType { pt := admissionv1beta1.PatchTypeJSONPatch; return &pt }(),
PatchType: func() *admissionv1.PatchType { pt := admissionv1.PatchTypeJSONPatch; return &pt }(),
},
}
resp := PatchResponseFromRaw([]byte(`{"a": "foo"}`), []byte(`{"a": "bar"}`))
Expand All @@ -220,7 +220,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
Describe("WithWarnings", func() {
It("should add the warnings to the existing response without removing any existing warnings", func() {
initialResponse := Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand All @@ -230,7 +230,7 @@ var _ = Describe("Admission Webhook Response Helpers", func() {
}
warnings := []string{"additional-warning-1", "additional-warning-2"}
expectedResponse := Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
AdmissionResponse: admissionv1.AdmissionResponse{
Allowed: true,
Result: &metav1.Status{
Code: http.StatusOK,
Expand Down

0 comments on commit 758d268

Please sign in to comment.