Skip to content

G
gitlab-ce
Commit 9e60bd9a authored by Pedro Pombeiro's avatar Pedro Pombeiro
Browse files
Options

Remove variable_inside_variable feature flag 

Changelog: other
parent 68f22b98
Hide whitespace changes
Inline Side-by-side
Showing with 225 additions and 391 deletions
app/presenters/ci/build_runner_presenter.rb
View file @ 9e60bd9a
......@@ -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
variables.sort_and_expand_all(project, keep_undefined: true).to_runner_variables
end
def refspecs
......
config/feature_flags/development/variable_inside_variable.yml deleted 100644 → 0
View file @ 68f22b98
---
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
View file @ 9e60bd9a
......@@ -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
View file @ 9e60bd9a
......@@ -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
View file @ 9e60bd9a
......@@ -1218,14 +1218,8 @@ 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
it "does not have errors" do
expect(subject.errors).to be_empty
end
end
......@@ -1238,36 +1232,20 @@ 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
it "returns an error" do
expect(subject.errors).to contain_exactly(
'rspec: circular variable reference detected: ["A", "B", "C"]')
end
context 'when FF :variable_inside_variable is enabled' do
before do
stub_feature_flags(variable_inside_variable: [project])
end
context 'with job:rules:[if:]' do
let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$C != null', when: 'always' }] } }
it "returns an error" do
expect(subject.errors).to contain_exactly(
'rspec: circular variable reference detected: ["A", "B", "C"]')
it "included? does not raise" do
expect { subject.included? }.not_to raise_error
end
context 'with job:rules:[if:]' do
let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$C != null', when: 'always' }] } }
it "included? does not raise" do
expect { subject.included? }.not_to raise_error
end
it "included? returns true" do
expect(subject.included?).to eq(true)
end
it "included? returns true" do
expect(subject.included?).to eq(true)
end
end
end
......
spec/lib/gitlab/ci/variables/collection_spec.rb
View file @ 9e60bd9a
......@@ -358,302 +358,212 @@ 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) }
let_it_be(:project) { create(:project) }
before do
stub_feature_flags(variable_inside_variable: [project_with_flag_enabled])
end
context 'table tests' do
using RSpec::Parameterized::TableSyntax
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
}
where do
{
"empty array": {
variables: [],
keep_undefined: false,
result: []
},
"simple expansions": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'key$variable$variable2' },
{ key: 'variable4', value: 'key$variable$variable3' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'keyvalueresult' },
{ key: 'variable4', value: 'keyvaluekeyvalueresult' }
]
},
"complex expansion": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'key${variable}' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'keyvalue' }
]
},
"unused variables": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result2' },
{ key: 'variable3', value: 'result3' },
{ key: 'variable4', value: 'key$variable$variable3' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result2' },
{ key: 'variable3', value: 'result3' },
{ key: 'variable4', value: 'keyvalueresult3' }
]
},
"complex expansions": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'key${variable}${variable2}' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'keyvalueresult' }
]
},
"escaped characters in complex expansions keeping undefined are kept intact": {
variables: [
{ key: 'variable3', value: 'key_${variable}_$${HOME}_%%HOME%%' },
{ key: 'variable', value: '$variable2' },
{ key: 'variable2', value: 'value' }
],
keep_undefined: true,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'value' },
{ key: 'variable3', value: 'key_value_$${HOME}_%%HOME%%' }
]
},
"escaped characters in complex expansions discarding undefined are kept intact": {
variables: [
{ key: 'variable2', value: 'key_${variable4}_$${HOME}_%%HOME%%' },
{ key: 'variable', value: 'value_$${HOME}_%%HOME%%' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value_$${HOME}_%%HOME%%' },
{ key: 'variable2', value: 'key__$${HOME}_%%HOME%%' }
]
},
"out-of-order expansion": {
variables: [
{ key: 'variable3', value: 'key$variable2$variable' },
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' }
],
keep_undefined: false,
result: [
{ key: 'variable2', value: 'result' },
{ key: 'variable', value: 'value' },
{ key: 'variable3', value: 'keyresultvalue' }
]
},
"out-of-order complex expansion": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'key${variable2}${variable}' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'keyresultvalue' }
]
},
"missing variable discarding original": {
variables: [
{ key: 'variable2', value: 'key$variable' }
],
keep_undefined: false,
result: [
{ key: 'variable2', value: 'key' }
]
},
"missing variable keeping original": {
variables: [
{ key: 'variable2', value: 'key$variable' }
],
keep_undefined: true,
result: [
{ key: 'variable2', value: 'key$variable' }
]
},
"complex expansions with missing variable keeping original": {
variables: [
{ key: 'variable4', value: 'key${variable}${variable2}${variable3}' },
{ key: 'variable', value: 'value' },
{ key: 'variable3', value: 'value3' }
],
keep_undefined: true,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable3', value: 'value3' },
{ key: 'variable4', value: 'keyvalue${variable2}value3' }
]
},
"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,
result: [
{ key: 'variable', value: '$variable2', raw: true },
{ key: 'variable2', value: 'value2' },
{ key: 'variable3', value: 'key_$variable2_value2' }
]
},
"variable value referencing password with special characters": {
variables: [
{ key: 'VAR', value: '$PASSWORD' },
{ key: 'PASSWORD', value: 'my_password$$_%%_$A' },
{ key: 'A', value: 'value' }
],
keep_undefined: false,
result: [
{ key: 'VAR', value: 'my_password$$_%%_value' },
{ key: 'PASSWORD', value: 'my_password$$_%%_value' },
{ key: 'A', value: 'value' }
]
},
"cyclic dependency causes original array to be returned": {
variables: [
{ key: 'variable', value: '$variable2' },
{ key: 'variable2', value: '$variable3' },
{ key: 'variable3', value: 'key$variable$variable2' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: '$variable2' },
{ key: 'variable2', value: '$variable3' },
{ key: 'variable3', value: 'key$variable$variable2' }
]
}
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) }
with_them do
let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
before do
stub_feature_flags(variable_inside_variable: [project_with_flag_enabled])
end
subject { collection.sort_and_expand_all(project, keep_undefined: keep_undefined) }
context 'table tests' do
using RSpec::Parameterized::TableSyntax
where do
{
"empty array": {
variables: [],
keep_undefined: false,
result: []
},
"simple expansions": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'key$variable$variable2' },
{ key: 'variable4', value: 'key$variable$variable3' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'keyvalueresult' },
{ key: 'variable4', value: 'keyvaluekeyvalueresult' }
]
},
"complex expansion": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'key${variable}' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'keyvalue' }
]
},
"unused variables": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result2' },
{ key: 'variable3', value: 'result3' },
{ key: 'variable4', value: 'key$variable$variable3' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result2' },
{ key: 'variable3', value: 'result3' },
{ key: 'variable4', value: 'keyvalueresult3' }
]
},
"complex expansions": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'key${variable}${variable2}' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'keyvalueresult' }
]
},
"escaped characters in complex expansions keeping undefined are kept intact": {
variables: [
{ key: 'variable3', value: 'key_${variable}_$${HOME}_%%HOME%%' },
{ key: 'variable', value: '$variable2' },
{ key: 'variable2', value: 'value' }
],
keep_undefined: true,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'value' },
{ key: 'variable3', value: 'key_value_$${HOME}_%%HOME%%' }
]
},
"escaped characters in complex expansions discarding undefined are kept intact": {
variables: [
{ key: 'variable2', value: 'key_${variable4}_$${HOME}_%%HOME%%' },
{ key: 'variable', value: 'value_$${HOME}_%%HOME%%' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value_$${HOME}_%%HOME%%' },
{ key: 'variable2', value: 'key__$${HOME}_%%HOME%%' }
]
},
"out-of-order expansion": {
variables: [
{ key: 'variable3', value: 'key$variable2$variable' },
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' }
],
keep_undefined: false,
result: [
{ key: 'variable2', value: 'result' },
{ key: 'variable', value: 'value' },
{ key: 'variable3', value: 'keyresultvalue' }
]
},
"out-of-order complex expansion": {
variables: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'key${variable2}${variable}' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable2', value: 'result' },
{ key: 'variable3', value: 'keyresultvalue' }
]
},
"missing variable discarding original": {
variables: [
{ key: 'variable2', value: 'key$variable' }
],
keep_undefined: false,
result: [
{ key: 'variable2', value: 'key' }
]
},
"missing variable keeping original": {
variables: [
{ key: 'variable2', value: 'key$variable' }
],
keep_undefined: true,
result: [
{ key: 'variable2', value: 'key$variable' }
]
},
"complex expansions with missing variable keeping original": {
variables: [
{ key: 'variable4', value: 'key${variable}${variable2}${variable3}' },
{ key: 'variable', value: 'value' },
{ key: 'variable3', value: 'value3' }
],
keep_undefined: true,
result: [
{ key: 'variable', value: 'value' },
{ key: 'variable3', value: 'value3' },
{ key: 'variable4', value: 'keyvalue${variable2}value3' }
]
},
"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,
result: [
{ key: 'variable', value: '$variable2', raw: true },
{ key: 'variable2', value: 'value2' },
{ key: 'variable3', value: 'key_$variable2_value2' }
]
},
"variable value referencing password with special characters": {
variables: [
{ key: 'VAR', value: '$PASSWORD' },
{ key: 'PASSWORD', value: 'my_password$$_%%_$A' },
{ key: 'A', value: 'value' }
],
keep_undefined: false,
result: [
{ key: 'VAR', value: 'my_password$$_%%_value' },
{ key: 'PASSWORD', value: 'my_password$$_%%_value' },
{ key: 'A', value: 'value' }
]
},
"cyclic dependency causes original array to be returned": {
variables: [
{ key: 'variable', value: '$variable2' },
{ key: 'variable2', value: '$variable3' },
{ key: 'variable3', value: 'key$variable$variable2' }
],
keep_undefined: false,
result: [
{ key: 'variable', value: '$variable2' },
{ key: 'variable2', value: '$variable3' },
{ key: 'variable3', value: 'key$variable$variable2' }
]
}
}
it 'returns Collection' do
is_expected.to be_an_instance_of(Gitlab::Ci::Variables::Collection)
end
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) }
it 'returns Collection' do
is_expected.to be_an_instance_of(Gitlab::Ci::Variables::Collection)
end
it 'expands variables' do
var_hash = result.to_h { |env| [env.fetch(:key), env.fetch(:value)] }
.with_indifferent_access
expect(subject.to_hash).to eq(var_hash)
end
it 'expands variables' do
var_hash = result.to_h { |env| [env.fetch(:key), env.fetch(:value)] }
.with_indifferent_access
expect(subject.to_hash).to eq(var_hash)
end
it 'preserves raw attribute' do
expect(subject.pluck(:key, :raw).to_h).to eq(collection.pluck(:key, :raw).to_h)
end
it 'preserves raw attribute' do
expect(subject.pluck(:key, :raw).to_h).to eq(collection.pluck(:key, :raw).to_h)
end
end
end
......
spec/presenters/ci/build_runner_presenter_spec.rb
View file @ 9e60bd9a
......@@ -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,21 +271,11 @@ 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
let(:sha) { project.repository.commit.sha }
let(:pipeline) { create(:ci_pipeline, sha: sha, project: project) }
let(:build) { create(:ci_build, pipeline: pipeline) }
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_behaves_like 'returns an array with the expected variables'
end
it_behaves_like 'returns an array with the expected variables'
end
describe '#runner_variables subset' do
......@@ -305,32 +290,12 @@ 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 },
{ key: 'B', value: 'refB-value-$D', public: false, masked: false },
{ key: 'A', value: 'refA-refB-value-$D', public: false, masked: false }
]
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
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7