diff --git a/pkg/apis/admissionregistration/fuzzer/fuzzer.go b/pkg/apis/admissionregistration/fuzzer/fuzzer.go index ba4f2c5e5bf..c68785abaa9 100644 --- a/pkg/apis/admissionregistration/fuzzer/fuzzer.go +++ b/pkg/apis/admissionregistration/fuzzer/fuzzer.go @@ -32,6 +32,10 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { obj.FailurePolicy = &p s := admissionregistration.SideEffectClassUnknown obj.SideEffects = &s + if obj.TimeoutSeconds == nil { + i := int32(30) + obj.TimeoutSeconds = &i + } }, } } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index 9a51b6631b3..90d31fd4199 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -26,6 +26,8 @@ import ( func strPtr(s string) *string { return &s } +func int32Ptr(i int32) *int32 { return &i } + func newValidatingWebhookConfiguration(hooks []admissionregistration.Webhook) *admissionregistration.ValidatingWebhookConfiguration { return &admissionregistration.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{ @@ -544,6 +546,63 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) { }), expectedError: `clientConfig.service.path: Invalid value: "/apis/foo.bar/v1alpha1/--bad": segment[3]: a DNS-1123 subdomain`, }, + { + name: "timeout seconds cannot be greater than 30", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(31), + }, + }), + expectedError: `webhooks[0].timeoutSeconds: Invalid value: 31: the timeout value must be between 1 and 30 seconds`, + }, + { + name: "timeout seconds cannot be smaller than 1", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(0), + }, + }), + expectedError: `webhooks[0].timeoutSeconds: Invalid value: 0: the timeout value must be between 1 and 30 seconds`, + }, + { + name: "timeout seconds must be positive", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(-1), + }, + }), + expectedError: `webhooks[0].timeoutSeconds: Invalid value: -1: the timeout value must be between 1 and 30 seconds`, + }, + { + name: "valid timeout seconds", + config: newValidatingWebhookConfiguration( + []admissionregistration.Webhook{ + { + Name: "webhook.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(1), + }, + { + Name: "webhook2.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(15), + }, + { + Name: "webhook3.k8s.io", + ClientConfig: validClientConfig, + TimeoutSeconds: int32Ptr(30), + }, + }), + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/test/e2e/apimachinery/webhook.go b/test/e2e/apimachinery/webhook.go index 92de1b41096..09b5d2c1652 100644 --- a/test/e2e/apimachinery/webhook.go +++ b/test/e2e/apimachinery/webhook.go @@ -64,6 +64,7 @@ const ( dummyValidatingWebhookConfigName = "e2e-test-dummy-validating-webhook-config" dummyMutatingWebhookConfigName = "e2e-test-dummy-mutating-webhook-config" crdWebhookConfigName = "e2e-test-webhook-config-crd" + slowWebhookConfigName = "e2e-test-webhook-config-slow" skipNamespaceLabelKey = "skip-webhook-admission" skipNamespaceLabelValue = "yes" @@ -201,6 +202,31 @@ var _ = SIGDescribe("AdmissionWebhook", func() { testCRDDenyWebhook(f) }) + It("Should honor timeout", func() { + policyFail := v1beta1.Fail + policyIgnore := v1beta1.Ignore + + By("Setting timeout (1s) shorter than webhook latency (5s)") + slowWebhookCleanup := registerSlowWebhook(f, context, &policyFail, int32Ptr(1)) + testSlowWebhookTimeoutFailEarly(f) + slowWebhookCleanup() + + By("Having no error when timeout is shorter than webhook latency and failure policy is ignore") + slowWebhookCleanup = registerSlowWebhook(f, context, &policyIgnore, int32Ptr(1)) + testSlowWebhookTimeoutNoError(f) + slowWebhookCleanup() + + By("Having no error when timeout is longer than webhook latency") + slowWebhookCleanup = registerSlowWebhook(f, context, &policyFail, int32Ptr(10)) + testSlowWebhookTimeoutNoError(f) + slowWebhookCleanup() + + By("Having no error when timeout is empty (defaulted to 10s in v1beta1)") + slowWebhookCleanup = registerSlowWebhook(f, context, &policyFail, nil) + testSlowWebhookTimeoutNoError(f) + slowWebhookCleanup() + }) + // TODO: add more e2e tests for mutating webhooks // 1. mutating webhook that mutates pod // 2. mutating webhook that sends empty patch @@ -357,6 +383,8 @@ func deployWebhookAndService(f *framework.Framework, image string, context *cert func strPtr(s string) *string { return &s } +func int32Ptr(i int32) *int32 { return &i } + func registerWebhook(f *framework.Framework, context *certContext) func() { client := f.ClientSet By("Registering the webhook via the AdmissionRegistration API") @@ -1445,3 +1473,69 @@ func testCRDDenyWebhook(f *framework.Framework) { framework.Failf("expect error contains %q, got %q", expectedErrMsg, err.Error()) } } + +func registerSlowWebhook(f *framework.Framework, context *certContext, policy *v1beta1.FailurePolicyType, timeout *int32) func() { + client := f.ClientSet + By("Registering slow webhook via the AdmissionRegistration API") + + namespace := f.Namespace.Name + configName := slowWebhookConfigName + + _, err := client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Create(&v1beta1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: configName, + }, + Webhooks: []v1beta1.Webhook{ + { + Name: "allow-configmap-with-delay-webhook.k8s.io", + Rules: []v1beta1.RuleWithOperations{{ + Operations: []v1beta1.OperationType{v1beta1.Create}, + Rule: v1beta1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"configmaps"}, + }, + }}, + ClientConfig: v1beta1.WebhookClientConfig{ + Service: &v1beta1.ServiceReference{ + Namespace: namespace, + Name: serviceName, + Path: strPtr("/always-allow-delay-5s"), + }, + CABundle: context.signingCert, + }, + FailurePolicy: policy, + TimeoutSeconds: timeout, + }, + }, + }) + framework.ExpectNoError(err, "registering slow webhook config %s with namespace %s", configName, namespace) + + // The webhook configuration is honored in 10s. + time.Sleep(10 * time.Second) + + return func() { + client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Delete(configName, nil) + } +} + +func testSlowWebhookTimeoutFailEarly(f *framework.Framework) { + By("Request fails when timeout (1s) is shorter than slow webhook latency (5s)") + client := f.ClientSet + name := "e2e-test-slow-webhook-configmap" + _, err := client.CoreV1().ConfigMaps(f.Namespace.Name).Create(&v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: name}}) + Expect(err).To(HaveOccurred(), "create configmap in namespace %s should have timed-out reaching slow webhook", f.Namespace.Name) + expectedErrMsg := `/always-allow-delay-5s?timeout=1s: context deadline exceeded` + if !strings.Contains(err.Error(), expectedErrMsg) { + framework.Failf("expect error contains %q, got %q", expectedErrMsg, err.Error()) + } +} + +func testSlowWebhookTimeoutNoError(f *framework.Framework) { + client := f.ClientSet + name := "e2e-test-slow-webhook-configmap" + _, err := client.CoreV1().ConfigMaps(f.Namespace.Name).Create(&v1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: name}}) + Expect(err).To(BeNil()) + err = client.CoreV1().ConfigMaps(f.Namespace.Name).Delete(name, &metav1.DeleteOptions{}) + Expect(err).To(BeNil()) +}