From 01ed3a1511be5d2076b5f602839ca0046055dd8b Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Tue, 4 Dec 2018 15:38:15 -0600 Subject: Allow users to add cluster with ancestors Include a new policy in Clusterables (projects and groups), which checks if another cluster can be added clusterable_has_cluster? and multiple_clusters_available private methods will be overriden in EE Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/34758 --- app/policies/concerns/clusterable_actions.rb | 14 +++++ app/policies/group_policy.rb | 10 +++- app/policies/project_policy.rb | 7 +++ app/presenters/clusterable_presenter.rb | 4 ++ app/views/clusters/clusters/_buttons.html.haml | 3 +- app/views/clusters/clusters/_empty_state.html.haml | 2 +- .../34758-extend-can-create-cluster-logic.yml | 5 ++ spec/policies/group_policy_spec.rb | 13 ++++- spec/policies/project_policy_spec.rb | 12 +++- spec/presenters/clusterable_presenter_spec.rb | 64 ++++++++++++++++++++++ .../policies/clusterable_shared_examples.rb | 37 +++++++++++++ 11 files changed, 165 insertions(+), 6 deletions(-) create mode 100644 app/policies/concerns/clusterable_actions.rb create mode 100644 changelogs/unreleased/34758-extend-can-create-cluster-logic.yml create mode 100644 spec/support/shared_examples/policies/clusterable_shared_examples.rb diff --git a/app/policies/concerns/clusterable_actions.rb b/app/policies/concerns/clusterable_actions.rb new file mode 100644 index 00000000000..08ddd742ea9 --- /dev/null +++ b/app/policies/concerns/clusterable_actions.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module ClusterableActions + private + + # Overridden on EE module + def multiple_clusters_available? + false + end + + def clusterable_has_clusters? + !subject.clusters.empty? + end +end diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 6b4e56ef5e4..ac98b80dc5c 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class GroupPolicy < BasePolicy + include ClusterableActions + desc "Group is public" with_options scope: :subject, score: 0 condition(:public_group) { @subject.public? } @@ -27,6 +29,9 @@ class GroupPolicy < BasePolicy GroupProjectsFinder.new(group: @subject, current_user: @user, options: { include_subgroups: true }).execute.any? end + condition(:has_clusters, scope: :subject) { clusterable_has_clusters? } + condition(:can_have_multiple_clusters) { multiple_clusters_available? } + with_options scope: :subject, score: 0 condition(:request_access_enabled) { @subject.request_access_enabled } @@ -44,7 +49,7 @@ class GroupPolicy < BasePolicy enable :read_label end - rule { admin } .enable :read_group + rule { admin }.enable :read_group rule { has_projects }.policy do enable :read_group @@ -66,6 +71,7 @@ class GroupPolicy < BasePolicy enable :admin_pipeline enable :admin_build enable :read_cluster + enable :add_cluster enable :create_cluster enable :update_cluster enable :admin_cluster @@ -105,6 +111,8 @@ class GroupPolicy < BasePolicy rule { owner & (~share_with_group_locked | ~has_parent | ~parent_share_with_group_locked | can_change_parent_share_with_group_lock) }.enable :change_share_with_group_lock + rule { ~can_have_multiple_clusters & has_clusters }.prevent :add_cluster + def access_level return GroupMember::NO_ACCESS if @user.nil? diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 1c082945299..bcbd9676f2e 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -2,6 +2,7 @@ class ProjectPolicy < BasePolicy extend ClassMethods + include ClusterableActions READONLY_FEATURES_WHEN_ARCHIVED = %i[ issue @@ -103,6 +104,9 @@ class ProjectPolicy < BasePolicy @subject.feature_available?(:merge_requests, @user) end + condition(:has_clusters, scope: :subject) { clusterable_has_clusters? } + condition(:can_have_multiple_clusters) { multiple_clusters_available? } + features = %w[ merge_requests issues @@ -257,6 +261,7 @@ class ProjectPolicy < BasePolicy enable :read_pages enable :update_pages enable :read_cluster + enable :add_cluster enable :create_cluster enable :update_cluster enable :admin_cluster @@ -381,6 +386,8 @@ class ProjectPolicy < BasePolicy (can?(:read_project_for_iids) & merge_requests_visible_to_user) | can?(:read_merge_request) end.enable :read_merge_request_iid + rule { ~can_have_multiple_clusters & has_clusters }.prevent :add_cluster + private def team_member? diff --git a/app/presenters/clusterable_presenter.rb b/app/presenters/clusterable_presenter.rb index 9cc137aa3bd..d94d9118eee 100644 --- a/app/presenters/clusterable_presenter.rb +++ b/app/presenters/clusterable_presenter.rb @@ -12,6 +12,10 @@ class ClusterablePresenter < Gitlab::View::Presenter::Delegated .fabricate! end + def can_add_cluster? + can?(current_user, :add_cluster, clusterable) + end + def can_create_cluster? can?(current_user, :create_cluster, clusterable) end diff --git a/app/views/clusters/clusters/_buttons.html.haml b/app/views/clusters/clusters/_buttons.html.haml index 9238903aa10..c81d1d5b05a 100644 --- a/app/views/clusters/clusters/_buttons.html.haml +++ b/app/views/clusters/clusters/_buttons.html.haml @@ -1,6 +1,5 @@ --# This partial is overridden in EE .nav-controls - - if clusterable.can_create_cluster? && clusterable.clusters.empty? + - if clusterable.can_add_cluster? = link_to s_('ClusterIntegration|Add Kubernetes cluster'), clusterable.new_path, class: 'btn btn-success js-add-cluster' - else %span.btn.btn-add-cluster.disabled.js-add-cluster diff --git a/app/views/clusters/clusters/_empty_state.html.haml b/app/views/clusters/clusters/_empty_state.html.haml index c926ec258f0..cfdbfe2dea1 100644 --- a/app/views/clusters/clusters/_empty_state.html.haml +++ b/app/views/clusters/clusters/_empty_state.html.haml @@ -9,6 +9,6 @@ = clusterable.empty_state_help_text = clusterable.learn_more_link - - if clusterable.can_create_cluster? + - if clusterable.can_add_cluster? .text-center = link_to s_('ClusterIntegration|Add Kubernetes cluster'), clusterable.new_path, class: 'btn btn-success' diff --git a/changelogs/unreleased/34758-extend-can-create-cluster-logic.yml b/changelogs/unreleased/34758-extend-can-create-cluster-logic.yml new file mode 100644 index 00000000000..65f5253a271 --- /dev/null +++ b/changelogs/unreleased/34758-extend-can-create-cluster-logic.yml @@ -0,0 +1,5 @@ +--- +title: Allow user to add Kubernetes cluster for clusterable when there are ancestor clusters +merge_request: 23569 +author: +type: other diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 9d0093e8159..e55401a80cd 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -25,7 +25,8 @@ describe GroupPolicy do :read_cluster, :create_cluster, :update_cluster, - :admin_cluster + :admin_cluster, + :add_cluster ] end @@ -382,4 +383,14 @@ describe GroupPolicy do it { expect_disallowed(:change_share_with_group_lock) } end end + + it_behaves_like 'clusterable policies' do + let(:clusterable) { create(:group) } + let(:cluster) do + create(:cluster, + :provided_by_gcp, + :group, + groups: [clusterable]) + end + end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 69468f9ad85..fa47b95899a 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -48,7 +48,7 @@ describe ProjectPolicy do update_deployment admin_project_snippet admin_project_member admin_note admin_wiki admin_project admin_commit_status admin_build admin_container_image - admin_pipeline admin_environment admin_deployment + admin_pipeline admin_environment admin_deployment add_cluster ] end @@ -465,4 +465,14 @@ describe ProjectPolicy do expect_disallowed(*maintainer_abilities) end end + + it_behaves_like 'clusterable policies' do + let(:clusterable) { create(:project, :repository) } + let(:cluster) do + create(:cluster, + :provided_by_gcp, + :project, + projects: [clusterable]) + end + end end diff --git a/spec/presenters/clusterable_presenter_spec.rb b/spec/presenters/clusterable_presenter_spec.rb index 4f4ae5e07c5..05afe5347d1 100644 --- a/spec/presenters/clusterable_presenter_spec.rb +++ b/spec/presenters/clusterable_presenter_spec.rb @@ -14,4 +14,68 @@ describe ClusterablePresenter do expect(subject).to be_kind_of(ProjectClusterablePresenter) end end + + shared_examples 'appropriate member permissions' do + context 'with a developer' do + before do + clusterable.add_developer(user) + end + + it { is_expected.to be_falsy } + end + + context 'with a maintainer' do + before do + clusterable.add_maintainer(user) + end + + it { is_expected.to be_truthy } + end + end + + describe '#can_create_cluster?' do + let(:user) { create(:user) } + + subject { described_class.new(clusterable).can_create_cluster? } + + before do + allow(clusterable).to receive(:current_user).and_return(user) + end + + context 'when clusterable is a group' do + let(:clusterable) { create(:group) } + + it_behaves_like 'appropriate member permissions' + end + + context 'when clusterable is a project' do + let(:clusterable) { create(:project, :repository) } + + it_behaves_like 'appropriate member permissions' + end + end + + describe '#can_add_cluster?' do + let(:user) { create(:user) } + + subject { described_class.new(clusterable).can_add_cluster? } + + before do + clusterable.add_maintainer(user) + + allow(clusterable).to receive(:current_user).and_return(user) + end + + context 'when clusterable is a group' do + let(:clusterable) { create(:group) } + + it_behaves_like 'appropriate member permissions' + end + + context 'when clusterable is a project' do + let(:clusterable) { create(:project, :repository) } + + it_behaves_like 'appropriate member permissions' + end + end end diff --git a/spec/support/shared_examples/policies/clusterable_shared_examples.rb b/spec/support/shared_examples/policies/clusterable_shared_examples.rb new file mode 100644 index 00000000000..d99f94c76c3 --- /dev/null +++ b/spec/support/shared_examples/policies/clusterable_shared_examples.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +shared_examples 'clusterable policies' do + describe '#add_cluster?' do + let(:current_user) { create(:user) } + + subject { described_class.new(current_user, clusterable) } + + context 'with a developer' do + before do + clusterable.add_developer(current_user) + end + + it { expect_disallowed(:add_cluster) } + end + + context 'with a maintainer' do + before do + clusterable.add_maintainer(current_user) + end + + context 'with no clusters' do + it { expect_allowed(:add_cluster) } + end + + context 'with an existing cluster' do + before do + cluster + end + + it { expect_disallowed(:add_cluster) } + end + end + end +end -- cgit v1.2.1