DNM: release 0.33.2

docs/content/docs/guide/gitops_commands.md

@@ -70,6 +70,9 @@ This means:
    3. `/retest <pipelinerun-name> branch:test`
    4. `/test <pipelinerun-name> branch:test`
 
+Please note that the `/ok-to-test` command does not work on pushed commits, as it is specifically intended for pull requests to manage authorization. Since only authorized users are allowed to send `GitOps` commands on pushed commits,
+there is no need to use the `ok-to-test` command in this context.
+
 To issue a `GitOps` comment on a pushed commit, you can follow these steps:
 
 1. Go to your repository.

pkg/provider/bitbucketserver/parse_payload.go

@@ -159,6 +159,11 @@ func (v *Provider) ParsePayload(_ context.Context, _ *params.Run, request *http.
 		processedEvent.EventType = triggertype.Push.String()
 		processedEvent.Organization = e.Repository.Project.Key
 		processedEvent.Repository = e.Repository.Slug
+
+		if len(e.Changes) == 0 {
+			return nil, fmt.Errorf("push event contains no commits under 'changes'; cannot proceed")
+		}
+
 		processedEvent.SHA = e.Changes[0].ToHash
 		processedEvent.URL = e.Repository.Links.Self[0].Href
 		processedEvent.BaseBranch = e.Changes[0].RefID

pkg/provider/bitbucketserver/parse_payload_test.go

@@ -612,9 +612,16 @@ func TestParsePayload(t *testing.T) {
 		{
 			name:         "good/push",
 			eventType:    "repo:refs_changed",
-			payloadEvent: bbv1test.MakePushEvent(ev1),
+			payloadEvent: bbv1test.MakePushEvent(ev1, []types.PushRequestEventChange{{ToHash: ev1.SHA, RefID: "base"}}),
 			expEvent:     ev1,
 		},
+		{
+			name:          "bad/changes are empty in push",
+			eventType:     "repo:refs_changed",
+			payloadEvent:  bbv1test.MakePushEvent(ev1, []types.PushRequestEventChange{}),
+			expEvent:      ev1,
+			wantErrSubstr: "push event contains no commits under 'changes'; cannot proceed",
+		},
 		{
 			name:         "good/comment ok-to-test",
 			eventType:    "pr:comment:added",

pkg/provider/bitbucketserver/test/test.go

@@ -330,7 +330,7 @@ func MakePREvent(event *info.Event, comment string) *types.PullRequestEvent {
 	return pr
 }
 
-func MakePushEvent(event *info.Event) *types.PushRequestEvent {
+func MakePushEvent(event *info.Event, changes []types.PushRequestEventChange) *types.PushRequestEvent {
 	iii, _ := strconv.Atoi(event.AccountID)
 
 	return &types.PushRequestEvent{
@@ -357,11 +357,6 @@ func MakePushEvent(event *info.Event) *types.PushRequestEvent {
 				},
 			},
 		},
-		Changes: []types.PushRequestEventChange{
-			{
-				ToHash: event.SHA,
-				RefID:  "base",
-			},
-		},
+		Changes: changes,
 	}
 }

pkg/provider/github/detect.go

@@ -102,12 +102,13 @@ func (v *Provider) detectTriggerTypeFromPayload(ghEventType string, eventInt any
 			if provider.IsTestRetestComment(event.GetComment().GetBody()) {
 				return triggertype.Retest, ""
 			}
-			if provider.IsOkToTestComment(event.GetComment().GetBody()) {
-				return triggertype.OkToTest, ""
-			}
 			if provider.IsCancelComment(event.GetComment().GetBody()) {
 				return triggertype.Cancel, ""
 			}
+			// Here, the `/ok-to-test` command is ignored because it is intended for pull requests.
+			// For unauthorized users, it has no relevance to pushed commits, as only authorized users
+			// are allowed to run CI on pushed commits. Therefore, the `ok-to-test` command holds no significance in this context.
+			// However, it is left to be processed by the `on-comment` annotation rather than returning an error.
 		}
 		return triggertype.Comment, ""
 	}

pkg/provider/github/detect_test.go

@@ -295,6 +295,19 @@ func TestProvider_Detect(t *testing.T) {
 			isGH:       true,
 			processReq: true,
 		},
+		{
+			name: "commit comment Event with /ok-to-test being ignore as GitOps command on pushed commits",
+			event: github.CommitCommentEvent{
+				Action: github.Ptr("created"),
+				Installation: &github.Installation{
+					ID: &idd,
+				},
+				Comment: &github.RepositoryComment{Body: github.Ptr("/ok-to-test")},
+			},
+			eventType:  "commit_comment",
+			isGH:       true,
+			processReq: true,
+		},
 	}
 
 	for _, tt := range tests {

pkg/provider/gitlab/acl_test.go

@@ -108,7 +108,7 @@ func TestIsAllowed(t *testing.T) {
 					thelp.MuxDisallowUserID(mux, tt.fields.targetProjectID, tt.allowMemberID)
 				}
 				if tt.ownerFile != "" {
-					thelp.MuxGetFile(mux, tt.fields.targetProjectID, "OWNERS", tt.ownerFile)
+					thelp.MuxGetFile(mux, tt.fields.targetProjectID, "OWNERS", tt.ownerFile, false)
 				}
 				if tt.commentContent != "" {
 					thelp.MuxDiscussionsNote(mux, tt.fields.targetProjectID,

pkg/provider/gitlab/gitlab.go

@@ -295,16 +295,16 @@ func (v *Provider) GetTektonDir(_ context.Context, event *info.Event, path, prov
 		}
 	}
 
-	return v.concatAllYamlFiles(nodes, event)
+	return v.concatAllYamlFiles(nodes, revision)
 }
 
 // concatAllYamlFiles concat all yaml files from a directory as one big multi document yaml string.
-func (v *Provider) concatAllYamlFiles(objects []*gitlab.TreeNode, runevent *info.Event) (string, error) {
+func (v *Provider) concatAllYamlFiles(objects []*gitlab.TreeNode, revision string) (string, error) {
 	var allTemplates string
 	for _, value := range objects {
 		if strings.HasSuffix(value.Name, ".yaml") ||
 			strings.HasSuffix(value.Name, ".yml") {
-			data, _, err := v.getObject(value.Path, runevent.HeadBranch, v.sourceProjectID)
+			data, _, err := v.getObject(value.Path, revision, v.sourceProjectID)
 			if err != nil {
 				return "", err
 			}

pkg/provider/gitlab/gitlab_test.go

@@ -399,6 +399,8 @@ func TestGetTektonDir(t *testing.T) {
 		args                 args
 		wantStr              string
 		wantErr              string
+		wantTreeAPIErr       bool
+		wantFilesAPIErr      bool
 		wantClient           bool
 		prcontent            string
 		filterMessageSnippet string
@@ -472,6 +474,38 @@ func TestGetTektonDir(t *testing.T) {
 			wantClient: true,
 			wantStr:    "kind: PipelineRun",
 		},
+		{
+			name:      "list tekton dir tree api call error",
+			prcontent: strings.TrimPrefix(string(samplePR), "---"),
+			args: args{
+				path: ".tekton",
+				event: &info.Event{
+					HeadBranch: "main",
+				},
+			},
+			fields: fields{
+				sourceProjectID: 100,
+			},
+			wantClient:     true,
+			wantTreeAPIErr: true,
+			wantErr:        "failed to list .tekton dir",
+		},
+		{
+			name:      "get file raw api call error",
+			prcontent: strings.TrimPrefix(string(samplePR), "---"),
+			args: args{
+				path: ".tekton",
+				event: &info.Event{
+					HeadBranch: "main",
+				},
+			},
+			fields: fields{
+				sourceProjectID: 100,
+			},
+			wantClient:      true,
+			wantFilesAPIErr: true,
+			wantErr:         "failed to get filename from api pr.yaml dir",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -493,7 +527,7 @@ func TestGetTektonDir(t *testing.T) {
 					muxbranch = tt.args.event.DefaultBranch
 				}
 				if tt.args.path != "" && tt.prcontent != "" {
-					thelp.MuxListTektonDir(t, mux, tt.fields.sourceProjectID, muxbranch, tt.prcontent)
+					thelp.MuxListTektonDir(t, mux, tt.fields.sourceProjectID, muxbranch, tt.prcontent, tt.wantTreeAPIErr, tt.wantFilesAPIErr)
 				}
 				defer tearDown()
 			}
@@ -528,7 +562,7 @@ func TestGetFileInsideRepo(t *testing.T) {
 		sourceProjectID: 10,
 		Client:          client,
 	}
-	thelp.MuxListTektonDir(t, mux, v.sourceProjectID, event.HeadBranch, content)
+	thelp.MuxListTektonDir(t, mux, v.sourceProjectID, event.HeadBranch, content, false, false)
 	got, err := v.GetFileInsideRepo(ctx, event, "pr.yaml", "")
 	assert.NilError(t, err)
 	assert.Equal(t, content, got)

pkg/provider/gitlab/test/test.go

@@ -66,8 +66,12 @@ func MuxDisallowUserID(mux *http.ServeMux, projectID, userID int) {
 	})
 }
 
-func MuxListTektonDir(_ *testing.T, mux *http.ServeMux, pid int, ref, prs string) {
+func MuxListTektonDir(_ *testing.T, mux *http.ServeMux, pid int, ref, prs string, wantTreeAPIErr, wantFilesAPIErr bool) {
 	mux.HandleFunc(fmt.Sprintf("/projects/%d/repository/tree", pid), func(rw http.ResponseWriter, r *http.Request) {
+		if wantTreeAPIErr {
+			rw.WriteHeader(http.StatusUnauthorized)
+			return
+		}
 		if r.URL.Query().Get("pagination") == "keyset" {
 			if r.URL.Query().Get("ref") == ref {
 				if r.URL.Query().Get("page_token") != "page2" {
@@ -87,8 +91,8 @@ func MuxListTektonDir(_ *testing.T, mux *http.ServeMux, pid int, ref, prs string
 		}
 	})
 
-	MuxGetFile(mux, pid, "pr.yaml", prs)
-	MuxGetFile(mux, pid, "random.yaml", `foo:bar`)
+	MuxGetFile(mux, pid, "pr.yaml", prs, wantFilesAPIErr)
+	MuxGetFile(mux, pid, "random.yaml", `foo:bar`, wantTreeAPIErr)
 }
 
 func MuxDiscussionsNoteEmpty(mux *http.ServeMux, pid, mrID int) {
@@ -120,8 +124,12 @@ func MuxDiscussionsNote(mux *http.ServeMux, pid, mrID int, author string, author
 	})
 }
 
-func MuxGetFile(mux *http.ServeMux, pid int, fname, content string) {
+func MuxGetFile(mux *http.ServeMux, pid int, fname, content string, wantErr bool) {
 	mux.HandleFunc(fmt.Sprintf("/projects/%d/repository/files/%s/raw", pid, fname), func(rw http.ResponseWriter, _ *http.Request) {
+		if wantErr {
+			rw.WriteHeader(http.StatusUnauthorized)
+			return
+		}
 		fmt.Fprint(rw, content)
 	})
 }

test/gitea_test.go

@@ -14,9 +14,7 @@ import (
 
 	"code.gitea.io/sdk/gitea"
 	"github.com/google/go-github/v68/github"
-	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/names"
-	yaml "gopkg.in/yaml.v2"
 	"gotest.tools/v3/assert"
 	"gotest.tools/v3/env"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -30,7 +28,6 @@ import (
 	tknpaclist "github.com/openshift-pipelines/pipelines-as-code/pkg/cmd/tknpac/list"
 	tknpacresolve "github.com/openshift-pipelines/pipelines-as-code/pkg/cmd/tknpac/resolve"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/git"
-	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
@@ -869,22 +866,6 @@ func TestGiteaErrorSnippetWithSecret(t *testing.T) {
 	tgitea.WaitForPullRequestCommentMatch(t, topts)
 }
 
-// TestGiteaNotExistingClusterTask checks that the pipeline run fails if the clustertask does not exist
-// This will test properly if we error the reason in UI see bug #1160.
-func TestGiteaNotExistingClusterTask(t *testing.T) {
-	topts := &tgitea.TestOpts{
-		Regexp:      regexp.MustCompile(`.*clustertasks.tekton.dev "foo-bar" not found`),
-		TargetEvent: triggertype.PullRequest.String(),
-		YAMLFiles: map[string]string{
-			".tekton/pr.yaml": "testdata/failures/not-existing-clustertask.yaml",
-		},
-		CheckForStatus: "failure",
-		ExpectEvents:   false,
-	}
-	_, f := tgitea.TestPR(t, topts)
-	defer f()
-}
-
 // TestGiteaBadLinkOfTask checks that we fail properly with the error from the
 // tekton pipelines controller. We check on the UI interface that we display
 // and inside the pac controller.
@@ -1128,65 +1109,6 @@ func TestGiteaPushToTagGreedy(t *testing.T) {
 	assert.NilError(t, err)
 }
 
-// TestGiteaClusterTasks is a test to verify that we can use cluster tasks with PaaC.
-func TestGiteaClusterTasks(t *testing.T) {
-	// we need to verify sure to create clustertask before pushing the files
-	// so we have to create a new client and do more manual things we get for free in TestPR
-	topts := &tgitea.TestOpts{
-		TargetEvent: "pull_request, push",
-		YAMLFiles: map[string]string{
-			".tekton/prcluster.yaml": "testdata/pipelinerunclustertasks.yaml",
-		},
-		ExpectEvents: false,
-	}
-	topts.TargetRefName = names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
-	topts.TargetNS = topts.TargetRefName
-
-	// create first the cluster tasks
-	ctname := fmt.Sprintf(".tekton/%s.yaml", topts.TargetNS)
-	newyamlFiles := map[string]string{ctname: "testdata/clustertask.yaml"}
-	entries, err := payload.GetEntries(newyamlFiles, topts.TargetNS, "main", "pull_request", map[string]string{})
-	assert.NilError(t, err)
-	//nolint: staticcheck
-	ct := v1beta1.ClusterTask{}
-	assert.NilError(t, yaml.Unmarshal([]byte(entries[ctname]), &ct))
-	ct.Name = "clustertask-" + topts.TargetNS
-
-	run := params.New()
-	ctx := context.Background()
-	assert.NilError(t, run.Clients.NewClients(ctx, &run.Info))
-	// TODO(chmou): this is for v1beta1, we need to figure out a way how to do that on v1
-	_, err = run.Clients.Tekton.TektonV1beta1().ClusterTasks().Create(context.TODO(), &ct, metav1.CreateOptions{})
-	assert.NilError(t, err)
-	assert.NilError(t, pacrepo.CreateNS(ctx, topts.TargetNS, run))
-	run.Clients.Log.Infof("%s has been created", ct.GetName())
-	defer (func() {
-		assert.NilError(t, topts.ParamsRun.Clients.Tekton.TektonV1beta1().ClusterTasks().Delete(context.TODO(), ct.Name, metav1.DeleteOptions{}))
-		run.Clients.Log.Infof("%s is deleted", ct.GetName())
-	})()
-
-	// start PR
-	_, f := tgitea.TestPR(t, topts)
-	defer f()
-
-	// wait for it
-	waitOpts := twait.Opts{
-		RepoName:  topts.TargetNS,
-		Namespace: topts.TargetNS,
-		// 0 means 1 🙃 (we test for >, while we actually should do >=, but i
-		// need to go all over the code to verify it's not going to break
-		// anything else)
-		MinNumberStatus: 0,
-		PollTimeout:     twait.DefaultTimeout,
-		TargetSHA:       topts.PullRequest.Head.Sha,
-	}
-	_, err = twait.UntilRepositoryUpdated(context.Background(), topts.ParamsRun.Clients, waitOpts)
-	assert.NilError(t, err)
-
-	topts.CheckForStatus = "success"
-	tgitea.WaitForStatus(t, topts, topts.TargetRefName, "", true)
-}
-
 // Local Variables:
 // compile-command: "go test -tags=e2e -v -run TestGiteaPush ."
 // End: