From 96956d47ef94dd69537105dfe67e3c5e74f3a1e6 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 24 May 2017 16:18:05 +0000
Subject: [PATCH 01/17] Backend implementation for protected variables

---
 app/models/ci/build.rb                         |  1 +
 app/models/project.rb                          | 18 +++++++++++++++---
 ...0524161101_add_protected_to_ci_variables.rb | 15 +++++++++++++++
 db/schema.rb                                   |  3 ++-
 4 files changed, 33 insertions(+), 4 deletions(-)
 create mode 100644 db/migrate/20170524161101_add_protected_to_ci_variables.rb

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 760ec8e5919..0dfc192bc7d 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -186,6 +186,7 @@ module Ci
       variables += yaml_variables
       variables += user_variables
       variables += project.secret_variables
+      variables += project.protected_variables if ProtectedBranch.protected?(project, ref)
       variables += trigger_request.user_variables if trigger_request
       variables
     end
diff --git a/app/models/project.rb b/app/models/project.rb
index cfca0dcd2f2..90586825f3f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1257,9 +1257,15 @@ class Project < ActiveRecord::Base
   end
 
   def secret_variables
-    variables.map do |variable|
-      { key: variable.key, value: variable.value, public: false }
-    end
+    filtered_variables = variables.to_a.reject(&:protected?)
+
+    build_variables(filtered_variables)
+  end
+
+  def protected_variables
+    filtered_variables = variables.to_a.select(&:protected?)
+
+    build_variables(filtered_variables)
   end
 
   def deployment_variables
@@ -1412,4 +1418,10 @@ class Project < ActiveRecord::Base
 
     raise ex
   end
+
+  def build_variables(filtered_variables)
+    filtered_variables.map do |variable|
+      { key: variable.key, value: variable.value, public: false }
+    end
+  end
 end
diff --git a/db/migrate/20170524161101_add_protected_to_ci_variables.rb b/db/migrate/20170524161101_add_protected_to_ci_variables.rb
new file mode 100644
index 00000000000..99d4861e889
--- /dev/null
+++ b/db/migrate/20170524161101_add_protected_to_ci_variables.rb
@@ -0,0 +1,15 @@
+class AddProtectedToCiVariables < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:ci_variables, :protected, :boolean, default: false)
+  end
+
+  def down
+    remove_column(:ci_variables, :protected)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 84e25427d7f..60a95408ac2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170521184006) do
+ActiveRecord::Schema.define(version: 20170524161101) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -355,6 +355,7 @@ ActiveRecord::Schema.define(version: 20170521184006) do
     t.string "encrypted_value_salt"
     t.string "encrypted_value_iv"
     t.integer "project_id", null: false
+    t.boolean "protected", default: false
   end
 
   add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree

From 9f6dc8b2b5fe3d4790d67f13660eb8bfabd4dbca Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 25 May 2017 18:05:27 +0800
Subject: [PATCH 02/17] Add tests and also pass protected vars to protected
 tags

---
 app/models/ci/build.rb         |  4 +++-
 spec/factories/ci/variables.rb |  4 ++++
 spec/models/ci/build_spec.rb   | 41 ++++++++++++++++++++++++++++++----
 spec/models/project_spec.rb    | 30 +++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 5 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 0dfc192bc7d..a43c8243bcb 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -186,7 +186,9 @@ module Ci
       variables += yaml_variables
       variables += user_variables
       variables += project.secret_variables
-      variables += project.protected_variables if ProtectedBranch.protected?(project, ref)
+      variables += project.protected_variables if
+        ProtectedBranch.protected?(project, ref) ||
+          ProtectedTag.protected?(project, ref)
       variables += trigger_request.user_variables if trigger_request
       variables
     end
diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
index c5fba597c1c..f83366136fd 100644
--- a/spec/factories/ci/variables.rb
+++ b/spec/factories/ci/variables.rb
@@ -3,6 +3,10 @@ FactoryGirl.define do
     sequence(:key) { |n| "VARIABLE_#{n}" }
     value 'VARIABLE_VALUE'
 
+    trait(:protected) do
+      protected true
+    end
+
     project factory: :empty_project
   end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index e971b4bc3f9..0cc1fc2b360 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1215,16 +1215,49 @@ describe Ci::Build, :models do
       it { is_expected.to include(tag_variable) }
     end
 
-    context 'when secure variable is defined' do
-      let(:secure_variable) do
+    context 'when secret variable is defined' do
+      let(:secret_variable) do
         { key: 'SECRET_KEY', value: 'secret_value', public: false }
       end
 
       before do
-        build.project.variables << Ci::Variable.new(key: 'SECRET_KEY', value: 'secret_value')
+        create(:ci_variable,
+               secret_variable.slice(:key, :value).merge(project: project))
       end
 
-      it { is_expected.to include(secure_variable) }
+      it { is_expected.to include(secret_variable) }
+    end
+
+    context 'when protected variable is defined' do
+      let(:protected_variable) do
+        { key: 'PROTECTED_KEY', value: 'protected_value', public: false }
+      end
+
+      before do
+        create(:ci_variable,
+               :protected,
+               protected_variable.slice(:key, :value).merge(project: project))
+      end
+
+      context 'when the branch is protected' do
+        before do
+          create(:protected_branch, project: build.project, name: build.ref)
+        end
+
+        it { is_expected.to include(protected_variable) }
+      end
+
+      context 'when the tag is protected' do
+        before do
+          create(:protected_tag, project: build.project, name: build.ref)
+        end
+
+        it { is_expected.to include(protected_variable) }
+      end
+
+      context 'when the ref is not protected' do
+        it { is_expected.not_to include(protected_variable) }
+      end
     end
 
     context 'when build is for triggers' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f2b4e9070b4..8478df059bc 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1710,6 +1710,36 @@ describe Project, models: true do
     end
   end
 
+  describe 'variables' 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
+
+    describe '#secret_variables' do
+      it 'contains only the secret variables' do
+        expect(project.secret_variables).to eq(
+          [{ key: secret_variable.key,
+             value: secret_variable.value,
+             public: false } ])
+      end
+    end
+
+    describe '#protected_variables' do
+      it 'contains only the protected variables' do
+        expect(project.protected_variables).to eq(
+          [{ key: protected_variable.key,
+             value: protected_variable.value,
+             public: false } ])
+      end
+    end
+  end
+
   describe '#update_project_statistics' do
     let(:project) { create(:empty_project) }
 

From efebdba21db9e11b876ecb54db48358e13b08ad3 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 25 May 2017 19:31:21 +0800
Subject: [PATCH 03/17] Frontend implementation, tests, and changelog

---
 .../projects/variables_controller.rb          |  3 +-
 .../pipelines_settings/_show.html.haml        |  2 +-
 app/views/projects/variables/_form.html.haml  |  9 ++++
 app/views/projects/variables/_table.html.haml |  3 ++
 .../unreleased/24196-protected-variables.yml  |  5 +++
 lib/gitlab/utils.rb                           |  8 ++++
 spec/features/variables_spec.rb               | 44 ++++++++++++++++++-
 spec/lib/gitlab/utils_spec.rb                 |  5 +++
 spec/models/project_spec.rb                   |  4 +-
 9 files changed, 78 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/24196-protected-variables.yml

diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb
index a4d1b1ee69b..0953eecaeb5 100644
--- a/app/controllers/projects/variables_controller.rb
+++ b/app/controllers/projects/variables_controller.rb
@@ -42,6 +42,7 @@ class Projects::VariablesController < Projects::ApplicationController
   private
 
   def project_params
-    params.require(:variable).permit([:id, :key, :value, :_destroy])
+    params.require(:variable)
+      .permit([:id, :key, :value, :protected, :_destroy])
   end
 end
diff --git a/app/views/projects/pipelines_settings/_show.html.haml b/app/views/projects/pipelines_settings/_show.html.haml
index 1b1910b5c0f..3b17daeb6da 100644
--- a/app/views/projects/pipelines_settings/_show.html.haml
+++ b/app/views/projects/pipelines_settings/_show.html.haml
@@ -42,7 +42,7 @@
           = f.label :build_timeout_in_minutes, 'Timeout', class: 'label-light'
           = f.number_field :build_timeout_in_minutes, class: 'form-control', min: '0'
           %p.help-block
-            Per job in minutes. If a job passes this threshold, it will be marked as failed.
+            Per job in minutes. If a job passes this threshold, it will be marked as failed
             = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank'
 
         %hr
diff --git a/app/views/projects/variables/_form.html.haml b/app/views/projects/variables/_form.html.haml
index 1ae86d258af..809628bc491 100644
--- a/app/views/projects/variables/_form.html.haml
+++ b/app/views/projects/variables/_form.html.haml
@@ -7,4 +7,13 @@
   .form-group
     = f.label :value, "Value", class: "label-light"
     = f.text_area :value, class: "form-control", placeholder: "PROJECT_VARIABLE"
+  .form-group
+    .checkbox
+      = f.label :protected do
+        = f.check_box :protected
+        %strong Protected
+      .help-block
+        This variable will be passed only to pipelines running on protected branches and tags
+        = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'protected-variables'), target: '_blank'
+
   = f.submit btn_text, class: "btn btn-save"
diff --git a/app/views/projects/variables/_table.html.haml b/app/views/projects/variables/_table.html.haml
index 0ce597dcf21..59cd3c4b592 100644
--- a/app/views/projects/variables/_table.html.haml
+++ b/app/views/projects/variables/_table.html.haml
@@ -1,12 +1,14 @@
 .table-responsive.variables-table
   %table.table
     %colgroup
+      %col
       %col
       %col
       %col{ width: 100 }
     %thead
       %th Key
       %th Value
+      %th Protected
       %th
     %tbody
       - @project.variables.order_key_asc.each do |variable|
@@ -14,6 +16,7 @@
           %tr
             %td.variable-key= variable.key
             %td.variable-value{ "data-value" => variable.value }******
+            %td.variable-protected= Gitlab::Utils.boolean_to_yes_no(variable.protected)
             %td.variable-menu
               = link_to namespace_project_variable_path(@project.namespace, @project, variable), class: "btn btn-transparent btn-variable-edit" do
                 %span.sr-only
diff --git a/changelogs/unreleased/24196-protected-variables.yml b/changelogs/unreleased/24196-protected-variables.yml
new file mode 100644
index 00000000000..71567a9d794
--- /dev/null
+++ b/changelogs/unreleased/24196-protected-variables.yml
@@ -0,0 +1,5 @@
+---
+title: Add protected variables which would only be passed to protected branches or
+  protected tags
+merge_request: 11688
+author:
diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb
index 4c395b4266e..fa182c4deda 100644
--- a/lib/gitlab/utils.rb
+++ b/lib/gitlab/utils.rb
@@ -21,5 +21,13 @@ module Gitlab
 
       nil
     end
+
+    def boolean_to_yes_no(bool)
+      if bool
+        'Yes'
+      else
+        'No'
+      end
+    end
   end
 end
diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index b83a230c1f8..08a2df63a40 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -19,7 +19,7 @@ describe 'Project variables', js: true do
     end
   end
 
-  it 'adds new variable' do
+  it 'adds new secret variable' do
     fill_in('variable_key', with: 'key')
     fill_in('variable_value', with: 'key value')
     click_button('Add new variable')
@@ -27,6 +27,7 @@ describe 'Project variables', js: true do
     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('No')
     end
   end
 
@@ -41,6 +42,19 @@ describe 'Project variables', js: true do
     end
   end
 
+  it 'adds new protected variable' do
+    fill_in('variable_key', with: 'key')
+    fill_in('variable_value', with: 'value')
+    check('Protected')
+    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('Yes')
+    end
+  end
+
   it 'reveals and hides new variable' do
     fill_in('variable_key', with: 'key')
     fill_in('variable_value', with: 'key value')
@@ -100,4 +114,32 @@ describe 'Project variables', js: true do
     expect(page).to have_content('Variable was successfully updated.')
     expect(project.variables.first.value).to eq('')
   end
+
+  it 'edits variable to be protected' do
+    page.within('.variables-table') do
+      find('.btn-variable-edit').click
+    end
+
+    expect(page).to have_content('Update variable')
+    check('Protected')
+    click_button('Save variable')
+
+    expect(page).to have_content('Variable was successfully updated.')
+    expect(project.variables.first).to be_protected
+  end
+
+  it 'edits variable to be unprotected' do
+    project.variables.first.update(protected: true)
+
+    page.within('.variables-table') do
+      find('.btn-variable-edit').click
+    end
+
+    expect(page).to have_content('Update variable')
+    check('Protected')
+    click_button('Save variable')
+
+    expect(page).to have_content('Variable was successfully updated.')
+    expect(project.variables.first).not_to be_protected
+  end
 end
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index 56772409989..b1959550a4a 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -29,5 +29,10 @@ describe Gitlab::Utils, lib: true do
       expect(to_boolean('')).to be_nil
       expect(to_boolean(nil)).to be_nil
     end
+
+    it 'converts booleans to Yes or No' do
+      expect(boolean_to_yes_no(true)).to eq('Yes')
+      expect(boolean_to_yes_no(false)).to eq('No')
+    end
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 8478df059bc..b9094387865 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1726,7 +1726,7 @@ describe Project, models: true do
         expect(project.secret_variables).to eq(
           [{ key: secret_variable.key,
              value: secret_variable.value,
-             public: false } ])
+             public: false }])
       end
     end
 
@@ -1735,7 +1735,7 @@ describe Project, models: true do
         expect(project.protected_variables).to eq(
           [{ key: protected_variable.key,
              value: protected_variable.value,
-             public: false } ])
+             public: false }])
       end
     end
   end

From bd66bf08b5b9a2cea9320100a91240e93f1c06e5 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 25 May 2017 19:49:46 +0800
Subject: [PATCH 04/17] API and doc for protected variables

---
 doc/api/build_variables.md          | 28 ++++++++++++++++------------
 lib/api/entities.rb                 |  1 +
 lib/api/variables.rb                |  4 +++-
 spec/requests/api/variables_spec.rb |  7 +++++--
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/doc/api/build_variables.md b/doc/api/build_variables.md
index 2aaf1c93705..d4f00256ed3 100644
--- a/doc/api/build_variables.md
+++ b/doc/api/build_variables.md
@@ -61,11 +61,12 @@ Create a new build variable.
 POST /projects/:id/variables
 ```
 
-| Attribute | Type    | required | Description           |
-|-----------|---------|----------|-----------------------|
-| `id`      | integer/string | yes      | The ID of a project or [urlencoded NAMESPACE/PROJECT_NAME of the project](README.md#namespaced-path-encoding) owned by the authenticated user   |
-| `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 |
+| Attribute   | Type    | required | Description           |
+|-------------|---------|----------|-----------------------|
+| `id`        | integer/string | yes      | The ID of a project or [urlencoded NAMESPACE/PROJECT_NAME of the project](README.md#namespaced-path-encoding) owned by the authenticated user   |
+| `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 |
 
 ```
 curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/variables" --form "key=NEW_VARIABLE" --form "value=new value"
@@ -74,7 +75,8 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitl
 ```json
 {
     "key": "NEW_VARIABLE",
-    "value": "new value"
+    "value": "new value",
+    "protected": false
 }
 ```
 
@@ -86,11 +88,12 @@ Update a project's build variable.
 PUT /projects/:id/variables/:key
 ```
 
-| Attribute | Type    | required | Description             |
-|-----------|---------|----------|-------------------------|
-| `id`      | integer/string | yes      | The ID of a project or [urlencoded NAMESPACE/PROJECT_NAME of the project](README.md#namespaced-path-encoding) owned by the authenticated user     |
-| `key`     | string  | yes      | The `key` of a variable   |
-| `value`   | string  | yes      | The `value` of a variable |
+| Attribute   | Type    | required | Description             |
+|-------------|---------|----------|-------------------------|
+| `id`        | integer/string | yes      | The ID of a project or [urlencoded NAMESPACE/PROJECT_NAME of the project](README.md#namespaced-path-encoding) owned by the authenticated user     |
+| `key`       | string  | yes      | The `key` of a variable   |
+| `value`     | string  | yes      | The `value` of a variable |
+| `protected` | boolean | no       | Whether the variable is protected |
 
 ```
 curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/variables/NEW_VARIABLE" --form "value=updated value"
@@ -99,7 +102,8 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitla
 ```json
 {
     "key": "NEW_VARIABLE",
-    "value": "updated value"
+    "value": "updated value",
+    "protected": true
 }
 ```
 
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 01cc8e8e1ca..b5308aeecf6 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -670,6 +670,7 @@ module API
 
     class Variable < Grape::Entity
       expose :key, :value
+      expose :protected?, as: :protected
     end
 
     class Pipeline < PipelineBasic
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index 5acde41551b..381c4ef50b0 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -42,6 +42,7 @@ module API
       params do
         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'
       end
       post ':id/variables' do
         variable = user_project.variables.create(declared(params, include_parent_namespaces: false).to_h)
@@ -59,13 +60,14 @@ module API
       params do
         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'
       end
       put ':id/variables/:key' do
         variable = user_project.variables.find_by(key: params[:key])
 
         return not_found!('Variable') unless variable
 
-        if variable.update(value: params[:value])
+        if variable.update(declared_params(include_missing: false).except(:key))
           present variable, with: Entities::Variable
         else
           render_validation_error!(variable)
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 63d6d3001ac..0c83fb41525 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -42,6 +42,7 @@ describe API::Variables do
 
         expect(response).to have_http_status(200)
         expect(json_response['value']).to eq(variable.value)
+        expect(json_response['protected']).to eq(variable.protected?)
       end
 
       it 'responds with 404 Not Found if requesting non-existing variable' do
@@ -72,12 +73,13 @@ describe API::Variables do
     context 'authorized user with proper permissions' do
       it 'creates variable' do
         expect do
-          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2'
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true
         end.to change{project.variables.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['protected']).to eq(true)
       end
 
       it 'does not allow to duplicate variable key' do
@@ -112,13 +114,14 @@ describe API::Variables do
         initial_variable = project.variables.first
         value_before = initial_variable.value
 
-        put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP'
+        put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP', protected: true
 
         updated_variable = project.variables.first
 
         expect(response).to have_http_status(200)
         expect(value_before).to eq(variable.value)
         expect(updated_variable.value).to eq('VALUE_1_UP')
+        expect(updated_variable).to be_protected
       end
 
       it 'responds with 404 Not Found if requesting non-existing variable' do

From ef10f4c53139ead567b12a2daf8832f02240d0fa Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 25 May 2017 20:29:12 +0800
Subject: [PATCH 05/17] Add docs for protected variables

---
 .../projects/variables/_content.html.haml     |  2 +-
 doc/ci/variables/README.md                    | 23 ++++++++++++++++---
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/app/views/projects/variables/_content.html.haml b/app/views/projects/variables/_content.html.haml
index 06477aba103..16e5a21dfc1 100644
--- a/app/views/projects/variables/_content.html.haml
+++ b/app/views/projects/variables/_content.html.haml
@@ -1,5 +1,5 @@
 %h4.prepend-top-0
-  Secret Variables
+  Secret and protected variables
 %p
   These variables will be set to environment by the runner.
 %p
diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 0d4d08106f8..2f2023c02ae 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -11,6 +11,7 @@ this order:
 
 1. [Trigger variables][triggers] (take precedence over all)
 1. [Secret variables](#secret-variables)
+1. [Protected variables](#protected-variables)
 1. YAML-defined [job-level variables](../yaml/README.md#job-variables)
 1. YAML-defined [global variables](../yaml/README.md#variables)
 1. [Deployment variables](#deployment-variables)
@@ -153,9 +154,26 @@ storing things like passwords, secret keys and credentials.
 
 Secret variables can be added by going to your project's
 **Settings ➔ Pipelines**, then finding the section called
-**Secret Variables**.
+**Secret and protected variables**.
 
-Once you set them, they will be available for all subsequent jobs.
+Once you set them, they will be available for all subsequent pipelines.
+
+## Protected variables
+
+>**Notes:**
+- This feature requires GitLab Runner 0.4.0 or higher.
+- A protected variable is a secret variable which is protected.
+
+All secret variables could be protected. Whenever a secret variable is
+protected, it would only be securely passed to pipelines running on the
+protected branches or protected tags. The other pipelines would not get any
+protected variables.
+
+Protected variables can be added by going to your project's
+**Settings ➔ Pipelines**, then finding the section called
+**Secret and protected variables**, and check *Protected*.
+
+Once you set them, they will be available for all subsequent pipelines.
 
 ## Deployment variables
 
@@ -381,7 +399,6 @@ export CI_REGISTRY_USER="gitlab-ci-token"
 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

From 7f0116768188118d9291de3dc2f62af2d40795b9 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 25 May 2017 20:53:03 +0800
Subject: [PATCH 06/17] Fix tests and rubocop offense

---
 app/models/ci/build.rb          | 2 +-
 spec/features/variables_spec.rb | 4 ++--
 spec/lib/gitlab/utils_spec.rb   | 6 +++++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index a43c8243bcb..81be74a5f23 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -188,7 +188,7 @@ module Ci
       variables += project.secret_variables
       variables += project.protected_variables if
         ProtectedBranch.protected?(project, ref) ||
-          ProtectedTag.protected?(project, ref)
+            ProtectedTag.protected?(project, ref)
       variables += trigger_request.user_variables if trigger_request
       variables
     end
diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index 08a2df63a40..5a5922d5f02 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -136,10 +136,10 @@ describe 'Project variables', js: true do
     end
 
     expect(page).to have_content('Update variable')
-    check('Protected')
+    uncheck('Protected')
     click_button('Save variable')
 
     expect(page).to have_content('Variable was successfully updated.')
-    expect(project.variables.first).not_to be_protected
+    expect(project.reload.variables.first).not_to be_protected
   end
 end
diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb
index b1959550a4a..00941aec380 100644
--- a/spec/lib/gitlab/utils_spec.rb
+++ b/spec/lib/gitlab/utils_spec.rb
@@ -1,5 +1,7 @@
+require 'spec_helper'
+
 describe Gitlab::Utils, lib: true do
-  delegate :to_boolean, to: :described_class
+  delegate :to_boolean, :boolean_to_yes_no, to: :described_class
 
   describe '.to_boolean' do
     it 'accepts booleans' do
@@ -29,7 +31,9 @@ describe Gitlab::Utils, lib: true do
       expect(to_boolean('')).to be_nil
       expect(to_boolean(nil)).to be_nil
     end
+  end
 
+  describe '.boolean_to_yes_no' do
     it 'converts booleans to Yes or No' do
       expect(boolean_to_yes_no(true)).to eq('Yes')
       expect(boolean_to_yes_no(false)).to eq('No')

From 9cc918a5caca931887026d258ea1dcd6499d7c2f Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Fri, 26 May 2017 15:35:30 +0800
Subject: [PATCH 07/17] Use be_truthy and add back missing link

---
 doc/ci/variables/README.md          | 1 +
 spec/requests/api/variables_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 2f2023c02ae..b431cb41f4c 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -399,6 +399,7 @@ export CI_REGISTRY_USER="gitlab-ci-token"
 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
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 0c83fb41525..83673864fe7 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -79,7 +79,7 @@ describe API::Variables do
         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['protected']).to eq(true)
+        expect(json_response['protected']).to be_truthy
       end
 
       it 'does not allow to duplicate variable key' do

From 2785bc4faa0eb5c46b0c8b46b77aa4cf1d4ee4b4 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Sat, 27 May 2017 01:46:57 +0800
Subject: [PATCH 08/17] Merge secret and protected vars to variables_for(ref)

Also introduce Ci::Variable#to_runner_variable to
build up the hash for runner.
---
 app/models/ci/build.rb          |  5 +---
 app/models/ci/variable.rb       |  4 +++
 app/models/project.rb           | 23 ++++++-----------
 spec/models/ci/build_spec.rb    |  2 +-
 spec/models/ci/variable_spec.rb |  7 ++++++
 spec/models/project_spec.rb     | 44 +++++++++++++++++++++++----------
 6 files changed, 52 insertions(+), 33 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 81be74a5f23..4e8f095e35b 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -185,10 +185,7 @@ module Ci
       variables += project.deployment_variables if has_environment?
       variables += yaml_variables
       variables += user_variables
-      variables += project.secret_variables
-      variables += project.protected_variables if
-        ProtectedBranch.protected?(project, ref) ||
-            ProtectedTag.protected?(project, ref)
+      variables += project.variables_for(ref)
       variables += trigger_request.user_variables if trigger_request
       variables
     end
diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 6c6586110c5..31eedb117fa 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -18,5 +18,9 @@ module Ci
        insecure_mode: true,
        key: Gitlab::Application.secrets.db_key_base,
        algorithm: 'aes-256-cbc'
+
+    def to_runner_variable
+      { key: key, value: value, public: false }
+    end
   end
 end
diff --git a/app/models/project.rb b/app/models/project.rb
index 90586825f3f..e85f9020563 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1256,16 +1256,15 @@ class Project < ActiveRecord::Base
     variables
   end
 
-  def secret_variables
-    filtered_variables = variables.to_a.reject(&:protected?)
+  def variables_for(ref)
+    vars = if ProtectedBranch.protected?(self, ref) ||
+               ProtectedTag.protected?(self, ref)
+             variables.to_a
+           else
+             variables.to_a.reject(&:protected?)
+           end
 
-    build_variables(filtered_variables)
-  end
-
-  def protected_variables
-    filtered_variables = variables.to_a.select(&:protected?)
-
-    build_variables(filtered_variables)
+    vars.map(&:to_runner_variable)
   end
 
   def deployment_variables
@@ -1418,10 +1417,4 @@ class Project < ActiveRecord::Base
 
     raise ex
   end
-
-  def build_variables(filtered_variables)
-    filtered_variables.map do |variable|
-      { key: variable.key, value: variable.value, public: false }
-    end
-  end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 0cc1fc2b360..6e7aa3d5841 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1384,7 +1384,7 @@ describe Ci::Build, :models do
         allow(project).to receive(:predefined_variables) { ['project'] }
         allow(pipeline).to receive(:predefined_variables) { ['pipeline'] }
         allow(build).to receive(:yaml_variables) { ['yaml'] }
-        allow(project).to receive(:secret_variables) { ['secret'] }
+        allow(project).to receive(:variables_for).with(build.ref) { ['secret'] }
       end
 
       it { is_expected.to eq(%w[predefined project pipeline yaml secret]) }
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index fe8c52d5353..38b869f59ae 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -36,4 +36,11 @@ describe Ci::Variable, models: true do
         to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt')
     end
   end
+
+  describe '#to_runner_variable' do
+    it 'returns a hash for the runner' do
+      expect(subject.to_runner_variable)
+        .to eq(key: subject.key, value: subject.value, public: false)
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b9094387865..7e5e6e899e2 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1710,7 +1710,7 @@ describe Project, models: true do
     end
   end
 
-  describe 'variables' do
+  describe '#variables_for' do
     let(:project) { create(:empty_project) }
 
     let!(:secret_variable) do
@@ -1721,22 +1721,40 @@ describe Project, models: true do
       create(:ci_variable, :protected, value: 'protected', project: project)
     end
 
-    describe '#secret_variables' do
-      it 'contains only the secret variables' do
-        expect(project.secret_variables).to eq(
-          [{ key: secret_variable.key,
-             value: secret_variable.value,
-             public: false }])
+    subject { project.variables_for('ref') }
+
+    shared_examples 'ref is protected' do
+      it 'contains all the variables' do
+        is_expected.to contain_exactly(
+          *[secret_variable, protected_variable].map(&:to_runner_variable))
       end
     end
 
-    describe '#protected_variables' do
-      it 'contains only the protected variables' do
-        expect(project.protected_variables).to eq(
-          [{ key: protected_variable.key,
-             value: protected_variable.value,
-             public: false }])
+    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.to_runner_variable)
+      end
+    end
+
+    context 'when the ref is a protected branch' do
+      before do
+        create(:protected_branch, name: 'ref', project: project)
+      end
+
+      it_behaves_like 'ref is protected'
+    end
+
+    context 'when the ref is a protected tag' do
+      before do
+        create(:protected_tag, name: 'ref', project: project)
+      end
+
+      it_behaves_like 'ref is protected'
     end
   end
 

From 8f44bc4dc10caf3c9856a8e4bea5ac145a315131 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Sat, 27 May 2017 02:17:46 +0800
Subject: [PATCH 09/17] Rubocop makes more sense with this

---
 app/models/project.rb | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index e85f9020563..fbf2a0a75ca 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1257,12 +1257,13 @@ class Project < ActiveRecord::Base
   end
 
   def variables_for(ref)
-    vars = if ProtectedBranch.protected?(self, ref) ||
-               ProtectedTag.protected?(self, ref)
-             variables.to_a
-           else
-             variables.to_a.reject(&:protected?)
-           end
+    vars =
+      if ProtectedBranch.protected?(self, ref) ||
+          ProtectedTag.protected?(self, ref)
+        variables.to_a
+      else
+        variables.to_a.reject(&:protected?)
+      end
 
     vars.map(&:to_runner_variable)
   end

From c4dded593a9df770dd08051fc645f713ca295f13 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 31 May 2017 22:45:51 +0800
Subject: [PATCH 10/17] Update docs and use protected secret variable as the
 name

---
 app/models/ci/build.rb                        |  2 +-
 app/models/ci/variable.rb                     |  1 +
 app/models/project.rb                         | 19 ++++++-------
 .../projects/variables/_content.html.haml     |  5 ++--
 app/views/projects/variables/_form.html.haml  |  2 +-
 db/schema.rb                                  |  2 +-
 doc/ci/variables/README.md                    | 15 ++++++-----
 spec/models/ci/build_spec.rb                  | 27 ++++++++++++++-----
 spec/models/project_spec.rb                   |  4 +--
 9 files changed, 48 insertions(+), 29 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 4e8f095e35b..b83068467ec 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -185,7 +185,7 @@ module Ci
       variables += project.deployment_variables if has_environment?
       variables += yaml_variables
       variables += user_variables
-      variables += project.variables_for(ref)
+      variables += project.secret_variables_for(ref).map(&:to_runner_variable)
       variables += trigger_request.user_variables if trigger_request
       variables
     end
diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 31eedb117fa..f235260208f 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -12,6 +12,7 @@ module Ci
                 message: "can contain only letters, digits and '_'." }
 
     scope :order_key_asc, -> { reorder(key: :asc) }
+    scope :unprotected, -> { where(protected: false) }
 
     attr_encrypted :value,
        mode: :per_attribute_iv_and_salt,
diff --git a/app/models/project.rb b/app/models/project.rb
index 6892ff1e2d8..2922bebbaa7 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1253,16 +1253,17 @@ class Project < ActiveRecord::Base
     variables
   end
 
-  def variables_for(ref)
-    vars =
-      if ProtectedBranch.protected?(self, ref) ||
-          ProtectedTag.protected?(self, ref)
-        variables.to_a
-      else
-        variables.to_a.reject(&:protected?)
-      end
+  def secret_variables_for(ref)
+    if protected_for?(ref)
+      variables
+    else
+      variables.unprotected
+    end
+  end
 
-    vars.map(&:to_runner_variable)
+  def protected_for?(ref)
+    ProtectedBranch.protected?(self, ref) ||
+      ProtectedTag.protected?(self, ref)
   end
 
   def deployment_variables
diff --git a/app/views/projects/variables/_content.html.haml b/app/views/projects/variables/_content.html.haml
index 16e5a21dfc1..98f618ca3b8 100644
--- a/app/views/projects/variables/_content.html.haml
+++ b/app/views/projects/variables/_content.html.haml
@@ -1,7 +1,8 @@
 %h4.prepend-top-0
-  Secret and protected variables
+  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.
+  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
diff --git a/app/views/projects/variables/_form.html.haml b/app/views/projects/variables/_form.html.haml
index 809628bc491..0a70a301cb4 100644
--- a/app/views/projects/variables/_form.html.haml
+++ b/app/views/projects/variables/_form.html.haml
@@ -14,6 +14,6 @@
         %strong Protected
       .help-block
         This variable will be passed only to pipelines running on protected branches and tags
-        = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'protected-variables'), target: '_blank'
+        = link_to icon('question-circle'), help_page_path('ci/variables/README', anchor: 'protected-secret-variables'), target: '_blank'
 
   = f.submit btn_text, class: "btn btn-save"
diff --git a/db/schema.rb b/db/schema.rb
index 59f4e4b2961..679f5c358cd 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1474,4 +1474,4 @@ ActiveRecord::Schema.define(version: 20170524161101) do
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
   add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
-end
\ No newline at end of file
+end
diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index b431cb41f4c..602e2aa5df1 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -154,24 +154,23 @@ storing things like passwords, secret keys and credentials.
 
 Secret variables can be added by going to your project's
 **Settings ➔ Pipelines**, then finding the section called
-**Secret and protected variables**.
+**Secret variables**.
 
 Once you set them, they will be available for all subsequent pipelines.
 
-## Protected variables
+## Protected secret variables
 
 >**Notes:**
-- This feature requires GitLab Runner 0.4.0 or higher.
-- A protected variable is a secret variable which is protected.
+- This feature requires GitLab 9.3 or higher, and GitLab Runner 0.4.0 or higher.
 
-All secret variables could be protected. Whenever a secret variable is
+Secret variables could be protected. Whenever a secret variable is
 protected, it would only be securely passed to pipelines running on the
-protected branches or protected tags. The other pipelines would not get any
+[protected branches] or [protected tags]. The other pipelines would not get any
 protected variables.
 
 Protected variables can be added by going to your project's
 **Settings ➔ Pipelines**, then finding the section called
-**Secret and protected variables**, and check *Protected*.
+**Secret variables**, and check *Protected*.
 
 Once you set them, they will be available for all subsequent pipelines.
 
@@ -403,3 +402,5 @@ export CI_REGISTRY_PASSWORD="longalfanumstring"
 [runner]: https://docs.gitlab.com/runner/
 [triggered]: ../triggers/README.md
 [triggers]: ../triggers/README.md#pass-job-variables-to-a-trigger
+[protected branches]: ../../user/project/protected_branches.md
+[protected tags]: ../../user/project/protected_tags.md
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 6e7aa3d5841..e2406290c6c 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1379,15 +1379,30 @@ describe Ci::Build, :models do
     end
 
     context 'returns variables in valid order' do
+      let(:build_pre_var) { { key: 'build', value: 'value' } }
+      let(:project_pre_var) { { key: 'project', value: 'value' } }
+      let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } }
+      let(:build_yaml_var) { { key: 'yaml', value: 'value' } }
+
       before do
-        allow(build).to receive(:predefined_variables) { ['predefined'] }
-        allow(project).to receive(:predefined_variables) { ['project'] }
-        allow(pipeline).to receive(:predefined_variables) { ['pipeline'] }
-        allow(build).to receive(:yaml_variables) { ['yaml'] }
-        allow(project).to receive(:variables_for).with(build.ref) { ['secret'] }
+        allow(build).to receive(:predefined_variables) { [build_pre_var] }
+        allow(project).to receive(:predefined_variables) { [project_pre_var] }
+        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
       end
 
-      it { is_expected.to eq(%w[predefined project pipeline yaml secret]) }
+      it do
+        is_expected.to eq(
+          [build_pre_var,
+           project_pre_var,
+           pipeline_pre_var,
+           build_yaml_var,
+           { key: 'secret', value: 'value', public: false }])
+      end
     end
   end
 
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 38964f278f3..36140b519d6 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1735,7 +1735,7 @@ describe Project, models: true do
     end
   end
 
-  describe '#variables_for' do
+  describe '#secret_variables_for' do
     let(:project) { create(:empty_project) }
 
     let!(:secret_variable) do
@@ -1746,7 +1746,7 @@ describe Project, models: true do
       create(:ci_variable, :protected, value: 'protected', project: project)
     end
 
-    subject { project.variables_for('ref') }
+    subject { project.secret_variables_for('ref') }
 
     shared_examples 'ref is protected' do
       it 'contains all the variables' do

From 76e738fc0ec65513cb4d92a8206109184c74a381 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 31 May 2017 22:52:37 +0800
Subject: [PATCH 11/17] Fix doc table of contents

---
 doc/ci/variables/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 602e2aa5df1..71f2088be74 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -11,7 +11,7 @@ this order:
 
 1. [Trigger variables][triggers] (take precedence over all)
 1. [Secret variables](#secret-variables)
-1. [Protected variables](#protected-variables)
+1. [Protected secret variables](#protected-secret-variables)
 1. YAML-defined [job-level variables](../yaml/README.md#job-variables)
 1. YAML-defined [global variables](../yaml/README.md#variables)
 1. [Deployment variables](#deployment-variables)

From 65563c7b1b64d3ab7133896ba291a3efc7afba86 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 1 Jun 2017 00:12:42 +0800
Subject: [PATCH 12/17] Now secret_variables_for would return the variables

---
 spec/models/project_spec.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 36140b519d6..a953faaaedf 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1750,8 +1750,7 @@ describe Project, models: true do
 
     shared_examples 'ref is protected' do
       it 'contains all the variables' do
-        is_expected.to contain_exactly(
-          *[secret_variable, protected_variable].map(&:to_runner_variable))
+        is_expected.to contain_exactly(secret_variable, protected_variable)
       end
     end
 
@@ -1762,7 +1761,7 @@ describe Project, models: true do
       end
 
       it 'contains only the secret variables' do
-        is_expected.to contain_exactly(secret_variable.to_runner_variable)
+        is_expected.to contain_exactly(secret_variable)
       end
     end
 

From 8a1a73a75a4410115b3cc5a85bf5fd85cb634182 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 1 Jun 2017 00:37:44 +0800
Subject: [PATCH 13/17] Make sure we're loading the fresh variables

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11688#note_31186872
---
 spec/features/variables_spec.rb | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index 5a5922d5f02..d0c982919db 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -99,7 +99,7 @@ describe 'Project variables', js: true do
     click_button('Save variable')
 
     expect(page).to have_content('Variable was successfully updated.')
-    expect(project.variables.first.value).to eq('key value')
+    expect(project.variables(true).first.value).to eq('key value')
   end
 
   it 'edits variable with empty value' do
@@ -112,7 +112,7 @@ describe 'Project variables', js: true do
     click_button('Save variable')
 
     expect(page).to have_content('Variable was successfully updated.')
-    expect(project.variables.first.value).to eq('')
+    expect(project.variables(true).first.value).to eq('')
   end
 
   it 'edits variable to be protected' do
@@ -125,7 +125,7 @@ describe 'Project variables', js: true do
     click_button('Save variable')
 
     expect(page).to have_content('Variable was successfully updated.')
-    expect(project.variables.first).to be_protected
+    expect(project.variables(true).first).to be_protected
   end
 
   it 'edits variable to be unprotected' do
@@ -140,6 +140,6 @@ describe 'Project variables', js: true do
     click_button('Save variable')
 
     expect(page).to have_content('Variable was successfully updated.')
-    expect(project.reload.variables.first).not_to be_protected
+    expect(project.variables(true).first).not_to be_protected
   end
 end

From 17c926313a242c6ad988a7ffd55aa6330c8aacfd Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 1 Jun 2017 03:49:10 +0800
Subject: [PATCH 14/17] Add test for Project#protected_for?

---
 spec/models/project_spec.rb | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 360fcae29a5..86ab2550bfb 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1796,6 +1796,43 @@ describe Project, models: true do
     end
   end
 
+  describe '#protected_for?' do
+    let(:project) { create(:empty_project) }
+
+    subject { project.protected_for?('ref') }
+
+    context 'when the ref is not protected' do
+      before do
+        stub_application_setting(
+          default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+      end
+
+      it 'returns false' do
+        is_expected.to be_falsey
+      end
+    end
+
+    context 'when the ref is a protected branch' do
+      before do
+        create(:protected_branch, name: 'ref', project: project)
+      end
+
+      it 'returns true' do
+        is_expected.to be_truthy
+      end
+    end
+
+    context 'when the ref is a protected tag' do
+      before do
+        create(:protected_tag, name: 'ref', project: project)
+      end
+
+      it 'returns true' do
+        is_expected.to be_truthy
+      end
+    end
+  end
+
   describe '#update_project_statistics' do
     let(:project) { create(:empty_project) }
 

From fb70cf077cbc3b4fe07fad930be67331d2e57817 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 1 Jun 2017 16:39:24 +0800
Subject: [PATCH 15/17] Merge two items into one in the doc

---
 doc/ci/variables/README.md | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 71f2088be74..0a031578a18 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -10,8 +10,7 @@ The variables can be overwritten and they take precedence over each other in
 this order:
 
 1. [Trigger variables][triggers] (take precedence over all)
-1. [Secret variables](#secret-variables)
-1. [Protected secret variables](#protected-secret-variables)
+1. [Secret variables](#secret-variables) or [protected secret variables](#protected-secret-variables)
 1. YAML-defined [job-level variables](../yaml/README.md#job-variables)
 1. YAML-defined [global variables](../yaml/README.md#variables)
 1. [Deployment variables](#deployment-variables)

From 0ab8c852db118701ae5a1d105c1da74a0b88f60f Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 1 Jun 2017 21:22:38 +0800
Subject: [PATCH 16/17] Just mention which GitLab version is required

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11688/diffs#note_31277454
---
 doc/ci/variables/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md
index 0a031578a18..76ad7c564a3 100644
--- a/doc/ci/variables/README.md
+++ b/doc/ci/variables/README.md
@@ -160,7 +160,7 @@ Once you set them, they will be available for all subsequent pipelines.
 ## Protected secret variables
 
 >**Notes:**
-- This feature requires GitLab 9.3 or higher, and GitLab Runner 0.4.0 or higher.
+This feature requires GitLab 9.3 or higher.
 
 Secret variables could be protected. Whenever a secret variable is
 protected, it would only be securely passed to pipelines running on the

From 7da88278c33d20d97cf0eabb76b3117219479d12 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Thu, 1 Jun 2017 21:48:31 +0800
Subject: [PATCH 17/17] Make sure protected can't be null; Test protected!

---
 db/schema.rb                    |  4 ++--
 spec/models/ci/variable_spec.rb | 26 ++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/db/schema.rb b/db/schema.rb
index 1b62940e495..fa1c5dc15c4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -356,7 +356,7 @@ ActiveRecord::Schema.define(version: 20170525174156) do
     t.string "encrypted_value_salt"
     t.string "encrypted_value_iv"
     t.integer "project_id", null: false
-    t.boolean "protected", default: false
+    t.boolean "protected", default: false, null: false
   end
 
   add_index "ci_variables", ["project_id"], name: "index_ci_variables_on_project_id", using: :btree
@@ -1493,4 +1493,4 @@ ActiveRecord::Schema.define(version: 20170525174156) do
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
   add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
-end
\ No newline at end of file
+end
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index 38b869f59ae..077b10227d7 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -12,11 +12,33 @@ describe Ci::Variable, models: true do
   it { is_expected.not_to allow_value('foo bar').for(:key) }
   it { is_expected.not_to allow_value('foo/bar').for(:key) }
 
-  before :each do
-    subject.value = secret_value
+  describe '.unprotected' do
+    subject { described_class.unprotected }
+
+    context 'when variable is protected' do
+      before do
+        create(:ci_variable, :protected)
+      end
+
+      it 'returns nothing' do
+        is_expected.to be_empty
+      end
+    end
+
+    context 'when variable is not protected' do
+      let(:variable) { create(:ci_variable, protected: false) }
+
+      it 'returns the variable' do
+        is_expected.to contain_exactly(variable)
+      end
+    end
   end
 
   describe '#value' do
+    before do
+      subject.value = secret_value
+    end
+
     it 'stores the encrypted value' do
       expect(subject.encrypted_value).not_to be_nil
     end