Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

⚠️ support delete validation in validator interface #525

Merged
merged 1 commit into from
Jul 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions examples/crd/pkg/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ func (c *ChaosPod) ValidateUpdate(old runtime.Object) error {
return nil
}

// ValidateDelete implements webhookutil.validator so a webhook will be registered for the type
func (c *ChaosPod) ValidateDelete() error {
log.Info("validate delete", "name", c.Name)

if c.Spec.NextStop.Before(&metav1.Time{Time: time.Now()}) {
return fmt.Errorf(".spec.nextStop must be later than current time")
}
return nil
}

// +kubebuilder:webhook:path=/mutate-chaosapps-metamagical-io-v1-chaospod,mutating=true,failurePolicy=fail,groups=chaosapps.metamagical.io,resources=chaospods,verbs=create;update,versions=v1,name=mchaospod.kb.io

var _ webhook.Defaulter = &ChaosPod{}
Expand Down
58 changes: 32 additions & 26 deletions pkg/builder/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,37 +96,43 @@ func (blder *WebhookBuilder) registerWebhooks() error {

// registerDefaultingWebhook registers a defaulting webhook if th
func (blder *WebhookBuilder) registerDefaultingWebhook() {
if defaulter, isDefaulter := blder.apiType.(admission.Defaulter); isDefaulter {
mwh := admission.DefaultingWebhookFor(defaulter)
if mwh != nil {
path := generateMutatePath(blder.gvk)

// Checking if the path is already registered.
// If so, just skip it.
if !blder.isAlreadyHandled(path) {
log.Info("Registering a mutating webhook",
"GVK", blder.gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, mwh)
}
defaulter, isDefaulter := blder.apiType.(admission.Defaulter)
if !isDefaulter {
log.Info("skip registering a mutating webhook, admission.Defaulter interface is not implemented", "GVK", blder.gvk)
return
}
mwh := admission.DefaultingWebhookFor(defaulter)
if mwh != nil {
path := generateMutatePath(blder.gvk)

// Checking if the path is already registered.
// If so, just skip it.
if !blder.isAlreadyHandled(path) {
log.Info("Registering a mutating webhook",
"GVK", blder.gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, mwh)
}
}
}

func (blder *WebhookBuilder) registerValidatingWebhook() {
if validator, isValidator := blder.apiType.(admission.Validator); isValidator {
vwh := admission.ValidatingWebhookFor(validator)
if vwh != nil {
path := generateValidatePath(blder.gvk)

// Checking if the path is already registered.
// If so, just skip it.
if !blder.isAlreadyHandled(path) {
log.Info("Registering a validating webhook",
"GVK", blder.gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, vwh)
}
validator, isValidator := blder.apiType.(admission.Validator)
if !isValidator {
log.Info("skip registering a validating webhook, admission.Validator interface is not implemented", "GVK", blder.gvk)
return
}
vwh := admission.ValidatingWebhookFor(validator)
if vwh != nil {
path := generateValidatePath(blder.gvk)

// Checking if the path is already registered.
// If so, just skip it.
if !blder.isAlreadyHandled(path) {
log.Info("Registering a validating webhook",
"GVK", blder.gvk,
"path", path)
blder.mgr.GetWebhookServer().Register(path, vwh)
}
}
}
Expand Down
111 changes: 111 additions & 0 deletions pkg/builder/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,103 @@ var _ = Describe("application", func() {
Expect(w.Body).To(ContainSubstring(`"allowed":true`))
Expect(w.Body).To(ContainSubstring(`"code":200`))
})

It("should scaffold a validating webhook if the type implements the Validator interface to validate deletes", func() {
By("creating a controller manager")
m, err := manager.New(cfg, manager.Options{})
Expect(err).NotTo(HaveOccurred())

By("registering the type in the Scheme")
builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()}
builder.Register(&TestValidator{}, &TestValidatorList{})
err = builder.AddToScheme(m.GetScheme())
Expect(err).NotTo(HaveOccurred())

err = WebhookManagedBy(m).
For(&TestValidator{}).
Complete()
Expect(err).NotTo(HaveOccurred())
svr := m.GetWebhookServer()
Expect(svr).NotTo(BeNil())

reader := strings.NewReader(`{
"kind":"AdmissionReview",
"apiVersion":"admission.k8s.io/v1beta1",
"request":{
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
"kind":{
"group":"",
"version":"v1",
"kind":"TestValidator"
},
"resource":{
"group":"",
"version":"v1",
"resource":"testvalidator"
},
"namespace":"default",
"operation":"DELETE",
"object":null,
"oldObject":{
"replica":1
}
}
}`)
stopCh := make(chan struct{})
close(stopCh)
// TODO: we may want to improve it to make it be able to inject dependencies,
// but not always try to load certs and return not found error.
err = svr.Start(stopCh)
if err != nil && !os.IsNotExist(err) {
Expect(err).NotTo(HaveOccurred())
}

By("sending a request to a validating webhook path to check for failed delete")
path := generateValidatePath(testValidatorGVK)
req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json")
w := httptest.NewRecorder()
svr.WebhookMux.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusOK))
By("sanity checking the response contains reasonable field")
Expect(w.Body).To(ContainSubstring(`"allowed":false`))
Expect(w.Body).To(ContainSubstring(`"code":403`))

reader = strings.NewReader(`{
"kind":"AdmissionReview",
"apiVersion":"admission.k8s.io/v1beta1",
"request":{
"uid":"07e52e8d-4513-11e9-a716-42010a800270",
"kind":{
"group":"",
"version":"v1",
"kind":"TestValidator"
},
"resource":{
"group":"",
"version":"v1",
"resource":"testvalidator"
},
"namespace":"default",
"operation":"DELETE",
"object":null,
"oldObject":{
"replica":0
}
}
}`)
By("sending a request to a validating webhook path with correct request")
path = generateValidatePath(testValidatorGVK)
req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader)
req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json")
w = httptest.NewRecorder()
svr.WebhookMux.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusOK))
By("sanity checking the response contains reasonable field")
Expect(w.Body).To(ContainSubstring(`"allowed":true`))
Expect(w.Body).To(ContainSubstring(`"code":200`))

})
})
})

Expand Down Expand Up @@ -357,6 +454,13 @@ func (v *TestValidator) ValidateUpdate(old runtime.Object) error {
return nil
}

func (v *TestValidator) ValidateDelete() error {
if v.Replica > 0 {
return errors.New("number of replica should be less than or equal to 0 to delete")
}
return nil
}

// TestDefaultValidator
var _ runtime.Object = &TestDefaultValidator{}

Expand Down Expand Up @@ -407,3 +511,10 @@ func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) error {
}
return nil
}

func (dv *TestDefaultValidator) ValidateDelete() error {
if dv.Replica > 0 {
return errors.New("number of replica should be less than or equal to 0 to delete")
}
return nil
}
15 changes: 15 additions & 0 deletions pkg/webhook/admission/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type Validator interface {
runtime.Object
ValidateCreate() error
ValidateUpdate(old runtime.Object) error
ValidateDelete() error
}

// ValidatingWebhookFor creates a new Webhook for validating the provided type.
Expand Down Expand Up @@ -89,5 +90,19 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response {
}
}

if req.Operation == v1beta1.Delete {
// In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346
// OldObject contains the object being deleted
err := h.decoder.DecodeRaw(req.OldObject, obj)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

req.OldObject is an empty object when kube-apiserver is v1.14 or earlier.
obj will be a object with zero values everywhere.
DecodeRaw really should return an error when there is an empty object.
I created #529. We should merge that first.

if err != nil {
return Errored(http.StatusBadRequest, err)
}

err = obj.ValidateDelete()
if err != nil {
return Denied(err.Error())
}
}

return Allowed("")
}