Remove some duplication from new cluster form

Also rename 'eks' to 'aws' to keep consistency with
the backend.

app/controllers/clusters/clusters_controller.rb

@@ -43,14 +43,11 @@ class Clusters::ClustersController < Clusters::BaseController
   def new
     return unless Feature.enabled?(:create_eks_clusters)
 
-    if params[:provider] == 'eks'
-      @eks_selected = true
+    if params[:provider] == 'aws'
       @aws_role = current_user.aws_role || Aws::Role.new
       @aws_role.ensure_role_external_id!
 
-    elsif params[:provider] == 'gke'
-      @gke_selected = true
-
+    elsif params[:provider] == 'gcp'
       redirect_to @authorize_url if @authorize_url && !@valid_gcp_token
     end
   end

app/helpers/clusters_helper.rb

@@ -6,6 +6,28 @@ module ClustersHelper
     false
   end
 
+  def create_new_cluster_label(provider: nil)
+    case provider
+    when 'aws'
+      s_('ClusterIntegration|Create new Cluster on EKS')
+    when 'gcp'
+      s_('ClusterIntegration|Create new Cluster on GKE')
+    else
+      s_('ClusterIntegration|Create new Cluster')
+    end
+  end
+
+  def new_cluster_partial(provider: nil)
+    case provider
+    when 'aws'
+      'clusters/clusters/aws/new'
+    when 'gcp'
+      'clusters/clusters/gcp/new'
+    else
+      'clusters/clusters/cloud_providers/cloud_provider_selector'
+    end
+  end
+
   def render_gcp_signup_offer
     return if Gitlab::CurrentSettings.current_application_settings.hide_third_party_offers?
     return unless show_gcp_signup_offer?

app/models/application_setting.rb

@@ -280,15 +280,15 @@ class ApplicationSetting < ApplicationRecord
   validates :eks_account_id,
             format: { with: Gitlab::Regex.aws_account_id_regex,
                       message: Gitlab::Regex.aws_account_id_message },
-            if: -> (setting) { setting.eks_integration_enabled? }
+            if: :eks_integration_enabled?
 
   validates :eks_access_key_id,
             length: { in: 16..128 },
-            if: -> (setting) { setting.eks_integration_enabled? }
+            if: :eks_integration_enabled?
 
   validates :eks_secret_access_key,
             presence: true,
-            if: -> (setting) { setting.eks_integration_enabled? }
+            if: :eks_integration_enabled?
 
   validates_with X509CertificateCredentialsValidator,
                  certificate: :external_auth_client_cert,

app/models/aws/role.rb

@@ -14,7 +14,7 @@ module Aws
         message: Gitlab::Regex.aws_arn_regex_message
       }
 
-    before_validation :ensure_role_external_id!, if: :new_record?
+    before_validation :ensure_role_external_id!, on: :create
 
     def ensure_role_external_id!
       self.role_external_id ||= SecureRandom.hex(20)

app/views/clusters/clusters/cloud_providers/_cloud_provider_selector.html.haml

@@ -6,6 +6,6 @@
     = create_cluster_label
   .d-flex
     = render partial: 'clusters/clusters/cloud_providers/cloud_provider_button',
-      locals: { provider: 'eks', label: eks_label, logo_path: 'illustrations/logos/amazon_eks.svg' }
+      locals: { provider: 'aws', label: eks_label, logo_path: 'illustrations/logos/amazon_eks.svg' }
     = render partial: 'clusters/clusters/cloud_providers/cloud_provider_button',
-      locals: { provider: 'gke', label: gke_label, logo_path: 'illustrations/logos/google_gke.svg', last: true }
+      locals: { provider: 'gcp', label: gke_label, logo_path: 'illustrations/logos/google_gke.svg', last: true }

app/views/clusters/clusters/gcp/_new.html.haml

+= render 'clusters/clusters/gcp/header'
+- if @valid_gcp_token
+  = render 'clusters/clusters/gcp/form'
+- elsif @authorize_url
+  = render 'clusters/clusters/gcp/signin_with_google_button'
+- else
+  = render 'clusters/clusters/gcp/gcp_not_configured'

app/views/clusters/clusters/new.html.haml

@@ -2,9 +2,6 @@
 - page_title _('Kubernetes Cluster')
 - create_eks_enabled = Feature.enabled?(:create_eks_clusters)
 - active_tab = local_assigns.fetch(:active_tab, 'create')
-- create_on_gke_tab_label = s_('ClusterIntegration|Create new Cluster on GKE')
-- create_on_eks_tab_label = s_('ClusterIntegration|Create new Cluster on EKS')
-- create_new_cluster_label = s_('ClusterIntegration|Create new Cluster')
 = javascript_include_tag 'https://apis.google.com/js/api.js'
 
 = render_gcp_signup_offer
@@ -18,14 +15,9 @@
         %a.nav-link{ href: '#create-cluster-pane', id: 'create-cluster-tab', class: active_when(active_tab == 'create'), data: { toggle: 'tab' }, role: 'tab' }
           %span
             - if create_eks_enabled
-              - if @gke_selected
-                = create_on_gke_tab_label
-              - elsif @eks_selected
-                = create_on_eks_tab_label
-              - else
-                = create_new_cluster_label
+              = create_new_cluster_label(provider: params[:provider])
             - else
-              = create_on_gke_tab_label
+              = create_new_cluster_label(provider: 'gcp')
       %li.nav-item{ role: 'presentation' }
         %a.nav-link{ href: '#add-cluster-pane', id: 'add-cluster-tab', class: active_when(active_tab == 'add'), data: { toggle: 'tab' }, role: 'tab' }
           %span Add existing cluster
@@ -33,27 +25,10 @@
     .tab-content.gitlab-tab-content
       - if create_eks_enabled
         .tab-pane{ id: 'create-cluster-pane', class: active_when(active_tab == 'create'), role: 'tabpanel' }
-          - if @gke_selected
-            = render 'clusters/clusters/gcp/header'
-            - if @valid_gcp_token
-              = render 'clusters/clusters/gcp/form'
-            - elsif @authorize_url
-              = render 'clusters/clusters/gcp/signin_with_google_button'
-            - else
-              = render 'clusters/clusters/gcp/gcp_not_configured'
-          - elsif @eks_selected
-            = render 'clusters/clusters/eks/index'
-          - else
-            = render 'clusters/clusters/cloud_providers/cloud_provider_selector'
+          = render new_cluster_partial(provider: params[:provider])
       - else
         .tab-pane{ id: 'create-cluster-pane', class: active_when(active_tab == 'create'), role: 'tabpanel' }
-          = render 'clusters/clusters/gcp/header'
-          - if @valid_gcp_token
-            = render 'clusters/clusters/gcp/form'
-          - elsif @authorize_url
-            = render 'clusters/clusters/gcp/signin_with_google_button'
-          - else
-            = render 'clusters/clusters/gcp/gcp_not_configured'
+          = render new_cluster_partial(provider: 'gcp')
 
       .tab-pane{ id: 'add-cluster-pane', class: active_when(active_tab == 'add'), role: 'tabpanel' }
         = render 'clusters/clusters/user/header'

spec/controllers/admin/clusters_controller_spec.rb

@@ -73,7 +73,7 @@ describe Admin::ClustersController do
   end
 
   describe 'GET #new' do
-    def get_new(provider: 'gke')
+    def get_new(provider: 'gcp')
       get :new, params: { provider: provider }
     end
 

spec/controllers/groups/clusters_controller_spec.rb

@@ -85,7 +85,7 @@ describe Groups::ClustersController do
   end
 
   describe 'GET new' do
-    def go(provider: 'gke')
+    def go(provider: 'gcp')
       get :new, params: { group_id: group, provider: provider }
     end
 

spec/controllers/projects/clusters_controller_spec.rb

@@ -79,7 +79,7 @@ describe Projects::ClustersController do
   end
 
   describe 'GET new' do
-    def go(provider: 'gke')
+    def go(provider: 'gcp')
       get :new, params: {
         namespace_id: project.namespace,
         project_id: project,

spec/helpers/clusters_helper_spec.rb

@@ -30,4 +30,60 @@ describe ClustersHelper do
       end
     end
   end
+
+  describe '#create_new_cluster_label' do
+    subject { helper.create_new_cluster_label(provider: provider) }
+
+    context 'GCP provider' do
+      let(:provider) { 'gcp' }
+
+      it { is_expected.to eq('Create new Cluster on GKE') }
+    end
+
+    context 'AWS provider' do
+      let(:provider) { 'aws' }
+
+      it { is_expected.to eq('Create new Cluster on EKS') }
+    end
+
+    context 'other provider' do
+      let(:provider) { 'other' }
+
+      it { is_expected.to eq('Create new Cluster') }
+    end
+
+    context 'no provider' do
+      let(:provider) { nil }
+
+      it { is_expected.to eq('Create new Cluster') }
+    end
+  end
+
+  describe '#render_new_provider_form' do
+    subject { helper.new_cluster_partial(provider: provider) }
+
+    context 'GCP provider' do
+      let(:provider) { 'gcp' }
+
+      it { is_expected.to eq('clusters/clusters/gcp/new') }
+    end
+
+    context 'AWS provider' do
+      let(:provider) { 'aws' }
+
+      it { is_expected.to eq('clusters/clusters/aws/new') }
+    end
+
+    context 'other provider' do
+      let(:provider) { 'other' }
+
+      it { is_expected.to eq('clusters/clusters/cloud_providers/cloud_provider_selector') }
+    end
+
+    context 'no provider' do
+      let(:provider) { nil }
+
+      it { is_expected.to eq('clusters/clusters/cloud_providers/cloud_provider_selector') }
+    end
+  end
 end

spec/models/aws/role_spec.rb

@@ -33,7 +33,7 @@ describe Aws::Role do
   end
 
   describe 'callbacks' do
-    describe '#ensure_role_external_id' do
+    describe '#ensure_role_external_id!' do
       subject { role.validate }
 
       context 'for a new record' do