summaryrefslogtreecommitdiff
path: root/spec
diff options
context:
space:
mode:
authorShinya Maeda <gitlab.shinyamaeda@gmail.com>2017-05-04 03:51:55 +0900
committerShinya Maeda <shinya@gitlab.com>2017-07-07 15:33:17 +0900
commit5b0954759cc24bdba97be89bb117c5440174f859 (patch)
tree0a60ceec84548de35910a46edd149be23d831f35 /spec
parent49430c47d4d34072ff43cc1e35213317802055d7 (diff)
downloadgitlab-ce-5b0954759cc24bdba97be89bb117c5440174f859.tar.gz
Basic BE change
Fix static-snalysis Move the precedence of group secure variable before project secure variable. Allow project_id to be null. Separate Ci::VariableProject and Ci::VariableGroup Add the forgotton files Add migration file to update type of ci_variables Fix form_for fpr VariableProject Fix test Change the table structure according to the yorik advice Add necessary migration files. Remove unnecessary migration spec. Revert safe_model_attributes.yml Fix models Fix spec Avoid self.variable. Use becomes for correct routing. Use unique index on group_id and key Add null: false for t.timestamps Fix schema version Rename VariableProject and VariableGroup to ProjectVariable and GroupVariable Rename the rest of them Add the rest of files Basic BE change Fix static-snalysis Move the precedence of group secure variable before project secure variable. Allow project_id to be null. Separate Ci::VariableProject and Ci::VariableGroup Add the forgotton files Add migration file to update type of ci_variables Fix form_for fpr VariableProject Fix test Change the table structure according to the yorik advice Add necessary migration files. Remove unnecessary migration spec. Revert safe_model_attributes.yml Fix models Fix spec Avoid self.variable. Use becomes for correct routing. Use unique index on group_id and key Add null: false for t.timestamps Fix schema version Rename VariableProject and VariableGroup to ProjectVariable and GroupVariable Rename the rest of them Add the rest of files Implement CURD Rename codes related to VariableGroup and VariableProject FE part Remove unneccesary changes Make Fe code up-to-date Add protected flag to migration file Protected group variables essential package Update schema Improve doc Fix logic and spec for models Fix logic and spec for controllers Fix logic and spec for views(pre feature) Add feature spec Fixed bugs. placeholder. reveal button. doc. Add changelog Remove unnecessary comment godfat nice catches Improve secret_variables_for arctecture Fix spec Fix StaticAnlysys & path_regex spec Revert "Improve secret_variables_for arctecture" This reverts commit c3216ca212322ecf6ca534cb12ce75811a4e77f1. Use ayufan suggestion for secret_variables_for Use find instead of find_by Fix spec message for variable is invalid Fix spec remove variable.group_id = group.id godffat spec nitpicks Use include Gitlab::Routing.url_helpers for presenter spec
Diffstat (limited to 'spec')
-rw-r--r--spec/controllers/groups/settings/ci_cd_controller_spec.rb20
-rw-r--r--spec/controllers/groups/variables_controller_spec.rb56
-rw-r--r--spec/controllers/projects/variables_controller_spec.rb3
-rw-r--r--spec/factories/ci/group_variables.rb12
-rw-r--r--spec/features/group_variables_spec.rb78
-rw-r--r--spec/features/variables_spec.rb10
-rw-r--r--spec/models/ci/build_spec.rb53
-rw-r--r--spec/models/ci/group_variable_spec.rb31
-rw-r--r--spec/models/ci/variable_spec.rb3
-rw-r--r--spec/models/group_spec.rb59
-rw-r--r--spec/presenters/ci/group_variable_presenter_spec.rb63
-rw-r--r--spec/presenters/ci/variable_presenter_spec.rb63
12 files changed, 442 insertions, 9 deletions
diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
new file mode 100644
index 00000000000..2e0efb57c74
--- /dev/null
+++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Groups::Settings::CiCdController do
+ let(:group) { create(:group) }
+ let(:user) { create(:user) }
+
+ before do
+ group.add_master(user)
+ sign_in(user)
+ end
+
+ describe 'GET #show' do
+ it 'renders show with 200 status code' do
+ get :show, group_id: group
+
+ expect(response).to have_http_status(200)
+ expect(response).to render_template(:show)
+ end
+ end
+end
diff --git a/spec/controllers/groups/variables_controller_spec.rb b/spec/controllers/groups/variables_controller_spec.rb
new file mode 100644
index 00000000000..c11fe93ffca
--- /dev/null
+++ b/spec/controllers/groups/variables_controller_spec.rb
@@ -0,0 +1,56 @@
+require 'spec_helper'
+
+describe Groups::VariablesController do
+ let(:group) { create(:group) }
+ let(:user) { create(:user) }
+
+ before do
+ sign_in(user)
+ group.add_master(user)
+ end
+
+ describe 'POST #create' do
+ context 'variable is valid' do
+ it 'shows a success flash message' do
+ post :create, group_id: group, variable: { key: "one", value: "two" }
+
+ expect(flash[:notice]).to include 'Variables were successfully updated.'
+ expect(response).to redirect_to(group_settings_ci_cd_path(group))
+ end
+ end
+
+ context 'variable is invalid' do
+ it 'renders show' do
+ post :create, group_id: group, variable: { key: "..one", value: "two" }
+
+ expect(response).to render_template("groups/variables/show")
+ end
+ end
+ end
+
+ describe 'POST #update' do
+ let(:variable) { create(:ci_group_variable) }
+
+ context 'updating a variable with valid characters' do
+ before do
+ group.variables << variable
+ end
+
+ it 'shows a success flash message' do
+ post :update, group_id: group,
+ id: variable.id, variable: { key: variable.key, value: 'two' }
+
+ expect(flash[:notice]).to include 'Variable was successfully updated.'
+ expect(response).to redirect_to(group_variables_path(group))
+ end
+
+ it 'renders the action #show if the variable key is invalid' do
+ post :update, group_id: group,
+ id: variable.id, variable: { key: '?', value: variable.value }
+
+ expect(response).to have_http_status(200)
+ expect(response).to render_template :show
+ end
+ end
+ end
+end
diff --git a/spec/controllers/projects/variables_controller_spec.rb b/spec/controllers/projects/variables_controller_spec.rb
index a0ecc756653..0304ea6f93c 100644
--- a/spec/controllers/projects/variables_controller_spec.rb
+++ b/spec/controllers/projects/variables_controller_spec.rb
@@ -21,7 +21,7 @@ describe Projects::VariablesController do
end
context 'variable is invalid' do
- it 'shows an alert flash message' do
+ it 'renders show' do
post :create, namespace_id: project.namespace.to_param, project_id: project,
variable: { key: "..one", value: "two" }
@@ -35,7 +35,6 @@ describe Projects::VariablesController do
context 'updating a variable with valid characters' do
before do
- variable.project_id = project.id
project.variables << variable
end
diff --git a/spec/factories/ci/group_variables.rb b/spec/factories/ci/group_variables.rb
new file mode 100644
index 00000000000..565ced9eb1a
--- /dev/null
+++ b/spec/factories/ci/group_variables.rb
@@ -0,0 +1,12 @@
+FactoryGirl.define do
+ factory :ci_group_variable, class: Ci::GroupVariable do
+ sequence(:key) { |n| "VARIABLE_#{n}" }
+ value 'VARIABLE_VALUE'
+
+ trait(:protected) do
+ protected true
+ end
+
+ group factory: :group
+ end
+end
diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb
new file mode 100644
index 00000000000..37814ba6238
--- /dev/null
+++ b/spec/features/group_variables_spec.rb
@@ -0,0 +1,78 @@
+require 'spec_helper'
+
+feature 'Group variables', js: true do
+ let(:user) { create(:user) }
+ let(:group) { create(:group) }
+
+ background do
+ group.add_master(user)
+ gitlab_sign_in(user)
+ end
+
+ context 'when user creates a new variable' do
+ background do
+ visit group_settings_ci_cd_path(group)
+ fill_in 'variable_key', with: 'AAA'
+ fill_in 'variable_value', with: 'AAA123'
+ find(:css, "#variable_protected").set(true)
+ click_on 'Add new variable'
+ end
+
+ scenario 'user sees the created variable' do
+ page.within('.variables-table') do
+ expect(find(".variable-key")).to have_content('AAA')
+ expect(find(".variable-value")).to have_content('******')
+ expect(find(".variable-protected")).to have_content('Yes')
+ end
+ click_on 'Reveal Values'
+ page.within('.variables-table') do
+ expect(find(".variable-value")).to have_content('AAA123')
+ end
+ end
+ end
+
+ context 'when user edits a variable' do
+ background do
+ create(:ci_group_variable, key: 'AAA', value: 'AAA123', protected: true,
+ group: group)
+
+ visit group_settings_ci_cd_path(group)
+
+ page.within('.variable-menu') do
+ click_on 'Update'
+ end
+
+ fill_in 'variable_key', with: 'BBB'
+ fill_in 'variable_value', with: 'BBB123'
+ find(:css, "#variable_protected").set(false)
+ click_on 'Save variable'
+ end
+
+ scenario 'user sees the updated variable' do
+ page.within('.variables-table') do
+ expect(find(".variable-key")).to have_content('BBB')
+ expect(find(".variable-value")).to have_content('******')
+ expect(find(".variable-protected")).to have_content('No')
+ end
+ end
+ end
+
+ context 'when user deletes a variable' do
+ background do
+ create(:ci_group_variable, key: 'BBB', value: 'BBB123', protected: false,
+ group: group)
+
+ visit group_settings_ci_cd_path(group)
+
+ page.within('.variable-menu') do
+ page.accept_alert 'Are you sure?' do
+ click_on 'Remove'
+ end
+ end
+ end
+
+ scenario 'user does not see the deleted variable' do
+ expect(page).to have_no_css('.variables-table')
+ end
+ end
+end
diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb
index 1a2dedf27eb..a7c24c7d718 100644
--- a/spec/features/variables_spec.rb
+++ b/spec/features/variables_spec.rb
@@ -82,7 +82,7 @@ describe 'Project variables', js: true do
it 'deletes variable' do
page.within('.variables-table') do
- find('.btn-variable-delete').click
+ click_on 'Remove'
end
expect(page).not_to have_selector('variables-table')
@@ -90,7 +90,7 @@ describe 'Project variables', js: true do
it 'edits variable' do
page.within('.variables-table') do
- find('.btn-variable-edit').click
+ click_on 'Update'
end
expect(page).to have_content('Update variable')
@@ -104,7 +104,7 @@ describe 'Project variables', js: true do
it 'edits variable with empty value' do
page.within('.variables-table') do
- find('.btn-variable-edit').click
+ click_on 'Update'
end
expect(page).to have_content('Update variable')
@@ -117,7 +117,7 @@ describe 'Project variables', js: true do
it 'edits variable to be protected' do
page.within('.variables-table') do
- find('.btn-variable-edit').click
+ click_on 'Update'
end
expect(page).to have_content('Update variable')
@@ -132,7 +132,7 @@ describe 'Project variables', js: true do
project.variables.first.update(protected: true)
page.within('.variables-table') do
- find('.btn-variable-edit').click
+ click_on 'Update'
end
expect(page).to have_content('Update variable')
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 431fcda165d..cf6d356c524 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1356,6 +1356,59 @@ describe Ci::Build, :models do
end
end
+ context 'when group secret variable is defined' do
+ let(:secret_variable) do
+ { key: 'SECRET_KEY', value: 'secret_value', public: false }
+ end
+
+ let(:group) { create(:group, :access_requestable) }
+
+ before do
+ build.project.update(group: group)
+
+ create(:ci_group_variable,
+ secret_variable.slice(:key, :value).merge(group: group))
+ end
+
+ it { is_expected.to include(secret_variable) }
+ end
+
+ context 'when group protected variable is defined' do
+ let(:protected_variable) do
+ { key: 'PROTECTED_KEY', value: 'protected_value', public: false }
+ end
+
+ let(:group) { create(:group, :access_requestable) }
+
+ before do
+ build.project.update(group: group)
+
+ create(:ci_group_variable,
+ :protected,
+ protected_variable.slice(:key, :value).merge(group: group))
+ 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
let(:trigger) { create(:ci_trigger, project: project) }
let(:trigger_request) { create(:ci_trigger_request_with_variables, pipeline: pipeline, trigger: trigger) }
diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb
new file mode 100644
index 00000000000..24b914face9
--- /dev/null
+++ b/spec/models/ci/group_variable_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+
+describe Ci::GroupVariable, models: true do
+ subject { build(:ci_group_variable) }
+
+ it { is_expected.to include_module(HasVariable) }
+ it { is_expected.to include_module(Presentable) }
+ it { is_expected.to validate_uniqueness_of(:key).scoped_to(:group_id) }
+
+ describe '.unprotected' do
+ subject { described_class.unprotected }
+
+ context 'when variable is protected' do
+ before do
+ create(:ci_group_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_group_variable, protected: false) }
+
+ it 'returns the variable' do
+ is_expected.to contain_exactly(variable)
+ end
+ end
+ end
+end
diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb
index 4ffbfa6c130..890ffaae494 100644
--- a/spec/models/ci/variable_spec.rb
+++ b/spec/models/ci/variable_spec.rb
@@ -3,10 +3,9 @@ require 'spec_helper'
describe Ci::Variable, models: true do
subject { build(:ci_variable) }
- let(:secret_value) { 'secret' }
-
describe 'validations' do
it { is_expected.to include_module(HasVariable) }
+ it { is_expected.to include_module(Presentable) }
it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }
end
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 4de1683b21c..dab33aa4418 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -13,6 +13,7 @@ describe Group, models: true do
it { is_expected.to have_many(:shared_projects).through(:project_group_links) }
it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
it { is_expected.to have_many(:labels).class_name('GroupLabel') }
+ it { is_expected.to have_many(:variables).class_name('Ci::GroupVariable') }
it { is_expected.to have_many(:uploads).dependent(:destroy) }
it { is_expected.to have_one(:chat_team) }
@@ -418,4 +419,62 @@ describe Group, models: true do
expect(calls).to eq 2
end
end
+
+ describe '#secret_variables_for' do
+ let(:project) { create(:empty_project, group: group) }
+
+ let!(:secret_variable) do
+ create(:ci_group_variable, value: 'secret', group: group)
+ end
+
+ let!(:protected_variable) do
+ create(:ci_group_variable, :protected, value: 'protected', group: group)
+ end
+
+ subject { group.secret_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)
+ end
+ end
+
+ context 'when the ref is not protected' do
+ before do
+ stub_application_setting(
+ default_branch_protection: Gitlab::Access::PROTECTION_NONE)
+ end
+
+ it 'contains only the secret variables' do
+ is_expected.to contain_exactly(secret_variable)
+ end
+ 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
+
+ context 'when group has a child' do
+ let!(:group_child) { create(:group, :access_requestable, parent: group) }
+ let!(:variable_child) { create(:ci_group_variable, group: group_child) }
+
+ subject { group_child.secret_variables_for('ref', project) }
+
+ it 'returns all variables belong to the group and parent groups' do
+ is_expected.to contain_exactly(secret_variable, protected_variable, variable_child)
+ end
+ end
+ end
end
diff --git a/spec/presenters/ci/group_variable_presenter_spec.rb b/spec/presenters/ci/group_variable_presenter_spec.rb
new file mode 100644
index 00000000000..d404028405b
--- /dev/null
+++ b/spec/presenters/ci/group_variable_presenter_spec.rb
@@ -0,0 +1,63 @@
+require 'spec_helper'
+
+describe Ci::GroupVariablePresenter do
+ include Gitlab::Routing.url_helpers
+
+ let(:group) { create(:group) }
+ let(:variable) { create(:ci_group_variable, group: group) }
+
+ subject(:presenter) do
+ described_class.new(variable)
+ end
+
+ it 'inherits from Gitlab::View::Presenter::Delegated' do
+ expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
+ end
+
+ describe '#initialize' do
+ it 'takes a variable and optional params' do
+ expect { presenter }.not_to raise_error
+ end
+
+ it 'exposes variable' do
+ expect(presenter.variable).to eq(variable)
+ end
+
+ it 'forwards missing methods to variable' do
+ expect(presenter.key).to eq(variable.key)
+ end
+ end
+
+ describe '#placeholder' do
+ subject { described_class.new(variable).placeholder }
+
+ it { is_expected.to eq('GROUP_VARIABLE') }
+ end
+
+ describe '#form_path' do
+ context 'when variable is persisted' do
+ subject { described_class.new(variable).form_path }
+
+ it { is_expected.to eq(group_variable_path(group, variable)) }
+ end
+
+ context 'when variable is not persisted' do
+ let(:variable) { build(:ci_group_variable, group: group) }
+ subject { described_class.new(variable).form_path }
+
+ it { is_expected.to eq(group_variables_path(group)) }
+ end
+ end
+
+ describe '#edit_path' do
+ subject { described_class.new(variable).edit_path }
+
+ it { is_expected.to eq(group_variable_path(group, variable)) }
+ end
+
+ describe '#delete_path' do
+ subject { described_class.new(variable).delete_path }
+
+ it { is_expected.to eq(group_variable_path(group, variable)) }
+ end
+end
diff --git a/spec/presenters/ci/variable_presenter_spec.rb b/spec/presenters/ci/variable_presenter_spec.rb
new file mode 100644
index 00000000000..3b615c4bf36
--- /dev/null
+++ b/spec/presenters/ci/variable_presenter_spec.rb
@@ -0,0 +1,63 @@
+require 'spec_helper'
+
+describe Ci::VariablePresenter do
+ include Gitlab::Routing.url_helpers
+
+ let(:project) { create(:empty_project) }
+ let(:variable) { create(:ci_variable, project: project) }
+
+ subject(:presenter) do
+ described_class.new(variable)
+ end
+
+ it 'inherits from Gitlab::View::Presenter::Delegated' do
+ expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
+ end
+
+ describe '#initialize' do
+ it 'takes a variable and optional params' do
+ expect { presenter }.not_to raise_error
+ end
+
+ it 'exposes variable' do
+ expect(presenter.variable).to eq(variable)
+ end
+
+ it 'forwards missing methods to variable' do
+ expect(presenter.key).to eq(variable.key)
+ end
+ end
+
+ describe '#placeholder' do
+ subject { described_class.new(variable).placeholder }
+
+ it { is_expected.to eq('PROJECT_VARIABLE') }
+ end
+
+ describe '#form_path' do
+ context 'when variable is persisted' do
+ subject { described_class.new(variable).form_path }
+
+ it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) }
+ end
+
+ context 'when variable is not persisted' do
+ let(:variable) { build(:ci_variable, project: project) }
+ subject { described_class.new(variable).form_path }
+
+ it { is_expected.to eq(namespace_project_variables_path(project.namespace, project)) }
+ end
+ end
+
+ describe '#edit_path' do
+ subject { described_class.new(variable).edit_path }
+
+ it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) }
+ end
+
+ describe '#delete_path' do
+ subject { described_class.new(variable).delete_path }
+
+ it { is_expected.to eq(namespace_project_variable_path(project.namespace, project, variable)) }
+ end
+end