diff options
-rw-r--r-- | app/controllers/projects/clusters/gcp_controller.rb | 31 | ||||
-rw-r--r-- | app/services/check_gcp_project_billing_service.rb | 5 | ||||
-rw-r--r-- | app/views/projects/clusters/gcp/_header.html.haml | 6 | ||||
-rw-r--r-- | app/views/projects/clusters/show.html.haml | 2 | ||||
-rw-r--r-- | app/workers/check_gcp_project_billing_worker.rb | 6 | ||||
-rw-r--r-- | doc/user/project/clusters/index.md | 13 | ||||
-rw-r--r-- | lib/google_api/cloud_platform/client.rb | 6 | ||||
-rw-r--r-- | spec/controllers/projects/clusters/gcp_controller_spec.rb | 7 | ||||
-rw-r--r-- | spec/features/projects/clusters/gcp_spec.rb | 36 | ||||
-rw-r--r-- | spec/services/check_gcp_project_billing_service_spec.rb | 21 | ||||
-rw-r--r-- | spec/support/google_api/cloud_platform_helpers.rb | 47 | ||||
-rw-r--r-- | spec/workers/check_gcp_project_billing_worker_spec.rb | 2 |
12 files changed, 135 insertions, 47 deletions
diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 25608df0b9c..4fc515bd03e 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,8 +1,9 @@ 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_google_project_billing, only: [:new, :create] before_action :authorize_create_cluster!, only: [:new, :create] + before_action :verify_billing, only: [:create] def login begin @@ -23,24 +24,34 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end def create + @cluster = ::Clusters::CreateService + .new(project, current_user, create_params) + .execute(token_in_session) + + if @cluster.persisted? + redirect_to project_cluster_path(project, @cluster) + else + render :new + end + end + + private + + def verify_billing case google_project_billing_status when 'true' - @cluster = ::Clusters::CreateService - .new(project, current_user, create_params) - .execute(token_in_session) - - return redirect_to project_cluster_path(project, @cluster) if @cluster.persisted? + return when 'false' - flash[:error] = _('Please enable billing for one of your projects to be able to create a cluster.') + flash[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" } else - flash[:error] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.') + flash[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.') end + @cluster = ::Clusters::Cluster.new(create_params) + render :new end - private - def create_params params.require(:cluster).permit( :enabled, diff --git a/app/services/check_gcp_project_billing_service.rb b/app/services/check_gcp_project_billing_service.rb index 854adf2177d..ea82b61b279 100644 --- a/app/services/check_gcp_project_billing_service.rb +++ b/app/services/check_gcp_project_billing_service.rb @@ -2,7 +2,10 @@ 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 + begin + client.projects_get_billing_info(project.project_id).billing_enabled + rescue + end end end end diff --git a/app/views/projects/clusters/gcp/_header.html.haml b/app/views/projects/clusters/gcp/_header.html.haml index e2d7326a312..bddb902115d 100644 --- a/app/views/projects/clusters/gcp/_header.html.haml +++ b/app/views/projects/clusters/gcp/_header.html.haml @@ -4,11 +4,11 @@ = s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:') %ul %li - - link_to_kubernetes_engine = link_to(s_('ClusterIntegration|access to Google Kubernetes Engine'), 'https://console.cloud.google.com', target: '_blank', rel: 'noopener noreferrer') + - link_to_kubernetes_engine = link_to(s_('ClusterIntegration|access to Google Kubernetes Engine'), 'https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|Your account must have %{link_to_kubernetes_engine}').html_safe % { link_to_kubernetes_engine: link_to_kubernetes_engine } %li - - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer') + - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', 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'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') + - link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), 'https://console.cloud.google.com/home/dashboard?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', 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/show.html.haml b/app/views/projects/clusters/show.html.haml index c7c84b5a42c..2049105dff6 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -1,6 +1,6 @@ - @content_class = "limit-container-width" unless fluid_layout - add_to_breadcrumbs "Clusters", project_clusters_path(@project) -- breadcrumb_title @cluster.id +- breadcrumb_title @cluster.name - page_title _("Cluster") - expanded = Rails.env.test? diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 557af14ee57..5466ccdda59 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -4,7 +4,7 @@ class CheckGcpProjectBillingWorker include ApplicationWorker include ClusterQueue - LEASE_TIMEOUT = 15.seconds.to_i + LEASE_TIMEOUT = 3.seconds.to_i SESSION_KEY_TIMEOUT = 5.minutes BILLING_TIMEOUT = 1.hour @@ -23,13 +23,13 @@ class CheckGcpProjectBillingWorker end def self.redis_shared_state_key_for(token) - "gitlab:gcp:#{token.hash}:billing_enabled" + "gitlab:gcp:#{Digest::SHA1.hexdigest(token)}:billing_enabled" end def perform(token_key) return unless token_key - token = self.get_session_token(token_key) + token = self.class.get_session_token(token_key) return unless token return unless try_obtain_lease_for(token) diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index d5619c7b563..218b9dee41d 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -25,11 +25,14 @@ prerequisites must be met: be enabled in GitLab at the instance level. If that's not the case, ask your administrator to enable it. - Your associated Google account must have the right privileges to manage - clusters on GKE. That would mean that a - [billing account](https://cloud.google.com/billing/docs/how-to/manage-billing-account) - must be set up. -- You must have Master [permissions] in order to be able to access the **Cluster** - page. + clusters on GKE. That would mean that a [billing + account](https://cloud.google.com/billing/docs/how-to/manage-billing-account) + must be set up and that you have to have permissions to access it. +- You must have Master [permissions] in order to be able to access the + **Cluster** page. +- You must have [Cloud Billing API](https://cloud.google.com/billing/) enabled +- You must have [Resource Manager + API](https://cloud.google.com/resource-manager/) If all of the above requirements are met, you can proceed to add a new GKE cluster. diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index f05d001fd02..ff638c07755 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -47,15 +47,15 @@ module GoogleApi service.authorization = access_token service.fetch_all(items: :projects) do |token| - service.list_projects(page_token: token) + service.list_projects(page_token: token, options: user_agent_header) end end - def projects_get_billing_info(project_name) + def projects_get_billing_info(project_id) service = Google::Apis::CloudbillingV1::CloudbillingService.new service.authorization = access_token - service.get_project_billing_info("projects/#{project_name}") + service.get_project_billing_info("projects/#{project_id}", options: user_agent_header) end def projects_zones_clusters_get(project_id, zone, cluster_id) diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index be19fa93183..775f9db1c6e 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -137,11 +137,14 @@ describe Projects::Clusters::GcpController do context 'when access token is valid' do before do stub_google_api_validate_token + allow_any_instance_of(described_class).to receive(:authorize_google_project_billing) end context 'when google project billing is enabled' do before do - 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 it 'creates a new cluster' do @@ -158,7 +161,7 @@ describe Projects::Clusters::GcpController do it 'renders the cluster form with an error' do go - expect(response).to set_flash[:error] + expect(response).to set_flash[:alert] expect(response).to render_template('new') end end diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 523cc08496b..8953b30bebf 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -13,6 +13,8 @@ feature 'Gcp Cluster', :js do end context 'when user has signed with Google' do + let(:project_id) { 'test-project-1234' } + before do allow_any_instance_of(Projects::Clusters::GcpController) .to receive(:token_in_session).and_return('token') @@ -23,7 +25,7 @@ feature 'Gcp Cluster', :js 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 + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return('true') end context 'when user does not have a cluster and visits cluster index page' do @@ -131,15 +133,41 @@ feature 'Gcp Cluster', :js do context 'when user does not have a GCP project with billing enabled' do before do + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return('false') + visit project_clusters_path(project) click_link 'Add cluster' click_link 'Create on GKE' + + 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 form with error' do + expect(page).to have_content('Please enable billing for one of your projects to be able to create a cluster, then try again.') + end + end + + context 'when gcp billing status is not in redis' do + before do + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return(nil) + + visit project_clusters_path(project) + + click_link 'Add cluster' + click_link 'Create on GKE' + + 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 check page' do - pending 'the frontend still has not been implemented' - expect(page).to have_link('Continue') + it 'user sees form with error' do + expect(page).to have_content('We could not verify that one of your projects on GCP has billing enabled. Please try again.') end end end diff --git a/spec/services/check_gcp_project_billing_service_spec.rb b/spec/services/check_gcp_project_billing_service_spec.rb index f0e39ba6f49..3e68d906e71 100644 --- a/spec/services/check_gcp_project_billing_service_spec.rb +++ b/spec/services/check_gcp_project_billing_service_spec.rb @@ -1,29 +1,30 @@ require 'spec_helper' describe CheckGcpProjectBillingService do + include GoogleApi::CloudPlatformHelpers + let(:service) { described_class.new } - let(:projects) { [double(name: 'first_project'), double(name: 'second_project')] } + let(:project_id) { 'test-project-1234' } 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) + stub_cloud_platform_projects_list(project_id: project_id) end subject { service.execute('bogustoken') } context 'google account has a billing enabled gcp project' do - let(:project_billing_enabled) { true } + before do + stub_cloud_platform_projects_get_billing_info(project_id, true) + end - it { is_expected.to eq(projects) } + it { is_expected.to all(satisfy { |project| project.project_id == project_id }) } end context 'google account does not have a billing enabled gcp project' do - let(:project_billing_enabled) { false } + before do + stub_cloud_platform_projects_get_billing_info(project_id, false) + end it { is_expected.to eq([]) } end diff --git a/spec/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb index 99752ed396e..2fdbddd40c2 100644 --- a/spec/support/google_api/cloud_platform_helpers.rb +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -10,10 +10,14 @@ 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') + def stub_cloud_platform_projects_list(options) + WebMock.stub_request(:get, cloud_platform_projects_list_url) + .to_return(cloud_platform_response(cloud_platform_projects_body(options))) + end + + def stub_cloud_platform_projects_get_billing_info(project_id, billing_enabled) + WebMock.stub_request(:get, cloud_platform_projects_get_billing_info_url(project_id)) + .to_return(cloud_platform_response(cloud_platform_projects_billing_info_body(project_id, billing_enabled))) end def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options) @@ -46,6 +50,14 @@ module GoogleApi .to_return(status: [500, "Internal Server Error"]) end + def cloud_platform_projects_list_url + "https://cloudresourcemanager.googleapis.com/v1/projects" + end + + def cloud_platform_projects_get_billing_info_url(project_id) + "https://cloudbilling.googleapis.com/v1/projects/#{project_id}/billingInfo" + end + def cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id) "https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/clusters/#{cluster_id}" end @@ -121,5 +133,32 @@ module GoogleApi "endTime": options[:endTime] || '' } end + + def cloud_platform_projects_body(**options) + { + "projects": [ + { + "projectNumber": options[:project_number] || "1234", + "projectId": options[:project_id] || "test-project-1234", + "lifecycleState": "ACTIVE", + "name": options[:name] || "test-project", + "createTime": "2017-12-16T01:48:29.129Z", + "parent": { + "type": "organization", + "id": "12345" + } + } + ] + } + end + + def cloud_platform_projects_billing_info_body(project_id, billing_enabled) + { + "name": "projects/#{project_id}/billingInfo", + "projectId": "#{project_id}", + "billingAccountName": "account-name", + "billingEnabled": billing_enabled + } + end end end diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index f52a903327c..7b7a7c1bc44 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -8,7 +8,7 @@ describe CheckGcpProjectBillingWorker do 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) + allow(described_class).to receive(:get_session_token).and_return(token) end context 'when there is no lease' do |