diff options
5 files changed, 111 insertions, 46 deletions
diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 940c2a5d84f..95c947001a3 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,14 +1,14 @@ class Projects::Clusters::GcpController < Projects::ApplicationController before_action :authorize_read_cluster! before_action :authorize_google_api, except: [:login] - before_action :authorize_google_project_billing, only: [:check] + before_action :authorize_google_project_billing, except: [:login, :check] before_action :authorize_create_cluster!, only: [:new, :create] STATUS_POLLING_INTERVAL = 1.minute.to_i def login begin - state = generate_session_key_redirect(gcp_check_namespace_project_clusters_path.to_s) + state = generate_session_key_redirect(gcp_new_namespace_project_clusters_path.to_s) @authorize_url = GoogleApi::CloudPlatform::Client.new( nil, callback_google_api_auth_url, @@ -76,6 +76,7 @@ class Projects::Clusters::GcpController < Projects::ApplicationController Gitlab::Redis::SharedState.with do |redis| unless redis.get(CheckGcpProjectBillingWorker.redis_shared_state_key_for(token_in_session)) == 'true' CheckGcpProjectBillingWorker.perform_async(token_in_session) + redirect_to action: 'check' end end end diff --git a/app/views/projects/clusters/new.html.haml b/app/views/projects/clusters/new.html.haml index 22014c49677..ddd13f8ea96 100644 --- a/app/views/projects/clusters/new.html.haml +++ b/app/views/projects/clusters/new.html.haml @@ -8,6 +8,6 @@ %h4.prepend-top-0= s_('ClusterIntegration|Choose how to set up cluster integration') %p= s_('ClusterIntegration|Create a new cluster on Google Kubernetes Engine right from GitLab') - = link_to s_('ClusterIntegration|Create on GKE'), gcp_check_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' + = link_to s_('ClusterIntegration|Create on GKE'), gcp_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' %p= s_('ClusterIntegration|Enter the details for an existing Kubernetes cluster') = link_to s_('ClusterIntegration|Add an existing cluster'), user_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index be4b8c1f8dc..852f3efe793 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -17,7 +17,6 @@ describe Projects::Clusters::GcpController do context 'when omniauth has been configured' do let(:key) { 'secret-key' } - let(:session_key_for_redirect_uri) do GoogleApi::CloudPlatform::Client.session_key_for_redirect_uri(key) end @@ -30,7 +29,7 @@ describe Projects::Clusters::GcpController do go expect(assigns(:authorize_url)).to include(key) - expect(session[session_key_for_redirect_uri]).to eq(gcp_check_project_clusters_path(project)) + expect(session[session_key_for_redirect_uri]).to eq(gcp_new_project_clusters_path(project)) end end @@ -72,47 +71,54 @@ describe Projects::Clusters::GcpController do end describe 'functionality' do - context 'when redis has wanted billing status' do - let(:token) { 'bogustoken' } + context 'when access token is valid' do before do - redis_double = double - allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) - allow(redis_double).to receive(:get).and_return('true') + stub_google_api_validate_token end - it 'should render json with billing status' do - go + context 'when redis has wanted billing status' do + let(:token) { 'bogustoken' } + + before do + redis_double = double + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + allow(redis_double).to receive(:get).and_return('true') + end - expect(response).to have_http_status(:ok) - expect(response.body).to include_json(billing: 'true') + it 'should render json with billing status' do + go + + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to include('billing' => 'true') + end end - it 'should not start worker' do - expect(CheckGcpProjectBillingWorker).not_to receive(:perform_async) + context 'when redis does not have billing status' do + before do + redis_double = double + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + allow(redis_double).to receive(:get).and_return(nil) + end + + it 'should render json with null billing status' do + go - go + expect(response).to have_http_status(:ok) + expect(JSON.parse(response.body)).to include('billing' => nil) + end end end - context 'when redis does not have billing status' do + context 'when access token is expired' do before do - redis_double = double - allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) - allow(redis_double).to receive(:get).and_return(nil) - end - - it 'should render json with null billing status' do - go - - expect(response).to have_http_status(:ok) - expect(response.body).to include_json(billing: nil) + stub_google_api_expired_token end - it 'should start worker' do - expect(CheckGcpProjectBillingWorker).to receive(:perform_async) + it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } + end - go - end + context 'when access token is not stored in session' do + it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } end end @@ -146,10 +152,36 @@ describe Projects::Clusters::GcpController do stub_google_api_validate_token end - it 'has new object' do - go + context 'when google project billing status is true' do + before do + stub_google_project_billing_status + end + + it 'has new object' do + go + + expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) + end + end + + context 'when google project billing status is not true' do + before do + redis_double = double + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + allow(redis_double).to receive(:get).and_return(nil) + end + + it 'redirects to check page' do + allow(CheckGcpProjectBillingWorker).to receive(:perform_async) + + expect(go).to redirect_to(gcp_check_project_clusters_path(project)) + end + + it 'calls gcp project billing check worker' do + expect(CheckGcpProjectBillingWorker).to receive(:perform_async) - expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) + go + end end end @@ -207,14 +239,40 @@ describe Projects::Clusters::GcpController do stub_google_api_validate_token end - context 'when creates a cluster on gke' do - it 'creates a new cluster' do - expect(ClusterProvisionWorker).to receive(:perform_async) - expect { go }.to change { Clusters::Cluster.count } - .and change { Clusters::Providers::Gcp.count } - expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) - expect(project.clusters.first).to be_gcp - expect(project.clusters.first).to be_kubernetes + context 'when google project billing status is true' do + before do + stub_google_project_billing_status + end + + context 'when creates a cluster on gke' do + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { go }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Gcp.count } + expect(response).to redirect_to(project_cluster_path(project, project.clusters.first)) + expect(project.clusters.first).to be_gcp + expect(project.clusters.first).to be_kubernetes + end + end + end + + context 'when google project billing status is not true' do + before do + redis_double = double + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + allow(redis_double).to receive(:get).and_return(nil) + end + + it 'redirects to check page' do + allow(CheckGcpProjectBillingWorker).to receive(:perform_async) + + expect(go).to redirect_to(gcp_check_project_clusters_path(project)) + end + + it 'calls gcp project billing check worker' do + expect(CheckGcpProjectBillingWorker).to receive(:perform_async) + + go end end end diff --git a/spec/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb index 8a073e58db8..9f9289fa580 100644 --- a/spec/support/google_api/cloud_platform_helpers.rb +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -10,6 +10,12 @@ module GoogleApi request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.ago.to_i.to_s end + def stub_google_project_billing_status + redis_double = double + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + allow(redis_double).to receive(:get).and_return('true') + 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))) diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index ce9632549b6..d7984ad0c1b 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -7,7 +7,7 @@ describe CheckGcpProjectBillingWorker do context 'when there is no lease' do before do - allow_any_instance_of(CheckGcpProjectBillingWorker).to receive(:try_obtain_lease_for).and_return('randomuuid') + allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return('randomuuid') end it 'calls the service' do @@ -21,7 +21,7 @@ describe CheckGcpProjectBillingWorker do expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return(true) expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) - expect(redis_double).to receive(:set).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for(token), anything) + expect(redis_double).to receive(:set).with(described_class.redis_shared_state_key_for(token), anything) subject end @@ -29,7 +29,7 @@ describe CheckGcpProjectBillingWorker do context 'when there is a lease' do before do - allow_any_instance_of(CheckGcpProjectBillingWorker).to receive(:try_obtain_lease_for).and_return(false) + allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return(false) end it 'does not call the service' do |