Merge branch '5347-fix-multiple-clusters-incorrect-details-injected' into 'master'

Resolve "Multiple clusters: incorrect cluster details injected - environment scope is ignored"

Closes #5347

See merge request gitlab-org/gitlab-ee!5047

app/models/clusters/cluster.rb

@@ -11,6 +11,7 @@ module Clusters
       Applications::Prometheus.application_name => Applications::Prometheus,
       Applications::Runner.application_name => Applications::Runner
     }.freeze
+    DEFAULT_ENVIRONMENT = '*'.freeze
 
     belongs_to :user
 
@@ -52,6 +53,7 @@ module Clusters
 
     scope :enabled, -> { where(enabled: true) }
     scope :disabled, -> { where(enabled: false) }
+    scope :default_environment, -> { where(environment_scope: DEFAULT_ENVIRONMENT) }
 
     def status_name
       if provider

app/models/concerns/deployment_platform.rb

 module DeploymentPlatform
-  # EE would override this and utilize the extra argument
+  # EE would override this and utilize environment argument
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def deployment_platform(environment: nil)
-    @deployment_platform ||=
-      find_cluster_platform_kubernetes ||
-      find_kubernetes_service_integration ||
-      build_cluster_and_deployment_platform
+    @deployment_platform ||= {}
+
+    @deployment_platform[environment] ||= find_deployment_platform(environment)
   end
 
   private
 
-  def find_cluster_platform_kubernetes
-    clusters.find_by(enabled: true)&.platform_kubernetes
+  def find_deployment_platform(environment)
+    find_cluster_platform_kubernetes(environment: environment) ||
+      find_kubernetes_service_integration ||
+      build_cluster_and_deployment_platform
+  end
+
+  # EE would override this and utilize environment argument
+  def find_cluster_platform_kubernetes(environment: nil)
+    clusters.enabled.default_environment
+      .last&.platform_kubernetes
   end
 
   def find_kubernetes_service_integration

ee/app/models/concerns/ee/deployment_platform.rb

@@ -2,15 +2,12 @@ module EE
   module DeploymentPlatform
     extend ::Gitlab::Utils::Override
 
-    override :deployment_platform
-    def deployment_platform(environment: nil)
+    override :find_cluster_platform_kubernetes
+    def find_cluster_platform_kubernetes(environment: nil)
       return super unless environment && feature_available?(:multiple_clusters)
 
-      @deployment_platform ||= # rubocop:disable Gitlab/ModuleWithInstanceVariables
-        clusters.enabled.on_environment(environment.name)
-          .last&.platform_kubernetes
-
-      super # Wildcard or KubernetesService
+      clusters.enabled.on_environment(environment.name)
+        .last&.platform_kubernetes
     end
   end
 end

ee/changelogs/unreleased/5347-fix-multiple-clusters-incorrect-details-injected.yml

+---
+title: Fixes incorrect assignation of cluster details
+merge_request: 5047
+author:
+type: fixed

ee/spec/features/projects/show_project_spec.rb

@@ -17,7 +17,7 @@ describe 'Project show page', :feature do
 
         it '"Kubernetes cluster" button linked to clusters page' do
           create(:cluster, :provided_by_gcp, projects: [project])
-          create(:cluster, :provided_by_gcp, projects: [project])
+          create(:cluster, :provided_by_gcp, :production_environment, projects: [project])
 
           visit project_path(project)
 

ee/spec/models/concerns/ee/deployment_platform_spec.rb

@@ -4,56 +4,56 @@ describe EE::DeploymentPlatform do
   describe '#deployment_platform' do
     let(:project) { create(:project) }
 
-    context 'when environment is specified' do
-      let(:environment) { create(:environment, project: project, name: 'review/name') }
-      let!(:default_cluster) { create(:cluster, :provided_by_user, projects: [project], environment_scope: '*') }
-      let!(:cluster) { create(:cluster, :provided_by_user, environment_scope: 'review/*', projects: [project]) }
-
-      subject { project.deployment_platform(environment: environment) }
-
-      shared_examples 'matching environment scope' do
-        context 'when multiple clusters is available' do
-          before do
-            stub_licensed_features(multiple_clusters: true)
-          end
+    shared_examples 'matching environment scope' do
+      context 'when multiple clusters license is available' do
+        before do
+          stub_licensed_features(multiple_clusters: true)
+        end
 
-          it 'returns environment specific cluster' do
-            is_expected.to eq(cluster.platform_kubernetes)
-          end
+        it 'returns environment specific cluster' do
+          is_expected.to eq(cluster.platform_kubernetes)
         end
+      end
 
-        context 'when multiple clusters is unavailable' do
-          before do
-            stub_licensed_features(multiple_clusters: false)
-          end
+      context 'when multiple clusters licence is unavailable' do
+        before do
+          stub_licensed_features(multiple_clusters: false)
+        end
 
-          it 'returns a kubernetes platform' do
-            is_expected.to be_kind_of(Clusters::Platforms::Kubernetes)
-          end
+        it 'returns a kubernetes platform' do
+          is_expected.to be_kind_of(Clusters::Platforms::Kubernetes)
         end
       end
+    end
 
-      shared_examples 'not matching environment scope' do
-        context 'when multiple clusters is available' do
-          before do
-            stub_licensed_features(multiple_clusters: true)
-          end
+    shared_examples 'not matching environment scope' do
+      context 'when multiple clusters license is available' do
+        before do
+          stub_licensed_features(multiple_clusters: true)
+        end
 
-          it 'returns default cluster' do
-            is_expected.to eq(default_cluster.platform_kubernetes)
-          end
+        it 'returns default cluster' do
+          is_expected.to eq(default_cluster.platform_kubernetes)
         end
+      end
 
-        context 'when multiple clusters is unavailable' do
-          before do
-            stub_licensed_features(multiple_clusters: false)
-          end
+      context 'when multiple clusters license is unavailable' do
+        before do
+          stub_licensed_features(multiple_clusters: false)
+        end
 
-          it 'returns a kubernetes platform' do
-            is_expected.to be_kind_of(Clusters::Platforms::Kubernetes)
-          end
+        it 'returns a kubernetes platform' do
+          is_expected.to be_kind_of(Clusters::Platforms::Kubernetes)
         end
       end
+    end
+
+    context 'when environment is specified' do
+      let!(:default_cluster) { create(:cluster, :provided_by_user, projects: [project], environment_scope: '*') }
+      let!(:cluster) { create(:cluster, :provided_by_user, environment_scope: 'review/*', projects: [project]) }
+      let(:environment) { create(:environment, project: project, name: 'review/name') }
+
+      subject { project.deployment_platform(environment: environment) }
 
       context 'when environment scope is exactly matched' do
         before do
@@ -133,5 +133,21 @@ describe EE::DeploymentPlatform do
         end
       end
     end
+
+    context 'with multiple clusters and multiple environments' do
+      let!(:cluster_1) { create(:cluster, :provided_by_user, projects: [project], environment_scope: 'staging/*') }
+      let!(:cluster_2) { create(:cluster, :provided_by_user, projects: [project], environment_scope: 'test/*') }
+      let(:environment_1) { create(:environment, project: project, name: 'staging/name') }
+      let(:environment_2) { create(:environment, project: project, name: 'test/name') }
+
+      before do
+        stub_licensed_features(multiple_clusters: true)
+      end
+
+      it 'should return the appropriate cluster' do
+        expect(project.deployment_platform(environment: environment_1)).to eq(cluster_1.platform_kubernetes)
+        expect(project.deployment_platform(environment: environment_2)).to eq(cluster_2.platform_kubernetes)
+      end
+    end
   end
 end

spec/controllers/projects/clusters_controller_spec.rb

@@ -18,7 +18,7 @@ describe Projects::ClustersController do
       context 'when project has one or more clusters' do
         let(:project) { create(:project) }
         let!(:enabled_cluster) { create(:cluster, :provided_by_gcp, projects: [project]) }
-        let!(:disabled_cluster) { create(:cluster, :disabled, :provided_by_gcp, projects: [project]) }
+        let!(:disabled_cluster) { create(:cluster, :disabled, :provided_by_gcp, :production_environment, projects: [project]) }
         it 'lists available clusters' do
           go
 
@@ -32,7 +32,7 @@ describe Projects::ClustersController do
 
           before do
             allow(Clusters::Cluster).to receive(:paginates_per).and_return(1)
-            create_list(:cluster, 2, :provided_by_gcp, projects: [project])
+            create_list(:cluster, 2, :provided_by_gcp, :production_environment, projects: [project])
             get :index, namespace_id: project.namespace, project_id: project, page: last_page
           end
 
@@ -420,7 +420,7 @@ describe Projects::ClustersController do
 
       context 'when cluster is provided by GCP' do
         context 'when cluster is created' do
-          let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) }
+          let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) }
 
           it "destroys and redirects back to clusters list" do
             expect { go }
@@ -434,7 +434,7 @@ describe Projects::ClustersController do
         end
 
         context 'when cluster is being created' do
-          let!(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) }
+          let!(:cluster) { create(:cluster, :providing_by_gcp, :production_environment, projects: [project]) }
 
           it "destroys and redirects back to clusters list" do
             expect { go }
@@ -448,7 +448,7 @@ describe Projects::ClustersController do
       end
 
       context 'when cluster is provided by user' do
-        let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) }
+        let!(:cluster) { create(:cluster, :provided_by_user, :production_environment, projects: [project]) }
 
         it "destroys and redirects back to clusters list" do
           expect { go }
@@ -463,7 +463,7 @@ describe Projects::ClustersController do
     end
 
     describe 'security' do
-      set(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) }
+      set(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) }
 
       it { expect { go }.to be_allowed_for(:admin) }
       it { expect { go }.to be_allowed_for(:owner).of(project) }

spec/factories/clusters/clusters.rb

@@ -2,7 +2,6 @@ FactoryBot.define do
   factory :cluster, class: Clusters::Cluster do
     user
     name 'test-cluster'
-    sequence(:environment_scope) { |n| "production#{n}/*" }
 
     trait :project do
       before(:create) do |cluster, evaluator|
@@ -33,5 +32,9 @@ FactoryBot.define do
     trait :disabled do
       enabled false
     end
+
+    trait :production_environment do
+      sequence(:environment_scope) { |n| "production#{n}/*" }
+    end
   end
 end

spec/finders/clusters_finder_spec.rb

@@ -6,7 +6,7 @@ describe ClustersFinder do
 
   describe '#execute' do
     let(:enabled_cluster) { create(:cluster, :provided_by_gcp, projects: [project]) }
-    let(:disabled_cluster) { create(:cluster, :disabled, :provided_by_gcp, projects: [project]) }
+    let(:disabled_cluster) { create(:cluster, :disabled, :provided_by_gcp, :production_environment, projects: [project]) }
 
     subject { described_class.new(project, user, scope).execute }
 

spec/models/environment_spec.rb

@@ -378,7 +378,7 @@ describe Environment do
 
       shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do
         it 'returns the terminals from the deployment service' do
-          expect(project.deployment_platform)
+          expect(project.deployment_platform(environment: environment))
             .to receive(:terminals).with(environment)
             .and_return(:fake_terminals)
 
@@ -419,7 +419,7 @@ describe Environment do
         end
 
         it 'returns the rollout status from the deployment service' do
-          expect(project.deployment_platform)
+          expect(project.deployment_platform(environment: environment))
             .to receive(:rollout_status).with(environment)
             .and_return(:fake_rollout_status)
 

spec/presenters/project_presenter_spec.rb

@@ -339,7 +339,7 @@ describe ProjectPresenter do
 
         it 'returns link to clusters page if more than one exists' do
           project.add_master(user)
-          create(:cluster, projects: [project])
+          create(:cluster, :production_environment, projects: [project])
           create(:cluster, projects: [project])
 
           expect(presenter.kubernetes_cluster_anchor_data).to eq(OpenStruct.new(enabled: true,

spec/services/clusters/create_service_spec.rb

@@ -81,7 +81,7 @@ describe Clusters::CreateService do
     end
 
     context 'when project has a cluster' do
-      let!(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) }
+      let!(:cluster) { create(:cluster, :provided_by_gcp, :production_environment, projects: [project]) }
 
       before do
         allow(project).to receive(:feature_available?).and_call_original