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/ --- app/controllers/clusters/base_controller.rb | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 app/controllers/clusters/base_controller.rb (limited to 'app/controllers/clusters/base_controller.rb') 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 --- app/controllers/clusters/base_controller.rb | 32 ++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) (limited to 'app/controllers/clusters/base_controller.rb') 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/base_controller.rb') 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/base_controller.rb') 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 ++++++++--------------------- 1 file changed, 8 insertions(+), 24 deletions(-) (limited to 'app/controllers/clusters/base_controller.rb') 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 -- cgit v1.2.1