Migrate core interceptors to use InterceptorType CRD

This commit does the following:
1. Add YAML definitions for installing each core interceptor using the
new InterceptorType CRD.
2. Modify the EventListener to use the InterceptorType CRD to find the
Interceptor's URL.

Fixes #868

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

DEVELOPMENT.md

@@ -227,6 +227,7 @@ You can stand up a version of this controller on-cluster (to your
 
 ```shell
 ko apply -f config/
+ko apply -f config/interceptors
 ```
 
 ### Redeploy controller

cmd/eventlistenersink/main.go

@@ -114,6 +114,7 @@ func main() {
 		TriggerBindingLister:        factory.Triggers().V1alpha1().TriggerBindings().Lister(),
 		ClusterTriggerBindingLister: factory.Triggers().V1alpha1().ClusterTriggerBindings().Lister(),
 		TriggerTemplateLister:       factory.Triggers().V1alpha1().TriggerTemplates().Lister(),
+		ClusterInterceptorLister:    factory.Triggers().V1alpha1().ClusterInterceptors().Lister(),
 	}
 	eventListenerBackoff := wait.Backoff{
 		Duration: 100 * time.Millisecond,

config/interceptors/core-interceptors.yaml

@@ -0,0 +1,57 @@
+# Copyright 2021 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: ClusterInterceptor
+metadata:
+  name: cel
+spec:
+  clientConfig:
+    service:
+      name: tekton-triggers-core-interceptors
+      namespace: tekton-pipelines
+      path: "cel"
+---
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: ClusterInterceptor
+metadata:
+  name: bitbucket
+spec:
+  clientConfig:
+    service:
+      name: tekton-triggers-core-interceptors
+      namespace: tekton-pipelines
+      path: "bitbucket"
+---
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: ClusterInterceptor
+metadata:
+  name: github
+spec:
+  clientConfig:
+    service:
+      name: tekton-triggers-core-interceptors
+      namespace: tekton-pipelines
+      path: "github"
+---
+apiVersion: triggers.tekton.dev/v1alpha1
+kind: ClusterInterceptor
+metadata:
+  name: gitlab
+spec:
+  clientConfig:
+    service:
+      name: tekton-triggers-core-interceptors
+      namespace: tekton-pipelines
+      path: "gitlab"

pkg/apis/triggers/v1alpha1/cluster_interceptor_validation.go

@@ -22,7 +22,7 @@ import (
 	"knative.dev/pkg/apis"
 )
 
-// Validate InterceptorTYpe
+// Validate ClusterInterceptor
 func (it *ClusterInterceptor) Validate(ctx context.Context) *apis.FieldError {
 	return it.Spec.validate(ctx)
 }

pkg/interceptors/interceptors.go

@@ -23,11 +23,10 @@ import (
 	"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 +170,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.ClusterInterceptor, error)
+
+// 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
+	}
+	if addr := ic.Status.Address; addr != nil {
+		if addr.URL != nil {
+			return addr.URL, nil
+		}
 	}
+	// If the status does not have a URL, try to generate it from the Spec.
+	return ic.ResolveAddress()
 }
 
-// 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 {

pkg/interceptors/interceptors_test.go

@@ -18,14 +18,16 @@ package interceptors_test
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"os"
 	"strings"
 	"testing"
 
+	duckv1 "knative.dev/pkg/apis/duck/v1"
+
 	"github.com/google/go-cmp/cmp"
 	pipelinev1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	triggersv1 "github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
@@ -35,6 +37,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"
 )
@@ -240,7 +243,7 @@ func TestGetInterceptorParams_Error(t *testing.T) {
 	}
 }
 
-func Test_GetSecretToken(t *testing.T) {
+func TestGetSecretToken(t *testing.T) {
 	tests := []struct {
 		name   string
 		cache  map[string]interface{}
@@ -305,48 +308,106 @@ 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) {
+	tests := []struct {
+		name   string
+		getter interceptors.InterceptorGetter
+		itype  string
+		want   string
+	}{{
+		name: "ClusterInterceptor has status.address.url",
+		getter: func(n string) (*triggersv1.ClusterInterceptor, error) {
+			return &triggersv1.ClusterInterceptor{
+				Status: triggersv1.ClusterInterceptorStatus{
+					AddressStatus: duckv1.AddressStatus{
+						Address: &duckv1.Addressable{
+							URL: &apis.URL{
+								Scheme: "http",
+								Host:   "some-host",
+								Path:   "cel",
+							},
+						},
+					},
+				},
+			}, nil
+		},
+		itype: "cel",
+		want:  "http://some-host/cel",
+	}, {
+		name: "ClusterInterceptor does not have a status",
+		getter: func(n string) (*triggersv1.ClusterInterceptor, error) {
+			return &triggersv1.ClusterInterceptor{
+				Spec: triggersv1.ClusterInterceptorSpec{
+					ClientConfig: triggersv1.ClientConfig{
+						URL: &apis.URL{
+							Scheme: "http",
+							Host:   "some-host",
+							Path:   n,
+						},
+					},
+				},
+			}, nil
+		},
+		itype: "cel",
+		want:  "http://some-host/cel",
+	}}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := interceptors.ResolveToURL(tc.getter, tc.itype)
+			if err != nil {
+				t.Fatalf("ResolveToURL() error: %s", err)
+			}
+
+			if got.String() != tc.want {
+				t.Fatalf("ResolveToURL want: %s; got: %s", tc.want, got)
+			}
+		})
+	}
+
+	t.Run("interceptor has no URL", func(t *testing.T) {
+		fakeGetter := func(name string) (*triggersv1.ClusterInterceptor, error) {
+			return &triggersv1.ClusterInterceptor{
+				Spec: triggersv1.ClusterInterceptorSpec{
+					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, triggersv1.ErrNilURL) {
+			t.Fatalf("ResolveToURL expected error to be %s but got %s", triggersv1.ErrNilURL, err)
 		}
 	})
 }

pkg/reconciler/v1alpha1/clusterinterceptor/controller.go

@@ -27,11 +27,11 @@ import (
 	"knative.dev/pkg/logging"
 )
 
-// NewController creates a new instance of an InterceptorType controller.
+// NewController creates a new instance of an ClusterInterceptor controller.
 func NewController() func(context.Context, configmap.Watcher) *controller.Impl {
 	return func(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
 		logger := logging.FromContext(ctx)
-		interceptorTypeInformer := clusterinterceptorinformer.Get(ctx)
+		ClusterInterceptorInformer := clusterinterceptorinformer.Get(ctx)
 		reconciler := &Reconciler{}
 
 		impl := clusterinterceptorreconciler.NewImpl(ctx, reconciler, func(impl *controller.Impl) controller.Options {
@@ -41,7 +41,7 @@ func NewController() func(context.Context, configmap.Watcher) *controller.Impl {
 		})
 
 		logger.Info("Setting up event handlers")
-		interceptorTypeInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		ClusterInterceptorInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 			AddFunc:    impl.Enqueue,
 			UpdateFunc: controller.PassNew(impl.Enqueue),
 			DeleteFunc: impl.Enqueue,

pkg/reconciler/v1alpha1/eventlistener/eventlistener.go

@@ -33,7 +33,6 @@ import (
 	eventlistenerreconciler "github.com/tektoncd/triggers/pkg/client/injection/reconciler/triggers/v1alpha1/eventlistener"
 	listers "github.com/tektoncd/triggers/pkg/client/listers/triggers/v1alpha1"
 	dynamicduck "github.com/tektoncd/triggers/pkg/dynamic"
-	"github.com/tektoncd/triggers/pkg/system"
 	"go.uber.org/zap"
 	"golang.org/x/xerrors"
 	appsv1 "k8s.io/api/apps/v1"
@@ -660,12 +659,7 @@ func getContainer(el *v1alpha1.EventListener, c Config, pod *duckv1.WithPod) cor
 		Name:      "config-logging",
 		MountPath: "/etc/config-logging",
 	}}
-
-	env := []corev1.EnvVar{{
-		Name:  "TEKTON_INSTALL_NAMESPACE",
-		Value: system.GetNamespace(),
-	}}
-
+	env := []corev1.EnvVar{}
 	if el.Spec.Resources.KubernetesResource != nil {
 		if len(el.Spec.Resources.KubernetesResource.Template.Spec.Containers) != 0 {
 			resources = el.Spec.Resources.KubernetesResource.Template.Spec.Containers[0].Resources