From 74bdaae6cb70f978f64792e7435621e961dd5e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 30 Nov 2017 18:09:32 +0100 Subject: Add link to create Google account in clusters page --- app/views/projects/clusters/login.html.haml | 2 ++ spec/features/projects/clusters_spec.rb | 1 + 2 files changed, 3 insertions(+) diff --git a/app/views/projects/clusters/login.html.haml b/app/views/projects/clusters/login.html.haml index fde030b500b..e4bc58b1fab 100644 --- a/app/views/projects/clusters/login.html.haml +++ b/app/views/projects/clusters/login.html.haml @@ -11,6 +11,8 @@ - if @authorize_url = link_to @authorize_url do = image_tag('auth_buttons/signin_with_google.png', width: '191px') + = s_('ClusterIntegration|or create a new') + = link_to('Google account', 'https://accounts.google.com/signup', 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/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index 197e6df4997..018758d1bf1 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -201,6 +201,7 @@ feature 'Clusters', :js do 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 -- cgit v1.2.1 From eb9f88311b0591586c73ddb35bdb07677464748c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 10 Dec 2017 18:10:51 +0100 Subject: Use special new account link * Redirects to GCP free trial signup afterwards * Adds GitLab referral info --- app/views/projects/clusters/login.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/clusters/login.html.haml b/app/views/projects/clusters/login.html.haml index e4bc58b1fab..521121111b0 100644 --- a/app/views/projects/clusters/login.html.haml +++ b/app/views/projects/clusters/login.html.haml @@ -12,7 +12,7 @@ = link_to @authorize_url do = image_tag('auth_buttons/signin_with_google.png', width: '191px') = s_('ClusterIntegration|or create a new') - = link_to('Google account', 'https://accounts.google.com/signup', target: '_blank', rel: 'noopener noreferrer') + = link_to('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 } -- cgit v1.2.1 From 497a0cd62ca8dff2e7b91f3366b69f4f2b2fa826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 13 Dec 2017 15:25:42 +0100 Subject: Fix cluster OAuth feature spec user flow --- spec/features/projects/clusters_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/projects/clusters_spec.rb b/spec/features/projects/clusters_spec.rb index dad943aac55..eae2910a8f6 100644 --- a/spec/features/projects/clusters_spec.rb +++ b/spec/features/projects/clusters_spec.rb @@ -82,6 +82,7 @@ feature 'Clusters', :js do before do visit project_clusters_path(project) + click_link 'Add cluster' click_link 'Create on GKE' end -- cgit v1.2.1 From 596ea9e3688537131fb918c1e9177d085a938d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 15 Dec 2017 19:39:15 +0100 Subject: Add Google Cloud client project list --- lib/google_api/cloud_platform/client.rb | 10 ++++++++++ spec/lib/google_api/cloud_platform/client_spec.rb | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index b0563fb2d69..f0dabcb1c5d 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -1,4 +1,5 @@ require 'google/apis/container_v1' +require 'google/apis/cloudresourcemanager_v1' module GoogleApi module CloudPlatform @@ -40,6 +41,15 @@ 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_zones_clusters_get(project_id, zone, cluster_id) service = Google::Apis::ContainerV1::ContainerService.new service.authorization = access_token diff --git a/spec/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb index ecb4034ec8b..0383b1080dc 100644 --- a/spec/lib/google_api/cloud_platform/client_spec.rb +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -50,6 +50,18 @@ 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_zones_clusters_get' do subject { client.projects_zones_clusters_get(spy, spy, spy) } let(:gke_cluster) { double } -- cgit v1.2.1 From 957bedb1c345644f633fdfd3440491810c5cb75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 15 Dec 2017 23:54:50 +0100 Subject: Add Google Cloud client project billing info --- lib/google_api/cloud_platform/client.rb | 8 ++++++++ spec/lib/google_api/cloud_platform/client_spec.rb | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index f0dabcb1c5d..c107d001bec 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -1,4 +1,5 @@ require 'google/apis/container_v1' +require 'google/apis/cloudbilling_v1' require 'google/apis/cloudresourcemanager_v1' module GoogleApi @@ -50,6 +51,13 @@ module GoogleApi end end + def projects_get_billing_info(project_name) + service = Google::Apis::CloudbillingV1::CloudbillingService.new + service.authorization = access_token + + service.get_project_billing_info(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/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb index 0383b1080dc..2d14c3b1a3a 100644 --- a/spec/lib/google_api/cloud_platform/client_spec.rb +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -62,6 +62,18 @@ describe GoogleApi::CloudPlatform::Client do it { is_expected.to eq(projects) } end + def projects_get_billing_info + subject { client.projects_get_billing_info } + 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 } -- cgit v1.2.1 From 84d8ca1171faf8ab6df0c256381b8afbf5aeb099 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 00:05:46 +0100 Subject: Change link for creating a new Google account --- app/views/projects/clusters/gcp/login.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/projects/clusters/gcp/login.html.haml b/app/views/projects/clusters/gcp/login.html.haml index 5ef80ccf8f7..878ebaded88 100644 --- a/app/views/projects/clusters/gcp/login.html.haml +++ b/app/views/projects/clusters/gcp/login.html.haml @@ -12,8 +12,8 @@ - if @authorize_url = link_to @authorize_url do = image_tag('auth_buttons/signin_with_google.png', width: '191px') - = s_('ClusterIntegration|or create a new') - = link_to('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') + = _('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 } -- cgit v1.2.1 From 87f01506cb0f0ba9ee2d7153157b0a07d628a559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 01:44:36 +0100 Subject: Add CheckGCPProjectBillingService --- app/services/check_gcp_project_billing_service.rb | 8 ++++++ .../check_gcp_project_billing_service_spec.rb | 30 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 app/services/check_gcp_project_billing_service.rb create mode 100644 spec/services/check_gcp_project_billing_service_spec.rb 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..45192a167eb --- /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.any? do |project| + client.projects_get_billing_info(project.name).billingEnabled + 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 new file mode 100644 index 00000000000..b7c42fcace1 --- /dev/null +++ b/spec/services/check_gcp_project_billing_service_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe CheckGCPProjectBillingService do + let(:service) { described_class.new } + + describe '#execute' do + before do + expect_any_instance_of(GoogleApi::CloudPlatform::Client) + .to receive(:projects_list).and_return([double(name: 'project_name')]) + + expect_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(true) } + end + + context 'google account does not have a billing enabled gcp project' do + let(:project_billing_enabled) { false } + + it { is_expected.to eq(false) } + end + end +end -- cgit v1.2.1 From 291480f5e17fea424692f979db91d2ec62d24dbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 02:12:44 +0100 Subject: Properly CamelCase service name --- app/services/check_gcp_project_billing_service.rb | 2 +- spec/lib/google_api/cloud_platform/client_spec.rb | 4 ++-- spec/services/check_gcp_project_billing_service_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/services/check_gcp_project_billing_service.rb b/app/services/check_gcp_project_billing_service.rb index 45192a167eb..88e9d7de8de 100644 --- a/app/services/check_gcp_project_billing_service.rb +++ b/app/services/check_gcp_project_billing_service.rb @@ -1,4 +1,4 @@ -class CheckGCPProjectBillingService +class CheckGcpProjectBillingService def execute(token) client = GoogleApi::CloudPlatform::Client.new(token, nil) client.projects_list.any? do |project| diff --git a/spec/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb index 2d14c3b1a3a..f65e41dfea3 100644 --- a/spec/lib/google_api/cloud_platform/client_spec.rb +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -62,8 +62,8 @@ describe GoogleApi::CloudPlatform::Client do it { is_expected.to eq(projects) } end - def projects_get_billing_info - subject { client.projects_get_billing_info } + describe '#projects_get_billing_info' do + subject { client.projects_get_billing_info('project') } let(:billing_info) { double } before do diff --git a/spec/services/check_gcp_project_billing_service_spec.rb b/spec/services/check_gcp_project_billing_service_spec.rb index b7c42fcace1..1b23b43b0d5 100644 --- a/spec/services/check_gcp_project_billing_service_spec.rb +++ b/spec/services/check_gcp_project_billing_service_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe CheckGCPProjectBillingService do +describe CheckGcpProjectBillingService do let(:service) { described_class.new } describe '#execute' do -- cgit v1.2.1 From 68b95cd01e674cd2dbce45c49f5be04c223b718d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 02:39:55 +0100 Subject: Add CheckGcpProjectBillingWorker --- app/workers/check_gcp_project_billing_worker.rb | 16 +++++++++++++++ .../check_gcp_project_billing_worker_spec.rb | 23 ++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 app/workers/check_gcp_project_billing_worker.rb create mode 100644 spec/workers/check_gcp_project_billing_worker_spec.rb 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..97638f65e8d --- /dev/null +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -0,0 +1,16 @@ +class CheckGcpProjectBillingWorker + include ApplicationWorker + + def self.redis_shared_state_key_for(token) + "gitlab:gcp:#{token}:billing_enabled" + end + + def perform(token) + return unless token + + billing_enabled = CheckGcpProjectBillingService.new.execute(token) + Gitlab::Redis::SharedState.with do |redis| + redis.set(self.class.redis_shared_state_key_for(token), 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 new file mode 100644 index 00000000000..c6e624f65be --- /dev/null +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe CheckGcpProjectBillingWorker do + describe '.perform' do + let(:token) { 'bogustoken' } + subject { described_class.new.perform(token) } + + it 'calls the service' do + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute) + + subject + end + + it 'stores billing status in redis' do + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return(true) + subject + + Gitlab::Redis::SharedState.with do |redis| + expect(redis.get("gitlab:gcp:#{token}:billing_enabled")).to eq('true') + end + end + end +end -- cgit v1.2.1 From 1de0261d5ec9385405291426f56b190148707700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 04:00:54 +0100 Subject: Isolate CheckGcpProjectBillingWorker specreturns --- spec/workers/check_gcp_project_billing_worker_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index c6e624f65be..70738f36324 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -12,12 +12,13 @@ describe CheckGcpProjectBillingWorker do end it 'stores billing status in redis' do + redis_double = double + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return(true) - subject + 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) - Gitlab::Redis::SharedState.with do |redis| - expect(redis.get("gitlab:gcp:#{token}:billing_enabled")).to eq('true') - end + subject end end end -- cgit v1.2.1 From 78f85f3fd3a6743948f044c332cd1243547ef0a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 04:11:44 +0100 Subject: Add check step for creating GCP clusters --- .../projects/clusters/gcp_controller.rb | 25 +++++++- app/views/projects/clusters/gcp/check.html.haml | 1 + app/views/projects/clusters/new.html.haml | 2 +- config/routes/project.rb | 1 + .../projects/clusters/gcp_controller_spec.rb | 71 +++++++++++++++++++++- 5 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 app/views/projects/clusters/gcp/check.html.haml diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index b64f7a2a6bd..558dae8e228 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,11 +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_create_cluster!, only: [:new, :create] + STATUS_POLLING_INTERVAL = 10_000 + def login begin - state = generate_session_key_redirect(gcp_new_namespace_project_clusters_path.to_s) + state = generate_session_key_redirect(gcp_check_namespace_project_clusters_path.to_s) @authorize_url = GoogleApi::CloudPlatform::Client.new( nil, callback_google_api_auth_url, @@ -15,6 +18,18 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end + def check + respond_to do |format| + format.json do + Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL) + + Gitlab::Redis::SharedState.with do |redis| + render json: { billing: redis.get(CheckGcpProjectBillingWorker.redis_shared_state_key_for(token_in_session)) } + end + end + end + end + def new @cluster = ::Clusters::Cluster.new.tap do |cluster| cluster.build_provider_gcp @@ -57,6 +72,14 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end + def authorize_google_project_billing + 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) + end + end + end + def token_in_session @token_in_session ||= session[GoogleApi::CloudPlatform::Client.session_key_for_token] diff --git a/app/views/projects/clusters/gcp/check.html.haml b/app/views/projects/clusters/gcp/check.html.haml new file mode 100644 index 00000000000..e965047ad7c --- /dev/null +++ b/app/views/projects/clusters/gcp/check.html.haml @@ -0,0 +1 @@ +Hello diff --git a/app/views/projects/clusters/new.html.haml b/app/views/projects/clusters/new.html.haml index ddd13f8ea96..22014c49677 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_new_namespace_project_clusters_path(@project.namespace, @project), class: 'btn append-bottom-20' + = link_to s_('ClusterIntegration|Create on GKE'), gcp_check_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/config/routes/project.rb b/config/routes/project.rb index 093da10f57f..26eb4fbeda3 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -189,6 +189,7 @@ constraints(ProjectUrlConstrainer.new) do get '/user/new', to: 'clusters/user#new' post '/user', to: 'clusters/user#create' + get '/gcp/check', to: 'clusters/gcp#check' get '/gcp/new', to: 'clusters/gcp#new' get '/gcp/login', to: 'clusters/gcp#login' post '/gcp', to: 'clusters/gcp#create' diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index ee7928beb7e..be4b8c1f8dc 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -30,7 +30,7 @@ describe Projects::Clusters::GcpController do go expect(assigns(:authorize_url)).to include(key) - expect(session[session_key_for_redirect_uri]).to eq(gcp_new_project_clusters_path(project)) + expect(session[session_key_for_redirect_uri]).to eq(gcp_check_project_clusters_path(project)) end end @@ -63,6 +63,75 @@ describe Projects::Clusters::GcpController do end end + describe 'GET check' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + describe 'functionality' do + 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 + + it 'should render json with billing status' do + go + + expect(response).to have_http_status(:ok) + expect(response.body).to include_json(billing: 'true') + end + + it 'should not start worker' do + expect(CheckGcpProjectBillingWorker).not_to receive(:perform_async) + + go + end + end + + 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 + + expect(response).to have_http_status(:ok) + expect(response.body).to include_json(billing: nil) + end + + it 'should start worker' do + expect(CheckGcpProjectBillingWorker).to receive(:perform_async) + + go + end + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + get :check, namespace_id: project.namespace, project_id: project, format: :json + end + end + describe 'GET new' do describe 'functionality' do let(:user) { create(:user) } -- cgit v1.2.1 From 99043d244c4d579f27382f003df9e3243287df2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 04:21:13 +0100 Subject: Add lease to CheckGcpProjectBillingWorker --- app/workers/check_gcp_project_billing_worker.rb | 11 +++++++ .../check_gcp_project_billing_worker_spec.rb | 36 ++++++++++++++++------ 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 97638f65e8d..254b0959063 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -1,16 +1,27 @@ class CheckGcpProjectBillingWorker include ApplicationWorker + LEASE_TIMEOUT = 1.minute.to_i + def self.redis_shared_state_key_for(token) "gitlab:gcp:#{token}:billing_enabled" end def perform(token) return unless token + return unless try_obtain_lease_for(token) billing_enabled = CheckGcpProjectBillingService.new.execute(token) Gitlab::Redis::SharedState.with do |redis| redis.set(self.class.redis_shared_state_key_for(token), billing_enabled) end end + + private + + def try_obtain_lease_for(token) + Gitlab::ExclusiveLease + .new("check_gcp_project_billing_worker:#{token}", timeout: LEASE_TIMEOUT) + .try_obtain + 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 70738f36324..ce9632549b6 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -5,20 +5,38 @@ describe CheckGcpProjectBillingWorker do let(:token) { 'bogustoken' } subject { described_class.new.perform(token) } - it 'calls the service' do - expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute) + context 'when there is no lease' do + before do + allow_any_instance_of(CheckGcpProjectBillingWorker).to receive(:try_obtain_lease_for).and_return('randomuuid') + end - subject + it 'calls the service' do + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute) + + subject + end + + it 'stores billing status in redis' do + redis_double = double + + 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) + + subject + end end - it 'stores billing status in redis' do - redis_double = double + context 'when there is a lease' do + before do + allow_any_instance_of(CheckGcpProjectBillingWorker).to receive(:try_obtain_lease_for).and_return(false) + end - 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) + it 'does not call the service' do + expect(CheckGcpProjectBillingService).not_to receive(:new) - subject + subject + end end end end -- cgit v1.2.1 From 935a27cfef3c5a4dd9291c21af69b41a7169817d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 04:22:16 +0100 Subject: Use 1 minute for status polling interval --- app/controllers/projects/clusters/gcp_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 558dae8e228..940c2a5d84f 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -4,7 +4,7 @@ class Projects::Clusters::GcpController < Projects::ApplicationController before_action :authorize_google_project_billing, only: [:check] before_action :authorize_create_cluster!, only: [:new, :create] - STATUS_POLLING_INTERVAL = 10_000 + STATUS_POLLING_INTERVAL = 1.minute.to_i def login begin -- cgit v1.2.1 From 914260930f800342c495114f507947ae35471e80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 05:26:07 +0100 Subject: Expand controller test suite matrix --- .../projects/clusters/gcp_controller.rb | 5 +- app/views/projects/clusters/new.html.haml | 2 +- .../projects/clusters/gcp_controller_spec.rb | 138 +++++++++++++++------ spec/support/google_api/cloud_platform_helpers.rb | 6 + .../check_gcp_project_billing_worker_spec.rb | 6 +- 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 -- cgit v1.2.1 From 6c0fd3c22dc767d8d4d90fa0a008874098a6f22c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 05:31:53 +0100 Subject: Handle html format in addition to json --- app/controllers/projects/clusters/gcp_controller.rb | 2 ++ config/routes/project.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 95c947001a3..34d4fd7d7ca 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -27,6 +27,8 @@ class Projects::Clusters::GcpController < Projects::ApplicationController render json: { billing: redis.get(CheckGcpProjectBillingWorker.redis_shared_state_key_for(token_in_session)) } end end + + format.html { render :check } end end diff --git a/config/routes/project.rb b/config/routes/project.rb index 26eb4fbeda3..9fbd0476bb8 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -189,9 +189,9 @@ constraints(ProjectUrlConstrainer.new) do get '/user/new', to: 'clusters/user#new' post '/user', to: 'clusters/user#create' - get '/gcp/check', to: 'clusters/gcp#check' get '/gcp/new', to: 'clusters/gcp#new' get '/gcp/login', to: 'clusters/gcp#login' + get '/gcp/check', to: 'clusters/gcp#check' post '/gcp', to: 'clusters/gcp#create' end end -- cgit v1.2.1 From c98238f18ac8aa68c971f0742b881e24daf258aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 06:07:28 +0100 Subject: Inluce projects namespace when checking billing --- lib/google_api/cloud_platform/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index c107d001bec..f05d001fd02 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -55,7 +55,7 @@ module GoogleApi service = Google::Apis::CloudbillingV1::CloudbillingService.new service.authorization = access_token - service.get_project_billing_info(project_name) + service.get_project_billing_info("projects/#{project_name}") end def projects_zones_clusters_get(project_id, zone, cluster_id) -- cgit v1.2.1 From 63859419b284ff9c4eba0a1f0df6d8d72764fc50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 16:07:44 +0100 Subject: Add CheckGcpProjectBillingWorker to sidekiq queue --- config/sidekiq_queues.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index bc7c431731a..0a7b4b7c101 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -66,5 +66,6 @@ - [propagate_service_template, 1] - [background_migration, 1] - [gcp_cluster, 1] + - [check_gcp_project_billing, 1] - [project_migrate_hashed_storage, 1] - [storage_migrator, 1] -- cgit v1.2.1 From 886fd13fceda053533a382d1652f9fcce475d0e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 17:02:26 +0100 Subject: Add Worker rerun action to GcpController --- .../projects/clusters/gcp_controller.rb | 11 ++++- config/routes/project.rb | 1 + .../projects/clusters/gcp_controller_spec.rb | 50 ++++++++++++++++++++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 34d4fd7d7ca..c965a055fdd 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,7 +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, except: [:login, :check] + before_action :authorize_google_project_billing, except: [:login, :check, :run_check] before_action :authorize_create_cluster!, only: [:new, :create] STATUS_POLLING_INTERVAL = 1.minute.to_i @@ -32,6 +32,15 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end + def run_check + respond_to do |format| + format.json do + CheckGcpProjectBillingWorker.perform_async(token_in_session) + head :no_content + end + end + end + def new @cluster = ::Clusters::Cluster.new.tap do |cluster| cluster.build_provider_gcp diff --git a/config/routes/project.rb b/config/routes/project.rb index 9fbd0476bb8..d1e8c0ee267 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -192,6 +192,7 @@ constraints(ProjectUrlConstrainer.new) do get '/gcp/new', to: 'clusters/gcp#new' get '/gcp/login', to: 'clusters/gcp#login' get '/gcp/check', to: 'clusters/gcp#check' + post '/gcp/check', to: 'clusters/gcp#run_check' post '/gcp', to: 'clusters/gcp#create' end end diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index 852f3efe793..4fa798c5856 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -138,6 +138,56 @@ describe Projects::Clusters::GcpController do end end + describe 'POST check' do + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + describe 'functionality' do + context 'when access token is valid' do + before do + stub_google_api_validate_token + end + + it 'calls check worker asynchronously' do + expect(CheckGcpProjectBillingWorker).to receive(:perform_async) + + expect(go).to have_http_status(:no_content) + end + end + + context 'when access token is expired' do + before do + stub_google_api_expired_token + end + + it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } + 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 + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:master).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + + def go + post :run_check, namespace_id: project.namespace, project_id: project, format: :json + end + end + describe 'GET new' do describe 'functionality' do let(:user) { create(:user) } -- cgit v1.2.1 From 614c0e0bf9c404ba43f835166183a2f1883071d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 16 Dec 2017 18:55:47 +0100 Subject: Update GCP feature spec with check page flow --- spec/features/projects/clusters/gcp_spec.rb | 153 ++++++++++++++++------------ 1 file changed, 86 insertions(+), 67 deletions(-) diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 67b8901f8fb..4b682acd47e 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -20,105 +20,124 @@ 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 - visit project_clusters_path(project) - - click_link 'Add cluster' - click_link 'Create on GKE' + stub_google_project_billing_status end - context 'when user filled form with valid parameters' do + context 'when user does not have a cluster and visits cluster index page' 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' - ) + 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 - allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) + it 'user sees a cluster details page and creation status' do + expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') - 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 + Clusters::Cluster.last.provider.make_created! - it 'user sees a cluster details page and creation status' do - expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') + expect(page).to have_content('Cluster was successfully created on Google Kubernetes Engine') + end - Clusters::Cluster.last.provider.make_created! + it 'user sees a error if something worng during creation' do + expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') - expect(page).to have_content('Cluster was successfully created on Google Kubernetes Engine') - end + Clusters::Cluster.last.provider.make_errored!('Something wrong!') - it 'user sees a error if something worng during creation' do - expect(page).to have_content('Cluster is being created on Google Kubernetes Engine...') + expect(page).to have_content('Something wrong!') + end + end - Clusters::Cluster.last.provider.make_errored!('Something wrong!') + context 'when user filled form with invalid parameters' do + before do + click_button 'Create cluster' + end - expect(page).to have_content('Something wrong!') + it 'user sees a validation error' do + expect(page).to have_css('#error_explanation') + end end end - context 'when user filled form with invalid parameters' do + context 'when user does have a cluster and visits cluster page' do + let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + before do - click_button 'Create cluster' + visit project_cluster_path(project, cluster) end - it 'user sees a validation error' do - expect(page).to have_css('#error_explanation') + it 'user sees a cluster details page' do + expect(page).to have_button('Save') + expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) end - end - end - context 'when user does have a cluster and visits cluster page' do - let(:cluster) { create(:cluster, :provided_by_gcp, projects: [project]) } + context 'when user disables the cluster' do + before do + page.find(:css, '.js-toggle-cluster').click + click_button 'Save' + end - before do - visit project_cluster_path(project, cluster) - end + it 'user sees the successful message' do + expect(page).to have_content('Cluster was successfully updated.') + end + end - it 'user sees a cluster details page' do - expect(page).to have_button('Save') - expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) - end + context 'when user changes cluster parameters' do + before do + fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' + click_button 'Save changes' + end - context 'when user disables the cluster' do - before do - page.find(:css, '.js-toggle-cluster').click - click_button 'Save' + 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 - it 'user sees the successful message' do - expect(page).to have_content('Cluster was successfully updated.') + 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 changes cluster parameters' do - before do - fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' - click_button 'Save changes' - end + context 'when user does not have a GCP project with billing enabled' do + before do + visit project_clusters_path(project) - 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 + click_link 'Add cluster' + click_link 'Create on GKE' 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 + expect(page).to have_link('Continue') end end end -- cgit v1.2.1 From 97b4e76f9b38720bd48d1ee72d339b4bf027d2bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 18:50:17 +0100 Subject: Mark the gcp check page feature spec pending --- spec/features/projects/clusters/gcp_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 4b682acd47e..4d0abb15b9a 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -137,7 +137,9 @@ feature 'Gcp Cluster', :js do end it 'user sees a check page' do - expect(page).to have_link('Continue') + pending 'the frontend still has not been implemented' do + expect(page).to have_link('Continue') + end end end end -- cgit v1.2.1 From ab2326f87cab96abc91e10d4a1602a5d2b78f1e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 18:53:26 +0100 Subject: Make GCP billing check mock more specific --- spec/support/google_api/cloud_platform_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb index 9f9289fa580..887ea5c99b1 100644 --- a/spec/support/google_api/cloud_platform_helpers.rb +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -13,7 +13,7 @@ module GoogleApi 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') + allow(redis_double).to receive(:get).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for).and_return('true') end def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options) -- cgit v1.2.1 From 59c7f46e2aa33d633fdc3f78c8a4faa792e40972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 19:02:06 +0100 Subject: Remove actions for async GCP project billing check --- .../projects/clusters/gcp_controller.rb | 31 ---- config/routes/project.rb | 2 - .../projects/clusters/gcp_controller_spec.rb | 200 ++------------------- 3 files changed, 11 insertions(+), 222 deletions(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 66a851c52c7..0c8305480ae 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -18,29 +18,6 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end - def check - respond_to do |format| - format.json do - Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL) - - Gitlab::Redis::SharedState.with do |redis| - render json: { billing: redis.get(CheckGcpProjectBillingWorker.redis_shared_state_key_for(token_in_session)) } - end - end - - format.html { render :check } - end - end - - def run_check - respond_to do |format| - format.json do - CheckGcpProjectBillingWorker.perform_async(token_in_session) - head :no_content - end - end - end - def new @cluster = ::Clusters::Cluster.new.tap do |cluster| cluster.build_provider_gcp @@ -84,14 +61,6 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end - def authorize_google_project_billing - 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 def token_in_session @token_in_session ||= diff --git a/config/routes/project.rb b/config/routes/project.rb index b315b53f293..239b5480321 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -192,8 +192,6 @@ constraints(ProjectUrlConstrainer.new) do get '/gcp/new', to: 'clusters/gcp#new' get '/gcp/login', to: 'clusters/gcp#login' - get '/gcp/check', to: 'clusters/gcp#check' - post '/gcp/check', to: 'clusters/gcp#run_check' post '/gcp', to: 'clusters/gcp#create' end end diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index 4fa798c5856..485b67de30a 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -62,132 +62,6 @@ describe Projects::Clusters::GcpController do end end - describe 'GET check' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - describe 'functionality' do - context 'when access token is valid' do - before do - stub_google_api_validate_token - end - - 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 - - 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 - - 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 - - expect(response).to have_http_status(:ok) - expect(JSON.parse(response.body)).to include('billing' => nil) - end - end - end - - context 'when access token is expired' do - before do - stub_google_api_expired_token - end - - it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } - 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 - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - get :check, namespace_id: project.namespace, project_id: project, format: :json - end - end - - describe 'POST check' do - let(:user) { create(:user) } - - before do - project.add_master(user) - sign_in(user) - end - - describe 'functionality' do - context 'when access token is valid' do - before do - stub_google_api_validate_token - end - - it 'calls check worker asynchronously' do - expect(CheckGcpProjectBillingWorker).to receive(:perform_async) - - expect(go).to have_http_status(:no_content) - end - end - - context 'when access token is expired' do - before do - stub_google_api_expired_token - end - - it { expect(go).to redirect_to(gcp_login_project_clusters_path(project)) } - 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 - - describe 'security' do - it { expect { go }.to be_allowed_for(:admin) } - it { expect { go }.to be_allowed_for(:owner).of(project) } - it { expect { go }.to be_allowed_for(:master).of(project) } - it { expect { go }.to be_denied_for(:developer).of(project) } - it { expect { go }.to be_denied_for(:reporter).of(project) } - it { expect { go }.to be_denied_for(:guest).of(project) } - it { expect { go }.to be_denied_for(:user) } - it { expect { go }.to be_denied_for(:external) } - end - - def go - post :run_check, namespace_id: project.namespace, project_id: project, format: :json - end - end - describe 'GET new' do describe 'functionality' do let(:user) { create(:user) } @@ -202,36 +76,10 @@ describe Projects::Clusters::GcpController do stub_google_api_validate_token end - 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) + it 'has new object' do + go - go - end + expect(assigns(:cluster)).to be_an_instance_of(Clusters::Cluster) end end @@ -289,40 +137,14 @@ describe Projects::Clusters::GcpController do stub_google_api_validate_token end - 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 + 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 -- cgit v1.2.1 From e395a2c1901aa08c5d1f26f94406552db44140fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 19:25:28 +0100 Subject: Implement GCP billing check in cluster form --- .../projects/clusters/gcp_controller.rb | 28 ++++++++++++++++------ .../projects/clusters/gcp_controller_spec.rb | 17 ++++++++++++- spec/support/google_api/cloud_platform_helpers.rb | 2 +- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 0c8305480ae..27c11ce554d 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,7 +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, except: [:login, :check, :run_check] + before_action :authorize_google_project_billing, only: [:new] before_action :authorize_create_cluster!, only: [:new, :create] STATUS_POLLING_INTERVAL = 1.minute.to_i @@ -25,15 +25,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 @@ -61,6 +66,15 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end end + def authorize_google_project_billing + CheckGcpProjectBillingWorker.perform_async(token_in_session) + 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 ||= diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index 485b67de30a..be19fa93183 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -77,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) @@ -137,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 } @@ -147,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/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb index 887ea5c99b1..99752ed396e 100644 --- a/spec/support/google_api/cloud_platform_helpers.rb +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -13,7 +13,7 @@ module GoogleApi 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).and_return('true') + 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) -- cgit v1.2.1 From 55f40164c9a012b31adc733d2f081b39970c6b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 20:45:00 +0100 Subject: Add CheckGcpProjectBilling worker to all_queues --- app/workers/all_queues.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 268b7028fd9..142e33e8325 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -97,3 +97,4 @@ - update_user_activity - upload_checksum - web_hook +- check_gcp_project_billing -- cgit v1.2.1 From ad1357d6ccb9be58f36f813c2296e48c18809c2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 21:10:50 +0100 Subject: Fix use of pending decorator in spec --- spec/features/projects/clusters/gcp_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 4d0abb15b9a..ad8c1ebc1f8 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -137,9 +137,8 @@ feature 'Gcp Cluster', :js do end it 'user sees a check page' do - pending 'the frontend still has not been implemented' do - expect(page).to have_link('Continue') - end + pending 'the frontend still has not been implemented' + expect(page).to have_link('Continue') end end end -- cgit v1.2.1 From 9416193b30e2fd9cd793d078da5df1ee9d78c5c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 21:30:43 +0100 Subject: Fix clusters/gcp feature spec --- spec/features/projects/clusters/gcp_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index ad8c1ebc1f8..51bd09e88e0 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -22,6 +22,7 @@ feature 'Gcp Cluster', :js do context 'when user has a GCP project with billing enabled' do before do + allow(CheckGcpProjectBillingWorker).to receive(:perform_async) stub_google_project_billing_status end -- cgit v1.2.1 From 3f9b3d53f6c9e109f2ce0306a22ddb30ae9ed8f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 22 Dec 2017 14:36:06 +0100 Subject: Add url to link in new GCP cluster header partial --- app/views/projects/clusters/gcp/_header.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 } -- cgit v1.2.1 From ccfd8a1240ddd4255f0dd561940d9325306d2a7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 00:06:43 +0100 Subject: Remove GCP cluster check page placeholder --- app/views/projects/clusters/gcp/check.html.haml | 1 - 1 file changed, 1 deletion(-) delete mode 100644 app/views/projects/clusters/gcp/check.html.haml diff --git a/app/views/projects/clusters/gcp/check.html.haml b/app/views/projects/clusters/gcp/check.html.haml deleted file mode 100644 index e965047ad7c..00000000000 --- a/app/views/projects/clusters/gcp/check.html.haml +++ /dev/null @@ -1 +0,0 @@ -Hello -- cgit v1.2.1 From b4e9e07cdd5cbfdcbcf93c56a3b0b005602b40fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 00:09:33 +0100 Subject: Remove polling interval from GcpController --- app/controllers/projects/clusters/gcp_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 27c11ce554d..93b44623d3c 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -4,8 +4,6 @@ class Projects::Clusters::GcpController < Projects::ApplicationController before_action :authorize_google_project_billing, only: [:new] before_action :authorize_create_cluster!, only: [:new, :create] - STATUS_POLLING_INTERVAL = 1.minute.to_i - def login begin state = generate_session_key_redirect(gcp_new_namespace_project_clusters_path.to_s) -- cgit v1.2.1 From 92a72ce43749226a8dc75f7522d09125a5b4b8cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 00:10:40 +0100 Subject: Change CheckGcpProjectBillingWorker lease to 15s --- app/workers/check_gcp_project_billing_worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 254b0959063..5c3a4ff4a35 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -1,7 +1,7 @@ class CheckGcpProjectBillingWorker include ApplicationWorker - LEASE_TIMEOUT = 1.minute.to_i + LEASE_TIMEOUT = 15.seconds.to_i def self.redis_shared_state_key_for(token) "gitlab:gcp:#{token}:billing_enabled" -- cgit v1.2.1 From 571db1a27fa9df038eb56a1c6ae049fc01b48a6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 00:35:19 +0100 Subject: Return list of billing enabled projects --- app/services/check_gcp_project_billing_service.rb | 2 +- app/workers/check_gcp_project_billing_worker.rb | 4 ++-- spec/services/check_gcp_project_billing_service_spec.rb | 9 +++++---- spec/workers/check_gcp_project_billing_worker_spec.rb | 4 ++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/services/check_gcp_project_billing_service.rb b/app/services/check_gcp_project_billing_service.rb index 88e9d7de8de..854adf2177d 100644 --- a/app/services/check_gcp_project_billing_service.rb +++ b/app/services/check_gcp_project_billing_service.rb @@ -1,7 +1,7 @@ class CheckGcpProjectBillingService def execute(token) client = GoogleApi::CloudPlatform::Client.new(token, nil) - client.projects_list.any? do |project| + client.projects_list.select do |project| client.projects_get_billing_info(project.name).billingEnabled end end diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 5c3a4ff4a35..8d80f9c9be3 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -11,9 +11,9 @@ class CheckGcpProjectBillingWorker return unless token return unless try_obtain_lease_for(token) - billing_enabled = CheckGcpProjectBillingService.new.execute(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) + redis.set(self.class.redis_shared_state_key_for(token), !billing_enabled_projects.empty?) 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 1b23b43b0d5..f0e39ba6f49 100644 --- a/spec/services/check_gcp_project_billing_service_spec.rb +++ b/spec/services/check_gcp_project_billing_service_spec.rb @@ -2,13 +2,14 @@ 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([double(name: 'project_name')]) + .to receive(:projects_list).and_return(projects) - expect_any_instance_of(GoogleApi::CloudPlatform::Client) + allow_any_instance_of(GoogleApi::CloudPlatform::Client) .to receive_message_chain(:projects_get_billing_info, :billingEnabled) .and_return(project_billing_enabled) end @@ -18,13 +19,13 @@ describe CheckGcpProjectBillingService do context 'google account has a billing enabled gcp project' do let(:project_billing_enabled) { true } - it { is_expected.to eq(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(false) } + it { is_expected.to eq([]) } 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 d7984ad0c1b..cdb114749ee 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -11,7 +11,7 @@ describe CheckGcpProjectBillingWorker do end it 'calls the service' do - expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute) + expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return([double]) subject end @@ -19,7 +19,7 @@ describe CheckGcpProjectBillingWorker do it 'stores billing status in redis' do redis_double = double - expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return(true) + 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) -- cgit v1.2.1 From 17b44dce06c2a70639bbad45bee696075fa21598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 17:44:41 +0100 Subject: Use token hash for redis key --- app/workers/check_gcp_project_billing_worker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 8d80f9c9be3..24575f84509 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 LEASE_TIMEOUT = 15.seconds.to_i def self.redis_shared_state_key_for(token) - "gitlab:gcp:#{token}:billing_enabled" + "gitlab:gcp:#{token.hash}:billing_enabled" end def perform(token) @@ -21,7 +21,7 @@ class CheckGcpProjectBillingWorker def try_obtain_lease_for(token) Gitlab::ExclusiveLease - .new("check_gcp_project_billing_worker:#{token}", timeout: LEASE_TIMEOUT) + .new("check_gcp_project_billing_worker:#{token.hash}", timeout: LEASE_TIMEOUT) .try_obtain end end -- cgit v1.2.1 From 12984a73029408ef4ca10446131613e9ac371eb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 17:48:40 +0100 Subject: Move worker to gcp_project namespace --- app/workers/all_queues.yml | 2 +- app/workers/check_gcp_project_billing_worker.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 142e33e8325..5da0de89d12 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 @@ -97,4 +98,3 @@ - update_user_activity - upload_checksum - web_hook -- check_gcp_project_billing diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 24575f84509..42aa6b39d86 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -1,5 +1,6 @@ class CheckGcpProjectBillingWorker include ApplicationWorker + include ClusterQueue LEASE_TIMEOUT = 15.seconds.to_i -- cgit v1.2.1 From 2885dc06602d8bff42421d38502f85965b7e8b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 6 Jan 2018 16:34:23 +0100 Subject: Remove check_gcp_project_billing queue from sidekiq --- config/sidekiq_queues.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 0fc233ab24e..31a38f2b508 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -65,6 +65,5 @@ - [propagate_service_template, 1] - [background_migration, 1] - [gcp_cluster, 1] - - [check_gcp_project_billing, 1] - [project_migrate_hashed_storage, 1] - [storage_migrator, 1] -- cgit v1.2.1 From a180306da8daff608f7910af0f759a7dba8f15be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 6 Jan 2018 19:02:18 +0100 Subject: Use token from redis in gcp project billing worker --- app/workers/check_gcp_project_billing_worker.rb | 15 ++++++- .../check_gcp_project_billing_worker_spec.rb | 46 +++++++++++++++------- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 42aa6b39d86..784d17c4654 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -1,14 +1,23 @@ +require 'securerandom' + class CheckGcpProjectBillingWorker include ApplicationWorker include ClusterQueue LEASE_TIMEOUT = 15.seconds.to_i + def self.generate_redis_token_key + SecureRandom.uuid + end + def self.redis_shared_state_key_for(token) "gitlab:gcp:#{token.hash}:billing_enabled" end - def perform(token) + def perform(token_key) + return unless token_key + + token = get_token(token_key) return unless token return unless try_obtain_lease_for(token) @@ -20,6 +29,10 @@ class CheckGcpProjectBillingWorker private + def get_token(token_key) + Gitlab::Redis::SharedState.with { |redis| redis.get(token_key) } + end + def try_obtain_lease_for(token) Gitlab::ExclusiveLease .new("check_gcp_project_billing_worker:#{token.hash}", timeout: LEASE_TIMEOUT) diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index cdb114749ee..b1687cc95c6 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -3,33 +3,51 @@ require 'spec_helper' describe CheckGcpProjectBillingWorker do describe '.perform' do let(:token) { 'bogustoken' } - subject { described_class.new.perform(token) } + subject { described_class.new.perform('token_key') } - context 'when there is no lease' do + context 'when there is a token in redis' do before do - allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return('randomuuid') + allow_any_instance_of(described_class).to receive(:get_token).and_return(token) end - it 'calls the service' do - expect(CheckGcpProjectBillingService).to receive_message_chain(:new, :execute).and_return([double]) + 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 - subject + 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) + + subject + end end - it 'stores billing status in redis' do - redis_double = double + 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 - 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) + it 'does not call the service' do + expect(CheckGcpProjectBillingService).not_to receive(:new) - subject + subject + end end end - context 'when there is a lease' do + context 'when there is no token in redis' do before do - allow_any_instance_of(described_class).to receive(:try_obtain_lease_for).and_return(false) + allow_any_instance_of(described_class).to receive(:get_token).and_return(nil) end it 'does not call the service' do -- cgit v1.2.1 From d13be3c2d98071234790e2fe11d180e8b7b13b6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 6 Jan 2018 19:41:49 +0100 Subject: Store OAuth token in Redis for worker to pick up --- app/controllers/projects/clusters/gcp_controller.rb | 7 ++++++- spec/features/projects/clusters/gcp_spec.rb | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 93b44623d3c..41b25ed1325 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -65,7 +65,12 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end def authorize_google_project_billing - CheckGcpProjectBillingWorker.perform_async(token_in_session) + redis_token_key = CheckGcpProjectBillingWorker.generate_redis_token_key + Gitlab::Redis::SharedState.with do |redis| + redis.set(redis_token_key, token_in_session, ex: 5.minutes) + end + + CheckGcpProjectBillingWorker.perform_async(redis_token_key) end def google_project_billing_status diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 940b054cfe9..523cc08496b 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -22,7 +22,7 @@ feature 'Gcp Cluster', :js do context 'when user has a GCP project with billing enabled' do before do - allow(CheckGcpProjectBillingWorker).to receive(:perform_async) + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) stub_google_project_billing_status end -- cgit v1.2.1 From 044064baef01a5838630c5e8f95a2ed9d2a801f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 6 Jan 2018 20:10:08 +0100 Subject: Add CHANGELOG entry --- ...-users-try-to-create-a-cluster-but-the-account-is-not-enabled.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/39957-redirect-to-gpc-page-if-users-try-to-create-a-cluster-but-the-account-is-not-enabled.yml 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 -- cgit v1.2.1 From 15b5b91d20d75c159898e716ba199c4b2e3a0af5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 7 Jan 2018 15:07:37 +0100 Subject: Separate let and subject line in spec --- spec/workers/check_gcp_project_billing_worker_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index b1687cc95c6..83fdf719358 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -3,6 +3,7 @@ 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 -- cgit v1.2.1 From e7a8564f39a46a5fa5f34f798b890c0a62ff12e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 7 Jan 2018 16:18:53 +0100 Subject: Refactor GCP session token exchange scheme --- .../projects/clusters/gcp_controller.rb | 6 +----- app/workers/check_gcp_project_billing_worker.rb | 25 +++++++++++++++++----- .../check_gcp_project_billing_worker_spec.rb | 4 ++-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 41b25ed1325..25608df0b9c 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -65,11 +65,7 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end def authorize_google_project_billing - redis_token_key = CheckGcpProjectBillingWorker.generate_redis_token_key - Gitlab::Redis::SharedState.with do |redis| - redis.set(redis_token_key, token_in_session, ex: 5.minutes) - end - + redis_token_key = CheckGcpProjectBillingWorker.store_session_token(token_in_session) CheckGcpProjectBillingWorker.perform_async(redis_token_key) end diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 784d17c4654..96d5c2a2193 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -5,9 +5,20 @@ class CheckGcpProjectBillingWorker include ClusterQueue LEASE_TIMEOUT = 15.seconds.to_i + SESSION_KEY_TIMEOUT = 5.minutes - def self.generate_redis_token_key - SecureRandom.uuid + 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) @@ -17,7 +28,7 @@ class CheckGcpProjectBillingWorker def perform(token_key) return unless token_key - token = get_token(token_key) + token = self.get_session_token(token_key) return unless token return unless try_obtain_lease_for(token) @@ -29,8 +40,12 @@ class CheckGcpProjectBillingWorker private - def get_token(token_key) - Gitlab::Redis::SharedState.with { |redis| redis.get(token_key) } + 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) diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index 83fdf719358..744ac65859a 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_token).and_return(token) + allow_any_instance_of(described_class).to receive(:get_session_token).and_return(token) end context 'when there is no lease' do @@ -48,7 +48,7 @@ describe CheckGcpProjectBillingWorker do context 'when there is no token in redis' do before do - allow_any_instance_of(described_class).to receive(:get_token).and_return(nil) + allow_any_instance_of(described_class).to receive(:get_session_token).and_return(nil) end it 'does not call the service' do -- cgit v1.2.1 From 684e574abafb6d31b713ca0e5c4fc0c0c66b7846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Sun, 7 Jan 2018 16:28:16 +0000 Subject: Set timeout to billing information --- app/workers/check_gcp_project_billing_worker.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 96d5c2a2193..557af14ee57 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -6,6 +6,7 @@ class CheckGcpProjectBillingWorker 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| @@ -34,7 +35,9 @@ class CheckGcpProjectBillingWorker 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?) + redis.set(self.class.redis_shared_state_key_for(token), + !billing_enabled_projects.empty?, + ex: BILLING_TIMEOUT) end end -- cgit v1.2.1 From e86eb09111d45787281251b80e0caab2e6c7de6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Sun, 7 Jan 2018 18:08:27 +0000 Subject: Fix check_gcp_project_billing_worker_spec.rb --- spec/workers/check_gcp_project_billing_worker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index 744ac65859a..f52a903327c 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -27,7 +27,7 @@ describe CheckGcpProjectBillingWorker do 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) + expect(redis_double).to receive(:set).with(described_class.redis_shared_state_key_for(token), anything, anything) subject end -- cgit v1.2.1