feat: add support for hardcoded dashboard uid

api/v1beta1/grafanadashboard_types.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"compress/gzip"
 	"crypto/sha256"
+	"encoding/json"
 	"fmt"
 	"io"
 	"time"
@@ -111,6 +112,7 @@ type GrafanaDashboardStatus struct {
 	NoMatchingInstances bool `json:"NoMatchingInstances,omitempty"`
 	// Last time the dashboard was resynced
 	LastResync metav1.Time `json:"lastResync,omitempty"`
+	UID        string      `json:"uid,omitempty"`
 }
 
 //+kubebuilder:object:root=true
@@ -220,6 +222,30 @@ func (in *GrafanaDashboard) IsAllowCrossNamespaceImport() bool {
 	return false
 }
 
+func (in *GrafanaDashboard) IsUpdatedUID(dashboardJson []byte) bool {
+	// Dashboard has just been created, status is not yet updated
+	if in.Status.UID == "" {
+		return false
+	}
+
+	type DashboardUID struct {
+		UID string `json:"uid,omitempty"`
+	}
+
+	dashboardUID := DashboardUID{}
+	err := json.Unmarshal(dashboardJson, &dashboardUID)
+	// here, we don't really care about catching json errors
+	if err != nil {
+		return false
+	}
+
+	if dashboardUID.UID == "" {
+		dashboardUID.UID = string(in.UID)
+	}
+
+	return in.Status.UID != dashboardUID.UID
+}
+
 func Gunzip(compressed []byte) ([]byte, error) {
 	gz, err := gzip.NewReader(bytes.NewReader(compressed))
 	if err != nil {

config/crd/bases/grafana.integreatly.org_grafanadashboards.yaml

@@ -121,6 +121,8 @@ spec:
               lastResync:
                 format: date-time
                 type: string
+              uid:
+                type: string
             type: object
         type: object
     served: true

config/grafana.integreatly.org_grafanadashboards.yaml

@@ -168,6 +168,8 @@ spec:
                 description: Last time the dashboard was resynced
                 format: date-time
                 type: string
+              uid:
+                type: string
             type: object
         type: object
     served: true

controllers/dashboard_controller.go

@@ -185,6 +185,31 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	controllerLog.Info("found matching Grafana instances for dashboard", "count", len(instances.Items))
 
+	dashboardJson, err := r.fetchDashboardJson(dashboard)
+	if err != nil {
+		controllerLog.Error(err, "error fetching dashboard", "dashboard", dashboard.Name)
+		return ctrl.Result{RequeueAfter: RequeueDelay}, nil
+	}
+
+	// Garbage collection for a case where dashboard uid get changed, dashboard creation is expected to happen in a separate reconcilication cycle
+	if dashboard.IsUpdatedUID(dashboardJson) {
+		controllerLog.Info("dashboard uid got updated, deleting dashboards with the old uid")
+		err = r.onDashboardDeleted(ctx, req.Namespace, req.Name)
+		if err != nil {
+			return ctrl.Result{RequeueAfter: RequeueDelay}, err
+		}
+
+		// Clean up uid, so further reconcilications can track changes there
+		dashboard.Status.UID = ""
+		err = r.Client.Status().Update(ctx, dashboard)
+		if err != nil {
+			return ctrl.Result{RequeueAfter: RequeueDelay}, err
+		}
+
+		// Status update should trigger the next reconciliation right away, no need to requeue for dashboard creation
+		return ctrl.Result{}, nil
+	}
+
 	success := true
 	for _, grafana := range instances.Items {
 		// check if this is a cross namespace import
@@ -213,7 +238,7 @@ func (r *GrafanaDashboardReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		}
 
 		// then import the dashboard into the matching grafana instances
-		err = r.onDashboardCreated(ctx, &grafana, dashboard)
+		err = r.onDashboardCreated(ctx, &grafana, dashboard, dashboardJson)
 		if err != nil {
 			controllerLog.Error(err, "error reconciling dashboard", "dashboard", dashboard.Name, "grafana", grafana.Name)
 			success = false
@@ -289,31 +314,23 @@ func (r *GrafanaDashboardReconciler) onDashboardDeleted(ctx context.Context, nam
 	return nil
 }
 
-func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDashboard) error {
-	dashboardJson, err := r.fetchDashboardJson(cr)
-	if err != nil {
-		return err
+func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, grafana *v1beta1.Grafana, cr *v1beta1.GrafanaDashboard, dashboardJson []byte) error {
+	if grafana.IsExternal() && cr.Spec.Plugins != nil {
+		return fmt.Errorf("external grafana instances don't support plugins, please remove spec.plugins from your dashboard cr")
 	}
 
-	// NOTE: previously, we had hash calculated directly from CR, though, with growing number of fetchers,
-	//       it's easier to do it here to make sure it's always the right set of data that's used
-	hash := cr.Hash(dashboardJson)
-
-	dashboardJson, err = r.resolveDatasources(cr, dashboardJson)
+	dashboardJson, err := r.resolveDatasources(cr, dashboardJson)
 	if err != nil {
 		return err
 	}
 
-	if grafana.IsExternal() && cr.Spec.Plugins != nil {
-		return fmt.Errorf("external grafana instances don't support plugins, please remove spec.plugins from your dashboard cr")
-	}
-
 	grafanaClient, err := client2.NewGrafanaClient(ctx, r.Client, grafana)
 	if err != nil {
 		return err
 	}
 
 	// update/create the dashboard if it doesn't exist in the instance or has been changed
+	hash := cr.Hash(dashboardJson)
 	exists, err := r.Exists(grafanaClient, cr)
 	if err != nil {
 		return err
@@ -328,12 +345,18 @@ func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, gra
 		return err
 	}
 
+	uid, _ := dashboardFromJson["uid"].(string) //nolint:errcheck
+	if uid == "" {
+		uid = string(cr.UID)
+	}
+
+	dashboardFromJson["uid"] = uid
+
 	folderID, err := r.GetOrCreateFolder(grafanaClient, cr)
 	if err != nil {
 		return errors.NewInternalError(err)
 	}
 
-	dashboardFromJson["uid"] = string(cr.UID)
 	resp, err := grafanaClient.NewDashboard(grapi.Dashboard{
 		Meta: grapi.DashboardMeta{
 			IsStarred: false,
@@ -353,6 +376,7 @@ func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, gra
 		return errors.NewBadRequest(fmt.Sprintf("error creating dashboard, status was %v", resp.Status))
 	}
 
+	// NOTE: When dashboard is uploaded with a new dashboardFromJson["uid"], but with an old name, the old uid is preserved. So, better to rely on the resp.UID here
 	grafana.Status.Dashboards = grafana.Status.Dashboards.Add(cr.Namespace, cr.Name, resp.UID)
 	err = r.Client.Status().Update(ctx, grafana)
 	if err != nil {
@@ -361,6 +385,7 @@ func (r *GrafanaDashboardReconciler) onDashboardCreated(ctx context.Context, gra
 
 	cr.Status.Hash = hash
 	cr.Status.LastResync = metav1.Time{Time: time.Now()}
+	cr.Status.UID = resp.UID
 
 	return r.Client.Status().Update(ctx, cr)
 }

controllers/dashboard_controller_test.go

@@ -17,11 +17,13 @@ limitations under the License.
 package controllers
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/grafana-operator/grafana-operator/v5/api/v1beta1"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 )
 
 func TestGetDashboardsToDelete(t *testing.T) {
@@ -117,3 +119,103 @@ func TestGetDashboardsToDelete(t *testing.T) {
 		}
 	}
 }
+
+func TestGrafanaDashboardIsUpdatedUID(t *testing.T) {
+	crUID := "crUID"
+	tests := []struct {
+		name         string
+		crUID        string
+		statusUID    string
+		dashboardUID string
+		want         bool
+	}{
+		{
+			name:         "Status.UID and dashboard UID are empty",
+			crUID:        crUID,
+			statusUID:    "",
+			dashboardUID: "newUID",
+			want:         false,
+		},
+		{
+			name:         "Status.UID is empty, dashboard UID is not",
+			crUID:        crUID,
+			statusUID:    "",
+			dashboardUID: "newUID",
+			want:         false,
+		},
+		{
+			name:         "Status.UID is not empty (same as CR uid), new UID is empty",
+			crUID:        crUID,
+			statusUID:    crUID,
+			dashboardUID: "",
+			want:         false,
+		},
+		{
+			name:         "Status.UID is not empty (different from CR uid), new UID is empty",
+			crUID:        crUID,
+			statusUID:    "oldUID",
+			dashboardUID: "",
+			want:         true,
+		},
+		{
+			name:         "Status.UID is not empty, new UID is different",
+			crUID:        crUID,
+			statusUID:    "oldUID",
+			dashboardUID: "newUID",
+			want:         true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cr := getDashboardCR(t, tt.crUID, tt.statusUID)
+			dashboardJson := getDashboardJSONWithUID(t, tt.dashboardUID)
+			got := cr.IsUpdatedUID(dashboardJson)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+func getDashboardCR(t *testing.T, crUID string, statusUID string) v1beta1.GrafanaDashboard {
+	t.Helper()
+
+	cr := v1beta1.GrafanaDashboard{
+		TypeMeta: metav1.TypeMeta{},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "mydashboard",
+			Namespace: "grafana-operator-system",
+			UID:       types.UID(crUID),
+		},
+		Spec: v1beta1.GrafanaDashboardSpec{
+			InstanceSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"dashboard": "grafana",
+				},
+			},
+		},
+		Status: v1beta1.GrafanaDashboardStatus{
+			UID: statusUID,
+		},
+	}
+
+	return cr
+}
+
+func getDashboardJSONWithUID(t *testing.T, uid string) []byte {
+	t.Helper()
+
+	type DashboardUID struct {
+		UID string `json:"uid,omitempty"`
+	}
+
+	dashboardUID := DashboardUID{
+		UID: uid,
+	}
+
+	dashboardJson, err := json.Marshal(dashboardUID)
+	if err != nil {
+		t.Errorf("failed to generate dashboard JSON with uid: %s", uid)
+	}
+
+	return dashboardJson
+}

deploy/helm/grafana-operator/crds/grafana.integreatly.org_grafanadashboards.yaml

@@ -121,6 +121,8 @@ spec:
               lastResync:
                 format: date-time
                 type: string
+              uid:
+                type: string
             type: object
         type: object
     served: true

deploy/kustomize/base/crds.yaml

@@ -167,6 +167,8 @@ spec:
                 description: Last time the dashboard was resynced
                 format: date-time
                 type: string
+              uid:
+                type: string
             type: object
         type: object
     served: true

docs/docs/api.md

@@ -423,6 +423,13 @@ GrafanaDashboardStatus defines the observed state of GrafanaDashboard
             <i>Format</i>: date-time<br/>
         </td>
         <td>false</td>
+      </tr><tr>
+        <td><b>uid</b></td>
+        <td>string</td>
+        <td>
+          <br/>
+        </td>
+        <td>false</td>
       </tr></tbody>
 </table>
 

docs/docs/dashboards.md

@@ -251,14 +251,11 @@ spec:
 
 Remember, depending on where you get your dashboards you might become rate limited if you have multiple dashboards with relatively short `contentCacheDuration` or if all the requests happens at the same time.
 
-## Dashboard UID management
+## Dashboard uid management
 
-When a dashboard is imported into a Grafana, it gets assigned a random uid unless it's hardcoded in dashboard's code. It creates two challenges from the operator's perspective:
+Whenever a dashboard is imported into a Grafana, it gets assigned a random `uid` unless it's hardcoded in dashboard's code. Random `uid` is undesirable from the operator's perspective as it would create the need to track those uids across Grafana instances.
 
-- If uid is not hardcoded, the same dashboard in each Grafana instance will be identified by its own random uid, so the operator needs to keep track of those across many instances;
-- If, at a certain point, uid gets added to an existing `GrafanaDashboard` CR or removed from there, the operator would need to garbage collect the dashboard with the previous uid and recreate it with a new one even if uid was the only change.
-
-To mitigate both scenarios, the operator follows a consistent uid approach. - Whenever a resource is created in Kubernetes, it gets automatically assigned a uid (`metadata.uid`). uid stays the same throughout the whole lifecycle of the resource. - When a CR with `GrafanaDashboard` is created, the operator takes `metadata.uid` and uses it as a dashboard's uid in all Grafana instances. As a consequence, it's not possible to override uid, any value specified in dashboard's spec will be ignored.
+To mitigate the scenario, if `uid` is not hardcoded, the operator will insert the value taken from CR's `metadata.uid` (this value is automatically generated by Kubernetes itself for all resources).
 
 ## Custom folders