Repository: openshift-pipelines/pipelines-as-code
Pull request #139 (2021-06-17 13:06:22)

Properly match on forcenamespace
Signed-off-by: Chmouel Boudjnah <chmouel@redhat.com>



Patch:
diff --git a/pkg/config/annotation_matcher.go b/pkg/config/annotation_matcher.go
index 59a56faf..8548fa69 100644
--- a/pkg/config/annotation_matcher.go
+++ b/pkg/config/annotation_matcher.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode"
+	apipac "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/cli"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/webvcs"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
@@ -42,8 +43,9 @@ func getAnnotationValues(annotation string) ([]string, error) {
 }
 
 func MatchPipelinerunByAnnotation(ctx context.Context, pruns []*v1beta1.PipelineRun, cs *cli.Clients,
-	runinfo *webvcs.RunInfo) (*v1beta1.PipelineRun, error) {
+	runinfo *webvcs.RunInfo) (*v1beta1.PipelineRun, *apipac.Repository, error) {
 	configurations := map[string]map[string]string{}
+	repo := &apipac.Repository{}
 	for _, prun := range pruns {
 		configurations[prun.GetGenerateName()] = map[string]string{}
 		if prun.GetObjectMeta().GetAnnotations() == nil {
@@ -54,7 +56,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, pruns []*v1beta1.Pipeline
 		if targetNS, ok := prun.GetObjectMeta().GetAnnotations()[pipelinesascode.
 			GroupName+"/"+onTargetNamespace]; ok {
 			configurations[prun.GetGenerateName()]["target-namespace"] = targetNS
-			repo, _ := GetRepoByCR(ctx, cs, targetNS, runinfo)
+			repo, _ = GetRepoByCR(ctx, cs, targetNS, runinfo)
 			if repo == nil {
 				cs.Log.Warnf("could not find Repository CRD in %s while pipelineRun %s targets it", targetNS, prun.GetGenerateName())
 				continue
@@ -66,7 +68,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, pruns []*v1beta1.Pipeline
 			matched, err := matchOnAnnotation(targetEvent, runinfo.EventType, false)
 			configurations[prun.GetGenerateName()]["target-event"] = targetEvent
 			if err != nil {
-				return nil, err
+				return nil, nil, err
 			}
 			if !matched {
 				continue
@@ -78,14 +80,14 @@ func MatchPipelinerunByAnnotation(ctx context.Context, pruns []*v1beta1.Pipeline
 			matched, err := matchOnAnnotation(targetBranch, runinfo.BaseBranch, true)
 			configurations[prun.GetGenerateName()]["target-branch"] = targetBranch
 			if err != nil {
-				return nil, err
+				return nil, nil, err
 			}
 			if !matched {
 				continue
 			}
 		}
 
-		return prun, nil
+		return prun, repo, nil
 	}
 
 	cs.Log.Infof("cannot match between event and pipelineRuns: URL=%s baseBranch=%s, "+
@@ -100,7 +102,7 @@ func MatchPipelinerunByAnnotation(ctx context.Context, pruns []*v1beta1.Pipeline
 	}
 
 	// TODO: more descriptive error message
-	return nil, fmt.Errorf("cannot match pipeline from webhook to pipelineruns")
+	return nil, nil, fmt.Errorf("cannot match pipeline from webhook to pipelineruns")
 }
 
 func matchOnAnnotation(annotations string, runinfoValue string, branchMatching bool) (bool, error) {
diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go
index a45ccb1b..a3e58153 100644
--- a/pkg/pipelineascode/pipelineascode.go
+++ b/pkg/pipelineascode/pipelineascode.go
@@ -77,7 +77,7 @@ func Run(ctx context.Context, cs *cli.Clients, k8int cli.KubeInteractionIntf, ru
 		return err
 	}
 
-	if repo == nil {
+	if repo == nil || repo.Spec.Namespace == "" {
 		msg := fmt.Sprintf("Could not find a namespace match for %s/%s on target-branch:%s event-type: %s", runinfo.Owner, runinfo.Repository, runinfo.BaseBranch, runinfo.EventType)
 		if runinfo.EventType == "pull_request" || runinfo.TriggerTarget == "issue-recheck" {
 			err = createStatus(ctx, cs, runinfo, "completed", "skipped", msg, "https://tenor.com/search/sad-cat-gifs", true)
@@ -146,10 +146,13 @@ func Run(ctx context.Context, cs *cli.Clients, k8int cli.KubeInteractionIntf, ru
 	}
 
 	// Match the pipelinerun with annotation
-	pipelineRun, err := config.MatchPipelinerunByAnnotation(ctx, pipelineRuns, cs, runinfo)
+	pipelineRun, annotationRepo, err := config.MatchPipelinerunByAnnotation(ctx, pipelineRuns, cs, runinfo)
 	if err != nil {
 		return err
 	}
+	if annotationRepo.Spec.Namespace != "" {
+		repo = annotationRepo
+	}
 
 	// Add labels on the soon to be created pipelinerun so UI/CLI can easily
 	// query them. Since K8s do not like slash in labels value and on push we


Test patch:
diff --git a/pkg/config/annotation_matcher_test.go b/pkg/config/annotation_matcher_test.go
index e3d93625..097d5f7d 100644
--- a/pkg/config/annotation_matcher_test.go
+++ b/pkg/config/annotation_matcher_test.go
@@ -39,13 +39,13 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
 		data    testclient.Data
 	}
 	tests := []struct {
-		name, wantRepoName, wantLog string
-		args                        args
-		wantErr                     bool
+		name, wantPRName, wantRepoName, wantLog string
+		args                                    args
+		wantErr                                 bool
 	}{
 		{
-			name:         "match a repository with target NS",
-			wantRepoName: pipelineTargetNSName,
+			name:       "match a repository with target NS",
+			wantPRName: pipelineTargetNSName,
 			args: args{
 				pruns:   []*tektonv1beta1.PipelineRun{pipelineTargetNS},
 				runinfo: &webvcs.RunInfo{URL: targetURL, EventType: "pull_request", BaseBranch: mainBranch},
@@ -56,6 +56,21 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:         "match same webhook on multiple repos with different NS",
+			wantPRName:   pipelineTargetNSName,
+			wantRepoName: "test-good",
+			args: args{
+				pruns:   []*tektonv1beta1.PipelineRun{pipelineTargetNS},
+				runinfo: &webvcs.RunInfo{URL: targetURL, EventType: "pull_request", BaseBranch: mainBranch},
+				data: testclient.Data{
+					Repositories: []*v1alpha1.Repository{
+						testnewrepo.NewRepo("test-other", targetURL, mainBranch, targetNamespace, targetNamespace, "pull_request"),
+						testnewrepo.NewRepo("test-good", targetURL, mainBranch, targetNamespace, targetNamespace, "pull_request"),
+					},
+				},
+			},
+		},
 		{
 			name:    "no match a repository with target NS",
 			wantErr: true,
@@ -79,7 +94,7 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
 			observer, log := zapobserver.New(zap.InfoLevel)
 			logger := zap.New(observer).Sugar()
 			client := &cli.Clients{PipelineAsCode: cs.PipelineAsCode, Log: logger}
-			got, err := MatchPipelinerunByAnnotation(ctx,
+			got, repo, err := MatchPipelinerunByAnnotation(ctx,
 				tt.args.pruns,
 				client, tt.args.runinfo)
 
@@ -92,7 +107,10 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
 			}
 
 			if tt.wantRepoName != "" {
-				assert.Assert(t, tt.wantRepoName == got.GetName())
+				assert.Assert(t, tt.wantRepoName == repo.GetName())
+			}
+			if tt.wantPRName != "" {
+				assert.Assert(t, tt.wantPRName == got.GetName())
 			}
 			if tt.wantLog != "" {
 				logmsg := log.TakeAll()
@@ -303,7 +321,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
-			got, err := MatchPipelinerunByAnnotation(ctx, tt.args.pruns, cs, tt.args.runinfo)
+			got, _, err := MatchPipelinerunByAnnotation(ctx, tt.args.pruns, cs, tt.args.runinfo)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("MatchPipelinerunByAnnotation() error = %v, wantErr %v", err, tt.wantErr)
 				return
diff --git a/pkg/config/repo_runinfo_matcher_test.go b/pkg/config/repo_runinfo_matcher_test.go
index d860a7de..42a8e88e 100644
--- a/pkg/config/repo_runinfo_matcher_test.go
+++ b/pkg/config/repo_runinfo_matcher_test.go
@@ -135,9 +135,6 @@ func Test_getRepoByCR(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if tt.name != "test-nomatch-event-type" {
-				return
-			}
 			ctx, _ := rtesting.SetupFakeContext(t)
 			cs, _ := testclient.SeedTestData(t, ctx, tt.args.data)
 			observer, _ := zapobserver.New(zap.InfoLevel)