Merge branch '52363-ui-changes-to-cluster-and-ado-pages' into 'master'

Moves domain setting to Clusters page

Closes #52363

See merge request gitlab-org/gitlab-ce!24580

8 files changed, 315 insertions, 112 deletions

diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb
index 0f28499194e..360030102e0 100644
--- a/spec/controllers/groups/clusters_controller_spec.rb
+++ b/spec/controllers/groups/clusters_controller_spec.rb
@@ -429,12 +429,14 @@ describe Groups::ClustersController do
     end
 
     let(:cluster) { create(:cluster, :provided_by_user, cluster_type: :group_type, groups: [group]) }
+    let(:domain) { 'test-domain.com' }
 
     let(:params) do
       {
         cluster: {
           enabled: false,
-          name: 'my-new-cluster-name'
+          name: 'my-new-cluster-name',
+          base_domain: domain
         }
       }
     end
@@ -447,6 +449,20 @@ describe Groups::ClustersController do
       expect(flash[:notice]).to eq('Kubernetes cluster was successfully updated.')
       expect(cluster.enabled).to be_falsey
       expect(cluster.name).to eq('my-new-cluster-name')
+      expect(cluster.domain).to eq('test-domain.com')
+    end
+
+    context 'when domain is invalid' do
+      let(:domain) { 'not-a-valid-domain' }
+
+      it 'should not update cluster attributes' do
+        go
+
+        cluster.reload
+        expect(response).to render_template(:show)
+        expect(cluster.name).not_to eq('my-new-cluster-name')
+        expect(cluster.domain).not_to eq('test-domain.com')
+      end
     end
 
     context 'when format is json' do
@@ -456,7 +472,8 @@ describe Groups::ClustersController do
             {
               cluster: {
                 enabled: false,
-                name: 'my-new-cluster-name'
+                name: 'my-new-cluster-name',
+                domain: domain
               }
             }
           end
diff --git a/spec/features/clusters/cluster_detail_page_spec.rb b/spec/features/clusters/cluster_detail_page_spec.rb
new file mode 100644
index 00000000000..0a9c4bcaf12
--- /dev/null
+++ b/spec/features/clusters/cluster_detail_page_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Clusterable > Show page' do
+  let(:current_user) { create(:user) }
+
+  before do
+    sign_in(current_user)
+  end
+
+  shared_examples 'editing domain' do
+    before do
+      clusterable.add_maintainer(current_user)
+    end
+
+    it 'allow the user to set domain' do
+      visit cluster_path
+
+      within '#cluster-integration' do
+        fill_in('cluster_base_domain', with: 'test.com')
+        click_on 'Save changes'
+      end
+
+      expect(page.status_code).to eq(200)
+      expect(page).to have_content('Kubernetes cluster was successfully updated.')
+    end
+
+    context 'when there is a cluster with ingress and external ip' do
+      before do
+        cluster.create_application_ingress!(external_ip: '192.168.1.100')
+
+        visit cluster_path
+      end
+
+      it 'shows help text with the domain as an alternative to custom domain' do
+        within '#cluster-integration' do
+          expect(page).to have_content('Alternatively 192.168.1.100.nip.io can be used instead of a custom domain')
+        end
+      end
+    end
+
+    context 'when there is no ingress' do
+      it 'alternative to custom domain is not shown' do
+        visit cluster_path
+
+        within '#cluster-integration' do
+          expect(page).not_to have_content('can be used instead of a custom domain.')
+        end
+      end
+    end
+  end
+
+  context 'when clusterable is a project' do
+    it_behaves_like 'editing domain' do
+      let(:clusterable) { create(:project) }
+      let(:cluster) { create(:cluster, :provided_by_gcp, :project, projects: [clusterable]) }
+      let(:cluster_path) { project_cluster_path(clusterable, cluster) }
+    end
+  end
+
+  context 'when clusterable is a group' do
+    it_behaves_like 'editing domain' do
+      let(:clusterable) { create(:group) }
+      let(:cluster) { create(:cluster, :provided_by_gcp, :group, groups: [clusterable]) }
+      let(:cluster_path) { group_cluster_path(clusterable, cluster) }
+    end
+  end
+end
diff --git a/spec/features/projects/settings/pipelines_settings_spec.rb b/spec/features/projects/settings/pipelines_settings_spec.rb
index 6f8ec0015ad..4c85abe9971 100644
--- a/spec/features/projects/settings/pipelines_settings_spec.rb
+++ b/spec/features/projects/settings/pipelines_settings_spec.rb
@@ -98,14 +98,12 @@ describe "Projects > Settings > Pipelines settings" do
             expect(page).not_to have_content('instance enabled')
             expect(find_field('project_auto_devops_attributes_enabled')).not_to be_checked
             check 'Default to Auto DevOps pipeline'
-            fill_in('project_auto_devops_attributes_domain', with: 'test.com')
             click_on 'Save changes'
           end
 
           expect(page.status_code).to eq(200)
           expect(project.auto_devops).to be_present
           expect(project.auto_devops).to be_enabled
-          expect(project.auto_devops.domain).to eq('test.com')
 
           page.within '#autodevops-settings' do
             expect(find_field('project_auto_devops_attributes_enabled')).to be_checked
@@ -113,29 +111,6 @@ describe "Projects > Settings > Pipelines settings" do
           end
         end
       end
-
-      context 'when there is a cluster with ingress and external_ip' do
-        before do
-          cluster = create(:cluster, projects: [project])
-          cluster.create_application_ingress!(external_ip: '192.168.1.100')
-        end
-
-        it 'shows the help text with the nip.io domain as an alternative to custom domain' do
-          visit project_settings_ci_cd_path(project)
-          expect(page).to have_content('192.168.1.100.nip.io can be used as an alternative to a custom domain')
-        end
-      end
-
-      context 'when there is no ingress' do
-        before do
-          create(:cluster, projects: [project])
-        end
-
-        it 'alternative to custom domain is not shown' do
-          visit project_settings_ci_cd_path(project)
-          expect(page).not_to have_content('can be used as an alternative to a custom domain')
-        end
-      end
     end
 
     describe 'runners registration token' do
diff --git a/spec/helpers/auto_devops_helper_spec.rb b/spec/helpers/auto_devops_helper_spec.rb
index 75c30dbfe48..223e562238d 100644
--- a/spec/helpers/auto_devops_helper_spec.rb
+++ b/spec/helpers/auto_devops_helper_spec.rb
@@ -90,39 +90,4 @@ describe AutoDevopsHelper do
       it { is_expected.to eq(false) }
     end
   end
-
-  describe '.auto_devops_warning_message' do
-    subject { helper.auto_devops_warning_message(project) }
-
-    context 'when the service is missing' do
-      before do
-        allow(helper).to receive(:missing_auto_devops_service?).and_return(true)
-      end
-
-      context 'when the domain is missing' do
-        before do
-          allow(helper).to receive(:missing_auto_devops_domain?).and_return(true)
-        end
-
-        it { is_expected.to match(/Auto Review Apps and Auto Deploy need a domain name and a .* to work correctly./) }
-      end
-
-      context 'when the domain is not missing' do
-        before do
-          allow(helper).to receive(:missing_auto_devops_domain?).and_return(false)
-        end
-
-        it { is_expected.to match(/Auto Review Apps and Auto Deploy need a .* to work correctly./) }
-      end
-    end
-
-    context 'when the domain is missing' do
-      before do
-        allow(helper).to receive(:missing_auto_devops_service?).and_return(false)
-        allow(helper).to receive(:missing_auto_devops_domain?).and_return(true)
-      end
-
-      it { is_expected.to eq('Auto Review Apps and Auto Deploy need a domain name to work correctly.') }
-    end
-  end
 end
diff --git a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb
new file mode 100644
index 00000000000..2ffc0e65fee
--- /dev/null
+++ b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb
@@ -0,0 +1,106 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20190204115450_migrate_auto_dev_ops_domain_to_cluster_domain.rb')
+
+describe MigrateAutoDevOpsDomainToClusterDomain, :migration do
+  include MigrationHelpers::ClusterHelpers
+
+  let(:migration) { described_class.new }
+  let(:project_auto_devops_table) { table(:project_auto_devops) }
+  let(:clusters_table) { table(:clusters) }
+  let(:cluster_projects_table) { table(:cluster_projects) }
+
+  # Following lets are needed by MigrationHelpers::ClusterHelpers
+  let(:cluster_kubernetes_namespaces_table) { table(:clusters_kubernetes_namespaces) }
+  let(:projects_table) { table(:projects) }
+  let(:namespaces_table) { table(:namespaces) }
+  let(:provider_gcp_table) { table(:cluster_providers_gcp) }
+  let(:platform_kubernetes_table) { table(:cluster_platforms_kubernetes) }
+
+  before do
+    setup_cluster_projects_with_domain(quantity: 20, domain: domain)
+  end
+
+  context 'with ProjectAutoDevOps with no domain' do
+    let(:domain) { nil }
+
+    it 'should not update cluster project' do
+      migrate!
+
+      expect(clusters_without_domain.count).to eq(clusters_table.count)
+    end
+  end
+
+  context 'with ProjectAutoDevOps with domain' do
+    let(:domain) { 'example-domain.com' }
+
+    it 'should update all cluster projects' do
+      migrate!
+
+      expect(clusters_with_domain.count).to eq(clusters_table.count)
+    end
+  end
+
+  context 'when only some ProjectAutoDevOps have domain set' do
+    let(:domain) { 'example-domain.com' }
+
+    before do
+      setup_cluster_projects_with_domain(quantity: 25, domain: nil)
+    end
+
+    it 'should only update specific cluster projects' do
+      migrate!
+
+      expect(clusters_with_domain.count).to eq(20)
+
+      project_auto_devops_with_domain.each do |project_auto_devops|
+        cluster_project = Clusters::Project.find_by(project_id: project_auto_devops.project_id)
+        cluster = Clusters::Cluster.find(cluster_project.cluster_id)
+
+        expect(cluster.domain).to be_present
+      end
+
+      expect(clusters_without_domain.count).to eq(25)
+
+      project_auto_devops_without_domain.each do |project_auto_devops|
+        cluster_project = Clusters::Project.find_by(project_id: project_auto_devops.project_id)
+        cluster = Clusters::Cluster.find(cluster_project.cluster_id)
+
+        expect(cluster.domain).not_to be_present
+      end
+    end
+  end
+
+  def setup_cluster_projects_with_domain(quantity:, domain:)
+    create_cluster_project_list(quantity)
+
+    cluster_projects = cluster_projects_table.last(quantity)
+
+    cluster_projects.each do |cluster_project|
+      specific_domain = "#{cluster_project.id}-#{domain}" if domain
+
+      project_auto_devops_table.create(
+        project_id: cluster_project.project_id,
+        enabled: true,
+        domain: specific_domain
+      )
+    end
+  end
+
+  def project_auto_devops_with_domain
+    project_auto_devops_table.where.not("domain IS NULL OR domain = ''")
+  end
+
+  def project_auto_devops_without_domain
+    project_auto_devops_table.where("domain IS NULL OR domain = ''")
+  end
+
+  def clusters_with_domain
+    clusters_table.where.not("domain IS NULL OR domain = ''")
+  end
+
+  def clusters_without_domain
+    clusters_table.where("domain IS NULL OR domain = ''")
+  end
+end
diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb
index 0161db740ee..92ce2b0999a 100644
--- a/spec/models/clusters/cluster_spec.rb
+++ b/spec/models/clusters/cluster_spec.rb
@@ -30,6 +30,7 @@ describe Clusters::Cluster do
   it { is_expected.to delegate_method(:available?).to(:application_ingress).with_prefix }
   it { is_expected.to delegate_method(:available?).to(:application_prometheus).with_prefix }
   it { is_expected.to delegate_method(:available?).to(:application_knative).with_prefix }
+  it { is_expected.to delegate_method(:external_ip).to(:application_ingress).with_prefix }
 
   it { is_expected.to respond_to :project }
 
@@ -514,4 +515,108 @@ describe Clusters::Cluster do
       it { is_expected.to be_falsey }
     end
   end
+
+  describe '#kube_ingress_domain' do
+    let(:cluster) { create(:cluster, :provided_by_gcp) }
+
+    subject { cluster.kube_ingress_domain }
+
+    context 'with domain set in cluster' do
+      let(:cluster) { create(:cluster, :provided_by_gcp, :with_domain) }
+
+      it { is_expected.to eq(cluster.domain) }
+    end
+
+    context 'with no domain on cluster' do
+      context 'with a project cluster' do
+        let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+        let(:project) { cluster.project }
+
+        context 'with domain set at instance level' do
+          before do
+            stub_application_setting(auto_devops_domain: 'global_domain.com')
+
+            it { is_expected.to eq('global_domain.com') }
+          end
+        end
+
+        context 'with domain set on ProjectAutoDevops' do
+          before do
+            auto_devops = project.build_auto_devops(domain: 'legacy-ado-domain.com')
+            auto_devops.save
+          end
+
+          it { is_expected.to eq('legacy-ado-domain.com') }
+        end
+
+        context 'with domain set as environment variable on project' do
+          before do
+            variable = project.variables.build(key: 'AUTO_DEVOPS_DOMAIN', value: 'project-ado-domain.com')
+            variable.save
+          end
+
+          it { is_expected.to eq('project-ado-domain.com') }
+        end
+
+        context 'with domain set as environment variable on the group project' do
+          let(:group) { create(:group) }
+
+          before do
+            project.update(parent_id: group.id)
+            variable = group.variables.build(key: 'AUTO_DEVOPS_DOMAIN', value: 'group-ado-domain.com')
+            variable.save
+          end
+
+          it { is_expected.to eq('group-ado-domain.com') }
+        end
+      end
+
+      context 'with a group cluster' do
+        let(:cluster) { create(:cluster, :group, :provided_by_gcp) }
+
+        context 'with domain set as environment variable for the group' do
+          let(:group) { cluster.group }
+
+          before do
+            variable = group.variables.build(key: 'AUTO_DEVOPS_DOMAIN', value: 'group-ado-domain.com')
+            variable.save
+          end
+
+          it { is_expected.to eq('group-ado-domain.com') }
+        end
+      end
+    end
+  end
+
+  describe '#predefined_variables' do
+    subject { cluster.predefined_variables }
+
+    context 'with an instance domain' do
+      let(:cluster) { create(:cluster, :provided_by_gcp) }
+
+      before do
+        stub_application_setting(auto_devops_domain: 'global_domain.com')
+      end
+
+      it 'should include KUBE_INGRESS_BASE_DOMAIN' do
+        expect(subject.to_hash).to include(KUBE_INGRESS_BASE_DOMAIN: 'global_domain.com')
+      end
+    end
+
+    context 'with a cluster domain' do
+      let(:cluster) { create(:cluster, :provided_by_gcp, domain: 'example.com') }
+
+      it 'should include KUBE_INGRESS_BASE_DOMAIN' do
+        expect(subject.to_hash).to include(KUBE_INGRESS_BASE_DOMAIN: 'example.com')
+      end
+    end
+
+    context 'with no domain' do
+      let(:cluster) { create(:cluster, :provided_by_gcp, :project) }
+
+      it 'should return an empty array' do
+        expect(subject.to_hash).to be_empty
+      end
+    end
+  end
 end
diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb
index 6c8a223092e..c273fa7e164 100644
--- a/spec/models/clusters/platforms/kubernetes_spec.rb
+++ b/spec/models/clusters/platforms/kubernetes_spec.rb
@@ -297,6 +297,19 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
         end
       end
     end
+
+    context 'with a domain' do
+      let!(:cluster) do
+        create(:cluster, :provided_by_gcp, :with_domain,
+               platform_kubernetes: kubernetes)
+      end
+
+      it 'sets KUBE_INGRESS_BASE_DOMAIN' do
+        expect(subject).to include(
+          { key: 'KUBE_INGRESS_BASE_DOMAIN', value: cluster.domain, public: true }
+        )
+      end
+    end
   end
 
   describe '#terminals' do
diff --git a/spec/views/projects/settings/ci_cd/_autodevops_form.html.haml_spec.rb b/spec/views/projects/settings/ci_cd/_autodevops_form.html.haml_spec.rb
index cb1b9e6f5fb..2a2539c80b5 100644
--- a/spec/views/projects/settings/ci_cd/_autodevops_form.html.haml_spec.rb
+++ b/spec/views/projects/settings/ci_cd/_autodevops_form.html.haml_spec.rb
@@ -7,56 +7,9 @@ describe 'projects/settings/ci_cd/_autodevops_form' do
     assign :project, project
   end
 
-  context 'when kubernetes is not active' do
-    context 'when auto devops domain is not defined' do
-      it 'shows warning message' do
-        render
+  it 'shows a warning message about Kubernetes cluster' do
+    render
 
-        expect(rendered).to have_css('.auto-devops-warning-message')
-        expect(rendered).to have_text('Auto Review Apps and Auto Deploy need a domain name and a')
-        expect(rendered).to have_link('Kubernetes cluster')
-      end
-    end
-
-    context 'when auto devops domain is defined' do
-      before do
-        project.build_auto_devops(domain: 'example.com')
-      end
-
-      it 'shows warning message' do
-        render
-
-        expect(rendered).to have_css('.auto-devops-warning-message')
-        expect(rendered).to have_text('Auto Review Apps and Auto Deploy need a')
-        expect(rendered).to have_link('Kubernetes cluster')
-      end
-    end
-  end
-
-  context 'when kubernetes is active' do
-    before do
-      create(:kubernetes_service, project: project)
-    end
-
-    context 'when auto devops domain is not defined' do
-      it 'shows warning message' do
-        render
-
-        expect(rendered).to have_css('.auto-devops-warning-message')
-        expect(rendered).to have_text('Auto Review Apps and Auto Deploy need a domain name to work correctly.')
-      end
-    end
-
-    context 'when auto devops domain is defined' do
-      before do
-        project.build_auto_devops(domain: 'example.com')
-      end
-
-      it 'does not show warning message' do
-        render
-
-        expect(rendered).not_to have_css('.auto-devops-warning-message')
-      end
-    end
+    expect(rendered).to have_text('You must add a Kubernetes cluster integration to this project with a domain in order for your deployment strategy to work correctly.')
   end
 end