diff options
author | Marcel Amirault <ravlen@gmail.com> | 2018-10-30 08:49:26 +0000 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-10-30 08:49:26 +0000 |
commit | 14fc739c94907255f70fe5032059bcdc4aba1e34 (patch) | |
tree | 3098367838e91c7a70636c9c50c656fb458b58d2 | |
parent | c71c1f03c78cb3362bbe2baaf49a8e3573c96d1a (diff) | |
download | gitlab-ce-14fc739c94907255f70fe5032059bcdc4aba1e34.tar.gz |
Renaming Secret Variables in the codebase
25 files changed, 69 insertions, 64 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c3163b687b4..0d70eae0d1e 100644 --- a/.gitlab-ci.yml +++ b/.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 diff --git a/app/assets/javascripts/ci_variable_list/ajax_variable_list.js b/app/assets/javascripts/ci_variable_list/ajax_variable_list.js index 1089d0a72d3..c7a917457f3 100644 --- a/app/assets/javascripts/ci_variable_list/ajax_variable_list.js +++ b/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 diff --git a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js index d3b2656743d..ae0a8c74964 100644 --- a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js +++ b/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, }); diff --git a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js index 8f5ac3d8082..15c6fb550c1 100644 --- a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js +++ b/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, }); diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index 93f3eb2be6d..c1dcc463de7 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/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 diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 3a1344651df..75e590f3f33 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/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 diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index cdfe8175a42..d73c02ba5d7 100644 --- a/app/models/ci/build.rb +++ b/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 diff --git a/app/models/group.rb b/app/models/group.rb index c67479110c9..adb9169cfcd 100644 --- a/app/models/group.rb +++ b/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) diff --git a/app/models/project.rb b/app/models/project.rb index 382fb4f463a..d593cbb223a 100644 --- a/app/models/project.rb +++ b/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 diff --git a/app/views/ci/variables/_index.html.haml b/app/views/ci/variables/_index.html.haml index e402801a776..f34305e94fa 100644 --- a/app/views/ci/variables/_index.html.haml +++ b/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}" } } diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml index 647948c7dff..a5e6abdba52 100644 --- a/app/views/groups/settings/ci_cd/show.html.haml +++ b/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') diff --git a/changelogs/unreleased/ravlen-rename-secret-variables-in-codebase.yml b/changelogs/unreleased/ravlen-rename-secret-variables-in-codebase.yml new file mode 100644 index 00000000000..211d51a3d43 --- /dev/null +++ b/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
\ No newline at end of file @@ -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' diff --git a/qa/qa/factory/resource/secret_variable.rb b/qa/qa/factory/resource/ci_variable.rb index 24ba3408810..a0aefc61f9f 100644 --- a/qa/qa/factory/resource/secret_variable.rb +++ b/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 diff --git a/qa/qa/page/project/settings/ci_cd.rb b/qa/qa/page/project/settings/ci_cd.rb index cc5fc370a5a..12c2409a5a7 100644 --- a/qa/qa/page/project/settings/ci_cd.rb +++ b/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 diff --git a/qa/qa/page/project/settings/secret_variables.rb b/qa/qa/page/project/settings/ci_variables.rb index 6a87ef472e4..e7a6e4bf628 100644 --- a/qa/qa/page/project/settings/secret_variables.rb +++ b/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 diff --git a/qa/qa/specs/features/browser_ui/4_verify/secret_variable/add_secret_variable_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/ci_variable/add_ci_variable_spec.rb index 292f24d9c0d..58b272adcf1 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/secret_variable/add_secret_variable_spec.rb +++ b/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 diff --git a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb index caf014c89ea..604641e54b8 100644 --- a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb +++ b/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 diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index 40cae0793dd..c2fce1e7df1 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/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' diff --git a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js b/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js index 4f8701bae01..1fc0e206d5e 100644 --- a/spec/javascripts/ci_variable_list/ajax_variable_list_spec.js +++ b/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, diff --git a/spec/lib/gitlab/ci/build/policy/variables_spec.rb b/spec/lib/gitlab/ci/build/policy/variables_spec.rb index 2ce858836e3..854c4cb718c 100644 --- a/spec/lib/gitlab/ci/build/policy/variables_spec.rb +++ b/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'"]) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index a046541031e..65e06f27f35 100644 --- a/spec/models/ci/build_spec.rb +++ b/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') diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 571b160d901..ada00f03928 100644 --- a/spec/models/group_spec.rb +++ b/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) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 62a38c66d99..e66838edd1a 100644 --- a/spec/models/project_spec.rb +++ b/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 diff --git a/spec/support/features/variable_list_shared_examples.rb b/spec/support/features/variable_list_shared_examples.rb index f7f851eb1eb..bce1fb01355 100644 --- a/spec/support/features/variable_list_shared_examples.rb +++ b/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') |