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/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

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?

app/controllers/clusters_controller.rb

@@ -11,14 +11,12 @@ class ClustersController < Clusters::BaseController
   before_action :authorize_admin_cluster!, only: [:destroy]
   before_action :update_applications_status, only: [:status]
 
-  layout :determine_layout
-
   helper_method :token_in_session
 
   STATUS_POLLING_INTERVAL = 10_000
 
   def index
-    clusters = ClustersFinder.new(project, current_user, :all).execute
+    clusters = ClustersFinder.new(clusterable, current_user, :all).execute
     @clusters = clusters.page(params[:page]).per(20)
   end
 
@@ -31,7 +29,7 @@ class ClustersController < Clusters::BaseController
         Gitlab::PollingInterval.set_header(response, interval: STATUS_POLLING_INTERVAL)
 
         render json: ClusterSerializer
-          .new(project: project, current_user: @current_user)
+          .new(current_user: @current_user)
           .represent_status(@cluster)
       end
     end
@@ -52,7 +50,7 @@ class ClustersController < Clusters::BaseController
         end
         format.html do
           flash[:notice] = _('Kubernetes cluster was successfully updated.')
-          redirect_to project_cluster_path(project, cluster)
+          redirect_to cluster_page_path(cluster)
         end
       end
     else
@@ -66,7 +64,7 @@ class ClustersController < Clusters::BaseController
   def destroy
     if cluster.destroy
       flash[:notice] = _('Kubernetes cluster integration was successfully removed.')
-      redirect_to project_clusters_path(project), status: :found
+      redirect_to clusters_page_path, status: :found
     else
       flash[:notice] = _('Kubernetes cluster integration was not removed.')
       render :show
@@ -76,10 +74,10 @@ class ClustersController < Clusters::BaseController
   def create_gcp
     @gcp_cluster = ::Clusters::CreateService
       .new(current_user, create_gcp_cluster_params)
-      .execute(project: project, access_token: token_in_session)
+      .execute(access_token: token_in_session)
 
     if @gcp_cluster.persisted?
-      redirect_to project_cluster_path(project, @gcp_cluster)
+      redirect_to cluster_page_path(@gcp_cluster)
     else
       generate_gcp_authorize_url
       validate_gcp_token
@@ -92,10 +90,10 @@ class ClustersController < Clusters::BaseController
   def create_user
     @user_cluster = ::Clusters::CreateService
       .new(current_user, create_user_cluster_params)
-      .execute(project: project, access_token: token_in_session)
+      .execute(access_token: token_in_session)
 
     if @user_cluster.persisted?
-      redirect_to project_cluster_path(project, @user_cluster)
+      redirect_to cluster_page_path(@user_cluster)
     else
       generate_gcp_authorize_url
       validate_gcp_token
@@ -107,14 +105,8 @@ class ClustersController < Clusters::BaseController
 
   private
 
-  def determine_layout
-    if project_type?
-      'project'
-    end
-  end
-
   def cluster
-    @cluster ||= project.clusters.find(params[:id])
+    @cluster ||= clusterable.clusters.find(params[:id])
                                  .present(current_user: current_user)
   end
 
@@ -155,7 +147,8 @@ class ClustersController < Clusters::BaseController
         :legacy_abac
       ]).merge(
         provider_type: :gcp,
-        platform_type: :kubernetes
+        platform_type: :kubernetes,
+        clusterable: clusterable
       )
   end
 
@@ -172,12 +165,13 @@ class ClustersController < Clusters::BaseController
         :authorization_type
       ]).merge(
         provider_type: :user,
-        platform_type: :kubernetes
+        platform_type: :kubernetes,
+        clusterable: clusterable
       )
   end
 
   def generate_gcp_authorize_url
-    state = generate_session_key_redirect(new_project_cluster_path(project).to_s)
+    state = generate_session_key_redirect(new_cluster_page_path.to_s)
 
     @authorize_url = GoogleApi::CloudPlatform::Client.new(
       nil, callback_google_api_auth_url,

app/finders/clusters_finder.rb

 # frozen_string_literal: true
 
 class ClustersFinder
-  def initialize(project, user, scope)
-    @project = project
+  def initialize(clusterable, user, scope)
+    @clusterable = clusterable
     @user = user
     @scope = scope || :active
   end
 
   def execute
-    clusters = project.clusters
+    clusters = clusterable.clusters
     filter_by_scope(clusters)
   end
 
   private
 
-  attr_reader :project, :user, :scope
+  attr_reader :clusterable, :user, :scope
 
   def filter_by_scope(clusters)
     case scope.to_sym

app/helpers/clusters_helper.rb

 # frozen_string_literal: true
 
 module ClustersHelper
-  def has_multiple_clusters?(project)
-    project.feature_available?(:multiple_clusters)
+  def has_multiple_clusters?
+    clusterable.feature_available?(:multiple_clusters)
+  end
+
+  def clusterable
+    @project
+  end
+
+  def can_create_cluster?
+    can?(current_user, :create_cluster, clusterable)
   end
 
   def render_gcp_signup_offer
@@ -13,4 +21,19 @@ module ClustersHelper
       render 'clusters/gcp_signup_offer_banner'
     end
   end
+
+  def hidden_clusterable_fields
+    clusterable_params.map do |key, value|
+      hidden_field_tag(key, value)
+    end.reduce(&:safe_concat)
+  end
+
+  def clusterable_params
+    case clusterable
+    when Project
+      { project_id: clusterable.to_param, namespace_id: clusterable.namespace.to_param }
+    else
+      {}
+    end
+  end
 end

app/services/clusters/create_service.rb

@@ -10,10 +10,11 @@ module Clusters
       @current_user, @params = user, params.dup
     end
 
-    def execute(project:, access_token: nil)
-      raise ArgumentError, _('Instance does not support multiple Kubernetes clusters') unless can_create_cluster?(project)
+    def execute(access_token: nil)
+      raise ArgumentError, 'Unknown clusterable provided' unless clusterable
+      raise ArgumentError, _('Instance does not support multiple Kubernetes clusters') unless can_create_cluster?
 
-      cluster_params = params.merge(user: current_user, cluster_type: :project_type, projects: [project])
+      cluster_params = params.merge(user: current_user).merge(clusterable_params)
       cluster_params[:provider_gcp_attributes].try do |provider|
         provider[:access_token] = access_token
       end
@@ -29,9 +30,20 @@ module Clusters
       Clusters::Cluster.create(cluster_params)
     end
 
+    def clusterable
+      @clusterable ||= params.delete(:clusterable)
+    end
+
+    def clusterable_params
+      case clusterable
+      when ::Project
+        { cluster_type: :project_type, projects: [clusterable] }
+      end
+    end
+
     # EE would override this method
-    def can_create_cluster?(project)
-      project.clusters.empty?
+    def can_create_cluster?
+      clusterable.clusters.empty?
     end
   end
 end

app/views/clusters/_advanced_settings.html.haml

@@ -12,4 +12,4 @@
       = s_('ClusterIntegration|Remove Kubernetes cluster integration')
     %p
       = s_("ClusterIntegration|Remove this Kubernetes cluster's configuration from this project. This will not delete your actual Kubernetes cluster.")
-    = link_to(s_('ClusterIntegration|Remove integration'), namespace_project_cluster_path(@project.namespace, @project, @cluster.id), method: :delete, class: 'btn btn-danger', data: { confirm: s_("ClusterIntegration|Are you sure you want to remove this Kubernetes cluster's integration? This will not delete your actual Kubernetes cluster.")})
+    = link_to(s_('ClusterIntegration|Remove integration'), cluster_path(@cluster, clusterable_params), method: :delete, class: 'btn btn-danger', data: { confirm: s_("ClusterIntegration|Are you sure you want to remove this Kubernetes cluster's integration? This will not delete your actual Kubernetes cluster.")})

app/views/clusters/_cluster.html.haml

@@ -2,7 +2,7 @@
   .table-section.section-30
     .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Kubernetes cluster")
     .table-mobile-content
-      = link_to cluster.name, namespace_project_cluster_path(@project.namespace, @project, cluster)
+      = link_to cluster.name, cluster_page_path(cluster)
   .table-section.section-30
     .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Environment scope")
     .table-mobile-content= cluster.environment_scope
@@ -16,7 +16,7 @@
         class: "#{'is-checked' if cluster.enabled?} #{'is-disabled' if !cluster.can_toggle_cluster?}",
         "aria-label": s_("ClusterIntegration|Toggle Kubernetes Cluster"),
         disabled: !cluster.can_toggle_cluster?,
-        data: { endpoint: namespace_project_cluster_path(@project.namespace, @project, cluster, format: :json) } }
+        data: { endpoint: cluster_path(cluster, clusterable_params.merge(format: :json)) } }
         %input.js-project-feature-toggle-input{ type: "hidden", value: cluster.enabled? }
         = icon("spinner spin", class: "loading-icon")
         %span.toggle-icon

app/views/clusters/_empty_state.html.haml

@@ -7,6 +7,6 @@
       - link_to_help_page = link_to(_('Learn more about Kubernetes'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer')
       %p= s_('ClusterIntegration|Kubernetes clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way. %{link_to_help_page}').html_safe % { link_to_help_page: link_to_help_page}
 
-      - if can?(current_user, :create_cluster, @project)
+      - if can_create_cluster?
         .text-center
-          = link_to s_('ClusterIntegration|Add Kubernetes cluster'), new_project_cluster_path(@project), class: 'btn btn-success'
+          = link_to s_('ClusterIntegration|Add Kubernetes cluster'), new_cluster_page_path, class: 'btn btn-success'

app/views/clusters/_integration_form.html.haml

-= form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster, html: { class: 'cluster_integration_form' } do |field|
+= form_for @cluster, url: cluster_path(@cluster), as: :cluster, html: { class: 'cluster_integration_form' } do |field|
   = form_errors(@cluster)
+  = hidden_clusterable_fields
   .form-group
     %h5= s_('ClusterIntegration|Integration status')
     %label.append-bottom-0.js-cluster-enable-toggle-area
@@ -13,7 +14,7 @@
           = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked')
       .form-text.text-muted= s_('ClusterIntegration|Enable or disable GitLab\'s connection to your Kubernetes cluster.')
 
-  - if has_multiple_clusters?(@project)
+  - if has_multiple_clusters?
     .form-group
       %h5= s_('ClusterIntegration|Environment scope')
       = field.text_field :environment_scope, class: 'col-md-6 form-control js-select-on-focus', placeholder: s_('ClusterIntegration|Environment scope')
@@ -23,7 +24,7 @@
     .form-group
       = field.submit _('Save changes'), class: 'btn btn-success'
 
-  - unless has_multiple_clusters?(@project)
+  - unless has_multiple_clusters?
     %h5= s_('ClusterIntegration|Environment scope')
     %p
       %code *

app/views/clusters/gcp/_form.html.haml

@@ -12,14 +12,15 @@
 
 %p= link_to('Select a different Google account', @authorize_url)
 
-= form_for @gcp_cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url:  create_gcp_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field|
+= form_for @gcp_cluster, html: { class: 'js-gke-cluster-creation prepend-top-20', data: { token: token_in_session } }, url: create_gcp_clusters_path, as: :cluster do |field|
   = form_errors(@gcp_cluster)
+  = hidden_clusterable_fields
   .form-group
     = field.label :name, s_('ClusterIntegration|Kubernetes cluster name'), class: 'label-bold'
     = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Kubernetes cluster name')
   .form-group
     = field.label :environment_scope, s_('ClusterIntegration|Environment scope'), class: 'label-bold'
-    = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope')
+    = field.text_field :environment_scope, class: 'form-control', readonly: !has_multiple_clusters?, placeholder: s_('ClusterIntegration|Environment scope')
 
   = field.fields_for :provider_gcp, @gcp_cluster.provider_gcp do |provider_gcp_field|
     .form-group

app/views/clusters/gcp/_show.html.haml

@@ -6,8 +6,9 @@
     %span.input-group-append
       = clipboard_button(text: @cluster.name, title: s_('ClusterIntegration|Copy Kubernetes cluster name'), class: 'input-group-text btn-default')
 
-= form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field|
+= form_for @cluster, url: cluster_path(@cluster), as: :cluster do |field|
   = form_errors(@cluster)
+  = hidden_clusterable_fields
 
   = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field|
     .form-group

app/views/clusters/show.html.haml

 - @content_class = "limit-container-width" unless fluid_layout
-- add_to_breadcrumbs "Kubernetes Clusters", project_clusters_path(@project)
+- add_to_breadcrumbs "Kubernetes Clusters", clusters_page_path
 - breadcrumb_title @cluster.name
 - page_title _("Kubernetes Cluster")
+- manage_prometheus_path = edit_project_service_path(@cluster.project, 'prometheus') if @project
 
 - expanded = Rails.env.test?
 
-- status_path = status_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster.id, format: :json) if can?(current_user, :admin_cluster, @cluster)
+- status_path = status_cluster_path(@cluster.id, clusterable_params.merge(format: :json)) if can?(current_user, :admin_cluster, @cluster)
 .edit-cluster-form.js-edit-cluster-form{ data: { status_path: status_path,
-  install_helm_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :helm),
-  install_ingress_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :ingress),
-  install_prometheus_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :prometheus),
-  install_runner_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :runner),
-  install_jupyter_path: install_applications_namespace_project_cluster_path(@cluster.project.namespace, @cluster.project, @cluster, :jupyter),
+  install_helm_path: install_applications_cluster_path(@cluster, :helm, clusterable_params),
+  install_ingress_path: install_applications_cluster_path(@cluster, :ingress, clusterable_params),
+  install_prometheus_path: install_applications_cluster_path(@cluster, :prometheus, clusterable_params),
+  install_runner_path: install_applications_cluster_path(@cluster, :runner, clusterable_params),
+  install_jupyter_path: install_applications_cluster_path(@cluster, :jupyter, clusterable_params),
   toggle_status: @cluster.enabled? ? 'true': 'false',
   cluster_status: @cluster.status_name,
   cluster_status_reason: @cluster.status_reason,
   help_path: help_page_path('user/project/clusters/index.md', anchor: 'installing-applications'),
   ingress_help_path: help_page_path('user/project/clusters/index.md', anchor: 'getting-the-external-ip-address'),
   ingress_dns_help_path: help_page_path('topics/autodevops/quick_start_guide.md', anchor: 'point-dns-at-cluster-ip'),
-  manage_prometheus_path: edit_project_service_path(@cluster.project, 'prometheus') } }
+  manage_prometheus_path: manage_prometheus_path } }
 
   .js-cluster-application-notice
     .flash-container

app/views/clusters/user/_form.html.haml

-= form_for @user_cluster, url: create_user_namespace_project_clusters_path(@project.namespace, @project), as: :cluster do |field|
+= form_for @user_cluster, url: create_user_clusters_path, as: :cluster do |field|
   = form_errors(@user_cluster)
+  = hidden_clusterable_fields
   .form-group
     = field.label :name, s_('ClusterIntegration|Kubernetes cluster name'), class: 'label-bold'
     = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Kubernetes cluster name')
-  - if has_multiple_clusters?(@project)
+  - if has_multiple_clusters?
     .form-group
       = field.label :environment_scope, s_('ClusterIntegration|Environment scope'), class: 'label-bold'
       = field.text_field :environment_scope, class: 'form-control', placeholder: s_('ClusterIntegration|Environment scope')

app/views/clusters/user/_show.html.haml

-= form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field|
+= form_for @cluster, url: cluster_path(@cluster), as: :cluster do |field|
   = form_errors(@cluster)
+  = hidden_clusterable_fields
   .form-group
     = field.label :name, s_('ClusterIntegration|Kubernetes cluster name'), class: 'label-bold'
     = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Kubernetes cluster name')

config/routes.rb

@@ -81,6 +81,21 @@ Rails.application.routes.draw do
       resources :milestones, module: :boards, only: [:index]
     end
 
+    resources :clusters, only: [:update, :destroy] do
+      collection do
+        post :create_user
+        post :create_gcp
+      end
+
+      member do
+        scope :applications do
+          post '/:application', to: 'clusters/applications#create', as: :install_applications
+        end
+
+        get :status, format: :json
+      end
+    end
+
     # UserCallouts
     resources :user_callouts, only: [:create]
 
@@ -92,20 +107,7 @@ Rails.application.routes.draw do
   end
 
   concern :clusterable do
-    resources :clusters, except: [:edit, :create], controller: '/clusters' do
-      collection do
-        post :create_gcp
-        post :create_user
-      end
-
-      member do
-        get :status, format: :json
-
-        scope :applications do
-          post '/:application', to: '/clusters/applications#create', as: :install_applications
-        end
-      end
-    end
+    resources :clusters, only: [:index, :new, :show], controller: '/clusters'
   end
 
   draw :api

spec/controllers/clusters_controller_spec.rb

@@ -325,8 +325,8 @@ describe ClustersController do
 
     def go
       get :status,
-        namespace_id: project.namespace,
-        project_id: project,
+        namespace_id: project.namespace.to_param,
+        project_id: project.to_param,
         id: cluster,
         format: :json
     end
@@ -405,8 +405,8 @@ describe ClustersController do
     end
 
     def go(format: :html)
-      put :update, params.merge(namespace_id: project.namespace,
-                                project_id: project,
+      put :update, params.merge(namespace_id: project.namespace.to_param,
+                                project_id: project.to_param,
                                 id: cluster,
                                 format: format
                                )

spec/services/clusters/create_service_spec.rb

@@ -5,18 +5,43 @@ describe Clusters::CreateService do
   let(:project) { create(:project) }
   let(:user) { create(:user) }
 
-  subject { described_class.new(user, params).execute(project: project, access_token: access_token) }
+  subject { described_class.new(user, params).execute(access_token: access_token) }
 
   context 'when provider is gcp' do
     context 'when project has no clusters' do
       context 'when correct params' do
-        include_context 'valid cluster create params'
+        let(:params) do
+          {
+            name: 'test-cluster',
+            provider_type: :gcp,
+            provider_gcp_attributes: {
+              gcp_project_id: 'gcp-project',
+              zone: 'us-central1-a',
+              num_nodes: 1,
+              machine_type: 'machine_type-a',
+              legacy_abac: 'true'
+            },
+            clusterable: project
+          }
+        end
 
         include_examples 'create cluster service success'
       end
 
       context 'when invalid params' do
-        include_context 'invalid cluster create params'
+        let(:params) do
+          {
+            name: 'test-cluster',
+            provider_type: :gcp,
+            provider_gcp_attributes: {
+              gcp_project_id: '!!!!!!!',
+              zone: 'us-central1-a',
+              num_nodes: 1,
+              machine_type: 'machine_type-a'
+            },
+            clusterable: project
+          }
+        end
 
         include_examples 'create cluster service error'
       end