diff options
author | Hordur Freyr Yngvason <hfyngvason@gitlab.com> | 2019-08-08 18:51:52 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2019-08-08 18:51:52 +0000 |
commit | 5f82ff1469510b4e51d531775a44e4bea92254fe (patch) | |
tree | 2762023eb50a91cabb54f8b454db49c147f447d4 | |
parent | 455d16d1bfd59000391a64f41ab86d5a847f008a (diff) | |
download | gitlab-ce-5f82ff1469510b4e51d531775a44e4bea92254fe.tar.gz |
Bring scoped environment variables to core
As decided in https://gitlab.com/gitlab-org/gitlab-ce/issues/53593
31 files changed, 494 insertions, 38 deletions
diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb index 646728e8167..1dffc57fcf0 100644 --- a/app/controllers/projects/variables_controller.rb +++ b/app/controllers/projects/variables_controller.rb @@ -38,6 +38,6 @@ class Projects::VariablesController < Projects::ApplicationController end def variable_params_attributes - %i[id variable_type key secret_value protected masked _destroy] + %i[id variable_type key secret_value protected masked environment_scope _destroy] end end diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index a77bbef0fca..760872d3e6b 100644 --- a/app/models/ci/variable.rb +++ b/app/models/ci/variable.rb @@ -6,6 +6,7 @@ module Ci include HasVariable include Presentable include Maskable + prepend HasEnvironmentScope belongs_to :project diff --git a/app/models/concerns/has_environment_scope.rb b/app/models/concerns/has_environment_scope.rb new file mode 100644 index 00000000000..9553abe4dd3 --- /dev/null +++ b/app/models/concerns/has_environment_scope.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module HasEnvironmentScope + extend ActiveSupport::Concern + + prepended do + validates( + :environment_scope, + presence: true, + format: { with: ::Gitlab::Regex.environment_scope_regex, + message: ::Gitlab::Regex.environment_scope_regex_message } + ) + + ## + # Select rows which have a scope that matches the given environment name. + # Rows are ordered by relevance, by default. The most relevant row is + # placed at the end of a list. + # + # options: + # - relevant_only: (boolean) + # You can get the most relevant row only. Other rows are not be + # selected even if its scope matches the environment name. + # This is equivalent to using `#last` from SQL standpoint. + # + scope :on_environment, -> (environment_name, relevant_only: false) do + order_direction = relevant_only ? 'DESC' : 'ASC' + + where = <<~SQL + environment_scope IN (:wildcard, :environment_name) OR + :environment_name LIKE + #{::Gitlab::SQL::Glob.to_like('environment_scope')} + SQL + + order = <<~SQL + CASE environment_scope + WHEN :wildcard THEN 0 + WHEN :environment_name THEN 2 + ELSE 1 + END #{order_direction} + SQL + + values = { + wildcard: '*', + environment_name: environment_name + } + + sanitized_order_sql = sanitize_sql_array([order, values]) + + # The query is trying to find variables with scopes matching the + # current environment name. Suppose the environment name is + # 'review/app', and we have variables with environment scopes like: + # * variable A: review + # * variable B: review/app + # * variable C: review/* + # * variable D: * + # And the query should find variable B, C, and D, because it would + # try to convert the scope into a LIKE pattern for each variable: + # * A: review + # * B: review/app + # * C: review/% + # * D: % + # Note that we'll match % and _ literally therefore we'll escape them. + # In this case, B, C, and D would match. We also want to prioritize + # the exact matched name, and put * last, and everything else in the + # middle. So the order should be: D < C < B + relation = where(where, values) + .order(Arel.sql(sanitized_order_sql)) # `order` cannot escape for us! + + relation = relation.limit(1) if relevant_only + + relation + end + end + + def environment_scope=(new_environment_scope) + super(new_environment_scope.to_s.strip) + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 960795b73cb..7a5e980b783 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1828,11 +1828,16 @@ class Project < ApplicationRecord end def ci_variables_for(ref:, environment: nil) - # EE would use the environment - if protected_for?(ref) - variables + result = if protected_for?(ref) + variables + else + variables.unprotected + end + + if environment + result.on_environment(environment) else - variables.unprotected + result.where(environment_scope: '*') end end diff --git a/app/serializers/variable_entity.rb b/app/serializers/variable_entity.rb index 4d48e13cfca..8b19925f153 100644 --- a/app/serializers/variable_entity.rb +++ b/app/serializers/variable_entity.rb @@ -7,4 +7,5 @@ class VariableEntity < Grape::Entity expose :protected?, as: :protected expose :masked?, as: :masked + expose :environment_scope end diff --git a/app/views/ci/variables/_environment_scope.html.haml b/app/views/ci/variables/_environment_scope.html.haml new file mode 100644 index 00000000000..15e61d85881 --- /dev/null +++ b/app/views/ci/variables/_environment_scope.html.haml @@ -0,0 +1,21 @@ +- form_field = local_assigns.fetch(:form_field, nil) +- variable = local_assigns.fetch(:variable, nil) + +- if @project + - environment_scope = variable&.environment_scope || '*' + - environment_scope_label = environment_scope == '*' ? s_('CiVariable|All environments') : environment_scope + + %input{ type: "hidden", name: "#{form_field}[variables_attributes][][environment_scope]", value: environment_scope } + = dropdown_tag(environment_scope_label, + options: { wrapper_class: 'ci-variable-body-item js-variable-environment-dropdown-wrapper', + toggle_class: 'js-variable-environment-toggle wide', + filter: true, + dropdown_class: "dropdown-menu-selectable", + placeholder: s_('CiVariable|Search environments'), + footer_content: true, + data: { selected: environment_scope } }) do + %ul.dropdown-footer-list + %li + %button{ class: "dropdown-create-new-item-button js-dropdown-create-new-item", title: s_('CiVariable|New environment') } + = s_('CiVariable|Create wildcard') + %code diff --git a/app/views/ci/variables/_environment_scope_header.html.haml b/app/views/ci/variables/_environment_scope_header.html.haml new file mode 100644 index 00000000000..4ba4ceec16c --- /dev/null +++ b/app/views/ci/variables/_environment_scope_header.html.haml @@ -0,0 +1,2 @@ +.bold.table-section.section-15.append-right-10 + = s_('CiVariables|Scope') diff --git a/changelogs/unreleased/bring-scoped-environment-variables-to-core.yml b/changelogs/unreleased/bring-scoped-environment-variables-to-core.yml new file mode 100644 index 00000000000..48dfe662206 --- /dev/null +++ b/changelogs/unreleased/bring-scoped-environment-variables-to-core.yml @@ -0,0 +1,5 @@ +--- +title: Bring scoped environment variables to core +merge_request: 30779 +author: +type: changed diff --git a/doc/README.md b/doc/README.md index ebfa45122cc..edce03baec9 100644 --- a/doc/README.md +++ b/doc/README.md @@ -279,7 +279,7 @@ The following documentation relates to the DevOps **Release** stage: | [Canary Deployments](user/project/canary_deployments.md) **(PREMIUM)** | Employ a popular CI strategy where a small portion of the fleet is updated to the new version first. | | [Deploy Boards](user/project/deploy_boards.md) **(PREMIUM)** | View the current health and status of each CI environment running on Kubernetes, displaying the status of the pods in the deployment. | | [Environments and deployments](ci/environments.md) | With environments, you can control the continuous deployment of your software within GitLab. | -| [Environment-specific variables](ci/variables/README.md#limiting-environment-scopes-of-environment-variables-premium) **(PREMIUM)** | Limit scope of variables to specific environments. | +| [Environment-specific variables](ci/variables/README.md#limiting-environment-scopes-of-environment-variables) | Limit scope of variables to specific environments. | | [GitLab CI/CD](ci/README.md) | Explore the features and capabilities of Continuous Deployment and Delivery with GitLab. | | [GitLab Pages](user/project/pages/index.md) | Build, test, and deploy a static site directly from GitLab. | | [Protected Runners](ci/runners/README.md#protected-runners) | Select Runners to only pick jobs for protected branches and tags. | diff --git a/doc/api/project_level_variables.md b/doc/api/project_level_variables.md index eab905bbc5f..591911bb8ec 100644 --- a/doc/api/project_level_variables.md +++ b/doc/api/project_level_variables.md @@ -74,7 +74,7 @@ POST /projects/:id/variables | `variable_type` | string | no | The type of a variable. Available types are: `env_var` (default) and `file` | | `protected` | boolean | no | Whether the variable is protected | | `masked` | boolean | no | Whether the variable is masked | -| `environment_scope` | string | no | The `environment_scope` of the variable **(PREMIUM)** | +| `environment_scope` | string | no | The `environment_scope` of the variable | ``` curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/variables" --form "key=NEW_VARIABLE" --form "value=new value" @@ -108,7 +108,7 @@ PUT /projects/:id/variables/:key | `variable_type` | string | no | The type of a variable. Available types are: `env_var` (default) and `file` | | `protected` | boolean | no | Whether the variable is protected | | `masked` | boolean | no | Whether the variable is masked | -| `environment_scope` | string | no | The `environment_scope` of the variable **(PREMIUM)** | +| `environment_scope` | string | no | The `environment_scope` of the variable | ``` curl --request PUT --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/variables/NEW_VARIABLE" --form "value=updated value" diff --git a/doc/ci/environments.md b/doc/ci/environments.md index f86ca8f74f2..61559e69182 100644 --- a/doc/ci/environments.md +++ b/doc/ci/environments.md @@ -692,7 +692,7 @@ with `review/` would have that particular variable. Some GitLab features can behave differently for each environment. For example, you can -[create a secret variable to be injected only into a production environment](variables/README.md#limiting-environment-scopes-of-environment-variables-premium). **(PREMIUM)** +[create a secret variable to be injected only into a production environment](variables/README.md#limiting-environment-scopes-of-environment-variables). In most cases, these features use the _environment specs_ mechanism, which offers an efficient way to implement scoping within each environment group. diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index 01edd873a8d..d741482b662 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -393,7 +393,7 @@ Protected variables can be added by going to your project's Once you set them, they will be available for all subsequent pipelines. -### Limiting environment scopes of environment variables **(PREMIUM)** +### Limiting environment scopes of environment variables You can limit the environment scope of a variable by [defining which environments][envs] it can be available for. diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 63a72f8f193..95220d6364c 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -190,7 +190,7 @@ Those environments are tied to jobs that use [Auto Deploy](#auto-deploy), so except for the environment scope, they would also need to have a different domain they would be deployed to. This is why you need to define a separate `KUBE_INGRESS_BASE_DOMAIN` variable for all the above -[based on the environment](../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables-premium). +[based on the environment](../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables). The following table is an example of how the three different clusters would be configured. @@ -662,10 +662,10 @@ repo or by specifying a project variable: You can also make use of the `HELM_UPGRADE_EXTRA_ARGS` environment variable to override the default values in the `values.yaml` file in the [default Helm chart](https://gitlab.com/gitlab-org/charts/auto-deploy-app). To apply your own `values.yaml` file to all Helm upgrade commands in Auto Deploy set `HELM_UPGRADE_EXTRA_ARGS` to `--values my-values.yaml`. -### Custom Helm chart per environment **(PREMIUM)** +### Custom Helm chart per environment You can specify the use of a custom Helm chart per environment by scoping the environment variable -to the desired environment. See [Limiting environment scopes of variables](../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables-premium). +to the desired environment. See [Limiting environment scopes of variables](../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables). ### Customizing `.gitlab-ci.yml` diff --git a/doc/user/group/clusters/index.md b/doc/user/group/clusters/index.md index 625c5440ec0..d41f44f85cc 100644 --- a/doc/user/group/clusters/index.md +++ b/doc/user/group/clusters/index.md @@ -86,7 +86,7 @@ The domain should have a wildcard DNS configured to the Ingress IP address. When adding more than one Kubernetes cluster to your project, you need to differentiate them with an environment scope. The environment scope associates clusters with [environments](../../../ci/environments.md) similar to how the -[environment-specific variables](../../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables-premium) +[environment-specific variables](../../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables) work. While evaluating which environment matches the environment scope of a diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 7dfd0d04637..a1c5148a2dd 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -468,7 +468,7 @@ If you don't want to use GitLab Runner in privileged mode, either: When adding more than one Kubernetes cluster to your project, you need to differentiate them with an environment scope. The environment scope associates clusters with [environments](../../../ci/environments.md) similar to how the -[environment-specific variables](../../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables-premium) work. +[environment-specific variables](../../../ci/variables/README.md#limiting-environment-scopes-of-environment-variables) work. The default environment scope is `*`, which means all jobs, regardless of their environment, will use that cluster. Each scope can only be used by a single diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 643b53f5e63..1496b5c5f9e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1346,6 +1346,7 @@ module API expose :variable_type, :key, :value expose :protected?, as: :protected, if: -> (entity, _) { entity.respond_to?(:protected?) } expose :masked?, as: :masked, if: -> (entity, _) { entity.respond_to?(:masked?) } + expose :environment_scope, if: -> (entity, _) { entity.respond_to?(:environment_scope) } end class Pipeline < PipelineBasic diff --git a/lib/api/helpers/variables_helpers.rb b/lib/api/helpers/variables_helpers.rb deleted file mode 100644 index 78a92d0f5a6..00000000000 --- a/lib/api/helpers/variables_helpers.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module API - module Helpers - module VariablesHelpers - extend ActiveSupport::Concern - extend Grape::API::Helpers - - params :optional_params_ee do - end - end - end -end diff --git a/lib/api/variables.rb b/lib/api/variables.rb index af1d7936556..f022b9e665a 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -7,8 +7,6 @@ module API before { authenticate! } before { authorize! :admin_build, user_project } - helpers Helpers::VariablesHelpers - helpers do def filter_variable_parameters(params) # This method exists so that EE can more easily filter out certain @@ -59,8 +57,7 @@ module API optional :protected, type: Boolean, desc: 'Whether the variable is protected' optional :masked, type: Boolean, desc: 'Whether the variable is masked' optional :variable_type, type: String, values: Ci::Variable.variable_types.keys, desc: 'The type of variable, must be one of env_var or file. Defaults to env_var' - - use :optional_params_ee + optional :environment_scope, type: String, desc: 'The environment_scope of the variable' end post ':id/variables' do variable_params = declared_params(include_missing: false) @@ -84,8 +81,7 @@ module API optional :protected, type: Boolean, desc: 'Whether the variable is protected' optional :masked, type: Boolean, desc: 'Whether the variable is masked' optional :variable_type, type: String, values: Ci::Variable.variable_types.keys, desc: 'The type of variable, must be one of env_var or file' - - use :optional_params_ee + optional :environment_scope, type: String, desc: 'The environment_scope of the variable' end # rubocop: disable CodeReuse/ActiveRecord put ':id/variables/:key' do diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 21614ea003e..e6372a42dda 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -46,6 +46,18 @@ module Gitlab "can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.', and spaces, but it cannot start or end with '/'" end + def environment_scope_regex_chars + "#{environment_name_regex_chars}\\*" + end + + def environment_scope_regex + @environment_scope_regex ||= /\A[#{environment_scope_regex_chars}]+\z/.freeze + end + + def environment_scope_regex_message + "can contain only letters, digits, '-', '_', '/', '$', '{', '}', '.', '*' and spaces" + end + def kubernetes_namespace_regex /\A[a-z0-9]([-a-z0-9]*[a-z0-9])?\z/ end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f0254be2044..dfd512cad53 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2263,6 +2263,9 @@ msgstr "" msgid "CiVariables|Remove variable row" msgstr "" +msgid "CiVariables|Scope" +msgstr "" + msgid "CiVariables|Specify variable values to be used in this run. The values specified in %{linkStart}CI/CD settings%{linkEnd} will be used as default" msgstr "" @@ -2284,15 +2287,24 @@ msgstr "" msgid "CiVariable|All environments" msgstr "" +msgid "CiVariable|Create wildcard" +msgstr "" + msgid "CiVariable|Error occurred while saving variables" msgstr "" msgid "CiVariable|Masked" msgstr "" +msgid "CiVariable|New environment" +msgstr "" + msgid "CiVariable|Protected" msgstr "" +msgid "CiVariable|Search environments" +msgstr "" + msgid "CiVariable|Toggle masked" msgstr "" diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb index a2a09e2580f..21e106660d0 100644 --- a/spec/controllers/projects/variables_controller_spec.rb +++ b/spec/controllers/projects/variables_controller_spec.rb @@ -36,5 +36,70 @@ describe Projects::VariablesController do end include_examples 'PATCH #update updates variables' + + context 'with environment scope' do + let!(:variable) { create(:ci_variable, project: project, environment_scope: 'custom_scope') } + + let(:variable_attributes) do + { id: variable.id, + key: variable.key, + secret_value: variable.value, + protected: variable.protected?.to_s, + environment_scope: variable.environment_scope } + end + let(:new_variable_attributes) do + { key: 'new_key', + secret_value: 'dummy_value', + protected: 'false', + environment_scope: 'new_scope' } + end + + context 'with same key and different environment scope' do + let(:variables_attributes) do + [ + variable_attributes, + new_variable_attributes.merge(key: variable.key) + ] + end + + it 'does not update the existing variable' do + expect { subject }.not_to change { variable.reload.value } + end + + it 'creates the new variable' do + expect { subject }.to change { owner.variables.count }.by(1) + end + + it 'returns a successful response including all variables' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('variables') + end + end + + context 'with same key and same environment scope' do + let(:variables_attributes) do + [ + variable_attributes, + new_variable_attributes.merge(key: variable.key, environment_scope: variable.environment_scope) + ] + end + + it 'does not update the existing variable' do + expect { subject }.not_to change { variable.reload.value } + end + + it 'does not create the new variable' do + expect { subject }.not_to change { owner.variables.count } + end + + it 'returns a bad request response' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end end end diff --git a/spec/features/project_variables_spec.rb b/spec/features/project_variables_spec.rb index 95685a3c7ff..9e3f8a843a1 100644 --- a/spec/features/project_variables_spec.rb +++ b/spec/features/project_variables_spec.rb @@ -17,4 +17,27 @@ describe 'Project variables', :js do end it_behaves_like 'variable list' + + it 'adds new variable with a special environment scope' do + page.within('.js-ci-variable-list-section .js-row:last-child') do + find('.js-ci-variable-input-key').set('somekey') + find('.js-ci-variable-input-value').set('somevalue') + + find('.js-variable-environment-toggle').click + find('.js-variable-environment-dropdown-wrapper .dropdown-input-field').set('review/*') + find('.js-variable-environment-dropdown-wrapper .js-dropdown-create-new-item').click + + expect(find('input[name="variables[variables_attributes][][environment_scope]"]', visible: false).value).to eq('review/*') + end + + click_button('Save variables') + wait_for_requests + + visit page_path + + page.within('.js-ci-variable-list-section .js-row:nth-child(2)') do + expect(find('.js-ci-variable-input-key').value).to eq('somekey') + expect(page).to have_content('review/*') + end + end end diff --git a/spec/frontend/fixtures/projects.rb b/spec/frontend/fixtures/projects.rb index b6c29003e57..91e3b65215a 100644 --- a/spec/frontend/fixtures/projects.rb +++ b/spec/frontend/fixtures/projects.rb @@ -18,8 +18,6 @@ describe 'Projects (JavaScript fixtures)', type: :controller do end before do - stub_licensed_features(variable_environment_scope: true) - project.add_maintainer(admin) sign_in(admin) allow(SecureRandom).to receive(:hex).and_return('securerandomhex:thereisnospoon') diff --git a/spec/lib/gitlab/ci/build/policy/variables_spec.rb b/spec/lib/gitlab/ci/build/policy/variables_spec.rb index 42a2a9fda2e..f712f47a558 100644 --- a/spec/lib/gitlab/ci/build/policy/variables_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/variables_spec.rb @@ -91,5 +91,38 @@ describe Gitlab::Ci::Build::Policy::Variables do expect(policy).to be_satisfied_by(pipeline, seed) end end + + context 'when using project ci variables in environment scope' do + let(:ci_build) do + build(:ci_build, pipeline: pipeline, + project: project, + ref: 'master', + stage: 'review', + environment: 'test/$CI_JOB_STAGE/1') + end + + before do + create(:ci_variable, project: project, + key: 'SCOPED_VARIABLE', + value: 'my-value-1') + + create(:ci_variable, project: project, + key: 'SCOPED_VARIABLE', + value: 'my-value-2', + environment_scope: 'test/review/*') + end + + it 'is satisfied by scoped variable match' do + policy = described_class.new(['$SCOPED_VARIABLE == "my-value-2"']) + + expect(policy).to be_satisfied_by(pipeline, seed) + end + + it 'is not satisfied when matching against overridden variable' do + policy = described_class.new(['$SCOPED_VARIABLE == "my-value-1"']) + + expect(policy).not_to be_satisfied_by(pipeline, seed) + end + end end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index a1079e54975..ba295386a55 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -32,6 +32,14 @@ describe Gitlab::Regex do it { is_expected.not_to match('/') } end + describe '.environment_scope_regex' do + subject { described_class.environment_scope_regex } + + it { is_expected.to match('foo') } + it { is_expected.to match('foo*Z') } + it { is_expected.not_to match('!!()()') } + end + describe '.environment_slug_regex' do subject { described_class.environment_slug_regex } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index b7e005e3883..4aac4b640f4 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2340,6 +2340,32 @@ describe Ci::Build do it_behaves_like 'containing environment variables' end end + + context 'when project has an environment specific variable' do + let(:environment_specific_variable) do + { key: 'MY_STAGING_ONLY_VARIABLE', value: 'environment_specific_variable', public: false, masked: false } + end + + before do + create(:ci_variable, environment_specific_variable.slice(:key, :value) + .merge(project: project, environment_scope: 'stag*')) + end + + it_behaves_like 'containing environment variables' + + context 'when environment scope does not match build environment' do + it { is_expected.not_to include(environment_specific_variable) } + end + + context 'when environment scope matches build environment' do + before do + create(:environment, name: 'staging', project: project) + build.update!(environment: 'staging') + end + + it { is_expected.to include(environment_specific_variable) } + end + end end context 'when build started manually' do diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index a231c7eaed8..3ff547456c6 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -10,6 +10,7 @@ describe Ci::Variable do describe 'validations' do it { is_expected.to include_module(Presentable) } it { is_expected.to include_module(Maskable) } + it { is_expected.to include_module(HasEnvironmentScope) } it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope).with_message(/\(\w+\) has already been taken/) } end diff --git a/spec/models/concerns/has_environment_scope_spec.rb b/spec/models/concerns/has_environment_scope_spec.rb new file mode 100644 index 00000000000..a6e1ba59263 --- /dev/null +++ b/spec/models/concerns/has_environment_scope_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe HasEnvironmentScope do + subject { build(:ci_variable) } + + it { is_expected.to allow_value('*').for(:environment_scope) } + it { is_expected.to allow_value('review/*').for(:environment_scope) } + it { is_expected.not_to allow_value('').for(:environment_scope) } + it { is_expected.not_to allow_value('!!()()').for(:environment_scope) } + + it do + is_expected.to validate_uniqueness_of(:key) + .scoped_to(:project_id, :environment_scope) + .with_message(/\(\w+\) has already been taken/) + end + + describe '.on_environment' do + let(:project) { create(:project) } + + it 'returns scoped objects' do + variable1 = create(:ci_variable, project: project, environment_scope: '*') + variable2 = create(:ci_variable, project: project, environment_scope: 'product/*') + create(:ci_variable, project: project, environment_scope: 'staging/*') + + expect(project.variables.on_environment('product/canary-1')).to eq([variable1, variable2]) + end + + it 'returns only the most relevant object if relevant_only is true' do + create(:ci_variable, project: project, environment_scope: '*') + variable2 = create(:ci_variable, project: project, environment_scope: 'product/*') + create(:ci_variable, project: project, environment_scope: 'staging/*') + + expect(project.variables.on_environment('product/canary-1', relevant_only: true)).to eq([variable2]) + end + + it 'returns scopes ordered by lowest precedence first' do + create(:ci_variable, project: project, environment_scope: '*') + create(:ci_variable, project: project, environment_scope: 'production*') + create(:ci_variable, project: project, environment_scope: 'production') + + result = project.variables.on_environment('production').map(&:environment_scope) + + expect(result).to eq(['*', 'production*', 'production']) + end + end + + describe '#environment_scope=' do + context 'when the new environment_scope is nil' do + it 'strips leading and trailing whitespaces' do + subject.environment_scope = nil + + expect(subject.environment_scope).to eq('') + end + end + + context 'when the new environment_scope has leadind and trailing whitespaces' do + it 'strips leading and trailing whitespaces' do + subject.environment_scope = ' * ' + + expect(subject.environment_scope).to eq('*') + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dde766c3813..29a589eba20 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2648,9 +2648,10 @@ describe Project do describe '#ci_variables_for' do let(:project) { create(:project) } + let(:environment_scope) { '*' } let!(:ci_variable) do - create(:ci_variable, value: 'secret', project: project) + create(:ci_variable, value: 'secret', project: project, environment_scope: environment_scope) end let!(:protected_variable) do @@ -2695,6 +2696,96 @@ describe Project do it_behaves_like 'ref is protected' end + + context 'when environment name is specified' do + let(:environment) { 'review/name' } + + subject do + project.ci_variables_for(ref: 'ref', environment: environment) + end + + context 'when environment scope is exactly matched' do + let(:environment_scope) { 'review/name' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope is matched by wildcard' do + let(:environment_scope) { 'review/*' } + + it { is_expected.to contain_exactly(ci_variable) } + end + + context 'when environment scope does not match' do + let(:environment_scope) { 'review/*/special' } + + it { is_expected.not_to contain_exactly(ci_variable) } + end + + context 'when environment scope has _' do + let(:environment_scope) { '*_*' } + + it 'does not treat it as wildcard' do + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains underscore' do + let(:environment) { 'foo_bar/test' } + let(:environment_scope) { 'foo_bar/*' } + + it 'matches literally for _' do + is_expected.to contain_exactly(ci_variable) + end + end + end + + # The environment name and scope cannot have % at the moment, + # but we're considering relaxing it and we should also make sure + # it doesn't break in case some data sneaked in somehow as we're + # not checking this integrity in database level. + context 'when environment scope has %' do + it 'does not treat it as wildcard' do + ci_variable.update_attribute(:environment_scope, '*%*') + + is_expected.not_to contain_exactly(ci_variable) + end + + context 'when environment name contains a percent' do + let(:environment) { 'foo%bar/test' } + + it 'matches literally for _' do + ci_variable.update(environment_scope: 'foo%bar/*') + + is_expected.to contain_exactly(ci_variable) + end + end + end + + context 'when variables with the same name have different environment scopes' do + let!(:partially_matched_variable) do + create(:ci_variable, + key: ci_variable.key, + value: 'partial', + environment_scope: 'review/*', + project: project) + end + + let!(:perfectly_matched_variable) do + create(:ci_variable, + key: ci_variable.key, + value: 'prefect', + environment_scope: 'review/name', + project: project) + end + + it 'puts variables matching environment scope more in the end' do + is_expected.to eq( + [ci_variable, + partially_matched_variable, + perfectly_matched_variable]) + end + end + end end describe '#any_lfs_file_locks?', :request_store do diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb index 55b1419a004..69f105b71a8 100644 --- a/spec/requests/api/variables_spec.rb +++ b/spec/requests/api/variables_spec.rb @@ -106,6 +106,30 @@ describe API::Variables do expect(response).to have_gitlab_http_status(400) end + + it 'creates variable with a specific environment scope' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end + + it 'allows duplicated variable key given different environment scopes' do + variable = create(:ci_variable, project: project) + + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) + + expect(response).to have_gitlab_http_status(201) + expect(json_response['key']).to eq(variable.key) + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') + end end context 'authorized user with invalid permissions' do diff --git a/spec/serializers/variable_entity_spec.rb b/spec/serializers/variable_entity_spec.rb index effc0022633..10664ff66ec 100644 --- a/spec/serializers/variable_entity_spec.rb +++ b/spec/serializers/variable_entity_spec.rb @@ -8,7 +8,7 @@ describe VariableEntity do subject { entity.as_json } it 'contains required fields' do - expect(subject).to include(:id, :key, :value, :protected) + expect(subject).to include(:id, :key, :value, :protected, :environment_scope) end end end |