diff options
| -rw-r--r-- | app/controllers/clusters/clusters_controller.rb | 10 | ||||
| -rw-r--r-- | app/models/clusters/cluster.rb | 5 | ||||
| -rw-r--r-- | app/models/clusters/platforms/kubernetes.rb | 4 | ||||
| -rw-r--r-- | app/presenters/clusters/cluster_presenter.rb | 4 | ||||
| -rw-r--r-- | app/validators/cluster_name_validator.rb | 8 | ||||
| -rw-r--r-- | app/views/clusters/clusters/_advanced_settings.html.haml | 2 | ||||
| -rw-r--r-- | app/views/clusters/clusters/show.html.haml | 2 | ||||
| -rw-r--r-- | app/views/clusters/platforms/kubernetes/_form.html.haml | 14 | ||||
| -rw-r--r-- | spec/features/clusters/cluster_detail_page_spec.rb | 80 | ||||
| -rw-r--r-- | spec/models/clusters/cluster_spec.rb | 16 | ||||
| -rw-r--r-- | spec/models/clusters/platforms/kubernetes_spec.rb | 2 | ||||
| -rw-r--r-- | spec/presenters/clusters/cluster_presenter_spec.rb | 16 |
12 files changed, 137 insertions, 26 deletions
diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index 68a2a83f0de..e82756e4643 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/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 ] ) diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index d9dec680beb..7a10b07ee9d 100644 --- a/app/models/clusters/cluster.rb +++ b/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 diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 63ef7ba6b45..2ae141190a8 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/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')) diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb index 7a5b68f9a4b..81994bbce7d 100644 --- a/app/presenters/clusters/cluster_presenter.rb +++ b/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 diff --git a/app/validators/cluster_name_validator.rb b/app/validators/cluster_name_validator.rb index 85fd63f08e5..79c9c67ae58 100644 --- a/app/validators/cluster_name_validator.rb +++ b/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 diff --git a/app/views/clusters/clusters/_advanced_settings.html.haml b/app/views/clusters/clusters/_advanced_settings.html.haml index 7037c80aa6b..8005dcbf65f 100644 --- a/app/views/clusters/clusters/_advanced_settings.html.haml +++ b/app/views/clusters/clusters/_advanced_settings.html.haml @@ -1,5 +1,5 @@ - 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') diff --git a/app/views/clusters/clusters/show.html.haml b/app/views/clusters/clusters/show.html.haml index 61188c6fa0b..62b947ca40d 100644 --- a/app/views/clusters/clusters/show.html.haml +++ b/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' diff --git a/app/views/clusters/platforms/kubernetes/_form.html.haml b/app/views/clusters/platforms/kubernetes/_form.html.haml index 4a452b83112..b5ddca7ccb9 100644 --- a/app/views/clusters/platforms/kubernetes/_form.html.haml +++ b/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? diff --git a/spec/features/clusters/cluster_detail_page_spec.rb b/spec/features/clusters/cluster_detail_page_spec.rb index b9fc52d0dce..d2e46d15730 100644 --- a/spec/features/clusters/cluster_detail_page_spec.rb +++ b/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 diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index acbcdc7d170..fabd2806d9a 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/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 diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index af65530e663..a79b436c22a 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/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' diff --git a/spec/presenters/clusters/cluster_presenter_spec.rb b/spec/presenters/clusters/cluster_presenter_spec.rb index 754ba0a594c..a9d786bc872 100644 --- a/spec/presenters/clusters/cluster_presenter_spec.rb +++ b/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 |