Skip to content

Commit

Permalink
Merge pull request #784 from yaa110/timeout-seconds
Browse files Browse the repository at this point in the history
✨ Adds new option `timeoutSeconds` to inform the the timeout for webhook resources
  • Loading branch information
k8s-ci-robot committed May 15, 2023
2 parents 8a6a861 + 33b1f0a commit 784dec2
Show file tree
Hide file tree
Showing 16 changed files with 301 additions and 16 deletions.
35 changes: 34 additions & 1 deletion pkg/webhook/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ limitations under the License.
//
// The markers take the form:
//
// +kubebuilder:webhook:webhookVersions=<[]string>,failurePolicy=<string>,matchPolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>,sideEffects=<string>,admissionReviewVersions=<[]string>,reinvocationPolicy=<string>
// +kubebuilder:webhook:webhookVersions=<[]string>,failurePolicy=<string>,matchPolicy=<string>,groups=<[]string>,resources=<[]string>,verbs=<[]string>,versions=<[]string>,name=<string>,path=<string>,mutating=<bool>,sideEffects=<string>,timeoutSeconds=<int>,admissionReviewVersions=<[]string>,reinvocationPolicy=<string>
package webhook

import (
Expand Down Expand Up @@ -81,6 +81,11 @@ type Config struct {
// If the value is "NoneOnDryRun", then the webhook is responsible for inspecting the "dryRun" property of the
// AdmissionReview sent in the request, and avoiding side effects if that value is "true."
SideEffects string `marker:",optional"`
// TimeoutSeconds allows configuring how long the API server should wait for a webhook to respond before treating the call as a failure.
// If the timeout expires before the webhook responds, the webhook call will be ignored or the API call will be rejected based on the failure policy.
// The timeout value must be between 1 and 30 seconds.
// The timeout for an admission webhook defaults to 10 seconds.
TimeoutSeconds int `marker:",optional"`

// Groups specifies the API groups that this webhook receives requests for.
Groups []string
Expand Down Expand Up @@ -159,6 +164,7 @@ func (c Config) ToMutatingWebhook() (admissionregv1.MutatingWebhook, error) {
MatchPolicy: matchPolicy,
ClientConfig: c.clientConfig(),
SideEffects: c.sideEffects(),
TimeoutSeconds: c.timeoutSeconds(),
AdmissionReviewVersions: c.AdmissionReviewVersions,
ReinvocationPolicy: c.reinvocationPolicy(),
}, nil
Expand All @@ -182,6 +188,7 @@ func (c Config) ToValidatingWebhook() (admissionregv1.ValidatingWebhook, error)
MatchPolicy: matchPolicy,
ClientConfig: c.clientConfig(),
SideEffects: c.sideEffects(),
TimeoutSeconds: c.timeoutSeconds(),
AdmissionReviewVersions: c.AdmissionReviewVersions,
}, nil
}
Expand Down Expand Up @@ -273,6 +280,15 @@ func (c Config) sideEffects() *admissionregv1.SideEffectClass {
return &sideEffects
}

// timeoutSeconds returns the timeoutSeconds config for a webhook.
func (c Config) timeoutSeconds() *int32 {
if c.TimeoutSeconds != 0 {
timeoutSeconds := int32(c.TimeoutSeconds)
return &timeoutSeconds
}
return nil
}

// reinvocationPolicy returns the reinvocationPolicy config for a mutating webhook.
func (c Config) reinvocationPolicy() *admissionregv1.ReinvocationPolicyType {
var reinvocationPolicy admissionregv1.ReinvocationPolicyType
Expand Down Expand Up @@ -381,6 +397,11 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
if err := checkSideEffectsForV1(objRaw.Webhooks[i].SideEffects); err != nil {
return err
}
// TimeoutSeconds must be nil or between 1 and 30 seconds, otherwise,
// return an error
if err := checkTimeoutSeconds(objRaw.Webhooks[i].TimeoutSeconds); err != nil {
return err
}
// AdmissionReviewVersions is required in admissionregistration/v1, if this is not set,
// return an error
if len(objRaw.Webhooks[i].AdmissionReviewVersions) == 0 {
Expand All @@ -407,6 +428,11 @@ func (g Generator) Generate(ctx *genall.GenerationContext) error {
if err := checkSideEffectsForV1(objRaw.Webhooks[i].SideEffects); err != nil {
return err
}
// TimeoutSeconds must be nil or between 1 and 30 seconds, otherwise,
// return an error
if err := checkTimeoutSeconds(objRaw.Webhooks[i].TimeoutSeconds); err != nil {
return err
}
// AdmissionReviewVersions is required in admissionregistration/v1, if this is not set,
// return an error
if len(objRaw.Webhooks[i].AdmissionReviewVersions) == 0 {
Expand Down Expand Up @@ -451,3 +477,10 @@ func checkSideEffectsForV1(sideEffects *admissionregv1.SideEffectClass) error {
}
return nil
}

func checkTimeoutSeconds(timeoutSeconds *int32) error {
if timeoutSeconds != nil && (*timeoutSeconds < 1 || *timeoutSeconds > 30) {
return fmt.Errorf("TimeoutSeconds must be between 1 and 30 seconds")
}
return nil
}
29 changes: 29 additions & 0 deletions pkg/webhook/parser_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,35 @@ var _ = Describe("Webhook Generation From Parsing to CustomResourceDefinition",
Expect(err).To(MatchError("SideEffects should not be set to `Some` or `Unknown` for v1 {Mutating,Validating}WebhookConfiguration"))
})

It("should fail with invalid timeout seconds", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expect(err).NotTo(HaveOccurred())
Expect(os.Chdir("./testdata/invalid-timeoutSeconds")).To(Succeed()) // go modules are directory-sensitive
defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }()

By("loading the roots")
pkgs, err := loader.LoadRoots(".")
Expect(err).NotTo(HaveOccurred())
Expect(pkgs).To(HaveLen(1))

By("setting up the parser")
reg := &markers.Registry{}
Expect(reg.Register(webhook.ConfigDefinition)).To(Succeed())

By("requesting that the manifest be generated")
outputDir, err := ioutil.TempDir("", "webhook-integration-test")
Expect(err).NotTo(HaveOccurred())
defer os.RemoveAll(outputDir)
genCtx := &genall.GenerationContext{
Collector: &markers.Collector{Registry: reg},
Roots: pkgs,
OutputRule: genall.OutputToDirectory(outputDir),
}
err = webhook.Generator{}.Generate(genCtx)
Expect(err).To(MatchError("TimeoutSeconds must be between 1 and 30 seconds"))
})

It("should properly generate the webhook definition", func() {
By("switching into testdata to appease go modules")
cwd, err := os.Getwd()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ webhooks:
resources:
- cronjobs
sideEffects: None
timeoutSeconds: 10
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
Expand Down Expand Up @@ -54,6 +55,7 @@ webhooks:
resources:
- cronjobs
sideEffects: None
timeoutSeconds: 10
- admissionReviewVersions:
- v1
- v1beta1
Expand All @@ -76,3 +78,4 @@ webhooks:
resources:
- cronjobs
sideEffects: NoneOnDryRun
timeoutSeconds: 10
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func (c *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
Complete()
}

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=None
// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=NoneOnDryRun,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=None,timeoutSeconds=10
// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=NoneOnDryRun,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1

var _ webhook.Defaulter = &CronJob{}
var _ webhook.Validator = &CronJob{}
Expand Down
3 changes: 3 additions & 0 deletions pkg/webhook/testdata/invalid-sideEffects/manifests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ webhooks:
resources:
- cronjobs
sideEffects: None
timeoutSeconds: 10
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
Expand Down Expand Up @@ -54,6 +55,7 @@ webhooks:
resources:
- cronjobs
sideEffects: None
timeoutSeconds: 10
- admissionReviewVersions:
- v1
- v1beta1
Expand All @@ -76,3 +78,4 @@ webhooks:
resources:
- cronjobs
sideEffects: NoneOnDryRun
timeoutSeconds: 10
6 changes: 3 additions & 3 deletions pkg/webhook/testdata/invalid-sideEffects/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ func (c *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
Complete()
}

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=Some,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=NoneOnDryRun,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=Some,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=NoneOnDryRun,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1

var _ webhook.Defaulter = &CronJob{}
var _ webhook.Validator = &CronJob{}
Expand Down
71 changes: 71 additions & 0 deletions pkg/webhook/testdata/invalid-timeoutSeconds/cronjob_types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

//go:generate ../../../../.run-controller-gen.sh webhook paths=. output:dir=.

// +groupName=testdata.kubebuilder.io
// +versionName=v1
package cronjob

import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.

// CronJobSpec defines the desired state of CronJob
type CronJobSpec struct {
// The schedule in Cron format, see https://en.wikipedia.org/wiki/Cron.
Schedule string `json:"schedule"`
}

// CronJobStatus defines the observed state of CronJob
type CronJobStatus struct {
// INSERT ADDITIONAL STATUS FIELD - define observed state of cluster
// Important: Run "make" to regenerate code after modifying this file

// Information when was the last time the job was successfully scheduled.
// +optional
LastScheduleTime *metav1.Time `json:"lastScheduleTime,omitempty"`
}

// +kubebuilder:object:root=true
// +kubebuilder:subresource:status
// +kubebuilder:resource:singular=mycronjob

// CronJob is the Schema for the cronjobs API
type CronJob struct {
/*
*/
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

Spec CronJobSpec `json:"spec,omitempty"`
Status CronJobStatus `json:"status,omitempty"`
}

// +kubebuilder:object:root=true

// CronJobList contains a list of CronJob
type CronJobList struct {
metav1.TypeMeta `json:",inline"`
metav1.ListMeta `json:"metadata,omitempty"`
Items []CronJob `json:"items"`
}

func init() {
SchemeBuilder.Register(&CronJob{}, &CronJobList{})
}
83 changes: 83 additions & 0 deletions pkg/webhook/testdata/invalid-timeoutSeconds/manifests.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
---
apiVersion: admissionregistration.k8s.io/v1
kind: MutatingWebhookConfiguration
metadata:
creationTimestamp: null
name: mutating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /mutate-testdata-kubebuilder-io-v1-cronjob
failurePolicy: Fail
matchPolicy: Equivalent
name: default.cronjob.testdata.kubebuilder.io
rules:
- apiGroups:
- testdata.kubebuiler.io
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- cronjobs
sideEffects: None
timeoutSeconds: 40
---
apiVersion: admissionregistration.k8s.io/v1
kind: ValidatingWebhookConfiguration
metadata:
creationTimestamp: null
name: validating-webhook-configuration
webhooks:
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-testdata-kubebuilder-io-v1-cronjob
failurePolicy: Fail
matchPolicy: Equivalent
name: validation.cronjob.testdata.kubebuilder.io
rules:
- apiGroups:
- testdata.kubebuiler.io
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- cronjobs
sideEffects: None
timeoutSeconds: 10
- admissionReviewVersions:
- v1
- v1beta1
clientConfig:
service:
name: webhook-service
namespace: system
path: /validate-testdata-kubebuilder-io-v1-cronjob
failurePolicy: Fail
matchPolicy: Equivalent
name: validation.cronjob.testdata.kubebuilder.io
rules:
- apiGroups:
- testdata.kubebuiler.io
apiVersions:
- v1
operations:
- CREATE
- UPDATE
resources:
- cronjobs
sideEffects: None
timeoutSeconds: -1
50 changes: 50 additions & 0 deletions pkg/webhook/testdata/invalid-timeoutSeconds/webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package cronjob

import (
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)

func (c *CronJob) SetupWebhookWithManager(mgr ctrl.Manager) error {
return ctrl.NewWebhookManagedBy(mgr).
For(c).
Complete()
}

// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=None,timeoutSeconds=40,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:verbs=create;update,path=/validate-testdata-kubebuilder-io-v1-cronjob,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=validation.cronjob.testdata.kubebuilder.io,sideEffects=None,timeoutSeconds=10,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:webhookVersions=v1,verbs=create;update,path=/mutate-testdata-kubebuilder-io-v1-cronjob,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=testdata.kubebuiler.io,resources=cronjobs,versions=v1,name=default.cronjob.testdata.kubebuilder.io,sideEffects=None,timeoutSeconds=-1,admissionReviewVersions=v1;v1beta1

var _ webhook.Defaulter = &CronJob{}
var _ webhook.Validator = &CronJob{}

func (c *CronJob) Default() {
}

func (c *CronJob) ValidateCreate() error {
return nil
}

func (c *CronJob) ValidateUpdate(_ runtime.Object) error {
return nil
}

func (c *CronJob) ValidateDelete() error {
return nil
}

0 comments on commit 784dec2

Please sign in to comment.