Automatic merge of gitlab-org/gitlab-ce master

app/controllers/groups/clusters_controller.rb

@@ -4,7 +4,6 @@ class Groups::ClustersController < Clusters::ClustersController
   include ControllerWithCrossProjectAccessCheck
 
   prepend_before_action :group
-  prepend_before_action :check_group_clusters_feature_flag!
   requires_cross_project_access
 
   layout 'group'
@@ -18,12 +17,4 @@ class Groups::ClustersController < Clusters::ClustersController
   def group
     @group ||= find_routable!(Group, params[:group_id] || params[:id])
   end
-
-  def check_group_clusters_feature_flag!
-    render_404 unless group_clusters_enabled?
-  end
-
-  def group_clusters_enabled?
-    group.group_clusters_enabled?
-  end
 end

app/helpers/groups_helper.rb

@@ -142,7 +142,7 @@ module GroupsHelper
       can?(current_user, "read_group_#{resource}".to_sym, @group)
     end
 
-    if can?(current_user, :read_cluster, @group) && @group.group_clusters_enabled?
+    if can?(current_user, :read_cluster, @group)
       links << :kubernetes
     end
 

app/models/clusters/instance.rb

@@ -9,9 +9,5 @@ module Clusters
     def feature_available?(feature)
       ::Feature.enabled?(feature, default_enabled: true)
     end
-
-    def self.enabled?
-      ::Feature.enabled?(:instance_clusters, default_enabled: true)
-    end
   end
 end

app/models/concerns/deployment_platform.rb

@@ -13,8 +13,8 @@ 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_group_cluster_platform_kubernetes(environment: environment) ||
+      find_instance_cluster_platform_kubernetes(environment: environment)
   end
 
   # EE would override this and utilize environment argument
@@ -23,24 +23,12 @@ module DeploymentPlatform
       .last&.platform_kubernetes
   end
 
-  def find_group_cluster_platform_kubernetes_with_feature_guard(environment: nil)
-    return unless group_clusters_enabled?
-
-    find_group_cluster_platform_kubernetes(environment: environment)
-  end
-
   # EE would override this and utilize environment argument
   def find_group_cluster_platform_kubernetes(environment: nil)
     Clusters::Cluster.enabled.default_environment.ancestor_clusters_for_clusterable(self)
       .first&.platform_kubernetes
   end
 
-  def find_instance_cluster_platform_kubernetes_with_feature_guard(environment: nil)
-    return unless Clusters::Instance.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

app/models/group.rb

@@ -410,10 +410,6 @@ class Group < Namespace
     ensure_runners_token!
   end
 
-  def group_clusters_enabled?
-    Feature.enabled?(:group_clusters, root_ancestor, default_enabled: true)
-  end
-
   def project_creation_level
     super || ::Gitlab::CurrentSettings.default_project_creation
   end

app/models/project.rb

@@ -306,7 +306,6 @@ class Project < ApplicationRecord
   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
   delegate :last_pipeline, to: :commit, allow_nil: true
   delegate :external_dashboard_url, to: :metrics_setting, allow_nil: true, prefix: true

app/policies/clusters/instance_policy.rb

@@ -6,9 +6,8 @@ module Clusters
 
     condition(:has_clusters, scope: :subject) { clusterable_has_clusters? }
     condition(:can_have_multiple_clusters) { multiple_clusters_available? }
-    condition(:instance_clusters_enabled) { Instance.enabled? }
 
-    rule { admin & instance_clusters_enabled }.policy do
+    rule { admin }.policy do
       enable :read_cluster
       enable :add_cluster
       enable :create_cluster

changelogs/unreleased/remove_group_and_instance_clusters_feature_flag.yml

+---
+title: Remove group and instance clusters feature flag
+merge_request: 30124
+author:
+type: changed

spec/controllers/admin/clusters/applications_controller_spec.rb

@@ -13,16 +13,6 @@ describe Admin::Clusters::ApplicationsController do
     it { expect { subject }.to be_allowed_for(:admin) }
     it { expect { subject }.to be_denied_for(:user) }
     it { expect { subject }.to be_denied_for(:external) }
-
-    context 'when instance clusters are disabled' do
-      before do
-        stub_feature_flags(instance_clusters: false)
-      end
-
-      it 'returns 404' do
-        is_expected.to have_http_status(:not_found)
-      end
-    end
   end
 
   let(:cluster) { create(:cluster, :instance, :provided_by_gcp) }

spec/controllers/admin/clusters_controller_spec.rb

@@ -17,66 +17,48 @@ describe Admin::ClustersController do
       get :index, params: params
     end
 
-    context 'when feature flag is not enabled' do
-      before do
-        stub_feature_flags(instance_clusters: false)
-      end
+    describe 'functionality' do
+      context 'when instance has one or more clusters' do
+        let!(:enabled_cluster) do
+          create(:cluster, :provided_by_gcp, :instance)
+        end
 
-      it 'responds with not found' do
-        get_index
+        let!(:disabled_cluster) do
+          create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance)
+        end
 
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
+        it 'lists available clusters' do
+          get_index
 
-    context 'when feature flag is enabled' do
-      before do
-        stub_feature_flags(instance_clusters: true)
-      end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:index)
+          expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster])
+        end
 
-      describe 'functionality' do
-        context 'when instance has one or more clusters' do
-          let!(:enabled_cluster) do
-            create(:cluster, :provided_by_gcp, :instance)
-          end
+        context 'when page is specified' do
+          let(:last_page) { Clusters::Cluster.instance_type.page.total_pages }
 
-          let!(:disabled_cluster) do
-            create(:cluster, :disabled, :provided_by_gcp, :production_environment, :instance)
+          before do
+            allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
+            create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance)
           end
 
-          it 'lists available clusters' do
-            get_index
+          it 'redirects to the page' do
+            get_index(page: last_page)
 
             expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to render_template(:index)
-            expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster])
-          end
-
-          context 'when page is specified' do
-            let(:last_page) { Clusters::Cluster.instance_type.page.total_pages }
-
-            before do
-              allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
-              create_list(:cluster, 2, :provided_by_gcp, :production_environment, :instance)
-            end
-
-            it 'redirects to the page' do
-              get_index(page: last_page)
-
-              expect(response).to have_gitlab_http_status(:ok)
-              expect(assigns(:clusters).current_page).to eq(last_page)
-            end
+            expect(assigns(:clusters).current_page).to eq(last_page)
           end
         end
+      end
 
-        context 'when instance does not have a cluster' do
-          it 'returns an empty state page' do
-            get_index
+      context 'when instance does not have a cluster' do
+        it 'returns an empty state page' do
+          get_index
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to render_template(:index, partial: :empty_state)
-            expect(assigns(:clusters)).to eq([])
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:index, partial: :empty_state)
+          expect(assigns(:clusters)).to eq([])
         end
       end
     end

spec/controllers/groups/clusters_controller_spec.rb

@@ -20,70 +20,52 @@ describe Groups::ClustersController do
       get :index, params: params.reverse_merge(group_id: group)
     end
 
-    context 'when feature flag is not enabled' do
-      before do
-        stub_feature_flags(group_clusters: false)
-      end
+    describe 'functionality' do
+      context 'when group has one or more clusters' do
+        let(:group) { create(:group) }
 
-      it 'renders 404' do
-        go
+        let!(:enabled_cluster) do
+          create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group])
+        end
 
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
+        let!(:disabled_cluster) do
+          create(:cluster, :disabled, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group])
+        end
 
-    context 'when feature flag is enabled' do
-      before do
-        stub_feature_flags(group_clusters: true)
-      end
+        it 'lists available clusters' do
+          go
 
-      describe 'functionality' do
-        context 'when group has one or more clusters' do
-          let(:group) { create(:group) }
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:index)
+          expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster])
+        end
 
-          let!(:enabled_cluster) do
-            create(:cluster, :provided_by_gcp, cluster_type: :group_type, groups: [group])
-          end
+        context 'when page is specified' do
+          let(:last_page) { group.clusters.page.total_pages }
 
-          let!(:disabled_cluster) do
-            create(:cluster, :disabled, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group])
+          before do
+            allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
+            create_list(:cluster, 2, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group])
           end
 
-          it 'lists available clusters' do
-            go
+          it 'redirects to the page' do
+            go(page: last_page)
 
             expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to render_template(:index)
-            expect(assigns(:clusters)).to match_array([enabled_cluster, disabled_cluster])
-          end
-
-          context 'when page is specified' do
-            let(:last_page) { group.clusters.page.total_pages }
-
-            before do
-              allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
-              create_list(:cluster, 2, :provided_by_gcp, :production_environment, cluster_type: :group_type, groups: [group])
-            end
-
-            it 'redirects to the page' do
-              go(page: last_page)
-
-              expect(response).to have_gitlab_http_status(:ok)
-              expect(assigns(:clusters).current_page).to eq(last_page)
-            end
+            expect(assigns(:clusters).current_page).to eq(last_page)
           end
         end
+      end
 
-        context 'when group does not have a cluster' do
-          let(:group) { create(:group) }
+      context 'when group does not have a cluster' do
+        let(:group) { create(:group) }
 
-          it 'returns an empty state page' do
-            go
+        it 'returns an empty state page' do
+          go
 
-            expect(response).to have_gitlab_http_status(:ok)
-            expect(response).to render_template(:index, partial: :empty_state)
-            expect(assigns(:clusters)).to eq([])
-          end
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template(:index, partial: :empty_state)
+          expect(assigns(:clusters)).to eq([])
         end
       end
     end

spec/models/concerns/deployment_platform_spec.rb

@@ -82,16 +82,6 @@ describe DeploymentPlatform do
           end
         end
       end
-
-      context 'feature flag disabled' do
-        before do
-          stub_feature_flags(group_clusters: false)
-        end
-
-        it 'returns nil' do
-          is_expected.to be_nil
-        end
-      end
     end
   end
 end

spec/models/group_spec.rb

@@ -866,35 +866,6 @@ describe Group do
     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
-
   describe '#first_auto_devops_config' do
     using RSpec::Parameterized::TableSyntax
 

spec/models/project_spec.rb

@@ -497,7 +497,6 @@ 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) }
     it { is_expected.to delegate_method(:last_pipeline).to(:commit).with_arguments(allow_nil: true) }
   end

spec/policies/clusters/instance_policy_spec.rb

@@ -16,21 +16,9 @@ describe Clusters::InstancePolicy do
     context 'when admin' do
       let(:user) { create(:admin) }
 
-      context 'with instance_level_clusters enabled' do
-        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
-
-      context 'with instance_level_clusters disabled' do
-        before do
-          stub_feature_flags(instance_clusters: false)
-        end
-
-        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
+      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
   end
 end