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/apis/triggers/v1alpha1/trigger_types.go

@@ -89,6 +89,23 @@ type TriggerInterceptor struct {
 	Bitbucket *BitbucketInterceptor `json:"bitbucket,omitempty"`
 }
 
+// GetName returns the name for the given interceptor
+func (i *TriggerInterceptor) GetName() string {
+	// This is temporary until we implement #869
+	name := ""
+	switch {
+	case i.Bitbucket != nil:
+		name = "bitbucket"
+	case i.CEL != nil:
+		name = "cel"
+	case i.GitHub != nil:
+		name = "github"
+	case i.GitLab != nil:
+		name = "gitlab"
+	}
+	return name
+}
+
 // WebhookInterceptor provides a webhook to intercept and pre-process events
 type WebhookInterceptor struct {
 	// ObjectRef is a reference to an object that will resolve to a cluster DNS

pkg/apis/triggers/v1alpha1/trigger_types_test.go

@@ -0,0 +1,62 @@
+/*
+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.
+*/
+
+package v1alpha1_test
+
+import (
+	"testing"
+
+	"github.com/tektoncd/triggers/pkg/apis/triggers/v1alpha1"
+)
+
+func TestGetName(t *testing.T) {
+	for _, tc := range []struct {
+		in   v1alpha1.EventInterceptor
+		want string
+	}{{
+		in: v1alpha1.EventInterceptor{
+			CEL: &v1alpha1.CELInterceptor{},
+		},
+		want: "cel",
+	}, {
+		in: v1alpha1.EventInterceptor{
+			GitLab: &v1alpha1.GitLabInterceptor{},
+		},
+		want: "gitlab",
+	}, {
+		in: v1alpha1.EventInterceptor{
+			GitHub: &v1alpha1.GitHubInterceptor{},
+		},
+		want: "github",
+	}, {
+		in: v1alpha1.EventInterceptor{
+			Bitbucket: &v1alpha1.BitbucketInterceptor{},
+		},
+		want: "bitbucket",
+	}, {
+		in: v1alpha1.EventInterceptor{
+			Webhook: &v1alpha1.WebhookInterceptor{},
+		},
+		want: "",
+	}} {
+		t.Run(tc.want, func(t *testing.T) {
+			got := tc.in.GetName()
+			if tc.want != got {
+				t.Fatalf("GetName() want: %s; got: %s", tc.want, got)
+			}
+		})
+	}
+}

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,23 @@ 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 {
-	// This is temporary until we implement #868
-	path := ""
-	switch {
-	case i.Bitbucket != nil:
-		path = "bitbucket"
-	case i.CEL != nil:
-		path = "cel"
-	case i.GitHub != nil:
-		path = "github"
-	case i.GitLab != nil:
-		path = "gitlab"
+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, fmt.Errorf("url resolution failed for interceptor %s with: %w", name, err)
 	}
-	return &url.URL{
-		Scheme: "http",
-		Host:   fmt.Sprintf("%s.%s.svc", CoreInterceptorsHost, os.Getenv("TEKTON_INSTALL_NAMESPACE")),
-		Path:   path,
+	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{}
@@ -297,56 +300,75 @@ func makeSecret(secretText string) *corev1.Secret {
 	}
 }
 
-func TestResolvePath(t *testing.T) {
-	for _, tc := range []struct {
-		in   triggersv1.EventInterceptor
-		want string
+func TestResolveToURL(t *testing.T) {
+	tests := []struct {
+		name   string
+		getter interceptors.InterceptorGetter
+		itype  string
+		want   string
 	}{{
-		in: triggersv1.EventInterceptor{
-			CEL: &triggersv1.CELInterceptor{},
-		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/cel",
-	}, {
-		in: triggersv1.EventInterceptor{
-			GitLab: &triggersv1.GitLabInterceptor{},
-		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/gitlab",
-	}, {
-		in: triggersv1.EventInterceptor{
-			GitHub: &triggersv1.GitHubInterceptor{},
-		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/github",
-	}, {
-		in: triggersv1.EventInterceptor{
-			Bitbucket: &triggersv1.BitbucketInterceptor{},
+		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
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc/bitbucket",
+		itype: "cel",
+		want:  "http://some-host/cel",
 	}, {
-		in: triggersv1.EventInterceptor{
-			Webhook: &triggersv1.WebhookInterceptor{},
+		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
 		},
-		want: "http://tekton-triggers-core-interceptors.tekton-pipelines.svc",
-	}} {
-		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() {
-				t.Fatalf("ResolveURL() want: %s; got: %s", tc.want, got)
+		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("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{},
+	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)
 		}
 	})
 }