fix: prevent stale failure statuses on commits

.yamllint

@@ -3,6 +3,7 @@ ignore: |
   .git
   test/testdata/failures/pipeline_bad_format.yaml
   test/testdata/failures/bad-yaml.yaml
+  test/testdata/failures/bad-pipelinerun.yaml
   *badyaml.yaml
   .github/workflows/*
 

pkg/consoleui/openshift.go

@@ -34,18 +34,30 @@ func (o *OpenshiftConsole) GetName() string {
 }
 
 func (o *OpenshiftConsole) URL() string {
+	if o.host == "" {
+		return "https://openshift.url.is.not.configured"
+	}
 	return "https://" + o.host
 }
 
 func (o *OpenshiftConsole) DetailURL(pr *tektonv1.PipelineRun) string {
+	if o.host == "" {
+		return fmt.Sprintf(openShiftPipelineDetailViewURL, "openshift.url.is.not.configured", pr.GetNamespace(), pr.GetName())
+	}
 	return fmt.Sprintf(openShiftPipelineDetailViewURL, o.host, pr.GetNamespace(), pr.GetName())
 }
 
 func (o *OpenshiftConsole) TaskLogURL(pr *tektonv1.PipelineRun, taskRunStatus *tektonv1.PipelineRunTaskRunStatus) string {
+	if o.host == "" {
+		return fmt.Sprintf(openShiftPipelineTaskLogURL, o.DetailURL(pr), taskRunStatus.PipelineTaskName)
+	}
 	return fmt.Sprintf(openShiftPipelineTaskLogURL, o.DetailURL(pr), taskRunStatus.PipelineTaskName)
 }
 
 func (o *OpenshiftConsole) NamespaceURL(pr *tektonv1.PipelineRun) string {
+	if o.host == "" {
+		return fmt.Sprintf(openShiftPipelineNamespaceViewURL, "openshift.url.is.not.configured", pr.GetNamespace())
+	}
 	return fmt.Sprintf(openShiftPipelineNamespaceViewURL, o.host, pr.GetNamespace())
 }
 

pkg/consoleui/openshift_test.go

@@ -127,4 +127,10 @@ func TestOpenshiftConsoleURLs(t *testing.T) {
 	assert.Equal(t, o.DetailURL(pr), "https://fakeconsole/k8s/ns/theNS/tekton.dev~v1~PipelineRun/pr")
 	assert.Equal(t, o.TaskLogURL(pr, trStatus), "https://fakeconsole/k8s/ns/theNS/tekton.dev~v1~PipelineRun/pr/logs/task")
 	assert.Equal(t, o.NamespaceURL(pr), "https://fakeconsole/pipelines/ns/theNS/pipeline-runs")
+
+	emptyHost := OpenshiftConsole{host: ""}
+	assert.Equal(t, emptyHost.URL(), "https://openshift.url.is.not.configured")
+	assert.Equal(t, emptyHost.DetailURL(pr), "https://openshift.url.is.not.configured/k8s/ns/theNS/tekton.dev~v1~PipelineRun/pr")
+	assert.Equal(t, emptyHost.TaskLogURL(pr, trStatus), "https://openshift.url.is.not.configured/k8s/ns/theNS/tekton.dev~v1~PipelineRun/pr/logs/task")
+	assert.Equal(t, emptyHost.NamespaceURL(pr), "https://openshift.url.is.not.configured/pipelines/ns/theNS/pipeline-runs")
 }

pkg/consoleui/tektondashboard.go

@@ -19,7 +19,7 @@ func (t *TektonDashboard) GetName() string {
 }
 
 func (t *TektonDashboard) DetailURL(pr *tektonv1.PipelineRun) string {
-	return fmt.Sprintf("%s/#/namespaces/%s/pipelineruns/%s", t.BaseURL, pr.GetNamespace(), pr.GetName())
+	return fmt.Sprintf("%s/#/namespaces/%s/pipelineruns/%s", t.URL(), pr.GetNamespace(), pr.GetName())
 }
 
 func (t *TektonDashboard) NamespaceURL(pr *tektonv1.PipelineRun) string {
@@ -31,6 +31,10 @@ func (t *TektonDashboard) TaskLogURL(pr *tektonv1.PipelineRun, taskRunStatus *te
 }
 
 func (t *TektonDashboard) URL() string {
+	// if BaseURL is not provided, return fake URL
+	if t.BaseURL == "" || t.BaseURL == "http://" || t.BaseURL == "https://" {
+		return "https://dashboard.url.is.not.configured"
+	}
 	return t.BaseURL
 }
 

pkg/pipelineascode/pipelineascode.go

@@ -121,6 +121,9 @@ func (p *PacRun) Run(ctx context.Context) error {
 				errMsgM := fmt.Sprintf("There was an error creating the PipelineRun: <b>%s</b>\n\n%s", match.PipelineRun.GetGenerateName(), err.Error())
 				p.eventEmitter.EmitMessage(repo, zap.ErrorLevel, "RepositoryPipelineRun", errMsg)
 				createStatusErr := p.vcx.CreateStatus(ctx, p.event, provider.StatusOpts{
+					PipelineRunName:          match.PipelineRun.GetName(),
+					PipelineRun:              match.PipelineRun,
+					OriginalPipelineRunName:  match.PipelineRun.GetAnnotations()[keys.OriginalPRName],
 					Status:                   CompletedStatus,
 					Conclusion:               failureConclusion,
 					Text:                     errMsgM,

test/gitlab_merge_request_test.go

@@ -1,6 +1,3 @@
-//go:build e2e
-// +build e2e
-
 package test
 
 import (
@@ -15,6 +12,7 @@ import (
 
 	"github.com/google/go-github/v74/github"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/keys"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/formatting"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/opscomments"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 	"github.com/openshift-pipelines/pipelines-as-code/test/pkg/cctx"
@@ -812,6 +810,147 @@ func TestGitlabMergeRequestValidationErrorsFromFork(t *testing.T) {
 	assert.Assert(t, foundValidationComment, "Validation error comment should appear on original project's MR. ")
 }
 
+func TestGitlabConsistentCommitStatusOnMR(t *testing.T) {
+	targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
+	ctx := context.Background()
+	runcnx, opts, glprovider, err := tgitlab.Setup(ctx)
+	assert.NilError(t, err)
+	ctx, err = cctx.GetControllerCtxInfo(ctx, runcnx)
+	assert.NilError(t, err)
+	runcnx.Clients.Log.Info("Testing GitLab consistent commit status on Merge Request scenario")
+
+	projectinfo, resp, err := glprovider.Client().Projects.GetProject(opts.ProjectID, nil)
+	assert.NilError(t, err)
+	if resp != nil && resp.StatusCode == http.StatusNotFound {
+		t.Errorf("Repository %d not found", opts.ProjectID)
+	}
+
+	err = tgitlab.CreateCRD(ctx, projectinfo, runcnx, opts, targetNS, nil)
+	assert.NilError(t, err)
+
+	entries, err := payload.GetEntries(map[string]string{
+		".tekton/bad-pipelinerun.yaml":  "testdata/failures/bad-pipelinerun.yaml",
+		".tekton/good-pipelinerun.yaml": "testdata/pipelinerun.yaml",
+	}, targetNS, projectinfo.DefaultBranch,
+		triggertype.PullRequest.String(), map[string]string{"PipelineName": "good-pipelinerun"})
+	assert.NilError(t, err)
+
+	targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
+	cloneURL, err := scm.MakeGitCloneURL(projectinfo.WebURL, opts.UserName, opts.Password)
+	assert.NilError(t, err)
+
+	commitTitle := "Add invalid .tekton file - " + targetRefName
+	scmOpts := &scm.Opts{
+		GitURL:        cloneURL,
+		CommitTitle:   commitTitle,
+		Log:           runcnx.Clients.Log,
+		WebURL:        projectinfo.WebURL,
+		TargetRefName: targetRefName,
+		BaseRefName:   projectinfo.DefaultBranch,
+	}
+	_ = scm.PushFilesToRefGit(t, scmOpts, entries)
+	runcnx.Clients.Log.Infof("Pushed invalid .tekton files to branch: %s", targetRefName)
+
+	mrTitle := "TestConsistentCommitStatusOnMR - " + targetRefName
+	mrOptions := &clientGitlab.CreateMergeRequestOptions{
+		Title:        &mrTitle,
+		SourceBranch: &targetRefName,
+		TargetBranch: &projectinfo.DefaultBranch,
+	}
+
+	mr, _, err := glprovider.Client().MergeRequests.CreateMergeRequest(projectinfo.ID, mrOptions)
+	assert.NilError(t, err)
+	runcnx.Clients.Log.Infof("Created merge request: %d", mr.IID)
+
+	defer tgitlab.TearDown(ctx, t, runcnx, glprovider, mr.IID, targetRefName, targetNS, projectinfo.ID)
+
+	sopt := twait.SuccessOpt{
+		Title:           commitTitle,
+		OnEvent:         "Merge Request",
+		TargetNS:        targetNS,
+		NumberofPRMatch: 1, // there must be one PipelineRun because one is invalid
+		SHA:             mr.SHA,
+	}
+	twait.Succeeded(ctx, t, runcnx, opts, sopt)
+	labelSelector := fmt.Sprintf("%s=%s", keys.SHA, formatting.CleanValueKubernetes(mr.SHA))
+	prsNew, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{
+		LabelSelector: labelSelector,
+	})
+	assert.NilError(t, err)
+	assert.Assert(t, len(prsNew.Items) == 1)
+
+	commitStatuses, _, err := glprovider.Client().Commits.GetCommitStatuses(projectinfo.ID, mr.SHA, &clientGitlab.GetCommitStatusesOptions{})
+	assert.NilError(t, err)
+	assert.Assert(t, len(commitStatuses) == 2)
+
+	// here we don't know which status is returned first from  GitLab API
+	// so we need to check both by their names
+	for _, cs := range commitStatuses {
+		switch cs.Name {
+		case "Pipelines as Code CI / bad-pipelinerun":
+			assert.Assert(t, cs.Status == "failed")
+		case "Pipelines as Code CI / good-pipelinerun":
+			assert.Assert(t, cs.Status == "success")
+		default:
+			t.Fatalf("unexpected commit status name: %s", cs.Name)
+		}
+	}
+
+	entries, err = payload.GetEntries(map[string]string{
+		".tekton/bad-pipelinerun.yaml":  "testdata/good-pipelinerun.yaml",
+		".tekton/good-pipelinerun.yaml": "testdata/pipelinerun.yaml",
+	}, targetNS, projectinfo.DefaultBranch,
+		triggertype.PullRequest.String(), map[string]string{"PipelineName": "good-pipelinerun"})
+	assert.NilError(t, err)
+
+	commitTitle = "Add good .tekton file - " + targetRefName
+	scmOpts = &scm.Opts{
+		GitURL:        cloneURL,
+		CommitTitle:   commitTitle,
+		Log:           runcnx.Clients.Log,
+		WebURL:        projectinfo.WebURL,
+		TargetRefName: targetRefName,
+		BaseRefName:   projectinfo.DefaultBranch,
+		PushForce:     true,
+	}
+	_ = scm.PushFilesToRefGit(t, scmOpts, entries)
+	runcnx.Clients.Log.Infof("Pushed good .tekton files to branch: %s", targetRefName)
+
+	// get latest MR because of last commit it is update
+	mr, _, err = glprovider.Client().MergeRequests.GetMergeRequest(projectinfo.ID, mr.IID, nil)
+	assert.NilError(t, err)
+
+	sopt = twait.SuccessOpt{
+		Title:           commitTitle,
+		OnEvent:         "Merge Request",
+		TargetNS:        targetNS,
+		NumberofPRMatch: 2,
+		SHA:             mr.SHA,
+	}
+
+	twait.Succeeded(ctx, t, runcnx, opts, sopt)
+	labelSelector = fmt.Sprintf("%s=%s", keys.SHA, formatting.CleanValueKubernetes(mr.SHA))
+	prsNew, err = runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{
+		LabelSelector: labelSelector,
+	})
+	assert.NilError(t, err)
+	assert.Assert(t, len(prsNew.Items) == 2)
+
+	commitStatuses, _, err = glprovider.Client().Commits.GetCommitStatuses(projectinfo.ID, mr.SHA, &clientGitlab.GetCommitStatusesOptions{})
+	assert.NilError(t, err)
+	assert.Assert(t, len(commitStatuses) == 2)
+
+	// now both should be success
+	for _, cs := range commitStatuses {
+		switch cs.Name {
+		case "Pipelines as Code CI / bad-pipelinerun", "Pipelines as Code CI / good-pipelinerun":
+			assert.Assert(t, cs.Status == "success")
+		default:
+			t.Fatalf("unexpected commit status name: %s", cs.Name)
+		}
+	}
+}
+
 // Local Variables:
 // compile-command: "go test -tags=e2e -v -run ^TestGitlabMergeRequest$"
 // End:

test/pkg/payload/get_entries.go

@@ -26,10 +26,11 @@ func GetEntries(yamlfile map[string]string, targetNS, targetBranch, targetEvent
 	}
 	entries := map[string]string{}
 	for target, file := range yamlfile {
-		name := strings.TrimSuffix(filepath.Base(target), filepath.Ext(target))
-		// add some random character to name so that each PR has different name
-		extraParams["PipelineName"] = name + "-" + strings.ToLower(random.AlphaString(4))
-		// PipelineName can be overridden by extraParams
+		if extraParams["PipelineName"] == "" {
+			name := strings.TrimSuffix(filepath.Base(target), filepath.Ext(target))
+			// add some random character to name so that each PR has different name
+			extraParams["PipelineName"] = name + "-" + strings.ToLower(random.AlphaString(4))
+		}
 		newParams := vinceMap(params, extraParams)
 
 		output, err := ApplyTemplate(file, newParams)

test/testdata/failures/bad-pipelinerun.yaml

@@ -0,0 +1,11 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: bad-pipelinerun
+  annotations:
+    pipelinesascode.tekton.dev/target-namespace: "\\ .TargetNamespace //"
+    pipelinesascode.tekton.dev/on-target-branch: "[\\ .TargetBranch //]"
+    pipelinesascode.tekton.dev/on-event: "[\\ .TargetEvent //]"
+spec:
+  pipelineRef:

test/testdata/good-pipelinerun.yaml

@@ -0,0 +1,19 @@
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  name: bad-pipelinerun  # it is still named bad because it is considered fixed version of bad-pipelinerun.yaml
+  annotations:
+    pipelinesascode.tekton.dev/target-namespace: "\\ .TargetNamespace //"
+    pipelinesascode.tekton.dev/on-target-branch: "[\\ .TargetBranch //]"
+    pipelinesascode.tekton.dev/on-event: "[\\ .TargetEvent //]"
+spec:
+  pipelineSpec:
+    tasks:
+      - name: task
+        displayName: "The Task name is Task"
+        taskSpec:
+          steps:
+            - name: task
+              image: registry.access.redhat.com/ubi9/ubi-micro
+              command: ["/bin/echo", "HELLOMOTO"]