fix: block merge on workspace block#67
Open
horakihor wants to merge 2 commits into
Open
Conversation
Signed-off-by: Ihor Horak <ihor.horak@zapier.com>
Signed-off-by: Ihor Horak <ihor.horak@zapier.com>
There was a problem hiding this comment.
Pull request overview
Ensures merge requests are properly gated when TFBuddy blocks a workspace due to target-branch drift by publishing failing commit statuses (so required-status checks can actually fail).
Changes:
- Extend the VCS
GitClientinterface withSetMergeRequestStatusfor publishing commit statuses without a TFC run. - Implement
SetMergeRequestStatusfor GitLab and invoke it when a workspace is blocked (publishing failed statuses for bothTFC/plan/<ws>andTFC/apply/<ws>). - Regenerate mocks and update trigger tests to assert the new status publications.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/vcs/interfaces.go | Adds SetMergeRequestStatus to the VCS client interface for merge gating. |
| pkg/vcs/gitlab/client.go | Implements SetMergeRequestStatus by creating commit statuses without pipeline attachment. |
| pkg/vcs/github/client.go | Adds a stub SetMergeRequestStatus implementation (currently TODO). |
| pkg/tfc_trigger/tfc_trigger.go | Publishes failing plan/apply commit statuses when a workspace is blocked. |
| pkg/tfc_trigger/tfc_trigger_test.go | Asserts SetMergeRequestStatus calls for blocked workspaces. |
| pkg/mocks/mock_vcs.go | Regenerates GoMock bindings for the updated interface. |
Files not reviewed (1)
- pkg/mocks/mock_vcs.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+352
to
+355
| func (c *Client) SetMergeRequestStatus(ctx context.Context, projectWithNS, commitSHA, name, state, description, targetURL string) error { | ||
| //TODO implement me | ||
| return nil | ||
| } |
Comment on lines
+468
to
+475
| func (t *TFCTrigger) publishBlockedStatus(ctx context.Context, workspace string) { | ||
| const description = "Blocked: target branch modified workspace paths since divergence." | ||
| for _, action := range []string{"plan", "apply"} { | ||
| name := fmt.Sprintf("TFC/%s/%s", action, workspace) | ||
| if err := t.gl.SetMergeRequestStatus(ctx, t.GetProjectNameWithNamespace(), t.GetCommitSHA(), name, "failed", description, ""); err != nil { | ||
| log.Error().Err(err).Str("ws", workspace).Str("action", action).Msg("could not publish blocked commit status") | ||
| } | ||
| } |
Comment on lines
+117
to
+121
| func (c *GitlabClient) SetMergeRequestStatus(ctx context.Context, projectWithNS, commitSHA, name, state, description, targetURL string) error { | ||
| _, span := otel.Tracer("TFC").Start(ctx, "SetMergeRequestStatus") | ||
| defer span.End() | ||
|
|
||
| opts := &gogitlab.SetCommitStatusOptions{ |
Comment on lines
+19
to
+20
| // SetMergeRequestStatus publishes a commit status without a TFC run, used | ||
| // to gate the merge when tfbuddy refuses to run a workspace. |
Comment on lines
+352
to
+355
| func (c *Client) SetMergeRequestStatus(ctx context.Context, projectWithNS, commitSHA, name, state, description, targetURL string) error { | ||
| //TODO implement me | ||
| return nil | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When tfbuddy detects that a workspace's relevant paths were modified on the
target branch since the source branch diverged, it refuses to start a TFC run
for that workspace and posts a
:no_entry:MR comment. However, it neverpublishes a failing GitLab commit status, so GitLab's required-status check
has nothing to fail on and the MR can still be merged.
This change publishes failing commit statuses for
TFC/plan/<ws>andTFC/apply/<ws>(both, to match the pre-emptive pattern inmr_status_updater.go) so the merge gate fires.Changes
pkg/vcs/interfaces.go— addSetMergeRequestStatus(ctx, project, sha, name, state, description, targetURL) errortoGitClient. Primitive args keep the trigger code VCS-agnostic.pkg/vcs/gitlab/client.go— implements the method: buildsgogitlab.SetCommitStatusOptionsand delegates to existingSetCommitStatus. Skips the 30s pipeline-lookup backoff used by run-status updates; a commit-level status is sufficient to gate the merge for blocked workspaces.pkg/vcs/github/client.go—nilstub consistent with the existingSetCommitStatusTODO.pkg/tfc_trigger/tfc_trigger.go—dispatchWorkspacesnow callspublishBlockedStatus(ctx, ws.Name)on the blocked branch, which setsTFC/plan/<ws>=failedandTFC/apply/<ws>=failed. Both are set so the merge gate fires regardless of which is the required status.pkg/mocks/mock_vcs.go— regenerated viago generate ./pkg/vcs/....pkg/tfc_trigger/tfc_trigger_test.go—TestTFCEvents_WorkspaceApplyModifiedBothSrcDstBranchesnow asserts bothSetMergeRequestStatuscalls.Rebase behavior
No explicit cleanup needed. A new commit means a new SHA; tfbuddy re-evaluates
on the push webhook:
mr_status_updater.go).