Skip to content

Commit 70c6732

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> Signed-off-by: Zaki Shaikh <zashaikh@redhat.com>
1 parent 7c46558 commit 70c6732

6 files changed

Lines changed: 207 additions & 2 deletions

File tree

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"
@@ -143,6 +146,65 @@ func TestBitbucketCloudCELExpressionOnPush(t *testing.T) {
143146
twait.Succeeded(ctx, t, runcnx, opts, sopt)
144147
}
145148

149+
func TestBitbucketCloudPRBuildStatusReported(t *testing.T) {
150+
targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
151+
ctx := context.Background()
152+
153+
runcnx, opts, bprovider, err := tbb.Setup(ctx)
154+
if err != nil {
155+
t.Skip(err.Error())
156+
return
157+
}
158+
bcrepo := tbb.CreateCRD(ctx, t, bprovider, runcnx, opts, targetNS)
159+
targetRefName := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test")
160+
title := "TestPullRequest - " + targetRefName
161+
162+
entries, err := payload.GetEntries(
163+
map[string]string{".tekton/pipelinerun.yaml": "testdata/pipelinerun.yaml"},
164+
targetNS, options.MainBranch, triggertype.PullRequest.String(), map[string]string{})
165+
assert.NilError(t, err)
166+
167+
pr, repobranch := tbb.MakePR(t, bprovider, runcnx, bcrepo, opts, title, targetRefName, entries)
168+
defer tbb.TearDown(ctx, t, runcnx, bprovider, opts, pr.ID, targetRefName, targetNS, false)
169+
170+
hash, ok := repobranch.Target["hash"].(string)
171+
assert.Assert(t, ok)
172+
173+
sopt := twait.SuccessOpt{
174+
TargetNS: targetNS,
175+
OnEvent: triggertype.PullRequest.String(),
176+
NumberofPRMatch: 1,
177+
SHA: hash,
178+
Title: title,
179+
MinNumberStatus: 1,
180+
}
181+
twait.Succeeded(ctx, t, runcnx, opts, sopt)
182+
183+
resp, err := bprovider.Client().Repositories.Commits.GetCommitStatuses(&bitbucket.CommitsOptions{
184+
Owner: opts.Organization,
185+
RepoSlug: opts.Repo,
186+
Revision: hash,
187+
})
188+
assert.NilError(t, err)
189+
190+
statusesMap, ok := resp.(map[string]any)
191+
assert.Equal(t, ok, true, "cannot convert Bitbucket commit statuses response to map[string]any")
192+
193+
statuses := []*types.Status{}
194+
195+
err = mapstructure.Decode(statusesMap["values"], &statuses)
196+
assert.NilError(t, err, fmt.Sprintf("cannot decode Bitbucket commit statuses from response payload: %v", err))
197+
198+
foundStatus := false
199+
for _, status := range statuses {
200+
if strings.Contains(status.Key, "pipelinerun-") {
201+
foundStatus = true
202+
break
203+
}
204+
}
205+
assert.Equal(t, foundStatus, true, "should have found the status for the pipeline run")
206+
}
207+
146208
// Local Variables:
147209
// compile-command: "go test -tags=e2e -v -run TestBitbucketCloudPullRequest$ ."
148210
// End:

0 commit comments

Comments
 (0)