Dashboard: Add dashboard validation warning to save drawer (#55732)

* add api route for validating a dashboard json

* add feature flag for showDashboardValidationWarnings

* tidy up

* comments and messages

* swagger specs

* fix typo

* more swagger

* tests!

* tidy test a little bit

* no more ioutil

* api will return different status code depending on validation error

* clean up

* handle 4xx errors

* remove console.log

* fix backend tests

* tidy up

* Swagger: Exclude alpha endpoints

Co-authored-by: Sofia Papagiannaki <1632407+papagian@users.noreply.github.com>

Makefile

@@ -46,7 +46,8 @@ $(SPEC_TARGET): $(SWAGGER) ## Generate API Swagger specification
 	SWAGGER_GENERATE_EXTENSION=false $(SWAGGER) generate spec -m -w pkg/server -o $(SPEC_TARGET) \
 	-x "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" \
 	-x "github.com/prometheus/alertmanager" \
-	-i pkg/api/swagger_tags.json
+	-i pkg/api/swagger_tags.json \
+	--exclude-tag=alpha
 
 swagger-api-spec: gen-go $(SPEC_TARGET) $(MERGED_SPEC_TARGET) validate-api-spec
 

packages/grafana-data/src/types/featureToggles.gen.ts

@@ -74,5 +74,6 @@ export interface FeatureToggles {
   increaseInMemDatabaseQueryCache?: boolean;
   newPanelChromeUI?: boolean;
   queryLibrary?: boolean;
+  showDashboardValidationWarnings?: boolean;
   mysqlAnsiQuotes?: boolean;
 }

pkg/api/api.go

@@ -469,6 +469,7 @@ func (hs *HTTPServer) registerRoutes() {
 			})
 
 			dashboardRoute.Post("/calculate-diff", authorize(reqSignedIn, ac.EvalPermission(dashboards.ActionDashboardsWrite)), routing.Wrap(hs.CalculateDashboardDiff))
+			dashboardRoute.Post("/validate", authorize(reqSignedIn, ac.EvalPermission(dashboards.ActionDashboardsWrite)), routing.Wrap(hs.ValidateDashboard))
 			dashboardRoute.Post("/trim", routing.Wrap(hs.TrimDashboard))
 
 			dashboardRoute.Post("/db", authorize(reqSignedIn, ac.EvalAny(ac.EvalPermission(dashboards.ActionDashboardsCreate), ac.EvalPermission(dashboards.ActionDashboardsWrite))), routing.Wrap(hs.PostDashboard))

pkg/api/dashboard.go

@@ -752,6 +752,70 @@ func (hs *HTTPServer) GetDashboardVersion(c *models.ReqContext) response.Respons
 	return response.JSON(http.StatusOK, dashVersionMeta)
 }
 
+// swagger:route POST /dashboards/validate dashboards alpha validateDashboard
+//
+// Validates a dashboard JSON against the schema.
+//
+// Produces:
+// - application/json
+//
+// Responses:
+// 200: validateDashboardResponse
+// 412: validateDashboardResponse
+// 422: validateDashboardResponse
+// 401: unauthorisedError
+// 403: forbiddenError
+// 500: internalServerError
+func (hs *HTTPServer) ValidateDashboard(c *models.ReqContext) response.Response {
+	cmd := models.ValidateDashboardCommand{}
+
+	if err := web.Bind(c.Req, &cmd); err != nil {
+		return response.Error(http.StatusBadRequest, "bad request data", err)
+	}
+
+	cm := hs.Coremodels.Dashboard()
+	dashboardBytes := []byte(cmd.Dashboard)
+
+	// POST api receives dashboard as a string of json (so line numbers for errors stay consistent),
+	// but we need to parse the schema version out of it
+	dashboardJson, err := simplejson.NewJson(dashboardBytes)
+	if err != nil {
+		return response.Error(http.StatusBadRequest, "unable to parse dashboard", err)
+	}
+
+	schemaVersion, err := dashboardJson.Get("schemaVersion").Int()
+
+	isValid := false
+	statusCode := http.StatusOK
+	validationMessage := ""
+
+	// Only try to validate if the schemaVersion is at least the handoff version
+	// (the minimum schemaVersion against which the dashboard schema is known to
+	// work), or if schemaVersion is absent (which will happen once the Thema
+	// schema becomes canonical).
+	if err != nil || schemaVersion >= dashboard.HandoffSchemaVersion {
+		v, _ := cuectx.JSONtoCUE("dashboard.json", dashboardBytes)
+		_, validationErr := cm.CurrentSchema().Validate(v)
+
+		if validationErr == nil {
+			isValid = true
+		} else {
+			validationMessage = validationErr.Error()
+			statusCode = http.StatusUnprocessableEntity
+		}
+	} else {
+		validationMessage = "invalid schema version"
+		statusCode = http.StatusPreconditionFailed
+	}
+
+	respData := &ValidateDashboardResponse{
+		IsValid: isValid,
+		Message: validationMessage,
+	}
+
+	return response.JSON(statusCode, respData)
+}
+
 // swagger:route POST /dashboards/calculate-diff dashboards calculateDashboardDiff
 //
 // Perform diff on two dashboards.
@@ -1185,3 +1249,9 @@ type DashboardVersionResponse struct {
 	// in: body
 	Body *dashver.DashboardVersionMeta `json:"body"`
 }
+
+// swagger:response validateDashboardResponse
+type ValidateDashboardResponse struct {
+	IsValid bool   `json:"isValid"`
+	Message string `json:"message,omitempty"`
+}

pkg/api/dashboard_test.go

@@ -728,6 +728,60 @@ func TestDashboardAPIEndpoint(t *testing.T) {
 		})
 	})
 
+	t.Run("Given a dashboard to validate", func(t *testing.T) {
+		sqlmock := mockstore.SQLStoreMock{}
+
+		t.Run("When an invalid dashboard json is posted", func(t *testing.T) {
+			cmd := models.ValidateDashboardCommand{
+				Dashboard: "{\"hello\": \"world\"}",
+			}
+
+			role := org.RoleAdmin
+			postValidateScenario(t, "When calling POST on", "/api/dashboards/validate", "/api/dashboards/validate", cmd, role, func(sc *scenarioContext) {
+				callPostDashboard(sc)
+
+				result := sc.ToJSON()
+				assert.Equal(t, 422, sc.resp.Code)
+				assert.False(t, result.Get("isValid").MustBool())
+				assert.NotEmpty(t, result.Get("message").MustString())
+			}, &sqlmock)
+		})
+
+		t.Run("When a dashboard with a too-low schema version is posted", func(t *testing.T) {
+			cmd := models.ValidateDashboardCommand{
+				Dashboard: "{\"schemaVersion\": 1}",
+			}
+
+			role := org.RoleAdmin
+			postValidateScenario(t, "When calling POST on", "/api/dashboards/validate", "/api/dashboards/validate", cmd, role, func(sc *scenarioContext) {
+				callPostDashboard(sc)
+
+				result := sc.ToJSON()
+				assert.Equal(t, 412, sc.resp.Code)
+				assert.False(t, result.Get("isValid").MustBool())
+				assert.Equal(t, "invalid schema version", result.Get("message").MustString())
+			}, &sqlmock)
+		})
+
+		t.Run("When a valid dashboard is posted", func(t *testing.T) {
+			devenvDashboard, readErr := os.ReadFile("../../devenv/dev-dashboards/home.json")
+			assert.Empty(t, readErr)
+
+			cmd := models.ValidateDashboardCommand{
+				Dashboard: string(devenvDashboard),
+			}
+
+			role := org.RoleAdmin
+			postValidateScenario(t, "When calling POST on", "/api/dashboards/validate", "/api/dashboards/validate", cmd, role, func(sc *scenarioContext) {
+				callPostDashboard(sc)
+
+				result := sc.ToJSON()
+				assert.Equal(t, 200, sc.resp.Code)
+				assert.True(t, result.Get("isValid").MustBool())
+			}, &sqlmock)
+		})
+	})
+
 	t.Run("Given two dashboards being compared", func(t *testing.T) {
 		fakeDashboardVersionService := dashvertest.NewDashboardVersionServiceFake()
 		fakeDashboardVersionService.ExpectedDashboardVersions = []*dashver.DashboardVersion{
@@ -1053,6 +1107,42 @@ func postDashboardScenario(t *testing.T, desc string, url string, routePattern s
 	})
 }
 
+func postValidateScenario(t *testing.T, desc string, url string, routePattern string, cmd models.ValidateDashboardCommand,
+	role org.RoleType, fn scenarioFunc, sqlmock sqlstore.Store) {
+	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {
+		cfg := setting.NewCfg()
+		hs := HTTPServer{
+			Cfg:                   cfg,
+			ProvisioningService:   provisioning.NewProvisioningServiceMock(context.Background()),
+			Live:                  newTestLive(t, sqlstore.InitTestDB(t)),
+			QuotaService:          &quotaimpl.Service{Cfg: cfg},
+			LibraryPanelService:   &mockLibraryPanelService{},
+			LibraryElementService: &mockLibraryElementService{},
+			SQLStore:              sqlmock,
+			Features:              featuremgmt.WithFeatures(),
+			Coremodels:            registry.NewBase(nil),
+		}
+
+		sc := setupScenarioContext(t, url)
+		sc.defaultHandler = routing.Wrap(func(c *models.ReqContext) response.Response {
+			c.Req.Body = mockRequestBody(cmd)
+			c.Req.Header.Add("Content-Type", "application/json")
+			sc.context = c
+			sc.context.SignedInUser = &user.SignedInUser{
+				OrgID:  testOrgID,
+				UserID: testUserID,
+			}
+			sc.context.OrgRole = role
+
+			return hs.ValidateDashboard(c)
+		})
+
+		sc.m.Post(routePattern, sc.defaultHandler)
+
+		fn(sc)
+	})
+}
+
 func postDiffScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.CalculateDiffOptions,
 	role org.RoleType, fn scenarioFunc, sqlmock sqlstore.Store, fakeDashboardVersionService *dashvertest.FakeDashboardVersionService) {
 	t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) {

pkg/models/dashboards.go

@@ -224,6 +224,10 @@ type SaveDashboardCommand struct {
 	Result *Dashboard `json:"-"`
 }
 
+type ValidateDashboardCommand struct {
+	Dashboard string `json:"dashboard" binding:"Required"`
+}
+
 type TrimDashboardCommand struct {
 	Dashboard *simplejson.Json `json:"dashboard" binding:"Required"`
 	Meta      *simplejson.Json `json:"meta"`

pkg/services/featuremgmt/registry.go

@@ -322,6 +322,10 @@ var (
 			State:           FeatureStateAlpha,
 			RequiresDevMode: true,
 		},
+		{
+			Name:        "showDashboardValidationWarnings",
+			Description: "Show warnings when Dashboards do not validate against the schema",
+		},
 		{
 			Name:        "mysqlAnsiQuotes",
 			Description: "Use double quote to escape keyword in Mysql query",

pkg/services/featuremgmt/toggles_gen.go

@@ -239,6 +239,10 @@ const (
 	// Reusable query library
 	FlagQueryLibrary = "queryLibrary"
 
+	// FlagShowDashboardValidationWarnings
+	// Show warnings when Dashboards do not validate against the schema
+	FlagShowDashboardValidationWarnings = "showDashboardValidationWarnings"
+
 	// FlagMysqlAnsiQuotes
 	// Use double quote to escape keyword in Mysql query
 	FlagMysqlAnsiQuotes = "mysqlAnsiQuotes"

public/api-merged.json

@@ -20075,6 +20075,17 @@
       "schema": {
         "$ref": "#/definitions/UserProfileDTO"
       }
+    },
+    "validateDashboardResponse": {
+      "description": "(empty)",
+      "headers": {
+        "isValid": {
+          "type": "boolean"
+        },
+        "message": {
+          "type": "string"
+        }
+      }
     }
   },
   "securityDefinitions": {

public/api-spec.json

@@ -16115,6 +16115,17 @@
       "schema": {
         "$ref": "#/definitions/UserProfileDTO"
       }
+    },
+    "validateDashboardResponse": {
+      "description": "",
+      "headers": {
+        "isValid": {
+          "type": "boolean"
+        },
+        "message": {
+          "type": "string"
+        }
+      }
     }
   },
   "securityDefinitions": {

public/app/core/services/backend_srv.ts

@@ -18,6 +18,7 @@ import { BackendSrv as BackendService, BackendSrvRequest, config, FetchError, Fe
 import appEvents from 'app/core/app_events';
 import { getConfig } from 'app/core/config';
 import { loadUrlToken } from 'app/core/utils/urlToken';
+import { DashboardModel } from 'app/features/dashboard/state';
 import { DashboardSearchItem } from 'app/features/search/types';
 import { getGrafanaStorage } from 'app/features/storage/storage';
 import { TokenRevokedModal } from 'app/features/users/TokenRevokedModal';
@@ -455,6 +456,19 @@ export class BackendSrv implements BackendService {
     return this.get<DashboardDTO>(`/api/dashboards/uid/${uid}`);
   }
 
+  validateDashboard(dashboard: DashboardModel) {
+    // We want to send the dashboard as a JSON string (in the JSON body payload) so we can get accurate error line numbers back
+    const dashboardJson = JSON.stringify(dashboard, replaceJsonNulls, 2);
+
+    return this.request<ValidateDashboardResponse>({
+      method: 'POST',
+      url: `/api/dashboards/validate`,
+      data: { dashboard: dashboardJson },
+      showSuccessAlert: false,
+      showErrorAlert: false,
+    });
+  }
+
   getPublicDashboardByUid(uid: string) {
     return this.get<DashboardDTO>(`/api/public/dashboards/${uid}`);
   }
@@ -472,3 +486,15 @@ export class BackendSrv implements BackendService {
 // Used for testing and things that really need BackendSrv
 export const backendSrv = new BackendSrv();
 export const getBackendSrv = (): BackendSrv => backendSrv;
+
+interface ValidateDashboardResponse {
+  isValid: boolean;
+  message?: string;
+}
+
+function replaceJsonNulls<T extends unknown>(key: string, value: T): T | undefined {
+  if (typeof value === 'number' && !Number.isFinite(value)) {
+    return undefined;
+  }
+  return value;
+}

public/app/features/dashboard/components/SaveDashboard/DashboardValidation.tsx

@@ -0,0 +1,80 @@
+import { css } from '@emotion/css';
+import React from 'react';
+import { useAsync } from 'react-use';
+
+import { GrafanaTheme2 } from '@grafana/data';
+import { FetchError } from '@grafana/runtime';
+import { Alert, useStyles2 } from '@grafana/ui';
+import { backendSrv } from 'app/core/services/backend_srv';
+
+import { DashboardModel } from '../../state';
+
+interface DashboardValidationProps {
+  dashboard: DashboardModel;
+}
+
+type ValidationResponse = Awaited<ReturnType<typeof backendSrv.validateDashboard>>;
+
+function DashboardValidation({ dashboard }: DashboardValidationProps) {
+  const styles = useStyles2(getStyles);
+  const { loading, value, error } = useAsync(async () => {
+    const saveModel = dashboard.getSaveModelClone();
+    const respPromise = backendSrv
+      .validateDashboard(saveModel)
+      // API returns schema validation errors in 4xx range, so resolve them rather than throwing
+      .catch((err: FetchError<ValidationResponse>) => {
+        if (err.status >= 500) {
+          throw err;
+        }
+
+        return err.data;
+      });
+
+    return respPromise;
+  }, [dashboard]);
+
+  let alert: React.ReactNode;
+
+  if (loading) {
+    alert = <Alert severity="info" title="Checking dashboard validity" />;
+  } else if (value) {
+    if (!value.isValid) {
+      alert = (
+        <Alert severity="warning" title="Dashboard failed schema validation">
+          <p>
+            Validation is provided for development purposes and should be safe to ignore. If you are a Grafana
+            developer, consider checking and updating the dashboard schema
+          </p>
+          <div className={styles.error}>{value.message}</div>
+        </Alert>
+      );
+    }
+  } else {
+    const errorMessage = error?.message ?? 'Unknown error';
+    alert = (
+      <Alert severity="info" title="Error checking dashboard validity">
+        <p className={styles.error}>{errorMessage}</p>
+      </Alert>
+    );
+  }
+
+  if (alert) {
+    return <div className={styles.root}>{alert}</div>;
+  }
+
+  return null;
+}
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  root: css({
+    marginTop: theme.spacing(1),
+  }),
+  error: css({
+    fontFamily: theme.typography.fontFamilyMonospace,
+    whiteSpace: 'pre-wrap',
+    overflowX: 'auto',
+    maxWidth: '100%',
+  }),
+});
+
+export default DashboardValidation;

public/app/features/dashboard/components/SaveDashboard/SaveDashboardDrawer.tsx

@@ -7,6 +7,7 @@ import { backendSrv } from 'app/core/services/backend_srv';
 
 import { jsonDiff } from '../VersionHistory/utils';
 
+import DashboardValidation from './DashboardValidation';
 import { SaveDashboardDiff } from './SaveDashboardDiff';
 import { SaveDashboardErrorProxy } from './SaveDashboardErrorProxy';
 import { SaveDashboardAsForm } from './forms/SaveDashboardAsForm';
@@ -68,7 +69,7 @@ export const SaveDashboardDrawer = ({ dashboard, onDismiss, onSaveSuccess, isCop
       }
     : onDismiss;
 
-  const renderBody = () => {
+  const renderSaveBody = () => {
     if (showDiff) {
       return <SaveDashboardDiff diff={data.diff} oldValue={previous.value} newValue={data.clone} />;
     }
@@ -161,7 +162,9 @@ export const SaveDashboardDrawer = ({ dashboard, onDismiss, onSaveSuccess, isCop
       expandable
       scrollableContent
     >
-      {renderBody()}
+      {renderSaveBody()}
+
+      {config.featureToggles.showDashboardValidationWarnings && <DashboardValidation dashboard={dashboard} />}
     </Drawer>
   );
 };

public/openapi3.json

@@ -1731,6 +1731,21 @@
           }
         },
         "description": "(empty)"
+      },
+      "validateDashboardResponse": {
+        "description": "(empty)",
+        "headers": {
+          "isValid": {
+            "schema": {
+              "type": "boolean"
+            }
+          },
+          "message": {
+            "schema": {
+              "type": "string"
+            }
+          }
+        }
       }
     },
     "schemas": {