Merge branch '34758-extend-can-create-cluster-logic' into 'master'

Allow user to add cluster when there are ancestor clusters

See merge request gitlab-org/gitlab-ce!23569

app/policies/concerns/clusterable_actions.rb

+# 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

app/policies/group_policy.rb

 # 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 }
 
@@ -45,7 +50,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
@@ -67,6 +72,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
@@ -106,6 +112,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?
 

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?

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

app/views/clusters/clusters/_buttons.html.haml

--# 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

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'

changelogs/unreleased/34758-extend-can-create-cluster-logic.yml

+---
+title: Allow user to add Kubernetes cluster for clusterable when there are ancestor clusters
+merge_request: 23569
+author:
+type: other

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

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

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

spec/support/shared_examples/policies/clusterable_shared_examples.rb

+# 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