diff --git a/.yamllint b/.yamllint index 68e2508c39..785cbc3440 100644 --- a/.yamllint +++ b/.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/* diff --git a/pkg/consoleui/openshift.go b/pkg/consoleui/openshift.go index cf54c8eedc..9e9f5039a9 100644 --- a/pkg/consoleui/openshift.go +++ b/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()) } diff --git a/pkg/consoleui/openshift_test.go b/pkg/consoleui/openshift_test.go index 578ea6957a..6307c59856 100644 --- a/pkg/consoleui/openshift_test.go +++ b/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") } diff --git a/pkg/consoleui/tektondashboard.go b/pkg/consoleui/tektondashboard.go index 882cd42bff..cc53a5c592 100644 --- a/pkg/consoleui/tektondashboard.go +++ b/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 } diff --git a/pkg/pipelineascode/pipelineascode.go b/pkg/pipelineascode/pipelineascode.go index e9e9af1f90..aa56e90220 100644 --- a/pkg/pipelineascode/pipelineascode.go +++ b/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: %s\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, diff --git a/test/gitlab_merge_request_test.go b/test/gitlab_merge_request_test.go index ee253fb603..008f4f459c 100644 --- a/test/gitlab_merge_request_test.go +++ b/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: diff --git a/test/pkg/payload/get_entries.go b/test/pkg/payload/get_entries.go index 761ccdec5d..bffbcc2938 100644 --- a/test/pkg/payload/get_entries.go +++ b/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) diff --git a/test/testdata/failures/bad-pipelinerun.yaml b/test/testdata/failures/bad-pipelinerun.yaml new file mode 100644 index 0000000000..80491be71a --- /dev/null +++ b/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: diff --git a/test/testdata/good-pipelinerun.yaml b/test/testdata/good-pipelinerun.yaml new file mode 100644 index 0000000000..bb660664f2 --- /dev/null +++ b/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"]