Merge branch 'master' of github.com:tektoncd/triggers into elServTimeout

tektoncd · Sep 21, 2020 · 8b22f56 · 8b22f56
2 parents 69eda8d + 2cc62a2
commit 8b22f56
Show file tree

Hide file tree

Showing 15 changed files with 804 additions and 188 deletions.
diff --git a/README.md b/README.md
@@ -82,6 +82,8 @@ events.
 | Version                                                                                  | Docs                                                                                   | Examples                                                                                | Getting Started                                                                                                                 |
 | ---------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------- |
 | [HEAD](https://github.com/tektoncd/triggers/blob/master/DEVELOPMENT.md#install-pipeline) | [Docs @ HEAD](https://github.com/tektoncd/triggers/blob/master/docs/README.md)         | [Examples @ HEAD](https://github.com/tektoncd/triggers/blob/master/examples)            | [Getting Started @ HEAD](https://github.com/tektoncd/triggers/blob/master/docs/getting-started#getting-started-with-triggers)   |
+| [v0.8.1](https://github.com/tektoncd/triggers/releases/tag/v0.8.1)                       | [Docs @ v0.8.1](https://github.com/tektoncd/triggers/tree/v0.8.1/docs#tekton-triggers) | [Examples @ v0.8.1](https://github.com/tektoncd/triggers/tree/v0.8.1/examples#examples) | [Getting Started @ v0.8.1](https://github.com/tektoncd/triggers/tree/v0.8.1/docs/getting-started#getting-started-with-triggers) |
+| [v0.8.0](https://github.com/tektoncd/triggers/releases/tag/v0.8.0)                       | [Docs @ v0.8.0](https://github.com/tektoncd/triggers/tree/v0.8.0/docs#tekton-triggers) | [Examples @ v0.8.0](https://github.com/tektoncd/triggers/tree/v0.8.0/examples#examples) | [Getting Started @ v0.8.0](https://github.com/tektoncd/triggers/tree/v0.8.0/docs/getting-started#getting-started-with-triggers) |
 | [v0.7.0](https://github.com/tektoncd/triggers/releases/tag/v0.7.0)                       | [Docs @ v0.7.0](https://github.com/tektoncd/triggers/tree/v0.7.0/docs#tekton-triggers) | [Examples @ v0.7.0](https://github.com/tektoncd/triggers/tree/v0.7.0/examples#examples) | [Getting Started @ v0.7.0](https://github.com/tektoncd/triggers/tree/v0.7.0/docs/getting-started#getting-started-with-triggers) |
 | [v0.6.1](https://github.com/tektoncd/triggers/releases/tag/v0.6.1)                       | [Docs @ v0.6.1](https://github.com/tektoncd/triggers/tree/v0.6.1/docs#tekton-triggers) | [Examples @ v0.6.1](https://github.com/tektoncd/triggers/tree/v0.6.1/examples#examples) | [Getting Started @ v0.6.1](https://github.com/tektoncd/triggers/tree/v0.6.1/docs/getting-started#getting-started-with-triggers) |
 | [v0.6.0](https://github.com/tektoncd/triggers/releases/tag/v0.6.0)                       | [Docs @ v0.6.0](https://github.com/tektoncd/triggers/tree/v0.6.0/docs#tekton-triggers) | [Examples @ v0.6.0](https://github.com/tektoncd/triggers/tree/v0.6.0/examples#examples) | [Getting Started @ v0.6.0](https://github.com/tektoncd/triggers/tree/v0.6.0/docs/getting-started#getting-started-with-triggers) |

diff --git a/cmd/eventlistenersink/main.go b/cmd/eventlistenersink/main.go
@@ -96,9 +96,12 @@ func main() {
 
 	// Listen and serve
 	logger.Infof("Listen and serve on port %s", sinkArgs.Port)
-	http.HandleFunc("/", r.HandleEvent)
+	mux := http.NewServeMux()
+	mux.HandleFunc("/", r.HandleEvent)
+
 	// For handling Liveness Probe
-	http.HandleFunc("/live", func(w http.ResponseWriter, r *http.Request) {
+	// TODO(dibyom): Livness, metrics etc. should be on a separate port
+	mux.HandleFunc("/live", func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(200)
 		fmt.Fprint(w, "ok")
 	})
@@ -108,9 +111,11 @@ func main() {
 		ReadTimeout:  sinkArgs.ELReadTimeOut * time.Second,
 		WriteTimeout: sinkArgs.ELWriteTimeOut * time.Second,
 		IdleTimeout:  sinkArgs.ELIdleTimeOut * time.Second,
-		Handler: http.TimeoutHandler(http.DefaultServeMux,
+		Handler: http.TimeoutHandler(mux,
 			sinkArgs.ELTimeOutHandler*time.Second, "EventListener Timeout!\n"),
 	}
 
-	logger.Fatal(srv.ListenAndServe())
+	if err := srv.ListenAndServe(); err != nil {
+		logger.Fatalf("faiiled to start eventlistener sink: %v", err)
+	}
 }
diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -40,6 +40,7 @@ var types = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
 	v1alpha1.SchemeGroupVersion.WithKind("EventListener"):         &v1alpha1.EventListener{},
 	v1alpha1.SchemeGroupVersion.WithKind("TriggerBinding"):        &v1alpha1.TriggerBinding{},
 	v1alpha1.SchemeGroupVersion.WithKind("TriggerTemplate"):       &v1alpha1.TriggerTemplate{},
+	v1alpha1.SchemeGroupVersion.WithKind("Trigger"):               &v1alpha1.Trigger{},
 }
 
 func NewDefaultingAdmissionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {

diff --git a/docs/eventlisteners.md b/docs/eventlisteners.md
@@ -24,6 +24,7 @@ using [Event Interceptors](#Interceptors).
 - [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)
 
@@ -174,9 +175,9 @@ check out the guide on [exposing EventListeners](./exposing-eventlisteners.md).
 ### Replicas
 
 The `replicas` field is optional. By default, the number of replicas of EventListener is 1.
-If you want to deploy more than one pod, you can specify the number to this field. 
+If you want to deploy more than one pod, you can specify the number to `replicas` field.
 
-**Note:** If user sets `replicas` field while creating eventlistener yaml then it won't respects replicas values edited by user manually or through any other mechanism (ex: HPA).
+**Note:** If user sets `replicas` field while creating/updating eventlistener yaml then it won't respects replicas values edited by user manually on deployment or through any other mechanism (ex: HPA).
 
 ### PodTemplate
 
@@ -748,6 +749,39 @@ The EventListener responds with following message after receiving the event:
 - `namespace` - Refers to the namespace of the EventListener
 - `eventID` - Refers to the uniqueID that gets assigned to each incoming request
 
+## How does the EventListener work?
+
+Lets understand how an EventListener works with an example using GitHub
+
+* Create a sample GitHub example
+```bash
+kubectl create -f https://github.com/tektoncd/triggers/tree/master/examples/github
+```
+
+* Once the EventListener is created,  the Triggers controller will create a new `Deployment` and `Service` for the EventListener. We can use `kubectl` to see them running:
+```bash
+kubectl get deployment
+NAME                             READY   UP-TO-DATE   AVAILABLE   AGE
+el-github-listener-interceptor   1/1     1            1           11s
+
+kubectl get svc
+NAME                             TYPE        CLUSTER-IP      EXTERNAL-IP   PORT(S)    AGE
+el-github-listener-interceptor   ClusterIP   10.99.188.140   <none>        8080/TCP   52s
+```
+The Triggers controller uses fields from the EventListener's `spec` (which is described in the [Syntax](https://github.com/tektoncd/triggers/blob/master/docs/eventlisteners.md#syntax) section, as well as [`metadata.labels`](https://github.com/tektoncd/triggers/blob/master/docs/eventlisteners.md#labels)
+in addition to some pre-configured information like (container `Image`, `Name`, `Port`) to create the **Deployment** and **Service**.
+
+We follow a naming convention while creating these resources. An EventListener named `foo` will create a deployment and a service both named `el-foo`.
+
+Once all the resources are up and running user can get a URL to send webhook events. This URL points to the service created above and points to the deployment.
+```bash
+kubectl get eventlistener
+NAME                          ADDRESS                                                              AVAILABLE   REASON
+github-listener-interceptor   http://el-github-listener-interceptor.ptest.svc.cluster.local:8080   True        MinimumReplicasAvailable
+```
+
+Follow [GitHub example](https://github.com/tektoncd/triggers/blob/master/examples/github/README.md) to try out locally.
+
 ## Examples
 
 For complete examples, see

diff --git a/pkg/apis/triggers/v1alpha1/event_listener_defaults.go b/pkg/apis/triggers/v1alpha1/event_listener_defaults.go
@@ -24,19 +24,12 @@ import (
 func (el *EventListener) SetDefaults(ctx context.Context) {
 	if IsUpgradeViaDefaulting(ctx) {
 		// set defaults
-		for i := range el.Spec.Triggers {
-			defaultBindings(&el.Spec.Triggers[i])
+		if el.Spec.Replicas != nil && *el.Spec.Replicas == 0 {
+			*el.Spec.Replicas = 1
 		}
-	}
-}
-
-// set default TriggerBinding kind for Bindings
-func defaultBindings(t *EventListenerTrigger) {
-	if len(t.Bindings) > 0 {
-		for _, b := range t.Bindings {
-			if b.Kind == "" {
-				b.Kind = NamespacedTriggerBindingKind
-			}
+		for i := range el.Spec.Triggers {
+			triggerSpecBindingArray(el.Spec.Triggers[i].Bindings).
+				defaultBindings()
 		}
 	}
 }
diff --git a/pkg/apis/triggers/v1alpha1/event_listener_defaults_test.go b/pkg/apis/triggers/v1alpha1/event_listener_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 TestEventListenerSetDefaults(t *testing.T) {
@@ -72,6 +73,32 @@ func TestEventListenerSetDefaults(t *testing.T) {
 				}},
 			},
 		},
+	}, {
+		name: "set replicas to 1 if provided replicas is 0 as part of eventlistener spec",
+		in: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Replicas: ptr.Int32(0),
+			},
+		},
+		wc: v1alpha1.WithUpgradeViaDefaulting,
+		want: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Replicas: ptr.Int32(1),
+			},
+		},
+	}, {
+		name: "different value for replicas other than 0",
+		in: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Replicas: ptr.Int32(2),
+			},
+		},
+		wc: v1alpha1.WithUpgradeViaDefaulting,
+		want: &v1alpha1.EventListener{
+			Spec: v1alpha1.EventListenerSpec{
+				Replicas: ptr.Int32(2),
+			},
+		},
 	}}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

diff --git a/pkg/apis/triggers/v1alpha1/event_listener_validation.go b/pkg/apis/triggers/v1alpha1/event_listener_validation.go
@@ -19,10 +19,7 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
-	"net/http"
 
-	"github.com/google/cel-go/cel"
-	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
 )
@@ -34,7 +31,7 @@ func (e *EventListener) Validate(ctx context.Context) *apis.FieldError {
 
 func (s *EventListenerSpec) validate(ctx context.Context) *apis.FieldError {
 	if s.Replicas != nil {
-		if *s.Replicas <= 0 {
+		if *s.Replicas < 0 {
 			return apis.ErrInvalidValue(*s.Replicas, "spec.replicas")
 		}
 	}
@@ -55,34 +52,17 @@ func (t *EventListenerTrigger) validate(ctx context.Context) *apis.FieldError {
 	}
 
 	// Validate optional Bindings
-	for i, b := range t.Bindings {
-		// Either Ref or Spec should be present
-		if b.Ref == "" && b.Spec == nil {
-			return apis.ErrMissingOneOf(fmt.Sprintf("bindings[%d].Ref", i), fmt.Sprintf("bindings[%d].Spec", i))
-		}
-
-		// Both Ref and Spec can't be present at the same time
-		if b.Ref != "" && b.Spec != nil {
-			return apis.ErrMultipleOneOf(fmt.Sprintf("bindings[%d].Ref", i), fmt.Sprintf("bindings[%d].Spec", i))
-		}
-
-		if b.Ref != "" && b.Kind != NamespacedTriggerBindingKind && b.Kind != ClusterTriggerBindingKind {
-			return apis.ErrInvalidValue(fmt.Errorf("invalid kind"), fmt.Sprintf("bindings[%d].kind", i))
-		}
+	if err := triggerSpecBindingArray(t.Bindings).validate(ctx); err != nil {
+		return err
 	}
-	// Validate required TriggerTemplate
-	// Optional explicit match
 	if t.Template != nil {
-		if t.Template.APIVersion != "" {
-			if t.Template.APIVersion != "v1alpha1" {
-				return apis.ErrInvalidValue(fmt.Errorf("invalid apiVersion"), "template.apiVersion")
-			}
-		}
-		if t.Template.Name == "" {
-			return apis.ErrMissingField("template.name")
+		// Validate required TriggerTemplate
+		if err := t.Template.validate(ctx); err != nil {
+			return err
 		}
 	}
 
+	// Validate optional Interceptors
 	for i, interceptor := range t.Interceptors {
 		if err := interceptor.validate(ctx).ViaField(fmt.Sprintf("interceptors[%d]", i)); err != nil {
 			return err
@@ -97,89 +77,3 @@ func (t *EventListenerTrigger) validate(ctx context.Context) *apis.FieldError {
 
 	return nil
 }
-
-func (i *EventInterceptor) validate(ctx context.Context) *apis.FieldError {
-	if i.Webhook == nil && i.GitHub == nil && i.GitLab == nil && i.CEL == nil && i.Bitbucket == nil {
-		return apis.ErrMissingField("interceptor")
-	}
-
-	// Enforce oneof
-	numSet := 0
-	if i.Webhook != nil {
-		numSet++
-	}
-	if i.GitHub != nil {
-		numSet++
-	}
-	if i.GitLab != nil {
-		numSet++
-	}
-	if i.Bitbucket != nil {
-		numSet++
-	}
-
-	if numSet > 1 {
-		return apis.ErrMultipleOneOf("interceptor.webhook", "interceptor.github", "interceptor.gitlab")
-	}
-
-	if i.Webhook != nil {
-		if i.Webhook.ObjectRef == nil || i.Webhook.ObjectRef.Name == "" {
-			return apis.ErrMissingField("interceptor.webhook.objectRef")
-		}
-		w := i.Webhook
-		if w.ObjectRef.Kind != "Service" {
-			return apis.ErrInvalidValue(fmt.Errorf("invalid kind"), "interceptor.webhook.objectRef.kind")
-		}
-
-		// Optional explicit match
-		if w.ObjectRef.APIVersion != "v1" {
-			return apis.ErrInvalidValue(fmt.Errorf("invalid apiVersion"), "interceptor.webhook.objectRef.apiVersion")
-		}
-
-		for i, header := range w.Header {
-			// Enforce non-empty canonical header keys
-			if len(header.Name) == 0 || http.CanonicalHeaderKey(header.Name) != header.Name {
-				return apis.ErrInvalidValue(fmt.Errorf("invalid header name"), fmt.Sprintf("interceptor.webhook.header[%d].name", i))
-			}
-			// Enforce non-empty header values
-			if header.Value.Type == pipelinev1.ParamTypeString {
-				if len(header.Value.StringVal) == 0 {
-					return apis.ErrInvalidValue(fmt.Errorf("invalid header value"), fmt.Sprintf("interceptor.webhook.header[%d].value", i))
-				}
-			} else if len(header.Value.ArrayVal) == 0 {
-				return apis.ErrInvalidValue(fmt.Errorf("invalid header value"), fmt.Sprintf("interceptor.webhook.header[%d].value", i))
-			}
-		}
-	}
-
-	// No github validation required yet.
-	// if i.GitHub != nil {
-	//
-	// }
-
-	// No gitlab validation required yet.
-	// if i.GitLab != nil {
-	//
-	// }
-
-	if i.CEL != nil {
-		if i.CEL.Filter == "" && len(i.CEL.Overlays) == 0 {
-			return apis.ErrMultipleOneOf("cel.filter", "cel.overlays")
-		}
-		env, err := cel.NewEnv()
-		if err != nil {
-			return apis.ErrInvalidValue(fmt.Errorf("failed to create a CEL env: %s", err), "cel.filter")
-		}
-		if i.CEL.Filter != "" {
-			if _, issues := env.Parse(i.CEL.Filter); issues != nil && issues.Err() != nil {
-				return apis.ErrInvalidValue(fmt.Errorf("failed to parse the CEL filter: %s", issues.Err()), "cel.filter")
-			}
-		}
-		for _, v := range i.CEL.Overlays {
-			if _, issues := env.Parse(v.Expression); issues != nil && issues.Err() != nil {
-				return apis.ErrInvalidValue(fmt.Errorf("failed to parse the CEL overlay: %s", issues.Err()), "cel.overlay")
-			}
-		}
-	}
-	return nil
-}
diff --git a/pkg/apis/triggers/v1alpha1/event_listener_validation_test.go b/pkg/apis/triggers/v1alpha1/event_listener_validation_test.go
@@ -425,10 +425,10 @@ func TestEventListenerValidate_error(t *testing.T) {
 					bldr.EventListenerTriggerName("1234567890123456789012345678901234567890123456789012345678901234"),
 				))),
 	}, {
-		name: "user specify invalid replicas which are 0 and negative values",
+		name: "user specify invalid replicas",
 		el: bldr.EventListener("name", "namespace",
 			bldr.EventListenerSpec(
-				bldr.EventListenerReplicas(0),
+				bldr.EventListenerReplicas(-1),
 				bldr.EventListenerTrigger("tt", "v1alpha1",
 					bldr.EventListenerTriggerBinding("tb", "TriggerBinding", "tb", "v1alpha1"),
 				))),

diff --git a/pkg/apis/triggers/v1alpha1/trigger_defaults.go b/pkg/apis/triggers/v1alpha1/trigger_defaults.go
@@ -0,0 +1,42 @@
+/*
+Copyright 2020 The Tekton Authors
+
+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 v1alpha1
+
+import (
+	"context"
+)
+
+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()
+	}
+}
+
+// set default TriggerBinding kind for Bindings in TriggerSpec
+func (t triggerSpecBindingArray) defaultBindings() {
+	if len(t) > 0 {
+		for _, b := range t {
+			if b.Kind == "" {
+				b.Kind = NamespacedTriggerBindingKind
+			}
+		}
+	}
+}