From b278d886ba65e2d3d438352b6243cd33b1ba4636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 9 Nov 2018 22:17:43 +0100 Subject: [PATCH 01/33] Support both ref and ref-name in protected_for? --- app/models/project.rb | 27 ++++++++++++--- spec/models/project_spec.rb | 68 ++++++++++++++++++++++++++++++++++--- 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index f5dc58cd67f..61840c972ee 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -36,6 +36,7 @@ class Project < ActiveRecord::Base extend Gitlab::ConfigHelper BoardLimitExceeded = Class.new(StandardError) + AmbiguousRef = Class.new(StandardError) STATISTICS_ATTRIBUTE = 'repositories_count'.freeze NUMBER_OF_PERMITTED_BOARDS = 1 @@ -1160,6 +1161,21 @@ class Project < ActiveRecord::Base end end + def resolve_ref(ref) + tag_exists = repository.tag_exists?(ref) + branch_exists = repository.branch_exists?(ref) + + if tag_exists && branch_exists + raise AmbiguousRef + elsif tag_exists + Gitlab::Git::TAG_REF_PREFIX + ref + elsif branch_exists + Gitlab::Git::BRANCH_REF_PREFIX + ref + else + ref + end + end + def root_ref?(branch) repository.root_ref == branch end @@ -1737,10 +1753,13 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - if repository.branch_exists?(ref) - ProtectedBranch.protected?(self, ref) - elsif repository.tag_exists?(ref) - ProtectedTag.protected?(self, ref) + full_ref = resolve_ref(ref) + ref_name = Gitlab::Git.ref_name(full_ref) + + if Gitlab::Git.branch_ref?(full_ref) + ProtectedBranch.protected?(self, ref_name) + elsif Gitlab::Git.tag_ref?(full_ref) + ProtectedTag.protected?(self, ref_name) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1c85411dc3b..a519435322c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2520,6 +2520,10 @@ describe Project do end context 'when the ref is not protected' do + before do + allow(project).to receive(:protected_for?).with('ref').and_return(false) + end + it 'contains only the CI variables' do is_expected.to contain_exactly(ci_variable) end @@ -2563,7 +2567,13 @@ describe Project do subject { project.protected_for?('ref') } + before do + allow(project).to receive(:resolve_ref).and_return(ref) + end + context 'when the ref is not protected' do + let(:ref) { 'refs/heads/ref' } + before do stub_application_setting( default_branch_protection: Gitlab::Access::PROTECTION_NONE) @@ -2575,9 +2585,9 @@ describe Project do end context 'when the ref is a protected branch' do + let(:ref) { 'refs/heads/ref' } + before do - allow(project).to receive(:repository).and_call_original - allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(true) create(:protected_branch, name: 'ref', project: project) end @@ -2587,9 +2597,9 @@ describe Project do end context 'when the ref is a protected tag' do + let(:ref) { 'refs/tags/ref' } + before do - allow(project).to receive_message_chain(:repository, :branch_exists?).and_return(false) - allow(project).to receive_message_chain(:repository, :tag_exists?).and_return(true) create(:protected_tag, name: 'ref', project: project) end @@ -2778,6 +2788,56 @@ describe Project do end end + describe '#resolve_ref' do + let(:project) { create(:project) } + + subject { project.resolve_ref(ref) } + + context 'when ref is full ref' do + let(:ref) { 'refs/heads/master' } + + it 'returns the unchanged ref' do + is_expected.to eq(ref) + end + end + + context 'when ref is a tag or branch name' do + let(:ref) { 'ref' } + + before do + allow(project.repository).to receive(:tag_exists?).and_return(tag_exists) + allow(project.repository).to receive(:branch_exists?).and_return(branch_exists) + end + + context 'when ref is ambiguous' do + let(:tag_exists) { true } + let(:branch_exists) { true } + + it 'raises an error' do + expect { subject }.to raise_error(described_class::AmbiguousRef) + end + end + + context 'when ref is tag name' do + let(:tag_exists) { true } + let(:branch_exists) { false } + + it 'returns full tag ref path' do + is_expected.to eq('refs/tags/ref') + end + end + + context 'when ref is branch name' do + let(:tag_exists) { false } + let(:branch_exists) { true } + + it 'returns full branch ref path' do + is_expected.to eq('refs/heads/ref') + end + end + end + end + describe '#http_url_to_repo' do let(:project) { create(:project) } From 1bf580688890a3b13e7ac0468f29108dafe08f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 10 Nov 2018 14:34:53 +0100 Subject: [PATCH 02/33] Use full ref when possible to avoid ambiguity --- app/models/ci/build.rb | 5 ++-- app/models/ci/pipeline.rb | 3 ++- app/models/concerns/has_ref.rb | 21 +++++++++++++++++ lib/gitlab/ci/pipeline/chain/command.rb | 2 +- spec/models/ci/build_spec.rb | 12 ++++++---- spec/models/ci/pipeline_spec.rb | 4 ++++ spec/models/concerns/has_ref_spec.rb | 31 +++++++++++++++++++++++++ 7 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 app/models/concerns/has_ref.rb create mode 100644 spec/models/concerns/has_ref_spec.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d86a6eceb59..f5c21481f63 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -10,6 +10,7 @@ module Ci include Importable include Gitlab::Utils::StrongMemoize include Deployable + include HasRef belongs_to :project, inverse_of: :builds belongs_to :runner @@ -640,11 +641,11 @@ module Ci def secret_group_variables return [] unless project.group - project.group.ci_variables_for(ref, project) + project.group.ci_variables_for(git_ref, project) end def secret_project_variables(environment: persisted_environment) - project.ci_variables_for(ref: ref, environment: environment) + project.ci_variables_for(ref: git_ref, environment: environment) end def steps diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d06022a0fb7..a1a11d2f7ab 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -11,6 +11,7 @@ module Ci include Gitlab::Utils::StrongMemoize include AtomicInternalId include EnumWithNil + include HasRef belongs_to :project, inverse_of: :all_pipelines belongs_to :user @@ -588,7 +589,7 @@ module Ci end def protected_ref? - strong_memoize(:protected_ref) { project.protected_for?(ref) } + strong_memoize(:protected_ref) { project.protected_for?(git_ref) } end def legacy_trigger diff --git a/app/models/concerns/has_ref.rb b/app/models/concerns/has_ref.rb new file mode 100644 index 00000000000..79816841f7f --- /dev/null +++ b/app/models/concerns/has_ref.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module HasRef + extend ActiveSupport::Concern + + def branch? + !tag? + end + + private + + def git_ref + if branch? + Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s + elsif tag? + Gitlab::Git::TAG_REF_PREFIX + ref.to_s + else + raise ArgumentError, 'Invalid pipeline type!' + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 100b9521412..316c283d90b 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -54,7 +54,7 @@ module Gitlab def protected_ref? strong_memoize(:protected_ref) do - project.protected_for?(ref) + project.protected_for?(origin_ref) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 89f78f629d4..d4056b4f7f1 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2385,6 +2385,8 @@ describe Ci::Build do end context 'when protected variable is defined' do + let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref } + let(:protected_variable) do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end @@ -2397,7 +2399,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2405,7 +2407,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2430,6 +2432,8 @@ describe Ci::Build do end context 'when group protected variable is defined' do + let(:ref) { Gitlab::Git::BRANCH_REF_PREFIX + build.ref } + let(:protected_variable) do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end @@ -2442,7 +2446,7 @@ describe Ci::Build do context 'when the branch is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } @@ -2450,7 +2454,7 @@ describe Ci::Build do context 'when the tag is protected' do before do - allow(build.project).to receive(:protected_for?).with(build.ref).and_return(true) + allow(build.project).to receive(:protected_for?).with(ref).and_return(true) end it { is_expected.to include(protected_variable) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b67c6a4cffa..17f33785fda 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -397,6 +397,10 @@ describe Ci::Pipeline, :mailer do end describe '#protected_ref?' do + before do + pipeline.project = create(:project, :repository) + end + it 'delegates method to project' do expect(pipeline).not_to be_protected_ref end diff --git a/spec/models/concerns/has_ref_spec.rb b/spec/models/concerns/has_ref_spec.rb new file mode 100644 index 00000000000..ec3cf9a8580 --- /dev/null +++ b/spec/models/concerns/has_ref_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HasRef do + describe '#branch?' do + let(:pipeline) { create(:ci_pipeline) } + + subject { pipeline.branch? } + + context 'is not a tag' do + before do + pipeline.tag = false + end + + it 'return true when tag is set to false' do + is_expected.to be_truthy + end + end + + context 'is not a tag' do + before do + pipeline.tag = true + end + + it 'return false when tag is set to true' do + is_expected.to be_falsey + end + end + end +end From 2edc02143b2d361275fb97ce2904a58e7dc8ff38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 10 Nov 2018 15:48:22 +0100 Subject: [PATCH 03/33] Prevent creating pipelines with ambiguous refs --- lib/gitlab/ci/pipeline/chain/build.rb | 1 - lib/gitlab/ci/pipeline/chain/command.rb | 6 ----- lib/gitlab/ci/pipeline/chain/populate.rb | 5 +++++ .../ci/pipeline/chain/validate/abilities.rb | 2 +- .../ci/pipeline/chain/validate/repository.rb | 6 +++++ .../gitlab/ci/pipeline/chain/command_spec.rb | 22 ------------------- .../gitlab/ci/pipeline/chain/populate_spec.rb | 2 ++ .../chain/validate/repository_spec.rb | 21 ++++++++++++++++++ 8 files changed, 35 insertions(+), 30 deletions(-) diff --git a/lib/gitlab/ci/pipeline/chain/build.rb b/lib/gitlab/ci/pipeline/chain/build.rb index d33d1edfe35..41632211374 100644 --- a/lib/gitlab/ci/pipeline/chain/build.rb +++ b/lib/gitlab/ci/pipeline/chain/build.rb @@ -17,7 +17,6 @@ module Gitlab user: @command.current_user, pipeline_schedule: @command.schedule, merge_request: @command.merge_request, - protected: @command.protected_ref?, variables_attributes: Array(@command.variables_attributes) ) diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 316c283d90b..ee5022e47c4 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -51,12 +51,6 @@ module Gitlab def before_sha self[:before_sha] || checkout_sha || Gitlab::Git::BLANK_SHA end - - def protected_ref? - strong_memoize(:protected_ref) do - project.protected_for?(origin_ref) - end - end end end end diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb index 633d3cd4f6b..45b4393ecf3 100644 --- a/lib/gitlab/ci/pipeline/chain/populate.rb +++ b/lib/gitlab/ci/pipeline/chain/populate.rb @@ -18,6 +18,11 @@ module Gitlab # @command.seeds_block&.call(pipeline) + ## + # Populate pipeline protected status + # + pipeline.protected = @command.project.protected_for?(@command.origin_ref) + ## # Populate pipeline with all stages, and stages with builds. # diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index ebd7e6e8289..e4979102fd9 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -31,7 +31,7 @@ module Gitlab if current_user allowed_to_create? else # legacy triggers don't have a corresponding user - !@command.protected_ref? + !@command.project.protected_for?(@command.origin_ref) end end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index d88851d8245..3cec55cdb71 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -16,6 +16,12 @@ module Gitlab unless @command.sha return error('Commit not found') end + + begin + @command.project.resolve_ref(@command.origin_ref) + rescue Project::AmbiguousRef + return error('Ref is ambiguous') + end end def break? diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 75a177d2d1f..14b4fbd1f5a 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -160,26 +160,4 @@ describe Gitlab::Ci::Pipeline::Chain::Command do end end end - - describe '#protected_ref?' do - let(:command) { described_class.new(project: project, origin_ref: 'my-branch') } - - subject { command.protected_ref? } - - context 'when a ref is protected' do - before do - expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true) - end - - it { is_expected.to eq(true) } - end - - context 'when a ref is unprotected' do - before do - expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false) - end - - it { is_expected.to eq(false) } - end - end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb index 284aed91e29..1b014ecfaa4 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb @@ -14,6 +14,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, + origin_ref: 'master', seeds_block: nil) end @@ -106,6 +107,7 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do Gitlab::Ci::Pipeline::Chain::Command.new( project: project, current_user: user, + origin_ref: 'master', seeds_block: seeds_block) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index fb1b53fc55c..7b30a8381e9 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -42,6 +42,27 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do end end + context 'when ref is ambiguous' do + let(:project) do + p = create(:project, :repository) + p.repository.add_tag(user, 'master', 'master') + p + end + let(:command) do + Gitlab::Ci::Pipeline::Chain::Command.new( + project: project, current_user: user, origin_ref: 'master') + end + + it 'breaks the chain' do + expect(step.break?).to be true + end + + it 'adds an error about missing ref' do + expect(pipeline.errors.to_a) + .to include 'Ref is ambiguous' + end + end + context 'when does not have existing SHA set' do let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( From 837ab8c5d83243db8efe1cec1c6e001d1cbeb43f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 15 Nov 2018 21:28:17 +0100 Subject: [PATCH 04/33] Make HasRef#git_ref public --- app/models/concerns/has_ref.rb | 4 ---- spec/models/concerns/has_ref_spec.rb | 28 ++++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/app/models/concerns/has_ref.rb b/app/models/concerns/has_ref.rb index 79816841f7f..d7089294efc 100644 --- a/app/models/concerns/has_ref.rb +++ b/app/models/concerns/has_ref.rb @@ -7,15 +7,11 @@ module HasRef !tag? end - private - def git_ref if branch? Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s elsif tag? Gitlab::Git::TAG_REF_PREFIX + ref.to_s - else - raise ArgumentError, 'Invalid pipeline type!' end end end diff --git a/spec/models/concerns/has_ref_spec.rb b/spec/models/concerns/has_ref_spec.rb index ec3cf9a8580..14db197dfe0 100644 --- a/spec/models/concerns/has_ref_spec.rb +++ b/spec/models/concerns/has_ref_spec.rb @@ -28,4 +28,32 @@ describe HasRef do end end end + + describe '#git_ref' do + subject { pipeline.git_ref } + + context 'when tag is true' do + let(:pipeline) { create(:ci_pipeline, tag: true) } + + it 'returns a tag ref' do + is_expected.to start_with(Gitlab::Git::TAG_REF_PREFIX) + end + end + + context 'when tag is false' do + let(:pipeline) { create(:ci_pipeline, tag: false) } + + it 'returns a branch ref' do + is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) + end + end + + context 'when tag is nil' do + let(:pipeline) { create(:ci_pipeline, tag: nil) } + + it 'returns a branch ref' do + is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) + end + end + end end From ce14c20a82af697b927666123443a25f19dab2ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 15 Nov 2018 21:56:13 +0100 Subject: [PATCH 05/33] Avoid using magic variable in spec --- .../gitlab/ci/pipeline/chain/validate/repository_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb index 7b30a8381e9..a7cad423d09 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/validate/repository_spec.rb @@ -44,9 +44,9 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Repository do context 'when ref is ambiguous' do let(:project) do - p = create(:project, :repository) - p.repository.add_tag(user, 'master', 'master') - p + create(:project, :repository).tap do |proj| + proj.repository.add_tag(user, 'master', 'master') + end end let(:command) do Gitlab::Ci::Pipeline::Chain::Command.new( From 855e7c32b9f3541fec085726d338802c8ca9b9f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 15 Nov 2018 22:54:21 +0100 Subject: [PATCH 06/33] Use Gitlab::Git::Ref in Project#resolve_ref Reworks Project#resolve_ref to return Gitlab::Git::Branch, Gitlab::Git::Tag or raise an AmbiguousRef error. --- app/models/project.rb | 11 +++--- app/models/repository.rb | 10 ++++++ lib/gitlab/git/branch.rb | 4 +++ lib/gitlab/git/ref.rb | 4 +++ lib/gitlab/git/tag.rb | 4 +++ spec/lib/gitlab/git/branch_spec.rb | 12 +++++++ spec/lib/gitlab/git/tag_spec.rb | 13 +++++++ spec/models/project_spec.rb | 55 +++++++++++++++++------------- spec/models/repository_spec.rb | 28 +++++++++++++++ 9 files changed, 113 insertions(+), 28 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 61840c972ee..6893f76dda9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1168,11 +1168,11 @@ class Project < ActiveRecord::Base if tag_exists && branch_exists raise AmbiguousRef elsif tag_exists - Gitlab::Git::TAG_REF_PREFIX + ref + repository.find_tag(ref) elsif branch_exists - Gitlab::Git::BRANCH_REF_PREFIX + ref + repository.find_branch(ref) else - ref + repository.find_ref(ref) end end @@ -1753,8 +1753,9 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - full_ref = resolve_ref(ref) - ref_name = Gitlab::Git.ref_name(full_ref) + resolved_ref = resolve_ref(ref) + full_ref = resolved_ref.full_ref + ref_name = resolved_ref.name if Gitlab::Git.branch_ref?(full_ref) ProtectedBranch.protected?(self, ref_name) diff --git a/app/models/repository.rb b/app/models/repository.rb index 35dd120856d..10635eb0cf4 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -182,6 +182,16 @@ class Repository tags.find { |tag| tag.name == name } end + def find_ref(ref) + if Gitlab::Git.tag_ref?(ref) + find_tag(Gitlab::Git.ref_name(ref)) + elsif Gitlab::Git.branch_ref?(ref) + find_branch(Gitlab::Git.ref_name(ref)) + else + nil + end + end + def add_branch(user, branch_name, ref) branch = raw_repository.add_branch(branch_name, user: user, target: ref) diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb index 9447cfa0fb6..d25eebd5f47 100644 --- a/lib/gitlab/git/branch.rb +++ b/lib/gitlab/git/branch.rb @@ -28,6 +28,10 @@ module Gitlab def state active? ? :active : :stale end + + def full_ref + Gitlab::Git::BRANCH_REF_PREFIX + name + end end end end diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb index eec91194949..b9c8b7c08e9 100644 --- a/lib/gitlab/git/ref.rb +++ b/lib/gitlab/git/ref.rb @@ -39,6 +39,10 @@ module Gitlab nil end end + + def full_ref + raise NotImplementedError + end end end end diff --git a/lib/gitlab/git/tag.rb b/lib/gitlab/git/tag.rb index 23d989ff258..ec89bc4f7e6 100644 --- a/lib/gitlab/git/tag.rb +++ b/lib/gitlab/git/tag.rb @@ -62,6 +62,10 @@ module Gitlab encode! @message end + def full_ref + Gitlab::Git::TAG_REF_PREFIX + name + end + private def message_from_gitaly_tag diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 0df282d0ae3..1b26b5ef7fd 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -124,6 +124,18 @@ describe Gitlab::Git::Branch, :seed_helper do it { expect(repository.branches.size).to eq(SeedRepo::Repo::BRANCHES.size) } + describe '#full_ref' do + subject do + described_class.new(repository, 'master', + repository.commit.sha, + repository.commit).full_ref + end + + it 'returns the full ref' do + is_expected.to eq('refs/heads/master') + end + end + def create_commit params[:message].delete!("\r") Rugged::Commit.create(rugged, params.merge(committer: committer.merge(time: Time.now))) diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index b51e3879f49..f5d0b6af6f0 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -69,4 +69,17 @@ describe Gitlab::Git::Tag, :seed_helper do end end end + + describe '#full_ref' do + subject do + described_class.new(repository, { name: 'master', + target: repository.commit.sha, + target_commit: repository.commit }) + .full_ref + end + + it 'returns the full ref' do + is_expected.to eq('refs/tags/master') + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a519435322c..e2e8a76ab72 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2563,7 +2563,7 @@ describe Project do end describe '#protected_for?' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } subject { project.protected_for?('ref') } @@ -2572,7 +2572,7 @@ describe Project do end context 'when the ref is not protected' do - let(:ref) { 'refs/heads/ref' } + let(:ref) { project.repository.find_branch('master') } before do stub_application_setting( @@ -2585,10 +2585,10 @@ describe Project do end context 'when the ref is a protected branch' do - let(:ref) { 'refs/heads/ref' } + let(:ref) { project.repository.find_branch('master') } before do - create(:protected_branch, name: 'ref', project: project) + create(:protected_branch, name: 'master', project: project) end it 'returns true' do @@ -2597,10 +2597,10 @@ describe Project do end context 'when the ref is a protected tag' do - let(:ref) { 'refs/tags/ref' } + let(:ref) { project.repository.find_tag('v1.0.0') } before do - create(:protected_tag, name: 'ref', project: project) + create(:protected_tag, name: 'v1.0.0', project: project) end it 'returns true' do @@ -2789,29 +2789,36 @@ describe Project do end describe '#resolve_ref' do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } subject { project.resolve_ref(ref) } context 'when ref is full ref' do - let(:ref) { 'refs/heads/master' } + context 'when ref exists' do + let(:ref) { 'refs/heads/master' } - it 'returns the unchanged ref' do - is_expected.to eq(ref) + it 'returns the ref' do + is_expected.to be_a(Gitlab::Git::Ref) + end + end + + context 'when ref does not exist' do + let(:ref) { 'refs/tags/doesnotexist' } + + it 'returns nil' do + is_expected.to eq(nil) + end end end context 'when ref is a tag or branch name' do let(:ref) { 'ref' } - before do - allow(project.repository).to receive(:tag_exists?).and_return(tag_exists) - allow(project.repository).to receive(:branch_exists?).and_return(branch_exists) - end - context 'when ref is ambiguous' do - let(:tag_exists) { true } - let(:branch_exists) { true } + before do + project.repository.add_tag(project.creator, ref, 'master') + project.repository.add_branch(project.creator, ref, 'master') + end it 'raises an error' do expect { subject }.to raise_error(described_class::AmbiguousRef) @@ -2819,20 +2826,22 @@ describe Project do end context 'when ref is tag name' do - let(:tag_exists) { true } - let(:branch_exists) { false } + before do + project.repository.add_tag(project.creator, ref, 'master') + end it 'returns full tag ref path' do - is_expected.to eq('refs/tags/ref') + is_expected.to be_a(Gitlab::Git::Tag) end end context 'when ref is branch name' do - let(:tag_exists) { false } - let(:branch_exists) { true } + before do + project.repository.add_branch(project.creator, ref, 'master') + end it 'returns full branch ref path' do - is_expected.to eq('refs/heads/ref') + is_expected.to be_a(Gitlab::Git::Branch) end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f09b4b67061..df0ab7bc0f4 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1699,6 +1699,34 @@ describe Repository do end end + describe '#find_ref' do + subject { repository.find_ref(ref) } + + context 'when ref is a tag' do + let(:ref) { 'refs/tags/v1.0.0' } + + it 'returns the tag' do + is_expected.to be_a(Gitlab::Git::Tag) + end + end + + context 'when ref is a branch' do + let(:ref) { 'refs/heads/master' } + + it 'returns the branch' do + is_expected.to be_a(Gitlab::Git::Branch) + end + end + + context 'when ref is invalid' do + let(:ref) { 'refs/notvalid/ref' } + + it 'returns nil' do + is_expected.to eq(nil) + end + end + end + describe '#avatar' do it 'returns nil if repo does not exist' do allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) From b0b5924eb418851ddfab848ab16b6acac27d42e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 15 Nov 2018 23:31:02 +0100 Subject: [PATCH 07/33] Use nil instead of raising AmbiguousRef --- app/models/project.rb | 5 +++-- lib/gitlab/ci/pipeline/chain/validate/repository.rb | 4 +--- spec/models/project_spec.rb | 12 ++++++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 6893f76dda9..e1acfbe7770 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -36,7 +36,6 @@ class Project < ActiveRecord::Base extend Gitlab::ConfigHelper BoardLimitExceeded = Class.new(StandardError) - AmbiguousRef = Class.new(StandardError) STATISTICS_ATTRIBUTE = 'repositories_count'.freeze NUMBER_OF_PERMITTED_BOARDS = 1 @@ -1166,7 +1165,7 @@ class Project < ActiveRecord::Base branch_exists = repository.branch_exists?(ref) if tag_exists && branch_exists - raise AmbiguousRef + nil elsif tag_exists repository.find_tag(ref) elsif branch_exists @@ -1754,6 +1753,8 @@ class Project < ActiveRecord::Base def protected_for?(ref) resolved_ref = resolve_ref(ref) + return false unless resolved_ref + full_ref = resolved_ref.full_ref ref_name = resolved_ref.name diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 3cec55cdb71..0b411422264 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -17,9 +17,7 @@ module Gitlab return error('Commit not found') end - begin - @command.project.resolve_ref(@command.origin_ref) - rescue Project::AmbiguousRef + unless @command.project.resolve_ref(@command.origin_ref) return error('Ref is ambiguous') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e2e8a76ab72..48cf693b678 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2571,6 +2571,14 @@ describe Project do allow(project).to receive(:resolve_ref).and_return(ref) end + context 'when ref is ambiguous' do + let(:ref) { nil } + + it 'returns false' do + is_expected.to be_falsey + end + end + context 'when the ref is not protected' do let(:ref) { project.repository.find_branch('master') } @@ -2820,8 +2828,8 @@ describe Project do project.repository.add_branch(project.creator, ref, 'master') end - it 'raises an error' do - expect { subject }.to raise_error(described_class::AmbiguousRef) + it 'returns nil' do + is_expected.to eq(nil) end end From b6c8d3ac9f3031f6174dde2f11b3876ab4ac8a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 15 Nov 2018 23:38:16 +0100 Subject: [PATCH 08/33] Reintroduce Command#protected_ref? --- lib/gitlab/ci/pipeline/chain/build.rb | 1 + lib/gitlab/ci/pipeline/chain/command.rb | 6 +++++ lib/gitlab/ci/pipeline/chain/populate.rb | 5 ----- .../ci/pipeline/chain/validate/abilities.rb | 2 +- lib/gitlab/git.rb | 4 ++-- .../gitlab/ci/pipeline/chain/command_spec.rb | 22 +++++++++++++++++++ .../lib/gitlab/ci/pipeline/seed/build_spec.rb | 3 ++- .../lib/gitlab/ci/pipeline/seed/stage_spec.rb | 3 ++- 8 files changed, 36 insertions(+), 10 deletions(-) diff --git a/lib/gitlab/ci/pipeline/chain/build.rb b/lib/gitlab/ci/pipeline/chain/build.rb index 41632211374..d33d1edfe35 100644 --- a/lib/gitlab/ci/pipeline/chain/build.rb +++ b/lib/gitlab/ci/pipeline/chain/build.rb @@ -17,6 +17,7 @@ module Gitlab user: @command.current_user, pipeline_schedule: @command.schedule, merge_request: @command.merge_request, + protected: @command.protected_ref?, variables_attributes: Array(@command.variables_attributes) ) diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index ee5022e47c4..316c283d90b 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -51,6 +51,12 @@ module Gitlab def before_sha self[:before_sha] || checkout_sha || Gitlab::Git::BLANK_SHA end + + def protected_ref? + strong_memoize(:protected_ref) do + project.protected_for?(origin_ref) + end + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb index 45b4393ecf3..633d3cd4f6b 100644 --- a/lib/gitlab/ci/pipeline/chain/populate.rb +++ b/lib/gitlab/ci/pipeline/chain/populate.rb @@ -18,11 +18,6 @@ module Gitlab # @command.seeds_block&.call(pipeline) - ## - # Populate pipeline protected status - # - pipeline.protected = @command.project.protected_for?(@command.origin_ref) - ## # Populate pipeline with all stages, and stages with builds. # diff --git a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb index e4979102fd9..ebd7e6e8289 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/abilities.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/abilities.rb @@ -31,7 +31,7 @@ module Gitlab if current_user allowed_to_create? else # legacy triggers don't have a corresponding user - !@command.project.protected_for?(@command.origin_ref) + !@command.protected_ref? end end diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index c4aac228b2f..2d950bb151c 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref.start_with?(TAG_REF_PREFIX) + ref =~ %r{#{TAG_REF_PREFIX}\w+} end def branch_ref?(ref) - ref.start_with?(BRANCH_REF_PREFIX) + ref =~ %r{#{BRANCH_REF_PREFIX}\w+} end def blank_ref?(ref) diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 14b4fbd1f5a..75a177d2d1f 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -160,4 +160,26 @@ describe Gitlab::Ci::Pipeline::Chain::Command do end end end + + describe '#protected_ref?' do + let(:command) { described_class.new(project: project, origin_ref: 'my-branch') } + + subject { command.protected_ref? } + + context 'when a ref is protected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(true) + end + + it { is_expected.to eq(true) } + end + + context 'when a ref is unprotected' do + before do + expect_any_instance_of(Project).to receive(:protected_for?).with('my-branch').and_return(false) + end + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index fffa727c2ed..2cf812b26dc 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Seed::Build do - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:attributes) do { name: 'rspec', diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index 05ce3412fd8..82f741845db 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Seed::Stage do - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:project, :repository) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:attributes) do { name: 'test', From 2b4883e05e59eff08088e378bf3061d9d8da13dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 16 Nov 2018 00:27:11 +0100 Subject: [PATCH 09/33] Check for Tag/Branch corectness --- spec/models/project_spec.rb | 14 +++++++++++--- spec/models/repository_spec.rb | 12 ++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 48cf693b678..204d611796b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2805,7 +2805,7 @@ describe Project do context 'when ref exists' do let(:ref) { 'refs/heads/master' } - it 'returns the ref' do + it 'returns a ref' do is_expected.to be_a(Gitlab::Git::Ref) end end @@ -2838,9 +2838,13 @@ describe Project do project.repository.add_tag(project.creator, ref, 'master') end - it 'returns full tag ref path' do + it 'returns a tag' do is_expected.to be_a(Gitlab::Git::Tag) end + + it 'returns the correct tag' do + expect(subject.name).to eq(ref) + end end context 'when ref is branch name' do @@ -2848,9 +2852,13 @@ describe Project do project.repository.add_branch(project.creator, ref, 'master') end - it 'returns full branch ref path' do + it 'returns a branch' do is_expected.to be_a(Gitlab::Git::Branch) end + + it 'returns the correct branch' do + expect(subject.name).to eq(ref) + end end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index df0ab7bc0f4..6f9fdcf4482 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1705,17 +1705,25 @@ describe Repository do context 'when ref is a tag' do let(:ref) { 'refs/tags/v1.0.0' } - it 'returns the tag' do + it 'returns a tag' do is_expected.to be_a(Gitlab::Git::Tag) end + + it 'returns the correct tag' do + expect(subject.name).to eq('v1.0.0') + end end context 'when ref is a branch' do let(:ref) { 'refs/heads/master' } - it 'returns the branch' do + it 'returns a branch' do is_expected.to be_a(Gitlab::Git::Branch) end + + it 'returns the correct branch' do + expect(subject.name).to eq('master') + end end context 'when ref is invalid' do From 38348aa1215daa2393b7d488c1ca8926d67dc09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 16 Nov 2018 13:20:52 +0100 Subject: [PATCH 10/33] Remove Gitlab::Git::Ref#full_ref --- app/models/project.rb | 12 +++++------- lib/gitlab/git.rb | 4 ++-- lib/gitlab/git/branch.rb | 4 ---- lib/gitlab/git/ref.rb | 4 ---- lib/gitlab/git/tag.rb | 4 ---- spec/lib/gitlab/git/branch_spec.rb | 12 ------------ spec/lib/gitlab/git/tag_spec.rb | 13 ------------- 7 files changed, 7 insertions(+), 46 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index e1acfbe7770..22ce916a36c 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1755,13 +1755,11 @@ class Project < ActiveRecord::Base resolved_ref = resolve_ref(ref) return false unless resolved_ref - full_ref = resolved_ref.full_ref - ref_name = resolved_ref.name - - if Gitlab::Git.branch_ref?(full_ref) - ProtectedBranch.protected?(self, ref_name) - elsif Gitlab::Git.tag_ref?(full_ref) - ProtectedTag.protected?(self, ref_name) + case resolved_ref + when Gitlab::Git::Branch + ProtectedBranch.protected?(self, resolved_ref.name) + when Gitlab::Git::Tag + ProtectedTag.protected?(self, resolved_ref.name) end end diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 2d950bb151c..f22ac800479 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref =~ %r{#{TAG_REF_PREFIX}\w+} + ref =~ /#{TAG_REF_PREFIX}\w+/ end def branch_ref?(ref) - ref =~ %r{#{BRANCH_REF_PREFIX}\w+} + ref =~ /#{BRANCH_REF_PREFIX}\w+/ end def blank_ref?(ref) diff --git a/lib/gitlab/git/branch.rb b/lib/gitlab/git/branch.rb index d25eebd5f47..9447cfa0fb6 100644 --- a/lib/gitlab/git/branch.rb +++ b/lib/gitlab/git/branch.rb @@ -28,10 +28,6 @@ module Gitlab def state active? ? :active : :stale end - - def full_ref - Gitlab::Git::BRANCH_REF_PREFIX + name - end end end end diff --git a/lib/gitlab/git/ref.rb b/lib/gitlab/git/ref.rb index b9c8b7c08e9..eec91194949 100644 --- a/lib/gitlab/git/ref.rb +++ b/lib/gitlab/git/ref.rb @@ -39,10 +39,6 @@ module Gitlab nil end end - - def full_ref - raise NotImplementedError - end end end end diff --git a/lib/gitlab/git/tag.rb b/lib/gitlab/git/tag.rb index ec89bc4f7e6..23d989ff258 100644 --- a/lib/gitlab/git/tag.rb +++ b/lib/gitlab/git/tag.rb @@ -62,10 +62,6 @@ module Gitlab encode! @message end - def full_ref - Gitlab::Git::TAG_REF_PREFIX + name - end - private def message_from_gitaly_tag diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 1b26b5ef7fd..0df282d0ae3 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -124,18 +124,6 @@ describe Gitlab::Git::Branch, :seed_helper do it { expect(repository.branches.size).to eq(SeedRepo::Repo::BRANCHES.size) } - describe '#full_ref' do - subject do - described_class.new(repository, 'master', - repository.commit.sha, - repository.commit).full_ref - end - - it 'returns the full ref' do - is_expected.to eq('refs/heads/master') - end - end - def create_commit params[:message].delete!("\r") Rugged::Commit.create(rugged, params.merge(committer: committer.merge(time: Time.now))) diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index f5d0b6af6f0..b51e3879f49 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -69,17 +69,4 @@ describe Gitlab::Git::Tag, :seed_helper do end end end - - describe '#full_ref' do - subject do - described_class.new(repository, { name: 'master', - target: repository.commit.sha, - target_commit: repository.commit }) - .full_ref - end - - it 'returns the full ref' do - is_expected.to eq('refs/tags/master') - end - end end From 35b3392ef7511521653a984a9d9c6dbaf4e6dccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 16 Nov 2018 13:51:26 +0100 Subject: [PATCH 11/33] Allow any character in tag / branch ref regex --- lib/gitlab/git.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index f22ac800479..81ec0e9f2db 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref =~ /#{TAG_REF_PREFIX}\w+/ + ref =~ /#{TAG_REF_PREFIX}.+/ end def branch_ref?(ref) - ref =~ /#{BRANCH_REF_PREFIX}\w+/ + ref =~ /#{BRANCH_REF_PREFIX}.+/ end def blank_ref?(ref) From 96a0a8cfb62d9ea66ee2a22e09a116d64f333703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 16 Nov 2018 21:03:21 +0100 Subject: [PATCH 12/33] Use strings instead of Gitlab::Git::Ref --- app/models/project.rb | 17 +++++++++-------- spec/models/project_spec.rb | 38 ++++++++++--------------------------- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 22ce916a36c..e9fa9cfabc9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1167,11 +1167,11 @@ class Project < ActiveRecord::Base if tag_exists && branch_exists nil elsif tag_exists - repository.find_tag(ref) + Gitlab::Git::TAG_REF_PREFIX + ref elsif branch_exists - repository.find_branch(ref) + Gitlab::Git::BRANCH_REF_PREFIX + ref else - repository.find_ref(ref) + ref end end @@ -1755,11 +1755,12 @@ class Project < ActiveRecord::Base resolved_ref = resolve_ref(ref) return false unless resolved_ref - case resolved_ref - when Gitlab::Git::Branch - ProtectedBranch.protected?(self, resolved_ref.name) - when Gitlab::Git::Tag - ProtectedTag.protected?(self, resolved_ref.name) + ref_name = Gitlab::Git.ref_name(resolved_ref) + + if Gitlab::Git.branch_ref?(resolved_ref) + ProtectedBranch.protected?(self, ref_name) + elsif Gitlab::Git.tag_ref?(resolved_ref) + ProtectedTag.protected?(self, ref_name) end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 204d611796b..cbc242308e8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2580,7 +2580,7 @@ describe Project do end context 'when the ref is not protected' do - let(:ref) { project.repository.find_branch('master') } + let(:ref) { 'refs/heads/master' } before do stub_application_setting( @@ -2593,7 +2593,7 @@ describe Project do end context 'when the ref is a protected branch' do - let(:ref) { project.repository.find_branch('master') } + let(:ref) { 'refs/heads/master' } before do create(:protected_branch, name: 'master', project: project) @@ -2605,7 +2605,7 @@ describe Project do end context 'when the ref is a protected tag' do - let(:ref) { project.repository.find_tag('v1.0.0') } + let(:ref) { 'refs/tags/v1.0.0' } before do create(:protected_tag, name: 'v1.0.0', project: project) @@ -2802,20 +2802,10 @@ describe Project do subject { project.resolve_ref(ref) } context 'when ref is full ref' do - context 'when ref exists' do - let(:ref) { 'refs/heads/master' } + let(:ref) { 'refs/heads/master' } - it 'returns a ref' do - is_expected.to be_a(Gitlab::Git::Ref) - end - end - - context 'when ref does not exist' do - let(:ref) { 'refs/tags/doesnotexist' } - - it 'returns nil' do - is_expected.to eq(nil) - end + it 'returns the ref' do + is_expected.to eq(ref) end end @@ -2838,12 +2828,8 @@ describe Project do project.repository.add_tag(project.creator, ref, 'master') end - it 'returns a tag' do - is_expected.to be_a(Gitlab::Git::Tag) - end - - it 'returns the correct tag' do - expect(subject.name).to eq(ref) + it 'returns the tag ref' do + is_expected.to eq("refs/tags/#{ref}") end end @@ -2852,12 +2838,8 @@ describe Project do project.repository.add_branch(project.creator, ref, 'master') end - it 'returns a branch' do - is_expected.to be_a(Gitlab::Git::Branch) - end - - it 'returns the correct branch' do - expect(subject.name).to eq(ref) + it 'returns the branch ref' do + is_expected.to eq("refs/heads/#{ref}") end end end From e0b5306cb79c7a535f96c0d2d864278b2193d641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 16 Nov 2018 21:06:58 +0100 Subject: [PATCH 13/33] Remove Repository#find_ref --- app/models/repository.rb | 10 ---------- spec/models/repository_spec.rb | 36 ---------------------------------- 2 files changed, 46 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 10635eb0cf4..35dd120856d 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -182,16 +182,6 @@ class Repository tags.find { |tag| tag.name == name } end - def find_ref(ref) - if Gitlab::Git.tag_ref?(ref) - find_tag(Gitlab::Git.ref_name(ref)) - elsif Gitlab::Git.branch_ref?(ref) - find_branch(Gitlab::Git.ref_name(ref)) - else - nil - end - end - def add_branch(user, branch_name, ref) branch = raw_repository.add_branch(branch_name, user: user, target: ref) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 6f9fdcf4482..f09b4b67061 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1699,42 +1699,6 @@ describe Repository do end end - describe '#find_ref' do - subject { repository.find_ref(ref) } - - context 'when ref is a tag' do - let(:ref) { 'refs/tags/v1.0.0' } - - it 'returns a tag' do - is_expected.to be_a(Gitlab::Git::Tag) - end - - it 'returns the correct tag' do - expect(subject.name).to eq('v1.0.0') - end - end - - context 'when ref is a branch' do - let(:ref) { 'refs/heads/master' } - - it 'returns a branch' do - is_expected.to be_a(Gitlab::Git::Branch) - end - - it 'returns the correct branch' do - expect(subject.name).to eq('master') - end - end - - context 'when ref is invalid' do - let(:ref) { 'refs/notvalid/ref' } - - it 'returns nil' do - is_expected.to eq(nil) - end - end - end - describe '#avatar' do it 'returns nil if repo does not exist' do allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) From d12bc3bfc47f9a4252e3f9eb60b1326eade7d3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 20 Nov 2018 14:56:43 +0100 Subject: [PATCH 14/33] Search for tag / branch ref from beginning --- lib/gitlab/git.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 81ec0e9f2db..44a62586a23 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref =~ /#{TAG_REF_PREFIX}.+/ + ref =~ /^#{TAG_REF_PREFIX}.+/ end def branch_ref?(ref) - ref =~ /#{BRANCH_REF_PREFIX}.+/ + ref =~ /^#{BRANCH_REF_PREFIX}.+/ end def blank_ref?(ref) From 24d682254a0ae68dfc605fc077c6a7610b97994a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 20 Nov 2018 15:07:25 +0100 Subject: [PATCH 15/33] Move Project#resolve_ref to Repository --- app/models/project.rb | 17 +------ app/models/repository.rb | 15 ++++++ .../ci/pipeline/chain/validate/repository.rb | 2 +- spec/models/project_spec.rb | 51 +------------------ spec/models/repository_spec.rb | 47 +++++++++++++++++ 5 files changed, 65 insertions(+), 67 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index e9fa9cfabc9..2ba273c6f98 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1160,21 +1160,6 @@ class Project < ActiveRecord::Base end end - def resolve_ref(ref) - tag_exists = repository.tag_exists?(ref) - branch_exists = repository.branch_exists?(ref) - - if tag_exists && branch_exists - nil - elsif tag_exists - Gitlab::Git::TAG_REF_PREFIX + ref - elsif branch_exists - Gitlab::Git::BRANCH_REF_PREFIX + ref - else - ref - end - end - def root_ref?(branch) repository.root_ref == branch end @@ -1752,7 +1737,7 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - resolved_ref = resolve_ref(ref) + resolved_ref = repository.resolve_ref(ref) return false unless resolved_ref ref_name = Gitlab::Git.ref_name(resolved_ref) diff --git a/app/models/repository.rb b/app/models/repository.rb index 35dd120856d..221d01c5b44 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -182,6 +182,21 @@ class Repository tags.find { |tag| tag.name == name } end + def resolve_ref(ref) + tag_exists = tag_exists?(ref) + branch_exists = branch_exists?(ref) + + if tag_exists && branch_exists + nil + elsif tag_exists + Gitlab::Git::TAG_REF_PREFIX + ref + elsif branch_exists + Gitlab::Git::BRANCH_REF_PREFIX + ref + else + ref + end + end + def add_branch(user, branch_name, ref) branch = raw_repository.add_branch(branch_name, user: user, target: ref) diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 0b411422264..6e95c0717c6 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -17,7 +17,7 @@ module Gitlab return error('Commit not found') end - unless @command.project.resolve_ref(@command.origin_ref) + unless @command.project.repository.resolve_ref(@command.origin_ref) return error('Ref is ambiguous') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index cbc242308e8..25560ddea8d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2568,7 +2568,7 @@ describe Project do subject { project.protected_for?('ref') } before do - allow(project).to receive(:resolve_ref).and_return(ref) + allow(project.repository).to receive(:resolve_ref).and_return(ref) end context 'when ref is ambiguous' do @@ -2796,55 +2796,6 @@ describe Project do end end - describe '#resolve_ref' do - let(:project) { create(:project, :repository) } - - subject { project.resolve_ref(ref) } - - context 'when ref is full ref' do - let(:ref) { 'refs/heads/master' } - - it 'returns the ref' do - is_expected.to eq(ref) - end - end - - context 'when ref is a tag or branch name' do - let(:ref) { 'ref' } - - context 'when ref is ambiguous' do - before do - project.repository.add_tag(project.creator, ref, 'master') - project.repository.add_branch(project.creator, ref, 'master') - end - - it 'returns nil' do - is_expected.to eq(nil) - end - end - - context 'when ref is tag name' do - before do - project.repository.add_tag(project.creator, ref, 'master') - end - - it 'returns the tag ref' do - is_expected.to eq("refs/tags/#{ref}") - end - end - - context 'when ref is branch name' do - before do - project.repository.add_branch(project.creator, ref, 'master') - end - - it 'returns the branch ref' do - is_expected.to eq("refs/heads/#{ref}") - end - end - end - end - describe '#http_url_to_repo' do let(:project) { create(:project) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f09b4b67061..3d6cf2cbc19 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1005,6 +1005,53 @@ describe Repository do end end + describe '#resolve_ref' do + subject { repository.resolve_ref(ref) } + + context 'when ref is full ref' do + let(:ref) { 'refs/heads/master' } + + it 'returns the ref' do + is_expected.to eq(ref) + end + end + + context 'when ref is a tag or branch name' do + let(:ref) { 'ref' } + + context 'when ref is ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + repository.add_branch(project.creator, ref, 'master') + end + + it 'returns nil' do + is_expected.to eq(nil) + end + end + + context 'when ref is tag name' do + before do + repository.add_tag(project.creator, ref, 'master') + end + + it 'returns the tag ref' do + is_expected.to eq("refs/tags/#{ref}") + end + end + + context 'when ref is branch name' do + before do + repository.add_branch(project.creator, ref, 'master') + end + + it 'returns the branch ref' do + is_expected.to eq("refs/heads/#{ref}") + end + end + end + end + describe '#add_branch' do let(:branch_name) { 'new_feature' } let(:target) { 'master' } From d307dccbc60f088496fb727f40530ba3003b61fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 22 Nov 2018 14:35:06 +0100 Subject: [PATCH 16/33] Use to_s.start_with? in tag/branch ref method --- lib/gitlab/git.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 44a62586a23..7c590846f74 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref =~ /^#{TAG_REF_PREFIX}.+/ + ref.to_s.start_with?(TAG_REF_PREFIX) end def branch_ref?(ref) - ref =~ /^#{BRANCH_REF_PREFIX}.+/ + ref.to_s.start_with?(BRANCH_REF_PREFIX) end def blank_ref?(ref) From 350ea9c55ab6b980ac5ec74d2a34b9cbac915e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 23 Nov 2018 00:11:42 +0100 Subject: [PATCH 17/33] Implement Repository#ambiguous_ref? This implements Repository#ambiguous_ref? and checks if a ref is ambiguous before trying to resolve the ref in Project#protected_for? --- app/models/project.rb | 6 +- app/models/repository.rb | 13 ++-- .../ci/pipeline/chain/validate/repository.rb | 2 +- spec/models/project_spec.rb | 27 +++++--- spec/models/repository_spec.rb | 68 +++++++++++-------- 5 files changed, 69 insertions(+), 47 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 2ba273c6f98..800c8afb49f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1737,10 +1737,10 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - resolved_ref = repository.resolve_ref(ref) - return false unless resolved_ref + return false unless ref && !repository.ambiguous_ref?(ref) - ref_name = Gitlab::Git.ref_name(resolved_ref) + resolved_ref = repository.resolve_ref(ref) + ref_name = resolved_ref == ref ? Gitlab::Git.ref_name(resolved_ref) : ref if Gitlab::Git.branch_ref?(resolved_ref) ProtectedBranch.protected?(self, ref_name) diff --git a/app/models/repository.rb b/app/models/repository.rb index 221d01c5b44..9e01f7f0df5 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -182,15 +182,14 @@ class Repository tags.find { |tag| tag.name == name } end - def resolve_ref(ref) - tag_exists = tag_exists?(ref) - branch_exists = branch_exists?(ref) + def ambiguous_ref?(ref) + tag_exists?(ref) && branch_exists?(ref) + end - if tag_exists && branch_exists - nil - elsif tag_exists + def resolve_ref(ref) + if tag_exists?(ref) Gitlab::Git::TAG_REF_PREFIX + ref - elsif branch_exists + elsif branch_exists?(ref) Gitlab::Git::BRANCH_REF_PREFIX + ref else ref diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 6e95c0717c6..09b3187569c 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -17,7 +17,7 @@ module Gitlab return error('Commit not found') end - unless @command.project.repository.resolve_ref(@command.origin_ref) + if @command.project.repository.ambiguous_ref?(@command.origin_ref) return error('Ref is ambiguous') end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 25560ddea8d..18ad69a1bda 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2565,13 +2565,9 @@ describe Project do describe '#protected_for?' do let(:project) { create(:project, :repository) } - subject { project.protected_for?('ref') } + subject { project.protected_for?(ref) } - before do - allow(project.repository).to receive(:resolve_ref).and_return(ref) - end - - context 'when ref is ambiguous' do + context 'when ref is nil' do let(:ref) { nil } it 'returns false' do @@ -2579,8 +2575,21 @@ describe Project do end end + context 'when ref is ambiguous' do + let(:ref) { 'ref' } + + before do + project.repository.add_branch(project.creator, ref, 'master') + project.repository.add_tag(project.creator, ref, 'master') + end + + it 'returns false' do + is_expected.to be_falsey + end + end + context 'when the ref is not protected' do - let(:ref) { 'refs/heads/master' } + let(:ref) { 'master' } before do stub_application_setting( @@ -2593,7 +2602,7 @@ describe Project do end context 'when the ref is a protected branch' do - let(:ref) { 'refs/heads/master' } + let(:ref) { 'master' } before do create(:protected_branch, name: 'master', project: project) @@ -2605,7 +2614,7 @@ describe Project do end context 'when the ref is a protected tag' do - let(:ref) { 'refs/tags/v1.0.0' } + let(:ref) { 'v1.0.0' } before do create(:protected_tag, name: 'v1.0.0', project: project) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 3d6cf2cbc19..cdececd49e0 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1005,7 +1005,36 @@ describe Repository do end end + describe '#ambiguous_ref?' do + let(:ref) { 'ref' } + + subject { repository.ambiguous_ref?(ref) } + + context 'when ref is ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + repository.add_branch(project.creator, ref, 'master') + end + + it 'should be true' do + is_expected.to eq(true) + end + end + + context 'when ref is not ambiguous' do + before do + repository.add_tag(project.creator, ref, 'master') + end + + it 'should be false' do + is_expected.to eq(false) + end + end + end + describe '#resolve_ref' do + let(:ref) { 'ref' } + subject { repository.resolve_ref(ref) } context 'when ref is full ref' do @@ -1016,38 +1045,23 @@ describe Repository do end end - context 'when ref is a tag or branch name' do - let(:ref) { 'ref' } - - context 'when ref is ambiguous' do - before do - repository.add_tag(project.creator, ref, 'master') - repository.add_branch(project.creator, ref, 'master') - end - - it 'returns nil' do - is_expected.to eq(nil) - end + context 'when ref is tag name' do + before do + repository.add_tag(project.creator, ref, 'master') end - context 'when ref is tag name' do - before do - repository.add_tag(project.creator, ref, 'master') - end + it 'returns the tag ref' do + is_expected.to eq("refs/tags/#{ref}") + end + end - it 'returns the tag ref' do - is_expected.to eq("refs/tags/#{ref}") - end + context 'when ref is branch name' do + before do + repository.add_branch(project.creator, ref, 'master') end - context 'when ref is branch name' do - before do - repository.add_branch(project.creator, ref, 'master') - end - - it 'returns the branch ref' do - is_expected.to eq("refs/heads/#{ref}") - end + it 'returns the branch ref' do + is_expected.to eq("refs/heads/#{ref}") end end end From d27de096c2d889be69b8e5ac82284bee8034d158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 23 Nov 2018 00:21:29 +0100 Subject: [PATCH 18/33] Revert "Use to_s.start_with? in tag/branch ref method" This reverts commit ec4730478b798270781257913ee4cede673d4d4e. --- lib/gitlab/git.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 7c590846f74..44a62586a23 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -54,11 +54,11 @@ module Gitlab end def tag_ref?(ref) - ref.to_s.start_with?(TAG_REF_PREFIX) + ref =~ /^#{TAG_REF_PREFIX}.+/ end def branch_ref?(ref) - ref.to_s.start_with?(BRANCH_REF_PREFIX) + ref =~ /^#{BRANCH_REF_PREFIX}.+/ end def blank_ref?(ref) From 9f6d228d2368911befc11038d64ccb9ccae131ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 26 Nov 2018 15:04:51 +0100 Subject: [PATCH 19/33] Simplify conditionals in Project#protected_ref? --- app/models/project.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 800c8afb49f..fa1820b2bdb 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1737,10 +1737,14 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - return false unless ref && !repository.ambiguous_ref?(ref) + return false if ref.nil? || repository.ambiguous_ref?(ref) resolved_ref = repository.resolve_ref(ref) - ref_name = resolved_ref == ref ? Gitlab::Git.ref_name(resolved_ref) : ref + ref_name = if resolved_ref == ref + Gitlab::Git.ref_name(resolved_ref) + else + ref + end if Gitlab::Git.branch_ref?(resolved_ref) ProtectedBranch.protected?(self, ref_name) From a1be58097943f7f568de6a90aa12cf7452ac2f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 26 Nov 2018 15:43:46 +0100 Subject: [PATCH 20/33] Implement Command#ambiguous_ref? --- lib/gitlab/ci/pipeline/chain/command.rb | 6 ++++++ .../ci/pipeline/chain/validate/repository.rb | 2 +- .../gitlab/ci/pipeline/chain/command_spec.rb | 20 +++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 316c283d90b..90208352c55 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -57,6 +57,12 @@ module Gitlab project.protected_for?(origin_ref) end end + + def ambiguous_ref? + strong_memoize(:ambiguous_ref) do + project.repository.ambiguous_ref?(origin_ref) + end + end end end end diff --git a/lib/gitlab/ci/pipeline/chain/validate/repository.rb b/lib/gitlab/ci/pipeline/chain/validate/repository.rb index 09b3187569c..9c6c2bc8e25 100644 --- a/lib/gitlab/ci/pipeline/chain/validate/repository.rb +++ b/lib/gitlab/ci/pipeline/chain/validate/repository.rb @@ -17,7 +17,7 @@ module Gitlab return error('Commit not found') end - if @command.project.repository.ambiguous_ref?(@command.origin_ref) + if @command.ambiguous_ref? return error('Ref is ambiguous') end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index 75a177d2d1f..6aa802ce6fd 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -182,4 +182,24 @@ describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected.to eq(false) } end end + + describe '#ambiguous_ref' do + let(:project) { create(:project, :repository) } + let(:command) { described_class.new(project: project, origin_ref: 'ref') } + + subject { command.ambiguous_ref? } + + context 'when ref is not ambiguous' do + it { is_expected. to eq(false) } + end + + context 'when ref is ambiguous' do + before do + project.repository.add_tag(project.creator, 'ref', 'master') + project.repository.add_branch(project.creator, 'ref', 'master') + end + + it { is_expected. to eq(true) } + end + end end From 63da51900c205981fc12cda25778265d78b607e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Tue, 27 Nov 2018 14:45:25 +0100 Subject: [PATCH 21/33] Make full ref in Repository#resolve_ref explicit --- app/models/repository.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 9e01f7f0df5..3db49a5fce0 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -191,7 +191,7 @@ class Repository Gitlab::Git::TAG_REF_PREFIX + ref elsif branch_exists?(ref) Gitlab::Git::BRANCH_REF_PREFIX + ref - else + elsif Gitlab::Git.tag_ref?(ref) || Gitlab::Git.branch_ref?(ref) ref end end From dfe7f57eef0c5c2319c5c9898ba0721b6a1c9913 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Nov 2018 15:18:14 +0100 Subject: [PATCH 22/33] Rename Repository#resolve_ref to expand_ref --- app/models/project.rb | 13 +++++++------ app/models/repository.rb | 4 +--- spec/models/repository_spec.rb | 10 +++++----- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index fa1820b2bdb..a8bef70f505 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1739,12 +1739,13 @@ class Project < ActiveRecord::Base def protected_for?(ref) return false if ref.nil? || repository.ambiguous_ref?(ref) - resolved_ref = repository.resolve_ref(ref) - ref_name = if resolved_ref == ref - Gitlab::Git.ref_name(resolved_ref) - else - ref - end + if Gitlab::Git.branch_ref?(ref) || Gitlab::Git.tag_ref?(ref) + resolved_ref = ref + ref_name = Gitlab::Git.ref_name(ref) + else + resolved_ref = repository.expand_ref(ref) + ref_name = ref + end if Gitlab::Git.branch_ref?(resolved_ref) ProtectedBranch.protected?(self, ref_name) diff --git a/app/models/repository.rb b/app/models/repository.rb index 3db49a5fce0..7352386d9d5 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -186,13 +186,11 @@ class Repository tag_exists?(ref) && branch_exists?(ref) end - def resolve_ref(ref) + def expand_ref(ref) if tag_exists?(ref) Gitlab::Git::TAG_REF_PREFIX + ref elsif branch_exists?(ref) Gitlab::Git::BRANCH_REF_PREFIX + ref - elsif Gitlab::Git.tag_ref?(ref) || Gitlab::Git.branch_ref?(ref) - ref end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index cdececd49e0..2063b4bbe75 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -1032,16 +1032,16 @@ describe Repository do end end - describe '#resolve_ref' do + describe '#expand_ref' do let(:ref) { 'ref' } - subject { repository.resolve_ref(ref) } + subject { repository.expand_ref(ref) } - context 'when ref is full ref' do + context 'when ref is not tag or branch name' do let(:ref) { 'refs/heads/master' } - it 'returns the ref' do - is_expected.to eq(ref) + it 'returns nil' do + is_expected.to eq(nil) end end From 08942de9b6a3ad361cbae8a83a8e8b7c7e4768ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 28 Nov 2018 15:43:58 +0100 Subject: [PATCH 23/33] Raise an error on ambiguous refs --- app/models/project.rb | 3 ++- app/models/repository.rb | 1 + spec/models/project_spec.rb | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index a8bef70f505..9a9ef5c2fa9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1737,7 +1737,8 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - return false if ref.nil? || repository.ambiguous_ref?(ref) + return false if ref.nil? + raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref) if Gitlab::Git.branch_ref?(ref) || Gitlab::Git.tag_ref?(ref) resolved_ref = ref diff --git a/app/models/repository.rb b/app/models/repository.rb index 7352386d9d5..c685752c294 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -26,6 +26,7 @@ class Repository delegate :bundle_to_disk, to: :raw_repository CreateTreeError = Class.new(StandardError) + AmbiguousRefError = Class.new(StandardError) # Methods that cache data from the Git repository. # diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 18ad69a1bda..a106642c3e5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2583,8 +2583,8 @@ describe Project do project.repository.add_tag(project.creator, ref, 'master') end - it 'returns false' do - is_expected.to be_falsey + it 'raises an error' do + expect { subject }.to raise_error(Repository::AmbiguousRefError) end end From 0f00c7818bf09e800c4ac6652077fffe7976ed6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 30 Nov 2018 11:34:59 +0100 Subject: [PATCH 24/33] Remove resolving conditional from protected_for --- app/models/project.rb | 9 ++------- spec/models/project_spec.rb | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index 9a9ef5c2fa9..f71bf65f417 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1740,13 +1740,8 @@ class Project < ActiveRecord::Base return false if ref.nil? raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref) - if Gitlab::Git.branch_ref?(ref) || Gitlab::Git.tag_ref?(ref) - resolved_ref = ref - ref_name = Gitlab::Git.ref_name(ref) - else - resolved_ref = repository.expand_ref(ref) - ref_name = ref - end + resolved_ref = repository.expand_ref(ref) + ref_name = Gitlab::Git.ref_name(resolved_ref) if Gitlab::Git.branch_ref?(resolved_ref) ProtectedBranch.protected?(self, ref_name) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index a106642c3e5..75f1f779bb0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2624,6 +2624,28 @@ describe Project do is_expected.to be_truthy end end + + context 'when ref name is a full branch ref' do + let(:ref) { 'refs/tags/something' } + + before do + project.repository.add_branch(project.creator, ref, 'master') + end + + it 'returns false' do + is_expected.to be_falsey + end + + context 'when ref is a protected branch' do + before do + create(:protected_branch, name: 'refs/tags/something', project: project) + end + + it 'returns true' do + is_expected.to be_truthy + end + end + end end describe '#update_project_statistics' do From 93d3c61eca258369bda0c28c5519cb14e4ff1457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 30 Nov 2018 12:18:53 +0100 Subject: [PATCH 25/33] Add specs when full ref is passed to protected_for --- app/models/project.rb | 2 +- spec/models/project_spec.rb | 22 +++++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/models/project.rb b/app/models/project.rb index f71bf65f417..0948e4625a8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1740,7 +1740,7 @@ class Project < ActiveRecord::Base return false if ref.nil? raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref) - resolved_ref = repository.expand_ref(ref) + resolved_ref = repository.expand_ref(ref) || ref ref_name = Gitlab::Git.ref_name(resolved_ref) if Gitlab::Git.branch_ref?(resolved_ref) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 75f1f779bb0..33bce0c0823 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2625,7 +2625,27 @@ describe Project do end end - context 'when ref name is a full branch ref' do + context 'when ref is a full ref' do + let(:ref) { 'refs/heads/master' } + + context 'when ref is not protected' do + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when ref is protected' do + before do + create(:protected_branch, name: 'master', project: project) + end + + it 'returns true' do + is_expected.to be_truthy + end + end + end + + context 'when ref name is a full tag ref' do let(:ref) { 'refs/tags/something' } before do From 86c0558c1c473e26df0a1cd5ea4b647175e8b857 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 30 Nov 2018 12:36:46 +0100 Subject: [PATCH 26/33] Refactor Project#protected_for? specs This refactors the Project#protected_for? specs to separate them into two contexts: when the ref is a full ref and when the ref is a ref name. --- spec/models/project_spec.rb | 138 ++++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 61 deletions(-) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 33bce0c0823..b4b9d921ba4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2567,30 +2567,7 @@ describe Project do subject { project.protected_for?(ref) } - context 'when ref is nil' do - let(:ref) { nil } - - it 'returns false' do - is_expected.to be_falsey - end - end - - context 'when ref is ambiguous' do - let(:ref) { 'ref' } - - before do - project.repository.add_branch(project.creator, ref, 'master') - project.repository.add_tag(project.creator, ref, 'master') - end - - it 'raises an error' do - expect { subject }.to raise_error(Repository::AmbiguousRefError) - end - end - - context 'when the ref is not protected' do - let(:ref) { 'master' } - + shared_examples 'ref is not protected' do before do stub_application_setting( default_branch_protection: Gitlab::Access::PROTECTION_NONE) @@ -2601,9 +2578,7 @@ describe Project do end end - context 'when the ref is a protected branch' do - let(:ref) { 'master' } - + shared_examples 'ref is protected branch' do before do create(:protected_branch, name: 'master', project: project) end @@ -2613,9 +2588,7 @@ describe Project do end end - context 'when the ref is a protected tag' do - let(:ref) { 'v1.0.0' } - + shared_examples 'ref is protected tag' do before do create(:protected_tag, name: 'v1.0.0', project: project) end @@ -2625,44 +2598,87 @@ describe Project do end end - context 'when ref is a full ref' do - let(:ref) { 'refs/heads/master' } - - context 'when ref is not protected' do - it 'returns false' do - is_expected.to be_falsey - end - end - - context 'when ref is protected' do - before do - create(:protected_branch, name: 'master', project: project) - end - - it 'returns true' do - is_expected.to be_truthy - end - end - end - - context 'when ref name is a full tag ref' do - let(:ref) { 'refs/tags/something' } - - before do - project.repository.add_branch(project.creator, ref, 'master') - end + context 'when ref is nil' do + let(:ref) { nil } it 'returns false' do is_expected.to be_falsey end + end + + context 'when ref is ref name' do + context 'when ref is ambiguous' do + let(:ref) { 'ref' } - context 'when ref is a protected branch' do before do - create(:protected_branch, name: 'refs/tags/something', project: project) + project.repository.add_branch(project.creator, 'ref', 'master') + project.repository.add_tag(project.creator, 'ref', 'master') end - it 'returns true' do - is_expected.to be_truthy + it 'raises an error' do + expect { subject }.to raise_error(Repository::AmbiguousRefError) + end + end + + context 'when the ref is not protected' do + let(:ref) { 'master' } + + it_behaves_like 'ref is not protected' + end + + context 'when the ref is a protected branch' do + let(:ref) { 'master' } + + it_behaves_like 'ref is protected branch' + end + + context 'when the ref is a protected tag' do + let(:ref) { 'v1.0.0' } + + it_behaves_like 'ref is protected tag' + end + end + + context 'when ref is full ref' do + context 'when the ref is not protected' do + let(:ref) { 'refs/heads/master' } + + it_behaves_like 'ref is not protected' + end + + context 'when the ref is a protected branch' do + let(:ref) { 'refs/heads/master' } + + it_behaves_like 'ref is protected branch' + end + + context 'when the ref is a protected tag' do + let(:ref) { 'refs/tags/v1.0.0' } + + it_behaves_like 'ref is protected tag' + end + + context 'when branch ref name is a full tag ref' do + let(:ref) { 'refs/tags/something' } + + before do + project.repository.add_branch(project.creator, ref, 'master') + end + + context 'when ref is not protected' do + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when ref is a protected branch' do + before do + create(:protected_branch, name: 'refs/tags/something', project: project) + end + + it 'returns true' do + is_expected.to be_truthy + end end end end @@ -2883,7 +2899,7 @@ describe Project do it 'shows full error updating an invalid MR' do error_message = 'Failed to replace merge_requests because one or more of the new records could not be saved.'\ - ' Validate fork Source project is not a fork of the target project' + ' Validate fork Source project is not a fork of the target project' expect { project.append_or_update_attribute(:merge_requests, [create(:merge_request)]) } .to raise_error(ActiveRecord::RecordNotSaved, error_message) From 44374434b5a0b4297686d5388f1124eda3adcbff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 30 Nov 2018 13:07:55 +0100 Subject: [PATCH 27/33] Conditionally assign ref_name for more efficiency --- app/models/project.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 0948e4625a8..79414665001 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1741,7 +1741,11 @@ class Project < ActiveRecord::Base raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref) resolved_ref = repository.expand_ref(ref) || ref - ref_name = Gitlab::Git.ref_name(resolved_ref) + ref_name = if resolved_ref == ref + Gitlab::Git.ref_name(resolved_ref) + else + ref + end if Gitlab::Git.branch_ref?(resolved_ref) ProtectedBranch.protected?(self, ref_name) From 9b4e45c35253a397f3ff79207736280293848bf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 3 Dec 2018 14:06:07 +0100 Subject: [PATCH 28/33] Check for explicit true or false in specs --- spec/models/project_spec.rb | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b4b9d921ba4..606636e9d58 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2574,7 +2574,7 @@ describe Project do end it 'returns false' do - is_expected.to be_falsey + is_expected.to be false end end @@ -2584,7 +2584,7 @@ describe Project do end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true end end @@ -2594,7 +2594,7 @@ describe Project do end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true end end @@ -2602,7 +2602,7 @@ describe Project do let(:ref) { nil } it 'returns false' do - is_expected.to be_falsey + is_expected.to be false end end @@ -2637,6 +2637,14 @@ describe Project do it_behaves_like 'ref is protected tag' end + + context 'when ref does not exist' do + let(:ref) { 'something' } + + it 'returns false' do + is_expected.to be false + end + end end context 'when ref is full ref' do @@ -2667,7 +2675,7 @@ describe Project do context 'when ref is not protected' do it 'returns false' do - is_expected.to be_falsey + is_expected.to be false end end @@ -2677,10 +2685,18 @@ describe Project do end it 'returns true' do - is_expected.to be_truthy + is_expected.to be true end end end + + context 'when ref does not exist' do + let(:ref) { 'refs/heads/something' } + + it 'returns false' do + is_expected.to be false + end + end end end From 1b28a2c1a9c9651316997f7e12b161b68001126b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 3 Dec 2018 14:10:42 +0100 Subject: [PATCH 29/33] Check resolved_ref before checking if protected --- app/models/project.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 79414665001..d75fecb5ff2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1737,10 +1737,11 @@ class Project < ActiveRecord::Base end def protected_for?(ref) - return false if ref.nil? raise Repository::AmbiguousRefError if repository.ambiguous_ref?(ref) resolved_ref = repository.expand_ref(ref) || ref + return false unless Gitlab::Git.tag_ref?(resolved_ref) || Gitlab::Git.branch_ref?(resolved_ref) + ref_name = if resolved_ref == ref Gitlab::Git.ref_name(resolved_ref) else From 6cd0965233822fe31366d0763633de6bb11d8d72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Dec 2018 19:39:47 +0100 Subject: [PATCH 30/33] Support merge_request pipeline ref types --- app/models/ci/pipeline.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index a1a11d2f7ab..4f64fff88ac 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -389,7 +389,7 @@ module Ci end def branch? - !tag? && !merge_request? + super && !merge_request? end def stuck? @@ -721,14 +721,10 @@ module Ci end def git_ref - if branch? + if merge_request? Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s - elsif merge_request? - Gitlab::Git::BRANCH_REF_PREFIX + ref.to_s - elsif tag? - Gitlab::Git::TAG_REF_PREFIX + ref.to_s else - raise ArgumentError, 'Invalid pipeline type!' + super end end From 78383c41bb51d50d970f0056e80f698960ed40dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Dec 2018 20:39:11 +0100 Subject: [PATCH 31/33] Use build for testing HasRef --- spec/models/concerns/has_ref_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/models/concerns/has_ref_spec.rb b/spec/models/concerns/has_ref_spec.rb index 14db197dfe0..8aed72d77a4 100644 --- a/spec/models/concerns/has_ref_spec.rb +++ b/spec/models/concerns/has_ref_spec.rb @@ -4,13 +4,13 @@ require 'spec_helper' describe HasRef do describe '#branch?' do - let(:pipeline) { create(:ci_pipeline) } + let(:build) { create(:ci_build) } - subject { pipeline.branch? } + subject { build.branch? } context 'is not a tag' do before do - pipeline.tag = false + build.tag = false end it 'return true when tag is set to false' do @@ -20,7 +20,7 @@ describe HasRef do context 'is not a tag' do before do - pipeline.tag = true + build.tag = true end it 'return false when tag is set to true' do @@ -30,10 +30,10 @@ describe HasRef do end describe '#git_ref' do - subject { pipeline.git_ref } + subject { build.git_ref } context 'when tag is true' do - let(:pipeline) { create(:ci_pipeline, tag: true) } + let(:build) { create(:ci_build, tag: true) } it 'returns a tag ref' do is_expected.to start_with(Gitlab::Git::TAG_REF_PREFIX) @@ -41,7 +41,7 @@ describe HasRef do end context 'when tag is false' do - let(:pipeline) { create(:ci_pipeline, tag: false) } + let(:build) { create(:ci_build, tag: false) } it 'returns a branch ref' do is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) @@ -49,7 +49,7 @@ describe HasRef do end context 'when tag is nil' do - let(:pipeline) { create(:ci_pipeline, tag: nil) } + let(:build) { create(:ci_build, tag: nil) } it 'returns a branch ref' do is_expected.to start_with(Gitlab::Git::BRANCH_REF_PREFIX) From 426488b7218e85ce69868ae4628801af2322b74a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Dec 2018 20:39:26 +0100 Subject: [PATCH 32/33] Use full ref when creating MR pipeline in specs --- spec/services/ci/create_pipeline_service_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index ccc6b0ef1c7..02fde579f6f 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -667,7 +667,7 @@ describe Ci::CreatePipelineService do stub_ci_pipeline_yaml_file(YAML.dump(config)) end - let(:ref_name) { 'feature' } + let(:ref_name) { 'refs/heads/feature' } context 'when source is merge request' do let(:source) { :merge_request } @@ -696,7 +696,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -709,7 +709,7 @@ describe Ci::CreatePipelineService do end context 'when ref is tag' do - let(:ref_name) { 'v1.1.0' } + let(:ref_name) { 'refs/tags/v1.1.0' } it 'does not create a merge request pipeline' do expect(pipeline).not_to be_persisted @@ -721,7 +721,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: target_project, target_branch: 'master') end @@ -786,7 +786,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end @@ -839,7 +839,7 @@ describe Ci::CreatePipelineService do let(:merge_request) do create(:merge_request, source_project: project, - source_branch: ref_name, + source_branch: Gitlab::Git.ref_name(ref_name), target_project: project, target_branch: 'master') end From 15526e023daa4c19dc1b619e02946a5dc775c49c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 8 Dec 2018 21:25:33 +0100 Subject: [PATCH 33/33] Add CHANGELOG entry --- .../security-master-secret-ci-variables-exposed.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/security-master-secret-ci-variables-exposed.yml diff --git a/changelogs/unreleased/security-master-secret-ci-variables-exposed.yml b/changelogs/unreleased/security-master-secret-ci-variables-exposed.yml new file mode 100644 index 00000000000..702181065f5 --- /dev/null +++ b/changelogs/unreleased/security-master-secret-ci-variables-exposed.yml @@ -0,0 +1,5 @@ +--- +title: Prevent leaking protected variables for ambiguous refs. +merge_request: +author: +type: security