Merge branch 'ee-fix-cluster-enviroment-missing' into 'master'

Ee fix cluster enviroment missing

Closes #4361

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

app/controllers/projects/clusters/gcp_controller.rb

@@ -39,6 +39,7 @@ class Projects::Clusters::GcpController < Projects::ApplicationController
     params.require(:cluster).permit(
       :enabled,
       :name,
+      :environment_scope,
       provider_gcp_attributes: [
         :gcp_project_id,
         :zone,

app/controllers/projects/clusters/user_controller.rb

@@ -26,6 +26,7 @@ class Projects::Clusters::UserController < Projects::ApplicationController
     params.require(:cluster).permit(
       :enabled,
       :name,
+      :environment_scope,
       platform_kubernetes_attributes: [
         :namespace,
         :api_url,

app/controllers/projects/clusters_controller.rb

@@ -70,23 +70,11 @@ class Projects::ClustersController < Projects::ApplicationController
                                  .present(current_user: current_user)
   end
 
-  def create_params
-    params.require(:cluster).permit(
-      :enabled,
-      :name,
-      :provider_type,
-      provider_gcp_attributes: [
-        :gcp_project_id,
-        :zone,
-        :num_nodes,
-        :machine_type
-      ])
-  end
-
   def update_params
     if cluster.managed?
       params.require(:cluster).permit(
         :enabled,
+        :environment_scope,
         platform_kubernetes_attributes: [
           :namespace
         ]
@@ -95,6 +83,7 @@ class Projects::ClustersController < Projects::ApplicationController
       params.require(:cluster).permit(
         :enabled,
         :name,
+        :environment_scope,
         platform_kubernetes_attributes: [
           :api_url,
           :token,

app/helpers/clusters_helper.rb

+module ClustersHelper
+  def has_multiple_clusters?(project)
+    project.feature_available?(:multiple_clusters)
+  end
+end

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

app/views/projects/clusters/gcp/_form.html.haml

@@ -8,6 +8,10 @@
     = field.label :name, s_('ClusterIntegration|Cluster name')
     = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name')
 
+  .form-group
+    = field.label :environment_scope, s_('ClusterIntegration|Environment scope')
+    = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope')
+
   = field.fields_for :provider_gcp, @cluster.provider_gcp do |provider_gcp_field|
     .form-group
       = provider_gcp_field.label :gcp_project_id, s_('ClusterIntegration|Google Cloud Platform project ID')

app/views/projects/clusters/gcp/_show.html.haml

@@ -8,6 +8,11 @@
 
 = form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field|
   = form_errors(@cluster)
+
+  .form-group
+    = field.label :environment_scope, s_('ClusterIntegration|Environment scope')
+    = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope')
+
   = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field|
     .form-group
       = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL')

app/views/projects/clusters/user/_form.html.haml

@@ -4,6 +4,10 @@
     = field.label :name, s_('ClusterIntegration|Cluster name')
     = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name')
 
+  .form-group
+    = field.label :environment_scope, s_('ClusterIntegration|Environment scope')
+    = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope')
+
   = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field|
     .form-group
       = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL')

app/views/projects/clusters/user/_show.html.haml

@@ -4,6 +4,10 @@
     = field.label :name, s_('ClusterIntegration|Cluster name')
     = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name')
 
+  .form-group
+    = field.label :environment_scope, s_('ClusterIntegration|Environment scope')
+    = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope')
+
   = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field|
     .form-group
       = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL')

spec/ee/spec/features/projects/clusters_spec.rb

 require 'spec_helper'
 
-feature 'Clusters', :js do
-  include GoogleApi::CloudPlatformHelpers
-
+feature 'EE Clusters' do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
@@ -11,31 +9,150 @@ feature 'Clusters', :js do
     gitlab_sign_in(user)
   end
 
-  context 'when user has a cluster and visits cluster index page' do
-    let!(:cluster) { create(:cluster, :project, :provided_by_gcp) }
-    let(:project) { cluster.project }
-
-    before do
-      visit project_clusters_path(project)
-    end
-
+  context 'when user has a cluster' do
     context 'when license has multiple clusters feature' do
       before do
-        allow_any_instance_of(EE::Project).to receive(:feature_available?).with(:multiple_clusters).and_return(true)
+        allow(License).to receive(:feature_available?).and_call_original
+        allow(License).to receive(:feature_available?).with(:multiple_clusters).and_return(true)
+      end
+
+      context 'when user adds an existing cluster' do
+        before do
+          create(:cluster, :provided_by_user, name: 'default-cluster', environment_scope: '*', projects: [project])
+          visit project_clusters_path(project)
+        end
+
+        it 'user sees a add cluster button ' do
+          expect(page).not_to have_selector('.js-add-cluster.readonly')
+          expect(page).to have_selector('.js-add-cluster')
+        end
+
+        context 'when user filled form with environment scope' do
+          before do
+            click_link 'Add cluster'
+            click_link 'Add an existing cluster'
+            fill_in 'cluster_name', with: 'staging-cluster'
+            fill_in 'cluster_environment_scope', with: 'staging/*'
+            click_button 'Add cluster'
+          end
+
+          it 'user sees a cluster details page' do
+            expect(page.find_field('cluster[name]').value).to eq('staging-cluster')
+            expect(page.find_field('cluster[environment_scope]').value).to eq('staging/*')
+          end
+        end
+
+        context 'when user updates environment scope' do
+          before do
+            click_link 'default-cluster'
+            fill_in 'cluster_environment_scope', with: 'production/*'
+            click_button 'Save changes'
+          end
+
+          it 'user sees a cluster details page' do
+            expect(page.find_field('cluster[environment_scope]').value).to eq('production/*')
+          end
+        end
+
+        context 'when user updates duplicated environment scope' do
+          before do
+            click_link 'Add cluster'
+            click_link 'Add an existing cluster'
+            fill_in 'cluster_name', with: 'staging-cluster'
+            fill_in 'cluster_environment_scope', with: '*'
+            click_button 'Add cluster'
+          end
+
+          it 'users sees an environment scope validation error' do
+            expect(page).to have_content('cannot add duplicated environment scope')
+          end
+        end
       end
 
-      it 'user sees a add cluster button ' do
-        expect(page).to have_selector('.js-add-cluster')
+      context 'when user adds an GKE cluster' do
+        before do
+          allow_any_instance_of(Projects::Clusters::GcpController)
+            .to receive(:token_in_session).and_return('token')
+          allow_any_instance_of(Projects::Clusters::GcpController)
+            .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s)
+          allow_any_instance_of(GoogleApi::CloudPlatform::Client)
+            .to receive(:projects_zones_clusters_create) do
+            OpenStruct.new(
+              self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123',
+              status: 'RUNNING'
+            )
+          end
+          allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil)
+
+          create(:cluster, :provided_by_gcp, name: 'default-cluster', environment_scope: '*', projects: [project])
+          visit project_clusters_path(project)
+        end
+
+        it 'user sees a add cluster button ' do
+          expect(page).not_to have_selector('.js-add-cluster.readonly')
+          expect(page).to have_selector('.js-add-cluster')
+        end
+
+        context 'when user filled form with environment scope' do
+          before do
+            click_link 'Add cluster'
+            click_link 'Create on GKE'
+            fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123'
+            fill_in 'cluster_name', with: 'staging-cluster'
+            fill_in 'cluster_environment_scope', with: 'staging/*'
+            click_button 'Create cluster'
+          end
+
+          it 'user sees a cluster details page' do
+            expect(page).to have_content('Enable cluster integration')
+            expect(page.find_field('cluster[environment_scope]').value).to eq('staging/*')
+          end
+        end
+
+        context 'when user updates environment scope' do
+          before do
+            click_link 'default-cluster'
+            fill_in 'cluster_environment_scope', with: 'production/*'
+            click_button 'Save changes'
+          end
+
+          it 'user sees a cluster details page' do
+            expect(page.find_field('cluster[environment_scope]').value).to eq('production/*')
+          end
+        end
+
+        context 'when user updates duplicated environment scope' do
+          before do
+            click_link 'Add cluster'
+            click_link 'Create on GKE'
+            fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123'
+            fill_in 'cluster_name', with: 'staging-cluster'
+            fill_in 'cluster_environment_scope', with: '*'
+            click_button 'Create cluster'
+          end
+
+          it 'users sees an environment scope validation error' do
+            expect(page).to have_content('cannot add duplicated environment scope')
+          end
+        end
       end
     end
 
     context 'when license does not have multiple clusters feature' do
       before do
-        allow_any_instance_of(EE::Project).to receive(:feature_available?).with(:multiple_clusters).and_return(false)
+        allow(License).to receive(:feature_available?).and_call_original
+        allow(License).to receive(:feature_available?).with(:multiple_clusters).and_return(false)
+        create(:cluster, :provided_by_user, name: 'default-cluster', environment_scope: '*', projects: [project])
       end
 
-      it 'user sees a disabled add cluster button ' do
-        expect(page).to have_selector('.js-add-cluster.disabled')
+      context 'when user visits cluster index page' do
+        before do
+          visit project_clusters_path(project)
+        end
+
+        it 'user sees a disabled add cluster button ' do
+          expect(page).to have_selector('.js-add-cluster.disabled')
+        end
       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