Commit 59adc07f authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'gcp-fix' into 'master'

Fix GCP redirect

Closes #41867

See merge request gitlab-org/gitlab-ce!16355
parents e55899ff cf842986
class Projects::Clusters::GcpController < Projects::ApplicationController class Projects::Clusters::GcpController < Projects::ApplicationController
before_action :authorize_read_cluster! before_action :authorize_read_cluster!
before_action :authorize_google_api, except: [:login] before_action :authorize_google_api, except: [:login]
before_action :authorize_google_project_billing, only: [:new] before_action :authorize_google_project_billing, only: [:new, :create]
before_action :authorize_create_cluster!, only: [:new, :create] before_action :authorize_create_cluster!, only: [:new, :create]
before_action :verify_billing, only: [:create]
def login def login
begin begin
...@@ -23,24 +24,34 @@ class Projects::Clusters::GcpController < Projects::ApplicationController ...@@ -23,24 +24,34 @@ class Projects::Clusters::GcpController < Projects::ApplicationController
end end
def create def create
@cluster = ::Clusters::CreateService
.new(project, current_user, create_params)
.execute(token_in_session)
if @cluster.persisted?
redirect_to project_cluster_path(project, @cluster)
else
render :new
end
end
private
def verify_billing
case google_project_billing_status case google_project_billing_status
when 'true' when 'true'
@cluster = ::Clusters::CreateService return
.new(project, current_user, create_params)
.execute(token_in_session)
return redirect_to project_cluster_path(project, @cluster) if @cluster.persisted?
when 'false' when 'false'
flash[:error] = _('Please enable billing for one of your projects to be able to create a cluster.') flash[:alert] = _('Please <a href=%{link_to_billing} target="_blank" rel="noopener noreferrer">enable billing for one of your projects to be able to create a cluster</a>, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" }
else else
flash[:error] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.') flash[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
end end
@cluster = ::Clusters::Cluster.new(create_params)
render :new render :new
end end
private
def create_params def create_params
params.require(:cluster).permit( params.require(:cluster).permit(
:enabled, :enabled,
......
...@@ -2,7 +2,10 @@ class CheckGcpProjectBillingService ...@@ -2,7 +2,10 @@ class CheckGcpProjectBillingService
def execute(token) def execute(token)
client = GoogleApi::CloudPlatform::Client.new(token, nil) client = GoogleApi::CloudPlatform::Client.new(token, nil)
client.projects_list.select do |project| client.projects_list.select do |project|
client.projects_get_billing_info(project.name).billingEnabled begin
client.projects_get_billing_info(project.project_id).billing_enabled
rescue
end
end end
end end
end end
...@@ -4,11 +4,11 @@ ...@@ -4,11 +4,11 @@
= s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:') = s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:')
%ul %ul
%li %li
- link_to_kubernetes_engine = link_to(s_('ClusterIntegration|access to Google Kubernetes Engine'), 'https://console.cloud.google.com', target: '_blank', rel: 'noopener noreferrer') - link_to_kubernetes_engine = link_to(s_('ClusterIntegration|access to Google Kubernetes Engine'), 'https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer')
= s_('ClusterIntegration|Your account must have %{link_to_kubernetes_engine}').html_safe % { link_to_kubernetes_engine: link_to_kubernetes_engine } = s_('ClusterIntegration|Your account must have %{link_to_kubernetes_engine}').html_safe % { link_to_kubernetes_engine: link_to_kubernetes_engine }
%li %li
- link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer') - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer')
= s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements } = s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements }
%li %li
- link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') - link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), 'https://console.cloud.google.com/home/dashboard?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer')
= s_('ClusterIntegration|This account must have permissions to create a cluster in the %{link_to_container_project} specified below').html_safe % { link_to_container_project: link_to_container_project } = 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 }
- @content_class = "limit-container-width" unless fluid_layout - @content_class = "limit-container-width" unless fluid_layout
- add_to_breadcrumbs "Clusters", project_clusters_path(@project) - add_to_breadcrumbs "Clusters", project_clusters_path(@project)
- breadcrumb_title @cluster.id - breadcrumb_title @cluster.name
- page_title _("Cluster") - page_title _("Cluster")
- expanded = Rails.env.test? - expanded = Rails.env.test?
......
...@@ -4,7 +4,7 @@ class CheckGcpProjectBillingWorker ...@@ -4,7 +4,7 @@ class CheckGcpProjectBillingWorker
include ApplicationWorker include ApplicationWorker
include ClusterQueue include ClusterQueue
LEASE_TIMEOUT = 15.seconds.to_i LEASE_TIMEOUT = 3.seconds.to_i
SESSION_KEY_TIMEOUT = 5.minutes SESSION_KEY_TIMEOUT = 5.minutes
BILLING_TIMEOUT = 1.hour BILLING_TIMEOUT = 1.hour
...@@ -23,13 +23,13 @@ class CheckGcpProjectBillingWorker ...@@ -23,13 +23,13 @@ class CheckGcpProjectBillingWorker
end end
def self.redis_shared_state_key_for(token) def self.redis_shared_state_key_for(token)
"gitlab:gcp:#{token.hash}:billing_enabled" "gitlab:gcp:#{Digest::SHA1.hexdigest(token)}:billing_enabled"
end end
def perform(token_key) def perform(token_key)
return unless token_key return unless token_key
token = self.get_session_token(token_key) token = self.class.get_session_token(token_key)
return unless token return unless token
return unless try_obtain_lease_for(token) return unless try_obtain_lease_for(token)
......
...@@ -22,11 +22,14 @@ prerequisites must be met: ...@@ -22,11 +22,14 @@ prerequisites must be met:
be enabled in GitLab at the instance level. If that's not the case, ask your be enabled in GitLab at the instance level. If that's not the case, ask your
administrator to enable it. administrator to enable it.
- Your associated Google account must have the right privileges to manage - Your associated Google account must have the right privileges to manage
clusters on GKE. That would mean that a clusters on GKE. That would mean that a [billing
[billing account](https://cloud.google.com/billing/docs/how-to/manage-billing-account) account](https://cloud.google.com/billing/docs/how-to/manage-billing-account)
must be set up. must be set up and that you have to have permissions to access it.
- You must have Master [permissions] in order to be able to access the **Cluster** - You must have Master [permissions] in order to be able to access the
page. **Cluster** page.
- You must have [Cloud Billing API](https://cloud.google.com/billing/) enabled
- You must have [Resource Manager
API](https://cloud.google.com/resource-manager/)
If all of the above requirements are met, you can proceed to add a new GKE If all of the above requirements are met, you can proceed to add a new GKE
cluster. cluster.
......
...@@ -47,15 +47,15 @@ module GoogleApi ...@@ -47,15 +47,15 @@ module GoogleApi
service.authorization = access_token service.authorization = access_token
service.fetch_all(items: :projects) do |token| service.fetch_all(items: :projects) do |token|
service.list_projects(page_token: token) service.list_projects(page_token: token, options: user_agent_header)
end end
end end
def projects_get_billing_info(project_name) def projects_get_billing_info(project_id)
service = Google::Apis::CloudbillingV1::CloudbillingService.new service = Google::Apis::CloudbillingV1::CloudbillingService.new
service.authorization = access_token service.authorization = access_token
service.get_project_billing_info("projects/#{project_name}") service.get_project_billing_info("projects/#{project_id}", options: user_agent_header)
end end
def projects_zones_clusters_get(project_id, zone, cluster_id) def projects_zones_clusters_get(project_id, zone, cluster_id)
......
...@@ -137,11 +137,14 @@ describe Projects::Clusters::GcpController do ...@@ -137,11 +137,14 @@ describe Projects::Clusters::GcpController do
context 'when access token is valid' do context 'when access token is valid' do
before do before do
stub_google_api_validate_token stub_google_api_validate_token
allow_any_instance_of(described_class).to receive(:authorize_google_project_billing)
end end
context 'when google project billing is enabled' do context 'when google project billing is enabled' do
before do before do
stub_google_project_billing_status redis_double = double
allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double)
allow(redis_double).to receive(:get).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for('token')).and_return('true')
end end
it 'creates a new cluster' do it 'creates a new cluster' do
...@@ -158,7 +161,7 @@ describe Projects::Clusters::GcpController do ...@@ -158,7 +161,7 @@ describe Projects::Clusters::GcpController do
it 'renders the cluster form with an error' do it 'renders the cluster form with an error' do
go go
expect(response).to set_flash[:error] expect(response).to set_flash[:alert]
expect(response).to render_template('new') expect(response).to render_template('new')
end end
end end
......
...@@ -13,6 +13,8 @@ feature 'Gcp Cluster', :js do ...@@ -13,6 +13,8 @@ feature 'Gcp Cluster', :js do
end end
context 'when user has signed with Google' do context 'when user has signed with Google' do
let(:project_id) { 'test-project-1234' }
before do before do
allow_any_instance_of(Projects::Clusters::GcpController) allow_any_instance_of(Projects::Clusters::GcpController)
.to receive(:token_in_session).and_return('token') .to receive(:token_in_session).and_return('token')
...@@ -23,7 +25,7 @@ feature 'Gcp Cluster', :js do ...@@ -23,7 +25,7 @@ feature 'Gcp Cluster', :js do
context 'when user has a GCP project with billing enabled' do context 'when user has a GCP project with billing enabled' do
before do before do
allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing)
stub_google_project_billing_status allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return('true')
end end
context 'when user does not have a cluster and visits cluster index page' do context 'when user does not have a cluster and visits cluster index page' do
...@@ -131,15 +133,41 @@ feature 'Gcp Cluster', :js do ...@@ -131,15 +133,41 @@ feature 'Gcp Cluster', :js do
context 'when user does not have a GCP project with billing enabled' do context 'when user does not have a GCP project with billing enabled' do
before do before do
allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing)
allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return('false')
visit project_clusters_path(project) visit project_clusters_path(project)
click_link 'Add cluster' click_link 'Add cluster'
click_link 'Create on GKE' click_link 'Create on GKE'
fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123'
fill_in 'cluster_name', with: 'dev-cluster'
click_button 'Create cluster'
end
it 'user sees form with error' do
expect(page).to have_content('Please enable billing for one of your projects to be able to create a cluster, then try again.')
end
end
context 'when gcp billing status is not in redis' do
before do
allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing)
allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return(nil)
visit project_clusters_path(project)
click_link 'Add cluster'
click_link 'Create on GKE'
fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123'
fill_in 'cluster_name', with: 'dev-cluster'
click_button 'Create cluster'
end end
it 'user sees a check page' do it 'user sees form with error' do
pending 'the frontend still has not been implemented' expect(page).to have_content('We could not verify that one of your projects on GCP has billing enabled. Please try again.')
expect(page).to have_link('Continue')
end end
end end
end end
......
require 'spec_helper' require 'spec_helper'
describe CheckGcpProjectBillingService do describe CheckGcpProjectBillingService do
include GoogleApi::CloudPlatformHelpers
let(:service) { described_class.new } let(:service) { described_class.new }
let(:projects) { [double(name: 'first_project'), double(name: 'second_project')] } let(:project_id) { 'test-project-1234' }
describe '#execute' do describe '#execute' do
before do before do
expect_any_instance_of(GoogleApi::CloudPlatform::Client) stub_cloud_platform_projects_list(project_id: project_id)
.to receive(:projects_list).and_return(projects)
allow_any_instance_of(GoogleApi::CloudPlatform::Client)
.to receive_message_chain(:projects_get_billing_info, :billingEnabled)
.and_return(project_billing_enabled)
end end
subject { service.execute('bogustoken') } subject { service.execute('bogustoken') }
context 'google account has a billing enabled gcp project' do context 'google account has a billing enabled gcp project' do
let(:project_billing_enabled) { true } before do
stub_cloud_platform_projects_get_billing_info(project_id, true)
end
it { is_expected.to eq(projects) } it { is_expected.to all(satisfy { |project| project.project_id == project_id }) }
end end
context 'google account does not have a billing enabled gcp project' do context 'google account does not have a billing enabled gcp project' do
let(:project_billing_enabled) { false } before do
stub_cloud_platform_projects_get_billing_info(project_id, false)
end
it { is_expected.to eq([]) } it { is_expected.to eq([]) }
end end
......
...@@ -10,10 +10,14 @@ module GoogleApi ...@@ -10,10 +10,14 @@ module GoogleApi
request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.ago.to_i.to_s request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.ago.to_i.to_s
end end
def stub_google_project_billing_status def stub_cloud_platform_projects_list(options)
redis_double = double WebMock.stub_request(:get, cloud_platform_projects_list_url)
allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) .to_return(cloud_platform_response(cloud_platform_projects_body(options)))
allow(redis_double).to receive(:get).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for('token')).and_return('true') end
def stub_cloud_platform_projects_get_billing_info(project_id, billing_enabled)
WebMock.stub_request(:get, cloud_platform_projects_get_billing_info_url(project_id))
.to_return(cloud_platform_response(cloud_platform_projects_billing_info_body(project_id, billing_enabled)))
end end
def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options) def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options)
...@@ -46,6 +50,14 @@ module GoogleApi ...@@ -46,6 +50,14 @@ module GoogleApi
.to_return(status: [500, "Internal Server Error"]) .to_return(status: [500, "Internal Server Error"])
end end
def cloud_platform_projects_list_url
"https://cloudresourcemanager.googleapis.com/v1/projects"
end
def cloud_platform_projects_get_billing_info_url(project_id)
"https://cloudbilling.googleapis.com/v1/projects/#{project_id}/billingInfo"
end
def cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id) def cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id)
"https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/clusters/#{cluster_id}" "https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/clusters/#{cluster_id}"
end end
...@@ -121,5 +133,32 @@ module GoogleApi ...@@ -121,5 +133,32 @@ module GoogleApi
"endTime": options[:endTime] || '' "endTime": options[:endTime] || ''
} }
end end
def cloud_platform_projects_body(**options)
{
"projects": [
{
"projectNumber": options[:project_number] || "1234",
"projectId": options[:project_id] || "test-project-1234",
"lifecycleState": "ACTIVE",
"name": options[:name] || "test-project",
"createTime": "2017-12-16T01:48:29.129Z",
"parent": {
"type": "organization",
"id": "12345"
}
}
]
}
end
def cloud_platform_projects_billing_info_body(project_id, billing_enabled)
{
"name": "projects/#{project_id}/billingInfo",
"projectId": "#{project_id}",
"billingAccountName": "account-name",
"billingEnabled": billing_enabled
}
end
end end
end end
...@@ -8,7 +8,7 @@ describe CheckGcpProjectBillingWorker do ...@@ -8,7 +8,7 @@ describe CheckGcpProjectBillingWorker do
context 'when there is a token in redis' do context 'when there is a token in redis' do
before do before do
allow_any_instance_of(described_class).to receive(:get_session_token).and_return(token) allow(described_class).to receive(:get_session_token).and_return(token)
end end
context 'when there is no lease' do context 'when there is no lease' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment