From 5b3c096c9e0c9e8e7e1cb35c1b9e347995b948f5 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 15 Oct 2018 16:37:51 +1300 Subject: Convert clusters to use a top-level controller In preparation so that we can create both cluster attached to project and cluster attached to group. - Move ClustersController to top level - Move Clusters::ApplicationsController to top-level too - Creates a Clusters::BaseController to share common functions - Do not rely on @project ivar. Anything could set the ivar. - Fix Vue page components due to new data-page value Because of the controller change we have gone from `projects:clusters:new` to `clusters:new`, so we need to update the file location of the page components. There is somewhere a function that will convert data-page to a file location. On that note, projects/clusters/gcp/new/, translate to Projects::Clusters::Gcp#new doesn't exist so replace that with clusters/create_gcp/ and clusters/create_user/ --- .../clusters/applications_controller.rb | 28 +++++++++++++++ app/controllers/clusters/base_controller.rb | 42 ++++++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 app/controllers/clusters/applications_controller.rb create mode 100644 app/controllers/clusters/base_controller.rb (limited to 'app/controllers/clusters') diff --git a/app/controllers/clusters/applications_controller.rb b/app/controllers/clusters/applications_controller.rb new file mode 100644 index 00000000000..a5ac5fe3f8e --- /dev/null +++ b/app/controllers/clusters/applications_controller.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class Clusters::ApplicationsController < Clusters::BaseController + before_action :cluster + before_action :authorize_create_cluster!, only: [:create] + + def create + Clusters::Applications::CreateService + .new(@cluster, current_user, create_cluster_application_params) + .execute(request) + + head :no_content + rescue Clusters::Applications::CreateService::InvalidApplicationError + render_404 + rescue StandardError + head :bad_request + end + + private + + def cluster + @cluster ||= project.clusters.find(params[:id]) || render_404 + end + + def create_cluster_application_params + params.permit(:application, :hostname) + end +end diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb new file mode 100644 index 00000000000..2804b236d17 --- /dev/null +++ b/app/controllers/clusters/base_controller.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +class Clusters::BaseController < ApplicationController + include RoutableActions + + skip_before_action :authenticate_user! + before_action :require_project_id + before_action :project, if: :project_type? + before_action :repository, if: :project_type? + before_action :authorize_read_cluster! + + private + + # We can extend to `#group_type?` in the future + def require_project_id + not_found unless project_type? + end + + def project + @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id])) + end + + def repository + @repository ||= project.repository + end + + def authorize_read_cluster! + access_denied! unless can?(current_user, :read_cluster, clusterable) + end + + def authorize_create_cluster! + access_denied! unless can?(current_user, :create_cluster, clusterable) + end + + def clusterable + project if project_type? + end + + def project_type? + params[:project_id].present? + end +end -- cgit v1.2.1 From 88800abcd8741b07114c2850e00b74fbecfbf90e Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 19 Oct 2018 14:42:30 +1300 Subject: Abstract out project out of ClustersController To the extent possible swap out `project` with `clusterable` - Abstract paths for showing cluster or clusters. This will allow us to swap in alternative paths for group level cluster - Push :project_id and :namespace_id params from the URL to the POST body. - Create a nice helper for to generate links for the destroy action For some reason, spec :project_id and :namespace_id param are not going through `to_param` for a JSON format. Manually call `to_param` to fix specs. - Move :layout to BaseController --- .../clusters/applications_controller.rb | 2 +- app/controllers/clusters/base_controller.rb | 32 +++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) (limited to 'app/controllers/clusters') diff --git a/app/controllers/clusters/applications_controller.rb b/app/controllers/clusters/applications_controller.rb index a5ac5fe3f8e..250f42f3096 100644 --- a/app/controllers/clusters/applications_controller.rb +++ b/app/controllers/clusters/applications_controller.rb @@ -19,7 +19,7 @@ class Clusters::ApplicationsController < Clusters::BaseController private def cluster - @cluster ||= project.clusters.find(params[:id]) || render_404 + @cluster ||= clusterable.clusters.find(params[:id]) || render_404 end def create_cluster_application_params diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb index 2804b236d17..2e9997dfc08 100644 --- a/app/controllers/clusters/base_controller.rb +++ b/app/controllers/clusters/base_controller.rb @@ -9,6 +9,10 @@ class Clusters::BaseController < ApplicationController before_action :repository, if: :project_type? before_action :authorize_read_cluster! + layout :determine_layout + + helper_method :clusters_page_path, :cluster_page_path, :new_cluster_page_path + private # We can extend to `#group_type?` in the future @@ -32,8 +36,34 @@ class Clusters::BaseController < ApplicationController access_denied! unless can?(current_user, :create_cluster, clusterable) end + def determine_layout + if project_type? + 'project' + end + end + def clusterable - project if project_type? + if project_type? + project + end + end + + def cluster_page_path(cluster) + if project_type? + project_cluster_path(project, cluster) + end + end + + def clusters_page_path + if project_type? + project_clusters_path(project) + end + end + + def new_cluster_page_path + if project_type? + new_project_cluster_path(project) + end end def project_type? -- cgit v1.2.1 From 1163b235391668d53ae0cea80bc22d40b365e0a7 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 30 Oct 2018 23:33:43 +1300 Subject: Move view and path concerns to presenters - Move show path for cluster to ClusterPresenter - Create ClusterablePresenter to encapsulate logic. Consolidates scattered methods from BaseController and ClustersHelper into an object. --- app/controllers/clusters/base_controller.rb | 24 ++---------------------- 1 file changed, 2 insertions(+), 22 deletions(-) (limited to 'app/controllers/clusters') diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb index 2e9997dfc08..8908b26b914 100644 --- a/app/controllers/clusters/base_controller.rb +++ b/app/controllers/clusters/base_controller.rb @@ -11,7 +11,7 @@ class Clusters::BaseController < ApplicationController layout :determine_layout - helper_method :clusters_page_path, :cluster_page_path, :new_cluster_page_path + helper_method :clusterable private @@ -43,27 +43,7 @@ class Clusters::BaseController < ApplicationController end def clusterable - if project_type? - project - end - end - - def cluster_page_path(cluster) - if project_type? - project_cluster_path(project, cluster) - end - end - - def clusters_page_path - if project_type? - project_clusters_path(project) - end - end - - def new_cluster_page_path - if project_type? - new_project_cluster_path(project) - end + @clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user) end def project_type? -- cgit v1.2.1 From 28dabc67f4db8271ac20c0db458ae2c86a906eee Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Wed, 31 Oct 2018 23:31:24 +1300 Subject: Restore 403 functionality for external auth (EE) When we unhooked ClustersController from Project::ApplicationsController, we missed an EE override to handle_not_found_or_authorized. Rather than carry on with override RoutingActions, make a specific proc for Project that we override in EE instead. Use that proc in both Clusters::BaseController and Project::ApplicationsController. --- app/controllers/clusters/base_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app/controllers/clusters') diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb index 8908b26b914..3a8575769c4 100644 --- a/app/controllers/clusters/base_controller.rb +++ b/app/controllers/clusters/base_controller.rb @@ -2,6 +2,7 @@ class Clusters::BaseController < ApplicationController include RoutableActions + include ProjectUnauthorized skip_before_action :authenticate_user! before_action :require_project_id @@ -21,7 +22,7 @@ class Clusters::BaseController < ApplicationController end def project - @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id])) + @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc) end def repository -- cgit v1.2.1 From 1a1fdf8efe1923ba781e978e858c009264020e72 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Thu, 1 Nov 2018 13:39:01 +1300 Subject: Resolve controller sharing concern Use ClustersController as base while having Projects::ClustersController to inform what `clusterable` is. Thanks @ayufan for the great suggestion ! - View changes to work with new approach - Fix javascript for new approach - Fix feature specs for new approach - Fix QA --- app/controllers/clusters/base_controller.rb | 32 +--- app/controllers/clusters/clusters_controller.rb | 218 ++++++++++++++++++++++++ 2 files changed, 226 insertions(+), 24 deletions(-) create mode 100644 app/controllers/clusters/clusters_controller.rb (limited to 'app/controllers/clusters') diff --git a/app/controllers/clusters/base_controller.rb b/app/controllers/clusters/base_controller.rb index 3a8575769c4..ef42f7c4074 100644 --- a/app/controllers/clusters/base_controller.rb +++ b/app/controllers/clusters/base_controller.rb @@ -2,31 +2,25 @@ class Clusters::BaseController < ApplicationController include RoutableActions - include ProjectUnauthorized skip_before_action :authenticate_user! - before_action :require_project_id - before_action :project, if: :project_type? - before_action :repository, if: :project_type? before_action :authorize_read_cluster! - layout :determine_layout - helper_method :clusterable private - # We can extend to `#group_type?` in the future - def require_project_id - not_found unless project_type? + def cluster + @cluster ||= clusterable.clusters.find(params[:id]) + .present(current_user: current_user) end - def project - @project ||= find_routable!(Project, File.join(params[:namespace_id], params[:project_id]), not_found_or_authorized_proc: project_unauthorized_proc) + def authorize_update_cluster! + access_denied! unless can?(current_user, :update_cluster, cluster) end - def repository - @repository ||= project.repository + def authorize_admin_cluster! + access_denied! unless can?(current_user, :admin_cluster, cluster) end def authorize_read_cluster! @@ -37,17 +31,7 @@ class Clusters::BaseController < ApplicationController access_denied! unless can?(current_user, :create_cluster, clusterable) end - def determine_layout - if project_type? - 'project' - end - end - def clusterable - @clusterable ||= ClusterablePresenter.fabricate(project, current_user: current_user) - end - - def project_type? - params[:project_id].present? + raise NotImplementedError end end diff --git a/app/controllers/clusters/clusters_controller.rb b/app/controllers/clusters/clusters_controller.rb new file mode 100644 index 00000000000..f6f2060ebb5 --- /dev/null +++ b/app/controllers/clusters/clusters_controller.rb @@ -0,0 +1,218 @@ +# frozen_string_literal: true + +class Clusters::ClustersController < Clusters::BaseController + include RoutableActions + + before_action :cluster, except: [:index, :new, :create_gcp, :create_user] + before_action :generate_gcp_authorize_url, only: [:new] + before_action :validate_gcp_token, only: [:new] + before_action :gcp_cluster, only: [:new] + before_action :user_cluster, only: [:new] + before_action :authorize_create_cluster!, only: [:new] + before_action :authorize_update_cluster!, only: [:update] + before_action :authorize_admin_cluster!, only: [:destroy] + before_action :update_applications_status, only: [:cluster_status] + + helper_method :token_in_session + + STATUS_POLLING_INTERVAL = 10_000 + + def index + clusters = ClustersFinder.new(clusterable, current_user, :all).execute + @clusters = clusters.page(params[:page]).per(20) + end + + def new + end + + # Overridding ActionController::Metal#status is NOT a good idea + def cluster_status + respond_to do |format| + format.json do + Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL) + + render json: ClusterSerializer + .new(current_user: @current_user) + .represent_status(@cluster) + end + end + end + + def show + end + + def update + Clusters::UpdateService + .new(current_user, update_params) + .execute(cluster) + + if cluster.valid? + respond_to do |format| + format.json do + head :no_content + end + format.html do + flash[:notice] = _('Kubernetes cluster was successfully updated.') + redirect_to cluster.show_path + end + end + else + respond_to do |format| + format.json { head :bad_request } + format.html { render :show } + end + end + end + + def destroy + if cluster.destroy + flash[:notice] = _('Kubernetes cluster integration was successfully removed.') + redirect_to clusterable.index_path, status: :found + else + flash[:notice] = _('Kubernetes cluster integration was not removed.') + render :show + end + end + + def create_gcp + @gcp_cluster = ::Clusters::CreateService + .new(current_user, create_gcp_cluster_params) + .execute(access_token: token_in_session) + .present(current_user: current_user) + + if @gcp_cluster.persisted? + redirect_to @gcp_cluster.show_path + else + generate_gcp_authorize_url + validate_gcp_token + user_cluster + + render :new, locals: { active_tab: 'gcp' } + end + end + + def create_user + @user_cluster = ::Clusters::CreateService + .new(current_user, create_user_cluster_params) + .execute(access_token: token_in_session) + .present(current_user: current_user) + + if @user_cluster.persisted? + redirect_to @user_cluster.show_path + else + generate_gcp_authorize_url + validate_gcp_token + gcp_cluster + + render :new, locals: { active_tab: 'user' } + end + end + + private + + def update_params + if cluster.managed? + params.require(:cluster).permit( + :enabled, + :environment_scope, + platform_kubernetes_attributes: [ + :namespace + ] + ) + else + params.require(:cluster).permit( + :enabled, + :name, + :environment_scope, + platform_kubernetes_attributes: [ + :api_url, + :token, + :ca_cert, + :namespace + ] + ) + end + end + + def create_gcp_cluster_params + params.require(:cluster).permit( + :enabled, + :name, + :environment_scope, + provider_gcp_attributes: [ + :gcp_project_id, + :zone, + :num_nodes, + :machine_type, + :legacy_abac + ]).merge( + provider_type: :gcp, + platform_type: :kubernetes, + clusterable: clusterable.subject + ) + end + + def create_user_cluster_params + params.require(:cluster).permit( + :enabled, + :name, + :environment_scope, + platform_kubernetes_attributes: [ + :namespace, + :api_url, + :token, + :ca_cert, + :authorization_type + ]).merge( + provider_type: :user, + platform_type: :kubernetes, + clusterable: clusterable.subject + ) + end + + def generate_gcp_authorize_url + state = generate_session_key_redirect(clusterable.new_path.to_s) + + @authorize_url = GoogleApi::CloudPlatform::Client.new( + nil, callback_google_api_auth_url, + state: state).authorize_url + rescue GoogleApi::Auth::ConfigMissingError + # no-op + end + + def gcp_cluster + @gcp_cluster = ::Clusters::Cluster.new.tap do |cluster| + cluster.build_provider_gcp + end + end + + def user_cluster + @user_cluster = ::Clusters::Cluster.new.tap do |cluster| + cluster.build_platform_kubernetes + end + end + + def validate_gcp_token + @valid_gcp_token = GoogleApi::CloudPlatform::Client.new(token_in_session, nil) + .validate_token(expires_at_in_session) + end + + def token_in_session + session[GoogleApi::CloudPlatform::Client.session_key_for_token] + end + + def expires_at_in_session + @expires_at_in_session ||= + session[GoogleApi::CloudPlatform::Client.session_key_for_expires_at] + end + + def generate_session_key_redirect(uri) + GoogleApi::CloudPlatform::Client.new_session_key_for_redirect_uri do |key| + session[key] = uri + end + end + + def update_applications_status + @cluster.applications.each(&:schedule_status_update) + end +end -- cgit v1.2.1