Commit cea105fe authored by Shinya Maeda's avatar Shinya Maeda

Stop exposing MR refs in favor of persistent pipeline refs

This commit fixes the inconsistent behavior of MR refs
parent 6e39c0ad
......@@ -34,7 +34,7 @@ module Ci
def refspecs
specs = []
specs << refspec_for_pipeline_ref if merge_request_ref?
specs << refspec_for_pipeline_ref if should_expose_merge_request_ref?
specs << refspec_for_persistent_ref if persistent_ref_exist?
if git_depth > 0
......@@ -50,6 +50,19 @@ module Ci
private
# We will stop exposing merge request refs when we fully depend on persistent refs
# (i.e. remove `refspec_for_pipeline_ref` when we remove `depend_on_persistent_pipeline_ref` feature flag.)
# `ci_force_exposing_merge_request_refs` is an extra feature flag that allows us to
# forcibly expose MR refs even if the `depend_on_persistent_pipeline_ref` feature flag enabled.
# This is useful when we see an unexpected behaviors/reports from users.
# See https://gitlab.com/gitlab-org/gitlab/issues/35140.
def should_expose_merge_request_ref?
return false unless merge_request_ref?
return true if Feature.enabled?(:ci_force_exposing_merge_request_refs, project)
Feature.disabled?(:depend_on_persistent_pipeline_ref, project, default_enabled: true)
end
def create_archive(artifacts)
return unless artifacts[:untracked] || artifacts[:paths]
......
---
title: Stop exposing MR refs in favor of persistent pipeline refs
merge_request: 22198
author:
type: fixed
......@@ -183,29 +183,81 @@ describe Ci::BuildRunnerPresenter do
let(:pipeline) { merge_request.all_pipelines.first }
let(:build) { create(:ci_build, ref: pipeline.ref, pipeline: pipeline) }
it 'returns the correct refspecs' do
is_expected
.to contain_exactly('+refs/merge-requests/1/head:refs/merge-requests/1/head')
end
context 'when GIT_DEPTH is zero' do
context 'when depend_on_persistent_pipeline_ref feature flag is enabled' do
before do
create(:ci_pipeline_variable, key: 'GIT_DEPTH', value: 0, pipeline: build.pipeline)
stub_feature_flags(ci_force_exposing_merge_request_refs: false)
pipeline.persistent_ref.create
end
it 'returns the correct refspecs' do
is_expected
.to contain_exactly('+refs/merge-requests/1/head:refs/merge-requests/1/head',
'+refs/heads/*:refs/remotes/origin/*',
'+refs/tags/*:refs/tags/*')
.to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
end
context 'when ci_force_exposing_merge_request_refs feature flag is enabled' do
before do
stub_feature_flags(ci_force_exposing_merge_request_refs: true)
end
it 'returns the correct refspecs' do
is_expected
.to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
'+refs/merge-requests/1/head:refs/merge-requests/1/head')
end
end
context 'when GIT_DEPTH is zero' do
before do
create(:ci_pipeline_variable, key: 'GIT_DEPTH', value: 0, pipeline: build.pipeline)
end
it 'returns the correct refspecs' do
is_expected
.to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
'+refs/heads/*:refs/remotes/origin/*',
'+refs/tags/*:refs/tags/*')
end
end
context 'when pipeline is legacy detached merge request pipeline' do
let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) }
it 'returns the correct refspecs' do
is_expected.to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}",
"+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}")
end
end
end
context 'when pipeline is legacy detached merge request pipeline' do
let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) }
context 'when depend_on_persistent_pipeline_ref feature flag is disabled' do
before do
stub_feature_flags(depend_on_persistent_pipeline_ref: false)
end
it 'returns the correct refspecs' do
is_expected.to contain_exactly("+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}")
is_expected
.to contain_exactly('+refs/merge-requests/1/head:refs/merge-requests/1/head')
end
context 'when GIT_DEPTH is zero' do
before do
create(:ci_pipeline_variable, key: 'GIT_DEPTH', value: 0, pipeline: build.pipeline)
end
it 'returns the correct refspecs' do
is_expected
.to contain_exactly('+refs/merge-requests/1/head:refs/merge-requests/1/head',
'+refs/heads/*:refs/remotes/origin/*',
'+refs/tags/*:refs/tags/*')
end
end
context 'when pipeline is legacy detached merge request pipeline' do
let(:merge_request) { create(:merge_request, :with_legacy_detached_merge_request_pipeline) }
it 'returns the correct refspecs' do
is_expected.to contain_exactly("+refs/heads/#{build.ref}:refs/remotes/origin/#{build.ref}")
end
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment