Add support to run PR without an event matching

Users now have the ability to explicitly start a PipelineRun using the /test comment, irrespective of whether the pipelinerun matches the annotations. This feature grants users explicit control over the execution of PipelineRuns for specific types of Pull Requests, allowing manual initiation instead of relying solely on events. This enhancement is particularly useful for scenarios where users need fine-grained control over the testing process, such as for Pull Requests managed by users rather than events. Add e2e for gitlab/github(ghe)/gitea Jira: https://issues.redhat.com/browse/SRVKP-3561 Signed-off-by: Chmouel Boudjnah <[email protected]>
openshift-pipelines · Mar 25, 2024 · 5e4489d · 5e4489d
1 parent 576b137
commit 5e4489d
Show file tree

Hide file tree

Showing 12 changed files with 340 additions and 41 deletions.
diff --git a/docs/content/docs/guide/running.md b/docs/content/docs/guide/running.md
@@ -114,14 +114,16 @@ entire suite of checks once again.
 
 ![github apps rerun check](/images/github-apps-rerun-checks.png)
 
-### GitOps command on pull or merge request
+## GitOps commands
 
-If you are targeting a push, pull or merge request you can use `GitOps` comment
-inside your pull request, to restart all or specific Pipelines.
+The GitOps commands are a way to trigger Pipelines-as-Code actions via comments
+on a `Pull Request` or comment on a `Push`ed commit.
 
-For example, you want to restart all your pipeline you can add a comment starting
-with `/retest` and all PipelineRun attached to that pull or merge request will be
-restarted :
+### GitOps commands on Pull Requests
+
+When you are on a Pull Request you may want to restart all your pipelineruns
+you can add a comment starting with `/retest` and all PipelineRuns attached to
+that pull request will be restarted :
 
 Example :
 
@@ -141,14 +143,11 @@ roses are red, violets are blue. pipeline are bound to flake by design.
 /test <pipelinerun-name>
 ```
 
-You can expose custom GitOps commands on `Pull Request` comment via the
-[on-comment]({{< relref "/docs/guide/authoringprs.md#matching-a-pipelinerun-on-a-regexp-in-a-comment" >}}) annotation.
-
-### GitOps command on push request
+### GitOps commands on pushed commits
 
-To trigger GitOps commands in response to a push request, you can include `GitOps`
-comments within your commit messages. These comments can be used to restart
-either all pipelines or specific ones. Here's how it works:
+If you want to trigger a GitOps command on a pushed commit, you can include the
+`GitOps` comments within your commit messages. These comments can be used to
+restart either all pipelines or specific ones. Here's how it works:
 
 For restarting all pipeline runs:
 
@@ -185,7 +184,7 @@ located, with the context of the **test** branch.
    3. `/retest <pipelinerun-name> branch:test`
    4. `/test <pipelinerun-name> branch:test`
 
-To add `GitOps` comments to a push request, follow these steps:
+To issue a `GitOps` comment on a pushed commit you can follow these steps:
 
 1. Go to your repository.
 2. Click on the **Commits** section.
@@ -196,12 +195,25 @@ To add `GitOps` comments to a push request, follow these steps:
 
 Please note that this feature is supported for the GitHub provider only.
 
-## Cancelling the PipelineRun
+### GitOps commands on non-matching PipelineRun
+
+The PipelineRun will be restarted regardless of the annotations if the comment
+`/test <pipelinerun-name>` or `/retest <pipelinerun-name>` is used . This let
+you have control of PipelineRuns that gets only triggered by a comment on the
+pull request.
+
+### Custom GitOps commands
+
+Using the [on-comment]({{< relref "/docs/guide/authoringprs.md#matching-a-pipelinerun-on-a-regexp-in-a-comment" >}}) annotation on your `PipelineRun` you can define custom GitOps commands that will be triggered by the comments on the pull request.
+
+See the [on-comment]({{< relref "/docs/guide/authoringprs.md#matching-a-pipelinerun-on-a-regexp-in-a-comment" >}}) guide for more information.
+
+## Cancelling a PipelineRun
 
 You can cancel a running PipelineRun by commenting on the PullRequest.
 
 For example if you want to cancel all your PipelinerRuns you can add a comment starting
-with `/cancel` and all PipelineRun attached to that pull or merge request will be cancelled.
+with `/cancel` and all PipelineRun attached to that pull request will be cancelled.
 
 Example :
 

diff --git a/pkg/pipelineascode/match.go b/pkg/pipelineascode/match.go
@@ -182,11 +182,13 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	}
 
 	// Match the PipelineRun with annotation
-	matchedPRs, err := matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx)
-	if err != nil {
-		// Don't fail when you don't have a match between pipeline and annotations
-		p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNoMatch", err.Error())
-		return nil, nil
+	var matchedPRs []matcher.Match
+	if p.event.TargetTestPipelineRun == "" {
+		if matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx); err != nil {
+			// Don't fail when you don't have a match between pipeline and annotations
+			p.eventEmitter.EmitMessage(nil, zap.WarnLevel, "RepositoryNoMatch", err.Error())
+			return nil, nil
+		}
 	}
 
 	// if the event is a comment event, but we don't have any match from the keys.OnComment then do the ACL checks again
@@ -215,13 +217,15 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 
 	// finally resolve with fetching the remote tasks (if enabled)
 	if p.run.Info.Pac.RemoteTasks {
-		// only resolve on the matched pipelineruns
-		types.PipelineRuns = nil
-		for _, match := range matchedPRs {
-			for pr := range pipelineRuns {
-				if match.PipelineRun.GetName() == "" && match.PipelineRun.GetGenerateName() == pipelineRuns[pr].GenerateName ||
-					match.PipelineRun.GetName() != "" && match.PipelineRun.GetName() == pipelineRuns[pr].Name {
-					types.PipelineRuns = append(types.PipelineRuns, pipelineRuns[pr])
+		// only resolve on the matched pipelineruns if we don't do explicit /test of unmatched pipelineruns
+		if p.event.TargetTestPipelineRun == "" {
+			types.PipelineRuns = nil
+			for _, match := range matchedPRs {
+				for pr := range pipelineRuns {
+					if match.PipelineRun.GetName() == "" && match.PipelineRun.GetGenerateName() == pipelineRuns[pr].GenerateName ||
+						match.PipelineRun.GetName() != "" && match.PipelineRun.GetName() == pipelineRuns[pr].Name {
+						types.PipelineRuns = append(types.PipelineRuns, pipelineRuns[pr])
+					}
 				}
 			}
 		}
@@ -239,6 +243,14 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	if err != nil {
 		return nil, err
 	}
+	// if we are doing explicit /test command then we only want to run the one that has matched the /test
+	if p.event.TargetTestPipelineRun != "" {
+		p.eventEmitter.EmitMessage(repo, zap.InfoLevel, "RepositoryMatchedPipelineRun", fmt.Sprintf("explicit testing via /test of PipelineRun %s", p.event.TargetTestPipelineRun))
+		return []matcher.Match{{
+			PipelineRun: pipelineRuns[0],
+			Repo:        repo,
+		}}, nil
+	}
 	matchedPRs, err = matcher.MatchPipelinerunByAnnotation(ctx, p.logger, pipelineRuns, p.run, p.event, p.vcx)
 	if err != nil {
 		// Don't fail when you don't have a match between pipeline and annotations

diff --git a/pkg/pipelineascode/match_test.go b/pkg/pipelineascode/match_test.go
@@ -85,11 +85,37 @@ func TestFilterRunningPipelineRunOnTargetTest(t *testing.T) {
 }
 
 func TestGetPipelineRunsFromRepo(t *testing.T) {
+	pullRequestEvent := &info.Event{
+		SHA:           "principale",
+		Organization:  "organizationes",
+		Repository:    "lagaffe",
+		URL:           "https://service/documentation",
+		HeadBranch:    "main",
+		BaseBranch:    "main",
+		Sender:        "fantasio",
+		EventType:     "pull_request",
+		TriggerTarget: "pull_request",
+	}
+	testExplicitNoMatchPREvent := &info.Event{
+		SHA:           "principale",
+		Organization:  "organizationes",
+		Repository:    "lagaffe",
+		URL:           "https://service/documentation",
+		HeadBranch:    "main",
+		BaseBranch:    "main",
+		Sender:        "fantasio",
+		TriggerTarget: "pull_request",
+		State: info.State{
+			TargetTestPipelineRun: "no-match",
+		},
+	}
+
 	tests := []struct {
 		name                  string
 		repositories          *v1alpha1.Repository
 		tektondir             string
 		expectedNumberOfPruns int
+		event                 *info.Event
 	}{
 		{
 			name: "more than one pipelinerun in .tekton dir",
@@ -102,6 +128,7 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 			},
 			tektondir:             "testdata/pull_request_multiplepipelineruns",
 			expectedNumberOfPruns: 2,
+			event:                 pullRequestEvent,
 		},
 		{
 			name: "single pipelinerun in .tekton dir",
@@ -114,6 +141,37 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 			},
 			tektondir:             "testdata/pull_request",
 			expectedNumberOfPruns: 1,
+			event:                 pullRequestEvent,
+		},
+		{
+			name: "no-match pipelineruns in .tekton dir, only matched should be returned",
+			repositories: &v1alpha1.Repository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "testrepo",
+					Namespace: "test",
+				},
+				Spec: v1alpha1.RepositorySpec{},
+			},
+			// we have 3 PR in there 2 that has a match on pull request and 1 that is a no-matching
+			// matching those two that is matching here
+			tektondir:             "testdata/no-match",
+			expectedNumberOfPruns: 2,
+			event:                 pullRequestEvent,
+		},
+		{
+			name: "no-match pipelineruns in .tekton dir, only match the no-match",
+			repositories: &v1alpha1.Repository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "testrepo",
+					Namespace: "test",
+				},
+				Spec: v1alpha1.RepositorySpec{},
+			},
+			// we have 3 PR in there 2 that has a match on pull request and 1 that is a no-matching
+			// matching that only one here
+			tektondir:             "testdata/no-match",
+			expectedNumberOfPruns: 1,
+			event:                 testExplicitNoMatchPREvent,
 		},
 	}
 	for _, tt := range tests {
@@ -124,19 +182,8 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 			fakeclient, mux, _, teardown := ghtesthelper.SetupGH()
 			defer teardown()
 
-			runevent := &info.Event{
-				SHA:           "principale",
-				Organization:  "organizationes",
-				Repository:    "lagaffe",
-				URL:           "https://service/documentation",
-				HeadBranch:    "main",
-				BaseBranch:    "main",
-				Sender:        "fantasio",
-				EventType:     "pull_request",
-				TriggerTarget: "pull_request",
-			}
 			if tt.tektondir != "" {
-				ghtesthelper.SetupGitTree(t, mux, tt.tektondir, runevent, false)
+				ghtesthelper.SetupGitTree(t, mux, tt.tektondir, tt.event, false)
 			}
 
 			stdata, _ := testclient.SeedTestData(t, ctx, testclient.Data{})
@@ -165,7 +212,7 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 				Token:  github.String("None"),
 				Logger: logger,
 			}
-			p := NewPacs(runevent, vcx, cs, k8int, logger)
+			p := NewPacs(tt.event, vcx, cs, k8int, logger)
 			matchedPRs, err := p.getPipelineRunsFromRepo(ctx, tt.repositories)
 			assert.NilError(t, err)
 			matchedPRNames := []string{}

diff --git a/pkg/pipelineascode/testdata/no-match/.tekton/matched1.yaml b/pkg/pipelineascode/testdata/no-match/.tekton/matched1.yaml
@@ -0,0 +1,10 @@
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: pull_request-1
+  annotations:
+    pipelinesascode.tekton.dev/on-target-branch: "[main]"
+    pipelinesascode.tekton.dev/on-event: "[pull_request]"
+spec:
+  pipelineRef:
+    name: pipeline1
diff --git a/pkg/pipelineascode/testdata/no-match/.tekton/matched2.yaml b/pkg/pipelineascode/testdata/no-match/.tekton/matched2.yaml
@@ -0,0 +1,10 @@
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: pull_request-2
+  annotations:
+    pipelinesascode.tekton.dev/on-target-branch: "[main]"
+    pipelinesascode.tekton.dev/on-event: "[pull_request]"
+spec:
+  pipelineRef:
+    name: pipeline1
diff --git a/pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml b/pkg/pipelineascode/testdata/no-match/.tekton/nomatch.yaml
@@ -0,0 +1,7 @@
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: no-match
+spec:
+  pipelineRef:
+    name: pipeline1
diff --git a/pkg/pipelineascode/testdata/no-match/.tekton/pipeline.yaml b/pkg/pipelineascode/testdata/no-match/.tekton/pipeline.yaml
@@ -0,0 +1,9 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: Pipeline
+metadata:
+  name: pipeline1
+tasks:
+  - name: task-from-tektondir
+    taskRef:
+      name: task-from-tektondir
diff --git a/pkg/pipelineascode/testdata/no-match/.tekton/task.yaml b/pkg/pipelineascode/testdata/no-match/.tekton/task.yaml
@@ -0,0 +1,12 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: task-from-tektondir
+spec:
+  steps:
+    - name: task-1
+      image: gcr.io/distroless/python3:nonroot
+      script: |
+        #!/usr/bin/python3
+        print("Hello task-from-tektondir")
diff --git a/test/gitea_test.go b/test/gitea_test.go
@@ -814,6 +814,82 @@ func TestGiteaOnCommentAnnotation(t *testing.T) {
 	assert.NilError(t, err)
 }
 
+// TestGiteaTestPipelineRunExplicitelyWithTestComment will test a pipelinerun
+// even if it hasn't matched when we are doing a /test comment.
+func TestGiteaTestPipelineRunExplicitelyWithTestComment(t *testing.T) {
+	var err error
+	ctx := context.Background()
+	topts := &tgitea.TestOpts{
+		TargetRefName: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test"),
+	}
+	topts.TargetNS = topts.TargetRefName
+	topts.ParamsRun, topts.Opts, topts.GiteaCNX, err = tgitea.Setup(ctx)
+	assert.NilError(t, err, fmt.Errorf("cannot do gitea setup: %w", err))
+	ctx, err = cctx.GetControllerCtxInfo(ctx, topts.ParamsRun)
+	assert.NilError(t, err)
+	assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, topts.ParamsRun))
+	assert.NilError(t, secret.Create(ctx, topts.ParamsRun, map[string]string{"secret": "SHHHHHHH"}, topts.TargetNS, "pac-secret"))
+	topts.TargetEvent = triggertype.PullRequest.String()
+	topts.YAMLFiles = map[string]string{
+		".tekton/pr.yaml": "testdata/pipelinerun-nomatch.yaml",
+	}
+	_, f := tgitea.TestPR(t, topts)
+	defer f()
+	tgitea.PostCommentOnPullRequest(t, topts, "/test no-match")
+	tgitea.WaitForStatus(t, topts, "heads/"+topts.TargetRefName, "", false)
+	waitOpts := twait.Opts{
+		RepoName:        topts.TargetNS,
+		Namespace:       topts.TargetNS,
+		MinNumberStatus: 1,
+		PollTimeout:     twait.DefaultTimeout,
+	}
+
+	repo, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)
+	assert.NilError(t, err)
+	assert.Equal(t, len(repo.Status), 1, "should have only 1 status")
+	assert.Equal(t, *repo.Status[0].EventType, opscomments.TestSingleCommentEventType.String(), "should have a test comment event in status")
+}
+
+func TestGiteaRetestAll(t *testing.T) {
+	var err error
+	ctx := context.Background()
+	topts := &tgitea.TestOpts{
+		TargetRefName: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test"),
+	}
+	topts.TargetNS = topts.TargetRefName
+	topts.ParamsRun, topts.Opts, topts.GiteaCNX, err = tgitea.Setup(ctx)
+	assert.NilError(t, err, fmt.Errorf("cannot do gitea setup: %w", err))
+	ctx, err = cctx.GetControllerCtxInfo(ctx, topts.ParamsRun)
+	assert.NilError(t, err)
+	assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, topts.ParamsRun))
+	assert.NilError(t, secret.Create(ctx, topts.ParamsRun, map[string]string{"secret": "SHHHHHHH"}, topts.TargetNS, "pac-secret"))
+	topts.TargetEvent = triggertype.PullRequest.String()
+	topts.YAMLFiles = map[string]string{
+		".tekton/pr.yaml":      "testdata/pipelinerun.yaml",
+		".tekton/nomatch.yaml": "testdata/pipelinerun-nomatch.yaml",
+	}
+	_, f := tgitea.TestPR(t, topts)
+	defer f()
+	tgitea.PostCommentOnPullRequest(t, topts, "/retest")
+	waitOpts := twait.Opts{
+		RepoName:        topts.TargetNS,
+		Namespace:       topts.TargetNS,
+		MinNumberStatus: 2,
+		PollTimeout:     twait.DefaultTimeout,
+	}
+
+	repo, err := twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)
+	assert.NilError(t, err)
+	var rt bool
+	for _, status := range repo.Status {
+		if *status.EventType == opscomments.RetestAllCommentEventType.String() {
+			rt = true
+		}
+	}
+	assert.Assert(t, rt, "should have a retest all comment event in status")
+	assert.Equal(t, len(repo.Status), 2, "should have only 2 status")
+}
+
 // Local Variables:
 // compile-command: "go test -tags=e2e -v -run TestGiteaPush ."
 // End: