From ebf87fd9c2e1c5bde72f2c08db0fff7e81882cb8 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Thu, 29 Nov 2018 13:50:19 +1300 Subject: [PATCH] Unify into :group_clusters feature flag With this MR, group clusters is now functional, so default to enabled. Have a single setting on the root ancestor group to enabled or disable group clusters feature as a whole --- app/controllers/groups/clusters_controller.rb | 8 +++-- app/helpers/groups_helper.rb | 2 +- app/models/concerns/deployment_platform.rb | 2 +- app/models/group.rb | 4 +++ app/models/namespace.rb | 2 +- app/models/project.rb | 2 ++ .../concerns/deployment_platform_spec.rb | 4 +-- spec/models/group_spec.rb | 29 +++++++++++++++++++ spec/models/namespace_spec.rb | 1 + spec/models/project_spec.rb | 25 ++++++++++++++++ 10 files changed, 71 insertions(+), 8 deletions(-) diff --git a/app/controllers/groups/clusters_controller.rb b/app/controllers/groups/clusters_controller.rb index 50c44b7a58b..b846fb21266 100644 --- a/app/controllers/groups/clusters_controller.rb +++ b/app/controllers/groups/clusters_controller.rb @@ -3,8 +3,8 @@ class Groups::ClustersController < Clusters::ClustersController include ControllerWithCrossProjectAccessCheck - prepend_before_action :check_group_clusters_feature_flag! prepend_before_action :group + prepend_before_action :check_group_clusters_feature_flag! requires_cross_project_access layout 'group' @@ -20,6 +20,10 @@ class Groups::ClustersController < Clusters::ClustersController end def check_group_clusters_feature_flag! - render_404 unless Feature.enabled?(:group_clusters) + render_404 unless group_clusters_enabled? + end + + def group_clusters_enabled? + group.group_clusters_enabled? end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index e9b9b9b7721..866fc555856 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -140,7 +140,7 @@ module GroupsHelper can?(current_user, "read_group_#{resource}".to_sym, @group) end - if can?(current_user, :read_cluster, @group) && Feature.enabled?(:group_clusters) + if can?(current_user, :read_cluster, @group) && @group.group_clusters_enabled? links << :kubernetes end diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index ad981f4a8a3..0107af5f8ec 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -25,7 +25,7 @@ module DeploymentPlatform end def find_group_cluster_platform_kubernetes_with_feature_guard(environment: nil) - return unless Feature.enabled?(:deploy_group_clusters, default_enabled: true) + return unless group_clusters_enabled? find_group_cluster_platform_kubernetes(environment: environment) end diff --git a/app/models/group.rb b/app/models/group.rb index 02ddc8762af..233747cc2c2 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -400,6 +400,10 @@ class Group < Namespace ensure_runners_token! end + def group_clusters_enabled? + Feature.enabled?(:group_clusters, root_ancestor, default_enabled: true) + end + private def update_two_factor_requirement diff --git a/app/models/namespace.rb b/app/models/namespace.rb index c38310ed62b..8865c164b11 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -243,7 +243,7 @@ class Namespace < ActiveRecord::Base end def root_ancestor - ancestors.reorder(nil).find_by(parent_id: nil) + self_and_ancestors.reorder(nil).find_by(parent_id: nil) end def subgroup? diff --git a/app/models/project.rb b/app/models/project.rb index 602347d4bac..5a35a6a1a2a 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -291,6 +291,8 @@ class Project < ActiveRecord::Base delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team delegate :add_master, to: :team # @deprecated delegate :group_runners_enabled, :group_runners_enabled=, :group_runners_enabled?, to: :ci_cd_settings + delegate :group_clusters_enabled?, to: :group, allow_nil: true + delegate :root_ancestor, to: :namespace, allow_nil: true # Validations validates :creator, presence: true, on: :create diff --git a/spec/models/concerns/deployment_platform_spec.rb b/spec/models/concerns/deployment_platform_spec.rb index bdd352c52b8..19ab4382b53 100644 --- a/spec/models/concerns/deployment_platform_spec.rb +++ b/spec/models/concerns/deployment_platform_spec.rb @@ -69,8 +69,6 @@ describe DeploymentPlatform do let(:group) { group_cluster.group } before do - stub_feature_flags(deploy_group_clusters: true) - project.update!(group: group) end @@ -118,7 +116,7 @@ describe DeploymentPlatform do context 'feature flag disabled' do before do - stub_feature_flags(deploy_group_clusters: false) + stub_feature_flags(group_clusters: false) end it 'returns nil' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ada00f03928..0c3a49cd0f2 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -745,4 +745,33 @@ describe Group do let(:uploader_class) { AttachmentUploader } end end + + describe '#group_clusters_enabled?' do + before do + # Override global stub in spec/spec_helper.rb + expect(Feature).to receive(:enabled?).and_call_original + end + + subject { group.group_clusters_enabled? } + + it { is_expected.to be_truthy } + + context 'explicitly disabled for root ancestor' do + before do + feature = Feature.get(:group_clusters) + feature.disable(group.root_ancestor) + end + + it { is_expected.to be_falsey } + end + + context 'explicitly disabled for root ancestor' do + before do + feature = Feature.get(:group_clusters) + feature.enable(group.root_ancestor) + end + + it { is_expected.to be_truthy } + end + end end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 84930d11a06..6ee19c0ddf4 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -721,6 +721,7 @@ describe Namespace do deep_nested_group = create(:group, parent: nested_group) very_deep_nested_group = create(:group, parent: deep_nested_group) + expect(root_group.root_ancestor).to eq(root_group) expect(nested_group.root_ancestor).to eq(root_group) expect(deep_nested_group.root_ancestor).to eq(root_group) expect(very_deep_nested_group.root_ancestor).to eq(root_group) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 68dcb8b3e22..21c81752301 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -412,6 +412,8 @@ describe Project do it { is_expected.to delegate_method(:members).to(:team).with_prefix(true) } it { is_expected.to delegate_method(:name).to(:owner).with_prefix(true).with_arguments(allow_nil: true) } + it { is_expected.to delegate_method(:group_clusters_enabled?).to(:group).with_arguments(allow_nil: true) } + it { is_expected.to delegate_method(:root_ancestor).to(:namespace).with_arguments(allow_nil: true) } end describe '#to_reference_with_postfix' do @@ -2129,6 +2131,29 @@ describe Project do end end + describe '#root_ancestor' do + let(:project) { create(:project) } + + subject { project.root_ancestor } + + it { is_expected.to eq(project.namespace) } + + context 'in a group' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + + it { is_expected.to eq(group) } + end + + context 'in a nested group', :nested_groups do + let(:root) { create(:group) } + let(:child) { create(:group, parent: root) } + let(:project) { create(:project, group: child) } + + it { is_expected.to eq(root) } + end + end + describe '#lfs_enabled?' do let(:project) { create(:project) }