Skip to content

Commit 3b07a7f

Browse files
authored
fix: attach pipeline ID to GitLab commit status updates (#65)
The closure in updateStatus used to assign the resolved pipeline ID, shadowing the outer variable and discarding the value. Every SetCommitStatus call was issued without a PipelineID, so GitLab couldn't associate the status with the current MR pipeline — leaving the apply check stuck after no-change plan/applies and causing pipeline-status links to point at stale runs. Signed-off-by: Mmadu Manasseh <mmadumanasseh@gmail.com>
1 parent 613401b commit 3b07a7f

2 files changed

Lines changed: 62 additions & 1 deletion

File tree

pkg/vcs/gitlab/mr_status_updater.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ func (p *RunStatusUpdater) updateStatus(ctx context.Context, state gogitlab.Buil
124124
var pipelineID *int
125125
getPipelineIDFn := func() error {
126126
log.Debug().Str("project", rmd.GetMRProjectNameWithNamespace()).Int("mergeRequestID", rmd.GetMRInternalID()).Msg("getting pipeline status")
127-
pipelineID := p.getLatestPipelineID(ctx, rmd)
127+
pipelineID = p.getLatestPipelineID(ctx, rmd)
128128
if pipelineID == nil {
129129
return errNoPipelineStatus
130130
}

pkg/vcs/gitlab/mr_status_updater_test.go

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,67 @@ func (m *commitStatusStateMatcher) String() string {
3131
return "matches commit status with state=" + m.expectedState
3232
}
3333

34+
// pipelineIDMatcher asserts that the commit status options carry the expected pipeline ID.
35+
type pipelineIDMatcher struct {
36+
expectedID int
37+
}
38+
39+
func (m *pipelineIDMatcher) Matches(x interface{}) bool {
40+
opts, ok := x.(*GitlabCommitStatusOptions)
41+
if !ok {
42+
return false
43+
}
44+
if opts.PipelineID == nil {
45+
return false
46+
}
47+
return *opts.PipelineID == m.expectedID
48+
}
49+
50+
func (m *pipelineIDMatcher) String() string {
51+
return "matches commit status whose PipelineID is set to the expected value"
52+
}
53+
54+
// TestUpdateStatusAttachesPipelineID guards against a regression of the closure
55+
// shadowing bug where the resolved pipeline ID was discarded and SetCommitStatus
56+
// was called with PipelineID == nil. Without an attached pipeline ID, GitLab
57+
// can't associate the status with the current MR pipeline, leaving the "apply"
58+
// check stuck and pipeline-status links pointing at stale runs.
59+
func TestUpdateStatusAttachesPipelineID(t *testing.T) {
60+
mockCtrl := gomock.NewController(t)
61+
defer mockCtrl.Finish()
62+
testSuite := mocks.CreateTestSuite(mockCtrl, mocks.TestOverrides{}, t)
63+
64+
const expectedPipelineID = 42
65+
testSuite.MockGitClient.EXPECT().
66+
GetPipelinesForCommit(gomock.Any(), gomock.Any(), gomock.Any()).
67+
Return([]vcs.ProjectPipeline{&GitlabPipeline{&gogitlab.PipelineInfo{ID: expectedPipelineID, Source: "merge_request_event"}}}, nil).
68+
AnyTimes()
69+
70+
testSuite.MockGitClient.EXPECT().
71+
SetCommitStatus(
72+
gomock.Any(),
73+
gomock.Any(),
74+
gomock.Any(),
75+
&pipelineIDMatcher{expectedID: expectedPipelineID},
76+
).
77+
Return(&GitlabCommitStatus{&gogitlab.CommitStatus{}}, nil).
78+
Times(1)
79+
80+
testSuite.InitTestSuite()
81+
r := &RunStatusUpdater{
82+
cfg: config.C,
83+
tfc: testSuite.MockApiClient,
84+
client: testSuite.MockGitClient,
85+
rs: testSuite.MockStreamClient,
86+
}
87+
88+
r.updateStatus(context.Background(), gogitlab.Success, "apply", &runstream.TFRunMetadata{
89+
Action: "apply",
90+
Workspace: "service-tfbuddy",
91+
RunID: "run-123",
92+
})
93+
}
94+
3495
func TestAutoMergeNoChangesApply(t *testing.T) {
3596

3697
mockCtrl := gomock.NewController(t)

0 commit comments

Comments
 (0)