Renaming Secret Variables in the codebase

.gitlab-ci.yml

@@ -265,7 +265,7 @@ package-and-qa:
     SCRIPT_NAME: trigger-build-docs
   environment:
     name: review-docs/$CI_COMMIT_REF_SLUG
-    # DOCS_REVIEW_APPS_DOMAIN and DOCS_GITLAB_REPO_SUFFIX are secret variables
+    # DOCS_REVIEW_APPS_DOMAIN and DOCS_GITLAB_REPO_SUFFIX are CI variables
     # Discussion: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14236/diffs#note_40140693
     url: http://$CI_ENVIRONMENT_SLUG.$DOCS_REVIEW_APPS_DOMAIN/$DOCS_GITLAB_REPO_SUFFIX
     on_stop: review-docs-cleanup

app/assets/javascripts/ci_variable_list/ajax_variable_list.js

@@ -47,7 +47,7 @@ export default class AjaxVariableList {
   }
 
   onSaveClicked() {
-    const loadingIcon = this.saveButton.querySelector('.js-secret-variables-save-loading-icon');
+    const loadingIcon = this.saveButton.querySelector('.js-ci-variables-save-loading-icon');
     loadingIcon.classList.toggle('hide', false);
     this.errorBox.classList.toggle('hide', true);
     // We use this to prevent a user from changing a key before we have a chance

app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js

@@ -9,7 +9,7 @@ document.addEventListener('DOMContentLoaded', () => {
   // eslint-disable-next-line no-new
   new AjaxVariableList({
     container: variableListEl,
-    saveButton: variableListEl.querySelector('.js-secret-variables-save-button'),
+    saveButton: variableListEl.querySelector('.js-ci-variables-save-button'),
     errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
     saveEndpoint: variableListEl.dataset.saveEndpoint,
   });

app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js

@@ -18,7 +18,7 @@ document.addEventListener('DOMContentLoaded', () => {
   // eslint-disable-next-line no-new
   new AjaxVariableList({
     container: variableListEl,
-    saveButton: variableListEl.querySelector('.js-secret-variables-save-button'),
+    saveButton: variableListEl.querySelector('.js-ci-variables-save-button'),
     errorBox: variableListEl.querySelector('.js-ci-variable-error-box'),
     saveEndpoint: variableListEl.dataset.saveEndpoint,
   });

app/controllers/groups/settings/ci_cd_controller.rb

@@ -7,7 +7,7 @@ module Groups
       before_action :authorize_admin_pipeline!
 
       def show
-        define_secret_variables
+        define_ci_variables
       end
 
       def reset_registration_token
@@ -19,7 +19,7 @@ module Groups
 
       private
 
-      def define_secret_variables
+      def define_ci_variables
         @variable = Ci::GroupVariable.new(group: group)
           .present(current_user: current_user)
         @variables = group.variables.order_key_asc

app/controllers/projects/settings/ci_cd_controller.rb

@@ -68,7 +68,7 @@ module Projects
 
       def define_variables
         define_runners_variables
-        define_secret_variables
+        define_ci_variables
         define_triggers_variables
         define_badges_variables
         define_auto_devops_variables
@@ -90,7 +90,7 @@ module Projects
         @group_runners = ::Ci::Runner.belonging_to_parent_group_of_project(@project.id)
       end
 
-      def define_secret_variables
+      def define_ci_variables
         @variable = ::Ci::Variable.new(project: project)
           .present(current_user: current_user)
         @variables = project.variables.order_key_asc

app/models/ci/build.rb

@@ -593,11 +593,11 @@ module Ci
     def secret_group_variables
       return [] unless project.group
 
-      project.group.secret_variables_for(ref, project)
+      project.group.ci_variables_for(ref, project)
     end
 
     def secret_project_variables(environment: persisted_environment)
-      project.secret_variables_for(ref: ref, environment: environment)
+      project.ci_variables_for(ref: ref, environment: environment)
     end
 
     def steps

app/models/group.rb

@@ -369,7 +369,7 @@ class Group < Namespace
     }
   end
 
-  def secret_variables_for(ref, project)
+  def ci_variables_for(ref, project)
     list_of_ids = [self] + ancestors
     variables = Ci::GroupVariable.where(group: list_of_ids)
     variables = variables.unprotected unless project.protected_for?(ref)

app/models/project.rb

@@ -1811,7 +1811,7 @@ class Project < ActiveRecord::Base
       .first
   end
 
-  def secret_variables_for(ref:, environment: nil)
+  def ci_variables_for(ref:, environment: nil)
     # EE would use the environment
     if protected_for?(ref)
       variables

app/views/ci/variables/_index.html.haml

@@ -9,8 +9,8 @@
         = render 'ci/variables/variable_row', form_field: 'variables', variable: variable
       = render 'ci/variables/variable_row', form_field: 'variables'
     .prepend-top-20
-      %button.btn.btn-success.js-secret-variables-save-button{ type: 'button' }
-        %span.hide.js-secret-variables-save-loading-icon
+      %button.btn.btn-success.js-ci-variables-save-button{ type: 'button' }
+        %span.hide.js-ci-variables-save-loading-icon
           = icon('spinner spin')
         = _('Save variables')
       %button.btn.btn-info.btn-inverted.prepend-left-10.js-secret-value-reveal-button{ type: 'button', data: { secret_reveal_status: "#{@variables.size == 0}" } }

app/views/groups/settings/ci_cd/show.html.haml

@@ -3,7 +3,7 @@
 
 - expanded = Rails.env.test?
 
-%section.settings#secret-variables.no-animate{ class: ('expanded' if expanded) }
+%section.settings#ci-variables.no-animate{ class: ('expanded' if expanded) }
   .settings-header
     %h4
       = _('Variables')

changelogs/unreleased/ravlen-rename-secret-variables-in-codebase.yml

@@ -0,0 +1,5 @@
+---
+title: "Secret Variables renamed to CI Variables in the codebase, to match UX"
+merge_request: 22414
+author: Marcel Amirault @ravlen
+type: changed

qa/qa.rb

@@ -53,7 +53,7 @@ module QA
       autoload :DeployKey, 'qa/factory/resource/deploy_key'
       autoload :DeployToken, 'qa/factory/resource/deploy_token'
       autoload :Branch, 'qa/factory/resource/branch'
-      autoload :SecretVariable, 'qa/factory/resource/secret_variable'
+      autoload :CiVariable, 'qa/factory/resource/ci_variable'
       autoload :Runner, 'qa/factory/resource/runner'
       autoload :PersonalAccessToken, 'qa/factory/resource/personal_access_token'
       autoload :KubernetesCluster, 'qa/factory/resource/kubernetes_cluster'
@@ -183,7 +183,7 @@ module QA
         autoload :DeployKeys, 'qa/page/project/settings/deploy_keys'
         autoload :DeployTokens, 'qa/page/project/settings/deploy_tokens'
         autoload :ProtectedBranches, 'qa/page/project/settings/protected_branches'
-        autoload :SecretVariables, 'qa/page/project/settings/secret_variables'
+        autoload :CiVariables, 'qa/page/project/settings/ci_variables'
         autoload :Runners, 'qa/page/project/settings/runners'
         autoload :MergeRequest, 'qa/page/project/settings/merge_request'
         autoload :Members, 'qa/page/project/settings/members'

qa/qa/factory/resource/ci_variable.rb

@@ -1,13 +1,13 @@
 module QA
   module Factory
     module Resource
-      class SecretVariable < Factory::Base
+      class CiVariable < Factory::Base
         attr_accessor :key, :value
 
         attribute :project do
           Factory::Resource::Project.fabricate! do |resource|
-            resource.name = 'project-with-secret-variables'
-            resource.description = 'project for adding secret variable test'
+            resource.name = 'project-with-ci-variables'
+            resource.description = 'project for adding CI variable test'
           end
         end
 
@@ -17,7 +17,7 @@ module QA
           Page::Project::Menu.perform(&:click_ci_cd_settings)
 
           Page::Project::Settings::CICD.perform do |setting|
-            setting.expand_secret_variables do |page|
+            setting.expand_ci_variables do |page|
               page.fill_variable(key, value)
 
               page.save_variables

qa/qa/page/project/settings/ci_cd.rb

@@ -25,9 +25,9 @@ module QA # rubocop:disable Naming/FileName
             end
           end
 
-          def expand_secret_variables(&block)
+          def expand_ci_variables(&block)
             expand_section(:variables_settings) do
-              Settings::SecretVariables.perform(&block)
+              Settings::CiVariables.perform(&block)
             end
           end
 

qa/qa/page/project/settings/ci_variables.rb

@@ -2,7 +2,7 @@ module QA
   module Page
     module Project
       module Settings
-        class SecretVariables < Page::Base
+        class CiVariables < Page::Base
           include Common
 
           view 'app/views/ci/variables/_variable_row.html.haml' do
@@ -12,7 +12,7 @@ module QA
           end
 
           view 'app/views/ci/variables/_index.html.haml' do
-            element :save_variables, '.js-secret-variables-save-button' # rubocop:disable QA/ElementWithPattern
+            element :save_variables, '.js-ci-variables-save-button' # rubocop:disable QA/ElementWithPattern
             element :reveal_values, '.js-secret-value-reveal-button' # rubocop:disable QA/ElementWithPattern
           end
 
@@ -33,7 +33,7 @@ module QA
           end
 
           def save_variables
-            find('.js-secret-variables-save-button').click
+            find('.js-ci-variables-save-button').click
           end
 
           def reveal_variables

qa/qa/specs/features/browser_ui/4_verify/ci_variable/add_ci_variable_spec.rb

@@ -2,24 +2,24 @@
 
 module QA
   context 'Verify' do
-    describe 'Secret variable support' do
-      it 'user adds a secret variable' do
+    describe 'CI variable support' do
+      it 'user adds a CI variable' do
         Runtime::Browser.visit(:gitlab, Page::Main::Login)
         Page::Main::Login.act { sign_in_using_credentials }
 
-        Factory::Resource::SecretVariable.fabricate! do |resource|
+        Factory::Resource::CiVariable.fabricate! do |resource|
           resource.key = 'VARIABLE_KEY'
-          resource.value = 'some secret variable'
+          resource.value = 'some CI variable'
         end
 
         Page::Project::Settings::CICD.perform do |settings|
-          settings.expand_secret_variables do |page|
+          settings.expand_ci_variables do |page|
             expect(page).to have_field(with: 'VARIABLE_KEY')
-            expect(page).not_to have_field(with: 'some secret variable')
+            expect(page).not_to have_field(with: 'some CI variable')
 
             page.reveal_variables
 
-            expect(page).to have_field(with: 'some secret variable')
+            expect(page).to have_field(with: 'some CI variable')
           end
         end
       end

qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb

@@ -55,7 +55,7 @@ module QA
 
           deploy_key_name = "DEPLOY_KEY_#{key.name}_#{key.bits}"
 
-          Factory::Resource::SecretVariable.fabricate! do |resource|
+          Factory::Resource::CiVariable.fabricate! do |resource|
             resource.project = @project
             resource.key = deploy_key_name
             resource.value = key.private_key

qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb

@@ -22,7 +22,7 @@ module QA
 
             # Disable code_quality check in Auto DevOps pipeline as it takes
             # too long and times out the test
-            Factory::Resource::SecretVariable.fabricate! do |resource|
+            Factory::Resource::CiVariable.fabricate! do |resource|
               resource.project = project
               resource.key = 'CODE_QUALITY_DISABLED'
               resource.value = '1'

spec/javascripts/ci_variable_list/ajax_variable_list_spec.js

@@ -24,7 +24,7 @@ describe('AjaxFormVariableList', () => {
     mock = new MockAdapter(axios);
 
     const ajaxVariableListEl = document.querySelector('.js-ci-variable-list-section');
-    saveButton = ajaxVariableListEl.querySelector('.js-secret-variables-save-button');
+    saveButton = ajaxVariableListEl.querySelector('.js-ci-variables-save-button');
     errorBox = container.querySelector('.js-ci-variable-error-box');
     ajaxVariableList = new AjaxFormVariableList({
       container,
@@ -44,7 +44,7 @@ describe('AjaxFormVariableList', () => {
 
   describe('onSaveClicked', () => {
     it('shows loading spinner while waiting for the request', done => {
-      const loadingIcon = saveButton.querySelector('.js-secret-variables-save-loading-icon');
+      const loadingIcon = saveButton.querySelector('.js-ci-variables-save-loading-icon');
 
       mock.onPatch(VARIABLE_PATCH_ENDPOINT).reply(() => {
         expect(loadingIcon.classList.contains(HIDE_CLASS)).toEqual(false);
@@ -172,7 +172,7 @@ describe('AjaxFormVariableList', () => {
       container = document.querySelector('.js-ci-variable-list-section');
 
       const ajaxVariableListEl = document.querySelector('.js-ci-variable-list-section');
-      saveButton = ajaxVariableListEl.querySelector('.js-secret-variables-save-button');
+      saveButton = ajaxVariableListEl.querySelector('.js-ci-variables-save-button');
       errorBox = container.querySelector('.js-ci-variable-error-box');
       ajaxVariableList = new AjaxFormVariableList({
         container,

spec/lib/gitlab/ci/build/policy/variables_spec.rb

@@ -54,7 +54,7 @@ describe Gitlab::Ci::Build::Policy::Variables do
       expect(policy).not_to be_satisfied_by(pipeline, seed)
     end
 
-    it 'allows to evaluate regular secret variables' do
+    it 'allows to evaluate regular CI variables' do
       create(:ci_variable, project: project, key: 'SECRET', value: 'my secret')
 
       policy = described_class.new(["$SECRET == 'my secret'"])

spec/models/ci/build_spec.rb

@@ -2027,17 +2027,17 @@ describe Ci::Build do
       it { is_expected.to include(tag_variable) }
     end
 
-    context 'when secret variable is defined' do
-      let(:secret_variable) do
+    context 'when CI variable is defined' do
+      let(:ci_variable) do
         { key: 'SECRET_KEY', value: 'secret_value', public: false }
       end
 
       before do
         create(:ci_variable,
-               secret_variable.slice(:key, :value).merge(project: project))
+               ci_variable.slice(:key, :value).merge(project: project))
       end
 
-      it { is_expected.to include(secret_variable) }
+      it { is_expected.to include(ci_variable) }
     end
 
     context 'when protected variable is defined' do
@@ -2072,17 +2072,17 @@ describe Ci::Build do
       end
     end
 
-    context 'when group secret variable is defined' do
-      let(:secret_variable) do
+    context 'when group CI variable is defined' do
+      let(:ci_variable) do
         { key: 'SECRET_KEY', value: 'secret_value', public: false }
       end
 
       before do
         create(:ci_group_variable,
-               secret_variable.slice(:key, :value).merge(group: group))
+               ci_variable.slice(:key, :value).merge(group: group))
       end
 
-      it { is_expected.to include(secret_variable) }
+      it { is_expected.to include(ci_variable) }
     end
 
     context 'when group protected variable is defined' do
@@ -2357,7 +2357,7 @@ describe Ci::Build do
             .to receive(:predefined_variables) { [project_pre_var] }
 
           allow_any_instance_of(Project)
-            .to receive(:secret_variables_for)
+            .to receive(:ci_variables_for)
             .with(ref: 'master', environment: nil) do
             [create(:ci_variable, key: 'secret', value: 'value')]
           end
@@ -2508,7 +2508,7 @@ describe Ci::Build do
   end
 
   describe '#scoped_variables_hash' do
-    context 'when overriding secret variables' do
+    context 'when overriding CI variables' do
       before do
         project.variables.create!(key: 'MY_VAR', value: 'my value 1')
         pipeline.variables.create!(key: 'MY_VAR', value: 'my value 2')

spec/models/group_spec.rb

@@ -653,10 +653,10 @@ describe Group do
     end
   end
 
-  describe '#secret_variables_for' do
+  describe '#ci_variables_for' do
     let(:project) { create(:project, group: group) }
 
-    let!(:secret_variable) do
+    let!(:ci_variable) do
       create(:ci_group_variable, value: 'secret', group: group)
     end
 
@@ -664,11 +664,11 @@ describe Group do
       create(:ci_group_variable, :protected, value: 'protected', group: group)
     end
 
-    subject { group.secret_variables_for('ref', project) }
+    subject { group.ci_variables_for('ref', project) }
 
     shared_examples 'ref is protected' do
       it 'contains all the variables' do
-        is_expected.to contain_exactly(secret_variable, protected_variable)
+        is_expected.to contain_exactly(ci_variable, protected_variable)
       end
     end
 
@@ -678,8 +678,8 @@ describe Group do
           default_branch_protection: Gitlab::Access::PROTECTION_NONE)
       end
 
-      it 'contains only the secret variables' do
-        is_expected.to contain_exactly(secret_variable)
+      it 'contains only the CI variables' do
+        is_expected.to contain_exactly(ci_variable)
       end
     end
 
@@ -712,9 +712,9 @@ describe Group do
       end
 
       it 'returns all variables belong to the group and parent groups' do
-        expected_array1 = [protected_variable, secret_variable]
+        expected_array1 = [protected_variable, ci_variable]
         expected_array2 = [variable_child, variable_child_2, variable_child_3]
-        got_array = group_child_3.secret_variables_for('ref', project).to_a
+        got_array = group_child_3.ci_variables_for('ref', project).to_a
 
         expect(got_array.shift(2)).to contain_exactly(*expected_array1)
         expect(got_array).to eq(expected_array2)

spec/models/project_spec.rb

@@ -2432,10 +2432,10 @@ describe Project do
     end
   end
 
-  describe '#secret_variables_for' do
+  describe '#ci_variables_for' do
     let(:project) { create(:project) }
 
-    let!(:secret_variable) do
+    let!(:ci_variable) do
       create(:ci_variable, value: 'secret', project: project)
     end
 
@@ -2443,7 +2443,7 @@ describe Project do
       create(:ci_variable, :protected, value: 'protected', project: project)
     end
 
-    subject { project.reload.secret_variables_for(ref: 'ref') }
+    subject { project.reload.ci_variables_for(ref: 'ref') }
 
     before do
       stub_application_setting(
@@ -2452,13 +2452,13 @@ describe Project do
 
     shared_examples 'ref is protected' do
       it 'contains all the variables' do
-        is_expected.to contain_exactly(secret_variable, protected_variable)
+        is_expected.to contain_exactly(ci_variable, protected_variable)
       end
     end
 
     context 'when the ref is not protected' do
-      it 'contains only the secret variables' do
-        is_expected.to contain_exactly(secret_variable)
+      it 'contains only the CI variables' do
+        is_expected.to contain_exactly(ci_variable)
       end
     end
 

spec/support/features/variable_list_shared_examples.rb

@@ -5,7 +5,7 @@ shared_examples 'variable list' do
     end
   end
 
-  it 'adds new secret variable' do
+  it 'adds new CI variable' do
     page.within('.js-ci-variable-list-section .js-row:last-child') do
       find('.js-ci-variable-input-key').set('key')
       find('.js-ci-variable-input-value').set('key value')