From 71ef46fa122d1fc3bba587640451b697e59f1903 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 19 Aug 2019 11:23:05 -0400 Subject: [PATCH] Use lesser of context or webhook-specific timeout in webhooks Kubernetes-commit: c63284b1f3996e7830c1aca85281d349d0091c82 --- .../plugin/webhook/mutating/dispatcher.go | 33 +++++++++++++++++-- .../plugin/webhook/validating/dispatcher.go | 31 +++++++++++++++-- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/pkg/admission/plugin/webhook/mutating/dispatcher.go b/pkg/admission/plugin/webhook/mutating/dispatcher.go index cf0e76a60..8c75fdcf2 100644 --- a/pkg/admission/plugin/webhook/mutating/dispatcher.go +++ b/pkg/admission/plugin/webhook/mutating/dispatcher.go @@ -136,7 +136,15 @@ func (a *mutatingDispatcher) Dispatch(ctx context.Context, attr admission.Attrib if ignoreClientCallFailures { klog.Warningf("Failed calling webhook, failing open %v: %v", hook.Name, callErr) utilruntime.HandleError(callErr) - continue + + select { + case <-ctx.Done(): + // parent context is canceled or timed out, no point in continuing + return apierrors.NewTimeoutError("request did not complete within requested timeout", 0) + default: + // individual webhook timed out, but parent context did not, continue + continue + } } klog.Warningf("Failed calling webhook, failing closed %v: %v", hook.Name, err) return apierrors.NewInternalError(err) @@ -181,10 +189,29 @@ func (a *mutatingDispatcher) callAttrMutatingHook(ctx context.Context, h *v1beta utiltrace.Field{"operation", attr.GetOperation()}, utiltrace.Field{"UID", uid}) defer trace.LogIfLong(500 * time.Millisecond) - r := client.Post().Context(ctx).Body(request) + + // if the webhook has a specific timeout, wrap the context to apply it if h.TimeoutSeconds != nil { - r = r.Timeout(time.Duration(*h.TimeoutSeconds) * time.Second) + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, time.Duration(*h.TimeoutSeconds)*time.Second) + defer cancel() } + + r := client.Post().Context(ctx).Body(request) + + // if the context has a deadline, set it as a parameter to inform the backend + if deadline, hasDeadline := ctx.Deadline(); hasDeadline { + // compute the timeout + if timeout := time.Until(deadline); timeout > 0 { + // if it's not an even number of seconds, round up to the nearest second + if truncated := timeout.Truncate(time.Second); truncated != timeout { + timeout = truncated + time.Second + } + // set the timeout + r.Timeout(timeout) + } + } + if err := r.Do().Into(response); err != nil { return false, &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} } diff --git a/pkg/admission/plugin/webhook/validating/dispatcher.go b/pkg/admission/plugin/webhook/validating/dispatcher.go index 6525524f5..3e1cb589a 100644 --- a/pkg/admission/plugin/webhook/validating/dispatcher.go +++ b/pkg/admission/plugin/webhook/validating/dispatcher.go @@ -80,6 +80,14 @@ func (d *validatingDispatcher) Dispatch(ctx context.Context, attr admission.Attr return nil } + // Check if the request has already timed out before spawning remote calls + select { + case <-ctx.Done(): + // parent context is canceled or timed out, no point in continuing + return apierrors.NewTimeoutError("request did not complete within requested timeout", 0) + default: + } + wg := sync.WaitGroup{} errCh := make(chan error, len(relevantHooks)) wg.Add(len(relevantHooks)) @@ -162,10 +170,29 @@ func (d *validatingDispatcher) callHook(ctx context.Context, h *v1beta1.Validati utiltrace.Field{"operation", attr.GetOperation()}, utiltrace.Field{"UID", uid}) defer trace.LogIfLong(500 * time.Millisecond) - r := client.Post().Context(ctx).Body(request) + + // if the webhook has a specific timeout, wrap the context to apply it if h.TimeoutSeconds != nil { - r = r.Timeout(time.Duration(*h.TimeoutSeconds) * time.Second) + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, time.Duration(*h.TimeoutSeconds)*time.Second) + defer cancel() } + + r := client.Post().Context(ctx).Body(request) + + // if the context has a deadline, set it as a parameter to inform the backend + if deadline, hasDeadline := ctx.Deadline(); hasDeadline { + // compute the timeout + if timeout := time.Until(deadline); timeout > 0 { + // if it's not an even number of seconds, round up to the nearest second + if truncated := timeout.Truncate(time.Second); truncated != timeout { + timeout = truncated + time.Second + } + // set the timeout + r.Timeout(timeout) + } + } + if err := r.Do().Into(response); err != nil { return &webhookutil.ErrCallingWebhook{WebhookName: h.Name, Reason: err} }