Automatic merge of gitlab-org/gitlab-ce master

app/controllers/clusters/clusters_controller.rb

@@ -123,25 +123,25 @@ class Clusters::ClustersController < Clusters::BaseController
   private
 
   def update_params
-    if cluster.managed?
+    if cluster.provided_by_user?
       params.require(:cluster).permit(
         :enabled,
+        :name,
         :environment_scope,
         :base_domain,
         platform_kubernetes_attributes: [
+          :api_url,
+          :token,
+          :ca_cert,
           :namespace
         ]
       )
     else
       params.require(:cluster).permit(
         :enabled,
-        :name,
         :environment_scope,
         :base_domain,
         platform_kubernetes_attributes: [
-          :api_url,
-          :token,
-          :ca_cert,
           :namespace
         ]
       )

app/models/clusters/cluster.rb

@@ -70,6 +70,7 @@ module Clusters
     delegate :external_hostname, to: :application_ingress, prefix: true, allow_nil: true
 
     alias_attribute :base_domain, :domain
+    alias_attribute :provided_by_user?, :user?
 
     enum cluster_type: {
       instance_type: 1,
@@ -149,10 +150,6 @@ module Clusters
       return platform_kubernetes if kubernetes?
     end
 
-    def managed?
-      !user?
-    end
-
     def all_projects
       if project_type?
         projects

app/models/clusters/platforms/kubernetes.rb

@@ -54,7 +54,7 @@ module Clusters
 
       delegate :project, to: :cluster, allow_nil: true
       delegate :enabled?, to: :cluster, allow_nil: true
-      delegate :managed?, to: :cluster, allow_nil: true
+      delegate :provided_by_user?, to: :cluster, allow_nil: true
       delegate :allow_user_defined_namespace?, to: :cluster, allow_nil: true
       delegate :kubernetes_namespace, to: :cluster
 
@@ -219,7 +219,7 @@ module Clusters
       end
 
       def prevent_modification
-        return unless managed?
+        return if provided_by_user?
 
         if api_url_changed? || token_changed? || ca_pem_changed?
           errors.add(:base, _('Cannot modify managed Kubernetes cluster'))

app/presenters/clusters/cluster_presenter.rb

@@ -48,6 +48,10 @@ module Clusters
       end
     end
 
+    def read_only_kubernetes_platform_fields?
+      !cluster.provided_by_user?
+    end
+
     private
 
     def clusterable

app/validators/cluster_name_validator.rb

@@ -5,7 +5,9 @@
 # Custom validator for ClusterName.
 class ClusterNameValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
-    if record.managed?
+    if record.provided_by_user?
+      record.errors.add(attribute, " has to be present") unless value.present?
+    else
       if record.persisted? && record.name_changed?
         record.errors.add(attribute, " can not be changed because it's synchronized with provider")
       end
@@ -17,10 +19,6 @@ class ClusterNameValidator < ActiveModel::EachValidator
       unless value =~ Gitlab::Regex.kubernetes_namespace_regex
         record.errors.add(attribute, Gitlab::Regex.kubernetes_namespace_regex_message)
       end
-    else
-      unless value.present?
-        record.errors.add(attribute, " has to be present")
-      end
     end
   end
 end

app/views/clusters/clusters/_advanced_settings.html.haml

 - if can?(current_user, :admin_cluster, @cluster)
-  - if @cluster.managed?
+  - unless @cluster.provided_by_user?
     .append-bottom-20
       %label.append-bottom-10
         = s_('ClusterIntegration|Google Kubernetes Engine')

app/views/clusters/clusters/show.html.haml

@@ -53,5 +53,5 @@
       %button.btn.js-settings-toggle{ type: 'button' }
         = expanded ? _('Collapse') : _('Expand')
       %p= s_("ClusterIntegration|Advanced options on this Kubernetes cluster's integration")
-    .settings-content
+    .settings-content#advanced-settings-section
       = render 'advanced_settings'

app/views/clusters/platforms/kubernetes/_form.html.haml

@@ -2,7 +2,7 @@
   = form_errors(cluster)
 
   .form-group
-    - if cluster.managed?
+    - if cluster.read_only_kubernetes_platform_fields?
       %label.append-bottom-10{ for: 'cluster-name' }
         = s_('ClusterIntegration|Kubernetes cluster name')
       .input-group
@@ -18,27 +18,27 @@
     .form-group
       = platform_field.label :api_url, s_('ClusterIntegration|API URL')
       .input-group
-        = platform_field.text_field :api_url, class: 'form-control js-select-on-focus', placeholder: s_('ClusterIntegration|API URL'), readonly: cluster.managed?
-        - if cluster.managed?
+        = platform_field.text_field :api_url, class: 'form-control js-select-on-focus', placeholder: s_('ClusterIntegration|API URL'), readonly: cluster.read_only_kubernetes_platform_fields?
+        - if cluster.read_only_kubernetes_platform_fields?
           %span.input-group-append
             = clipboard_button(text: platform.api_url, title: s_('ClusterIntegration|Copy API URL'), class: 'input-group-text btn-default')
 
     .form-group
       = platform_field.label :ca_cert, s_('ClusterIntegration|CA Certificate')
       .input-group
-        = platform_field.text_area :ca_cert, class: 'form-control js-select-on-focus', placeholder: s_('ClusterIntegration|Certificate Authority bundle (PEM format)'), readonly: cluster.managed?
-        - if cluster.managed?
+        = platform_field.text_area :ca_cert, class: 'form-control js-select-on-focus', placeholder: s_('ClusterIntegration|Certificate Authority bundle (PEM format)'), readonly: cluster.read_only_kubernetes_platform_fields?
+        - if cluster.read_only_kubernetes_platform_fields?
           %span.input-group-append.clipboard-addon
             = clipboard_button(text: platform.ca_cert, title: s_('ClusterIntegration|Copy CA Certificate'), class: 'input-group-text btn-blank')
 
     .form-group
       = platform_field.label :token, s_('ClusterIntegration|Token')
       .input-group
-        = platform_field.text_field :token, class: 'form-control js-cluster-token js-select-on-focus', type: 'password', placeholder: s_('ClusterIntegration|Token'), readonly: cluster.managed?
+        = platform_field.text_field :token, class: 'form-control js-cluster-token js-select-on-focus', type: 'password', placeholder: s_('ClusterIntegration|Token'), readonly: cluster.read_only_kubernetes_platform_fields?
         %span.input-group-append
           %button.btn.btn-default.input-group-text.js-show-cluster-token{ type: 'button' }
             = s_('ClusterIntegration|Show')
-          - if cluster.managed?
+          - if cluster.read_only_kubernetes_platform_fields?
             = clipboard_button(text: platform.token, title: s_('ClusterIntegration|Copy Token'), class: 'btn-default')
 
     - if cluster.allow_user_defined_namespace?

spec/features/clusters/cluster_detail_page_spec.rb

@@ -53,12 +53,80 @@ describe 'Clusterable > Show page' do
     end
   end
 
+  shared_examples 'editing a GCP cluster' do
+    before do
+      clusterable.add_maintainer(current_user)
+      visit cluster_path
+    end
+
+    it 'is not able to edit the name, API url, CA certificate nor token' do
+      within('#js-cluster-details') do
+        cluster_name_field = find('.cluster-name')
+        api_url_field = find('#cluster_platform_kubernetes_attributes_api_url')
+        ca_certificate_field = find('#cluster_platform_kubernetes_attributes_ca_cert')
+        token_field = find('#cluster_platform_kubernetes_attributes_token')
+
+        expect(cluster_name_field).to be_readonly
+        expect(api_url_field).to be_readonly
+        expect(ca_certificate_field).to be_readonly
+        expect(token_field).to be_readonly
+      end
+    end
+
+    it 'displays GKE information' do
+      within('#advanced-settings-section') do
+        expect(page).to have_content('Google Kubernetes Engine')
+        expect(page).to have_content('Manage your Kubernetes cluster by visiting')
+      end
+    end
+  end
+
+  shared_examples 'editing a user-provided cluster' do
+    before do
+      clusterable.add_maintainer(current_user)
+      visit cluster_path
+    end
+
+    it 'is able to edit the name, API url, CA certificate and token' do
+      within('#js-cluster-details') do
+        cluster_name_field = find('#cluster_name')
+        api_url_field = find('#cluster_platform_kubernetes_attributes_api_url')
+        ca_certificate_field = find('#cluster_platform_kubernetes_attributes_ca_cert')
+        token_field = find('#cluster_platform_kubernetes_attributes_token')
+
+        expect(cluster_name_field).not_to be_readonly
+        expect(api_url_field).not_to be_readonly
+        expect(ca_certificate_field).not_to be_readonly
+        expect(token_field).not_to be_readonly
+      end
+    end
+
+    it 'does not display GKE information' do
+      within('#advanced-settings-section') do
+        expect(page).not_to have_content('Google Kubernetes Engine')
+        expect(page).not_to have_content('Manage your Kubernetes cluster by visiting')
+      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
+
+    it_behaves_like 'editing a GCP cluster' do
+      let(:clusterable) { create(:project) }
+      let(:cluster) { create(:cluster, :provided_by_gcp, :project, projects: [clusterable]) }
+      let(:cluster_path) { project_cluster_path(clusterable, cluster) }
+    end
+
+    it_behaves_like 'editing a user-provided cluster' do
+      let(:clusterable) { create(:project) }
+      let(:cluster) { create(:cluster, :provided_by_user, :project, projects: [clusterable]) }
+      let(:cluster_path) { project_cluster_path(clusterable, cluster) }
+    end
   end
 
   context 'when clusterable is a group' do
@@ -67,5 +135,17 @@ describe 'Clusterable > Show page' do
       let(:cluster) { create(:cluster, :provided_by_gcp, :group, groups: [clusterable]) }
       let(:cluster_path) { group_cluster_path(clusterable, cluster) }
     end
+
+    it_behaves_like 'editing a GCP cluster' do
+      let(:clusterable) { create(:group) }
+      let(:cluster) { create(:cluster, :provided_by_gcp, :group, groups: [clusterable]) }
+      let(:cluster_path) { group_cluster_path(clusterable, cluster) }
+    end
+
+    it_behaves_like 'editing a user-provided cluster' do
+      let(:clusterable) { create(:group) }
+      let(:cluster) { create(:cluster, :provided_by_user, :group, groups: [clusterable]) }
+      let(:cluster_path) { group_cluster_path(clusterable, cluster) }
+    end
   end
 end

spec/models/clusters/cluster_spec.rb

@@ -620,4 +620,20 @@ describe Clusters::Cluster do
       end
     end
   end
+
+  describe '#provided_by_user?' do
+    subject { cluster.provided_by_user? }
+
+    context 'with a GCP provider' do
+      let(:cluster) { create(:cluster, :provided_by_gcp) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'with an user provider' do
+      let(:cluster) { create(:cluster, :provided_by_user) }
+
+      it { is_expected.to be_truthy }
+    end
+  end
 end

spec/models/clusters/platforms/kubernetes_spec.rb

@@ -15,7 +15,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
   it { is_expected.to delegate_method(:project).to(:cluster) }
   it { is_expected.to delegate_method(:enabled?).to(:cluster) }
-  it { is_expected.to delegate_method(:managed?).to(:cluster) }
+  it { is_expected.to delegate_method(:provided_by_user?).to(:cluster) }
   it { is_expected.to delegate_method(:kubernetes_namespace).to(:cluster) }
 
   it_behaves_like 'having unique enum values'

spec/presenters/clusters/cluster_presenter_spec.rb

@@ -228,4 +228,20 @@ describe Clusters::ClusterPresenter do
       it { is_expected.to eq(group_cluster_path(group, cluster)) }
     end
   end
+
+  describe '#read_only_kubernetes_platform_fields?' do
+    subject { described_class.new(cluster).read_only_kubernetes_platform_fields? }
+
+    context 'with a user-provided cluster' do
+      let(:cluster) { build_stubbed(:cluster, :provided_by_user) }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'with a GCP-provided cluster' do
+      let(:cluster) { build_stubbed(:cluster, :provided_by_gcp) }
+
+      it { is_expected.to be_truthy }
+    end
+  end
 end