Merge branch 'remove-ff-ci_force_exposing_merge_request_refs' into 'master'

Remove FF ci_force_exposing_merge_request_refs

See merge request gitlab-org/gitlab!30591

app/models/ci/persistent_ref.rb

@@ -14,16 +14,12 @@ module Ci
     delegate :ref_exists?, :create_ref, :delete_refs, to: :repository
 
     def exist?
-      return unless enabled?
-
       ref_exists?(path)
     rescue
       false
     end
 
     def create
-      return unless enabled?
-
       create_ref(sha, path)
     rescue => e
       Gitlab::ErrorTracking
@@ -31,8 +27,6 @@ module Ci
     end
 
     def delete
-      return unless enabled?
-
       delete_refs(path)
     rescue Gitlab::Git::Repository::NoRepository
       # no-op
@@ -44,11 +38,5 @@ module Ci
     def path
       "refs/#{Repository::REF_PIPELINES}/#{pipeline.id}"
     end
-
-    private
-
-    def enabled?
-      Feature.enabled?(:depend_on_persistent_pipeline_ref, project, default_enabled: true)
-    end
   end
 end

app/presenters/ci/build_runner_presenter.rb

@@ -34,7 +34,6 @@ module Ci
 
     def refspecs
       specs = []
-      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,19 +49,6 @@ 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]
 
@@ -100,10 +86,6 @@ module Ci
       "+#{Gitlab::Git::TAG_REF_PREFIX}#{ref}:#{RUNNER_REMOTE_TAG_PREFIX}#{ref}"
     end
 
-    def refspec_for_pipeline_ref
-      "+#{ref}:#{ref}"
-    end
-
     def refspec_for_persistent_ref
       "+#{persistent_ref_path}:#{persistent_ref_path}"
     end

doc/ci/pipelines/index.md

@@ -549,15 +549,3 @@ To illustrate its life cycle:
    even if the commit history of the `example` branch has been overwritten by force-push.
 1. GitLab Runner fetches the persistent pipeline ref and gets source code from the checkout-SHA.
 1. When the pipeline finished, its persistent ref is cleaned up in a background process.
-
-NOTE: **NOTE**: At this moment, this feature is on by default and can be manually disabled
-by disabling `depend_on_persistent_pipeline_ref` feature flag. If you're interested in
-manually disabling this behavior, please ask the administrator
-to execute the following commands in rails console.
-
-```shell
-> sudo gitlab-rails console                                        # Login to Rails console of GitLab instance.
-> project = Project.find_by_full_path('namespace/project-name')    # Get the project instance.
-> Feature.disable(:depend_on_persistent_pipeline_ref, project)     # Disable the feature flag for specific project
-> Feature.disable(:depend_on_persistent_pipeline_ref)              # Disable the feature flag system-wide
-```

spec/models/ci/persistent_ref_spec.rb

@@ -45,18 +45,6 @@ describe Ci::PersistentRef do
         expect(pipeline.persistent_ref).to be_exist
       end
 
-      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 'does not create a persistent ref' do
-          expect(project.repository).not_to receive(:create_ref)
-
-          subject
-        end
-      end
-
       context 'when sha does not exist in the repository' do
         let(:sha) { 'not-exist' }
 

spec/presenters/ci/build_runner_presenter_spec.rb

@@ -173,81 +173,34 @@ describe Ci::BuildRunnerPresenter do
       let(:pipeline) { merge_request.all_pipelines.first }
       let(:build) { create(:ci_build, ref: pipeline.ref, pipeline: pipeline) }
 
-      context 'when depend_on_persistent_pipeline_ref feature flag is enabled' do
-        before do
-          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/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) }
+      before do
+        pipeline.persistent_ref.create
+      end
 
-          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
+      it 'returns the correct refspecs' do
+        is_expected
+          .to contain_exactly("+refs/pipelines/#{pipeline.id}:refs/pipelines/#{pipeline.id}")
       end
 
-      context 'when depend_on_persistent_pipeline_ref feature flag is disabled' do
+      context 'when GIT_DEPTH is zero' do
         before do
-          stub_feature_flags(depend_on_persistent_pipeline_ref: false)
+          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')
-        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
+            .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) }
+      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
+        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