From d4ad47ae4c9bb5d5de005506acf7ce4d1865eb85 Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Thu, 5 Dec 2024 08:43:04 +0100 Subject: [PATCH] gitlab: Create status with pr name In gitlab the check names where showing up as a single one for every pipelines. This would cause issue when there is multiple pipelinerun running and one of them fails. If other success it would override it and would show as success in the Pipeline. We now create each status like in github with the applicationName / OriginalPipelineName Signed-off-by: Chmouel Boudjnah --- pkg/provider/gitea/gitea.go | 13 +------- pkg/provider/github/status.go | 16 ++------- pkg/provider/github/status_test.go | 50 ---------------------------- pkg/provider/gitlab/gitlab.go | 4 ++- pkg/provider/provider.go | 17 ++++++++++ pkg/provider/provider_test.go | 52 ++++++++++++++++++++++++++++++ 6 files changed, 76 insertions(+), 76 deletions(-) diff --git a/pkg/provider/gitea/gitea.go b/pkg/provider/gitea/gitea.go index 84db571ca..f46f525f9 100644 --- a/pkg/provider/gitea/gitea.go +++ b/pkg/provider/gitea/gitea.go @@ -169,7 +169,7 @@ func (v *Provider) createStatusCommit(event *info.Event, pacopts *info.PacOpts, State: state, TargetURL: status.DetailsURL, Description: status.Title, - Context: getCheckName(status, pacopts), + Context: provider.GetCheckName(status, pacopts), } if _, _, err := v.Client.CreateStatus(event.Organization, event.Repository, event.SHA, gStatus); err != nil { return err @@ -197,17 +197,6 @@ func (v *Provider) createStatusCommit(event *info.Event, pacopts *info.PacOpts, return nil } -// TODO: move to common since used in github and here. -func getCheckName(status provider.StatusOpts, pacopts *info.PacOpts) string { - if pacopts.ApplicationName != "" { - if status.OriginalPipelineRunName == "" { - return pacopts.ApplicationName - } - return fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName) - } - return status.OriginalPipelineRunName -} - func (v *Provider) GetTektonDir(_ context.Context, event *info.Event, path, provenance string) (string, error) { // default set provenance from the SHA revision := event.SHA diff --git a/pkg/provider/github/status.go b/pkg/provider/github/status.go index 8306f3129..53397dac8 100644 --- a/pkg/provider/github/status.go +++ b/pkg/provider/github/status.go @@ -34,16 +34,6 @@ const taskStatusTemplate = ` {{- end }} ` -func getCheckName(status provider.StatusOpts, pacopts *info.PacOpts) string { - if pacopts.ApplicationName != "" { - if status.OriginalPipelineRunName == "" { - return pacopts.ApplicationName - } - return fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName) - } - return status.OriginalPipelineRunName -} - func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status provider.StatusOpts) (*int64, error) { opt := github.ListOptions{PerPage: v.PaginedNumber} for { @@ -114,7 +104,7 @@ func (v *Provider) canIUseCheckrunID(checkrunid *int64) bool { func (v *Provider) createCheckRunStatus(ctx context.Context, runevent *info.Event, status provider.StatusOpts) (*int64, error) { now := github.Timestamp{Time: time.Now()} checkrunoption := github.CreateCheckRunOptions{ - Name: getCheckName(status, v.pacInfo), + Name: provider.GetCheckName(status, v.pacInfo), HeadSHA: runevent.SHA, Status: github.String("in_progress"), DetailsURL: github.String(status.DetailsURL), @@ -254,7 +244,7 @@ func (v *Provider) getOrUpdateCheckRunStatus(ctx context.Context, runevent *info checkRunOutput.Text = github.String(text) opts := github.UpdateCheckRunOptions{ - Name: getCheckName(statusOpts, pacopts), + Name: provider.GetCheckName(statusOpts, pacopts), Status: github.String(statusOpts.Status), Output: checkRunOutput, } @@ -323,7 +313,7 @@ func (v *Provider) createStatusCommit(ctx context.Context, runevent *info.Event, State: github.String(status.Conclusion), TargetURL: github.String(status.DetailsURL), Description: github.String(status.Title), - Context: github.String(getCheckName(status, v.pacInfo)), + Context: github.String(provider.GetCheckName(status, v.pacInfo)), CreatedAt: &github.Timestamp{Time: now}, } diff --git a/pkg/provider/github/status_test.go b/pkg/provider/github/status_test.go index 77e6af6b7..2157dae92 100644 --- a/pkg/provider/github/status_test.go +++ b/pkg/provider/github/status_test.go @@ -561,56 +561,6 @@ func TestGithubProvidercreateStatusCommit(t *testing.T) { } } -func TestGetCheckName(t *testing.T) { - type args struct { - status provider.StatusOpts - pacopts *info.PacOpts - } - tests := []struct { - name string - args args - want string - }{ - { - name: "no application name", - args: args{ - status: provider.StatusOpts{ - OriginalPipelineRunName: "HELLO", - }, - pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: ""}}, - }, - want: "HELLO", - }, - { - name: "application and pipelinerun name", - args: args{ - status: provider.StatusOpts{ - OriginalPipelineRunName: "MOTO", - }, - pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "HELLO"}}, - }, - want: "HELLO / MOTO", - }, - { - name: "application no pipelinerun name", - args: args{ - status: provider.StatusOpts{ - OriginalPipelineRunName: "", - }, - pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "PAC"}}, - }, - want: "PAC", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := getCheckName(tt.args.status, tt.args.pacopts); got != tt.want { - t.Errorf("getCheckName() = %v, want %v", got, tt.want) - } - }) - } -} - func TestProviderGetExistingCheckRunID(t *testing.T) { tests := []struct { name string diff --git a/pkg/provider/gitlab/gitlab.go b/pkg/provider/gitlab/gitlab.go index b02264b5b..4ca9fdf8a 100644 --- a/pkg/provider/gitlab/gitlab.go +++ b/pkg/provider/gitlab/gitlab.go @@ -196,11 +196,13 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusOpts body := fmt.Sprintf("**%s%s** has %s\n\n%s\n\nFull log available [here](%s)", v.pacInfo.ApplicationName, onPr, statusOpts.Title, statusOpts.Text, detailsURL) + contextName := provider.GetCheckName(statusOpts, v.pacInfo) opt := &gitlab.SetCommitStatusOptions{ State: gitlab.BuildStateValue(statusOpts.Conclusion), - Name: gitlab.Ptr(v.pacInfo.ApplicationName), + Name: gitlab.Ptr(contextName), TargetURL: gitlab.Ptr(detailsURL), Description: gitlab.Ptr(statusOpts.Title), + Context: gitlab.Ptr(contextName), } // In case we have access, set the status. Typically, on a Merge Request (MR) diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index ef822595b..aa442f44a 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -6,6 +6,7 @@ import ( "regexp" "strings" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" "gopkg.in/yaml.v2" ) @@ -133,3 +134,19 @@ func ValidateYaml(content []byte, filename string) error { } return nil } + +// GetCheckName returns the name of the check to be created based on the status +// and the pacopts. +// If the pacopts.ApplicationName is set, it will be used as the check name. +// Otherwise, the OriginalPipelineRunName will be used. +// If the OriginalPipelineRunName is not set, an empty string will be returned. +// The check name will be in the format "ApplicationName / OriginalPipelineRunName". +func GetCheckName(status StatusOpts, pacopts *info.PacOpts) string { + if pacopts.ApplicationName != "" { + if status.OriginalPipelineRunName == "" { + return pacopts.ApplicationName + } + return fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName) + } + return status.OriginalPipelineRunName +} diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go index 73ec02039..447439c3e 100644 --- a/pkg/provider/provider_test.go +++ b/pkg/provider/provider_test.go @@ -3,6 +3,8 @@ package provider import ( "testing" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/info" + "github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings" "gotest.tools/v3/assert" ) @@ -494,3 +496,53 @@ func TestCompareHostOfURLS(t *testing.T) { }) } } + +func TestGetCheckName(t *testing.T) { + type args struct { + status StatusOpts + pacopts *info.PacOpts + } + tests := []struct { + name string + args args + want string + }{ + { + name: "no application name", + args: args{ + status: StatusOpts{ + OriginalPipelineRunName: "HELLO", + }, + pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: ""}}, + }, + want: "HELLO", + }, + { + name: "application and pipelinerun name", + args: args{ + status: StatusOpts{ + OriginalPipelineRunName: "MOTO", + }, + pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "HELLO"}}, + }, + want: "HELLO / MOTO", + }, + { + name: "application no pipelinerun name", + args: args{ + status: StatusOpts{ + OriginalPipelineRunName: "", + }, + pacopts: &info.PacOpts{Settings: settings.Settings{ApplicationName: "PAC"}}, + }, + want: "PAC", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := GetCheckName(tt.args.status, tt.args.pacopts); got != tt.want { + t.Errorf("GetCheckName() = %v, want %v", got, tt.want) + } + }) + } +}