Put triggerGroups behind alpha feature-gate

This commit adds validation to ensure that triggerGroups can only be used in the v1beta1 apiVersion when `enable-api-fields` is set to `alpha`.
tektoncd · Oct 12, 2021 · 7da5283 · 7da5283
1 parent a9a05ae
commit 7da5283
Show file tree

Hide file tree

Showing 4 changed files with 92 additions and 27 deletions.
diff --git a/docs/eventlisteners.md b/docs/eventlisteners.md
@@ -171,7 +171,7 @@ rules:
 
 `TriggerGroups` is a feature that allows you to specify a set of interceptors that will process before a set of 
 `Trigger` resources are processed by the eventlistener. The goal of this feature is described in 
-[TEP-0053](https://github.com/tektoncd/community/blob/main/teps/0053-nested-triggers.md).TriggerGroups` allow for 
+[TEP-0053](https://github.com/tektoncd/community/blob/main/teps/0053-nested-triggers.md). `TriggerGroups` allow for 
 a common set of interceptors to be defined inline in the `EventListenerSpec` before `Triggers` are invoked. 
 
 `TriggerGroups` is currently an `alpha` feature. To use it, you use use the v1beta1 API version with the 

diff --git a/pkg/apis/triggers/v1beta1/event_listener_validation.go b/pkg/apis/triggers/v1beta1/event_listener_validation.go
@@ -22,6 +22,8 @@ import (
 	"encoding/json"
 	"fmt"
 
+	"github.com/tektoncd/triggers/pkg/apis/config"
+
 	"github.com/tektoncd/triggers/pkg/apis/triggers"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -73,8 +75,15 @@ func (s *EventListenerSpec) validate(ctx context.Context) (errs *apis.FieldError
 		errs = errs.Also(validateCustomObject(s.Resources.CustomResource).ViaField("spec.resources.customResource"))
 	}
 
-	for i, group := range s.TriggerGroups {
-		errs = errs.Also(group.validate(ctx).ViaField(fmt.Sprintf("spec.triggerGroups[%d]", i)))
+	if len(s.TriggerGroups) > 0 {
+		err := ValidateEnabledAPIFields(ctx, "spec.triggerGroups", config.AlphaAPIFieldValue)
+		if err != nil {
+			errs = errs.Also(err)
+		} else {
+			for i, group := range s.TriggerGroups {
+				errs = errs.Also(group.validate(ctx).ViaField(fmt.Sprintf("spec.triggerGroups[%d]", i)))
+			}
+		}
 	}
 	return errs
 }

diff --git a/pkg/apis/triggers/v1beta1/event_listener_validation_test.go b/pkg/apis/triggers/v1beta1/event_listener_validation_test.go
@@ -61,9 +61,17 @@ func Test_EventListenerValidate_OnDelete(t *testing.T) {
 }
 
 func Test_EventListenerValidate(t *testing.T) {
+	ctxWithAlphaFieldsEnabled, err := test.FeatureFlagsToContext(context.Background(), map[string]string{
+		"enable-api-fields": "alpha",
+	})
+	if err != nil {
+		t.Fatalf("unexpected error initializing feature flags: %v", err)
+	}
+
 	tests := []struct {
 		name    string
 		el      *triggersv1beta1.EventListener
+		ctx     context.Context
 		wantErr *apis.FieldError
 	}{{
 		name: "TriggerTemplate Does Not Exist",
@@ -450,6 +458,7 @@ func Test_EventListenerValidate(t *testing.T) {
 		},
 	}, {
 		name: "valid EventListener with embedded TriggerTemplate",
+		ctx:  ctxWithAlphaFieldsEnabled,
 		el: &triggersv1beta1.EventListener{
 			ObjectMeta: myObjectMeta,
 			Spec: triggersv1beta1.EventListenerSpec{
@@ -484,6 +493,7 @@ func Test_EventListenerValidate(t *testing.T) {
 		},
 	}, {
 		name: "Valid event listener with TriggerGroup and namespaceSelector",
+		ctx:  ctxWithAlphaFieldsEnabled,
 		el: &triggersv1beta1.EventListener{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "name",
@@ -514,7 +524,11 @@ func Test_EventListenerValidate(t *testing.T) {
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			err := tc.el.Validate(context.Background())
+			ctx := tc.ctx
+			if ctx == nil {
+				ctx = context.Background()
+			}
+			err := tc.el.Validate(ctx)
 			if err != nil {
 				t.Errorf("EventListener.Validate() expected no error, but got one, EventListener: %v, error: %v", tc.el, err)
 			}
@@ -523,9 +537,17 @@ func Test_EventListenerValidate(t *testing.T) {
 }
 
 func TestEventListenerValidate_error(t *testing.T) {
+	ctxWithAlphaFieldsEnabled, err := test.FeatureFlagsToContext(context.Background(), map[string]string{
+		"enable-api-fields": "alpha",
+	})
+	if err != nil {
+		t.Fatalf("unexpected error initializing feature flags: %v", err)
+	}
+
 	tests := []struct {
 		name    string
 		el      *triggersv1beta1.EventListener
+		ctx     context.Context
 		wantErr *apis.FieldError
 	}{{
 		name: "Invalid EventListener name",
@@ -1189,7 +1211,8 @@ func TestEventListenerValidate_error(t *testing.T) {
 		},
 		wantErr: apis.ErrMultipleOneOf("spec.triggers[0].template or bindings or interceptors", "spec.triggers[0].triggerRef"),
 	}, {
-		name: "missing label and namespace selector",
+		name: "triggerGroups is not allowed if alpha fields are not enabled",
+		ctx:  context.Background(), // By default, enable-api-felds is set to stable, not alpha
 		el: &triggersv1beta1.EventListener{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "name",
@@ -1198,6 +1221,11 @@ func TestEventListenerValidate_error(t *testing.T) {
 			Spec: triggersv1beta1.EventListenerSpec{
 				TriggerGroups: []triggersv1beta1.EventListenerTriggerGroup{{
 					Name: "my-group",
+					TriggerSelector: triggersv1beta1.EventListenerTriggerSelector{
+						NamespaceSelector: triggersv1beta1.NamespaceSelector{
+							MatchNames: []string{"default"},
+						},
+					},
 					Interceptors: []*triggersv1beta1.TriggerInterceptor{{
 						Ref: triggersv1beta1.InterceptorRef{
 							Name: "cel",
@@ -1210,33 +1238,63 @@ func TestEventListenerValidate_error(t *testing.T) {
 				}},
 			},
 		},
-		wantErr: apis.ErrMissingOneOf("spec.triggerGroups[0].triggerSelector.labelSelector", "spec.triggerGroups[0].triggerSelector.namespaceSelector"),
-	}, {
-		name: "triggerGroup requires interceptor",
-		el: &triggersv1beta1.EventListener{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "name",
-				Namespace: "namespace",
+		wantErr: apis.ErrGeneric("spec.triggerGroups requires \"enable-api-fields\" feature gate to be \"alpha\" but it is \"stable\""),
+	},
+		{
+			name: "missing label and namespace selector",
+			ctx:  ctxWithAlphaFieldsEnabled,
+			el: &triggersv1beta1.EventListener{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: triggersv1beta1.EventListenerSpec{
+					TriggerGroups: []triggersv1beta1.EventListenerTriggerGroup{{
+						Name: "my-group",
+						Interceptors: []*triggersv1beta1.TriggerInterceptor{{
+							Ref: triggersv1beta1.InterceptorRef{
+								Name: "cel",
+							},
+							Params: []triggersv1beta1.InterceptorParams{{
+								Name:  "filter",
+								Value: test.ToV1JSON(t, "has(body.repository)"),
+							}},
+						}},
+					}},
+				},
 			},
-			Spec: triggersv1beta1.EventListenerSpec{
-				TriggerGroups: []triggersv1beta1.EventListenerTriggerGroup{{
-					Name: "my-group",
-					TriggerSelector: triggersv1beta1.EventListenerTriggerSelector{
-						LabelSelector: &metav1.LabelSelector{
-							MatchLabels: map[string]string{
-								"foo": "bar",
+			wantErr: apis.ErrMissingOneOf("spec.triggerGroups[0].triggerSelector.labelSelector", "spec.triggerGroups[0].triggerSelector.namespaceSelector"),
+		}, {
+			name: "triggerGroup requires interceptor",
+			ctx:  ctxWithAlphaFieldsEnabled,
+			el: &triggersv1beta1.EventListener{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "name",
+					Namespace: "namespace",
+				},
+				Spec: triggersv1beta1.EventListenerSpec{
+					TriggerGroups: []triggersv1beta1.EventListenerTriggerGroup{{
+						Name: "my-group",
+						TriggerSelector: triggersv1beta1.EventListenerTriggerSelector{
+							LabelSelector: &metav1.LabelSelector{
+								MatchLabels: map[string]string{
+									"foo": "bar",
+								},
 							},
 						},
-					},
-				}},
+					}},
+				},
 			},
-		},
-		wantErr: apis.ErrMissingField("spec.triggerGroups[0].interceptors"),
-	}}
+			wantErr: apis.ErrMissingField("spec.triggerGroups[0].interceptors"),
+		}}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got := tc.el.Validate(context.Background())
+			ctx := tc.ctx
+			if ctx == nil {
+				ctx = context.Background()
+			}
+			got := tc.el.Validate(ctx)
 
 			if diff := cmp.Diff(tc.wantErr.Error(), got.Error()); diff != "" {
 				t.Error("EventListener.Validate() (-want, +got) =", diff)

diff --git a/pkg/sink/sink.go b/pkg/sink/sink.go
@@ -236,8 +236,6 @@ func (r Sink) processTriggerGroups(g triggersv1.EventListenerTriggerGroup, reque
 			r.processTrigger(t, localRequest, event, eventID, log, extensions)
 		}(*t)
 	}
-
-	return
 }
 
 func (r Sink) selectTriggers(namespaceSelector triggersv1.NamespaceSelector, labelSelector *metav1.LabelSelector) ([]*triggersv1.Trigger, error) {
-Original file line number
+Diff line change
@@ Expand Up @@
     			r.processTrigger(t, localRequest, event, eventID, log, extensions)
     		}(*t)
     	}
-    	return
     }
     func (r Sink) selectTriggers(namespaceSelector triggersv1.NamespaceSelector, labelSelector *metav1.LabelSelector) ([]*triggersv1.Trigger, error) {
@@ Expand Down @@