Merge branch 'fix/sm/environment-scope-validation-per-project' into...

Merge branch 'fix/sm/environment-scope-validation-per-project' into ee-fix-cluster-enviroment-missing

app/models/clusters/cluster.rb

@@ -27,7 +27,7 @@ module Clusters
     accepts_nested_attributes_for :platform_kubernetes, update_only: true
 
     validates :name, cluster_name: true
-    validate :unique_environment_scope, if: :has_project?
+    validate :unique_environment_scope
     validate :restrict_modification, on: :update
 
     delegate :status, to: :provider, allow_nil: true
@@ -94,7 +94,7 @@ module Clusters
     private
 
     def unique_environment_scope
-      if project.clusters.where(environment_scope: environment_scope).where.not(id: self.id).exists?
+      if project && project.clusters.where(environment_scope: environment_scope).where.not(id: self.id).exists?
         errors.add(:base, "cannot add duplicated environment scope")
         return false
       end
@@ -110,9 +110,5 @@ module Clusters
 
       true
     end
-
-    def has_project?
-      projects.exists?
-    end
   end
 end

spec/ee/spec/models/ee/project_spec.rb

@@ -709,7 +709,7 @@ describe Project do
     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, projects: [project]) }
+      let!(:cluster) { create(:cluster, :provided_by_user, environment_scope: 'review/*', projects: [project]) }
 
       subject { project.deployment_platform(environment: environment) }
 
@@ -782,8 +782,6 @@ describe Project do
       end
 
       context 'when environment scope has _' do
-        let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) }
-
         before do
           stub_licensed_features(multiple_clusters: true)
         end
@@ -807,8 +805,6 @@ describe Project do
       # it doesn't break in case some data sneaked in somehow as we're
       # not checking this integrity in database level.
       context 'when environment scope has %' do
-        let!(:cluster) { create(:cluster, :provided_by_user, projects: [project]) }
-
         before do
           stub_licensed_features(multiple_clusters: true)
         end
@@ -827,15 +823,14 @@ describe Project do
         end
       end
 
-      context 'when variables with the same name have different environment scopes' do
-        let!(:partially_matched_cluster) { create(:cluster, :provided_by_user, projects: [project], environment_scope: 'review/*') }
+      context 'when perfectly matched cluster exists' do
         let!(:perfectly_matched_cluster) { create(:cluster, :provided_by_user, projects: [project], environment_scope: 'review/name') }
 
         before do
           stub_licensed_features(multiple_clusters: true)
         end
 
-        it 'puts variables matching environment scope more in the end' do
+        it 'returns perfectly matched cluster as highest precedence' do
           is_expected.to eq(perfectly_matched_cluster.platform_kubernetes)
         end
       end

spec/factories/clusters/clusters.rb

@@ -2,9 +2,10 @@ FactoryBot.define do
   factory :cluster, class: Clusters::Cluster do
     user
     name 'test-cluster'
+    sequence(:environment_scope) { |n| "production#{n}/*" }
 
     trait :project do
-      after(:create) do |cluster, evaluator|
+      before(:create) do |cluster, evaluator|
         cluster.projects << create(:project)
       end
     end

spec/models/clusters/cluster_spec.rb

@@ -137,20 +137,20 @@ describe Clusters::Cluster do
       end
 
       context 'when identical environment scope exists in project' do
-        let(:cluster) { create(:cluster, projects: [project], environment_scope: 'product/*') }
+        let(:cluster) { build(:cluster, projects: [project], environment_scope: 'product/*') }
 
         it { is_expected.to be_falsey }
       end
 
       context 'when identical environment scope does not exist in project' do
-        let(:cluster) { create(:cluster, projects: [project], environment_scope: '*') }
+        let(:cluster) { build(:cluster, projects: [project], environment_scope: '*') }
 
         it { is_expected.to be_truthy }
       end
 
       context 'when identical environment scope exists in different project' do
         let(:project2) { create(:project) }
-        let(:cluster) { create(:cluster, projects: [project2], environment_scope: 'product/*') }
+        let(:cluster) { build(:cluster, projects: [project2], environment_scope: 'product/*') }
 
         it { is_expected.to be_truthy }
       end