Merge branch '219395-metrics-dashboard-validation-fix-500-for-empty-' into 'master'

Fix metrics dashboard YAML validation

See merge request gitlab-org/gitlab!33935

app/models/performance_monitoring/prometheus_dashboard.rb

@@ -11,12 +11,7 @@ module PerformanceMonitoring
 
     class << self
       def from_json(json_content)
-        dashboard = new(
-          dashboard: json_content['dashboard'],
-          panel_groups: json_content['panel_groups']&.map { |group| PrometheusPanelGroup.from_json(group) }
-        )
-
-        dashboard.tap(&:validate!)
+        build_from_hash(json_content).tap(&:validate!)
       end
 
       def find_for(project:, user:, path:, options: {})
@@ -30,6 +25,17 @@ module PerformanceMonitoring
           }.merge(dashboard_response[:dashboard])
         )
       end
+
+      private
+
+      def build_from_hash(attributes)
+        return new unless attributes.is_a?(Hash)
+
+        new(
+          dashboard: attributes['dashboard'],
+          panel_groups: attributes['panel_groups']&.map { |group| PrometheusPanelGroup.from_json(group) }
+        )
+      end
     end
 
     def to_yaml

app/models/performance_monitoring/prometheus_metric.rb

@@ -10,16 +10,24 @@ module PerformanceMonitoring
     validates :query, presence: true, unless: :query_range
     validates :query_range, presence: true, unless: :query
 
-    def self.from_json(json_content)
-      metric = PrometheusMetric.new(
-        id: json_content['id'],
-        unit: json_content['unit'],
-        label: json_content['label'],
-        query: json_content['query'],
-        query_range: json_content['query_range']
-      )
+    class << self
+      def from_json(json_content)
+        build_from_hash(json_content).tap(&:validate!)
+      end
 
-      metric.tap(&:validate!)
+      private
+
+      def build_from_hash(attributes)
+        return new unless attributes.is_a?(Hash)
+
+        new(
+          id: attributes['id'],
+          unit: attributes['unit'],
+          label: attributes['label'],
+          query: attributes['query'],
+          query_range: attributes['query_range']
+        )
+      end
     end
   end
 end

app/models/performance_monitoring/prometheus_panel.rb

@@ -8,17 +8,24 @@ module PerformanceMonitoring
 
     validates :title, presence: true
     validates :metrics, presence: true
+    class << self
+      def from_json(json_content)
+        build_from_hash(json_content).tap(&:validate!)
+      end
 
-    def self.from_json(json_content)
-      panel = new(
-        type: json_content['type'],
-        title: json_content['title'],
-        y_label: json_content['y_label'],
-        weight: json_content['weight'],
-        metrics: json_content['metrics']&.map { |metric| PrometheusMetric.from_json(metric) }
-      )
+      private
 
-      panel.tap(&:validate!)
+      def build_from_hash(attributes)
+        return new unless attributes.is_a?(Hash)
+
+        new(
+          type: attributes['type'],
+          title: attributes['title'],
+          y_label: attributes['y_label'],
+          weight: attributes['weight'],
+          metrics: attributes['metrics']&.map { |metric| PrometheusMetric.from_json(metric) }
+        )
+      end
     end
 
     def id(group_title)

app/models/performance_monitoring/prometheus_panel_group.rb

@@ -8,15 +8,22 @@ module PerformanceMonitoring
 
     validates :group, presence: true
     validates :panels, presence: true
+    class << self
+      def from_json(json_content)
+        build_from_hash(json_content).tap(&:validate!)
+      end
 
-    def self.from_json(json_content)
-      panel_group = new(
-        group: json_content['group'],
-        priority: json_content['priority'],
-        panels: json_content['panels']&.map { |panel| PrometheusPanel.from_json(panel) }
-      )
+      private
 
-      panel_group.tap(&:validate!)
+      def build_from_hash(attributes)
+        return new unless attributes.is_a?(Hash)
+
+        new(
+          group: attributes['group'],
+          priority: attributes['priority'],
+          panels: attributes['panels']&.map { |panel| PrometheusPanel.from_json(panel) }
+        )
+      end
     end
   end
 end

changelogs/unreleased/219395-metrics-dashboard-validation-fix-500-for-empty-.yml

+---
+title: Fixed dashboard YAML file validaiton for files which do not contain object
+  as root element
+merge_request: 33935
+author:
+type: fixed

spec/models/performance_monitoring/prometheus_dashboard_spec.rb

@@ -47,6 +47,24 @@ describe PerformanceMonitoring::PrometheusDashboard do
         end
       end
 
+      context 'dashboard content is missing' do
+        let(:json_content) { nil }
+
+        it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"]
+      end
+
+      context 'dashboard content is NOT a hash' do
+        let(:json_content) { YAML.safe_load("'test'") }
+
+        it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"]
+      end
+
+      context 'content is an array' do
+        let(:json_content) { [{ "dashboard" => "Dashboard Title" }] }
+
+        it_behaves_like 'validation failed', panel_groups: ["can't be blank"], dashboard: ["can't be blank"]
+      end
+
       context 'dashboard definition is missing panels_groups and dashboard keys' do
         let(:json_content) do
           {

spec/models/performance_monitoring/prometheus_metric_spec.rb

@@ -24,6 +24,14 @@ describe PerformanceMonitoring::PrometheusMetric do
     end
 
     describe 'validations' do
+      context 'json_content is not a hash' do
+        let(:json_content) { nil }
+
+        subject { described_class.from_json(json_content) }
+
+        it { expect { subject }.to raise_error(ActiveModel::ValidationError) }
+      end
+
       context 'when unit is missing' do
         before do
           json_content['unit'] = nil

spec/models/performance_monitoring/prometheus_panel_group_spec.rb

@@ -30,6 +30,14 @@ describe PerformanceMonitoring::PrometheusPanelGroup do
     end
 
     describe 'validations' do
+      context 'json_content is not a hash' do
+        let(:json_content) { nil }
+
+        subject { described_class.from_json(json_content) }
+
+        it { expect { subject }.to raise_error(ActiveModel::ValidationError) }
+      end
+
       context 'when group is missing' do
         before do
           json_content.delete('group')

spec/models/performance_monitoring/prometheus_panel_spec.rb

@@ -42,6 +42,14 @@ describe PerformanceMonitoring::PrometheusPanel do
     end
 
     describe 'validations' do
+      context 'json_content is not a hash' do
+        let(:json_content) { nil }
+
+        subject { described_class.from_json(json_content) }
+
+        it { expect { subject }.to raise_error(ActiveModel::ValidationError) }
+      end
+
       context 'when title is missing' do
         before do
           json_content['title'] = nil