Migrate core interceptors to use Interceptor CRD

This commit does the following: 1. Add YAML definitions for each core interceptor using the Interceptor CRD. 2. Modify the EventListener to use the Interceptor CRD to find the Interceptor's URL. 3. Update roles/RBAC/unit tests for the above 2 changes. Fixes tektoncd#868 Signed-off-by: Dibyo Mukherjee <[email protected]>
dibyom · Feb 4, 2021 · 08c9c98 · 08c9c98
1 parent b33dc78
commit 08c9c98
Show file tree

Hide file tree

Showing 12 changed files with 279 additions and 60 deletions.
diff --git a/cmd/eventlistenersink/main.go b/cmd/eventlistenersink/main.go
@@ -113,6 +113,7 @@ func main() {
 		TriggerBindingLister:        factory.Triggers().V1alpha1().TriggerBindings().Lister(),
 		ClusterTriggerBindingLister: factory.Triggers().V1alpha1().ClusterTriggerBindings().Lister(),
 		TriggerTemplateLister:       factory.Triggers().V1alpha1().TriggerTemplates().Lister(),
+		InterceptorLister:           factory.Triggers().V1alpha1().Interceptors().Lister(),
 	}
 
 	// Listen and serve

diff --git a/config/core-interceptors.yaml b/config/core-interceptors.yaml
@@ -0,0 +1,70 @@
+# 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.
+
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: Interceptor
+metadata:
+  name: cel
+spec:
+  clientConfig:
+    url: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/cel"
+  # Until #869 is implemented, the params field is only informational
+  params:
+    - name: "filter"
+      description: "A CEL expression that evaluates to true or false"
+    - name: "overlays"
+      description: "CEL expressions whose values get mapped to user defined keys"
+---
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: Interceptor
+metadata:
+  name: bitbucket
+spec:
+  clientConfig:
+    url: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/bitbucket"
+  # Until #869 is implemented, the params field is only informational
+  params:
+    - name: "eventTypes"
+      description: "A list of event types to accept"
+    - name: "secretRef"
+      description: "A reference to a secret containing a shared token for payload validation"
+---
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: Interceptor
+metadata:
+  name: github
+spec:
+  clientConfig:
+    url: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/github"
+  # Until #869 is implemented, the params field is only informational
+  params:
+    - name: "eventTypes"
+      description: "A list of event types to accept"
+    - name: "secretRef"
+      description: "A reference to a secret containing a shared token for payload validation"
+---
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: Interceptor
+metadata:
+  name: gitlab
+spec:
+  clientConfig:
+    url: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/gitlab"
+  # Until #869 is implemented, the params field is only informational
+  params:
+    - name: "eventTypes"
+      description: "A list of event types to accept"
+    - name: "secretRef"
+      description: "A reference to a secret containing a shared token for payload validation"
+---
diff --git a/docs/interceptors.md b/docs/interceptors.md
@@ -0,0 +1,51 @@
+<!--
+---
+linkTitle: "Interceptor"
+weight: 9
+---
+-->
+# Interceptor
+
+A `Interceptor` is cluster scoped resource that can be invoked during the
+processing of a trigger to modify the behavior or payload of Triggers. The
+custom resource describes how an EventListener can connect to a workload that
+is running the interceptor business logic (and in the future what extra
+paramters the interceptor accepts).
+
+**NOTE**: This doc is a WIP. Please also see the [Interceptors section](./eventlisteners.md#interceptors) in the EventListener doc.
+
+- [Interceptors](#interceptors)
+  - [Syntax](#syntax)
+  -- [clientConfig](#clientConfig)
+
+## Syntax
+
+To define a configuration file for an `Interceptor` resource, you can specify
+the following fields:
+
+- Required:
+  - [`apiVersion`][kubernetes-overview] - Specifies the API version, for example
+    `triggers.tekton.dev/v1alpha1`.
+  - [`kind`][kubernetes-overview] - Specifies the `Trigger` resource
+    object.
+  - [`metadata`][kubernetes-overview] - Specifies data to uniquely identify the
+    `Interceptor` resource object, for example a `name`.
+  - [`spec`][kubernetes-overview] - Specifies the configuration information for
+    your Interceptor resource object. The spec include:
+    - [`clientConfig`] -  Specifies how a client (e.g. an EventListener) can communicate with the Interceptor.
+
+### clientConfig
+
+The `clientConfig` field describes how a client can communicate with an
+interceptor. At the moment, it can contain only the `url` field whose value is
+a resolvable URL. EventListeners will send forward requests to this URL.
+
+```yaml
+spec:
+  clientConfig:
+    url: "http://interceptor-svc.default.svc/"
+```
+
+## Examples
+
+Triggers ships with a few built-in interceptors. These configurations can be found [`config/interceptors`](../config/interceptors).
diff --git a/examples/rbac.yaml b/examples/rbac.yaml
@@ -13,9 +13,8 @@ rules:
   resources: ["eventlisteners", "triggerbindings", "triggertemplates", "triggers"]
   verbs: ["get", "list", "watch"]
 - apiGroups: [""]
- # secrets are only needed for GitHub/GitLab interceptors
  # configmaps is needed for updating logging config
-  resources: ["configmaps", "secrets"]
+  resources: ["configmaps"]
   verbs: ["get", "list", "watch"]
  # Permissions to create resources in associated TriggerTemplates
 - apiGroups: ["tekton.dev"]
@@ -48,7 +47,7 @@ metadata:
 rules:
   # EventListeners need to be able to fetch any clustertriggerbindings
 - apiGroups: ["triggers.tekton.dev"]
-  resources: ["clustertriggerbindings"]
+  resources: ["clustertriggerbindings", "interceptors"]
   verbs: ["get", "list", "watch"]
 ---
 apiVersion: rbac.authorization.k8s.io/v1

diff --git a/pkg/interceptors/interceptors.go b/pkg/interceptors/interceptors.go
@@ -20,14 +20,14 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"net/url"
-	"os"
 	"path"
 
 	"google.golang.org/grpc/codes"
+	"knative.dev/pkg/apis"
 
 	triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -171,28 +171,40 @@ func UnmarshalParams(ip map[string]interface{}, p interface{}) error {
 	return nil
 }
 
-// ResolveURL returns the URL for the given core interceptor
-func ResolveURL(i *triggersv1.TriggerInterceptor) *url.URL {
+// Getname returns the name for the given core interceptor
+func GetName(i *triggersv1.TriggerInterceptor) string {
 	// This is temporary until we implement #868
-	path := ""
+	name := ""
 	switch {
 	case i.Bitbucket != nil:
-		path = "bitbucket"
+		name = "bitbucket"
 	case i.CEL != nil:
-		path = "cel"
+		name = "cel"
 	case i.GitHub != nil:
-		path = "github"
+		name = "github"
 	case i.GitLab != nil:
-		path = "gitlab"
+		name = "gitlab"
 	}
-	return &url.URL{
-		Scheme: "http",
-		Host:   fmt.Sprintf("%s.%s.svc", CoreInterceptorsHost, os.Getenv("TEKTON_INSTALL_NAMESPACE")),
-		Path:   path,
+	return name
+}
+
+type InterceptorGetter func(name string) (*triggersv1.Interceptor, error)
+
+var ErrNilURL = errors.New("interceptor URL was nil")
+
+// ResolveToURL finds an Interceptor's URL.
+func ResolveToURL(getter InterceptorGetter, name string) (*apis.URL, error) {
+	ic, err := getter(name)
+	if err != nil {
+		return nil, err
+	}
+	url := ic.Spec.ClientConfig.URL
+	if url == nil {
+		return nil, ErrNilURL
 	}
+	return ic.Spec.ClientConfig.URL, nil
 }
 
-// Execute executes the InterceptorRequest using the given httpClient
 func Execute(ctx context.Context, client *http.Client, req *triggersv1.InterceptorRequest, url string) (*triggersv1.InterceptorResponse, error) {
 	b, err := json.Marshal(req)
 	if err != nil {

diff --git a/pkg/interceptors/interceptors_test.go b/pkg/interceptors/interceptors_test.go
@@ -18,11 +18,11 @@ package interceptors_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"os"
 	"strings"
 	"testing"
 
@@ -35,6 +35,7 @@ import (
 	"google.golang.org/grpc/codes"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"knative.dev/pkg/apis"
 	fakekubeclient "knative.dev/pkg/client/injection/kube/client/fake"
 	rtesting "knative.dev/pkg/reconciler/testing"
 )
@@ -305,48 +306,76 @@ func TestResolvePath(t *testing.T) {
 		in: triggersv1.EventInterceptor{
 			CEL: &triggersv1.CELInterceptor{},
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/cel",
+		want: "cel",
 	}, {
 		in: triggersv1.EventInterceptor{
 			GitLab: &triggersv1.GitLabInterceptor{},
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/gitlab",
+		want: "gitlab",
 	}, {
 		in: triggersv1.EventInterceptor{
 			GitHub: &triggersv1.GitHubInterceptor{},
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/github",
+		want: "github",
 	}, {
 		in: triggersv1.EventInterceptor{
 			Bitbucket: &triggersv1.BitbucketInterceptor{},
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/bitbucket",
+		want: "bitbucket",
 	}, {
 		in: triggersv1.EventInterceptor{
 			Webhook: &triggersv1.WebhookInterceptor{},
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc",
+		want: "",
 	}} {
 		t.Run(tc.want, func(t *testing.T) {
-			os.Setenv("TEKTON_INSTALL_NAMESPACE", "tekton-pipelines")
-			t.Cleanup(func() { os.Unsetenv("TEKTON_INSTALL_NAMESPACE") })
-			got := interceptors.ResolveURL(&tc.in)
-			if tc.want != got.String() {
+			got := interceptors.GetName(&tc.in)
+			if tc.want != got {
 				t.Fatalf("ResolveURL() want: %s; got: %s", tc.want, got)
 			}
 		})
 	}
+}
 
-	t.Run("different namespaces", func(t *testing.T) {
-		os.Setenv("TEKTON_INSTALL_NAMESPACE", "another-namespace")
-		t.Cleanup(func() { os.Unsetenv("TEKTON_INSTALL_NAMESPACE") })
-		in := &triggersv1.EventInterceptor{
-			Bitbucket: &triggersv1.BitbucketInterceptor{},
+func TestResolveToURL(t *testing.T) {
+	t.Run("resolves URL", func(t *testing.T) {
+		fakeGetter := func(n string) (*triggersv1.Interceptor, error) {
+			return &triggersv1.Interceptor{
+				Spec: triggersv1.InterceptorSpec{
+					ClientConfig: triggersv1.ClientConfig{
+						URL: &apis.URL{
+							Scheme: "http",
+							Host:   "some-host",
+							Path:   n,
+						},
+					},
+				},
+			}, nil
+		}
+
+		got, err := interceptors.ResolveToURL(fakeGetter, "cel")
+		if err != nil {
+			t.Fatalf("ResolveToURL() error: %s", err)
+		}
+		want := "http://some-host/cel"
+		if got.String() != want {
+			t.Fatalf("ResolveToURL want: %s; got: %s", want, got)
+		}
+	})
+
+	t.Run("interceptor has no URL", func(t *testing.T) {
+		fakeGetter := func(name string) (*triggersv1.Interceptor, error) {
+			return &triggersv1.Interceptor{
+				Spec: triggersv1.InterceptorSpec{
+					ClientConfig: triggersv1.ClientConfig{
+						URL: nil,
+					},
+				},
+			}, nil
 		}
-		got := interceptors.ResolveURL(in)
-		want := "http://tekton-triggers-core-interceptors.another-namespace.svc/bitbucket"
-		if want != got.String() {
-			t.Fatalf("ResolveURL() want: %s; got: %s", want, got)
+		_, err := interceptors.ResolveToURL(fakeGetter, "cel")
+		if !errors.Is(err, interceptors.ErrNilURL) {
+			t.Fatalf("ResolveToURL expected error to be %s but got %s", interceptors.ErrNilURL, err)
 		}
 	})
 }

diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener.go
@@ -26,7 +26,6 @@ import (
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
 	listers "github.com/tektoncd/triggers/pkg/client/listers/triggers/v1alpha1"
-	"github.com/tektoncd/triggers/pkg/system"
 	"go.uber.org/zap"
 	"golang.org/x/xerrors"
 	appsv1 "k8s.io/api/apps/v1"
@@ -463,9 +462,6 @@ func getContainer(el *v1alpha1.EventListener, c Config) corev1.Container {
 				FieldPath: "metadata.namespace",
 			},
 		},
-	}, {
-		Name:  "TEKTON_INSTALL_NAMESPACE",
-		Value: system.GetNamespace(),
 	}}
 
 	certEnv := map[string]*corev1.EnvVarSource{}

diff --git a/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go b/pkg/reconciler/v1alpha1/eventlistener/eventlistener_test.go
@@ -248,9 +248,6 @@ func makeDeployment(ops ...func(d *appsv1.Deployment)) *appsv1.Deployment {
 									FieldPath: "metadata.namespace",
 								},
 							},
-						}, {
-							Name:  "TEKTON_INSTALL_NAMESPACE",
-							Value: "tekton-pipelines",
 						}},
 					}},
 					Volumes: []corev1.Volume{{
@@ -336,9 +333,6 @@ var withTLSConfig = func(d *appsv1.Deployment) {
 					FieldPath: "metadata.namespace",
 				},
 			},
-		}, {
-			Name:  "TEKTON_INSTALL_NAMESPACE",
-			Value: "tekton-pipelines",
 		}, {
 			Name: "TLS_CERT",
 			ValueFrom: &corev1.EnvVarSource{