Adds support for name/value embedded Bindings

This commit adds support for a more concise syntax for TriggerBindings embedded
in a Trigger or EventListener:

NEW SYNTAX:
```
bindings:
- ref: some-name
- name: commit_id # embedded binding
  value: "$(body.head_commit_id)
```

OLD SYNTAX
```
bindings:
- name: some-name
  spec:
    params:
    - name: commit_id # embedded binding
      value: "$(body.head_commit_id)
```

The old embedded syntax is deprecated and Triggers will convert any old style
embedded bindings to the new syntax via the MutatingAdmissionWebhook.

BREAKING CHANGE: With this change, users cannot specify both `name` and `ref`
for a single binding. Use `ref` to refer to a TriggerBinding resource and
`name` for embedded bindings.

Fixes #617

Signed-off-by: Dibyo Mukherjee <dibyo@google.com>

Author: dibyom

cmd/binding-eval/cmd/root.go

@@ -77,9 +77,14 @@ func evalBinding(w io.Writer, bindingPath, httpPath string) error {
 		return fmt.Errorf("error reading bindings: %w", err)
 	}
 
+	bindingParams := []v1alpha1.Param{}
+	for _, b := range bindings {
+		bindingParams = append(bindingParams, b.Spec.Params...)
+	}
 	t := template.ResolvedTrigger{
-		TriggerBindings: bindings,
+		BindingParams: bindingParams,
 	}
+
 	params, err := template.ResolveParams(t, body, r.Header)
 	if err != nil {
 		return fmt.Errorf("error resolving params: %w", err)

cmd/binding-eval/cmd/root_test.go

@@ -1,5 +1,3 @@
-// +build e2e
-
 /*
 Copyright 2020 The Tekton Authors
 

cmd/triggerrun/cmd/root_test.go

@@ -1,5 +1,3 @@
-// +build e2e
-
 /*
 Copyright 2020 The Tekton Authors
 
@@ -215,22 +213,14 @@ func Test_processTriggerSpec(t *testing.T) {
 		name: "testing-name",
 		args: args{
 			t: &v1alpha1.Trigger{
-				TypeMeta: metav1.TypeMeta{
-					Kind:       "TriggerRun",
-					APIVersion: "tekton.dev/v1alpha1"},
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "my-triggerRun",
 				},
 				Spec: v1alpha1.TriggerSpec{
-					Bindings: []*v1alpha1.TriggerSpecBinding{
-						{Ref: "git-event-binding"}, // These should be references to TriggerBindings defined below
-					},
-					Template: v1alpha1.TriggerSpecTemplate{
-						Name: "simple-pipeline-template", // This should be a reference to a TriggerTemplate defined below
-					},
+					Bindings: []*v1alpha1.TriggerSpecBinding{{Ref: "git-event-binding"}},     // These should be references to TriggerBindings defined below
+					Template: v1alpha1.TriggerSpecTemplate{Name: "simple-pipeline-template"}, // This should be a reference to a TriggerTemplate defined below
 				},
 			},
-
 			request: r,
 			event:   eventBody,
 			resources: test.Resources{

docs/eventlisteners.md

@@ -14,19 +14,32 @@ and apply them to [TriggerTemplates](./triggertemplates.md) in order to create
 Tekton resources. In addition, EventListeners allow lightweight event processing
 using [Event Interceptors](#Interceptors).
 
-- [Syntax](#syntax)
-  - [ServiceAccountName](#serviceAccountName)
-  - [Replicas](#replicas)
-  - [PodTemplate](#podTemplate)
-  - [Triggers](#triggers)
-    - [Interceptors](#interceptors)
-- [Logging](#logging)
-- [Labels](#labels)
-- [Annotations](#annotations)
-- [EventListener Response](#eventlistener-response)
-- [How does the EventListener work?](#how-does-the-eventlistener-work)
-- [Examples](#examples)
-- [Multi-Tenant Concerns](#multi-tenant-concerns)
+- [EventListener](#eventlistener)
+  - [Syntax](#syntax)
+    - [ServiceAccountName](#serviceaccountname)
+    - [Triggers](#triggers)
+    - [ServiceType](#servicetype)
+    - [Replicas](#replicas)
+    - [PodTemplate](#podtemplate)
+    - [Resources](#resources)
+    - [Logging](#logging)
+  - [Labels](#labels)
+  - [Annotations](#annotations)
+  - [Interceptors](#interceptors)
+    - [Webhook Interceptors](#webhook-interceptors)
+      - [Event Interceptor Services](#event-interceptor-services)
+    - [GitHub Interceptors](#github-interceptors)
+    - [GitLab Interceptors](#gitlab-interceptors)
+    - [Bitbucket Interceptors](#bitbucket-interceptors)
+    - [CEL Interceptors](#cel-interceptors)
+      - [Overlays](#overlays)
+  - [EventListener Response](#eventlistener-response)
+  - [How does the EventListener work?](#how-does-the-eventlistener-work)
+  - [Examples](#examples)
+  - [Multi-Tenant Concerns](#multi-tenant-concerns)
+    - [Multiple EventListeners (One EventListener Per Namespace)](#multiple-eventlisteners-one-eventlistener-per-namespace)
+    - [Multiple EventListeners (Multiple EventListeners per Namespace)](#multiple-eventlisteners-multiple-eventlisteners-per-namespace)
+    - [ServiceAccount per EventListenerTrigger](#serviceaccount-per-eventlistenertrigger)
 
 ## Syntax
 
@@ -102,7 +115,7 @@ The `triggers` field is required. Each EventListener can consist of one or more
 - `name` - (Optional) a valid
   [Kubernetes name](https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#syntax-and-character-set)
 - [`interceptors`](#interceptors) - (Optional) list of interceptors to use
-- `bindings` - (Optional) A list of `TriggerBindings` reference to use or embedded TriggerBindingsSpecs to use.
+- `bindings` - (Optional) A list of bindings to use. Can either be a reference to existing `TriggerBinding` resources or embedded name/value pairs.
 - `template` - (Optional) The name of `TriggerTemplate` to use
 - `triggerRef` - (Optional) Reference to the [`Trigger`](./triggers.md).
 
@@ -115,12 +128,9 @@ triggers:
       - github:
           eventTypes: ["pull_request"]
     bindings:
-      - ref: pipeline-binding
-      - name: message-binding
-        spec:
-            params:
-              - name: message
-                value: Hello from the Triggers EventListener!
+      - ref: pipeline-binding # Reference to a TriggerBinding object
+      - name: message # Embedded Binding
+        value: Hello from the Triggers EventListener!
     template:
       name: pipeline-template
 ```

docs/triggers.md

@@ -8,7 +8,8 @@ weight: 9
 
 A `Trigger` is resource that combines `TriggerTemplate`, `TriggerBindings` and `interceptors`. The `Trigger` is processed by EventListener which referenced it when it receives an incoming.
 
-- [Syntax](#syntax)
+- [Triggers](#triggers)
+  - [Syntax](#syntax)
 
 ## Syntax
 
@@ -24,7 +25,7 @@ the following fields:
     `Trigger` resource object, for example a `name`.
   - [`spec`][kubernetes-overview] - Specifies the configuration information for
     your Trigger resource object. The spec include:
-    - [`bindings`] -  A list of `TriggerBindings` reference to use or embedded TriggerBindingsSpecs to use
+    - [`bindings`] - (Optional) A list of bindings to use. Can either be a reference to existing `TriggerBinding` resources or embedded name/value pairs.
     - [`template`] -  The name of `TriggerTemplate` to use
     - [`interceptors`](./eventlisteners.md#interceptors) - (Optional) list of interceptors to use
     - [`serviceAccountName`] - (Optional) Specifies the ServiceAccount provided to EventListener by Trigger to create resources

examples/gitlab/gitlab-push-listener.yaml

@@ -34,17 +34,6 @@ spec:
                     value: $(tt.params.gitrepositoryurl)
 ---
 apiVersion: triggers.tekton.dev/v1alpha1
-kind: TriggerBinding
-metadata:
-  name: gitlab-push-binding
-spec:
-  params:
-    - name: gitrevision
-      value: $(body.checkout_sha)
-    - name: gitrepositoryurl
-      value: $(body.repository.git_http_url)
----
-apiVersion: triggers.tekton.dev/v1alpha1
 kind: EventListener
 metadata:
   name: gitlab-listener
@@ -60,6 +49,9 @@ spec:
             eventTypes:
               - Push Hook  # Only push events
       bindings:
-        - ref: gitlab-push-binding
+        - name: gitrevision
+          value: $(body.checkout_sha)
+        - name: gitrepositoryurl
+          value: $(body.repository.git_http_url)
       template:
         name: gitlab-echo-template

pkg/apis/triggers/v1alpha1/trigger_defaults.go

@@ -18,16 +18,34 @@ package v1alpha1
 
 import (
 	"context"
+
+	"knative.dev/pkg/ptr"
 )
 
 type triggerSpecBindingArray []*TriggerSpecBinding
 
 // SetDefaults sets the defaults on the object.
 func (t *Trigger) SetDefaults(ctx context.Context) {
-	if IsUpgradeViaDefaulting(ctx) {
-		// set defaults
-		triggerSpecBindingArray(t.Spec.Bindings).defaultBindings()
+	if !IsUpgradeViaDefaulting(ctx) {
+		return
+	}
+	triggerSpecBindingArray(t.Spec.Bindings).defaultBindings()
+
+	// Upgrade old style embedded bindings to new concise syntax
+	bindings := []*TriggerSpecBinding{}
+	for _, b := range t.Spec.Bindings {
+		if b.Spec == nil {
+			bindings = append(bindings, b)
+		} else {
+			for _, p := range b.Spec.Params {
+				bindings = append(bindings, &TriggerSpecBinding{
+					Name:  p.Name,
+					Value: ptr.String(p.Value),
+				})
+			}
+		}
 	}
+	t.Spec.Bindings = bindings
 }
 
 // set default TriggerBinding kind for Bindings in TriggerSpec

pkg/apis/triggers/v1alpha1/trigger_defaults_test.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
+	"knative.dev/pkg/ptr"
 )
 
 func TestTriggerSetDefaults(t *testing.T) {
@@ -31,41 +32,85 @@ func TestTriggerSetDefaults(t *testing.T) {
 		want *v1alpha1.Trigger
 		wc   func(context.Context) context.Context
 	}{{
-		name: "default binding",
+		name: "default binding kind",
 		in: &v1alpha1.Trigger{
 			Spec: v1alpha1.TriggerSpec{
-				Bindings: []*v1alpha1.TriggerSpecBinding{
-					{
-						Ref: "binding",
-					},
-					{
-						Kind: v1alpha1.NamespacedTriggerBindingKind,
-						Ref:  "namespace-binding",
-					},
-					{
-						Kind: v1alpha1.ClusterTriggerBindingKind,
-						Ref:  "cluster-binding",
-					},
-				},
+				Bindings: []*v1alpha1.TriggerSpecBinding{{
+					Ref: "binding",
+				}, {
+					Kind: v1alpha1.NamespacedTriggerBindingKind,
+					Ref:  "namespace-binding",
+				}, {
+					Kind: v1alpha1.ClusterTriggerBindingKind,
+					Ref:  "cluster-binding",
+				}},
 			},
 		},
 		wc: v1alpha1.WithUpgradeViaDefaulting,
 		want: &v1alpha1.Trigger{
 			Spec: v1alpha1.TriggerSpec{
-				Bindings: []*v1alpha1.TriggerSpecBinding{
-					{
-						Kind: v1alpha1.NamespacedTriggerBindingKind,
-						Ref:  "binding",
-					},
-					{
-						Kind: v1alpha1.NamespacedTriggerBindingKind,
-						Ref:  "namespace-binding",
-					},
-					{
-						Kind: v1alpha1.ClusterTriggerBindingKind,
-						Ref:  "cluster-binding",
+				Bindings: []*v1alpha1.TriggerSpecBinding{{
+					Kind: v1alpha1.NamespacedTriggerBindingKind,
+					Ref:  "binding",
+				}, {
+					Kind: v1alpha1.NamespacedTriggerBindingKind,
+					Ref:  "namespace-binding",
+				}, {
+					Kind: v1alpha1.ClusterTriggerBindingKind,
+					Ref:  "cluster-binding",
+				}},
+			},
+		},
+	}, {
+		name: "default old embed binding to new",
+		in: &v1alpha1.Trigger{
+			Spec: v1alpha1.TriggerSpec{
+				Bindings: []*v1alpha1.TriggerSpecBinding{{
+					Kind: v1alpha1.NamespacedTriggerBindingKind,
+					Ref:  "binding",
+				}, {
+					Kind: v1alpha1.NamespacedTriggerBindingKind,
+					Spec: &v1alpha1.TriggerBindingSpec{
+						Params: []v1alpha1.Param{{
+							Name:  "p1",
+							Value: "v1",
+						}, {
+							Name:  "p2",
+							Value: "v2",
+						}},
 					},
-				},
+				}},
+			},
+		},
+		wc: v1alpha1.WithUpgradeViaDefaulting,
+		want: &v1alpha1.Trigger{
+			Spec: v1alpha1.TriggerSpec{
+				Bindings: []*v1alpha1.TriggerSpecBinding{{
+					Kind: v1alpha1.NamespacedTriggerBindingKind,
+					Ref:  "binding",
+				}, {
+					Name:  "p1",
+					Value: ptr.String("v1"),
+				}, {
+					Name:  "p2",
+					Value: ptr.String("v2"),
+				}},
+			},
+		},
+	}, {
+		name: "upgrade context not set",
+		in: &v1alpha1.Trigger{
+			Spec: v1alpha1.TriggerSpec{
+				Bindings: []*v1alpha1.TriggerSpecBinding{{
+					Ref: "binding",
+				}},
+			},
+		},
+		want: &v1alpha1.Trigger{
+			Spec: v1alpha1.TriggerSpec{
+				Bindings: []*v1alpha1.TriggerSpecBinding{{
+					Ref: "binding", // If upgrade context was set, Kind should have been added
+				}},
 			},
 		},
 	}}

pkg/apis/triggers/v1alpha1/trigger_types.go

@@ -44,11 +44,27 @@ type TriggerSpecTemplate struct {
 }
 
 type TriggerSpecBinding struct {
-	Name       string              `json:"name,omitempty"`
-	Kind       TriggerBindingKind  `json:"kind,omitempty"`
-	Ref        string              `json:"ref,omitempty"`
-	Spec       *TriggerBindingSpec `json:"spec,omitempty"`
-	APIVersion string              `json:"apiversion,omitempty"`
+	// Name is the name of the binding param
+	// Mutually exclusive with Ref
+	Name string `json:"name,omitempty"`
+	// Value is the value of the binding param. Can contain JSONPath
+	// Has to be pointer since "" is a valid value
+	// Required if Name is also specified.
+	Value *string `json:"value,omitempty"`
+
+	// Ref is a reference to a TriggerBinding kind.
+	// Mutually exclusive with Name
+	Ref string `json:"ref,omitempty"`
+
+	// Kind can only be provided if Ref is also provided. Defaults to TriggerBinding
+	Kind TriggerBindingKind `json:"kind,omitempty"`
+
+	// Spec is the deprecated  way of embedding TriggerBindings.
+	// TODO(#782): Remove deprecated syntax.
+	Spec *TriggerBindingSpec `json:"spec,omitempty"`
+
+	// APIVersion of the binding ref
+	APIVersion string `json:"apiversion,omitempty"`
 }
 
 // +genclient