Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

⚠️ pkg/webhook/admission: upgrade v1beta1 admission types to v1 #1284

Merged
merged 1 commit into from
Dec 3, 2020
Merged

⚠️ pkg/webhook/admission: upgrade v1beta1 admission types to v1 #1284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
419 changes: 217 additions & 202 deletions pkg/builder/webhook_test.go
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions pkg/webhook/admission/decode_test.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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