[ENHANCEMENT] Allow migration of empty panel dashboard

perses · Mar 24, 2024 · af595e6 · af595e6
1 parent 2d8523a
commit af595e6
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 94 deletions.
diff --git a/internal/api/migrate/mapping.cuepart b/internal/api/migrate/mapping.cuepart
@@ -128,52 +128,55 @@ spec: {
         }
     }]
     // go through the top-level panels 1st time, to fill the panels section of Perses
-    panels: { for grafanaPanelId, grafanaPanel in #grafanaDashboard.panels {
-        // function-like pattern to factorize the panel conversion logic
-        _panelConversion: {
-            #panel: _
 
-            kind: "Panel"
-            spec: {
-                display: {
-                    name: [ // switch
-                        if #panel.title != _|_ if #panel.title != "" {
-                            #panel.title
-                        },
-                        "empty"
-                    ][0]
-                    description: *#panel.description | ""
-                }
-                plugin: [ // switch
-                    %(conditional_panels)
-                ][0]
-                // in case of no targets, the resulting empty array will be removed via unmarshaling in Go (omitempty)
-                queries: [ if #panel.targets != _|_ for _, target in #panel.targets {
-                    kind: "TimeSeriesQuery"
-                    spec: {
-                        #target: target
-                        plugin: [ // switch
-                            %(conditional_timeseries_queries)
+    if #grafanaDashboard.panels != null {
+        panels: {for grafanaPanelId, grafanaPanel in #grafanaDashboard.panels {
+            // function-like pattern to factorize the panel conversion logic
+            _panelConversion: {
+                #panel: _
+
+                kind: "Panel"
+                spec: {
+                    display: {
+                        name: [ // switch
+                            if #panel.title != _|_ if #panel.title != "" {
+                                #panel.title
+                            },
+                            "empty"
                         ][0]
+                        description: *#panel.description | ""
                     }
-                }]
+                    plugin: [ // switch
+                        %(conditional_panels)
+                    ][0]
+                    // in case of no targets, the resulting empty array will be removed via unmarshaling in Go (omitempty)
+                    queries: [ if #panel.targets != _|_ for _, target in #panel.targets {
+                        kind: "TimeSeriesQuery"
+                        spec: {
+                            #target: target
+                            plugin: [ // switch
+                                %(conditional_timeseries_queries)
+                            ][0]
+                        }
+                    }]
+                }
             }
-        }
-        // if the current panel is a row, go through its children panels
-        if grafanaPanel.panels != _|_ {
-            for innerPanelId, innerPanel in grafanaPanel.panels { 
-                "\(grafanaPanelId)_\(innerPanelId)": {
-                    _panelConversion & {#panel: innerPanel}
+            // if the current panel is a row, go through its children panels
+            if grafanaPanel.panels != _|_ {
+                for innerPanelId, innerPanel in grafanaPanel.panels { 
+                    "\(grafanaPanelId)_\(innerPanelId)": {
+                        _panelConversion & {#panel: innerPanel}
+                    }
                 }
             }
-        }
-        // else
-        if grafanaPanel.panels == _|_ { 
-            "\(grafanaPanelId)": {
-                _panelConversion & {#panel: grafanaPanel}
+            // else
+            if grafanaPanel.panels == _|_ { 
+                "\(grafanaPanelId)": {
+                    _panelConversion & {#panel: grafanaPanel}
+                }
             }
-        }
-    }}
+        }}
+    }
     // function-like pattern to factorize the position conversion logic
     _positionConversion: {
         #gridPos: _
@@ -187,35 +190,37 @@ spec: {
             "$ref": "#/spec/panels/\(#id)"
         }
     }
+    if #grafanaDashboard.panels != null {
     // we have to go through the panels again here, since in Grafana a panel embeds its position info while in Perses panels & layouts are split
-    layouts: [
+        layouts: [
         // create a first grid to gather standalone panels, if there are some (= only if first encountered panel is not a row)
-        if #grafanaDashboard.panels[0] != _|_ if #grafanaDashboard.panels[0].panels == _|_ {
-            {
+            if #grafanaDashboard.panels[0] != _|_ if #grafanaDashboard.panels[0].panels == _|_ {
+                {
+                    kind: "Grid"
+                    spec: {
+                        // go through the top-level panels a 2nd time and append an item only for the ones that are not rows neither belong to a row
+                        items: [ for grafanaPanelId, grafanaPanel in #grafanaDashboard.panels if grafanaPanel.panels == _|_ {
+                            _positionConversion & {#gridPos: grafanaPanel.gridPos, #id: grafanaPanelId}
+                        }]
+                    }
+                }
+            },
+            // go through the top-level panels a 3rd time & match only the rows, to create the corresponding grids
+            for grafanaPanelId, grafanaPanel in #grafanaDashboard.panels if grafanaPanel.panels != _|_ { // if the panel is a row
                 kind: "Grid"
                 spec: {
-                    // go through the top-level panels a 2nd time and append an item only for the ones that are not rows neither belong to a row
-                    items: [ for grafanaPanelId, grafanaPanel in #grafanaDashboard.panels if grafanaPanel.panels == _|_ {
-                        _positionConversion & {#gridPos: grafanaPanel.gridPos, #id: grafanaPanelId}
-                    }]
-                }
-            }
-        },
-        // go through the top-level panels a 3rd time & match only the rows, to create the corresponding grids
-        for grafanaPanelId, grafanaPanel in #grafanaDashboard.panels if grafanaPanel.panels != _|_ { // if the panel is a row
-            kind: "Grid"
-            spec: {
-                display: {
-                    title: grafanaPanel.title,
-                    collapse: {
-                        open: !grafanaPanel.collapsed
+                    display: {
+                        title: grafanaPanel.title,
+                        collapse: {
+                            open: !grafanaPanel.collapsed
+                        }
                     }
+                    // go through the children panels of the current row
+                    items: [ for innerPanelId, innnerPanel in grafanaPanel.panels {
+                        _positionConversion & {#gridPos: innnerPanel.gridPos, #id: "\(grafanaPanelId)_\(innerPanelId)"}
+                    }]
                 }
-                // go through the children panels of the current row
-                items: [ for innerPanelId, innnerPanel in grafanaPanel.panels {
-                    _positionConversion & {#gridPos: innnerPanel.gridPos, #id: "\(grafanaPanelId)_\(innerPanelId)"}
-                }]
             }
-        }
-    ]
+        ]
+    }
 }
diff --git a/internal/api/migrate/migrate.go b/internal/api/migrate/migrate.go
@@ -23,7 +23,7 @@ import (
 
 	"cuelang.org/go/cue"
 	"cuelang.org/go/cue/cuecontext"
-	"github.com/perses/perses/internal/api/interface"
+	apiinterface "github.com/perses/perses/internal/api/interface"
 	"github.com/perses/perses/internal/api/schemas"
 	"github.com/perses/perses/pkg/model/api/config"
 	v1 "github.com/perses/perses/pkg/model/api/v1"
@@ -231,7 +231,7 @@ func rearrangeGrafanaPanelsWithinExpandedRows(grafanaDashboardRaw json.RawMessag
 	}
 
 	// iterate over the panels & achieve recomposition of parent->children relationship when needed
-	var newPanelList []map[string]any
+	newPanelList := make([]map[string]any, 0)
 	var parentRow map[string]any
 	for _, panelAsAny := range panels {
 		panel, ok := panelAsAny.(map[string]any)

diff --git a/internal/api/migrate/migrate_test.go b/internal/api/migrate/migrate_test.go
@@ -58,6 +58,12 @@ func TestMigrate(t *testing.T) {
 			expectedPersesDashboardFile: "old_grafana_query_perses_dashboard.json",
 			expectedErrorStr:            "",
 		},
+		{
+			title:                       "dashboard without panels should be migrated without error",
+			inputGrafanaDashboardFile:   "empty_panels_list_grafana_dashboard.json",
+			expectedPersesDashboardFile: "empty_panels_list_perses_dashboard.json",
+			expectedErrorStr:            "",
+		},
 	}
 
 	for _, test := range testSuite {

diff --git a/internal/api/migrate/testdata/empty_panels_list_grafana_dashboard.json b/internal/api/migrate/testdata/empty_panels_list_grafana_dashboard.json
@@ -0,0 +1,40 @@
+{
+    "annotations": {
+      "list": [
+        {
+          "builtIn": 1,
+          "datasource": {
+            "type": "grafana",
+            "uid": "-- Grafana --"
+          },
+          "enable": true,
+          "hide": true,
+          "iconColor": "rgba(0, 211, 255, 1)",
+          "name": "Annotations & Alerts",
+          "type": "dashboard"
+        }
+      ]
+    },
+    "editable": true,
+    "fiscalYearStartMonth": 0,
+    "graphTooltip": 0,
+    "id": 28,
+    "links": [],
+    "panels": [],
+    "schemaVersion": 39,
+    "tags": [],
+    "templating": {
+      "list": []
+    },
+    "time": {
+      "from": "now-6h",
+      "to": "now"
+    },
+    "timeRangeUpdatedDuringEditOrView": false,
+    "timepicker": {},
+    "timezone": "browser",
+    "title": "test dashboard",
+    "uid": "ddgn9k36v60owc",
+    "version": 1,
+    "weekStart": ""
+  }
diff --git a/internal/api/migrate/testdata/empty_panels_list_perses_dashboard.json b/internal/api/migrate/testdata/empty_panels_list_perses_dashboard.json
@@ -0,0 +1,18 @@
+{
+  "kind":"Dashboard", 
+  "metadata": {
+    "createdAt":"0001-01-01T00:00:00Z", 
+    "name":"ddgn9k36v60owc", 
+    "project":"", 
+    "updatedAt":"0001-01-01T00:00:00Z", 
+    "version":0
+  }, 
+  "spec": {
+    "display":{
+      "name":"test dashboard"
+    }, 
+    "duration":"1h", 
+    "layouts":[], 
+    "panels":{}
+  }
+}
diff --git a/pkg/model/api/v1/dashboard.go b/pkg/model/api/v1/dashboard.go
@@ -22,6 +22,7 @@ import (
 	"github.com/perses/perses/pkg/model/api/v1/common"
 	"github.com/perses/perses/pkg/model/api/v1/dashboard"
 	"github.com/prometheus/common/model"
+	"github.com/sirupsen/logrus"
 )
 
 type PanelDisplay struct {
@@ -129,7 +130,8 @@ func (d *DashboardSpec) UnmarshalYAML(unmarshal func(interface{}) error) error {
 
 func (d *DashboardSpec) validate() error {
 	if len(d.Panels) == 0 {
-		return fmt.Errorf("spec.panels cannot be empty")
+		logrus.Trace("the dashboard panels list is empty")
+		return nil
 	}
 	variables := make(map[string]bool, len(d.Variables))
 	for i, variable := range d.Variables {

diff --git a/pkg/model/api/v1/dashboard_test.go b/pkg/model/api/v1/dashboard_test.go
@@ -524,20 +524,6 @@ func TestUnmarshalDashboardError(t *testing.T) {
 `,
 			err: fmt.Errorf("spec cannot be empty"),
 		},
-		{
-			title: "panel list cannot be empty",
-			jason: `
-{
-  "kind": "Dashboard",
-  "metadata": {
-    "name": "test",
-    "project": "perses"
-  },
-  "spec": {}
-}
-`,
-			err: fmt.Errorf("spec.panels cannot be empty"),
-		},
 	}
 	for _, test := range testSuite {
 		t.Run(test.title, func(t *testing.T) {

diff --git a/pkg/model/api/v1/ephemeraldashboard_test.go b/pkg/model/api/v1/ephemeraldashboard_test.go
@@ -536,20 +536,6 @@ func TestUnmarshalEphemeralDashboardError(t *testing.T) {
 `,
 			err: fmt.Errorf("spec cannot be empty"),
 		},
-		{
-			title: "panel list cannot be empty",
-			jason: `
-{
-  "kind": "EphemeralDashboard",
-  "metadata": {
-    "name": "test",
-    "project": "perses"
-  },
-  "spec": {}
-}
-`,
-			err: fmt.Errorf("spec.panels cannot be empty"),
-		},
 	}
 	for _, test := range testSuite {
 		t.Run(test.title, func(t *testing.T) {