Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Commit

Permalink
chore: webhook functions now return AdmissionResponse
Browse files Browse the repository at this point in the history 
Having these functions modify the AdmissionResponse passed in as
parameter makes it hard to reason about the code.

controller.webhook.util.Unmarshal was removed for the sake of not
having to return nil in case of success. This adds a bit more code to
the calling functions but at the same time helps bring across the
intended behaviour.
  • Loading branch information
Max Jonas Werner committed Sep 28, 2020
1 parent 9f45a5a commit 01e60d9
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 59 deletions.
31 changes: 21 additions & 10 deletions pkg/controller/webhook/federatedtypeconfig/webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ package federatedtypeconfig

import (
"context"
"encoding/json"
"net/http"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
Expand All @@ -38,29 +42,36 @@ type FederatedTypeConfigAdmissionHook struct{}
var _ admission.Handler = &FederatedTypeConfigAdmissionHook{}

func (a *FederatedTypeConfigAdmissionHook) Handle(ctx context.Context, admissionSpec admission.Request) admission.Response {
status := admission.Response{}

klog.V(4).Infof("Validating %q AdmissionRequest = %s", ResourceName, webhook.AdmissionRequestDebugString(admissionSpec))

// We want to let through:
// - Requests that are not for create, update
// - Requests for things that are not FederatedTypeConfigs
if webhook.Allowed(admissionSpec, resourcePluralName, &status) {
return status
if webhook.Allowed(admissionSpec, resourcePluralName) {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: true,
},
}
}

admittingObject := &v1beta1.FederatedTypeConfig{}
err := webhook.Unmarshal(&admissionSpec.Object, admittingObject, &status)
if err != nil {
return status
if err := json.Unmarshal(admissionSpec.Object.Raw, admittingObject); err != nil {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: err.Error(),
},
},
}
}

klog.V(4).Infof("Validating %q = %+v", ResourceName, *admittingObject)

isStatusSubResource := len(admissionSpec.SubResource) != 0
webhook.Validate(&status, func() field.ErrorList {
return webhook.Validate(func() field.ErrorList {
return validation.ValidateFederatedTypeConfig(admittingObject, isStatusSubResource)
})

return status
}
31 changes: 21 additions & 10 deletions pkg/controller/webhook/kubefedcluster/webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ package kubefedcluster

import (
"context"
"encoding/json"
"net/http"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
Expand All @@ -38,29 +42,36 @@ type KubeFedClusterAdmissionHook struct{}
var _ admission.Handler = &KubeFedClusterAdmissionHook{}

func (a *KubeFedClusterAdmissionHook) Handle(ctx context.Context, admissionSpec admission.Request) admission.Response {
status := admission.Response{}

klog.V(4).Infof("Validating %q AdmissionRequest = %s", ResourceName, webhook.AdmissionRequestDebugString(admissionSpec))

// We want to let through:
// - Requests that are not for create, update
// - Requests for things that are not FederatedTypeConfigs
if webhook.Allowed(admissionSpec, resourcePluralName, &status) {
return status
if webhook.Allowed(admissionSpec, resourcePluralName) {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: true,
},
}
}

admittingObject := &v1beta1.KubeFedCluster{}
err := webhook.Unmarshal(&admissionSpec.Object, admittingObject, &status)
if err != nil {
return status
if err := json.Unmarshal(admissionSpec.Object.Raw, admittingObject); err != nil {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: err.Error(),
},
},
}
}

klog.V(4).Infof("Validating %q = %+v", ResourceName, *admittingObject)

isStatusSubResource := admissionSpec.SubResource == "status"
webhook.Validate(&status, func() field.ErrorList {
return webhook.Validate(func() field.ErrorList {
return validation.ValidateKubeFedCluster(admittingObject, isStatusSubResource)
})

return status
}
53 changes: 37 additions & 16 deletions pkg/controller/webhook/kubefedconfig/webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,36 +45,50 @@ type KubeFedConfigValidator struct{}
var _ admission.Handler = &KubeFedConfigValidator{}

func (a *KubeFedConfigValidator) Handle(ctx context.Context, admissionSpec admission.Request) admission.Response {
status := admission.Response{}

klog.V(4).Infof("Validating %q AdmissionRequest = %s", ResourceName, webhook.AdmissionRequestDebugString(admissionSpec))

if webhook.Allowed(admissionSpec, resourcePluralName, &status) {
return status
if webhook.Allowed(admissionSpec, resourcePluralName) {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: true,
},
}
}

admittingObject := &v1beta1.KubeFedConfig{}
err := webhook.Unmarshal(&admissionSpec.Object, admittingObject, &status)
if err != nil {
return status
if err := json.Unmarshal(admissionSpec.Object.Raw, admittingObject); err != nil {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: err.Error(),
},
},
}
}

var oldObject *v1beta1.KubeFedConfig
if admissionSpec.Operation == admissionv1beta1.Update {
oldObject = &v1beta1.KubeFedConfig{}
err = webhook.Unmarshal(&admissionSpec.OldObject, oldObject, &status)
if err != nil {
return status
if err := json.Unmarshal(admissionSpec.OldObject.Raw, oldObject); err != nil {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: err.Error(),
},
},
}
}
}

klog.V(4).Infof("Validating %q = %+v", ResourceName, *admittingObject)

webhook.Validate(&status, func() field.ErrorList {
return webhook.Validate(func() field.ErrorList {
return validation.ValidateKubeFedConfig(admittingObject, oldObject)
})

return status
}

type KubeFedConfigDefaulter struct{}
Expand All @@ -86,9 +100,16 @@ func (a *KubeFedConfigDefaulter) Handle(ctx context.Context, admissionSpec admis
klog.V(4).Infof("Admitting %q AdmissionRequest = %s", ResourceName, webhook.AdmissionRequestDebugString(admissionSpec))

admittingObject := &v1beta1.KubeFedConfig{}
err := webhook.Unmarshal(&admissionSpec.Object, admittingObject, &status)
if err != nil {
return status
if err := json.Unmarshal(admissionSpec.Object.Raw, admittingObject); err != nil {
return admission.Response{
AdmissionResponse: admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: err.Error(),
},
},
}
}

klog.V(4).Infof("Admitting %q = %+v", ResourceName, *admittingObject)
Expand Down
35 changes: 12 additions & 23 deletions pkg/controller/webhook/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ import (
"fmt"
"net/http"

"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

admissionv1beta1 "k8s.io/api/admission/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"sigs.k8s.io/kubefed/pkg/apis/core/v1beta1"
)
Expand Down Expand Up @@ -56,43 +55,33 @@ func NewValidatingResource(resourcePluralName string) schema.GroupVersionResourc

// Allowed returns true if the admission request for the plural name of the
// resource passed in should be allowed to pass through, false otherwise.
func Allowed(a admission.Request, pluralResourceName string, status *admission.Response) bool {
func Allowed(a admission.Request, pluralResourceName string) bool {
// We want to let through:
// - Requests that are not for create, update
// - Requests for things that are not <pluralResourceName>
createOrUpdate := a.Operation == admissionv1beta1.Create || a.Operation == admissionv1beta1.Update
isMyGroupAndResource := a.Resource.Group == v1beta1.SchemeGroupVersion.Group && a.Resource.Resource == pluralResourceName
if !createOrUpdate || !isMyGroupAndResource {
status.Allowed = true
return true
}
return false
return !createOrUpdate || !isMyGroupAndResource
}

func Unmarshal(rawExt *runtime.RawExtension, object interface{}, status *admission.Response) error {
err := json.Unmarshal(rawExt.Raw, object)
if err != nil {
status.Allowed = false
status.Result = &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: err.Error(),
}
}

return err
func Unmarshal(rawExt *runtime.RawExtension, object interface{}) error {
return json.Unmarshal(rawExt.Raw, object)
}

func Validate(status *admission.Response, validateFn func() field.ErrorList) {
func Validate(validateFn func() field.ErrorList) admission.Response {
res := admission.Response{}
errs := validateFn()
if len(errs) != 0 {
status.Allowed = false
status.Result = &metav1.Status{
res.Allowed = false
res.Result = &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusForbidden, Reason: metav1.StatusReasonForbidden,
Message: errs.ToAggregate().Error(),
}
} else {
status.Allowed = true
res.Allowed = true
}

return res
}

func AdmissionRequestDebugString(a admission.Request) string {
Expand Down

0 comments on commit 01e60d9

Please sign in to comment.