Merge branch '3734-support-multiple-kubernetes-cluster-per-project-final' into 'master'

(EEP) Support multiple Kubernetes cluster per project

Closes #3734

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

app/models/ci/build.rb

@@ -234,7 +234,7 @@ module Ci
       variables += pipeline.predefined_variables
       variables += runner.predefined_variables if runner
       variables += project.container_registry_variables
-      variables += project.deployment_variables if has_environment?
+      variables += project.deployment_variables(environment: environment) if has_environment?
       variables += project.auto_devops_variables
       variables += yaml_variables
       variables += user_variables

app/models/ci/variable.rb

@@ -3,7 +3,7 @@ module Ci
     extend Gitlab::Ci::Model
     include HasVariable
     include Presentable
-    prepend EE::Ci::Variable
+    prepend HasEnvironmentScope
 
     belongs_to :project
 

app/models/clusters/cluster.rb

 module Clusters
   class Cluster < ActiveRecord::Base
     include Presentable
+    prepend HasEnvironmentScope
 
     self.table_name = 'clusters'
 
@@ -26,6 +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 :restrict_modification, on: :update
 
     delegate :status, to: :provider, allow_nil: true
@@ -91,6 +93,15 @@ module Clusters
 
     private
 
+    def unique_environment_scope
+      if project.clusters.where(environment_scope: environment_scope).where.not(id: self.id).exists?
+        errors.add(:base, "cannot add duplicated environment scope")
+        return false
+      end
+
+      true
+    end
+
     def restrict_modification
       if provider&.on_creation?
         errors.add(:base, "cannot modify during creation")
@@ -99,5 +110,9 @@ module Clusters
 
       true
     end
+
+    def has_project?
+      projects.exists?
+    end
   end
 end

app/models/environment.rb

@@ -137,16 +137,20 @@ class Environment < ActiveRecord::Base
     end
   end
 
+  def deployment_platform
+    project.deployment_platform(environment: self)
+  end
+
   def has_terminals?
-    project.deployment_platform.present? && available? && last_deployment.present?
+    deployment_platform.present? && available? && last_deployment.present?
   end
 
   def terminals
-    project.deployment_platform.terminals(self) if has_terminals?
+    deployment_platform.terminals(self) if has_terminals?
   end
 
   def rollout_status
-    project.deployment_platform.rollout_status(self) if has_terminals?
+    deployment_platform.rollout_status(self) if has_terminals?
   end
 
   def has_metrics?

app/models/project.rb

@@ -905,8 +905,7 @@ class Project < ActiveRecord::Base
     @ci_service ||= ci_services.reorder(nil).find_by(active: true)
   end
 
-  # TODO: This will be extended for multiple enviroment clusters
-  def deployment_platform
+  def deployment_platform(environment: nil)
     @deployment_platform ||= clusters.find_by(enabled: true)&.platform_kubernetes
     @deployment_platform ||= services.where(category: :deployment).reorder(nil).find_by(active: true)
   end
@@ -1557,10 +1556,8 @@ class Project < ActiveRecord::Base
       ProtectedTag.protected?(self, ref)
   end
 
-  def deployment_variables
-    return [] unless deployment_platform
-
-    deployment_platform.predefined_variables
+  def deployment_variables(environment: nil)
+    deployment_platform(environment: environment)&.predefined_variables || []
   end
 
   def auto_devops_variables

app/views/projects/clusters/_tabs.html.haml

@@ -15,4 +15,7 @@
         = s_("ClusterIntegration|All")
         %span.badge= @all_count
   .nav-controls
-    = link_to s_("ClusterIntegration|Add cluster"), new_project_cluster_path(@project), class: "btn btn-success btn-add-cluster disabled has-tooltip js-add-cluster", title: s_("ClusterIntegration|Multiple clusters are available in GitLab Entreprise Edition Premium and Ultimate")
+    - if @project.feature_available?(:multiple_clusters)
+      = link_to s_("ClusterIntegration|Add cluster"), new_project_cluster_path(@project), class: "btn btn-success btn-add-cluster has-tooltip js-add-cluster"
+    - else
+      = link_to s_("ClusterIntegration|Add cluster"), new_project_cluster_path(@project), class: "btn btn-success btn-add-cluster disabled has-tooltip js-add-cluster", title: s_("ClusterIntegration|Multiple clusters are available in GitLab Entreprise Edition Premium and Ultimate")

changelogs/unreleased-ee/3734-support-multiple-kubernetes-cluster-per-project-final.yml

+---
+title: Support multiple Kubernetes cluster per project
+merge_request: 3603
+author:
+type: added

ee/app/models/concerns/has_environment_scope.rb

+module HasEnvironmentScope
+  extend ActiveSupport::Concern
+
+  prepended do
+    validates(
+      :environment_scope,
+      presence: true,
+      format: { with: ::Gitlab::Regex.environment_scope_regex,
+                message: ::Gitlab::Regex.environment_scope_regex_message }
+    )
+
+    scope :on_environment, -> (environment_name) do
+      where = <<~SQL
+        environment_scope IN (:wildcard, :environment_name) OR
+          :environment_name LIKE
+            #{::Gitlab::SQL::Glob.to_like('environment_scope')}
+      SQL
+
+      order = <<~SQL
+        CASE environment_scope
+          WHEN %{wildcard} THEN 0
+          WHEN %{environment_name} THEN 2
+          ELSE 1
+        END
+      SQL
+
+      values = {
+        wildcard: '*',
+        environment_name: environment_name
+      }
+
+      quoted_values = values.transform_values do |value|
+        # Note that the connection could be
+        # Gitlab::Database::LoadBalancing::ConnectionProxy
+        # which supports `quote` via `method_missing`
+        ActiveRecord::Base.connection.quote(value)
+      end
+
+      # The query is trying to find variables with scopes matching the
+      # current environment name. Suppose the environment name is
+      # 'review/app', and we have variables with environment scopes like:
+      # * variable A: review
+      # * variable B: review/app
+      # * variable C: review/*
+      # * variable D: *
+      # And the query should find variable B, C, and D, because it would
+      # try to convert the scope into a LIKE pattern for each variable:
+      # * A: review
+      # * B: review/app
+      # * C: review/%
+      # * D: %
+      # Note that we'll match % and _ literally therefore we'll escape them.
+      # In this case, B, C, and D would match. We also want to prioritize
+      # the exact matched name, and put * last, and everything else in the
+      # middle. So the order should be: D < C < B
+      where(where, values)
+        .order(order % quoted_values) # `order` cannot escape for us!
+    end
+  end
+
+  def environment_scope=(new_environment_scope)
+    super(new_environment_scope.to_s.strip)
+  end
+end

ee/app/models/ee/ci/variable.rb

-module EE
-  module Ci
-    module Variable
-      extend ActiveSupport::Concern
-
-      prepended do
-        validates(
-          :environment_scope,
-          presence: true,
-          format: { with: ::Gitlab::Regex.environment_scope_regex,
-                    message: ::Gitlab::Regex.environment_scope_regex_message }
-        )
-      end
-
-      def environment_scope=(new_environment_scope)
-        super(new_environment_scope.to_s.strip)
-      end
-    end
-  end
-end

ee/app/models/ee/project.rb

@@ -255,58 +255,20 @@ module EE
       end
     end
 
+    def deployment_platform(environment: nil)
+      return super unless environment && feature_available?(:multiple_clusters)
+
+      @deployment_platform ||= clusters.enabled.on_environment(environment.name)
+                                               .last&.platform_kubernetes
+
+      super # Wildcard or KubernetesService
+    end
+
     def secret_variables_for(ref:, environment: nil)
       return super.where(environment_scope: '*') unless
         environment && feature_available?(:variable_environment_scope)
 
-      query = super
-
-      where = <<~SQL
-        environment_scope IN (:wildcard, :environment_name) OR
-          :environment_name LIKE
-            #{::Gitlab::SQL::Glob.to_like('environment_scope')}
-      SQL
-
-      order = <<~SQL
-        CASE environment_scope
-          WHEN %{wildcard} THEN 0
-          WHEN %{environment_name} THEN 2
-          ELSE 1
-        END
-      SQL
-
-      values = {
-        wildcard: '*',
-        environment_name: environment.name
-      }
-
-      quoted_values = values.transform_values do |value|
-        # Note that the connection could be
-        # Gitlab::Database::LoadBalancing::ConnectionProxy
-        # which supports `quote` via `method_missing`
-        self.class.connection.quote(value)
-      end
-
-      # The query is trying to find variables with scopes matching the
-      # current environment name. Suppose the environment name is
-      # 'review/app', and we have variables with environment scopes like:
-      # * variable A: review
-      # * variable B: review/app
-      # * variable C: review/*
-      # * variable D: *
-      # And the query should find variable B, C, and D, because it would
-      # try to convert the scope into a LIKE pattern for each variable:
-      # * A: review
-      # * B: review/app
-      # * C: review/%
-      # * D: %
-      # Note that we'll match % and _ literally therefore we'll escape them.
-      # In this case, B, C, and D would match. We also want to prioritize
-      # the exact matched name, and put * last, and everything else in the
-      # middle. So the order should be: D < C < B
-      query
-        .where(where, values)
-        .order(order % quoted_values) # `order` cannot escape for us!
+      super.on_environment(environment.name)
     end
 
     def execute_hooks(data, hooks_scope = :push_hooks)

lib/gitlab/prometheus/queries/query_additional_metrics.rb

@@ -76,7 +76,7 @@ module Gitlab
             timeframe_start: timeframe_start,
             timeframe_end: timeframe_end,
             ci_environment_slug: environment.slug,
-            kube_namespace: environment.project.deployment_platform&.actual_namespace || '',
+            kube_namespace: environment.project.deployment_platform(environment: environment)&.actual_namespace || '',
             environment_filter: %{container_name!="POD",environment="#{environment.slug}"}
           }
         end

spec/ee/spec/models/concerns/has_environment_scope_spec.rb

+require 'spec_helper'
+
+describe HasEnvironmentScope do
+  subject { build(:ci_variable) }
+
+  it { is_expected.to allow_value('*').for(:environment_scope) }
+  it { is_expected.to allow_value('review/*').for(:environment_scope) }
+  it { is_expected.not_to allow_value('').for(:environment_scope) }
+  it { is_expected.not_to allow_value('<>').for(:environment_scope) }
+
+  it do
+    is_expected.to validate_uniqueness_of(:key)
+      .scoped_to(:project_id, :environment_scope)
+  end
+
+  describe '.on_environment' do
+    let(:project) { create(:project) }
+    let!(:cluster1) { create(:cluster, projects: [project], environment_scope: '*') }
+    let!(:cluster2) { create(:cluster, projects: [project], environment_scope: 'product/*') }
+    let!(:cluster3) { create(:cluster, projects: [project], environment_scope: 'staging/*') }
+    let(:environment_name) { 'product/*' }
+
+    it 'returns scoped objects' do
+      expect(project.clusters.on_environment(environment_name)).to eq([cluster1, cluster2])
+    end
+  end
+
+  describe '#environment_scope=' do
+    context 'when the new environment_scope is nil' do
+      it 'strips leading and trailing whitespaces' do
+        subject.environment_scope = nil
+
+        expect(subject.environment_scope).to eq('')
+      end
+    end
+
+    context 'when the new environment_scope has leadind and trailing whitespaces' do
+      it 'strips leading and trailing whitespaces' do
+        subject.environment_scope = ' * '
+
+        expect(subject.environment_scope).to eq('*')
+      end
+    end
+  end
+end

spec/ee/spec/models/ee/ci/variable_spec.rb

@@ -3,30 +3,8 @@ require 'spec_helper'
 describe Ci::Variable do
   subject { build(:ci_variable) }
 
-  it { is_expected.to allow_value('*').for(:environment_scope) }
-  it { is_expected.to allow_value('review/*').for(:environment_scope) }
-  it { is_expected.not_to allow_value('').for(:environment_scope) }
-
   it do
     is_expected.to validate_uniqueness_of(:key)
       .scoped_to(:project_id, :environment_scope)
   end
-
-  describe '#environment_scope=' do
-    context 'when the new environment_scope is nil' do
-      it 'strips leading and trailing whitespaces' do
-        subject.environment_scope = nil
-
-        expect(subject.environment_scope).to eq('')
-      end
-    end
-
-    context 'when the new environment_scope has leadind and trailing whitespaces' do
-      it 'strips leading and trailing whitespaces' do
-        subject.environment_scope = ' * '
-
-        expect(subject.environment_scope).to eq('*')
-      end
-    end
-  end
 end

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

@@ -703,6 +703,145 @@ describe Project do
     end
   end
 
+  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, 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
+
+          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
+
+          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
+
+          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
+
+          it 'returns a kubernetes platform' do
+            is_expected.to be_kind_of(Clusters::Platforms::Kubernetes)
+          end
+        end
+      end
+
+      context 'when environment scope is exactly matched' do
+        before do
+          cluster.update(environment_scope: 'review/name')
+        end
+
+        it_behaves_like 'matching environment scope'
+      end
+
+      context 'when environment scope is matched by wildcard' do
+        before do
+          cluster.update(environment_scope: 'review/*')
+        end
+
+        it_behaves_like 'matching environment scope'
+      end
+
+      context 'when environment scope does not match' do
+        before do
+          cluster.update(environment_scope: 'review/*/special')
+        end
+
+        it_behaves_like 'not matching environment scope'
+      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
+
+        it 'does not treat it as wildcard' do
+          cluster.update(environment_scope: 'foo_bar/*')
+
+          is_expected.to eq(default_cluster.platform_kubernetes)
+        end
+
+        it 'matches literally for _' do
+          cluster.update(environment_scope: 'foo_bar/*')
+          environment.update(name: 'foo_bar/test')
+
+          is_expected.to eq(cluster.platform_kubernetes)
+        end
+      end
+
+      # The environment name and scope cannot have % at the moment,
+      # but we're considering relaxing it and we should also make sure
+      # 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
+
+        it 'does not treat it as wildcard' do
+          cluster.update_attribute(:environment_scope, '*%*')
+
+          is_expected.to eq(default_cluster.platform_kubernetes)
+        end
+
+        it 'matches literally for _' do
+          cluster.update(environment_scope: 'foo%bar/*')
+          environment.update_attribute(:name, 'foo%bar/test')
+
+          is_expected.to eq(cluster.platform_kubernetes)
+        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/*') }
+        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
+          is_expected.to eq(perfectly_matched_cluster.platform_kubernetes)
+        end
+      end
+    end
+  end
+
   describe '#secret_variables_for' do
     let(:project) { create(:project) }
 

spec/features/projects/clusters_spec.rb

@@ -35,8 +35,24 @@ feature 'Clusters', :js do
       expect(page).to have_selector('.gl-responsive-table-row', count: 2)
     end
 
-    it 'user sees a disabled add cluster button ' do
-      expect(page).to have_selector('.js-add-cluster.disabled')
+    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)
+      end
+
+      it 'user sees a add cluster button ' do
+        expect(page).to have_selector('.js-add-cluster')
+      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)
+      end
+
+      it 'user sees a disabled add cluster button ' do
+        expect(page).to have_selector('.js-add-cluster.disabled')
+      end
     end
 
     it 'user sees navigation tabs' do

spec/models/ci/variable_spec.rb

@@ -9,6 +9,7 @@ describe Ci::Variable do
       stub_licensed_features(variable_environment_scope: true)
     end
 
+    it { is_expected.to include_module(HasEnvironmentScope) }
     it { is_expected.to include_module(HasVariable) }
     it { is_expected.to include_module(Presentable) }
     it { is_expected.to validate_uniqueness_of(:key).scoped_to(:project_id, :environment_scope) }

spec/models/clusters/cluster_spec.rb

@@ -10,6 +10,7 @@ describe Clusters::Cluster do
   it { is_expected.to delegate_method(:status_name).to(:provider) }
   it { is_expected.to delegate_method(:on_creation?).to(:provider) }
   it { is_expected.to respond_to :project }
+  it { is_expected.to include_module(HasEnvironmentScope) }
 
   describe '.enabled' do
     subject { described_class.enabled }
@@ -127,6 +128,33 @@ describe Clusters::Cluster do
         it { expect(cluster.update(enabled: false)).to be_truthy }
       end
     end
+
+    context 'when validates unique_environment_scope' do
+      let(:project) { create(:project) }
+
+      before do
+        create(:cluster, projects: [project], environment_scope: 'product/*')
+      end
+
+      context 'when identical environment scope exists in project' do
+        let(:cluster) { create(: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: '*') }
+
+        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/*') }
+
+        it { is_expected.to be_truthy }
+      end
+    end
   end
 
   describe '#provider' do

spec/models/project_spec.rb

@@ -2384,6 +2384,55 @@ describe Project do
 
         it_behaves_like 'same behavior between KubernetesService and Platform::Kubernetes'
       end
+
+      context 'when multiple clusters (EEP) is enabled' do
+        before do
+          stub_licensed_features(multiple_clusters: true)
+        end
+
+        let(:project) { create(:project) }
+
+        let!(:default_cluster) do
+          create(:cluster,
+                 platform_type: :kubernetes,
+                 projects: [project],
+                 environment_scope: '*',
+                 platform_kubernetes: default_cluster_kubernetes)
+        end
+
+        let!(:review_env_cluster) do
+          create(:cluster,
+                 platform_type: :kubernetes,
+                 projects: [project],
+                 environment_scope: 'review/*',
+                 platform_kubernetes: review_env_cluster_kubernetes)
+        end
+
+        let(:default_cluster_kubernetes) { create(:cluster_platform_kubernetes, token: 'default-AAA') }
+        let(:review_env_cluster_kubernetes) { create(:cluster_platform_kubernetes, token: 'review-AAA') }
+
+        subject { project.deployment_variables(environment: environment) }
+
+        context 'when environment name is review/name' do
+          let!(:environment) { create(:environment, project: project, name: 'review/name') }
+
+          it 'returns variables from this service' do
+            expect(subject).to include(
+              { key: 'KUBE_TOKEN', value: 'review-AAA', public: false }
+            )
+          end
+        end
+
+        context 'when environment name is other' do
+          let!(:environment) { create(:environment, project: project, name: 'staging/name') }
+
+          it 'returns variables from this service' do
+            expect(subject).to include(
+              { key: 'KUBE_TOKEN', value: 'default-AAA', public: false }
+            )
+          end
+        end
+      end
     end
   end