From 8db382b05545fdef0a60bcff65f8c23e8b1ed282 Mon Sep 17 00:00:00 2001 From: James Fargher Date: Thu, 18 Apr 2019 14:45:31 +1200 Subject: [PATCH] Allow projects to use instance level clusters There are two cluster hierarchies one for the deployment platform and one for controllers. The main difference is that deployment platforms do not check user permissions and only return the first match. --- app/controllers/admin/clusters_controller.rb | 12 ++++++-- app/models/clusters/cluster.rb | 4 ++- app/models/clusters/instance.rb | 4 +++ app/models/concerns/deployment_platform.rb | 17 +++++++++++ spec/finders/cluster_ancestors_finder_spec.rb | 29 ++++++++++++++----- spec/models/clusters/cluster_spec.rb | 9 ++++++ .../policies/clusters/instance_policy_spec.rb | 2 ++ 7 files changed, 66 insertions(+), 11 deletions(-) diff --git a/app/controllers/admin/clusters_controller.rb b/app/controllers/admin/clusters_controller.rb index 777bdf5c981..f2b54125d48 100644 --- a/app/controllers/admin/clusters_controller.rb +++ b/app/controllers/admin/clusters_controller.rb @@ -8,10 +8,18 @@ class Admin::ClustersController < Clusters::ClustersController private def clusterable - @clusterable ||= InstanceClusterablePresenter.fabricate(Clusters::Instance.new, current_user: current_user) + @clusterable ||= InstanceClusterablePresenter.fabricate(cluster_instance, current_user: current_user) + end + + def cluster_instance + @cluster_instance ||= Clusters::Instance.new end def check_instance_clusters_feature_flag! - render_404 unless Feature.enabled?(:instance_clusters, default_enabled: true) + render_404 unless instance_clusters_enabled? + end + + def instance_clusters_enabled? + cluster_instance.instance_clusters_enabled? end end diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 7220159ac95..9299e61dad3 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -115,10 +115,12 @@ module Clusters } def self.ancestor_clusters_for_clusterable(clusterable, hierarchy_order: :asc) + return [] if clusterable.is_a?(Instance) + hierarchy_groups = clusterable.ancestors_upto(hierarchy_order: hierarchy_order).eager_load(:clusters) hierarchy_groups = hierarchy_groups.merge(current_scope) if current_scope - hierarchy_groups.flat_map(&:clusters) + hierarchy_groups.flat_map(&:clusters) + Instance.new.clusters end def status_name diff --git a/app/models/clusters/instance.rb b/app/models/clusters/instance.rb index fde83c5a8ad..bbbf6da16fb 100644 --- a/app/models/clusters/instance.rb +++ b/app/models/clusters/instance.rb @@ -8,4 +8,8 @@ class Clusters::Instance def feature_available?(feature) ::Feature.enabled?(feature, default_enabled: true) end + + def instance_clusters_enabled? + ::Feature.enabled?(:instance_clusters, default_enabled: true) + end end diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index 0107af5f8ec..c2dafec1a54 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -14,6 +14,7 @@ module DeploymentPlatform def find_deployment_platform(environment) find_cluster_platform_kubernetes(environment: environment) || find_group_cluster_platform_kubernetes_with_feature_guard(environment: environment) || + find_instance_cluster_platform_kubernetes_with_feature_guard(environment: environment) || find_kubernetes_service_integration || build_cluster_and_deployment_platform end @@ -36,6 +37,22 @@ module DeploymentPlatform .first&.platform_kubernetes end + def find_instance_cluster_platform_kubernetes_with_feature_guard(environment: nil) + return unless instance_clusters_enabled? + + find_instance_cluster_platform_kubernetes(environment: environment) + end + + # EE would override this and utilize environment argument + def find_instance_cluster_platform_kubernetes(environment: nil) + Clusters::Instance.new.clusters.enabled.default_environment + .first&.platform_kubernetes + end + + def instance_clusters_enabled? + Feature.enabled?(:instance_clusters, default_enabled: true) + end + def find_kubernetes_service_integration services.deployment.reorder(nil).find_by(active: true) end diff --git a/spec/finders/cluster_ancestors_finder_spec.rb b/spec/finders/cluster_ancestors_finder_spec.rb index 332086c42e2..750042b6b54 100644 --- a/spec/finders/cluster_ancestors_finder_spec.rb +++ b/spec/finders/cluster_ancestors_finder_spec.rb @@ -8,11 +8,15 @@ describe ClusterAncestorsFinder, '#execute' do let(:user) { create(:user) } let!(:project_cluster) do - create(:cluster, :provided_by_user, cluster_type: :project_type, projects: [project]) + create(:cluster, :provided_by_user, :project, projects: [project]) end let!(:group_cluster) do - create(:cluster, :provided_by_user, cluster_type: :group_type, groups: [group]) + create(:cluster, :provided_by_user, :group, groups: [group]) + end + + let!(:instance_cluster) do + create(:cluster, :provided_by_user, :instance) end subject { described_class.new(clusterable, user).execute } @@ -25,7 +29,7 @@ describe ClusterAncestorsFinder, '#execute' do end it 'returns the project clusters followed by group clusters' do - is_expected.to eq([project_cluster, group_cluster]) + is_expected.to eq([project_cluster, group_cluster, instance_cluster]) end context 'nested groups', :nested_groups do @@ -33,11 +37,11 @@ describe ClusterAncestorsFinder, '#execute' do let(:parent_group) { create(:group) } let!(:parent_group_cluster) do - create(:cluster, :provided_by_user, cluster_type: :group_type, groups: [parent_group]) + create(:cluster, :provided_by_user, :group, groups: [parent_group]) end it 'returns the project clusters followed by group clusters ordered ascending the hierarchy' do - is_expected.to eq([project_cluster, group_cluster, parent_group_cluster]) + is_expected.to eq([project_cluster, group_cluster, parent_group_cluster, instance_cluster]) end end end @@ -58,7 +62,7 @@ describe ClusterAncestorsFinder, '#execute' do end it 'returns the list of group clusters' do - is_expected.to eq([group_cluster]) + is_expected.to eq([group_cluster, instance_cluster]) end context 'nested groups', :nested_groups do @@ -66,12 +70,21 @@ describe ClusterAncestorsFinder, '#execute' do let(:parent_group) { create(:group) } let!(:parent_group_cluster) do - create(:cluster, :provided_by_user, cluster_type: :group_type, groups: [parent_group]) + create(:cluster, :provided_by_user, :group, groups: [parent_group]) end it 'returns the list of group clusters ordered ascending the hierarchy' do - is_expected.to eq([group_cluster, parent_group_cluster]) + is_expected.to eq([group_cluster, parent_group_cluster, instance_cluster]) end end end + + context 'for an instance' do + let(:clusterable) { Clusters::Instance.new } + let(:user) { create(:admin) } + + it 'returns the list of instance clusters' do + is_expected.to eq([instance_cluster]) + end + end end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index e1506c06044..58203da5b22 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -325,6 +325,15 @@ describe Clusters::Cluster do end end + context 'when group and instance have configured kubernetes clusters' do + let(:project) { create(:project, group: group) } + let!(:instance_cluster) { create(:cluster, :provided_by_gcp, :instance) } + + it 'returns clusters in order, descending the hierachy' do + is_expected.to eq([group_cluster, instance_cluster]) + end + end + context 'when sub-group has configured kubernetes cluster', :nested_groups do let(:sub_group_cluster) { create(:cluster, :provided_by_gcp, :group) } let(:sub_group) { sub_group_cluster.group } diff --git a/spec/policies/clusters/instance_policy_spec.rb b/spec/policies/clusters/instance_policy_spec.rb index ac0f9da5d19..f4652c2ad00 100644 --- a/spec/policies/clusters/instance_policy_spec.rb +++ b/spec/policies/clusters/instance_policy_spec.rb @@ -9,6 +9,7 @@ describe Clusters::InstancePolicy do describe 'rules' do context 'when user' do + it { expect(policy).to be_disallowed :read_cluster } it { expect(policy).to be_disallowed :update_cluster } it { expect(policy).to be_disallowed :admin_cluster } end @@ -16,6 +17,7 @@ describe Clusters::InstancePolicy do context 'when admin' do let(:user) { create(:admin) } + it { expect(policy).to be_allowed :read_cluster } it { expect(policy).to be_allowed :update_cluster } it { expect(policy).to be_allowed :admin_cluster } end