From 7b030bcd6b4b435e9d9f510a0c7b6e25c76a3ba8 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 8 Jan 2018 14:00:08 +0000 Subject: Merge branch '40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes' into 'master' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes Closes #40418 See merge request gitlab-org/gitlab-ce!15589 (cherry picked from commit 4b92efd90cedaa0aff218d11fdce279701128bea) 5b2ca1c6 Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes e4745492 Add test. Disable KubernetesService when migrated b9fbfe5a Fix unmanaged_kubernetes_service scope for multiple clusters 40c6af54 Fix migration file typos and reorder Table definition 27111e29 Restructure spec 4dc14576 Fix comments 8e6ffe35 Fix test 7eeada80 Add env_scope tests f083739e Add logic to swtich environment_scope by the situation 9b7719b6 Use explicit namespace for avoiding reference from application code 665972e2 Avoid quotes in ActiveRecord query c8059881 Opitmize migration process by using both unmanaged_kubernetes_service and… b8a275d3 Use bulk_insert instead of AR create acfb8464 Fix static anylysy 8bc3221f Fix query to look for proper unmanaged kubernetes service 2d3c7d29 Use batch update for Service deactivation 1c404c91 Add a new test for emptified params 183dbdc8 Revert bulk_insert and bring back AR insert(one by one) 54d20d1b Add changelog 290c2248 Fix change log 58d074e0 Fix StaticSnalysys df658c7b Disable STI of ActiveRecord. Refactoring specs. c425ff75 Fix static analysys 67327952 Add memoization for properties --- ...tesservice-to-clusters-platforms-kubernetes.yml | 5 + ...rnetes_service_to_new_clusters_architectures.rb | 151 ++++++++++ ...s_service_to_new_clusters_architectures_spec.rb | 312 +++++++++++++++++++++ 3 files changed, 468 insertions(+) create mode 100644 changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml create mode 100644 db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb create mode 100644 spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb diff --git a/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml new file mode 100644 index 00000000000..5e158d831a6 --- /dev/null +++ b/changelogs/unreleased/40418-migrate-existing-data-from-kubernetesservice-to-clusters-platforms-kubernetes.yml @@ -0,0 +1,5 @@ +--- +title: Migrate existing data from KubernetesService to Clusters::Platforms::Kubernetes +merge_request: 15589 +author: +type: changed diff --git a/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb new file mode 100644 index 00000000000..11b581e4b57 --- /dev/null +++ b/db/post_migrate/20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb @@ -0,0 +1,151 @@ +class MigrateKubernetesServiceToNewClustersArchitectures < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME = 'KubernetesService'.freeze + + disable_ddl_transaction! + + class Project < ActiveRecord::Base + self.table_name = 'projects' + + has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' + has_many :clusters, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + has_many :services, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service' + has_one :kubernetes_service, -> { where(category: 'deployment', type: 'KubernetesService') }, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Service', inverse_of: :project, foreign_key: :project_id + end + + class Cluster < ActiveRecord::Base + self.table_name = 'clusters' + + has_many :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::ClustersProject' + has_many :projects, through: :cluster_projects, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' + has_one :platform_kubernetes, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::PlatformsKubernetes' + + accepts_nested_attributes_for :platform_kubernetes + + enum platform_type: { + kubernetes: 1 + } + + enum provider_type: { + user: 0, + gcp: 1 + } + end + + class ClustersProject < ActiveRecord::Base + self.table_name = 'cluster_projects' + + belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project' + end + + class PlatformsKubernetes < ActiveRecord::Base + self.table_name = 'cluster_platforms_kubernetes' + + belongs_to :cluster, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Cluster' + + attr_encrypted :token, + mode: :per_attribute_iv, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' + end + + class Service < ActiveRecord::Base + include EachBatch + + self.table_name = 'services' + self.inheritance_column = :_type_disabled # Disable STI, otherwise KubernetesModel will be looked up + + belongs_to :project, class_name: 'MigrateKubernetesServiceToNewClustersArchitectures::Project', foreign_key: :project_id + + scope :unmanaged_kubernetes_service, -> do + joins('LEFT JOIN projects ON projects.id = services.project_id') + .joins('LEFT JOIN cluster_projects ON cluster_projects.project_id = projects.id') + .joins('LEFT JOIN cluster_platforms_kubernetes ON cluster_platforms_kubernetes.cluster_id = cluster_projects.cluster_id') + .where(category: 'deployment', type: 'KubernetesService', template: false) + .where("services.properties LIKE '%api_url%'") + .where("(services.properties NOT LIKE CONCAT('%', cluster_platforms_kubernetes.api_url, '%')) OR cluster_platforms_kubernetes.api_url IS NULL") + .group(:id) + .order(id: :asc) + end + + scope :kubernetes_service_without_template, -> do + where(category: 'deployment', type: 'KubernetesService', template: false) + end + + def api_url + parsed_properties['api_url'] + end + + def ca_pem + parsed_properties['ca_pem'] + end + + def namespace + parsed_properties['namespace'] + end + + def token + parsed_properties['token'] + end + + private + + def parsed_properties + @parsed_properties ||= JSON.parse(self.properties) + end + end + + def find_dedicated_environement_scope(project) + environment_scopes = project.clusters.map(&:environment_scope) + + return '*' if environment_scopes.exclude?('*') # KubernetesService should be added as a default cluster (environment_scope: '*') at first place + return 'migrated/*' if environment_scopes.exclude?('migrated/*') # If it's conflicted, the KubernetesService added as a migrated cluster + + unique_iid = 0 + + # If it's still conflicted, finding an unique environment scope incrementaly + loop do + candidate = "migrated#{unique_iid}/*" + return candidate if environment_scopes.exclude?(candidate) + + unique_iid += 1 + end + end + + def up + ActiveRecord::Base.transaction do + MigrateKubernetesServiceToNewClustersArchitectures::Service + .unmanaged_kubernetes_service.find_each(batch_size: 1) do |kubernetes_service| + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create( + enabled: kubernetes_service.active, + user_id: nil, # KubernetesService doesn't have + name: DEFAULT_KUBERNETES_SERVICE_CLUSTER_NAME, + provider_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.provider_types[:user], + platform_type: MigrateKubernetesServiceToNewClustersArchitectures::Cluster.platform_types[:kubernetes], + projects: [kubernetes_service.project], + environment_scope: find_dedicated_environement_scope(kubernetes_service.project), + platform_kubernetes_attributes: { + api_url: kubernetes_service.api_url, + ca_cert: kubernetes_service.ca_pem, + namespace: kubernetes_service.namespace, + username: nil, # KubernetesService doesn't have + encrypted_password: nil, # KubernetesService doesn't have + encrypted_password_iv: nil, # KubernetesService doesn't have + token: kubernetes_service.token # encrypted_token and encrypted_token_iv + } ) + end + end + + MigrateKubernetesServiceToNewClustersArchitectures::Service + .kubernetes_service_without_template.each_batch(of: 100) do |kubernetes_service| + kubernetes_service.update_all(active: false) + end + end + + def down + # noop + end +end diff --git a/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb new file mode 100644 index 00000000000..df0015b6dd3 --- /dev/null +++ b/spec/migrations/migrate_kubernetes_service_to_new_clusters_architectures_spec.rb @@ -0,0 +1,312 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20171124104327_migrate_kubernetes_service_to_new_clusters_architectures.rb') + +describe MigrateKubernetesServiceToNewClustersArchitectures, :migration do + context 'when unique KubernetesService exists' do + shared_examples 'KubernetesService migration' do + let(:sample_num) { 2 } + + let(:projects) do + (1..sample_num).each_with_object([]) do |n, array| + array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! + end + end + + let!(:kubernetes_services) do + projects.map do |project| + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: active, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"https://kubernetes#{project.id}.com\",\"ca_pem\":\"ca_pem#{project.id}\",\"token\":\"token#{project.id}\"}") + end + end + + it 'migrates the KubernetesService to Platform::Kubernetes' do + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) + + projects.each do |project| + project.clusters.last.tap do |cluster| + expect(cluster.enabled).to eq(active) + expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) + expect(project.kubernetes_service).not_to be_active + end + end + end + end + + context 'when KubernetesService is active' do + let(:active) { true } + + it_behaves_like 'KubernetesService migration' + end + end + + context 'when unique KubernetesService spawned from Service Template' do + let(:sample_num) { 2 } + + let(:projects) do + (1..sample_num).each_with_object([]) do |n, array| + array << MigrateKubernetesServiceToNewClustersArchitectures::Project.create! + end + end + + let!(:kubernetes_service_template) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + template: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"https://sample.kubernetes.com\",\"ca_pem\":\"ca_pem-sample\",\"token\":\"token-sample\"}") + end + + let!(:kubernetes_services) do + projects.map do |project| + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"#{kubernetes_service_template.api_url}\",\"ca_pem\":\"#{kubernetes_service_template.ca_pem}\",\"token\":\"#{kubernetes_service_template.token}\"}") + end + end + + it 'migrates the KubernetesService to Platform::Kubernetes without template' do + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(sample_num) + + projects.each do |project| + project.clusters.last.tap do |cluster| + expect(cluster.platform_kubernetes.api_url).to eq(project.kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_cert).to eq(project.kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(project.kubernetes_service.token) + expect(project.kubernetes_service).not_to be_active + end + end + end + end + + context 'when managed KubernetesService exists' do + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: cluster.enabled, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"#{cluster.platform_kubernetes.api_url}\"}") + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do # Because the corresponding Platform::Kubernetes already exists + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } + + kubernetes_service.reload + expect(kubernetes_service).not_to be_active + end + end + + context 'when production cluster has already been existed' do # i.e. There are no environment_scope conflicts + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: 'production/*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end + + it 'migrates the KubernetesService to Platform::Kubernetes' do + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + end + end + end + + context 'when default cluster has already been existed' do + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let!(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: '*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end + + it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('migrated/*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + end + end + end + + context 'when default cluster and migrated cluster has already been existed' do + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + let!(:cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: '*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:migrated_cluster) do + MigrateKubernetesServiceToNewClustersArchitectures::Cluster.create!( + projects: [project], + name: 'sample-cluster', + platform_type: :kubernetes, + provider_type: :user, + environment_scope: 'migrated/*', + platform_kubernetes_attributes: { + api_url: 'https://sample.kubernetes.com', + ca_cert: 'ca_pem-sample', + token: 'token-sample' + } ) + end + + let!(:kubernetes_service) do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: true, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"api_url\":\"https://debug.kube.com\"}") + end + + it 'migrates the KubernetesService to Platform::Kubernetes with dedicated environment_scope' do # Because environment_scope is duplicated + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) + + kubernetes_service.reload + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('migrated0/*') + expect(cluster.platform_kubernetes.api_url).to eq(kubernetes_service.api_url) + expect(cluster.platform_kubernetes.ca_cert).to eq(kubernetes_service.ca_pem) + expect(cluster.platform_kubernetes.token).to eq(kubernetes_service.token) + expect(kubernetes_service).not_to be_active + end + end + end + + context 'when KubernetesService has nullified parameters' do + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + before do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: false, + category: 'deployment', + type: 'KubernetesService', + properties: "{}") + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } + + expect(project.kubernetes_service).not_to be_active + end + end + + # Platforms::Kubernetes validates `token` reagdless of the activeness, + # whereas KubernetesService validates `token` if only it's activated + # However, in this migration file, there are no validations because of the re-defined model class + # therefore, we should safely add this raw to Platform::Kubernetes + context 'when KubernetesService has empty token' do + let(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + before do + MigrateKubernetesServiceToNewClustersArchitectures::Service.create!( + project: project, + active: false, + category: 'deployment', + type: 'KubernetesService', + properties: "{\"namespace\":\"prod\",\"api_url\":\"http://111.111.111.111\",\"ca_pem\":\"a\",\"token\":\"\"}") + end + + it 'does not migrate the KubernetesService and disables the kubernetes_service' do + expect { migrate! }.to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count }.by(1) + + project.clusters.last.tap do |cluster| + expect(cluster.environment_scope).to eq('*') + expect(cluster.platform_kubernetes.namespace).to eq('prod') + expect(cluster.platform_kubernetes.api_url).to eq('http://111.111.111.111') + expect(cluster.platform_kubernetes.ca_cert).to eq('a') + expect(cluster.platform_kubernetes.token).to be_empty + expect(project.kubernetes_service).not_to be_active + end + end + end + + context 'when KubernetesService does not exist' do + let!(:project) { MigrateKubernetesServiceToNewClustersArchitectures::Project.create! } + + it 'does not migrate the KubernetesService' do + expect { migrate! }.not_to change { MigrateKubernetesServiceToNewClustersArchitectures::Cluster.count } + end + end +end -- cgit v1.2.3 From 782378e2e9d6ddabae79819521723eefd31b0c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Trzci=C5=84ski?= Date: Thu, 11 Jan 2018 11:27:19 +0000 Subject: Merge branch 'gcp-fix' into 'master' Fix GCP redirect Closes #41867 See merge request gitlab-org/gitlab-ce!16355 (cherry picked from commit 59adc07f00f4f3ea326194d77c31580edfdfb2a4) b44583e9 Extract GCP billing check as method cf6258af Fix billing checking 0cdd56e6 Fix link to billing e52bae3b Fix CheckGcpProjectBillingService spec b8b2f5ff Fix CheckGcpProjectBillingWorker spec 8ba3e473 Fix GCP Controller spec 1f0a4fe6 Add missing user agent header to GCP client 6ef28ace Add API requirements to docs 0b294fc2 Use new tab for link in flash cf95756a Refactor GCP redirect test suite e6012d3e Change failed GCP billing check wording 35598274 Fix breadcumb of clusters show page cf842986 Update links for GCP instructions --- .../projects/clusters/gcp_controller.rb | 31 +++++++++----- app/services/check_gcp_project_billing_service.rb | 5 ++- app/views/projects/clusters/gcp/_header.html.haml | 6 +-- app/views/projects/clusters/show.html.haml | 2 +- app/workers/check_gcp_project_billing_worker.rb | 6 +-- doc/user/project/clusters/index.md | 13 +++--- lib/google_api/cloud_platform/client.rb | 6 +-- .../projects/clusters/gcp_controller_spec.rb | 7 +++- spec/features/projects/clusters/gcp_spec.rb | 36 +++++++++++++++-- .../check_gcp_project_billing_service_spec.rb | 21 +++++----- spec/support/google_api/cloud_platform_helpers.rb | 47 ++++++++++++++++++++-- .../check_gcp_project_billing_worker_spec.rb | 2 +- 12 files changed, 135 insertions(+), 47 deletions(-) diff --git a/app/controllers/projects/clusters/gcp_controller.rb b/app/controllers/projects/clusters/gcp_controller.rb index 25608df0b9c..4fc515bd03e 100644 --- a/app/controllers/projects/clusters/gcp_controller.rb +++ b/app/controllers/projects/clusters/gcp_controller.rb @@ -1,8 +1,9 @@ class Projects::Clusters::GcpController < Projects::ApplicationController before_action :authorize_read_cluster! before_action :authorize_google_api, except: [:login] - before_action :authorize_google_project_billing, only: [:new] + before_action :authorize_google_project_billing, only: [:new, :create] before_action :authorize_create_cluster!, only: [:new, :create] + before_action :verify_billing, only: [:create] def login begin @@ -23,24 +24,34 @@ class Projects::Clusters::GcpController < Projects::ApplicationController end def create + @cluster = ::Clusters::CreateService + .new(project, current_user, create_params) + .execute(token_in_session) + + if @cluster.persisted? + redirect_to project_cluster_path(project, @cluster) + else + render :new + end + end + + private + + def verify_billing case google_project_billing_status when 'true' - @cluster = ::Clusters::CreateService - .new(project, current_user, create_params) - .execute(token_in_session) - - return redirect_to project_cluster_path(project, @cluster) if @cluster.persisted? + return when 'false' - flash[:error] = _('Please enable billing for one of your projects to be able to create a cluster.') + flash[:alert] = _('Please enable billing for one of your projects to be able to create a cluster, then try again.').html_safe % { link_to_billing: "https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral" } else - flash[:error] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.') + flash[:alert] = _('We could not verify that one of your projects on GCP has billing enabled. Please try again.') end + @cluster = ::Clusters::Cluster.new(create_params) + render :new end - private - def create_params params.require(:cluster).permit( :enabled, diff --git a/app/services/check_gcp_project_billing_service.rb b/app/services/check_gcp_project_billing_service.rb index 854adf2177d..ea82b61b279 100644 --- a/app/services/check_gcp_project_billing_service.rb +++ b/app/services/check_gcp_project_billing_service.rb @@ -2,7 +2,10 @@ class CheckGcpProjectBillingService def execute(token) client = GoogleApi::CloudPlatform::Client.new(token, nil) client.projects_list.select do |project| - client.projects_get_billing_info(project.name).billingEnabled + begin + client.projects_get_billing_info(project.project_id).billing_enabled + rescue + end end end end diff --git a/app/views/projects/clusters/gcp/_header.html.haml b/app/views/projects/clusters/gcp/_header.html.haml index e2d7326a312..bddb902115d 100644 --- a/app/views/projects/clusters/gcp/_header.html.haml +++ b/app/views/projects/clusters/gcp/_header.html.haml @@ -4,11 +4,11 @@ = s_('ClusterIntegration|Please make sure that your Google account meets the following requirements:') %ul %li - - link_to_kubernetes_engine = link_to(s_('ClusterIntegration|access to Google Kubernetes Engine'), 'https://console.cloud.google.com', target: '_blank', rel: 'noopener noreferrer') + - link_to_kubernetes_engine = link_to(s_('ClusterIntegration|access to Google Kubernetes Engine'), 'https://console.cloud.google.com/freetrial?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|Your account must have %{link_to_kubernetes_engine}').html_safe % { link_to_kubernetes_engine: link_to_kubernetes_engine } %li - - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart', target: '_blank', rel: 'noopener noreferrer') + - link_to_requirements = link_to(s_('ClusterIntegration|meets the requirements'), 'https://cloud.google.com/kubernetes-engine/docs/quickstart?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|Make sure your account %{link_to_requirements} to create clusters').html_safe % { link_to_requirements: link_to_requirements } %li - - link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), 'https://console.cloud.google.com/home/dashboard', target: '_blank', rel: 'noopener noreferrer') + - link_to_container_project = link_to(s_('ClusterIntegration|Google Kubernetes Engine project'), 'https://console.cloud.google.com/home/dashboard?utm_campaign=2018_cpanel&utm_source=gitlab&utm_medium=referral', target: '_blank', rel: 'noopener noreferrer') = s_('ClusterIntegration|This account must have permissions to create a cluster in the %{link_to_container_project} specified below').html_safe % { link_to_container_project: link_to_container_project } diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index c7c84b5a42c..2049105dff6 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -1,6 +1,6 @@ - @content_class = "limit-container-width" unless fluid_layout - add_to_breadcrumbs "Clusters", project_clusters_path(@project) -- breadcrumb_title @cluster.id +- breadcrumb_title @cluster.name - page_title _("Cluster") - expanded = Rails.env.test? diff --git a/app/workers/check_gcp_project_billing_worker.rb b/app/workers/check_gcp_project_billing_worker.rb index 557af14ee57..5466ccdda59 100644 --- a/app/workers/check_gcp_project_billing_worker.rb +++ b/app/workers/check_gcp_project_billing_worker.rb @@ -4,7 +4,7 @@ class CheckGcpProjectBillingWorker include ApplicationWorker include ClusterQueue - LEASE_TIMEOUT = 15.seconds.to_i + LEASE_TIMEOUT = 3.seconds.to_i SESSION_KEY_TIMEOUT = 5.minutes BILLING_TIMEOUT = 1.hour @@ -23,13 +23,13 @@ class CheckGcpProjectBillingWorker end def self.redis_shared_state_key_for(token) - "gitlab:gcp:#{token.hash}:billing_enabled" + "gitlab:gcp:#{Digest::SHA1.hexdigest(token)}:billing_enabled" end def perform(token_key) return unless token_key - token = self.get_session_token(token_key) + token = self.class.get_session_token(token_key) return unless token return unless try_obtain_lease_for(token) diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index d5619c7b563..218b9dee41d 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -25,11 +25,14 @@ prerequisites must be met: be enabled in GitLab at the instance level. If that's not the case, ask your administrator to enable it. - Your associated Google account must have the right privileges to manage - clusters on GKE. That would mean that a - [billing account](https://cloud.google.com/billing/docs/how-to/manage-billing-account) - must be set up. -- You must have Master [permissions] in order to be able to access the **Cluster** - page. + clusters on GKE. That would mean that a [billing + account](https://cloud.google.com/billing/docs/how-to/manage-billing-account) + must be set up and that you have to have permissions to access it. +- You must have Master [permissions] in order to be able to access the + **Cluster** page. +- You must have [Cloud Billing API](https://cloud.google.com/billing/) enabled +- You must have [Resource Manager + API](https://cloud.google.com/resource-manager/) If all of the above requirements are met, you can proceed to add a new GKE cluster. diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index f05d001fd02..ff638c07755 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -47,15 +47,15 @@ module GoogleApi service.authorization = access_token service.fetch_all(items: :projects) do |token| - service.list_projects(page_token: token) + service.list_projects(page_token: token, options: user_agent_header) end end - def projects_get_billing_info(project_name) + def projects_get_billing_info(project_id) service = Google::Apis::CloudbillingV1::CloudbillingService.new service.authorization = access_token - service.get_project_billing_info("projects/#{project_name}") + service.get_project_billing_info("projects/#{project_id}", options: user_agent_header) end def projects_zones_clusters_get(project_id, zone, cluster_id) diff --git a/spec/controllers/projects/clusters/gcp_controller_spec.rb b/spec/controllers/projects/clusters/gcp_controller_spec.rb index be19fa93183..775f9db1c6e 100644 --- a/spec/controllers/projects/clusters/gcp_controller_spec.rb +++ b/spec/controllers/projects/clusters/gcp_controller_spec.rb @@ -137,11 +137,14 @@ describe Projects::Clusters::GcpController do context 'when access token is valid' do before do stub_google_api_validate_token + allow_any_instance_of(described_class).to receive(:authorize_google_project_billing) end context 'when google project billing is enabled' do before do - stub_google_project_billing_status + redis_double = double + allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) + allow(redis_double).to receive(:get).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for('token')).and_return('true') end it 'creates a new cluster' do @@ -158,7 +161,7 @@ describe Projects::Clusters::GcpController do it 'renders the cluster form with an error' do go - expect(response).to set_flash[:error] + expect(response).to set_flash[:alert] expect(response).to render_template('new') end end diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 523cc08496b..8953b30bebf 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -13,6 +13,8 @@ feature 'Gcp Cluster', :js do end context 'when user has signed with Google' do + let(:project_id) { 'test-project-1234' } + before do allow_any_instance_of(Projects::Clusters::GcpController) .to receive(:token_in_session).and_return('token') @@ -23,7 +25,7 @@ feature 'Gcp Cluster', :js do context 'when user has a GCP project with billing enabled' do before do allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) - stub_google_project_billing_status + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return('true') end context 'when user does not have a cluster and visits cluster index page' do @@ -131,15 +133,41 @@ feature 'Gcp Cluster', :js do context 'when user does not have a GCP project with billing enabled' do before do + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return('false') + visit project_clusters_path(project) click_link 'Add cluster' click_link 'Create on GKE' + + fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' + fill_in 'cluster_name', with: 'dev-cluster' + click_button 'Create cluster' + end + + it 'user sees form with error' do + expect(page).to have_content('Please enable billing for one of your projects to be able to create a cluster, then try again.') + end + end + + context 'when gcp billing status is not in redis' do + before do + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:authorize_google_project_billing) + allow_any_instance_of(Projects::Clusters::GcpController).to receive(:google_project_billing_status).and_return(nil) + + visit project_clusters_path(project) + + click_link 'Add cluster' + click_link 'Create on GKE' + + fill_in 'cluster_provider_gcp_attributes_gcp_project_id', with: 'gcp-project-123' + fill_in 'cluster_name', with: 'dev-cluster' + click_button 'Create cluster' end - it 'user sees a check page' do - pending 'the frontend still has not been implemented' - expect(page).to have_link('Continue') + it 'user sees form with error' do + expect(page).to have_content('We could not verify that one of your projects on GCP has billing enabled. Please try again.') end end end diff --git a/spec/services/check_gcp_project_billing_service_spec.rb b/spec/services/check_gcp_project_billing_service_spec.rb index f0e39ba6f49..3e68d906e71 100644 --- a/spec/services/check_gcp_project_billing_service_spec.rb +++ b/spec/services/check_gcp_project_billing_service_spec.rb @@ -1,29 +1,30 @@ require 'spec_helper' describe CheckGcpProjectBillingService do + include GoogleApi::CloudPlatformHelpers + let(:service) { described_class.new } - let(:projects) { [double(name: 'first_project'), double(name: 'second_project')] } + let(:project_id) { 'test-project-1234' } describe '#execute' do before do - expect_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_list).and_return(projects) - - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive_message_chain(:projects_get_billing_info, :billingEnabled) - .and_return(project_billing_enabled) + stub_cloud_platform_projects_list(project_id: project_id) end subject { service.execute('bogustoken') } context 'google account has a billing enabled gcp project' do - let(:project_billing_enabled) { true } + before do + stub_cloud_platform_projects_get_billing_info(project_id, true) + end - it { is_expected.to eq(projects) } + it { is_expected.to all(satisfy { |project| project.project_id == project_id }) } end context 'google account does not have a billing enabled gcp project' do - let(:project_billing_enabled) { false } + before do + stub_cloud_platform_projects_get_billing_info(project_id, false) + end it { is_expected.to eq([]) } end diff --git a/spec/support/google_api/cloud_platform_helpers.rb b/spec/support/google_api/cloud_platform_helpers.rb index 99752ed396e..2fdbddd40c2 100644 --- a/spec/support/google_api/cloud_platform_helpers.rb +++ b/spec/support/google_api/cloud_platform_helpers.rb @@ -10,10 +10,14 @@ module GoogleApi request.session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] = 1.hour.ago.to_i.to_s end - def stub_google_project_billing_status - redis_double = double - allow(Gitlab::Redis::SharedState).to receive(:with).and_yield(redis_double) - allow(redis_double).to receive(:get).with(CheckGcpProjectBillingWorker.redis_shared_state_key_for('token')).and_return('true') + def stub_cloud_platform_projects_list(options) + WebMock.stub_request(:get, cloud_platform_projects_list_url) + .to_return(cloud_platform_response(cloud_platform_projects_body(options))) + end + + def stub_cloud_platform_projects_get_billing_info(project_id, billing_enabled) + WebMock.stub_request(:get, cloud_platform_projects_get_billing_info_url(project_id)) + .to_return(cloud_platform_response(cloud_platform_projects_billing_info_body(project_id, billing_enabled))) end def stub_cloud_platform_get_zone_cluster(project_id, zone, cluster_id, **options) @@ -46,6 +50,14 @@ module GoogleApi .to_return(status: [500, "Internal Server Error"]) end + def cloud_platform_projects_list_url + "https://cloudresourcemanager.googleapis.com/v1/projects" + end + + def cloud_platform_projects_get_billing_info_url(project_id) + "https://cloudbilling.googleapis.com/v1/projects/#{project_id}/billingInfo" + end + def cloud_platform_get_zone_cluster_url(project_id, zone, cluster_id) "https://container.googleapis.com/v1/projects/#{project_id}/zones/#{zone}/clusters/#{cluster_id}" end @@ -121,5 +133,32 @@ module GoogleApi "endTime": options[:endTime] || '' } end + + def cloud_platform_projects_body(**options) + { + "projects": [ + { + "projectNumber": options[:project_number] || "1234", + "projectId": options[:project_id] || "test-project-1234", + "lifecycleState": "ACTIVE", + "name": options[:name] || "test-project", + "createTime": "2017-12-16T01:48:29.129Z", + "parent": { + "type": "organization", + "id": "12345" + } + } + ] + } + end + + def cloud_platform_projects_billing_info_body(project_id, billing_enabled) + { + "name": "projects/#{project_id}/billingInfo", + "projectId": "#{project_id}", + "billingAccountName": "account-name", + "billingEnabled": billing_enabled + } + end end end diff --git a/spec/workers/check_gcp_project_billing_worker_spec.rb b/spec/workers/check_gcp_project_billing_worker_spec.rb index f52a903327c..7b7a7c1bc44 100644 --- a/spec/workers/check_gcp_project_billing_worker_spec.rb +++ b/spec/workers/check_gcp_project_billing_worker_spec.rb @@ -8,7 +8,7 @@ describe CheckGcpProjectBillingWorker do context 'when there is a token in redis' do before do - allow_any_instance_of(described_class).to receive(:get_session_token).and_return(token) + allow(described_class).to receive(:get_session_token).and_return(token) end context 'when there is no lease' do -- cgit v1.2.3