Defend against dashboard errors, rework sequence

5 files changed, 46 insertions, 9 deletions

diff --git a/lib/gitlab/metrics_dashboard/processor.rb b/lib/gitlab/metrics_dashboard/processor.rb
index ef9d75947f0..8fa95f1ee2d 100644
--- a/lib/gitlab/metrics_dashboard/processor.rb
+++ b/lib/gitlab/metrics_dashboard/processor.rb
@@ -3,23 +3,21 @@
 module Gitlab
   module MetricsDashboard
     # Responsible for processesing a dashboard hash, inserting
-    # relevantDB records & sorting for proper rendering in
+    # relevant DB records & sorting for proper rendering in
     # the UI. These includes shared metric info, custom metrics
     # info, and alerts (only in EE).
     class Processor
+      SEQUENCE = [
+        Stages::CommonMetricsInserter,
+        Stages::ProjectMetricsInserter,
+        Stages::Sorter
+      ].freeze
+
       def initialize(project, environment)
         @project = project
         @environment = environment
       end
 
-      def sequence
-        [
-          Stages::CommonMetricsInserter,
-          Stages::ProjectMetricsInserter,
-          Stages::Sorter
-        ]
-      end
-
       # Returns a new dashboard hash with the results of
       # running transforms on the dashboard.
       def process(dashboard)
@@ -30,6 +28,12 @@ module Gitlab
 
         dashboard
       end
+
+      private
+
+      def sequence
+        SEQUENCE
+      end
     end
   end
 end
diff --git a/lib/gitlab/metrics_dashboard/service.rb b/lib/gitlab/metrics_dashboard/service.rb
index a65f01ca54e..84f174c7d1b 100644
--- a/lib/gitlab/metrics_dashboard/service.rb
+++ b/lib/gitlab/metrics_dashboard/service.rb
@@ -14,6 +14,8 @@ module Gitlab
         dashboard = process_dashboard(dashboard_string)
 
         success(dashboard: dashboard)
+      rescue Gitlab::MetricsDashboard::Stages::BaseStage::DashboardLayoutError => e
+        error(e.message, :unprocessable_entity)
       end
 
       private
@@ -27,6 +29,7 @@ module Gitlab
         "metrics_dashboard_#{SYSTEM_DASHBOARD_NAME}"
       end
 
+      # Returns a new dashboard Hash, supplemented with DB info
       def process_dashboard(dashboard)
         Processor.new(project, params[:environment]).process(dashboard)
       end
diff --git a/lib/gitlab/metrics_dashboard/stages/base_stage.rb b/lib/gitlab/metrics_dashboard/stages/base_stage.rb
index bdbf0c196cc..df49126a07b 100644
--- a/lib/gitlab/metrics_dashboard/stages/base_stage.rb
+++ b/lib/gitlab/metrics_dashboard/stages/base_stage.rb
@@ -4,6 +4,8 @@ module Gitlab
   module MetricsDashboard
     module Stages
       class BaseStage
+        DashboardLayoutError = Class.new(StandardError)
+
         DEFAULT_PANEL_TYPE = 'area-chart'
 
         attr_reader :project, :environment
@@ -23,9 +25,27 @@ module Gitlab
 
         protected
 
+        def missing_panel_groups!
+          raise DashboardLayoutError.new('Top-level key :panel_groups must be an array')
+        end
+
+        def missing_panels!
+          raise DashboardLayoutError.new('Each "panel_group" must define an array :panels')
+        end
+
+        def missing_metrics!
+          raise DashboardLayoutError.new('Each "panel" must define an array :metrics')
+        end
+
         def for_metrics(dashboard)
+          missing_panel_groups! unless dashboard[:panel_groups].is_a?(Array)
+
           dashboard[:panel_groups].each do |panel_group|
+            missing_panels! unless panel_group[:panels].is_a?(Array)
+
             panel_group[:panels].each do |panel|
+              missing_metrics! unless panel[:metrics].is_a?(Array)
+
               panel[:metrics].each do |metric|
                 yield metric
               end
diff --git a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb
index 8edb21c89c1..ab33ee75cce 100644
--- a/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb
+++ b/lib/gitlab/metrics_dashboard/stages/project_metrics_inserter.rb
@@ -60,15 +60,21 @@ module Gitlab
         end
 
         def find_panel_group(panel_groups, metric)
+          return unless panel_groups
+
           panel_groups.find { |group| group[:group] == metric.group_title }
         end
 
         def find_panel(panels, metric)
+          return unless panels
+
           panel_identifiers = [DEFAULT_PANEL_TYPE, metric.title, metric.y_label]
           panels.find { |panel| panel.values_at(:type, :title, :y_label) == panel_identifiers }
         end
 
         def find_metric(metrics, metric)
+          return unless metrics
+
           metrics.find { |m| m[:id] == metric.identifier }
         end
 
diff --git a/lib/gitlab/metrics_dashboard/stages/sorter.rb b/lib/gitlab/metrics_dashboard/stages/sorter.rb
index a2429e65efa..ca310c9637a 100644
--- a/lib/gitlab/metrics_dashboard/stages/sorter.rb
+++ b/lib/gitlab/metrics_dashboard/stages/sorter.rb
@@ -5,6 +5,8 @@ module Gitlab
     module Stages
       class Sorter < BaseStage
         def transform!(dashboard)
+          missing_panel_groups! unless dashboard[:panel_groups].is_a? Array
+
           sort_groups!(dashboard)
           sort_panels!(dashboard)
         end
@@ -19,6 +21,8 @@ module Gitlab
         # Sorts the panels in the dashboard by the :weight key
         def sort_panels!(dashboard)
           dashboard[:panel_groups].each do |group|
+            missing_panels! unless group[:panels].is_a? Array
+
             group[:panels] = group[:panels].sort_by { |panel| -panel[:weight].to_i }
           end
         end