Merge branch '212873-allow-cloning-of-out-of-the-box-dashboards' into 'master'

Allow cloning of all out of the box dashboards

See merge request gitlab-org/gitlab!35607

app/services/metrics/dashboard/clone_dashboard_service.rb

@@ -6,6 +6,7 @@ module Metrics
   module Dashboard
     class CloneDashboardService < ::BaseService
       include Stepable
+      include Gitlab::Utils::StrongMemoize
 
       ALLOWED_FILE_TYPE = '.yml'
       USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT
@@ -18,15 +19,17 @@ module Metrics
         :refresh_repository_method_caches
 
       class << self
-        def allowed_dashboard_templates
-          @allowed_dashboard_templates ||= Set[::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH].freeze
-        end
-
         def sequences
           @sequences ||= {
-            ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
-                                                                             ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter,
-                                                                             ::Gitlab::Metrics::Dashboard::Stages::Sorter].freeze
+            ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [
+              ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,
+              ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter,
+              ::Gitlab::Metrics::Dashboard::Stages::Sorter
+            ].freeze,
+
+            ::Metrics::Dashboard::SelfMonitoringDashboardService::DASHBOARD_PATH => [
+              ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter
+            ].freeze
           }.freeze
         end
       end
@@ -56,8 +59,12 @@ module Metrics
         success(result)
       end
 
+      # Only allow out of the box metrics dashboards to be cloned. This can be
+      # changed to allow cloning of any metrics dashboard, if desired.
+      # However, only metrics dashboards should be allowed. If any file is
+      # allowed to be cloned, this will become a security risk.
       def check_dashboard_template(result)
-        return error(_('Not found.'), :not_found) unless self.class.allowed_dashboard_templates.include?(params[:dashboard])
+        return error(_('Not found.'), :not_found) unless dashboard_service&.out_of_the_box_dashboard?
 
         success(result)
       end
@@ -78,6 +85,12 @@ module Metrics
         success(result.merge(http_status: :created, dashboard: dashboard_details))
       end
 
+      def dashboard_service
+        strong_memoize(:dashboard_service) do
+          Gitlab::Metrics::Dashboard::ServiceSelector.call(dashboard_service_options)
+        end
+      end
+
       def dashboard_attrs
         {
           commit_message: params[:commit_message],
@@ -149,11 +162,18 @@ module Metrics
       end
 
       def raw_dashboard
-        YAML.safe_load(File.read(Rails.root.join(dashboard_template)))
+        dashboard_service.new(project, current_user, dashboard_service_options).raw_dashboard
+      end
+
+      def dashboard_service_options
+        {
+          embedded: false,
+          dashboard_path: dashboard_template
+        }
       end
 
       def sequence
-        self.class.sequences[dashboard_template]
+        self.class.sequences[dashboard_template] || []
       end
     end
   end

ee/app/services/ee/metrics/dashboard/clone_dashboard_service.rb

@@ -11,11 +11,6 @@ module EE
         class_methods do
           extend ::Gitlab::Utils::Override
 
-          override :allowed_dashboard_templates
-          def allowed_dashboard_templates
-            @allowed_dashboard_templates ||= (Set[::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH] + super).freeze
-          end
-
           override :sequences
           def sequences
             super.merge(::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH => [::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter,

spec/services/metrics/dashboard/clone_dashboard_service_spec.rb

@@ -83,6 +83,10 @@ RSpec.describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memor
 
         it_behaves_like 'valid dashboard cloning process', ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH, [::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter, ::Gitlab::Metrics::Dashboard::Stages::Sorter]
 
+        it_behaves_like 'valid dashboard cloning process',
+          ::Metrics::Dashboard::SelfMonitoringDashboardService::DASHBOARD_PATH,
+          [::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter]
+
         context 'selected branch already exists' do
           let(:branch) { 'existing_branch' }