summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-07-06 13:11:07 +0000
committerDouwe Maan <douwe@gitlab.com>2017-07-06 13:11:07 +0000
commitec3b10e078de03ac32ef85872d3d1518ef8526d5 (patch)
tree032fc483f456228b19587cafbce8d31c9ab5603a
parent1bdd295394d115cb88a9b6be7f44851c6e58a491 (diff)
parentd9435d61218f677395f3b53976a41ac5f361f24b (diff)
downloadgitlab-ce-ec3b10e078de03ac32ef85872d3d1518ef8526d5.tar.gz
Merge branch '34716-environment-specific-variables-ce' into 'master'
Backports for ee-2112 Closes #34716 See merge request !12671
-rw-r--r--app/controllers/projects/variables_controller.rb15
-rw-r--r--app/models/ci/build.rb21
-rw-r--r--app/models/project.rb3
-rw-r--r--doc/ci/variables/README.md11
-rw-r--r--doc/downgrade_ee_to_ce/README.md13
-rw-r--r--lib/api/variables.rb8
-rw-r--r--lib/gitlab/regex.rb6
-rw-r--r--lib/gitlab/sql/glob.rb22
-rw-r--r--spec/lib/gitlab/sql/glob_spec.rb53
-rw-r--r--spec/models/ci/build_spec.rb7
-rw-r--r--spec/models/ci/variable_spec.rb4
-rw-r--r--spec/models/project_spec.rb12
12 files changed, 140 insertions, 35 deletions
diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb
index 573d1c05622..326d31ecec2 100644
--- a/app/controllers/projects/variables_controller.rb
+++ b/app/controllers/projects/variables_controller.rb
@@ -14,7 +14,7 @@ class Projects::VariablesController < Projects::ApplicationController
def update
@variable = @project.variables.find(params[:id])
- if @variable.update_attributes(project_params)
+ if @variable.update_attributes(variable_params)
redirect_to project_variables_path(project), notice: 'Variable was successfully updated.'
else
render action: "show"
@@ -22,9 +22,9 @@ class Projects::VariablesController < Projects::ApplicationController
end
def create
- @variable = Ci::Variable.new(project_params)
+ @variable = @project.variables.new(variable_params)
- if @variable.valid? && @project.variables << @variable
+ if @variable.save
flash[:notice] = 'Variables were successfully updated.'
redirect_to project_settings_ci_cd_path(project)
else
@@ -43,8 +43,11 @@ class Projects::VariablesController < Projects::ApplicationController
private
- def project_params
- params.require(:variable)
- .permit([:id, :key, :value, :protected, :_destroy])
+ def variable_params
+ params.require(:variable).permit(*variable_params_attributes)
+ end
+
+ def variable_params_attributes
+ %i[id key value protected _destroy]
end
end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 5fb4c7e3cb3..48586ba8910 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -186,6 +186,12 @@ module Ci
# Variables whose value does not depend on environment
def simple_variables
+ variables(environment: nil)
+ end
+
+ # All variables, including those dependent on environment, which could
+ # contain unexpanded variables.
+ def variables(environment: persisted_environment)
variables = predefined_variables
variables += project.predefined_variables
variables += pipeline.predefined_variables
@@ -194,15 +200,11 @@ module Ci
variables += project.deployment_variables if has_environment?
variables += yaml_variables
variables += user_variables
- variables += project.secret_variables_for(ref).map(&:to_runner_variable)
+ variables += secret_variables(environment: environment)
variables += trigger_request.user_variables if trigger_request
- variables
- end
+ variables += persisted_environment_variables if environment
- # All variables, including those dependent on environment, which could
- # contain unexpanded variables.
- def variables
- simple_variables.concat(persisted_environment_variables)
+ variables
end
def merge_request
@@ -370,6 +372,11 @@ module Ci
]
end
+ def secret_variables(environment: persisted_environment)
+ project.secret_variables_for(ref: ref, environment: environment)
+ .map(&:to_runner_variable)
+ end
+
def steps
[Gitlab::Ci::Build::Step.from_commands(self),
Gitlab::Ci::Build::Step.from_after_script(self)].compact
diff --git a/app/models/project.rb b/app/models/project.rb
index 5c673fdb7ba..74c15d2508b 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1345,7 +1345,8 @@ class Project < ActiveRecord::Base
variables
end
- def secret_variables_for(ref)
+ def secret_variables_for(ref:, environment: nil)
+ # EE would use the environment
if protected_for?(ref)
variables
else
diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index ee23ac0adbe..3501aae75ec 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -160,7 +160,7 @@ Secret variables can be added by going to your project's
Once you set them, they will be available for all subsequent pipelines.
-## Protected secret variables
+### Protected secret variables
>**Notes:**
This feature requires GitLab 9.3 or higher.
@@ -426,10 +426,11 @@ export CI_REGISTRY_PASSWORD="longalfanumstring"
```
[ce-13784]: https://gitlab.com/gitlab-org/gitlab-ce/issues/13784
-[runner]: https://docs.gitlab.com/runner/
-[triggered]: ../triggers/README.md
-[triggers]: ../triggers/README.md#pass-job-variables-to-a-trigger
+[eep]: https://about.gitlab.com/gitlab-ee/ "Available only in GitLab Enterprise Edition Premium"
+[envs]: ../environments.md
[protected branches]: ../../user/project/protected_branches.md
[protected tags]: ../../user/project/protected_tags.md
+[runner]: https://docs.gitlab.com/runner/
[shellexecutors]: https://docs.gitlab.com/runner/executors/
-[eep]: https://about.gitlab.com/gitlab-ee/ "Available only in GitLab Enterprise Edition Premium"
+[triggered]: ../triggers/README.md
+[triggers]: ../triggers/README.md#pass-job-variables-to-a-trigger
diff --git a/doc/downgrade_ee_to_ce/README.md b/doc/downgrade_ee_to_ce/README.md
index fe4b6d73771..75bae324585 100644
--- a/doc/downgrade_ee_to_ce/README.md
+++ b/doc/downgrade_ee_to_ce/README.md
@@ -46,6 +46,19 @@ $ sudo gitlab-rails runner "Service.where(type: ['JenkinsService', 'JenkinsDepre
$ bundle exec rails runner "Service.where(type: ['JenkinsService', 'JenkinsDeprecatedService']).delete_all" production
```
+### Secret variables environment scopes
+
+If you're using this feature and there are variables sharing the same
+key, but they have different scopes in a project, then you might want to
+revisit the environment scope setting for those variables.
+
+In CE, environment scopes are completely ignored, therefore you could
+accidentally get a variable which you're not expecting for a particular
+environment. Make sure that you have the right variables in this case.
+
+Data is completely preserved, so you could always upgrade back to EE and
+restore the behavior if you leave it alone.
+
## Downgrade to CE
After performing the above mentioned steps, you are now ready to downgrade your
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index 10374995497..7fa528fb2d3 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -45,7 +45,9 @@ module API
optional :protected, type: String, desc: 'Whether the variable is protected'
end
post ':id/variables' do
- variable = user_project.variables.create(declared_params(include_missing: false))
+ variable_params = declared_params(include_missing: false)
+
+ variable = user_project.variables.create(variable_params)
if variable.valid?
present variable, with: Entities::Variable
@@ -67,7 +69,9 @@ module API
return not_found!('Variable') unless variable
- if variable.update(declared_params(include_missing: false).except(:key))
+ variable_params = declared_params(include_missing: false).except(:key)
+
+ if variable.update(variable_params)
present variable, with: Entities::Variable
else
render_validation_error!(variable)
diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb
index 057f32eaef7..c1ee20b6977 100644
--- a/lib/gitlab/regex.rb
+++ b/lib/gitlab/regex.rb
@@ -30,8 +30,12 @@ module Gitlab
@container_repository_regex ||= %r{\A[a-z0-9]+(?:[-._/][a-z0-9]+)*\Z}
end
+ def environment_name_regex_chars
+ 'a-zA-Z0-9_/\\$\\{\\}\\. -'
+ end
+
def environment_name_regex
- @environment_name_regex ||= /\A[a-zA-Z0-9_\\\/\${}. -]+\z/.freeze
+ @environment_name_regex ||= /\A[#{environment_name_regex_chars}]+\z/.freeze
end
def environment_name_regex_message
diff --git a/lib/gitlab/sql/glob.rb b/lib/gitlab/sql/glob.rb
new file mode 100644
index 00000000000..5e89e12b2b1
--- /dev/null
+++ b/lib/gitlab/sql/glob.rb
@@ -0,0 +1,22 @@
+module Gitlab
+ module SQL
+ module Glob
+ extend self
+
+ # Convert a simple glob pattern with wildcard (*) to SQL LIKE pattern
+ # with SQL expression
+ def to_like(pattern)
+ <<~SQL
+ REPLACE(REPLACE(REPLACE(#{pattern},
+ #{q('%')}, #{q('\\%')}),
+ #{q('_')}, #{q('\\_')}),
+ #{q('*')}, #{q('%')})
+ SQL
+ end
+
+ def q(string)
+ ActiveRecord::Base.connection.quote(string)
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/sql/glob_spec.rb b/spec/lib/gitlab/sql/glob_spec.rb
new file mode 100644
index 00000000000..451c583310d
--- /dev/null
+++ b/spec/lib/gitlab/sql/glob_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe Gitlab::SQL::Glob, lib: true do
+ describe '.to_like' do
+ it 'matches * as %' do
+ expect(glob('apple', '*')).to be(true)
+ expect(glob('apple', 'app*')).to be(true)
+ expect(glob('apple', 'apple*')).to be(true)
+ expect(glob('apple', '*pple')).to be(true)
+ expect(glob('apple', 'ap*le')).to be(true)
+
+ expect(glob('apple', '*a')).to be(false)
+ expect(glob('apple', 'app*a')).to be(false)
+ expect(glob('apple', 'ap*l')).to be(false)
+ end
+
+ it 'matches % literally' do
+ expect(glob('100%', '100%')).to be(true)
+
+ expect(glob('100%', '%')).to be(false)
+ end
+
+ it 'matches _ literally' do
+ expect(glob('^_^', '^_^')).to be(true)
+
+ expect(glob('^A^', '^_^')).to be(false)
+ end
+ end
+
+ def glob(string, pattern)
+ match(string, subject.to_like(quote(pattern)))
+ end
+
+ def match(string, pattern)
+ value = query("SELECT #{quote(string)} LIKE #{pattern}")
+ .rows.flatten.first
+
+ case value
+ when 't', 1
+ true
+ else
+ false
+ end
+ end
+
+ def query(sql)
+ ActiveRecord::Base.connection.select_all(sql)
+ end
+
+ def quote(string)
+ ActiveRecord::Base.connection.quote(string)
+ end
+end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index a7ba3a7c43e..2b10791ad6d 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1496,9 +1496,10 @@ describe Ci::Build, :models do
allow(pipeline).to receive(:predefined_variables) { [pipeline_pre_var] }
allow(build).to receive(:yaml_variables) { [build_yaml_var] }
- allow(project).to receive(:secret_variables_for).with(build.ref) do
- [create(:ci_variable, key: 'secret', value: 'value')]
- end
+ allow(project).to receive(:secret_variables_for)
+ .with(ref: 'master', environment: nil) do
+ [create(:ci_variable, key: 'secret', value: 'value')]
+ end
end
it do
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index 50f7c029af8..4ffbfa6c130 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -8,10 +8,6 @@ describe Ci::Variable, models: true do
describe 'validations' do
it { is_expected.to include_module(HasVariable) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }
- it { is_expected.to validate_length_of(:key).is_at_most(255) }
- it { is_expected.to allow_value('foo').for(:key) }
- it { is_expected.not_to allow_value('foo bar').for(:key) }
- it { is_expected.not_to allow_value('foo/bar').for(:key) }
end
describe '.unprotected' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 75fa2d2eb46..99bfab70088 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1875,7 +1875,12 @@ describe Project, models: true do
create(:ci_variable, :protected, value: 'protected', project: project)
end
- subject { project.secret_variables_for('ref') }
+ subject { project.secret_variables_for(ref: 'ref') }
+
+ before do
+ stub_application_setting(
+ default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+ end
shared_examples 'ref is protected' do
it 'contains all the variables' do
@@ -1884,11 +1889,6 @@ describe Project, models: true do
end
context 'when the ref is not protected' do
- before do
- stub_application_setting(
- default_branch_protection: Gitlab::Access::PROTECTION_NONE)
- end
-
it 'contains only the secret variables' do
is_expected.to contain_exactly(secret_variable)
end