From 5a042ef2fbe1bd57b9428c89b49d2fa1e248ad46 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 6 Nov 2018 21:55:10 +1300 Subject: Only project clusters has Project Namespace field Group clusters should not allow Project Namespace so don't show that field input too --- app/controllers/clusters/clusters_controller.rb | 4 ++-- app/models/clusters/cluster.rb | 2 +- app/models/clusters/platforms/kubernetes.rb | 9 +++++++++ app/presenters/clusters/cluster_presenter.rb | 4 ++++ app/views/clusters/clusters/gcp/_show.html.haml | 7 ++++--- app/views/clusters/clusters/user/_form.html.haml | 7 ++++--- app/views/clusters/clusters/user/_show.html.haml | 7 ++++--- spec/controllers/groups/clusters_controller_spec.rb | 18 +++--------------- spec/controllers/projects/clusters_controller_spec.rb | 4 ++-- spec/factories/clusters/platforms/kubernetes.rb | 2 +- spec/models/clusters/platforms/kubernetes_spec.rb | 12 ++++++++++++ 11 files changed, 46 insertions(+), 30 deletions(-) diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb index f6f2060ebb5..2e9c77ae55c 100644 --- a/app/controllers/clusters/clusters_controller.rb +++ b/app/controllers/clusters/clusters_controller.rb @@ -183,13 +183,13 @@ class Clusters::ClustersController < Clusters::BaseController def gcp_cluster @gcp_cluster = ::Clusters::Cluster.new.tap do |cluster| cluster.build_provider_gcp - end + end.present(current_user: current_user) end def user_cluster @user_cluster = ::Clusters::Cluster.new.tap do |cluster| cluster.build_platform_kubernetes - end + end.present(current_user: current_user) end def validate_gcp_token diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 48d6c0daa0f..4369d09a89c 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -29,7 +29,7 @@ module Clusters # we force autosave to happen when we save `Cluster` model has_one :provider_gcp, class_name: 'Clusters::Providers::Gcp', autosave: true - has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', autosave: true + has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', inverse_of: :cluster, autosave: true has_one :application_helm, class_name: 'Clusters::Applications::Helm' has_one :application_ingress, class_name: 'Clusters::Applications::Ingress' diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index a0933958b50..a90a5395749 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -38,6 +38,8 @@ module Clusters validates :namespace, exclusion: { in: RESERVED_NAMESPACES } + validate :no_namespace, unless: :project_type? + # We expect to be `active?` only when enabled and cluster is created (the api_url is assigned) validates :api_url, url: true, presence: true validates :token, presence: true @@ -52,6 +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 :project_type?, to: :cluster, allow_nil: true delegate :kubernetes_namespace, to: :cluster alias_method :active?, :enabled? @@ -207,6 +210,12 @@ module Clusters self.token = self.token&.strip end + def no_namespace + if namespace + errors.add(:namespace, 'only allowed for project cluster') + end + end + def prevent_modification return unless managed? diff --git a/app/presenters/clusters/cluster_presenter.rb b/app/presenters/clusters/cluster_presenter.rb index 7e6eccb648c..01e7ed92515 100644 --- a/app/presenters/clusters/cluster_presenter.rb +++ b/app/presenters/clusters/cluster_presenter.rb @@ -8,6 +8,10 @@ module Clusters "https://console.cloud.google.com/kubernetes/clusters/details/#{provider.zone}/#{name}" if gcp? end + def allow_project_namespace? + cluster.project_type? + end + def can_toggle_cluster? can?(current_user, :update_cluster, cluster) && created? end diff --git a/app/views/clusters/clusters/gcp/_show.html.haml b/app/views/clusters/clusters/gcp/_show.html.haml index ca55ccb8fdf..677e20bfa0a 100644 --- a/app/views/clusters/clusters/gcp/_show.html.haml +++ b/app/views/clusters/clusters/gcp/_show.html.haml @@ -33,9 +33,10 @@ = s_('ClusterIntegration|Show') = clipboard_button(text: @cluster.platform_kubernetes.token, title: s_('ClusterIntegration|Copy Token'), class: 'btn-default') - .form-group - = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)') - = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') + - if @cluster.allow_project_namespace? + .form-group + = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)') + = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') .form-group .form-check diff --git a/app/views/clusters/clusters/user/_form.html.haml b/app/views/clusters/clusters/user/_form.html.haml index e4758938059..55ef17cbcf5 100644 --- a/app/views/clusters/clusters/user/_form.html.haml +++ b/app/views/clusters/clusters/user/_form.html.haml @@ -21,9 +21,10 @@ = platform_kubernetes_field.label :token, s_('ClusterIntegration|Token'), class: 'label-bold' = platform_kubernetes_field.text_field :token, class: 'form-control', placeholder: s_('ClusterIntegration|Service token'), autocomplete: 'off' - .form-group - = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)'), class: 'label-bold' - = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') + - if @user_cluster.allow_project_namespace? + .form-group + = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)'), class: 'label-bold' + = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') .form-group .form-check diff --git a/app/views/clusters/clusters/user/_show.html.haml b/app/views/clusters/clusters/user/_show.html.haml index ad8c35e32e3..b7a03723324 100644 --- a/app/views/clusters/clusters/user/_show.html.haml +++ b/app/views/clusters/clusters/user/_show.html.haml @@ -22,9 +22,10 @@ %button.js-show-cluster-token.btn-blank{ type: 'button' } = s_('ClusterIntegration|Show') - .form-group - = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)'), class: 'label-bold' - = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') + - if @cluster.allow_project_namespace? + .form-group + = platform_kubernetes_field.label :namespace, s_('ClusterIntegration|Project namespace (optional, unique)'), class: 'label-bold' + = platform_kubernetes_field.text_field :namespace, class: 'form-control', placeholder: s_('ClusterIntegration|Project namespace') .form-group .form-check diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 92a7ee6d84b..6be1d300aa4 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -146,7 +146,7 @@ describe Groups::ClustersController do it 'has new object' do go - expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::Cluster) + expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end end @@ -167,7 +167,7 @@ describe Groups::ClustersController do it 'has new object' do go - expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::Cluster) + expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end end @@ -281,7 +281,6 @@ describe Groups::ClustersController do platform_kubernetes_attributes: { api_url: 'http://my-url', token: 'test', - namespace: 'aaa' } } } @@ -315,7 +314,6 @@ describe Groups::ClustersController do platform_kubernetes_attributes: { api_url: 'http://my-url', token: 'test', - namespace: 'aaa', authorization_type: 'rbac' } } @@ -433,9 +431,6 @@ describe Groups::ClustersController do cluster: { enabled: false, name: 'my-new-cluster-name', - platform_kubernetes_attributes: { - namespace: 'my-namespace' - } } } end @@ -448,7 +443,6 @@ 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.platform_kubernetes.namespace).to eq('my-namespace') end context 'when format is json' do @@ -459,9 +453,6 @@ describe Groups::ClustersController do cluster: { enabled: false, name: 'my-new-cluster-name', - platform_kubernetes_attributes: { - namespace: 'my-namespace' - } } } end @@ -473,7 +464,6 @@ describe Groups::ClustersController do expect(response).to have_http_status(:no_content) expect(cluster.enabled).to be_falsey expect(cluster.name).to eq('my-new-cluster-name') - expect(cluster.platform_kubernetes.namespace).to eq('my-namespace') end end @@ -482,9 +472,7 @@ describe Groups::ClustersController do { cluster: { enabled: false, - platform_kubernetes_attributes: { - namespace: 'my invalid namespace #@' - } + name: '' } } end diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index e7bb4035e55..483222363bb 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -122,7 +122,7 @@ describe Projects::ClustersController do it 'has new object' do go - expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::Cluster) + expect(assigns(:gcp_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end end @@ -143,7 +143,7 @@ describe Projects::ClustersController do it 'has new object' do go - expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::Cluster) + expect(assigns(:user_cluster)).to be_an_instance_of(Clusters::ClusterPresenter) end end diff --git a/spec/factories/clusters/platforms/kubernetes.rb b/spec/factories/clusters/platforms/kubernetes.rb index 4a0d1b181ea..8169c457ab7 100644 --- a/spec/factories/clusters/platforms/kubernetes.rb +++ b/spec/factories/clusters/platforms/kubernetes.rb @@ -10,7 +10,7 @@ FactoryBot.define do username 'xxxxxx' password 'xxxxxx' - after(:create) do |platform_kubernetes, evaluator| + before(:create) do |platform_kubernetes, evaluator| pem_file = File.expand_path(Rails.root.join('spec/fixtures/clusters/sample_cert.pem')) platform_kubernetes.ca_cert = File.read(pem_file) end diff --git a/spec/models/clusters/platforms/kubernetes_spec.rb b/spec/models/clusters/platforms/kubernetes_spec.rb index 2bcccc8184a..f5d261c4e9d 100644 --- a/spec/models/clusters/platforms/kubernetes_spec.rb +++ b/spec/models/clusters/platforms/kubernetes_spec.rb @@ -58,6 +58,18 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching it { is_expected.to be_truthy } end + + context 'for group cluster' do + let(:namespace) { 'namespace-123' } + let(:cluster) { build(:cluster, :group, :provided_by_user) } + let(:kubernetes) { cluster.platform_kubernetes } + + before do + kubernetes.namespace = namespace + end + + it { is_expected.to be_falsey } + end end context 'when validates api_url' do -- cgit v1.2.1