Merge branch 'pedropombeiro/variable_inside_variable' into 'master'

Send fully-resolved variable values to the runner

See merge request gitlab-org/gitlab!48627

app/presenters/ci/build_runner_presenter.rb

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

changelogs/unreleased/pedropombeiro-variable_inside_variable.yml

+---
+title: Resolve nested variable values sent to the runner
+merge_request: 48627
+author:
+type: added

doc/ci/variables/where_variables_can_be_used.md

@@ -28,7 +28,7 @@ There are two places defined variables can be used. On the:
 | `environment:name`                         | yes              | GitLab                 | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/><br/>- Variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`).<br/>- Any other variables related to environment (currently only `CI_ENVIRONMENT_URL`).<br/>- [Persisted variables](#persisted-variables).                                                                                 |
 | `resource_group`                           | yes              | GitLab                 | Similar to `environment:url`, but the variables expansion doesn't support the following:<br/><br/>- Variables that are based on the environment's name (`CI_ENVIRONMENT_NAME`, `CI_ENVIRONMENT_SLUG`).<br/>- Any other variables related to environment (currently only `CI_ENVIRONMENT_URL`).<br/>- [Persisted variables](#persisted-variables).                                                                                 |
 | `include`                                  | yes              | GitLab                 | The variable expansion is made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab. <br/><br/>Predefined project variables are supported: `GITLAB_FEATURES`, `CI_DEFAULT_BRANCH`, and all variables that start with `CI_PROJECT_` (for example `CI_PROJECT_NAME`).                                                                                                            |
-| `variables`                                | yes              | Runner                 | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism)                                                                                                                                                                                                                                                                                   |
+| `variables`                                | yes              | GitLab/Runner          | The variable expansion is first made by the [internal variable expansion mechanism](#gitlab-internal-variable-expansion-mechanism) in GitLab, and then any unrecognized or unavailable variables are expanded by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism).                                                                                                   |
 | `image`                                    | yes              | Runner                 | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism)                                                                                                                                                                                                                                                                                   |
 | `services:[]`                              | yes              | Runner                 | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism)                                                                                                                                                                                                                                                                                   |
 | `services:[]:name`                         | yes              | Runner                 | The variable expansion is made by GitLab Runner's [internal variable expansion mechanism](#gitlab-runner-internal-variable-expansion-mechanism)                                                                                                                                                                                                                                                                                   |
@@ -61,6 +61,54 @@ The expanded part needs to be in a form of `$variable`, or `${variable}` or `%va
 Each form is handled in the same way, no matter which OS/shell handles the job,
 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.
+> - It's [deployed behind a feature flag](../../user/feature_flags.md), disabled by default.
+> - It can be enabled or disabled for a single project.
+> - It's disabled on GitLab.com.
+> - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enabling-the-nested-variable-expansion-feature). **(FREE SELF)**
+
+GitLab expands job variable values recursively before sending them to the runner. For example:
+
+```yaml
+- BUILD_ROOT_DIR: '${CI_BUILDS_DIR}'
+- OUT_PATH: '${BUILD_ROOT_DIR}/out'
+- 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`.
+
+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.
+For example, a variable like `CI_BUILDS_DIR` is known by the runner only at runtime.
+
+##### Enabling the nested variable expansion feature **(FREE SELF)**
+
+This feature comes with the `:variable_inside_variable` feature flag disabled by default.
+
+To enable this feature, ask a GitLab administrator with [Rails console access](../../administration/feature_flags.md#how-to-enable-and-disable-features-behind-flags) to run the
+following command:
+
+```ruby
+# For the instance
+Feature.enable(:variable_inside_variable)
+# For a single project
+Feature.enable(:variable_inside_variable, Project.find(<project id>))
+```
+
+To disable it:
+
+```ruby
+# For the instance
+Feature.disable(:variable_inside_variable)
+# For a single project
+Feature.disable(:variable_inside_variable, Project.find(<project id>))
+```
+
 ### GitLab Runner internal variable expansion mechanism
 
 - Supported: project/group variables, `.gitlab-ci.yml` variables, `config.toml` variables, and
@@ -70,16 +118,17 @@ because the expansion is done in GitLab before any runner gets the job.
 The runner uses Go's `os.Expand()` method for variable expansion. It means that it handles
 only variables defined as `$variable` and `${variable}`. What's also important, is that
 the expansion is done only once, so nested variables may or may not work, depending on the
-ordering of variables definitions.
+ordering of variables definitions, and whether [nested variable expansion](#nested-variable-expansion)
+is enabled in GitLab.
 
 ### Execution shell environment
 
-This is an expansion that takes place during the `script` execution.
-How it works depends on the used shell (`bash`, `sh`, `cmd`, PowerShell). For example, if the job's
+This is an expansion phase that takes place during the `script` execution.
+Its behavior depends on the shell used (`bash`, `sh`, `cmd`, PowerShell). For example, if the job's
 `script` contains a line `echo $MY_VARIABLE-${MY_VARIABLE_2}`, it should be properly handled
 by bash/sh (leaving empty strings or some values depending whether the variables were
 defined or not), but don't work with Windows' `cmd` or PowerShell, since these shells
-are using a different variables syntax.
+use a different variables syntax.
 
 Supported:
 
@@ -88,10 +137,10 @@ Supported:
   `.gitlab-ci.yml` variables, `config.toml` variables, and variables from triggers and pipeline schedules).
 - The `script` may also use all variables defined in the lines before. So, for example, if you define
   a variable `export MY_VARIABLE="test"`:
-  - In `before_script`, it works in the following lines of `before_script` and
+  - In `before_script`, it works in the subsequent lines of `before_script` and
     all lines of the related `script`.
-  - In `script`, it works in the following lines of `script`.
-  - In `after_script`, it works in following lines of `after_script`.
+  - In `script`, it works in the subsequent lines of `script`.
+  - In `after_script`, it works in subsequent lines of `after_script`.
 
 In the case of `after_script` scripts, they can:
 
@@ -99,7 +148,7 @@ In the case of `after_script` scripts, they can:
   section.
 - Not use variables defined in `before_script` and `script`.
 
-These restrictions are because `after_script` scripts are executed in a
+These restrictions exist because `after_script` scripts are executed in a
 [separated shell context](../yaml/README.md#after_script).
 
 ## Persisted variables

spec/presenters/ci/build_runner_presenter_spec.rb

@@ -85,7 +85,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
       Ci::JobArtifact::DEFAULT_FILE_NAMES.each do |file_type, filename|
         context file_type.to_s do
           let(:report) { { "#{file_type}": [filename] } }
-          let(:build) { create(:ci_build, options: { artifacts: { reports: report } } ) }
+          let(:build) { create(:ci_build, options: { artifacts: { reports: report } }) }
 
           let(:report_expectation) do
             {
@@ -106,7 +106,7 @@ RSpec.describe Ci::BuildRunnerPresenter do
 
     context "when option has both archive and reports specification" do
       let(:report) { { junit: ['junit.xml'] } }
-      let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: report } } ) }
+      let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: report } }) }
 
       let(:report_expectation) do
         {
@@ -272,27 +272,82 @@ RSpec.describe Ci::BuildRunnerPresenter do
     end
   end
 
-  describe '#variables' do
-    subject { presenter.variables }
+  describe '#runner_variables' do
+    subject { presenter.runner_variables }
 
-    let(:build) { create(:ci_build) }
+    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
+
+    shared_examples 'returns an array with the expected variables' do
+      it 'returns an array' do
+        is_expected.to be_an_instance_of(Array)
+      end
+
+      it 'returns the expected variables' do
+        is_expected.to eq(presenter.variables.to_runner_variables)
+      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(:build) { create(:ci_build, pipeline: pipeline) }
 
-    it 'returns a Collection' do
-      is_expected.to be_an_instance_of(Gitlab::Ci::Variables::Collection)
+      it_behaves_like 'returns an array with the expected variables'
     end
   end
 
-  describe '#runner_variables' do
-    subject { presenter.runner_variables }
+  describe '#runner_variables subset' do
+    subject { presenter.runner_variables.select { |v| %w[A B C].include?(v.fetch(:key)) } }
 
     let(:build) { create(:ci_build) }
 
-    it 'returns an array' do
-      is_expected.to be_an_instance_of(Array)
-    end
+    context 'with references in pipeline variables' do
+      before do
+        create(:ci_pipeline_variable, key: 'A', value: 'refA-$B', pipeline: build.pipeline)
+        create(:ci_pipeline_variable, key: 'B', value: 'refB-$C-$D', pipeline: build.pipeline)
+        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 the expected variables' do
-      is_expected.to eq(presenter.variables.to_runner_variables)
+        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 },
+                              { key: 'B', value: 'refB-value-$D', public: false, masked: false },
+                              { key: 'A', value: 'refA-refB-value-$D', public: false, masked: false }
+                            ]
+        end
+      end
     end
   end
 end