Merge branch 'pedropombeiro/297382/remove-variable_inside_variable-FF' into 'master'

Remove variable_inside_variable feature flag

See merge request gitlab-org/gitlab!73662

app/presenters/ci/build_runner_presenter.rb

@@ -33,11 +33,7 @@ module Ci
     end
 
     def runner_variables
-      if Feature.enabled?(:variable_inside_variable, project, default_enabled: :yaml)
       variables.sort_and_expand_all(project, keep_undefined: true).to_runner_variables
-      else
-        variables.to_runner_variables
-      end
     end
 
     def refspecs

config/feature_flags/development/variable_inside_variable.yml

----
-name: variable_inside_variable
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50156
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/297382
-milestone: '13.11'
-type: development
-group: group::runner
-default_enabled: true

doc/ci/variables/where_variables_can_be_used.md

@@ -63,14 +63,12 @@ because the expansion is done in GitLab before any runner gets the job.
 
 #### Nested variable expansion
 
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48627) in GitLab 13.10. [Deployed behind the `variable_inside_variable` feature flag](../../user/feature_flags.md), disabled by default.
-> - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.3.
-> - [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.4.
+- [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48627) in GitLab 13.10. [Deployed behind the `variable_inside_variable` feature flag](../../user/feature_flags.md), disabled by default.
+- [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.3.
+- [Enabled on self-managed](https://gitlab.com/gitlab-org/gitlab/-/issues/297382) in GitLab 14.4.
+- Feature flag `variable_inside_variable` removed in GitLab 14.5.
 
-FLAG:
-On self-managed GitLab, by default this feature is available. To hide the feature, ask an administrator to [disable the feature flag](../../administration/feature_flags.md) named `variable_inside_variable`. On GitLab.com, this feature is available.
-
-GitLab expands job variable values recursively before sending them to the runner. For example:
+GitLab expands job variable values recursively before sending them to the runner. For example, in the following scenario:
 
 ```yaml
 - BUILD_ROOT_DIR: '${CI_BUILDS_DIR}'
@@ -78,10 +76,7 @@ GitLab expands job variable values recursively before sending them to the runner
 - PACKAGE_PATH: '${OUT_PATH}/pkg'
 ```
 
-If nested variable expansion is:
-
-- **Disabled**: the runner receives `${BUILD_ROOT_DIR}/out/pkg`. This is not a valid path.
-- **Enabled**: the runner receives a valid, fully-formed path. For example, if `${CI_BUILDS_DIR}` is `/output`, then `PACKAGE_PATH` would be `/output/out/pkg`.
+The runner receives a valid, fully-formed path. For example, if `${CI_BUILDS_DIR}` is `/output`, then `PACKAGE_PATH` would be `/output/out/pkg`.
 
 References to unavailable variables are left intact. In this case, the runner
 [attempts to expand the variable value](#gitlab-runner-internal-variable-expansion-mechanism) at runtime.

lib/gitlab/ci/variables/collection.rb

@@ -90,8 +90,6 @@ module Gitlab
         end
 
         def sort_and_expand_all(project, keep_undefined: false)
-          return self if Feature.disabled?(:variable_inside_variable, project, default_enabled: :yaml)
-
           sorted = Sort.new(self)
           return self.class.new(self, sorted.errors) unless sorted.valid?
 

spec/lib/gitlab/ci/pipeline/seed/build_spec.rb

@@ -1218,16 +1218,10 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
         ]
       end
 
-      context 'when FF :variable_inside_variable is enabled' do
-        before do
-          stub_feature_flags(variable_inside_variable: [project])
-        end
-
       it "does not have errors" do
         expect(subject.errors).to be_empty
       end
     end
-    end
 
     context 'containing cyclic reference' do
       let(:pipeline_variables) do
@@ -1238,21 +1232,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
         ]
       end
 
-      context 'when FF :variable_inside_variable is disabled' do
-        before do
-          stub_feature_flags(variable_inside_variable: false)
-        end
-
-        it "does not have errors" do
-          expect(subject.errors).to be_empty
-        end
-      end
-
-      context 'when FF :variable_inside_variable is enabled' do
-        before do
-          stub_feature_flags(variable_inside_variable: [project])
-        end
-
       it "returns an error" do
         expect(subject.errors).to contain_exactly(
           'rspec: circular variable reference detected: ["A", "B", "C"]')
@@ -1271,5 +1250,4 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do
       end
     end
   end
-  end
 end

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

@@ -358,96 +358,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection do
   end
 
   describe '#sort_and_expand_all' do
-    context 'when FF :variable_inside_variable is disabled' do
-      let_it_be(:project_with_flag_disabled) { create(:project) }
-      let_it_be(:project_with_flag_enabled) { create(:project) }
-
-      before do
-        stub_feature_flags(variable_inside_variable: [project_with_flag_enabled])
-      end
-
-      context 'table tests' do
-        using RSpec::Parameterized::TableSyntax
-
-        where do
-          {
-            "empty array": {
-              variables: [],
-              keep_undefined: false
-            },
-            "simple expansions": {
-              variables: [
-                { key: 'variable', value: 'value' },
-                { key: 'variable2', value: 'result' },
-                { key: 'variable3', value: 'key$variable$variable2' }
-              ],
-              keep_undefined: false
-            },
-            "complex expansion": {
-              variables: [
-                { key: 'variable', value: 'value' },
-                { key: 'variable2', value: 'key${variable}' }
-              ],
-              keep_undefined: false
-            },
-            "out-of-order variable reference": {
-              variables: [
-                { key: 'variable2', value: 'key${variable}' },
-                { key: 'variable', value: 'value' }
-              ],
-              keep_undefined: false
-            },
-            "complex expansions with raw variable": {
-              variables: [
-                { key: 'variable3', value: 'key_${variable}_${variable2}' },
-                { key: 'variable', value: '$variable2', raw: true },
-                { key: 'variable2', value: 'value2' }
-              ],
-              keep_undefined: false
-            },
-            "escaped characters in complex expansions are kept intact": {
-              variables: [
-                { key: 'variable3', value: 'key_${variable}_$${HOME}_%%HOME%%' },
-                { key: 'variable', value: '$variable2' },
-                { key: 'variable2', value: 'value2' }
-              ],
-              keep_undefined: false
-            },
-            "array with cyclic dependency": {
-              variables: [
-                { key: 'variable', value: '$variable2' },
-                { key: 'variable2', value: '$variable3' },
-                { key: 'variable3', value: 'key$variable$variable2' }
-              ],
-              keep_undefined: true
-            }
-          }
-        end
-
-        with_them do
-          let(:collection) { Gitlab::Ci::Variables::Collection.new(variables, keep_undefined: keep_undefined) }
-
-          subject { collection.sort_and_expand_all(project_with_flag_disabled) }
-
-          it 'returns Collection' do
-            is_expected.to be_an_instance_of(Gitlab::Ci::Variables::Collection)
-          end
-
-          it 'does not expand variables' do
-            var_hash = variables.pluck(:key, :value).to_h
-            expect(subject.to_hash).to eq(var_hash)
-          end
-        end
-      end
-    end
-
-    context 'when FF :variable_inside_variable is enabled' do
-      let_it_be(:project_with_flag_disabled) { create(:project) }
-      let_it_be(:project_with_flag_enabled) { create(:project) }
-
-      before do
-        stub_feature_flags(variable_inside_variable: [project_with_flag_enabled])
-      end
+    let_it_be(:project) { create(:project) }
 
     context 'table tests' do
       using RSpec::Parameterized::TableSyntax
@@ -639,7 +550,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection do
       with_them do
         let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
 
-          subject { collection.sort_and_expand_all(project_with_flag_enabled, keep_undefined: keep_undefined) }
+        subject { collection.sort_and_expand_all(project, keep_undefined: keep_undefined) }
 
         it 'returns Collection' do
           is_expected.to be_an_instance_of(Gitlab::Ci::Variables::Collection)
@@ -657,5 +568,4 @@ RSpec.describe Gitlab::Ci::Variables::Collection do
       end
     end
   end
-  end
 end

spec/presenters/ci/build_runner_presenter_spec.rb

@@ -259,12 +259,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
   describe '#runner_variables' do
     subject { presenter.runner_variables }
 
-    let_it_be(:project_with_flag_disabled) { create(:project, :repository) }
-    let_it_be(:project_with_flag_enabled) { create(:project, :repository) }
-
-    before do
-      stub_feature_flags(variable_inside_variable: [project_with_flag_enabled])
-    end
+    let_it_be(:project) { create(:project, :repository) }
 
     shared_examples 'returns an array with the expected variables' do
       it 'returns an array' do
@@ -276,22 +271,12 @@ RSpec.describe Ci::BuildRunnerPresenter do
       end
     end
 
-    context 'when FF :variable_inside_variable is disabled' do
-      let(:sha) { project_with_flag_disabled.repository.commit.sha }
-      let(:pipeline) { create(:ci_pipeline, sha: sha, project: project_with_flag_disabled) }
-      let(:build) { create(:ci_build, pipeline: pipeline) }
-
-      it_behaves_like 'returns an array with the expected variables'
-    end
-
-    context 'when FF :variable_inside_variable is enabled' do
-      let(:sha) { project_with_flag_enabled.repository.commit.sha }
-      let(:pipeline) { create(:ci_pipeline, sha: sha, project: project_with_flag_enabled) }
+    let(:sha) { project.repository.commit.sha }
+    let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) }
     let(:build) { create(:ci_build, pipeline: pipeline) }
 
     it_behaves_like 'returns an array with the expected variables'
   end
-  end
 
   describe '#runner_variables subset' do
     subject { presenter.runner_variables.select { |v| %w[A B C].include?(v.fetch(:key)) } }
@@ -305,25 +290,6 @@ RSpec.describe Ci::BuildRunnerPresenter do
         create(:ci_pipeline_variable, key: 'C', value: 'value', pipeline: build.pipeline)
       end
 
-      context 'when FF :variable_inside_variable is disabled' do
-        before do
-          stub_feature_flags(variable_inside_variable: false)
-        end
-
-        it 'returns non-expanded variables' do
-          is_expected.to eq [
-                              { key: 'A', value: 'refA-$B', public: false, masked: false },
-                              { key: 'B', value: 'refB-$C-$D', public: false, masked: false },
-                              { key: 'C', value: 'value', public: false, masked: false }
-                            ]
-        end
-      end
-
-      context 'when FF :variable_inside_variable is enabled' do
-        before do
-          stub_feature_flags(variable_inside_variable: [build.project])
-        end
-
       it 'returns expanded and sorted variables' do
         is_expected.to eq [
                             { key: 'C', value: 'value', public: false, masked: false },
@@ -333,5 +299,4 @@ RSpec.describe Ci::BuildRunnerPresenter do
       end
     end
   end
-  end
 end