refactor(providers): constantize provider StatusOpt conclusion options

The Conclusion field of StatusOpts has a discrete number of supported
values. These being typed as strings has introduced bugs in the past
and makes adding new values difficult. Defining the supported values
as constant enums improves both the safety of the values and the ease
of working with them.

Author: aThorp96

pkg/adapter/sinker.go

@@ -13,6 +13,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/pipelineascode"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	"go.uber.org/zap"
 )
 
@@ -156,9 +157,9 @@ func (s *sinker) setupClient(ctx context.Context, repo *v1alpha1.Repository) err
 
 // createSkipCIStatus creates a neutral status check on the git provider when CI is skipped.
 func (s *sinker) createSkipCIStatus(ctx context.Context) error {
-	statusOpts := provider.StatusOpts{
+	statusOpts := status.StatusOpts{
 		Status:     "completed",
-		Conclusion: "neutral",
+		Conclusion: status.ConclusionNeutral,
 		Title:      "CI Skipped",
 		Summary:    fmt.Sprintf("%s - CI has been skipped", s.pacInfo.ApplicationName),
 		Text:       "Commit contains a skip CI command. Use /test or /retest to manually trigger CI if needed.",

pkg/formatting/pipelinerun.go

@@ -1,21 +1,22 @@
 package formatting
 
 import (
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	corev1 "k8s.io/api/core/v1"
 	"knative.dev/pkg/apis"
 )
 
 // PipelineRunStatus return status of PR  success failed or skipped.
-func PipelineRunStatus(pr *tektonv1.PipelineRun) string {
+func PipelineRunStatus(pr *tektonv1.PipelineRun) status.Conclusion {
 	if len(pr.Status.Conditions) == 0 {
-		return "neutral"
+		return status.ConclusionNeutral
 	}
 	if pr.Status.GetCondition(apis.ConditionSucceeded).GetReason() == tektonv1.PipelineRunSpecStatusCancelled {
-		return "cancelled"
+		return status.ConclusionCancelled
 	}
 	if pr.Status.Conditions[0].Status == corev1.ConditionFalse {
-		return "failure"
+		return status.ConclusionFailure
 	}
-	return "success"
+	return status.ConclusionSuccess
 }

pkg/formatting/pipelinerun_test.go

@@ -77,7 +77,7 @@ func TestPipelineRunStatus(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			output := PipelineRunStatus(tt.pr)
+			output := string(PipelineRunStatus(tt.pr))
 			assert.Equal(t, output, tt.name, "PipelineRunStatus() = %v, want %v", output, tt.name)
 		})
 	}

pkg/pipelineascode/errors.go

@@ -9,6 +9,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
 	pacerrors "github.com/openshift-pipelines/pipelines-as-code/pkg/errors"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
+	providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"go.uber.org/zap"
 )
@@ -19,7 +20,7 @@ const (
 
 var regexpIgnoreErrors = regexp.MustCompile(`.*no kind.*is registered for version.*in scheme.*`)
 
-func (p *PacRun) checkAccessOrError(ctx context.Context, repo *v1alpha1.Repository, status provider.StatusOpts, viamsg string) (bool, error) {
+func (p *PacRun) checkAccessOrError(ctx context.Context, repo *v1alpha1.Repository, status providerstatus.StatusOpts, viamsg string) (bool, error) {
 	p.debugf("checkAccessOrError: checking access for sender=%s via=%s", p.event.Sender, viamsg)
 	allowed, err := p.vcx.IsAllowed(ctx, p.event)
 	if err != nil {

pkg/pipelineascode/errors_test.go

@@ -9,7 +9,7 @@ import (
 	pacerrors "github.com/openshift-pipelines/pipelines-as-code/pkg/errors"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/events"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
-	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
+	providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	testclient "github.com/openshift-pipelines/pipelines-as-code/pkg/test/clients"
 	testprovider "github.com/openshift-pipelines/pipelines-as-code/pkg/test/provider"
 	"go.uber.org/zap"
@@ -94,7 +94,7 @@ func TestCheckAccessOrErrror(t *testing.T) {
 
 			// Call the function
 			repo := &v1alpha1.Repository{}
-			status := provider.StatusOpts{}
+			status := providerstatus.StatusOpts{}
 			allowed, err := p.checkAccessOrError(context.Background(), repo, status, "via test")
 
 			// Verify results

pkg/pipelineascode/match.go

@@ -13,7 +13,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/matcher"
 	"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/pkg/provider"
+	providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/resolve"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/secrets"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/templates"
@@ -92,10 +92,10 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
 	// trigger CI on the repository, as any user is able to comment on a pushed commit in open-source repositories.
 	if p.event.TriggerTarget == triggertype.Push && opscomments.IsAnyOpsEventType(p.event.EventType) {
 		p.debugf("verifyRepoAndUser: checking access for gitops comment on push")
-		status := provider.StatusOpts{
+		status := providerstatus.StatusOpts{
 			Status:       CompletedStatus,
 			Title:        "Permission denied",
-			Conclusion:   failureConclusion,
+			Conclusion:   providerstatus.ConclusionFailure,
 			DetailsURL:   p.event.URL,
 			AccessDenied: true,
 		}
@@ -109,10 +109,10 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
 	// on comment we skip it for now, we are going to check later on
 	if p.event.TriggerTarget != triggertype.Push && p.event.EventType != opscomments.NoOpsCommentEventType.String() {
 		p.debugf("verifyRepoAndUser: checking access for trigger target=%s event_type=%s", p.event.TriggerTarget, p.event.EventType)
-		status := provider.StatusOpts{
+		status := providerstatus.StatusOpts{
 			Status:       queuedStatus,
 			Title:        "Pending approval, waiting for an /ok-to-test",
-			Conclusion:   pendingConclusion,
+			Conclusion:   providerstatus.ConclusionPending,
 			DetailsURL:   p.event.URL,
 			AccessDenied: true,
 		}
@@ -122,10 +122,10 @@ func (p *PacRun) verifyRepoAndUser(ctx context.Context) (*v1alpha1.Repository, e
 		// When /ok-to-test is approved, update the parent "Pipelines as Code CI" status to success
 		// to indicate the approval was successful before pipelines start running.
 		if p.event.EventType == opscomments.OkToTestCommentEventType.String() {
-			approvalStatus := provider.StatusOpts{
+			approvalStatus := providerstatus.StatusOpts{
 				Status:     CompletedStatus,
 				Title:      "Approved",
-				Conclusion: successConclusion,
+				Conclusion: providerstatus.ConclusionSuccess,
 				DetailsURL: p.event.URL,
 			}
 			if err := p.vcx.CreateStatus(ctx, p.event, approvalStatus); err != nil {
@@ -282,10 +282,10 @@ func (p *PacRun) getPipelineRunsFromRepo(ctx context.Context, repo *v1alpha1.Rep
 	// if the event is a comment event, but we don't have any match from the keys.OnComment then do the ACL checks again
 	// we skipped previously so we can get the match from the event to the pipelineruns
 	if p.event.EventType == opscomments.NoOpsCommentEventType.String() || p.event.EventType == opscomments.OnCommentEventType.String() {
-		status := provider.StatusOpts{
+		status := providerstatus.StatusOpts{
 			Status:       queuedStatus,
 			Title:        "Pending approval, waiting for an /ok-to-test",
-			Conclusion:   pendingConclusion,
+			Conclusion:   providerstatus.ConclusionPending,
 			DetailsURL:   p.event.URL,
 			AccessDenied: true,
 		}
@@ -484,11 +484,11 @@ func (p *PacRun) checkNeedUpdate(_ string) (string, bool) {
 
 func (p *PacRun) createNeutralStatus(ctx context.Context, title, text string) error {
 	p.debugf("createNeutralStatus: title=%s", title)
-	status := provider.StatusOpts{
+	status := providerstatus.StatusOpts{
 		Status:     CompletedStatus,
 		Title:      title,
 		Text:       text,
-		Conclusion: neutralConclusion,
+		Conclusion: providerstatus.ConclusionNeutral,
 		DetailsURL: p.event.URL,
 	}
 	if err := p.vcx.CreateStatus(ctx, p.event, status); err != nil {

pkg/pipelineascode/pipelineascode.go

@@ -19,6 +19,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
+	providerstatus "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/secrets"
 	tektonv1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
 	"go.uber.org/zap"
@@ -27,14 +28,10 @@ import (
 )
 
 const (
-	tektonDir         = ".tekton"
-	CompletedStatus   = "completed"
-	inProgressStatus  = "in_progress"
-	queuedStatus      = "queued"
-	successConclusion = "success"
-	failureConclusion = "failure"
-	pendingConclusion = "pending"
-	neutralConclusion = "neutral"
+	tektonDir        = ".tekton"
+	CompletedStatus  = "completed"
+	inProgressStatus = "in_progress"
+	queuedStatus     = "queued"
 )
 
 type PacRun struct {
@@ -87,9 +84,9 @@ func (p *PacRun) Run(ctx context.Context) error {
 
 	matchedPRs, repo, err := p.matchRepoPR(ctx)
 	if err != nil {
-		createStatusErr := p.vcx.CreateStatus(ctx, p.event, provider.StatusOpts{
+		createStatusErr := p.vcx.CreateStatus(ctx, p.event, providerstatus.StatusOpts{
 			Status:     CompletedStatus,
-			Conclusion: failureConclusion,
+			Conclusion: providerstatus.ConclusionFailure,
 			Text:       fmt.Sprintf("There was an issue validating the commit: %q", err),
 			DetailsURL: p.run.Clients.ConsoleUI().URL(),
 		})
@@ -161,13 +158,13 @@ func (p *PacRun) Run(ctx context.Context) error {
 				if prName == "" {
 					prName = match.PipelineRun.GetGenerateName()
 				}
-				createStatusErr := p.vcx.CreateStatus(ctx, p.event, provider.StatusOpts{
+				createStatusErr := p.vcx.CreateStatus(ctx, p.event, providerstatus.StatusOpts{
 					PipelineRunName:          prName,
 					PipelineRun:              match.PipelineRun,
 					OriginalPipelineRunName:  match.PipelineRun.GetAnnotations()[keys.OriginalPRName],
 					Status:                   CompletedStatus,
-					Conclusion:               failureConclusion,
 					Title:                    "pipelinerun start failure",
+					Conclusion:               providerstatus.ConclusionFailure,
 					Text:                     errMsgM,
 					DetailsURL:               p.run.Clients.ConsoleUI().URL(),
 					InstanceCountForCheckRun: i,
@@ -313,9 +310,9 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
 	if err != nil {
 		return nil, fmt.Errorf("cannot create message template: %w", err)
 	}
-	status := provider.StatusOpts{
+	status := providerstatus.StatusOpts{
 		Status:                  inProgressStatus,
-		Conclusion:              pendingConclusion,
+		Conclusion:              providerstatus.ConclusionPending,
 		Text:                    msg,
 		DetailsURL:              consoleURL,
 		PipelineRunName:         pr.GetName(),

pkg/provider/bitbucketcloud/bitbucket.go

@@ -17,6 +17,7 @@ import (
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud/types"
 	providerMetrics "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/providermetrics"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/status"
 	"go.uber.org/zap"
 )
 
@@ -82,26 +83,30 @@ func (v *Provider) GetConfig() *info.ProviderConfig {
 	}
 }
 
-func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts provider.StatusOpts) error {
+func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts status.StatusOpts) error {
+	var state types.CommitStatusState
+
 	switch statusopts.Conclusion {
-	case "skipped":
-		statusopts.Conclusion = "STOPPED"
+	case status.ConclusionSkipped:
+		state = types.StateStopped
 		statusopts.Title = "➖ Skipping this commit"
-	case "neutral":
-		statusopts.Conclusion = "STOPPED"
+	case status.ConclusionNeutral:
+		state = types.StateStopped
 		statusopts.Title = "➖ CI has stopped"
-	case "failure":
-		statusopts.Conclusion = "FAILED"
+	case status.ConclusionFailure:
+		state = types.StateFailed
 		statusopts.Title = "❌ Failed"
-	case "pending":
-		statusopts.Conclusion = "INPROGRESS"
+	case status.ConclusionPending:
+		state = types.StateInProgress
 		statusopts.Title = "⚡ CI has started"
-	case "success":
-		statusopts.Conclusion = "SUCCESSFUL"
+	case status.ConclusionSuccess:
+		state = types.StateSuccessful
 		statusopts.Title = "✅ Commit has been validated"
-	case "completed":
-		statusopts.Conclusion = "SUCCESSFUL"
+	case status.ConclusionCompleted:
+		state = types.StateSuccessful
 		statusopts.Title = "✅ Completed"
+	case status.ConclusionCancelled:
+		// TODO
 	}
 	detailsURL := event.Provider.URL
 	if statusopts.DetailsURL != "" {
@@ -111,7 +116,7 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
 	cso := &bitbucket.CommitStatusOptions{
 		Key:         provider.GetCheckName(statusopts, v.pacInfo),
 		Url:         detailsURL,
-		State:       statusopts.Conclusion,
+		State:       string(state),
 		Description: statusopts.Title,
 	}
 	cmo := &bitbucket.CommitsOptions{
@@ -133,7 +138,7 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
 	}
 
 	eventType := triggertype.IsPullRequestType(event.EventType)
-	if statusopts.Conclusion != "STOPPED" && statusopts.Status == "completed" &&
+	if state != types.StateStopped && statusopts.Status == "completed" &&
 		statusopts.Text != "" &&
 		(eventType == triggertype.PullRequest || event.TriggerTarget == triggertype.PullRequest) {
 		onPr := ""