From b2c73fde791ebac6c2cc615fce19294190b05609 Mon Sep 17 00:00:00 2001
From: Nick Thomas <nick@gitlab.com>
Date: Mon, 17 Jun 2019 16:12:05 +0100
Subject: [PATCH] Look for new branches more carefully
In certain cases, GitLab can miss a PostReceive invocation the first
time a branch is pushed. When this happens, the "branch created" hooks
are not run, which means various features don't work until the branch
is deleted and pushed again.
This MR changes the `Git::BranchPushService` so it checks the cache of
existing branches in addition to the `oldrev` reported for the branch.
If the branch name isn't in the cache, chances are we haven't run the
service yet (it's what refreshes the cache), so we can go ahead and
run it, even through `oldrev` is set.
If the cache has been cleared by some other means in the meantime, then
we'll still fail to run the hooks when we should. Fixing that in the
general case is a larger problem, and we'd need to devote significant
engineering effort to it.
There's a chance that we'll run the relevant hooks *multiple times*
with this change, if there's a race between the branch being created,
and the `PostReceive` worker being run multiple times, but this can
already happen, since Sidekiq is "at-least-once" execution of jobs. So,
this should be safe.
---
app/services/git/branch_hooks_service.rb | 8 ++++-
.../59257-find-new-branches-harder.yml | 5 +++
.../services/git/branch_hooks_service_spec.rb | 34 +++++++++++++++++++
3 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 changelogs/unreleased/59257-find-new-branches-harder.yml
diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb
index 4aee48f22e7..c41f445c3c4 100644
--- a/app/services/git/branch_hooks_service.rb
+++ b/app/services/git/branch_hooks_service.rb
@@ -105,8 +105,14 @@ module Git
CreateGpgSignatureWorker.perform_async(signable, project.id)
end
+ # It's not sufficient to just check for a blank SHA as it's possible for the
+ # branch to be pushed, but for the `post-receive` hook to never run:
+ # https://gitlab.com/gitlab-org/gitlab-ce/issues/59257
def creating_branch?
- Gitlab::Git.blank_ref?(params[:oldrev])
+ strong_memoize(:creating_branch) do
+ Gitlab::Git.blank_ref?(params[:oldrev]) ||
+ !project.repository.branch_exists?(branch_name)
+ end
end
def updating_branch?
diff --git a/changelogs/unreleased/59257-find-new-branches-harder.yml b/changelogs/unreleased/59257-find-new-branches-harder.yml
new file mode 100644
index 00000000000..631eaa22ef0
--- /dev/null
+++ b/changelogs/unreleased/59257-find-new-branches-harder.yml
@@ -0,0 +1,5 @@
+---
+title: Look for new branches more carefully
+merge_request: 29761
+author:
+type: fixed
diff --git a/spec/services/git/branch_hooks_service_spec.rb b/spec/services/git/branch_hooks_service_spec.rb
index b5694628269..23be400059e 100644
--- a/spec/services/git/branch_hooks_service_spec.rb
+++ b/spec/services/git/branch_hooks_service_spec.rb
@@ -344,4 +344,38 @@ describe Git::BranchHooksService do
end
end
end
+
+ describe 'New branch detection' do
+ let(:branch) { 'fix' }
+
+ context 'oldrev is the blank SHA' do
+ let(:oldrev) { Gitlab::Git::BLANK_SHA }
+
+ it 'is treated as a new branch' do
+ expect(service).to receive(:branch_create_hooks)
+
+ service.execute
+ end
+ end
+
+ context 'oldrev is set' do
+ context 'Gitaly does not know about the branch' do
+ it 'is treated as a new branch' do
+ allow(project.repository).to receive(:branch_names) { [] }
+
+ expect(service).to receive(:branch_create_hooks)
+
+ service.execute
+ end
+ end
+
+ context 'Gitaly knows about the branch' do
+ it 'is not treated as a new branch' do
+ expect(service).not_to receive(:branch_create_hooks)
+
+ service.execute
+ end
+ end
+ end
+ end
end
--
2.30.9