diff options
-rw-r--r-- | app/models/project.rb | 6 | ||||
-rw-r--r-- | app/models/repository.rb | 13 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/chain/validate/repository.rb | 2 | ||||
-rw-r--r-- | spec/models/project_spec.rb | 23 | ||||
-rw-r--r-- | spec/models/repository_spec.rb | 68 |
5 files changed, 67 insertions, 45 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,14 +2565,23 @@ 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) + 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) { nil } + 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 @@ -2580,7 +2589,7 @@ describe Project do 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 + 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 + 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 |