Bring scoped environment variables to core

As decided in https://gitlab.com/gitlab-org/gitlab-ce/issues/53593

11 files changed, 339 insertions, 4 deletions

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