From b1a4d94091b87ca0a8d36a7b75095f7ae5d7ccef Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 17 Sep 2016 20:33:41 -0700 Subject: Only create a protected branch upon a push to a new branch if a rule for that branch doesn't exist A customer ran into an issue where a Sidekiq task retried over and over, leading to duplicate master branches in their protected branch list. Closes #22177 --- CHANGELOG | 1 + app/services/git_push_service.rb | 2 +- spec/services/git_push_service_spec.rb | 15 +++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index e9445a18a18..b4921fedb52 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -6,6 +6,7 @@ v 8.12.0 (unreleased) - Bump fog-aws to v0.11.0 to support ap-south-1 region - Add ability to fork to a specific namespace using API. (ritave) - Cleanup misalignments in Issue list view !6206 + - Only create a protected branch upon a push to a new branch if a rule for that branch doesn't exist - Prune events older than 12 months. (ritave) - Prepend blank line to `Closes` message on merge request linked to issue (lukehowell) - Fix issues/merge-request templates dropdown for forked projects diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 78feb37aa2a..948041063c0 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -87,7 +87,7 @@ class GitPushService < BaseService project.change_head(branch_name) # Set protection on the default branch if configured - if current_application_settings.default_branch_protection != PROTECTION_NONE + if current_application_settings.default_branch_protection != PROTECTION_NONE && !@project.protected_branch?(@project.default_branch) params = { name: @project.default_branch, diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 6ac1fa8f182..22724434a7f 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -253,6 +253,21 @@ describe GitPushService, services: true do expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MASTER]) end + it "when pushing a branch for the first time with an existing branch permission configured" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + + create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + expect_any_instance_of(ProtectedBranches::CreateService).not_to receive(:execute) + + execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) + + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) + expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end + it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) -- cgit v1.2.1