summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/ci/build.rb1
-rw-r--r--app/models/ci/pipeline.rb7
-rw-r--r--changelogs/unreleased/fix-gb-exclude-persisted-variables-from-environment-name.yml5
-rw-r--r--doc/ci/environments.md1
-rw-r--r--doc/ci/variables/README.md1
-rw-r--r--spec/models/ci/build_spec.rb10
-rw-r--r--spec/models/ci/pipeline_spec.rb28
-rw-r--r--spec/services/ci/create_pipeline_service_spec.rb22
8 files changed, 71 insertions, 4 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 1c42ed4d3e5..495430931aa 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -599,6 +599,7 @@ module Ci
break variables unless persisted?
variables
+ .concat(pipeline.persisted_variables)
.append(key: 'CI_JOB_ID', value: id.to_s)
.append(key: 'CI_JOB_TOKEN', value: token, public: false)
.append(key: 'CI_BUILD_ID', value: id.to_s)
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index c26f0b6dcdc..53af87a271a 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -523,9 +523,14 @@ module Ci
strong_memoize(:legacy_trigger) { trigger_requests.first }
end
+ def persisted_variables
+ Gitlab::Ci::Variables::Collection.new.tap do |variables|
+ variables.append(key: 'CI_PIPELINE_ID', value: id.to_s) if persisted?
+ end
+ end
+
def predefined_variables
Gitlab::Ci::Variables::Collection.new
- .append(key: 'CI_PIPELINE_ID', value: id.to_s)
.append(key: 'CI_CONFIG_PATH', value: ci_yaml_file_path)
.append(key: 'CI_PIPELINE_SOURCE', value: source.to_s)
.append(key: 'CI_COMMIT_MESSAGE', value: git_commit_message)
diff --git a/changelogs/unreleased/fix-gb-exclude-persisted-variables-from-environment-name.yml b/changelogs/unreleased/fix-gb-exclude-persisted-variables-from-environment-name.yml
new file mode 100644
index 00000000000..92426832f30
--- /dev/null
+++ b/changelogs/unreleased/fix-gb-exclude-persisted-variables-from-environment-name.yml
@@ -0,0 +1,5 @@
+---
+title: Exclude CI_PIPELINE_ID from variables supported in dynamic environment name
+merge_request: 19032
+author:
+type: fixed
diff --git a/doc/ci/environments.md b/doc/ci/environments.md
index 517e25f00f7..3a491f0073c 100644
--- a/doc/ci/environments.md
+++ b/doc/ci/environments.md
@@ -252,6 +252,7 @@ including predefined, secure variables and `.gitlab-ci.yml`
[`variables`](yaml/README.md#variables). You however cannot use variables
defined under `script` or on the Runner's side. There are other variables that
are unsupported in environment name context:
+- `CI_PIPELINE_ID`
- `CI_JOB_ID`
- `CI_JOB_TOKEN`
- `CI_BUILD_ID`
diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index f66b2b374ba..aedf7958c8a 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -553,6 +553,7 @@ We do not support variables containing tokens because of security reasons.
You can find a full list of unsupported variables below:
+- `CI_PIPELINE_ID`
- `CI_JOB_ID`
- `CI_JOB_TOKEN`
- `CI_BUILD_ID`
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index af5f5047803..96173889ccd 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -629,6 +629,14 @@ describe Ci::Build do
it { is_expected.to eq('review/host') }
end
+
+ context 'when using persisted variables' do
+ let(:build) do
+ create(:ci_build, environment: 'review/x$CI_BUILD_ID')
+ end
+
+ it { is_expected.to eq('review/x') }
+ end
end
describe '#starts_environment?' do
@@ -1525,6 +1533,7 @@ describe Ci::Build do
let(:container_registry_enabled) { false }
let(:predefined_variables) do
[
+ { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true },
{ key: 'CI_JOB_ID', value: build.id.to_s, public: true },
{ key: 'CI_JOB_TOKEN', value: build.token, public: false },
{ key: 'CI_BUILD_ID', value: build.id.to_s, public: true },
@@ -1556,7 +1565,6 @@ describe Ci::Build do
{ key: 'CI_PROJECT_NAMESPACE', value: project.namespace.full_path, public: true },
{ key: 'CI_PROJECT_URL', value: project.web_url, public: true },
{ key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true },
- { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true },
{ key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true },
{ key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true },
{ key: 'CI_COMMIT_MESSAGE', value: pipeline.git_commit_message, public: true },
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index e7845b693a1..e4f4c62bd22 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -167,13 +167,39 @@ describe Ci::Pipeline, :mailer do
end
end
+ describe '#persisted_variables' do
+ context 'when pipeline is not persisted yet' do
+ subject { build(:ci_pipeline).persisted_variables }
+
+ it 'does not contain some variables' do
+ keys = subject.map { |variable| variable[:key] }
+
+ expect(keys).not_to include 'CI_PIPELINE_ID'
+ end
+ end
+
+ context 'when pipeline is persisted' do
+ subject { build_stubbed(:ci_pipeline).persisted_variables }
+
+ it 'does contains persisted variables' do
+ keys = subject.map { |variable| variable[:key] }
+
+ expect(keys).to eq %w[CI_PIPELINE_ID]
+ end
+ end
+ end
+
describe '#predefined_variables' do
subject { pipeline.predefined_variables }
it 'includes all predefined variables in a valid order' do
keys = subject.map { |variable| variable[:key] }
- expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE CI_COMMIT_MESSAGE CI_COMMIT_TITLE CI_COMMIT_DESCRIPTION]
+ expect(keys).to eq %w[CI_CONFIG_PATH
+ CI_PIPELINE_SOURCE
+ CI_COMMIT_MESSAGE
+ CI_COMMIT_TITLE
+ CI_COMMIT_DESCRIPTION]
end
end
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 9a0b6efd8a9..2b88fcc9a96 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -395,7 +395,27 @@ describe Ci::CreatePipelineService do
result = execute_service
expect(result).to be_persisted
- expect(Environment.find_by(name: "review/master")).not_to be_nil
+ expect(Environment.find_by(name: "review/master")).to be_present
+ end
+ end
+
+ context 'with environment name including persisted variables' do
+ before do
+ config = YAML.dump(
+ deploy: {
+ environment: { name: "review/id1$CI_PIPELINE_ID/id2$CI_BUILD_ID" },
+ script: 'ls'
+ }
+ )
+
+ stub_ci_pipeline_yaml_file(config)
+ end
+
+ it 'skipps persisted variables in environment name' do
+ result = execute_service
+
+ expect(result).to be_persisted
+ expect(Environment.find_by(name: "review/id1/id2")).to be_present
end
end
View Lorry Controller Status