Repository: openshift-pipelines/pipelines-as-code
Pull request #68 (2021-05-20 12:42:15)

Make sure we match the eventType
when matching event to pipeline we were not matching properly the
eventType so that would not be filtered properly.

Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>



Patch:
diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go
index 4fbfc673..14d5329a 100644
--- a/pkg/pipelineascode/pipelineascode.go
+++ b/pkg/pipelineascode/pipelineascode.go
@@ -24,7 +24,7 @@ type Options struct {
 	RunInfo     webvcs.RunInfo
 }
 
-func getRepoByCR(ctx context.Context, cs *cli.Clients, url, branch, forceNamespace string) (apipac.Repository, error) {
+func getRepoByCR(ctx context.Context, cs *cli.Clients, runinfo *webvcs.RunInfo) (apipac.Repository, error) {
 	var repository apipac.Repository
 
 	repositories, err := cs.PipelineAsCode.PipelinesascodeV1alpha1().Repositories("").List(
@@ -32,13 +32,17 @@ func getRepoByCR(ctx context.Context, cs *cli.Clients, url, branch, forceNamespa
 	if err != nil {
 		return repository, err
 	}
+
 	for _, value := range repositories.Items {
-		if value.Spec.URL == url && value.Spec.Branch == branch {
-			if forceNamespace != "" && value.Namespace != forceNamespace {
-				return repository, fmt.Errorf(
-					"repo CR matches but should be installed in %s as configured from tekton.yaml on the main branch",
-					forceNamespace)
-			}
+		if value.Spec.URL == runinfo.URL && value.Spec.Branch == runinfo.BaseBranch &&
+			value.Spec.EventType == runinfo.EventType {
+
+			// TODO: figure it out when we renable forceNamespace
+			// if forceNamespace != "" && value.Namespace != forceNamespace {
+			//	return repository, fmt.Errorf(
+			//		"repo CR matches but should be installed in %s as configured from tekton.yaml on the main branch",
+			//		forceNamespace)
+			// }
 
 			// Disallow attempts for hijacks. If the installed CR is not configured on the
 			// Namespace the Spec is targeting then disallow it.
@@ -99,7 +103,7 @@ func Run(ctx context.Context, cs *cli.Clients, k8int cli.KubeInteractionIntf, ru
 
 	// Match the Event to a Repository Resource,
 	// TODO: we need to be able to force a Namespace from the configuration as annotation as an extra layer of security for // hijacking
-	repo, err := getRepoByCR(ctx, cs, runinfo.URL, runinfo.BaseBranch, "")
+	repo, err := getRepoByCR(ctx, cs, runinfo)
 	if err != nil {
 		return err
 	}


Test patch:
diff --git a/pkg/pipelineascode/pipelineascode_test.go b/pkg/pipelineascode/pipelineascode_test.go
index fe97344b..e2dc29a4 100644
--- a/pkg/pipelineascode/pipelineascode_test.go
+++ b/pkg/pipelineascode/pipelineascode_test.go
@@ -31,7 +31,7 @@ import (
 	rtesting "knative.dev/pkg/reconciler/testing"
 )
 
-func newRepo(name, url, branch, installNamespace, namespace string) *v1alpha1.Repository {
+func newRepo(name, url, branch, installNamespace, namespace, eventtype string) *v1alpha1.Repository {
 	cw := clockwork.NewFakeClock()
 	return &v1alpha1.Repository{
 		TypeMeta: metav1.TypeMeta{},
@@ -43,6 +43,7 @@ func newRepo(name, url, branch, installNamespace, namespace string) *v1alpha1.Re
 			Namespace: namespace,
 			URL:       url,
 			Branch:    branch,
+			EventType: eventtype,
 		},
 		Status: []v1alpha1.RepositoryRunStatus{
 			{
@@ -94,12 +95,12 @@ func TestFilterByGood(t *testing.T) {
 	url := "https://psg.fr"
 	d := testclient.Data{
 		Repositories: []*v1alpha1.Repository{
-			newRepo("test-good", url, branch, targetNamespace, targetNamespace),
+			newRepo("test-good", url, branch, targetNamespace, targetNamespace, "pull_request"),
 		},
 	}
 	cs, _ := testclient.SeedTestData(t, ctx, d)
 	client := &cli.Clients{PipelineAsCode: cs.PipelineAsCode}
-	repo, err := getRepoByCR(ctx, client, url, branch, "")
+	repo, err := getRepoByCR(ctx, client, &webvcs.RunInfo{URL: url, BaseBranch: branch, EventType: "pull_request"})
 	assert.NilError(t, err)
 	assert.Equal(t, repo.Spec.Namespace, targetNamespace)
 }
@@ -114,12 +115,13 @@ func TestFilterByNotMatch(t *testing.T) {
 	otherurl := "https://marseille.com"
 	d := testclient.Data{
 		Repositories: []*v1alpha1.Repository{
-			newRepo("test-notmatch", url, branch, targetNamespace, targetNamespace),
+			newRepo("test-notmatch", url, branch, targetNamespace, targetNamespace, "pull_request"),
 		},
 	}
 	cs, _ := testclient.SeedTestData(t, ctx, d)
 	client := &cli.Clients{PipelineAsCode: cs.PipelineAsCode}
-	repo, err := getRepoByCR(ctx, client, otherurl, branch, "")
+
+	repo, err := getRepoByCR(ctx, client, &webvcs.RunInfo{URL: otherurl, BaseBranch: branch})
 	assert.NilError(t, err)
 	assert.Equal(t, repo.Spec.Namespace, "")
 }
@@ -134,33 +136,33 @@ func TestFilterByNotInItsNamespace(t *testing.T) {
 	url := "https://psg.fr"
 	d := testclient.Data{
 		Repositories: []*v1alpha1.Repository{
-			newRepo(testname, url, branch, installNamespace, targetNamespace),
+			newRepo(testname, url, branch, installNamespace, targetNamespace, "pull_request"),
 		},
 	}
 	cs, _ := testclient.SeedTestData(t, ctx, d)
 	client := &cli.Clients{PipelineAsCode: cs.PipelineAsCode}
-	repo, err := getRepoByCR(ctx, client, url, branch, "")
+	repo, err := getRepoByCR(ctx, client, &webvcs.RunInfo{URL: url, EventType: "pull_request", BaseBranch: branch})
 	assert.ErrorContains(t, err, fmt.Sprintf("repo CR %s matches but belongs to", testname))
 	assert.Equal(t, repo.Spec.Namespace, "")
 }
 
 func TestFilterForceNamespace(t *testing.T) {
 	// The CR should belong to the namespace it target
+	t.Skip("TODO: to renable when we get there")
 	ctx, _ := rtesting.SetupFakeContext(t)
 	testname := "test-not-in-its-namespace"
 	branch := "mainone"
 	targetNamespace := "namespace"
-	forcedNamespace := "asmonaco"
 
 	url := "https://psg.fr"
 	d := testclient.Data{
 		Repositories: []*v1alpha1.Repository{
-			newRepo(testname, url, branch, targetNamespace, targetNamespace),
+			newRepo(testname, url, branch, targetNamespace, targetNamespace, "pull_request"),
 		},
 	}
 	cs, _ := testclient.SeedTestData(t, ctx, d)
 	client := &cli.Clients{PipelineAsCode: cs.PipelineAsCode}
-	_, err := getRepoByCR(ctx, client, url, branch, forcedNamespace)
+	_, err := getRepoByCR(ctx, client, &webvcs.RunInfo{URL: url, BaseBranch: branch})
 	assert.ErrorContains(t, err, "as configured from tekton.yaml on the main branch")
 }
 
@@ -210,7 +212,7 @@ func TestRunDeniedFromForcedNamespace(t *testing.T) {
 	}
 	datas := testclient.Data{
 		Repositories: []*v1alpha1.Repository{
-			newRepo("repo", runinfo.URL, runinfo.BaseBranch, installedNamespace, installedNamespace),
+			newRepo("repo", runinfo.URL, runinfo.BaseBranch, installedNamespace, installedNamespace, "pull_request"),
 		},
 	}
 	stdata, _ := testclient.SeedTestData(t, ctx, datas)
@@ -396,9 +398,7 @@ func TestRun(t *testing.T) {
 			finalStatusText: "not find a namespace match",
 			repositories: []*v1alpha1.Repository{
 				newRepo("test-run", "https://nowhere.com",
-					"a branch",
-					"namespace",
-					"namespace"),
+					"a branch", "namespace", "namespace", "pull_request"),
 			},
 		},
 		{
@@ -424,7 +424,7 @@ func TestRun(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
 			if tt.repositories == nil {
 				tt.repositories = []*v1alpha1.Repository{
-					newRepo("test-run", tt.runinfo.URL, tt.runinfo.BaseBranch, "namespace", "namespace"),
+					newRepo("test-run", tt.runinfo.URL, tt.runinfo.BaseBranch, "namespace", "namespace", tt.runinfo.EventType),
 				}
 			}
 			tdata := testclient.Data{