Skip to content

Commit

Permalink
Simplify design and error on multiple mutation mechanisms
Browse files Browse the repository at this point in the history
  • Loading branch information
alculquicondor committed Sep 24, 2024
1 parent ea977b9 commit 44738ee
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 32 deletions.
39 changes: 25 additions & 14 deletions pkg/builder/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import (
// WebhookBuilder builds a Webhook.
type WebhookBuilder struct {
apiType runtime.Object
mutatorFactory admission.HandlerFactory
mutationHandler admission.Handler
customDefaulter admission.CustomDefaulter
customValidator admission.CustomValidator
gvk schema.GroupVersionKind
Expand Down Expand Up @@ -66,9 +66,9 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder {
return blder
}

// WithMutatorFactory takes an admission.HandlerFactory, a MutatingWebhook will be wired for the handler that this factory creates.
func (blder *WebhookBuilder) WithMutatorFactory(factory admission.HandlerFactory) *WebhookBuilder {
blder.mutatorFactory = factory
// WithMutationHandler takes an admission.Handler, a MutatingWebhook will be wired for it.
func (blder *WebhookBuilder) WithMutationHandler(h admission.Handler) *WebhookBuilder {
blder.mutationHandler = h
return blder
}

Expand Down Expand Up @@ -147,7 +147,9 @@ func (blder *WebhookBuilder) registerWebhooks() error {
}

// Register webhook(s) for type
blder.registerDefaultingWebhook()
if err := blder.registerDefaultingWebhook(); err != nil {
return err
}
blder.registerValidatingWebhook()

err = blder.registerConversionWebhook()
Expand All @@ -158,8 +160,11 @@ func (blder *WebhookBuilder) registerWebhooks() error {
}

// registerDefaultingWebhook registers a defaulting webhook if necessary.
func (blder *WebhookBuilder) registerDefaultingWebhook() {
mwh := blder.getDefaultingWebhook()
func (blder *WebhookBuilder) registerDefaultingWebhook() error {
mwh, err := blder.getDefaultingWebhook()
if err != nil {
return err
}
if mwh != nil {
mwh.LogConstructor = blder.logConstructor
path := generateMutatePath(blder.gvk)
Expand All @@ -173,21 +178,27 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
blder.mgr.GetWebhookServer().Register(path, mwh)
}
}
return nil
}

func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
func (blder *WebhookBuilder) getDefaultingWebhook() (*admission.Webhook, error) {
var w *admission.Webhook
if factory := blder.mutatorFactory; factory != nil {
w = admission.WithHandlerFactory(blder.mgr.GetScheme(), blder.apiType, factory)
} else if defaulter := blder.customDefaulter; defaulter != nil {
if handler := blder.mutationHandler; handler != nil {
w = &admission.Webhook{Handler: handler}
}
if defaulter := blder.customDefaulter; defaulter != nil {
if w != nil {
return nil, errors.New("A WebhookBuilder can only define a MutationHandler or a Defaulter, but not both")

Check failure on line 191 in pkg/builder/webhook.go

View workflow job for this annotation

GitHub Actions / lint

ST1005: error strings should not be capitalized (stylecheck)
}
w = admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter)
} else {
return nil
}
if w == nil {
return nil, nil
}
if blder.recoverPanic != nil {
w = w.WithRecoverPanic(*blder.recoverPanic)
}
return w
return w, nil
}

// registerValidatingWebhook registers a validating webhook if necessary.
Expand Down
8 changes: 1 addition & 7 deletions pkg/builder/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ func runTests(admissionReviewVersion string) {
ExpectWithOffset(1, err).NotTo(HaveOccurred())

err = WebhookManagedBy(m).
WithMutatorFactory(mutatorFactoryForTestDefaulter(m.GetScheme())).
For(&TestDefaulter{}).
WithMutationHandler(admission.WithCustomDefaulter(m.GetScheme(), &TestDefaulter{}, &TestCustomDefaulter{})).
WithLogConstructor(func(base logr.Logger, req *admission.Request) logr.Logger {
return admission.DefaultLogConstructor(testingLogger, req)
}).
Expand Down Expand Up @@ -668,12 +668,6 @@ func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) err

var _ admission.CustomDefaulter = &TestCustomDefaulter{}

func mutatorFactoryForTestDefaulter(scheme *runtime.Scheme) admission.HandlerFactory {
return func(obj runtime.Object, _ admission.Decoder) admission.Handler {
return admission.WithCustomDefaulter(scheme, obj, &TestCustomDefaulter{}).Handler
}
}

// TestCustomValidator.

type TestCustomValidator struct{}
Expand Down
11 changes: 0 additions & 11 deletions pkg/webhook/admission/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"gomodules.xyz/jsonpatch/v2"
admissionv1 "k8s.io/api/admission/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/json"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -96,9 +95,6 @@ func (r *Response) Complete(req Request) error {
return nil
}

// HandlerFactory can create a Handler for the given type.
type HandlerFactory func(obj runtime.Object, decoder Decoder) Handler

// Handler can handle an AdmissionRequest.
type Handler interface {
// Handle yields a response to an AdmissionRequest.
Expand All @@ -118,13 +114,6 @@ func (f HandlerFunc) Handle(ctx context.Context, req Request) Response {
return f(ctx, req)
}

// WithHandlerFactory creates a new Webhook for a handler factory.
func WithHandlerFactory(scheme *runtime.Scheme, obj runtime.Object, factory HandlerFactory) *Webhook {
return &Webhook{
Handler: factory(obj, NewDecoder(scheme)),
}
}

// Webhook represents each individual webhook.
//
// It must be registered with a webhook.Server or
Expand Down

0 comments on commit 44738ee

Please sign in to comment.