Skip to content

Commit f695fc1

Browse files
zakiskclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 47ee286 commit f695fc1

9 files changed

Lines changed: 221 additions & 14 deletions

File tree

pkg/apis/pipelinesascode/keys/keys.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,10 @@ const (
6464
SecretCreated = pipelinesascode.GroupName + "/secret-created"
6565
CloneURL = pipelinesascode.GroupName + "/clone-url"
6666
// PublicGithubAPIURL default is "https://api.github.com" but it can be overridden by X-GitHub-Enterprise-Host header.
67-
PublicGithubAPIURL = "https://api.github.com"
68-
GithubApplicationID = "github-application-id"
69-
GithubPrivateKey = "github-private-key"
70-
ResultsRecordSummary = "results.tekton.dev/recordSummaryAnnotations"
71-
67+
PublicGithubAPIURL = "https://api.github.com"
68+
GithubApplicationID = "github-application-id"
69+
GithubPrivateKey = "github-private-key"
70+
ResultsRecordSummary = "results.tekton.dev/recordSummaryAnnotations"
7271
SpanContextAnnotation = "tekton.dev/pipelinerunSpanContext"
7372
)
7473

pkg/matcher/annotation_matcher.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ func getTargetBranch(prun *tektonv1.PipelineRun, event *info.Event) (bool, strin
157157

158158
type Match struct {
159159
PipelineRun *tektonv1.PipelineRun
160+
Index int
160161
Repo *apipac.Repository
161162
Config map[string]string
162163
}

pkg/pipelineascode/pipelineascode.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ func (p *PacRun) Run(ctx context.Context) error {
146146

147147
go func(match matcher.Match, i int) {
148148
defer wg.Done()
149+
match.Index = i
149150
p.debugf("starting pipelinerun %s (index=%d)", match.PipelineRun.GetGenerateName(), i)
150151
pr, err := p.startPR(ctx, match)
151152
if err != nil {
@@ -259,13 +260,14 @@ func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.Pi
259260
return nil, fmt.Errorf("cannot create message template: %w", err)
260261
}
261262
status := providerstatus.StatusOpts{
262-
Status: inProgressStatus,
263-
Conclusion: providerstatus.ConclusionPending,
264-
Text: msg,
265-
DetailsURL: consoleURL,
266-
PipelineRunName: pr.GetName(),
267-
PipelineRun: pr,
268-
OriginalPipelineRunName: pr.GetAnnotations()[keys.OriginalPRName],
263+
Status: inProgressStatus,
264+
Conclusion: providerstatus.ConclusionPending,
265+
Text: msg,
266+
DetailsURL: consoleURL,
267+
PipelineRunName: pr.GetName(),
268+
PipelineRun: pr,
269+
OriginalPipelineRunName: pr.GetAnnotations()[keys.OriginalPRName],
270+
InstanceCountForCheckRun: match.Index,
269271
}
270272

271273
// Patch the pipelineRun with the appropriate annotations and labels.

pkg/provider/bitbucketcloud/bitbucket.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,10 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
113113
detailsURL = statusopts.DetailsURL
114114
}
115115

116+
key := provider.GetBBCloudStatusKey(statusopts, v.pacInfo)
117+
116118
cso := &bitbucket.CommitStatusOptions{
117-
Key: provider.GetCheckName(statusopts, v.pacInfo),
119+
Key: key,
118120
Url: detailsURL,
119121
State: string(state),
120122
Description: statusopts.Title,

pkg/provider/bitbucketcloud/test/bbcloudtest.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ func MuxCreateCommitstatus(t *testing.T, mux *http.ServeMux, event *info.Event,
189189
err := json.Unmarshal(bit, cso)
190190
assert.NilError(t, err)
191191
pacOpts := &info.PacOpts{Settings: settings.Settings{ApplicationName: applicationName}}
192-
assert.Equal(t, provider.GetCheckName(expStatus, pacOpts), cso.Key)
192+
assert.Equal(t, provider.GetBBCloudStatusKey(expStatus, pacOpts), cso.Key)
193193

194194
if expStatus.DetailsURL != "" {
195195
assert.Equal(t, expStatus.DetailsURL, cso.Url)

pkg/provider/bitbucketcloud/types/types.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,19 @@ type Comments struct {
125125
Values []Comment
126126
}
127127

128+
type Status struct {
129+
Key string `json:"key"`
130+
Type string `json:"type"`
131+
State string `json:"state"`
132+
Name string `json:"name"`
133+
RefName string `json:"refname"`
134+
Commit Commit `json:"commit"`
135+
URL string `json:"url"`
136+
Repository Repository `json:"repository"`
137+
Description string `json:"description"`
138+
Links Links `json:"links"`
139+
}
140+
128141
type CommitStatusState string
129142

130143
// See: https://api.bitbucket.org/swagger.json attribute:

pkg/provider/provider.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package provider
22

33
import (
4+
"crypto/sha256"
5+
"encoding/hex"
46
"fmt"
57
"net/url"
68
"regexp"
@@ -221,6 +223,42 @@ func GetCheckName(status providerstatus.StatusOpts, pacopts *info.PacOpts) strin
221223
return status.OriginalPipelineRunName
222224
}
223225

226+
// GetBBCloudStatusKey returns a unique key for Bitbucket Cloud build commit status.
227+
// Bitbucket Cloud limits the key to 40 characters. When both ApplicationName and
228+
// OriginalPipelineRunName are set and their combined form ("{appName} / {prName}")
229+
// fits within 40 chars, that combined form is used. Otherwise, the
230+
// OriginalPipelineRunName alone is used. If the name exceeds 40 chars, it is
231+
// truncated to 33 chars with a 6-char hash suffix for uniqueness. If only
232+
// ApplicationName is set, it is returned truncated to 40 chars.
233+
func GetBBCloudStatusKey(status providerstatus.StatusOpts, pacopts *info.PacOpts) string {
234+
if pacopts.ApplicationName != "" {
235+
if status.OriginalPipelineRunName == "" {
236+
if len(pacopts.ApplicationName) > 40 {
237+
return pacopts.ApplicationName[:40]
238+
}
239+
return pacopts.ApplicationName
240+
}
241+
key := fmt.Sprintf("%s / %s", pacopts.ApplicationName, status.OriginalPipelineRunName)
242+
// if application name and pipeline run name combined are less than 40 characters, return the key
243+
// otherwise, skip it here and let the only pipeline run being returned.
244+
if len(key) <= 40 {
245+
return key
246+
}
247+
}
248+
249+
if len(status.OriginalPipelineRunName) > 40 {
250+
hash := getNameHash(status.OriginalPipelineRunName)
251+
return fmt.Sprintf("%s-%s", status.OriginalPipelineRunName[:33], hash)
252+
}
253+
254+
return status.OriginalPipelineRunName
255+
}
256+
257+
func getNameHash(input string) string {
258+
hash := sha256.Sum256([]byte(input))
259+
return hex.EncodeToString(hash[:])[:6]
260+
}
261+
224262
func IsZeroSHA(sha string) bool {
225263
return sha == "0000000000000000000000000000000000000000"
226264
}

pkg/provider/provider_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package provider
22

33
import (
4+
"strings"
45
"testing"
56

67
"github.com/openshift-pipelines/pipelines-as-code/pkg/apis/pipelinesascode/v1alpha1"
@@ -867,3 +868,92 @@ func TestGetGitOpsCommentPrefix(t *testing.T) {
867868
})
868869
}
869870
}
871+
872+
func TestGetBBCloudStatusKey(t *testing.T) {
873+
tests := []struct {
874+
name string
875+
status status.StatusOpts
876+
pacopts *info.PacOpts
877+
expected string
878+
}{
879+
{
880+
name: "application name only no pipeline run name",
881+
status: status.StatusOpts{OriginalPipelineRunName: ""},
882+
pacopts: &info.PacOpts{
883+
Settings: settings.Settings{ApplicationName: "MyApp"},
884+
},
885+
expected: "MyApp",
886+
},
887+
{
888+
name: "application name longer than 40 truncated",
889+
status: status.StatusOpts{OriginalPipelineRunName: ""},
890+
pacopts: &info.PacOpts{
891+
Settings: settings.Settings{ApplicationName: strings.Repeat("x", 50)},
892+
},
893+
expected: strings.Repeat("x", 40),
894+
},
895+
{
896+
name: "app and pr name combined fit in 40 chars",
897+
status: status.StatusOpts{OriginalPipelineRunName: "my-pr"},
898+
pacopts: &info.PacOpts{
899+
Settings: settings.Settings{ApplicationName: "MyApp"},
900+
},
901+
expected: "MyApp / my-pr",
902+
},
903+
{
904+
name: "app and pr name combined exceed 40 chars falls back to pr name only",
905+
status: status.StatusOpts{OriginalPipelineRunName: "this-is-a-very-long-pipeline-run-name"},
906+
pacopts: &info.PacOpts{
907+
Settings: settings.Settings{ApplicationName: "MyApplication"},
908+
},
909+
expected: "this-is-a-very-long-pipeline-run-name",
910+
},
911+
{
912+
name: "no application name short pipeline run name",
913+
status: status.StatusOpts{OriginalPipelineRunName: "my-pr"},
914+
pacopts: &info.PacOpts{
915+
Settings: settings.Settings{ApplicationName: ""},
916+
},
917+
expected: "my-pr",
918+
},
919+
{
920+
name: "no application name pipeline run name exactly 40",
921+
status: status.StatusOpts{OriginalPipelineRunName: strings.Repeat("a", 40)},
922+
pacopts: &info.PacOpts{
923+
Settings: settings.Settings{ApplicationName: ""},
924+
},
925+
expected: strings.Repeat("a", 40),
926+
},
927+
{
928+
name: "no application name pipeline run name longer than 40 truncated with hash",
929+
status: status.StatusOpts{OriginalPipelineRunName: strings.Repeat("c", 50)},
930+
pacopts: &info.PacOpts{
931+
Settings: settings.Settings{ApplicationName: ""},
932+
},
933+
expected: strings.Repeat("c", 33) + "-5de6bf",
934+
},
935+
{
936+
name: "no application name no pipeline run name",
937+
status: status.StatusOpts{OriginalPipelineRunName: ""},
938+
pacopts: &info.PacOpts{
939+
Settings: settings.Settings{ApplicationName: ""},
940+
},
941+
expected: "",
942+
},
943+
{
944+
name: "long pr name with app produces stable hash",
945+
status: status.StatusOpts{OriginalPipelineRunName: strings.Repeat("b", 50)},
946+
pacopts: &info.PacOpts{
947+
Settings: settings.Settings{ApplicationName: "App"},
948+
},
949+
expected: strings.Repeat("b", 33) + "-c01a9b",
950+
},
951+
}
952+
for _, tt := range tests {
953+
t.Run(tt.name, func(t *testing.T) {
954+
got := GetBBCloudStatusKey(tt.status, tt.pacopts)
955+
assert.Equal(t, tt.expected, got)
956+
assert.Assert(t, len(got) <= 40, "key length %d exceeds 40 char limit: %s", len(got), got)
957+
})
958+
}
959+
}

test/bitbucket_cloud_pullrequest_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ package test
55
import (
66
"context"
77
"fmt"
8+
"strings"
89
"testing"
910
"time"
1011

1112
"github.com/ktrysmt/go-bitbucket"
13+
"github.com/mitchellh/mapstructure"
1214
"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
15+
"github.com/openshift-pipelines/pipelines-as-code/pkg/provider/bitbucketcloud/types"
1316
tbb "github.com/openshift-pipelines/pipelines-as-code/test/pkg/bitbucketcloud"
1417
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/options"
1518
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
@@ -107,6 +110,65 @@ func TestBitbucketCloudPullRequestCancelInProgressMerged(t *testing.T) {
107110
assert.Equal(t, prs.Items[0].GetStatusCondition().GetCondition(apis.ConditionSucceeded).GetReason(), "Cancelled", "should have been cancelled")
108111
}
109112

113+
func TestBitbucketCloudPRBuildStatusReported(t *testing.T) {
114+
targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
115+
ctx := context.Background()
116+
117+
runcnx, opts, bprovider, err := tbb.Setup(ctx)
118+
if err != nil {
119+
t.Skip(err.Error())
120+
return
121+
}
122+
bcrepo := tbb.CreateCRD(ctx, t, bprovider, runcnx, opts, targetNS)
123+
targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
124+
title := "TestPullRequest - " + targetRefName
125+
126+
entries, err := payload.GetEntries(
127+
map[string]string{".tekton/pipelinerun.yaml": "testdata/pipelinerun.yaml"},
128+
targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{})
129+
assert.NilError(t, err)
130+
131+
pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetRefName, entries)
132+
defer tbb.TearDown(ctx, t, runcnx, bprovider, opts, pr.ID, targetRefName, targetNS, false)
133+
134+
hash, ok := repobranch.Target["hash"].(string)
135+
assert.Assert(t, ok)
136+
137+
sopt := twait.SuccessOpt{
138+
TargetNS: targetNS,
139+
OnEvent: triggertype.PullRequest.String(),
140+
NumberofPRMatch: 1,
141+
SHA: hash,
142+
Title: title,
143+
MinNumberStatus: 1,
144+
}
145+
twait.Succeeded(ctx, t, runcnx, opts, sopt)
146+
147+
resp, err := bprovider.Client().Repositories.Commits.GetCommitStatuses(&bitbucket.CommitsOptions{
148+
Owner: opts.Organization,
149+
RepoSlug: opts.Repo,
150+
Revision: hash,
151+
})
152+
assert.NilError(t, err)
153+
154+
statusesMap, ok := resp.(map[string]any)
155+
assert.Equal(t, ok, true, "cannot convert Bitbucket commit statuses response to map[string]any")
156+
157+
statuses := []*types.Status{}
158+
159+
err = mapstructure.Decode(statusesMap["values"], &statuses)
160+
assert.NilError(t, err, fmt.Sprintf("cannot decode Bitbucket commit statuses from response payload: %v", err))
161+
162+
foundStatus := false
163+
for _, status := range statuses {
164+
if strings.Contains(status.Key, "pipelinerun-") {
165+
foundStatus = true
166+
break
167+
}
168+
}
169+
assert.Equal(t, foundStatus, true, "should have found the status for the pipeline run")
170+
}
171+
110172
// Local Variables:
111173
// compile-command: "go test -tags=e2e -v -run TestBitbucketCloudPullRequest$ ."
112174
// End:

0 commit comments

Comments
 (0)