Merge branch '2302-environment-specific-variables' into 'master'

Environment-specific variables

Closes #2302

See merge request !2112

app/controllers/ee/projects/variables_controller.rb

+module EE
+  module Projects
+    module VariablesController
+      extend ActiveSupport::Concern
+
+      def variable_params_attributes
+        attrs = super
+
+        attrs.unshift(:environment_scope) if
+            project.feature_available?(:variable_environment_scope)
+
+        attrs
+      end
+    end
+  end
+end

app/controllers/projects/variables_controller.rb

 class Projects::VariablesController < Projects::ApplicationController
+  prepend ::EE::Projects::VariablesController
+
   before_action :authorize_admin_build!
 
   layout 'project_settings'
@@ -14,7 +16,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 namespace_project_variables_path(project.namespace, project), notice: 'Variable was successfully updated.'
     else
       render action: "show"
@@ -22,9 +24,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 namespace_project_settings_ci_cd_path(project.namespace, project)
     else
@@ -43,8 +45,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

app/models/ci/build.rb

@@ -187,6 +187,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
@@ -195,15 +201,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
@@ -381,6 +383,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

app/models/ci/variable.rb

@@ -2,10 +2,11 @@ module Ci
   class Variable < ActiveRecord::Base
     extend Ci::Model
     include HasVariable
+    prepend EE::Ci::Variable
 
     belongs_to :project
 
-    validates :key, uniqueness: { scope: :project_id }
+    validates :key, uniqueness: { scope: [:project_id, :environment_scope] }
 
     scope :unprotected, -> { where(protected: false) }
   end

app/models/ee/ci/variable.rb

+module EE
+  module Ci
+    module Variable
+      extend ActiveSupport::Concern
+
+      prepended do
+        validates(
+          :environment_scope,
+          presence: true,
+          format: { with: ::Gitlab::Regex.environment_scope_regex,
+                    message: ::Gitlab::Regex.environment_scope_regex_message }
+        )
+      end
+    end
+  end
+end

app/models/ee/project.rb

@@ -210,6 +210,56 @@ module EE
       end
     end
 
+    def secret_variables_for(ref:, environment: nil)
+      return super.where(environment_scope: '*') unless
+        environment && feature_available?(:variable_environment_scope)
+
+      query = super
+
+      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
+      SQL
+
+      values = {
+        wildcard: '*',
+        environment_name: environment.name
+      }
+
+      quoted_values =
+        values.transform_values(&self.class.connection.method(:quote))
+
+      # 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
+      query
+        .where(where, values)
+        .order(order % quoted_values) # `order` cannot escape for us!
+    end
+
     def cache_has_external_issue_tracker
       super unless ::Gitlab::Geo.secondary?
     end

app/models/license.rb

@@ -22,6 +22,7 @@ class License < ActiveRecord::Base
   PUSH_RULES_FEATURE = 'GitLab_PushRules'.freeze
   RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze
   SERVICE_DESK_FEATURE = 'GitLab_ServiceDesk'.freeze
+  VARIABLE_ENVIRONMENT_SCOPE_FEATURE = 'VariableEnvironmentScope'.freeze
 
   FEATURE_CODES = {
     auditor_user: AUDITOR_USER_FEATURE,
@@ -30,6 +31,7 @@ class License < ActiveRecord::Base
     object_storage: OBJECT_STORAGE_FEATURE,
     related_issues: RELATED_ISSUES_FEATURE,
     service_desk: SERVICE_DESK_FEATURE,
+    variable_environment_scope: VARIABLE_ENVIRONMENT_SCOPE_FEATURE,
 
     # Features that make sense to Namespace:
     burndown_charts: BURNDOWN_CHARTS_FEATURE,
@@ -79,7 +81,8 @@ class License < ActiveRecord::Base
     { FILE_LOCK_FEATURE => 1 },
     { GEO_FEATURE => 1 },
     { OBJECT_STORAGE_FEATURE => 1 },
-    { SERVICE_DESK_FEATURE => 1 }
+    { SERVICE_DESK_FEATURE => 1 },
+    { VARIABLE_ENVIRONMENT_SCOPE_FEATURE => 1 }
   ].freeze
 
   EEU_FEATURES = [

app/models/project.rb

@@ -1315,7 +1315,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

app/views/projects/variables/_content.html.haml

 %h4.prepend-top-0
   Secret variables
   = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'secret-variables'), target: '_blank'
-%p
-  These variables will be set to environment by the runner, and could be protected by exposing only to protected branches or tags.
+- if @project.feature_available?(:variable_environment_scope)
+  %p
+    These variables will be set to environment by the runner, and could be protected by exposing only to protected branches or tags, or some particular environments.
+- else
+  %p
+    These variables will be set to environment by the runner, and could be protected by exposing only to protected branches or tags.
 %p
   So you can use them for passwords, secret keys or whatever you want.
 %p

app/views/projects/variables/_form.html.haml

@@ -7,6 +7,7 @@
   .form-group
     = f.label :value, "Value", class: "label-light"
     = f.text_area :value, class: "form-control", placeholder: "PROJECT_VARIABLE"
+  = render 'projects/variables/ee/environment_scope', f: f
   .form-group
     .checkbox
       = f.label :protected do

app/views/projects/variables/_table.html.haml

@@ -9,6 +9,8 @@
       %th Key
       %th Value
       %th Protected
+      - if @project.feature_available?(:variable_environment_scope)
+        %th Environment scope
       %th
     %tbody
       - @project.variables.order_key_asc.each do |variable|
@@ -17,6 +19,8 @@
             %td.variable-key= variable.key
             %td.variable-value{ "data-value" => variable.value }******
             %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected)
+            - if @project.feature_available?(:variable_environment_scope)
+              %td.variable-environment-scope= variable.environment_scope
             %td.variable-menu
               = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent btn-variable-edit" do
                 %span.sr-only

app/views/projects/variables/ee/_environment_scope.html.haml

+- if @project.feature_available?(:variable_environment_scope)
+  .form-group
+    = f.label :environment_scope, "Environment scope", class: "label-light"
+    = f.text_field :environment_scope, class: "form-control", placeholder: "*"
+    .help-block
+      This variable will be passed only to jobs with a matching environment name.
+      <code>*</code> is a wildcard that matches all environments (existing or not).
+      = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'limiting-environment-scopes-of-secret-variables'), target: '_blank'

changelogs/unreleased-ee/2302-environment-specific-variables.yml

+---
+title: Add environment scope to secret variables to specify environments
+merge_request: 2112
+author:

db/migrate/20170622135451_rename_duplicated_variable_key.rb

+class RenameDuplicatedVariableKey < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    execute(<<~SQL)
+      UPDATE ci_variables
+      SET #{key} = CONCAT(#{key}, #{underscore}, id)
+      WHERE id IN (
+        SELECT *
+        FROM ( -- MySQL requires an extra layer
+          SELECT dup.id
+          FROM ci_variables dup
+          INNER JOIN (SELECT max(id) AS id, #{key}, project_id
+                      FROM ci_variables tmp
+                      GROUP BY #{key}, project_id) var
+          USING (#{key}, project_id) where dup.id <> var.id
+        ) dummy
+      )
+    SQL
+  end
+
+  def down
+    # noop
+  end
+
+  def key
+    # key needs to be quoted in MySQL
+    quote_column_name('key')
+  end
+
+  def underscore
+    quote('_')
+  end
+end

db/migrate/20170622135628_add_environment_scope_to_ci_variables.rb

+class AddEnvironmentScopeToCiVariables < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:ci_variables, :environment_scope, :string, default: '*')
+  end
+
+  def down
+    remove_column(:ci_variables, :environment_scope)
+  end
+end

db/migrate/20170622135728_add_unique_constraint_to_ci_variables.rb

+class AddUniqueConstraintToCiVariables < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless this_index_exists?
+      add_concurrent_index(:ci_variables, columns, name: index_name, unique: true)
+    end
+  end
+
+  def down
+    if this_index_exists?
+      if Gitlab::Database.mysql? && !index_exists?(:ci_variables, :project_id)
+        # Need to add this index for MySQL project_id foreign key constraint
+        add_concurrent_index(:ci_variables, :project_id)
+      end
+
+      remove_concurrent_index(:ci_variables, columns, name: index_name)
+    end
+  end
+
+  private
+
+  def this_index_exists?
+    index_exists?(:ci_variables, columns, name: index_name)
+  end
+
+  def columns
+    @columns ||= [:project_id, :key, :environment_scope]
+  end
+
+  def index_name
+    'index_ci_variables_on_project_id_and_key_and_environment_scope'
+  end
+end

db/migrate/20170623080805_remove_ci_variables_project_id_index.rb

+class RemoveCiVariablesProjectIdIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    if index_exists?(:ci_variables, :project_id)
+      remove_concurrent_index(:ci_variables, :project_id)
+    end
+  end
+
+  def down
+    unless index_exists?(:ci_variables, :project_id)
+      add_concurrent_index(:ci_variables, :project_id)
+    end
+  end
+end

db/schema.rb

@@ -442,9 +442,10 @@ ActiveRecord::Schema.define(version: 20170627211700) do
     t.string "encrypted_value_iv"
     t.integer "project_id", null: false
     t.boolean "protected", default: false, null: false
+    t.string "environment_scope", default: "*", null: false
   end
 
-  add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree
+  add_index "ci_variables", ["project_id", "key", "environment_scope"], name: "index_ci_variables_on_project_id_and_key_and_environment_scope", unique: true, using: :btree
 
   create_table "container_repositories", force: :cascade do |t|
     t.integer "project_id", null: false

doc/api/build_variables.md

@@ -67,6 +67,7 @@ POST /projects/:id/variables
 | `key`       | string  | yes      | The `key` of a variable; must have no more than 255 characters; only `A-Z`, `a-z`, `0-9`, and `_` are allowed |
 | `value`     | string  | yes      | The `value` of a variable |
 | `protected` | boolean | no       | Whether the variable is protected |
+| `environment_scope` | string | no | The `environment_scope` of the variable |
 
 ```
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/variables" --form "key=NEW_VARIABLE" --form "value=new value"
@@ -76,7 +77,8 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitl
 {
     "key": "NEW_VARIABLE",
     "value": "new value",
-    "protected": false
+    "protected": false,
+    "environment_scope": "*"
 }
 ```
 
@@ -94,6 +96,7 @@ PUT /projects/:id/variables/:key
 | `key`       | string  | yes      | The `key` of a variable   |
 | `value`     | string  | yes      | The `value` of a variable |
 | `protected` | boolean | no       | Whether the variable is protected |
+| `environment_scope` | string | no | The `environment_scope` of the variable |
 
 ```
 curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/variables/NEW_VARIABLE" --form "value=updated value"
@@ -103,7 +106,8 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitla
 {
     "key": "NEW_VARIABLE",
     "value": "updated value",
-    "protected": true
+    "protected": true,
+    "environment_scope": "*"
 }
 ```
 

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.
@@ -176,6 +176,24 @@ 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 secret variables
+
+>**Notes:**
+[Introduced][ee-2112] in [GitLab Enterprise Edition Premium][eep] 9.4.
+
+You can limit the environment scope of a secret variable by
+[defining which environments][envs] it can be available for.
+
+Wildcards can be used, and the default environment scope is `*` which means
+any jobs will have this variable, not matter if an environment is defined or
+not.
+
+For example, if the environment scope is `production`, then only the jobs
+having the environment `production` defined would have this specific variable.
+Wildcards (`*`) can be used along with the environment name, therefore if the
+environment scope is `review/*` then any jobs with environment names starting
+with `review/` would have that particular variable.
+
 ## Deployment variables
 
 >**Note:**
@@ -426,10 +444,12 @@ 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
+[envs]: ../environments.md
+[eep]: https://about.gitlab.com/gitlab-ee/ "Available only in GitLab Enterprise Edition Premium"
+[ee-2112]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2112
 [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

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

lib/api/entities.rb

@@ -782,6 +782,11 @@ module API
     class Variable < Grape::Entity
       expose :key, :value
       expose :protected?, as: :protected
+
+      # EE
+      expose :environment_scope, if: ->(variable, options) {
+        variable.project.feature_available?(:variable_environment_scope)
+      }
     end
 
     class Pipeline < PipelineBasic

lib/api/variables.rb

@@ -43,9 +43,18 @@ module API
         requires :key, type: String, desc: 'The key of the variable'
         requires :value, type: String, desc: 'The value of the variable'
         optional :protected, type: String, desc: 'Whether the variable is protected'
+
+        # EE
+        optional :environment_scope, type: String, desc: 'The environment_scope of the variable'
       end
       post ':id/variables' do
-        variable = user_project.variables.create(declared_params(include_missing: false))
+        variable_params = declared_params(include_missing: false)
+
+        # EE
+        variable_params.delete(:environment_scope) unless
+            user_project.feature_available?(:variable_environment_scope)
+
+        variable = user_project.variables.create(variable_params)
 
         if variable.valid?
           present variable, with: Entities::Variable
@@ -61,13 +70,22 @@ module API
         optional :key, type: String, desc: 'The key of the variable'
         optional :value, type: String, desc: 'The value of the variable'
         optional :protected, type: String, desc: 'Whether the variable is protected'
+
+        # EE
+        optional :environment_scope, type: String, desc: 'The environment_scope of the variable'
       end
       put ':id/variables/:key' do
         variable = user_project.variables.find_by(key: params[:key])
 
         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)
+
+        # EE
+        variable_params.delete(:environment_scope) unless
+            user_project.feature_available?(:variable_environment_scope)
+
+        if variable.update(variable_params)
           present variable, with: Entities::Variable
         else
           render_validation_error!(variable)

lib/ee/gitlab/regex.rb

+module EE
+  module Gitlab
+    module Regex
+      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
+    end
+  end
+end

lib/gitlab/regex.rb

 module Gitlab
   module Regex
     extend self
+    extend EE::Gitlab::Regex
 
     def namespace_name_regex
       @namespace_name_regex ||= /\A[\p{Alnum}\p{Pd}_\. ]*\z/.freeze
@@ -38,8 +39,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

lib/gitlab/sql/glob.rb

+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

spec/features/variables_ee_spec.rb

+require 'spec_helper'
+
+describe 'Project variables EE', js: true do
+  let(:user)     { create(:user) }
+  let(:project)  { create(:empty_project) }
+  let(:variable) { create(:ci_variable, key: 'test_key', value: 'test value') }
+
+  let(:variable_environment_scope) { true }
+
+  before do
+    stub_licensed_features(
+      variable_environment_scope: variable_environment_scope)
+
+    login_as(user)
+    project.team << [user, :master]
+    project.variables << variable
+
+    visit namespace_project_settings_ci_cd_path(project.namespace, project)
+  end
+
+  it 'adds new variable with a special environment scope' do
+    expect(page).to have_selector('#variable_environment_scope')
+
+    fill_in('variable_key', with: 'key')
+    fill_in('variable_value', with: 'value')
+    fill_in('variable_environment_scope', with: 'review/*')
+    click_button('Add new variable')
+
+    expect(page).to have_content('Variables were successfully updated.')
+    page.within('.variables-table') do
+      expect(page).to have_content('key')
+      expect(page).to have_content('review/*')
+    end
+  end
+
+  context 'when variable environment scope is not available' do
+    let(:variable_environment_scope) { false }
+
+    it 'does not show variable environment scope element' do
+      expect(page).not_to have_selector('#variable_environment_scope')
+    end
+  end
+
+  context 'when editing a variable for environment' do
+    before do
+      page.within('.variables-table') do
+        find('.btn-variable-edit').click
+      end
+    end
+
+    it 'edits variable to be another environment scope' do
+      expect(page).to have_selector('#variable_environment_scope')
+
+      fill_in('variable_environment_scope', with: 'review/*')
+      click_button('Save variable')
+
+      expect(page).to have_content('Variable was successfully updated.')
+      expect(project.variables(true).first.environment_scope).to eq('review/*')
+    end
+
+    context 'when variable environment scope is not available' do
+      let(:variable_environment_scope) { false }
+
+      it 'does not show environment scope element' do
+        expect(page).not_to have_selector('#variable_environment_scope')
+      end
+    end
+  end
+end

spec/lib/ee/gitlab/service_desk_spec.rb

@@ -2,8 +2,7 @@ require 'spec_helper'
 
 describe EE::Gitlab::ServiceDesk, lib: true do
   before do
-    allow(License).to receive(:feature_available?).and_call_original
-    allow(License).to receive(:feature_available?).with(:service_desk) { true }
+    stub_licensed_features(service_desk: true)
     allow(::Gitlab::IncomingEmail).to receive(:enabled?) { true }
     allow(::Gitlab::IncomingEmail).to receive(:supports_wildcard?) { true }
   end
@@ -14,7 +13,7 @@ describe EE::Gitlab::ServiceDesk, lib: true do
 
   context 'when license does not support service desk' do
     before do
-      allow(License).to receive(:feature_available?).with(:service_desk) { false }
+      stub_licensed_features(service_desk: false)
     end
 
     it { is_expected.to be_falsy }

spec/lib/gitlab/sql/glob_spec.rb

+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

spec/migrations/rename_duplicated_variable_key_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20170622135451_rename_duplicated_variable_key.rb')
+
+describe RenameDuplicatedVariableKey, :migration do
+  let(:variables) { table(:ci_variables) }
+  let(:projects) { table(:projects) }
+
+  before do
+    projects.create!(id: 1)
+    variables.create!(id: 1, key: 'key1', project_id: 1)
+    variables.create!(id: 2, key: 'key2', project_id: 1)
+    variables.create!(id: 3, key: 'keyX', project_id: 1)
+    variables.create!(id: 4, key: 'keyX', project_id: 1)
+    variables.create!(id: 5, key: 'keyY', project_id: 1)
+    variables.create!(id: 6, key: 'keyX', project_id: 1)
+    variables.create!(id: 7, key: 'key7', project_id: 1)
+    variables.create!(id: 8, key: 'keyY', project_id: 1)
+  end
+
+  it 'correctly remove duplicated records with smaller id' do
+    migrate!
+
+    expect(variables.pluck(:id, :key)).to contain_exactly(
+      [1, 'key1'],
+      [2, 'key2'],
+      [3, 'keyX_3'],
+      [4, 'keyX_4'],
+      [5, 'keyY_5'],
+      [6, 'keyX'],
+      [7, 'key7'],
+      [8, 'keyY']
+    )
+  end
+end

spec/models/ci/build_spec.rb

@@ -127,7 +127,7 @@ describe Ci::Build, :models do
 
   describe '#browsable_artifacts?' do
     subject { build.browsable_artifacts? }
-  
+
     context 'artifacts metadata does not exist' do
       before do
         build.update_attributes(artifacts_metadata: nil)
@@ -1526,9 +1526,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

spec/models/ci/variable_spec.rb

@@ -4,7 +4,15 @@ describe Ci::Variable, models: true do
   subject { build(:ci_variable) }
 
   it { is_expected.to include_module(HasVariable) }
-  it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id) }
+
+  describe 'validations' do
+    # EE
+    before do
+      stub_licensed_features(variable_environment_scope: true)
+    end
+
+    it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }
+  end
 
   describe '.unprotected' do
     subject { described_class.unprotected }

spec/models/ee/ci/build_spec.rb

@@ -10,18 +10,18 @@ describe Ci::Build, models: true do
                          status: 'success')
   end
 
-  let(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:job) { create(:ci_build, pipeline: pipeline) }
 
   describe '#shared_runners_minutes_limit_enabled?' do
-    subject { build.shared_runners_minutes_limit_enabled? }
+    subject { job.shared_runners_minutes_limit_enabled? }
 
     context 'for shared runner' do
       before do
-        build.runner = create(:ci_runner, :shared)
+        job.runner = create(:ci_runner, :shared)
       end
 
       it do
-        expect(build.project).to receive(:shared_runners_minutes_limit_enabled?)
+        expect(job.project).to receive(:shared_runners_minutes_limit_enabled?)
           .and_return(true)
 
         is_expected.to be_truthy
@@ -30,7 +30,7 @@ describe Ci::Build, models: true do
 
     context 'with specific runner' do
       before do
-        build.runner = create(:ci_runner, :specific)
+        job.runner = create(:ci_runner, :specific)
       end
 
       it { is_expected.to be_falsey }
@@ -42,29 +42,67 @@ describe Ci::Build, models: true do
   end
 
   context 'updates pipeline minutes' do
-    let(:build) { create(:ci_build, :running, pipeline: pipeline) }
+    let(:job) { create(:ci_build, :running, pipeline: pipeline) }
 
     %w(success drop cancel).each do |event|
       it "for event #{event}" do
         expect(UpdateBuildMinutesService)
           .to receive(:new).and_call_original
 
-        build.public_send(event)
+        job.public_send(event)
       end
     end
   end
 
   describe '#stick_build_if_status_changed' do
     it 'sticks the build if the status changed' do
-      build = create(:ci_build, :pending)
+      job = create(:ci_build, :pending)
 
       allow(Gitlab::Database::LoadBalancing).to receive(:enable?)
         .and_return(true)
 
       expect(Gitlab::Database::LoadBalancing::Sticking).to receive(:stick)
-        .with(:build, build.id)
+        .with(:build, job.id)
 
-      build.update(status: :running)
+      job.update(status: :running)
+    end
+  end
+
+  describe '#variables' do
+    subject { job.variables }
+
+    context 'when environment specific variable is defined' do
+      let(:environment_varialbe) do
+        { key: 'ENV_KEY', value: 'environment', public: false }
+      end
+
+      before do
+        job.update(environment: 'staging')
+        create(:environment, name: 'staging', project: job.project)
+
+        variable =
+          build(:ci_variable,
+                environment_varialbe.slice(:key, :value)
+                  .merge(project: project, environment_scope: 'stag*'))
+
+        variable.save!
+      end
+
+      context 'when variable environment scope is available' do
+        before do
+          stub_licensed_features(variable_environment_scope: true)
+        end
+
+        it { is_expected.to include(environment_varialbe) }
+      end
+
+      context 'when variable environment scope is not available' do
+        before do
+          stub_licensed_features(variable_environment_scope: false)
+        end
+
+        it { is_expected.not_to include(environment_varialbe) }
+      end
     end
   end
 end

spec/models/ee/ci/variable_spec.rb

+require 'spec_helper'
+
+describe Ci::Variable, models: true 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 do
+    is_expected.to validate_uniqueness_of(:key)
+      .scoped_to(:project_id, :environment_scope)
+  end
+end

spec/models/ee/project_spec.rb

@@ -339,6 +339,172 @@ describe Project, models: true do
     end
   end
 
+  describe '#secret_variables_for' do
+    let(:project) { create(:empty_project) }
+
+    let!(:secret_variable) do
+      create(:ci_variable, value: 'secret', project: project)
+    end
+
+    let!(:protected_variable) do
+      create(:ci_variable, :protected, value: 'protected', project: project)
+    end
+
+    subject { project.secret_variables_for(ref: 'ref') }
+
+    before do
+      stub_application_setting(
+        default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+    end
+
+    context 'when environment is specified' do
+      let(:environment) { create(:environment, name: 'review/name') }
+
+      subject do
+        project.secret_variables_for(ref: 'ref', environment: environment)
+      end
+
+      shared_examples 'matching environment scope' do
+        context 'when variable environment scope is available' do
+          before do
+            stub_licensed_features(variable_environment_scope: true)
+          end
+
+          it 'contains the secret variable' do
+            is_expected.to contain_exactly(secret_variable)
+          end
+        end
+
+        context 'when variable environment scope is unavailable' do
+          before do
+            stub_licensed_features(variable_environment_scope: false)
+          end
+
+          it 'does not contain the secret variable' do
+            is_expected.not_to contain_exactly(secret_variable)
+          end
+        end
+      end
+
+      shared_examples 'not matching environment scope' do
+        context 'when variable environment scope is available' do
+          before do
+            stub_licensed_features(variable_environment_scope: true)
+          end
+
+          it 'does not contain the secret variable' do
+            is_expected.not_to contain_exactly(secret_variable)
+          end
+        end
+
+        context 'when variable environment scope is unavailable' do
+          before do
+            stub_licensed_features(variable_environment_scope: false)
+          end
+
+          it 'does not contain the secret variable' do
+            is_expected.not_to contain_exactly(secret_variable)
+          end
+        end
+      end
+
+      context 'when environment scope is exactly matched' do
+        before do
+          secret_variable.update(environment_scope: 'review/name')
+        end
+
+        it_behaves_like 'matching environment scope'
+      end
+
+      context 'when environment scope is matched by wildcard' do
+        before do
+          secret_variable.update(environment_scope: 'review/*')
+        end
+
+        it_behaves_like 'matching environment scope'
+      end
+
+      context 'when environment scope does not match' do
+        before do
+          secret_variable.update(environment_scope: 'review/*/special')
+        end
+
+        it_behaves_like 'not matching environment scope'
+      end
+
+      context 'when environment scope has _' do
+        before do
+          stub_licensed_features(variable_environment_scope: true)
+        end
+
+        it 'does not treat it as wildcard' do
+          secret_variable.update(environment_scope: '*_*')
+
+          is_expected.not_to contain_exactly(secret_variable)
+        end
+
+        it 'matches literally for _' do
+          secret_variable.update(environment_scope: 'foo_bar/*')
+          environment.update(name: 'foo_bar/test')
+
+          is_expected.to contain_exactly(secret_variable)
+        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
+        before do
+          stub_licensed_features(variable_environment_scope: true)
+        end
+
+        it 'does not treat it as wildcard' do
+          secret_variable.update_attribute(:environment_scope, '*%*')
+
+          is_expected.not_to contain_exactly(secret_variable)
+        end
+
+        it 'matches literally for _' do
+          secret_variable.update(environment_scope: 'foo%bar/*')
+          environment.update_attribute(:name, 'foo%bar/test')
+
+          is_expected.to contain_exactly(secret_variable)
+        end
+      end
+
+      context 'when variables with the same name have different environment scopes' do
+        let!(:partially_matched_variable) do
+          create(:ci_variable,
+                 key: secret_variable.key,
+                 value: 'partial',
+                 environment_scope: 'review/*',
+                 project: project)
+        end
+
+        let!(:perfectly_matched_variable) do
+          create(:ci_variable,
+                 key: secret_variable.key,
+                 value: 'prefect',
+                 environment_scope: 'review/name',
+                 project: project)
+        end
+
+        before do
+          stub_licensed_features(variable_environment_scope: true)
+        end
+
+        it 'puts variables matching environment scope more in the end' do
+          is_expected.to eq(
+            [secret_variable,
+             partially_matched_variable,
+             perfectly_matched_variable])
+        end
+      end
+    end
+  end
+
   describe '#approvals_before_merge' do
     [
       { license: true,  database: 5,  expected: 5 },

spec/models/project_spec.rb

@@ -2326,7 +2326,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
@@ -2335,11 +2340,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

spec/requests/api/variables_ee_spec.rb

+require 'spec_helper'
+
+describe API::Variables do
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+
+  describe 'POST /projects/:id/variables' do
+    context 'with variable environment scope available' do
+      before do
+        stub_licensed_features(variable_environment_scope: true)
+
+        project.add_master(user)
+      end
+
+      it 'creates variable with a specific environment scope' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*'
+        end.to change { project.variables(true).count }.by(1)
+
+        expect(response).to have_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), key: variable.key, value: 'VALUE_2', environment_scope: 'review/*'
+        end.to change { project.variables(true).count }.by(1)
+
+        expect(response).to have_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
+  end
+end