Update docs and use protected secret variable as the name

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