Skip to content

Commit

Permalink
Skip webhook validation on delete for v1beta1
Browse files Browse the repository at this point in the history
This commit implements SupportedVerbs for each Triggers API type,
specifying which API operations validation should be performed for.
Instead of Knative's default (create, update, and delete), this commit
updates validation to be performed only for create and update.
This allows resources to be deleted from the cluster even if they are no
longer valid.
  • Loading branch information
lbernick authored and tekton-robot committed Mar 1, 2023
1 parent 5b593e7 commit 74b4749
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 136 deletions.
8 changes: 0 additions & 8 deletions docs/triggers-api.md
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
<!--
---
title: Triggers API
linkTitle: Triggers API
weight: 1000
---
-->

<p>Packages:</p>
<ul>
<li>
Expand Down
12 changes: 9 additions & 3 deletions pkg/apis/triggers/v1beta1/cluster_trigger_binding_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@ import (
"context"

"github.com/tektoncd/pipeline/pkg/apis/validate"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"knative.dev/pkg/apis"
"knative.dev/pkg/webhook/resourcesemantics"
)

var _ resourcesemantics.VerbLimited = (*ClusterTriggerBinding)(nil)

// SupportedVerbs returns the operations that validation should be called for
func (ctb *ClusterTriggerBinding) SupportedVerbs() []admissionregistrationv1.OperationType {
return []admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}
}

func (ctb *ClusterTriggerBinding) Validate(ctx context.Context) *apis.FieldError {
if apis.IsInDelete(ctx) {
return nil
}
if err := validate.ObjectMetadata(ctb.GetObjectMeta()); err != nil {
return err.ViaField("metadata")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,12 @@ package v1beta1_test

import (
"context"
"strings"
"testing"

"github.com/tektoncd/triggers/pkg/apis/triggers/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/apis"
)

func Test_ClusterTriggerBindingValidate_OnDelete(t *testing.T) {
tb := &v1beta1.ClusterTriggerBinding{
ObjectMeta: metav1.ObjectMeta{
Name: strings.Repeat("foo", 64), // Length should be lower than 63
},
Spec: v1beta1.TriggerBindingSpec{
Params: []v1beta1.Param{{
Name: "param1",
Value: "$(body.input1)",
}, {
Name: "param2",
Value: "$(body.input2)",
}},
},
}
err := tb.Validate(apis.WithinDelete(context.Background()))
if err != nil {
t.Errorf("TriggerBinding.Validate() on Delete expected no error, but got one, TriggerBinding: %v, error: %v", tb, err)
}
}

func Test_ClusterTriggerBindingValidate(t *testing.T) {
tests := []struct {
name string
Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/triggers/v1beta1/event_listener_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ import (
"fmt"

"github.com/tektoncd/triggers/pkg/apis/triggers"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/webhook/resourcesemantics"
)

var (
Expand All @@ -37,12 +39,15 @@ var (
)
)

var _ resourcesemantics.VerbLimited = (*EventListener)(nil)

// SupportedVerbs returns the operations that validation should be called for
func (e *EventListener) SupportedVerbs() []admissionregistrationv1.OperationType {
return []admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}
}

// Validate EventListener.
func (e *EventListener) Validate(ctx context.Context) *apis.FieldError {
if apis.IsInDelete(ctx) {
return nil
}

var errs *apis.FieldError
if len(e.ObjectMeta.Name) > 60 {
// Since `el-` is added as the prefix of EventListener services, the name of EventListener must be no more than 60 characters long.
Expand Down
21 changes: 0 additions & 21 deletions pkg/apis/triggers/v1beta1/event_listener_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1beta1_test

import (
"context"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -40,26 +39,6 @@ var myObjectMeta = metav1.ObjectMeta{
Namespace: "namespace",
}

func Test_EventListenerValidate_OnDelete(t *testing.T) {
el := &triggersv1beta1.EventListener{
ObjectMeta: metav1.ObjectMeta{
Name: strings.Repeat("foo", 64), // Length should be lower than 63
Namespace: "namespace",
},
Spec: triggersv1beta1.EventListenerSpec{
Triggers: []triggersv1beta1.EventListenerTrigger{{
Template: &triggersv1beta1.EventListenerTemplate{
Ref: ptr.String(""),
},
}},
},
}
err := el.Validate(apis.WithinDelete(context.Background()))
if err != nil {
t.Errorf("EventListener.Validate() on Delete expected no error, but got one, EventListener: %v, error: %v", el, err)
}
}

func Test_EventListenerValidate(t *testing.T) {
ctxWithAlphaFieldsEnabled, err := test.FeatureFlagsToContext(context.Background(), map[string]string{
"enable-api-fields": "alpha",
Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/triggers/v1beta1/trigger_binding_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,22 @@ import (
"strings"

"github.com/tektoncd/pipeline/pkg/apis/validate"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/apis"
"knative.dev/pkg/webhook/resourcesemantics"
)

var _ resourcesemantics.VerbLimited = (*TriggerBinding)(nil)

// SupportedVerbs returns the operations that validation should be called for
func (tb *TriggerBinding) SupportedVerbs() []admissionregistrationv1.OperationType {
return []admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}
}

// Validate TriggerBinding.
func (tb *TriggerBinding) Validate(ctx context.Context) *apis.FieldError {
if apis.IsInDelete(ctx) {
return nil
}

errs := validate.ObjectMetadata(tb.GetObjectMeta()).ViaField("metadata")
return errs.Also(tb.Spec.Validate(ctx).ViaField("spec"))
}
Expand Down
24 changes: 0 additions & 24 deletions pkg/apis/triggers/v1beta1/trigger_binding_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,37 +18,13 @@ package v1beta1_test

import (
"context"
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/triggers/pkg/apis/triggers/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/apis"
)

func Test_TriggerBindingValidate_OnDelete(t *testing.T) {
tb := &v1beta1.TriggerBinding{
ObjectMeta: metav1.ObjectMeta{
Name: strings.Repeat("foo", 64), // Length should be lower than 63
Namespace: "namespace",
},
Spec: v1beta1.TriggerBindingSpec{
Params: []v1beta1.Param{{
Name: "param1",
Value: "$(body.input1)",
}, {
Name: "param1",
Value: "$(body.input2)",
}},
},
}
err := tb.Validate(apis.WithinDelete(context.Background()))
if err != nil {
t.Errorf("TriggerBinding.Validate() on Delete expected no error, but got one, TriggerBinding: %v, error: %v", tb, err)
}
}

func Test_TriggerBindingValidate(t *testing.T) {
tests := []struct {
name string
Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/triggers/v1beta1/trigger_template_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,26 @@ import (

"github.com/tektoncd/pipeline/pkg/apis/validate"
"github.com/tektoncd/triggers/pkg/apis/config"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/apis"
"knative.dev/pkg/webhook/resourcesemantics"
)

// paramsRegexp captures TriggerTemplate parameter names $(tt.params.NAME)
var paramsRegexp = regexp.MustCompile(`\$\(tt.params.(?P<var>[_a-zA-Z][_a-zA-Z0-9.-]*)\)`)

var _ resourcesemantics.VerbLimited = (*TriggerTemplate)(nil)

// SupportedVerbs returns the operations that validation should be called for
func (t *TriggerTemplate) SupportedVerbs() []admissionregistrationv1.OperationType {
return []admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}
}

// Validate validates a TriggerTemplate.
func (t *TriggerTemplate) Validate(ctx context.Context) *apis.FieldError {
if apis.IsInDelete(ctx) {
return nil
}

errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
return errs.Also(t.Spec.validate(ctx).ViaField("spec"))
}
Expand Down
21 changes: 0 additions & 21 deletions pkg/apis/triggers/v1beta1/trigger_template_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1beta1_test

import (
"context"
"strings"
"testing"

pipelinev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
Expand Down Expand Up @@ -100,26 +99,6 @@ func invalidParamResourceTemplate(t *testing.T) runtime.RawExtension {
})
}

func TestTriggerTemplate_Validate_OnDelete(t *testing.T) {
tt := &v1beta1.TriggerTemplate{
ObjectMeta: metav1.ObjectMeta{
Name: strings.Repeat("foo", 64), // Length should be lower than 63
Namespace: "foo",
},
Spec: v1beta1.TriggerTemplateSpec{
Params: []v1beta1.ParamSpec{{
Name: "foo",
Description: "desc",
Default: ptr.String("val"),
}},
},
}
err := tt.Validate(apis.WithinDelete(context.Background()))
if err != nil {
t.Errorf("TriggerTemplate.Validate() on Delete expected no error, but got one, TriggerTemplate: %v, error: %v", tt, err)
}
}

func TestTriggerTemplate_Validate(t *testing.T) {
tcs := []struct {
name string
Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/triggers/v1beta1/trigger_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@ import (

pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/pipeline/pkg/apis/validate"
admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
"knative.dev/pkg/apis"
"knative.dev/pkg/webhook/resourcesemantics"
)

var _ resourcesemantics.VerbLimited = (*Trigger)(nil)

// SupportedVerbs returns the operations that validation should be called for
func (t *Trigger) SupportedVerbs() []admissionregistrationv1.OperationType {
return []admissionregistrationv1.OperationType{admissionregistrationv1.Create, admissionregistrationv1.Update}
}

// Validate validates a Trigger
func (t *Trigger) Validate(ctx context.Context) *apis.FieldError {
if apis.IsInDelete(ctx) {
return nil
}

errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
return errs.Also(t.Spec.validate(ctx).ViaField("spec"))
}
Expand Down
20 changes: 0 additions & 20 deletions pkg/apis/triggers/v1beta1/trigger_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,15 @@ package v1beta1_test

import (
"context"
"strings"
"testing"

pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
"github.com/tektoncd/triggers/pkg/apis/triggers/v1beta1"
"github.com/tektoncd/triggers/test"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"knative.dev/pkg/apis"
"knative.dev/pkg/ptr"
)

func Test_TriggerValidate_OnDelete(t *testing.T) {
tr := &v1beta1.Trigger{
ObjectMeta: metav1.ObjectMeta{
Name: strings.Repeat("foo", 64), // Length should be lower than 63
Namespace: "namespace",
},
Spec: v1beta1.TriggerSpec{
// Binding with no spec is invalid, but shouldn't block the delete
Bindings: []*v1beta1.TriggerSpecBinding{{Name: "", Kind: v1beta1.NamespacedTriggerBindingKind, Ref: "", APIVersion: "v1beta1"}},
Template: v1beta1.TriggerSpecTemplate{Ref: ptr.String("tt")},
},
}
err := tr.Validate(apis.WithinDelete(context.Background()))
if err != nil {
t.Errorf("Trigger.Validate() on Delete expected no error, but got one, Trigger: %v, error: %v", tr, err)
}
}

func Test_TriggerValidate(t *testing.T) {
tests := []struct {
name string
Expand Down

0 comments on commit 74b4749

Please sign in to comment.