diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index d3b9d8a9bbc..25608df0b9c 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,6 +1,7 @@ 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: [:new] before_action :authorize_create_cluster!, only: [:new, :create] def login @@ -22,15 +23,20 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end def create - @cluster = ::Clusters::CreateService - .new(project, current_user, create_params) - .execute(token_in_session) + case google_project_billing_status + when 'true' + @cluster = ::Clusters::CreateService + .new(project, current_user, create_params) + .execute(token_in_session) - if @cluster.persisted? - redirect_to project_cluster_path(project, @cluster) + return redirect_to project_cluster_path(project, @cluster) if @cluster.persisted? + when 'false' + flash[:error] = _('Please enable billing for one of your projects to be able to create a cluster.') else - render :new + flash[:error] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.') end + + render :new end private @@ -58,6 +64,17 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end + def authorize_google_project_billing + redis_token_key = CheckGcpProjectBillingWorker.store_session_token(token_in_session) + CheckGcpProjectBillingWorker.perform_async(redis_token_key) + end + + def google_project_billing_status + Gitlab::Redis::SharedState.with do |redis| + redis.get(CheckGcpProjectBillingWorker.redis_shared_state_key_for(token_in_session)) + end + end + def token_in_session @token_in_session ||= session[GoogleApi::CloudPlatform::Client.session_key_for_token] diff --git a/app/services/check_gcp_project_billing_service.rb b/app/services/check_gcp_project_billing_service.rb new file mode 100644 index 00000000000..854adf2177d --- /dev/null +++ b/app/services/check_gcp_project_billing_service.rb @@ -0,0 +1,8 @@ +class CheckGcpProjectBillingService + def execute(token) + client = GoogleApi::CloudPlatform::Client.new(token, nil) + client.projects_list.select do |project| + client.projects_get_billing_info(project.name).billingEnabled + end + end +end diff --git a/app/views/projects/clusters/gcp/_header.html.haml b/app/views/projects/clusters/gcp/_header.html.haml index f23d5b80e4f..e2d7326a312 100644 --- a/app/views/projects/clusters/gcp/_header.html.haml +++ b/app/views/projects/clusters/gcp/_header.html.haml @@ -10,5 +10,5 @@ - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements } %li - - link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), target: '_blank', rel: 'noopener noreferrer') + - link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|This account must have permissions to create a cluster in the %{link_to_container_project} specified below').html_safe % { link_to_container_project: link_to_container_project } diff --git a/app/views/projects/clusters/gcp/login.html.haml b/app/views/projects/clusters/gcp/login.html.haml index e97ce01893a..878ebaded88 100644 --- a/app/views/projects/clusters/gcp/login.html.haml +++ b/app/views/projects/clusters/gcp/login.html.haml @@ -12,6 +12,8 @@ - if @authorize_url = link_to @authorize_url do = image_tag('auth_buttons/signin_with_google.png', width: '191px') + = _('or') + = link_to('create a new Google account', 'https://accounts.google.com/SignUpWithoutGmail?service=cloudconsole&continue=https%3A%2F%2Fconsole.cloud.google.com%2Ffreetrial%3Futm_campaign%3D2018_cpanel%26utm_source%3Dgitlab%26utm_medium%3Dreferral', target: '_blank', rel: 'noopener noreferrer') - else - link = link_to(s_('ClusterIntegration|properly configured'), help_page_path("integration/google"), target: '_blank', rel: 'noopener noreferrer') = s_('Google authentication is not %{link_to_documentation}. Ask your GitLab administrator if you want to use this service.').html_safe % { link_to_documentation: link } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index fafd9e5ef00..50e876b1d19 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -22,6 +22,7 @@ - gcp_cluster:cluster_provision - gcp_cluster:cluster_wait_for_app_installation - gcp_cluster:wait_for_cluster_creation +- gcp_cluster:check_gcp_project_billing - github_import_advance_stage - github_importer:github_import_import_diff_note diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb new file mode 100644 index 00000000000..557af14ee57 --- /dev/null +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -0,0 +1,59 @@ +require 'securerandom' + +class CheckGcpProjectBillingWorker + include ApplicationWorker + include ClusterQueue + + LEASE_TIMEOUT = 15.seconds.to_i + SESSION_KEY_TIMEOUT = 5.minutes + BILLING_TIMEOUT = 1.hour + + def self.get_session_token(token_key) + Gitlab::Redis::SharedState.with do |redis| + redis.get(get_redis_session_key(token_key)) + end + end + + def self.store_session_token(token) + generate_token_key.tap do |token_key| + Gitlab::Redis::SharedState.with do |redis| + redis.set(get_redis_session_key(token_key), token, ex: SESSION_KEY_TIMEOUT) + end + end + end + + def self.redis_shared_state_key_for(token) + "gitlab:gcp:#{token.hash}:billing_enabled" + end + + def perform(token_key) + return unless token_key + + token = self.get_session_token(token_key) + return unless token + return unless try_obtain_lease_for(token) + + billing_enabled_projects = CheckGcpProjectBillingService.new.execute(token) + Gitlab::Redis::SharedState.with do |redis| + redis.set(self.class.redis_shared_state_key_for(token), + !billing_enabled_projects.empty?, + ex: BILLING_TIMEOUT) + end + end + + private + + def self.generate_token_key + SecureRandom.uuid + end + + def self.get_redis_session_key(token_key) + "gitlab:gcp:session:#{token_key}" + end + + def try_obtain_lease_for(token) + Gitlab::ExclusiveLease + .new("check_gcp_project_billing_worker:#{token.hash}", timeout: LEASE_TIMEOUT) + .try_obtain + end +end diff --git a/changelogs/unreleased/39957-redirect-to-gpc-page-if-users-try-to-create-a-cluster-but-the-account-is-not-enabled.yml b/changelogs/unreleased/39957-redirect-to-gpc-page-if-users-try-to-create-a-cluster-but-the-account-is-not-enabled.yml new file mode 100644 index 00000000000..d8fd1f14bd4 --- /dev/null +++ b/changelogs/unreleased/39957-redirect-to-gpc-page-if-users-try-to-create-a-cluster-but-the-account-is-not-enabled.yml @@ -0,0 +1,5 @@ +--- +title: Implement checking GCP project billing status in cluster creation form. +merge_request: 15665 +author: +type: changed diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index b0563fb2d69..f05d001fd02 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -1,4 +1,6 @@ require 'google/apis/container_v1' +require 'google/apis/cloudbilling_v1' +require 'google/apis/cloudresourcemanager_v1' module GoogleApi module CloudPlatform @@ -40,6 +42,22 @@ module GoogleApi true end + def projects_list + service = Google::Apis::CloudresourcemanagerV1::CloudResourceManagerService.new + service.authorization = access_token + + service.fetch_all(items: :projects) do |token| + service.list_projects(page_token: token) + end + end + + def projects_get_billing_info(project_name) + service = Google::Apis::CloudbillingV1::CloudbillingService.new + service.authorization = access_token + + service.get_project_billing_info("projects/#{project_name}") + end + def projects_zones_clusters_get(project_id, zone, cluster_id) service = Google::Apis::ContainerV1::ContainerService.new service.authorization = access_token diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index ee7928beb7e..be19fa93183 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 @@ -78,6 +77,8 @@ describe Projects::Clusters::GcpController do end it 'has new object' do + expect(controller).to receive(:authorize_google_project_billing) + go expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) @@ -138,7 +139,11 @@ describe Projects::Clusters::GcpController do stub_google_api_validate_token end - context 'when creates a cluster on gke' do + context 'when google project billing is enabled' do + before do + stub_google_project_billing_status + end + it 'creates a new cluster' do expect(ClusterProvisionWorker).to receive(:perform_async) expect { go }.to change { Clusters::Cluster.count } @@ -148,6 +153,15 @@ describe Projects::Clusters::GcpController do expect(project.clusters.first).to be_kubernetes end end + + context 'when google project billing is not enabled' do + it 'renders the cluster form with an error' do + go + + expect(response).to set_flash[:error] + expect(response).to render_template('new') + end + end end context 'when access token is expired' do diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 882a2756b72..523cc08496b 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -20,7 +20,116 @@ feature 'Gcp Cluster', :js do .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) end - context 'when user does not have a cluster and visits cluster index page' do + context 'when user has a GCP project with billing enabled' do + before do + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) + stub_google_project_billing_status + end + + context 'when user does not have a cluster and visits cluster index page' do + before do + visit project_clusters_path(project) + + click_link 'Add cluster' + click_link 'Create on GKE' + end + + context 'when user filled form with valid parameters' do + before do + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_zones_clusters_create) do + OpenStruct.new( + self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123', + status: 'RUNNING' + ) + end + + allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) + + fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' + fill_in 'cluster_name', with: 'dev-cluster' + click_button 'Create cluster' + end + + it 'user sees a cluster details page and creation status' do + expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') + + Clusters::Cluster.last.provider.make_created! + + expect(page).to have_content('Cluster was successfully created on Google Kubernetes Engine') + end + + it 'user sees a error if something worng during creation' do + expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') + + Clusters::Cluster.last.provider.make_errored!('Something wrong!') + + expect(page).to have_content('Something wrong!') + end + end + + context 'when user filled form with invalid parameters' do + before do + click_button 'Create cluster' + end + + it 'user sees a validation error' do + expect(page).to have_css('#error_explanation') + end + end + end + + context 'when user does have a cluster and visits cluster page' do + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + + before do + visit project_cluster_path(project, cluster) + end + + it 'user sees a cluster details page' do + expect(page).to have_button('Save changes') + expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) + end + + context 'when user disables the cluster' do + before do + page.find(:css, '.js-toggle-cluster').click + page.within('#cluster-integration') { click_button 'Save changes' } + end + + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + end + end + + context 'when user changes cluster parameters' do + before do + fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' + page.within('#js-cluster-details') { click_button 'Save changes' } + end + + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + expect(cluster.reload.platform_kubernetes.namespace).to eq('my-namespace') + end + end + + context 'when user destroy the cluster' do + before do + page.accept_confirm do + click_link 'Remove integration' + end + end + + it 'user sees creation form with the successful message' do + expect(page).to have_content('Cluster integration was successfully removed.') + expect(page).to have_link('Add cluster') + end + end + end + end + + context 'when user does not have a GCP project with billing enabled' do before do visit project_clusters_path(project) @@ -28,97 +137,9 @@ feature 'Gcp Cluster', :js do click_link 'Create on GKE' end - context 'when user filled form with valid parameters' do - before do - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_create) do - OpenStruct.new( - self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123', - status: 'RUNNING' - ) - end - - allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) - - fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' - fill_in 'cluster_name', with: 'dev-cluster' - click_button 'Create cluster' - end - - it 'user sees a cluster details page and creation status' do - expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') - - Clusters::Cluster.last.provider.make_created! - - expect(page).to have_content('Cluster was successfully created on Google Kubernetes Engine') - end - - it 'user sees a error if something worng during creation' do - expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') - - Clusters::Cluster.last.provider.make_errored!('Something wrong!') - - expect(page).to have_content('Something wrong!') - end - end - - context 'when user filled form with invalid parameters' do - before do - click_button 'Create cluster' - end - - it 'user sees a validation error' do - expect(page).to have_css('#error_explanation') - end - end - end - - context 'when user does have a cluster and visits cluster page' do - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } - - before do - visit project_cluster_path(project, cluster) - end - - it 'user sees a cluster details page' do - expect(page).to have_button('Save changes') - expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) - end - - context 'when user disables the cluster' do - before do - page.find(:css, '.js-toggle-cluster').click - page.within('#cluster-integration') { click_button 'Save changes' } - end - - it 'user sees the successful message' do - expect(page).to have_content('Cluster was successfully updated.') - end - end - - context 'when user changes cluster parameters' do - before do - fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' - page.within('#js-cluster-details') { click_button 'Save changes' } - end - - it 'user sees the successful message' do - expect(page).to have_content('Cluster was successfully updated.') - expect(cluster.reload.platform_kubernetes.namespace).to eq('my-namespace') - end - end - - context 'when user destroy the cluster' do - before do - page.accept_confirm do - click_link 'Remove integration' - end - end - - it 'user sees creation form with the successful message' do - expect(page).to have_content('Cluster integration was successfully removed.') - expect(page).to have_link('Add cluster') - end + it 'user sees a check page' do + pending 'the frontend still has not been implemented' + expect(page).to have_link('Continue') end end end diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 93929bf6814..eae2910a8f6 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -77,4 +77,18 @@ feature 'Clusters', :js do end end end + + context 'when user has not signed in Google' do + before do + visit project_clusters_path(project) + + click_link 'Add cluster' + click_link 'Create on GKE' + end + + it 'user sees a login page' do + expect(page).to have_css('.signin-with-google') + expect(page).to have_link('Google account') + end + end end diff --git a/spec/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb index ecb4034ec8b..f65e41dfea3 100644 --- a/spec/lib/google_api/cloud_platform/client_spec.rb +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -50,6 +50,30 @@ describe GoogleApi::CloudPlatform::Client do end end + describe '#projects_list' do + subject { client.projects_list } + let(:projects) { double } + + before do + allow_any_instance_of(Google::Apis::CloudresourcemanagerV1::CloudResourceManagerService) + .to receive(:fetch_all).and_return(projects) + end + + it { is_expected.to eq(projects) } + end + + describe '#projects_get_billing_info' do + subject { client.projects_get_billing_info('project') } + let(:billing_info) { double } + + before do + allow_any_instance_of(Google::Apis::CloudbillingV1::CloudbillingService) + .to receive(:get_project_billing_info).and_return(billing_info) + end + + it { is_expected.to eq(billing_info) } + end + describe '#projects_zones_clusters_get' do subject { client.projects_zones_clusters_get(spy, spy, spy) } let(:gke_cluster) { double } diff --git a/spec/services/check_gcp_project_billing_service_spec.rb b/spec/services/check_gcp_project_billing_service_spec.rb new file mode 100644 index 00000000000..f0e39ba6f49 --- /dev/null +++ b/spec/services/check_gcp_project_billing_service_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe CheckGcpProjectBillingService do + let(:service) { described_class.new } + let(:projects) { [double(name: 'first_project'), double(name: 'second_project')] } + + describe '#execute' do + before do + expect_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_list).and_return(projects) + + allow_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive_message_chain(:projects_get_billing_info, :billingEnabled) + .and_return(project_billing_enabled) + end + + subject { service.execute('bogustoken') } + + context 'google account has a billing enabled gcp project' do + let(:project_billing_enabled) { true } + + it { is_expected.to eq(projects) } + end + + context 'google account does not have a billing enabled gcp project' do + let(:project_billing_enabled) { false } + + it { is_expected.to eq([]) } + 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..99752ed396e 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).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for('token')).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 new file mode 100644 index 00000000000..f52a903327c --- /dev/null +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -0,0 +1,61 @@ +require 'spec_helper' + +describe CheckGcpProjectBillingWorker do + describe '.perform' do + let(:token) { 'bogustoken' } + + subject { described_class.new.perform('token_key') } + + context 'when there is a token in redis' do + before do + allow_any_instance_of(described_class).to receive(:get_session_token).and_return(token) + end + + context 'when there is no lease' do + before do + allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return('randomuuid') + end + + it 'calls the service' do + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return([double]) + + subject + end + + it 'stores billing status in redis' do + redis_double = double + + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return([double]) + expect(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + expect(redis_double).to receive(:set).with(described_class.redis_shared_state_key_for(token), anything, anything) + + subject + end + end + + context 'when there is a lease' do + before do + allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return(false) + end + + it 'does not call the service' do + expect(CheckGcpProjectBillingService).not_to receive(:new) + + subject + end + end + end + + context 'when there is no token in redis' do + before do + allow_any_instance_of(described_class).to receive(:get_session_token).and_return(nil) + end + + it 'does not call the service' do + expect(CheckGcpProjectBillingService).not_to receive(:new) + + subject + end + end + end +end