Merge branch 'mwaw/210289-add-metrics-dashboard-validation-to-grapql' into 'master'

Add metrics dashboard schema validation as graphQL field for metrics dashboard resource

Closes #210289

See merge request gitlab-org/gitlab!33592

app/graphql/types/metrics/dashboard_type.rb

@@ -10,6 +10,9 @@ module Types
       field :path, GraphQL::STRING_TYPE, null: true,
             description: 'Path to a file with the dashboard definition'
 
+      field :schema_validation_warnings, [GraphQL::STRING_TYPE], null: true,
+            description: 'Dashboard schema validation warnings'
+
       field :annotations, Types::Metrics::Dashboards::AnnotationType.connection_type, null: true,
             description: 'Annotations added to the dashboard',
             resolver: Resolvers::Metrics::Dashboards::AnnotationResolver

app/models/performance_monitoring/prometheus_dashboard.rb

@@ -15,15 +15,17 @@ module PerformanceMonitoring
       end
 
       def find_for(project:, user:, path:, options: {})
-        dashboard_response = Gitlab::Metrics::Dashboard::Finder.find(project, user, options.merge(dashboard_path: path))
-        return unless dashboard_response[:status] == :success
-
-        new(
-          {
-            path: path,
-            environment: options[:environment]
-          }.merge(dashboard_response[:dashboard])
-        )
+        template = { path: path, environment: options[:environment] }
+        rsp = Gitlab::Metrics::Dashboard::Finder.find(project, user, options.merge(dashboard_path: path))
+
+        case rsp[:http_status] || rsp[:status]
+        when :success
+          new(template.merge(rsp[:dashboard] || {})) # when there is empty dashboard file returned rsp is still a success
+        when :unprocessable_entity
+          new(template) # validation error
+        else
+          nil # any other error
+        end
       end
 
       private
@@ -42,6 +44,15 @@ module PerformanceMonitoring
       self.as_json(only: yaml_valid_attributes).to_yaml
     end
 
+    # This method is planned to be refactored as a part of https://gitlab.com/gitlab-org/gitlab/-/issues/219398
+    # implementation. For new existing logic was reused to faster deliver MVC
+    def schema_validation_warnings
+      self.class.from_json(self.as_json)
+      nil
+    rescue ActiveModel::ValidationError => exception
+      exception.model.errors.map { |attr, error| "#{attr}: #{error}" }
+    end
+
     private
 
     def yaml_valid_attributes

changelogs/unreleased/mwaw-210289-add-metrics-dashboard-validation-to-grapql.yml

+---
+title: Add dashboard schema validation warnings as metrics dashboard GraphQL field
+merge_request: 33592
+author:
+type: added

doc/api/graphql/reference/gitlab_schema.graphql

@@ -7047,6 +7047,11 @@ type MetricsDashboard {
   Path to a file with the dashboard definition
   """
   path: String
+
+  """
+  Dashboard schema validation warnings
+  """
+  schemaValidationWarnings: [String!]
 }
 
 type MetricsDashboardAnnotation {

doc/api/graphql/reference/gitlab_schema.json

@@ -19633,6 +19633,28 @@
               },
               "isDeprecated": false,
               "deprecationReason": null
+            },
+            {
+              "name": "schemaValidationWarnings",
+              "description": "Dashboard schema validation warnings",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "LIST",
+                "name": null,
+                "ofType": {
+                  "kind": "NON_NULL",
+                  "name": null,
+                  "ofType": {
+                    "kind": "SCALAR",
+                    "name": "String",
+                    "ofType": null
+                  }
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
             }
           ],
           "inputFields": null,

doc/api/graphql/reference/index.md

@@ -1060,6 +1060,7 @@ Autogenerated return type of MergeRequestSetWip
 | Name  | Type  | Description |
 | ---   |  ---- | ----------  |
 | `path` | String | Path to a file with the dashboard definition |
+| `schemaValidationWarnings` | String! => Array | Dashboard schema validation warnings |
 
 ## MetricsDashboardAnnotation
 

doc/user/project/integrations/prometheus.md

@@ -362,6 +362,8 @@ When **Metrics Dashboard YAML definition is invalid** at least one of the follow
 1. `query_range: can't be blank` [learn more](#metrics-metrics-properties)
 1. `unit: can't be blank` [learn more](#metrics-metrics-properties)
 
+Metrics Dashboard YAML definition validation information is also available as a [GraphQL API field](../../../api/graphql/reference/index.md#metricsdashboard)
+
 #### Dashboard YAML properties
 
 Dashboards have several components:

spec/graphql/types/metrics/dashboard_type_spec.rb

@@ -7,7 +7,7 @@ describe GitlabSchema.types['MetricsDashboard'] do
 
   it 'has the expected fields' do
     expected_fields = %w[
-        path annotations
+        path annotations schema_validation_warnings
       ]
 
     expect(described_class).to have_graphql_fields(*expected_fields)

spec/models/performance_monitoring/prometheus_dashboard_spec.rb

@@ -42,7 +42,7 @@ describe PerformanceMonitoring::PrometheusDashboard do
         it 'raises error with corresponding messages', :aggregate_failures do
           expect { subject }.to raise_error do |error|
             expect(error).to be_kind_of(ActiveModel::ValidationError)
-            expect(error.model.errors.messages).to eql(errors_messages)
+            expect(error.model.errors.messages).to eq(errors_messages)
           end
         end
       end
@@ -190,20 +190,51 @@ describe PerformanceMonitoring::PrometheusDashboard do
         dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment })
 
         expect(dashboard_instance).to be_instance_of described_class
-        expect(dashboard_instance.environment).to be environment
-        expect(dashboard_instance.path).to be path
+        expect(dashboard_instance.environment).to eq environment
+        expect(dashboard_instance.path).to eq path
       end
     end
 
     context 'dashboard has NOT been found' do
       it 'returns nil' do
-        allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(status: :error)
+        allow(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(http_status: :not_found)
 
         dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment })
 
         expect(dashboard_instance).to be_nil
       end
     end
+
+    context 'dashboard has invalid schema', :aggregate_failures do
+      it 'still returns dashboard object' do
+        expect(Gitlab::Metrics::Dashboard::Finder).to receive(:find).and_return(http_status: :unprocessable_entity)
+
+        dashboard_instance = described_class.find_for(project: project, user: user, path: path, options: { environment: environment })
+
+        expect(dashboard_instance).to be_instance_of described_class
+        expect(dashboard_instance.environment).to eq environment
+        expect(dashboard_instance.path).to eq path
+      end
+    end
+  end
+
+  describe '#schema_validation_warnings' do
+    context 'when schema is valid' do
+      it 'returns nil' do
+        expect(described_class).to receive(:from_json)
+        expect(described_class.new.schema_validation_warnings).to be_nil
+      end
+    end
+
+    context 'when schema is invalid' do
+      it 'returns array with errors messages' do
+        instance = described_class.new
+        instance.errors.add(:test, 'test error')
+
+        expect(described_class).to receive(:from_json).and_raise(ActiveModel::ValidationError.new(instance))
+        expect(described_class.new.schema_validation_warnings).to eq ['test: test error']
+      end
+    end
   end
 
   describe '#to_yaml' do

spec/requests/api/graphql/metrics/dashboard_query_spec.rb

@@ -9,25 +9,19 @@ describe 'Getting Metrics Dashboard' do
   let(:project) { create(:project) }
   let!(:environment) { create(:environment, project: project) }
 
-  let(:fields) do
-    <<~QUERY
-      #{all_graphql_fields_for('MetricsDashboard'.classify)}
-    QUERY
-  end
-
   let(:query) do
-    %(
-      query {
-        project(fullPath:"#{project.full_path}") {
-          environments(name: "#{environment.name}") {
-            nodes {
-              metricsDashboard(path: "#{path}"){
-                #{fields}
-              }
-            }
-          }
-        }
-      }
+    graphql_query_for(
+      'project', { 'fullPath' => project.full_path },
+      query_graphql_field(
+        :environments, { 'name' => environment.name },
+        query_graphql_field(
+          :nodes, nil,
+          query_graphql_field(
+            :metricsDashboard, { 'path' => path },
+            all_graphql_fields_for('MetricsDashboard'.classify)
+          )
+        )
+      )
     )
   end
 
@@ -63,7 +57,29 @@ describe 'Getting Metrics Dashboard' do
       it 'returns metrics dashboard' do
         dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard']
 
-        expect(dashboard).to eql("path" => path)
+        expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => nil)
+      end
+
+      context 'invalid dashboard' do
+        let(:path) { '.gitlab/dashboards/metrics.yml' }
+        let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "---\ndasboard: ''" }) }
+
+        it 'returns metrics dashboard' do
+          dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard')
+
+          expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: can't be blank"])
+        end
+      end
+
+      context 'empty dashboard' do
+        let(:path) { '.gitlab/dashboards/metrics.yml' }
+        let(:project) { create(:project, :repository, :custom_repo, namespace: current_user.namespace, files: { path => "" }) }
+
+        it 'returns metrics dashboard' do
+          dashboard = graphql_data.dig('project', 'environments', 'nodes', 0, 'metricsDashboard')
+
+          expect(dashboard).to eql("path" => path, "schemaValidationWarnings" => ["dashboard: can't be blank", "panel_groups: can't be blank"])
+        end
       end
     end
 
@@ -72,7 +88,7 @@ describe 'Getting Metrics Dashboard' do
 
       it_behaves_like 'a working graphql query'
 
-      it 'return snil' do
+      it 'returns nil' do
         dashboard = graphql_data.dig('project', 'environments', 'nodes')[0]['metricsDashboard']
 
         expect(dashboard).to be_nil