Cleanup secret variables refactoring in variables builder

Changelog: other

config/feature_flags/development/ci_variables_builder_memoize_secret_variables.yml

----
-name: ci_variables_builder_memoize_secret_variables
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/79850
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351995
-milestone: '14.8'
-type: development
-group: group::pipeline execution
-default_enabled: false

lib/gitlab/ci/variables/builder.rb

@@ -24,8 +24,8 @@ module Gitlab
             variables.concat(user_variables(job.user))
             variables.concat(job.dependency_variables) if dependencies
             variables.concat(secret_instance_variables)
-            variables.concat(secret_group_variables(environment: environment, ref: job.git_ref))
-            variables.concat(secret_project_variables(environment: environment, ref: job.git_ref))
+            variables.concat(secret_group_variables(environment: environment))
+            variables.concat(secret_project_variables(environment: environment))
             variables.concat(job.trigger_request.user_variables) if job.trigger_request
             variables.concat(pipeline.variables)
             variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
@@ -75,21 +75,21 @@ module Gitlab
           end
         end
 
-        def secret_group_variables(environment:, ref:)
-          if memoize_secret_variables?
-            memoized_secret_group_variables(environment: environment)
-          else
-            return [] unless project.group
-
-            project.group.ci_variables_for(ref, project, environment: environment)
+        def secret_group_variables(environment:)
+          strong_memoize_with(:secret_group_variables, environment) do
+            group_variables_builder
+              .secret_variables(
+                environment: environment,
+                protected_ref: protected_ref?)
           end
         end
 
-        def secret_project_variables(environment:, ref:)
-          if memoize_secret_variables?
-            memoized_secret_project_variables(environment: environment)
-          else
-            project.ci_variables_for(ref: ref, environment: environment)
+        def secret_project_variables(environment:)
+          strong_memoize_with(:secret_project_variables, environment) do
+            project_variables_builder
+              .secret_variables(
+                environment: environment,
+                protected_ref: protected_ref?)
           end
         end
 
@@ -120,24 +120,6 @@ module Gitlab
           end
         end
 
-        def memoized_secret_project_variables(environment:)
-          strong_memoize_with(:secret_project_variables, environment) do
-            project_variables_builder
-              .secret_variables(
-                environment: environment,
-                protected_ref: protected_ref?)
-          end
-        end
-
-        def memoized_secret_group_variables(environment:)
-          strong_memoize_with(:secret_group_variables, environment) do
-            group_variables_builder
-              .secret_variables(
-                environment: environment,
-                protected_ref: protected_ref?)
-          end
-        end
-
         def ci_node_total_value(job)
           parallel = job.options&.dig(:parallel)
           parallel = parallel.dig(:total) if parallel.is_a?(Hash)
@@ -150,14 +132,6 @@ module Gitlab
           end
         end
 
-        def memoize_secret_variables?
-          strong_memoize(:memoize_secret_variables) do
-            ::Feature.enabled?(:ci_variables_builder_memoize_secret_variables,
-              project,
-              default_enabled: :yaml)
-          end
-        end
-
         def strong_memoize_with(name, *args)
           container = strong_memoize(name) { {} }
 

spec/lib/gitlab/ci/variables/builder_spec.rb

@@ -278,6 +278,14 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
   end
 
   shared_examples "secret CI variables" do
+    let(:protected_variable_item) do
+      Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable)
+    end
+
+    let(:unprotected_variable_item) do
+      Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable)
+    end
+
     context 'when ref is branch' do
       context 'when ref is protected' do
         before do
@@ -338,43 +346,20 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
     let_it_be(:protected_variable) { create(:ci_instance_variable, protected: true) }
     let_it_be(:unprotected_variable) { create(:ci_instance_variable, protected: false) }
 
-    let(:protected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable) }
-    let(:unprotected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable) }
-
     include_examples "secret CI variables"
   end
 
   describe '#secret_group_variables' do
-    subject { builder.secret_group_variables(ref: job.git_ref, environment: job.expanded_environment_name) }
+    subject { builder.secret_group_variables(environment: job.expanded_environment_name) }
 
     let_it_be(:protected_variable) { create(:ci_group_variable, protected: true, group: group) }
     let_it_be(:unprotected_variable) { create(:ci_group_variable, protected: false, group: group) }
 
-    context 'with ci_variables_builder_memoize_secret_variables disabled' do
-      before do
-        stub_feature_flags(ci_variables_builder_memoize_secret_variables: false)
-      end
-
-      let(:protected_variable_item) { protected_variable }
-      let(:unprotected_variable_item) { unprotected_variable }
-
-      include_examples "secret CI variables"
-    end
-
-    context 'with ci_variables_builder_memoize_secret_variables enabled' do
-      before do
-        stub_feature_flags(ci_variables_builder_memoize_secret_variables: true)
-      end
-
-      let(:protected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable) }
-      let(:unprotected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable) }
-
     include_examples "secret CI variables"
 
     context 'variables memoization' do
       let_it_be(:scoped_variable) { create(:ci_group_variable, group: group, environment_scope: 'scoped') }
 
-        let(:ref) { job.git_ref }
       let(:environment) { job.expanded_environment_name }
       let(:scoped_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(scoped_variable) }
 
@@ -394,7 +379,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
           end
 
           2.times do
-              expect(builder.secret_group_variables(ref: ref, environment: 'production'))
+            expect(builder.secret_group_variables(environment: 'production'))
               .to contain_exactly(unprotected_variable_item, protected_variable_item)
           end
         end
@@ -422,45 +407,24 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
           end
 
           2.times do
-              expect(builder.secret_group_variables(ref: 'other', environment: nil))
+            expect(builder.secret_group_variables(environment: nil))
               .to contain_exactly(unprotected_variable_item)
 
-              expect(builder.secret_group_variables(ref: 'other', environment: 'scoped'))
+            expect(builder.secret_group_variables(environment: 'scoped'))
               .to contain_exactly(unprotected_variable_item, scoped_variable_item)
           end
         end
       end
     end
   end
-  end
 
   describe '#secret_project_variables' do
     let_it_be(:protected_variable) { create(:ci_variable, protected: true, project: project) }
     let_it_be(:unprotected_variable) { create(:ci_variable, protected: false, project: project) }
 
-    let(:ref) { job.git_ref }
     let(:environment) { job.expanded_environment_name }
 
-    subject { builder.secret_project_variables(ref: ref, environment: environment) }
-
-    context 'with ci_variables_builder_memoize_secret_variables disabled' do
-      before do
-        stub_feature_flags(ci_variables_builder_memoize_secret_variables: false)
-      end
-
-      let(:protected_variable_item) { protected_variable }
-      let(:unprotected_variable_item) { unprotected_variable }
-
-      include_examples "secret CI variables"
-    end
-
-    context 'with ci_variables_builder_memoize_secret_variables enabled' do
-      before do
-        stub_feature_flags(ci_variables_builder_memoize_secret_variables: true)
-      end
-
-      let(:protected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(protected_variable) }
-      let(:unprotected_variable_item) { Gitlab::Ci::Variables::Collection::Item.fabricate(unprotected_variable) }
+    subject { builder.secret_project_variables(environment: environment) }
 
     include_examples "secret CI variables"
 
@@ -485,7 +449,7 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
           end
 
           2.times do
-              expect(builder.secret_project_variables(ref: ref, environment: 'production'))
+            expect(builder.secret_project_variables(environment: 'production'))
               .to contain_exactly(unprotected_variable_item, protected_variable_item)
           end
         end
@@ -513,15 +477,14 @@ RSpec.describe Gitlab::Ci::Variables::Builder do
           end
 
           2.times do
-              expect(builder.secret_project_variables(ref: 'other', environment: nil))
+            expect(builder.secret_project_variables(environment: nil))
               .to contain_exactly(unprotected_variable_item)
 
-              expect(builder.secret_project_variables(ref: 'other', environment: 'scoped'))
+            expect(builder.secret_project_variables(environment: 'scoped'))
               .to contain_exactly(unprotected_variable_item, scoped_variable_item)
           end
         end
       end
     end
   end
-  end
 end