From a715ee54e7c33b41ae19581408aa56f253985b85 Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Tue, 28 Apr 2026 12:20:28 +0530 Subject: [PATCH] fix(bitbucket-cloud): fix status key handling for build statuses Bitbucket Cloud limits commit status keys to 40 characters and uses them to identify build statuses. Generated a new key on each CreateStatus call causing duplicate statuses instead of updating existing ones. Fix the 40-char truncation to safely handle short names, use 1-based indexing for status keys, and persist the generated key as a PipelineRun annotation (pipeline-run-status-key) so subsequent calls reuse the same key. Move GetCheckName to provider.GetBBCloudStatusKey, add Index field to matcher.Match for stable instance counts, and add e2e test to verify build status reporting. https://redhat.atlassian.net/browse/SRVKP-11710 Co-Authored-By: Claude Opus 4.6 (1M context) Signed-off-by: Zaki Shaikh --- pkg/provider/bitbucketcloud/bitbucket.go | 4 +- .../bitbucketcloud/test/bbcloudtest.go | 2 +- pkg/provider/bitbucketcloud/types/types.go | 13 +++ pkg/provider/provider.go | 38 ++++++++ pkg/provider/provider_test.go | 90 +++++++++++++++++++ test/bitbucket_cloud_pullrequest_test.go | 62 +++++++++++++ 6 files changed, 207 insertions(+), 2 deletions(-) diff --git a/pkg/provider/bitbucketcloud/bitbucket.go b/pkg/provider/bitbucketcloud/bitbucket.go index 151904f768..6a47921326 100644 --- a/pkg/provider/bitbucketcloud/bitbucket.go +++ b/pkg/provider/bitbucketcloud/bitbucket.go @@ -113,8 +113,10 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts detailsURL = statusopts.DetailsURL } + key := provider.GetBBCloudStatusKey(statusopts, v.pacInfo) + cso := &bitbucket.CommitStatusOptions{ - Key: provider.GetCheckName(statusopts, v.pacInfo), + Key: key, Url: detailsURL, State: string(state), Description: statusopts.Title, diff --git a/pkg/provider/bitbucketcloud/test/bbcloudtest.go b/pkg/provider/bitbucketcloud/test/bbcloudtest.go index 522c0a085b..dcb1b25d3c 100644 --- a/pkg/provider/bitbucketcloud/test/bbcloudtest.go +++ b/pkg/provider/bitbucketcloud/test/bbcloudtest.go @@ -189,7 +189,7 @@ func MuxCreateCommitstatus(t *testing.T, mux *http.ServeMux, event *info.Event, err := json.Unmarshal(bit, cso) assert.NilError(t, err) pacOpts := &info.PacOpts{Settings: settings.Settings{ApplicationName: applicationName}} - assert.Equal(t, provider.GetCheckName(expStatus, pacOpts), cso.Key) + assert.Equal(t, provider.GetBBCloudStatusKey(expStatus, pacOpts), cso.Key) if expStatus.DetailsURL != "" { assert.Equal(t, expStatus.DetailsURL, cso.Url) diff --git a/pkg/provider/bitbucketcloud/types/types.go b/pkg/provider/bitbucketcloud/types/types.go index fe71f4cf8a..e535c2e5f8 100644 --- a/pkg/provider/bitbucketcloud/types/types.go +++ b/pkg/provider/bitbucketcloud/types/types.go @@ -125,6 +125,19 @@ type Comments struct { Values []Comment } +type Status struct { + Key string `json:"key"` + Type string `json:"type"` + State string `json:"state"` + Name string `json:"name"` + RefName string `json:"refname"` + Commit Commit `json:"commit"` + URL string `json:"url"` + Repository Repository `json:"repository"` + Description string `json:"description"` + Links Links `json:"links"` +} + type CommitStatusState string // See: https://api.bitbucket.org/swagger.json attribute: diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index f58f27d7d8..af47891a62 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -1,6 +1,8 @@ package provider import ( + "crypto/sha256" + "encoding/hex" "fmt" "net/url" "regexp" @@ -221,6 +223,42 @@ func GetCheckName(status providerstatus.StatusOpts, pacopts *info.PacOpts) strin return status.OriginalPipelineRunName } +// GetBBCloudStatusKey returns a unique key for Bitbucket Cloud build commit status. +// Bitbucket Cloud limits the key to 40 characters. When both ApplicationName and +// OriginalPipelineRunName are set and their combined form ("{appName} / {prName}") +// fits within 40 chars, that combined form is used. Otherwise, the +// OriginalPipelineRunName alone is used. If the name exceeds 40 chars, it is +// truncated to 33 chars with a 6-char hash suffix for uniqueness. If only +// ApplicationName is set, it is returned truncated to 40 chars. +func GetBBCloudStatusKey(status providerstatus.StatusOpts, pacopts *info.PacOpts) string { + if pacopts.ApplicationName != "" { + if status.OriginalPipelineRunName == "" { + if len(pacopts.ApplicationName) > 40 { + return pacopts.ApplicationName[:40] + } + return pacopts.ApplicationName + } + key := fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName) + // if application name and pipeline run name combined are less than 40 characters, return the key + // otherwise, skip it here and let the only pipeline run being returned. + if len(key) <= 40 { + return key + } + } + + if len(status.OriginalPipelineRunName) > 40 { + hash := getNameHash(status.OriginalPipelineRunName) + return fmt.Sprintf("%s-%s", status.OriginalPipelineRunName[:33], hash) + } + + return status.OriginalPipelineRunName +} + +func getNameHash(input string) string { + hash := sha256.Sum256([]byte(input)) + return hex.EncodeToString(hash[:])[:6] +} + func IsZeroSHA(sha string) bool { return sha == "0000000000000000000000000000000000000000" } diff --git a/pkg/provider/provider_test.go b/pkg/provider/provider_test.go index 0b79c36f03..1ee0e7c47e 100644 --- a/pkg/provider/provider_test.go +++ b/pkg/provider/provider_test.go @@ -1,6 +1,7 @@ package provider import ( + "strings" "testing" "github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1" @@ -867,3 +868,92 @@ func TestGetGitOpsCommentPrefix(t *testing.T) { }) } } + +func TestGetBBCloudStatusKey(t *testing.T) { + tests := []struct { + name string + status status.StatusOpts + pacopts *info.PacOpts + expected string + }{ + { + name: "application name only no pipeline run name", + status: status.StatusOpts{OriginalPipelineRunName: ""}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: "MyApp"}, + }, + expected: "MyApp", + }, + { + name: "application name longer than 40 truncated", + status: status.StatusOpts{OriginalPipelineRunName: ""}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: strings.Repeat("x", 50)}, + }, + expected: strings.Repeat("x", 40), + }, + { + name: "app and pr name combined fit in 40 chars", + status: status.StatusOpts{OriginalPipelineRunName: "my-pr"}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: "MyApp"}, + }, + expected: "MyApp / my-pr", + }, + { + name: "app and pr name combined exceed 40 chars falls back to pr name only", + status: status.StatusOpts{OriginalPipelineRunName: "this-is-a-very-long-pipeline-run-name"}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: "MyApplication"}, + }, + expected: "this-is-a-very-long-pipeline-run-name", + }, + { + name: "no application name short pipeline run name", + status: status.StatusOpts{OriginalPipelineRunName: "my-pr"}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: ""}, + }, + expected: "my-pr", + }, + { + name: "no application name pipeline run name exactly 40", + status: status.StatusOpts{OriginalPipelineRunName: strings.Repeat("a", 40)}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: ""}, + }, + expected: strings.Repeat("a", 40), + }, + { + name: "no application name pipeline run name longer than 40 truncated with hash", + status: status.StatusOpts{OriginalPipelineRunName: strings.Repeat("c", 50)}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: ""}, + }, + expected: strings.Repeat("c", 33) + "-5de6bf", + }, + { + name: "no application name no pipeline run name", + status: status.StatusOpts{OriginalPipelineRunName: ""}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: ""}, + }, + expected: "", + }, + { + name: "long pr name with app produces stable hash", + status: status.StatusOpts{OriginalPipelineRunName: strings.Repeat("b", 50)}, + pacopts: &info.PacOpts{ + Settings: settings.Settings{ApplicationName: "App"}, + }, + expected: strings.Repeat("b", 33) + "-c01a9b", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := GetBBCloudStatusKey(tt.status, tt.pacopts) + assert.Equal(t, tt.expected, got) + assert.Assert(t, len(got) <= 40, "key length %d exceeds 40 char limit: %s", len(got), got) + }) + } +} diff --git a/test/bitbucket_cloud_pullrequest_test.go b/test/bitbucket_cloud_pullrequest_test.go index d536e80e1f..98bbc2a7bf 100644 --- a/test/bitbucket_cloud_pullrequest_test.go +++ b/test/bitbucket_cloud_pullrequest_test.go @@ -5,11 +5,14 @@ package test import ( "context" "fmt" + "strings" "testing" "time" "github.com/ktrysmt/go-bitbucket" + "github.com/mitchellh/mapstructure" "github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype" + "github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud/types" tbb "github.com/openshift-pipelines/pipelines-as-code/test/pkg/bitbucketcloud" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/options" "github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload" @@ -143,6 +146,65 @@ func TestBitbucketCloudCELExpressionOnPush(t *testing.T) { twait.Succeeded(ctx, t, runcnx, opts, sopt) } +func TestBitbucketCloudPRBuildStatusReported(t *testing.T) { + targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns") + ctx := context.Background() + + runcnx, opts, bprovider, err := tbb.Setup(ctx) + if err != nil { + t.Skip(err.Error()) + return + } + bcrepo := tbb.CreateCRD(ctx, t, bprovider, runcnx, opts, targetNS) + targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test") + title := "TestPullRequest - " + targetRefName + + entries, err := payload.GetEntries( + map[string]string{".tekton/my-long-pipelinerun-name-exceeding-forty-chars.yaml": "testdata/pipelinerun.yaml"}, + targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{}) + assert.NilError(t, err) + + pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetRefName, entries) + defer tbb.TearDown(ctx, t, runcnx, bprovider, opts, pr.ID, targetRefName, targetNS, false) + + hash, ok := repobranch.Target["hash"].(string) + assert.Assert(t, ok) + + sopt := twait.SuccessOpt{ + TargetNS: targetNS, + OnEvent: triggertype.PullRequest.String(), + NumberofPRMatch: 1, + SHA: hash, + Title: title, + MinNumberStatus: 1, + } + twait.Succeeded(ctx, t, runcnx, opts, sopt) + + resp, err := bprovider.Client().Repositories.Commits.GetCommitStatuses(&bitbucket.CommitsOptions{ + Owner: opts.Organization, + RepoSlug: opts.Repo, + Revision: hash, + }) + assert.NilError(t, err) + + statusesMap, ok := resp.(map[string]any) + assert.Equal(t, ok, true, "cannot convert Bitbucket commit statuses response to map[string]any") + + statuses := []*types.Status{} + + err = mapstructure.Decode(statusesMap["values"], &statuses) + assert.NilError(t, err, fmt.Sprintf("cannot decode Bitbucket commit statuses from response payload: %v", err)) + + foundStatus := false + for _, status := range statuses { + if strings.Contains(status.Key, "my-long-pipelinerun-name") { + foundStatus = true + break + } + } + assert.Equal(t, foundStatus, true, "should have found the status for the pipeline run") +} + // Local Variables: // compile-command: "go test -tags=e2e -v -run TestBitbucketCloudPullRequest$ ." // End: