specs for controller. Improved validation

app/controllers/projects/clusters_controller.rb

@@ -27,13 +27,14 @@ class Projects::ClustersController < Projects::ApplicationController
   end
 
   def new
-    @cluster = Clusters::Cluster.new(
-      platform_type: :kubernetes,
-      provider_type: :gcp).tap do |cluster|
-      cluster.build_provider_gcp
-      cluster.build_platform_kubernetes
-      cluster.projects << project
-    end
+    # @cluster = Clusters::Cluster.new(
+    #   platform_type: :kubernetes,
+    #   provider_type: :gcp).tap do |cluster|
+    #   cluster.build_provider_gcp
+    #   cluster.build_platform_kubernetes
+    #   cluster.projects << project
+    # end
+    @cluster = Clusters::Cluster.new
   end
 
   def create

app/models/clusters/cluster.rb

@@ -10,11 +10,13 @@ module Clusters
     has_many :projects, through: :cluster_projects, class_name: '::Project'
 
     has_one :provider_gcp, class_name: 'Clusters::Providers::Gcp'
-    has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes'
+    has_one :platform_kubernetes, class_name: 'Clusters::Platforms::Kubernetes', validate: { if: :update }
 
-    accepts_nested_attributes_for :provider_gcp
+    accepts_nested_attributes_for :provider_gcp, update_only: true
     accepts_nested_attributes_for :platform_kubernetes, update_only: true
 
+    validates :provider_type, presence: true
+    validates :platform_type, presence: true
     validates :name, cluster_name: true
     validate :restrict_modification, on: :update
 

app/models/clusters/platforms/kubernetes.rb

@@ -30,8 +30,8 @@ module Clusters
           message: Gitlab::Regex.kubernetes_namespace_regex_message
         }
 
-      validates :api_url, url: true, presence: true, on: :update
-      validates :token, presence: true, on: :update
+      validates :api_url, url: true, presence: true
+      validates :token, presence: true
 
       after_save :clear_reactive_cache!
 

app/services/clusters/create_service.rb

@@ -28,10 +28,10 @@ module Clusters
     def cluster_params
       return @cluster_params if defined?(@cluster_params)
 
-      params[:provider_gcp_attributes][:machine_type] ||=
-        GoogleApi::CloudPlatform::Client::DEFAULT_MACHINE_TYPE
-
-      params[:provider_gcp_attributes][:access_token] ||= access_token
+      params[:provider_gcp_attributes].try do |h|
+        h[:machine_type] ||= GoogleApi::CloudPlatform::Client::DEFAULT_MACHINE_TYPE
+        h[:access_token] ||= access_token
+      end
 
       @cluster_params = params.merge(user: current_user)
     end

spec/factories/clusters/cluster.rb

@@ -42,7 +42,9 @@ FactoryGirl.define do
       end
 
       after(:create) do |cluster, evaluator|
-        create(:platform_kubernetes, cluster: cluster)
+        build(:platform_kubernetes, cluster: cluster).tap do |platform|
+          platform.save!(validate: false)
+        end
       end
     end
   end

spec/factories/clusters/platforms/kubernetes.rb

@@ -3,19 +3,16 @@ FactoryGirl.define do
     cluster
     namespace nil
 
-    trait :ca_cert do
-      after(: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
-    end
-
     trait :configured do
       api_url 'https://kubernetes.example.com'
-      ca_cert nil
       token 'a' * 40
       username 'xxxxxx'
       password 'xxxxxx'
+
+      after(: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
     end
   end
 end

spec/models/clusters/cluster_spec.rb

@@ -10,6 +10,8 @@ describe Clusters::Cluster do
   it { is_expected.to delegate_method(:status_name).to(:provider) }
   it { is_expected.to delegate_method(:on_creation?).to(:provider) }
   it { is_expected.to respond_to :project }
+  it { is_expected.to validate_presence_of(:provider_type) }
+  it { is_expected.to validate_presence_of(:platform_type) }
 
   describe '.enabled' do
     subject { described_class.enabled }

spec/models/clusters/platforms/kubernetes_spec.rb

@@ -11,7 +11,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
   describe 'before_validation' do
     context 'when namespace includes upper case' do
-      let(:kubernetes) { create(:platform_kubernetes, namespace: namespace) }
+      let(:kubernetes) { create(:platform_kubernetes, :configured, namespace: namespace) }
       let(:namespace) { 'ABC' }
 
       it 'converts to lower case' do
@@ -24,7 +24,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     subject { kubernetes.valid? }
 
     context 'when validates namespace' do
-      let(:kubernetes) { build(:platform_kubernetes, namespace: namespace) }
+      let(:kubernetes) { build(:platform_kubernetes, :configured, namespace: namespace) }
 
       context 'when namespace is blank' do
         let(:namespace) { '' }
@@ -52,74 +52,42 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     end
 
     context 'when validates api_url' do
-      context 'when updates a record' do
-        let(:kubernetes) { create(:platform_kubernetes) }
+      let(:kubernetes) { build(:platform_kubernetes, :configured) }
 
-        before do
-          kubernetes.api_url = api_url
-        end
-
-        context 'when api_url is invalid url' do
-          let(:api_url) { '!!!!!!' }
-
-          it { expect(kubernetes.save).to be_falsey }
-        end
-
-        context 'when api_url is nil' do
-          let(:api_url) { nil }
-
-          it { expect(kubernetes.save).to be_falsey }
-        end
+      before do
+        kubernetes.api_url = api_url
+      end
 
-        context 'when api_url is valid url' do
-          let(:api_url) { 'https://111.111.111.111' }
+      context 'when api_url is invalid url' do
+        let(:api_url) { '!!!!!!' }
 
-          it { expect(kubernetes.save).to be_truthy }
-        end
+        it { expect(kubernetes.save).to be_falsey }
       end
 
-      context 'when creates a record' do
-        let(:kubernetes) { build(:platform_kubernetes) }
+      context 'when api_url is nil' do
+        let(:api_url) { nil }
 
-        before do
-          kubernetes.api_url = api_url
-        end
+        it { expect(kubernetes.save).to be_falsey }
+      end
 
-        context 'when api_url is nil' do
-          let(:api_url) { nil }
+      context 'when api_url is valid url' do
+        let(:api_url) { 'https://111.111.111.111' }
 
-          it { expect(kubernetes.save).to be_truthy }
-        end
+        it { expect(kubernetes.save).to be_truthy }
       end
     end
 
     context 'when validates token' do
-      context 'when updates a record' do
-        let(:kubernetes) { create(:platform_kubernetes) }
+      let(:kubernetes) { build(:platform_kubernetes, :configured) }
 
-        before do
-          kubernetes.token = token
-        end
-
-        context 'when token is nil' do
-          let(:token) { nil }
-
-          it { expect(kubernetes.save).to be_falsey }
-        end
+      before do
+        kubernetes.token = token
       end
 
-      context 'when creates a record' do
-        let(:kubernetes) { build(:platform_kubernetes) }
+      context 'when token is nil' do
+        let(:token) { nil }
 
-        before do
-          kubernetes.token = token
-        end
-
-        context 'when token is nil' do
-          let(:token) { nil }
-
-          it { expect(kubernetes.save).to be_truthy }
-        end
+        it { expect(kubernetes.save).to be_falsey }
       end
     end
   end
@@ -128,7 +96,8 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     subject { kubernetes.actual_namespace }
 
     let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) }
-    let(:kubernetes) { create(:platform_kubernetes, namespace: namespace) }
+    let(:project) { cluster.project }
+    let(:kubernetes) { create(:platform_kubernetes, :configured, namespace: namespace) }
 
     context 'when namespace is present' do
       let(:namespace) { 'namespace-123' }
@@ -139,7 +108,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     context 'when namespace is not present' do
       let(:namespace) { nil }
 
-      it { is_expected.to eq("#{cluster.project.path}-#{cluster.project.id}") }
+      it { is_expected.to eq("#{project.path}-#{project.id}") }
     end
   end
 
@@ -154,12 +123,13 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
   describe '#default_namespace' do
     subject { kubernetes.default_namespace }
 
-    let(:kubernetes) { create(:platform_kubernetes) }
+    let(:kubernetes) { create(:platform_kubernetes, :configured) }
 
     context 'when cluster belongs to a project' do
       let!(:cluster) { create(:cluster, :project, platform_kubernetes: kubernetes) }
+      let(:project) { cluster.project }
 
-      it { is_expected.to eq("#{cluster.project.path}-#{cluster.project.id}") }
+      it { is_expected.to eq("#{project.path}-#{project.id}") }
     end
 
     context 'when cluster belongs to nothing' do
@@ -229,7 +199,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
 
     let!(:cluster) { create(:cluster, :project, platform_kubernetes: service) }
     let(:project) { cluster.project }
-    let(:service) { create(:platform_kubernetes) }
+    let(:service) { create(:platform_kubernetes, :configured) }
     let(:environment) { build(:environment, project: project, name: "env", slug: "env-000000") }
 
     context 'with invalid pods' do
@@ -268,7 +238,7 @@ describe Clusters::Platforms::Kubernetes, :use_clean_rails_memory_store_caching
     subject { service.calculate_reactive_cache }
 
     let!(:cluster) { create(:cluster, :project, enabled: enabled, platform_kubernetes: service) }
-    let(:service) { create(:platform_kubernetes, :ca_cert) }
+    let(:service) { create(:platform_kubernetes, :configured) }
     let(:enabled) { true }
 
     context 'when cluster is disabled' do

spec/support/google_api_helpers.rb → spec/support/google_api/cloud_platform_helpers.rb

 module GoogleApi
   module CloudPlatformHelpers
+    def stub_google_api_validate_token
+      request.session[GoogleApi::CloudPlatform::Client.session_key_for_token] = 'token'
+      request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.since.to_i.to_s
+    end
+
+    def stub_google_api_expired_token
+      request.session[GoogleApi::CloudPlatform::Client.session_key_for_token] = 'token'
+      request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.ago.to_i.to_s
+    end
+
     def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options)
       WebMock.stub_request(:get, cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id))
         .to_return(cloud_platform_response(cloud_platform_cluster_body(options)))

spec/support/kubernetes_helpers.rb

@@ -14,7 +14,7 @@ module KubernetesHelpers
   end
 
   def stub_kubeclient_pods(response = nil)
-    stub_kubeclient_discover
+    stub_kubeclient_discover(service.api_url)
     pods_url = service.api_url + "/api/v1/namespaces/#{service.actual_namespace}/pods"
 
     WebMock.stub_request(:get, pods_url).to_return(response || kube_pods_response)