fix: don't use dashboard cache if url is different

api/integreatly/v1alpha1/grafana_types.go

@@ -41,7 +41,7 @@ type GrafanaSpec struct {
 
 	// DashboardContentCacheDuration sets a default for when a `GrafanaDashboard` resource doesn't specify a `contentCacheDuration`.
 	// If left unset or 0 the default behavior is to cache indefinitely.
-	DashboardContentCacheDuration *metav1.Duration `json:"dashboardContentCacheDuration,omitempty"`
+	DashboardContentCacheDuration metav1.Duration `json:"dashboardContentCacheDuration,omitempty"`
 }
 
 type ReadinessProbeSpec struct {

api/integreatly/v1alpha1/grafanadashboard_types.go

@@ -18,14 +18,14 @@ package v1alpha1
 
 import (
 	"bytes"
-	"crypto/sha1" // nolint
+	"compress/gzip"
+	"crypto/sha1" //nolint
 	"crypto/sha256"
 	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
-
-	"compress/gzip"
+	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -82,7 +82,7 @@ type GrafanaDashboardRef struct {
 
 type GrafanaDashboardStatus struct {
 	ContentCache     []byte                 `json:"contentCache,omitempty"`
-	ContentTimestamp *metav1.Time           `json:"contentTimestamp,omitempty"`
+	ContentTimestamp metav1.Time            `json:"contentTimestamp,omitempty"`
 	ContentUrl       string                 `json:"contentUrl,omitempty"`
 	Error            *GrafanaDashboardError `json:"error,omitempty"`
 }
@@ -171,7 +171,7 @@ func (d *GrafanaDashboard) Parse(optional string) (map[string]interface{}, error
 		dashboardBytes = []byte(optional)
 	}
 
-	var parsed = make(map[string]interface{})
+	parsed := make(map[string]interface{})
 	err := json.Unmarshal(dashboardBytes, &parsed)
 	return parsed, err
 }
@@ -191,6 +191,34 @@ func (d *GrafanaDashboard) UID() string {
 	return fmt.Sprintf("%x", sha1.Sum([]byte(d.Namespace+d.Name))) // nolint
 }
 
+func (d *GrafanaDashboard) GetContentCache(url string) string {
+	var cacheDuration time.Duration
+	if d.Spec.ContentCacheDuration != nil {
+		cacheDuration = d.Spec.ContentCacheDuration.Duration
+	}
+
+	return d.Status.getContentCache(url, cacheDuration)
+}
+
+// getContentCache returns content cache when the following conditions are met: url is the same, data is not expired, gzipped data is not corrupted
+func (s *GrafanaDashboardStatus) getContentCache(url string, cacheDuration time.Duration) string {
+	if s.ContentUrl != url {
+		return ""
+	}
+
+	notExpired := cacheDuration <= 0 || s.ContentTimestamp.Add(cacheDuration).After(time.Now())
+	if !notExpired {
+		return ""
+	}
+
+	cache, err := Gunzip(s.ContentCache)
+	if err != nil {
+		return ""
+	}
+
+	return string(cache)
+}
+
 func Gunzip(compressed []byte) ([]byte, error) {
 	decoder, err := gzip.NewReader(bytes.NewReader(compressed))
 	if err != nil {

api/integreatly/v1alpha1/grafanadashboard_types_test.go

@@ -7,6 +7,10 @@ import (
 	"reflect"
 	"strings"
 	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 // Encoded via cat | gzip | base64
@@ -78,3 +82,86 @@ func TestDecompress(t *testing.T) {
 		t.Fail()
 	}
 }
+
+func TestGrafanaDashboardStatus_getContentCache(t *testing.T) {
+	timestamp := metav1.Time{Time: time.Now().Add(-1 * time.Hour)}
+	infinite := 0 * time.Second
+	dashboardJSON := `{"dummyField": "dummyData"}`
+
+	cachedDashboard, err := Gzip(dashboardJSON)
+	assert.Nil(t, err)
+
+	url := "http://127.0.0.1:8080/1.json"
+
+	// Correctly populated cache
+	status := GrafanaDashboardStatus{
+		ContentCache:     cachedDashboard,
+		ContentTimestamp: timestamp,
+		ContentUrl:       url,
+	}
+
+	// Corrupted cache
+	statusCorrupted := GrafanaDashboardStatus{
+		ContentCache:     []byte("abc"),
+		ContentTimestamp: timestamp,
+		ContentUrl:       url,
+	}
+
+	tests := []struct {
+		name     string
+		status   GrafanaDashboardStatus
+		url      string
+		duration time.Duration
+		want     string
+	}{
+		{
+			name:     "no cache: fields are not populated",
+			url:      status.ContentUrl,
+			duration: infinite,
+			status:   GrafanaDashboardStatus{},
+			want:     "",
+		},
+		{
+			name:     "no cache: url is different",
+			url:      "http://another-url/2.json",
+			duration: infinite,
+			status:   status,
+			want:     "",
+		},
+		{
+			name:     "no cache: expired",
+			url:      status.ContentUrl,
+			duration: 1 * time.Minute,
+			status:   status,
+			want:     "",
+		},
+		{
+			name:     "no cache: corrupted gzip",
+			url:      statusCorrupted.ContentUrl,
+			duration: infinite,
+			status:   statusCorrupted,
+			want:     "",
+		},
+		{
+			name:     "valid cache: not expired yet",
+			url:      status.ContentUrl,
+			duration: 24 * time.Hour,
+			status:   status,
+			want:     dashboardJSON,
+		},
+		{
+			name:     "valid cache: not expired yet (infinite)",
+			url:      status.ContentUrl,
+			duration: infinite,
+			status:   status,
+			want:     dashboardJSON,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.status.getContentCache(tt.url, tt.duration)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}

controllers/common/controllerState.go

@@ -7,7 +7,7 @@ var ControllerEvents = make(chan ControllerState, 1)
 type ControllerState struct {
 	DashboardSelectors            []*v1.LabelSelector
 	DashboardNamespaceSelector    *v1.LabelSelector
-	DashboardContentCacheDuration *v1.Duration
+	DashboardContentCacheDuration v1.Duration
 	AdminUrl                      string
 	GrafanaReady                  bool
 	ClientTimeout                 int

controllers/grafana/grafana_controller.go

@@ -16,7 +16,6 @@ import (
 	v1 "k8s.io/api/core/v1"
 	netv1 "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/tools/record"
@@ -308,10 +307,6 @@ func (r *ReconcileGrafana) manageSuccess(cr *grafanav1alpha1.Grafana, state *com
 		cr.Status.InstalledDashboards = r.Config.GetDashboards("")
 	}
 
-	if cr.Spec.DashboardContentCacheDuration == nil {
-		cr.Spec.DashboardContentCacheDuration = &metav1.Duration{Duration: 0}
-	}
-
 	instance := &grafanav1alpha1.Grafana{}
 	err := r.Client.Get(r.Context, request.NamespacedName, instance)
 	if err != nil {

controllers/grafanadashboard/dashboard_pipeline.go

@@ -53,9 +53,6 @@ type DashboardPipelineImpl struct {
 }
 
 func NewDashboardPipeline(client client.Client, dashboard *v1alpha1.GrafanaDashboard, ctx context.Context) DashboardPipeline {
-	if dashboard.Spec.ContentCacheDuration == nil {
-		dashboard.Spec.ContentCacheDuration = &metav1.Duration{Duration: 24 * time.Hour}
-	}
 	return &DashboardPipelineImpl{
 		Client:    client,
 		Dashboard: dashboard,
@@ -117,16 +114,6 @@ func (r *DashboardPipelineImpl) validateJson() error {
 	return err
 }
 
-func (r *DashboardPipelineImpl) shouldUseContentCache() bool {
-	if r.Dashboard.Status.ContentCache != nil && r.Dashboard.Spec.ContentCacheDuration != nil && r.Dashboard.Status.ContentTimestamp != nil {
-		cacheDuration := r.Dashboard.Spec.ContentCacheDuration.Duration
-		contentTimeStamp := r.Dashboard.Status.ContentTimestamp
-
-		return cacheDuration <= 0 || contentTimeStamp.Add(cacheDuration).After(time.Now())
-	}
-	return false
-}
-
 // Try to get the dashboard json definition either from a provided URL or from the
 // raw json in the dashboard resource. The priority is as follows:
 //  0. try to use previously fetched content from url or grafanaCom if it is valid
@@ -136,23 +123,8 @@ func (r *DashboardPipelineImpl) shouldUseContentCache() bool {
 //  3. no configmap specified: try to use embedded json
 //  4. no json specified: try to use embedded jsonnet
 func (r *DashboardPipelineImpl) obtainJson() error {
-	// TODO(DeanBrunt): Add earlier validation for this
-	if r.Dashboard.Spec.Url != "" && r.Dashboard.Spec.GrafanaCom != nil {
-		return errors.New("both dashboard url and grafana.com source specified")
-	}
-
 	var returnErr error
 
-	if r.shouldUseContentCache() {
-		jsonBytes, err := v1alpha1.Gunzip(r.Dashboard.Status.ContentCache)
-		if err != nil {
-			returnErr = fmt.Errorf("failed to decode/decompress gzipped json: %w", err)
-		} else {
-			r.JSON = string(jsonBytes)
-			return nil
-		}
-	}
-
 	if r.Dashboard.Spec.GrafanaCom != nil {
 		url, err := r.getGrafanaComDashboardUrl()
 		if err != nil {
@@ -237,6 +209,11 @@ func (r *DashboardPipelineImpl) loadJsonnet(source string) (string, error) {
 
 // Try to obtain the dashboard json from a provided url
 func (r *DashboardPipelineImpl) loadDashboardFromURL(source string) error {
+	r.JSON = r.Dashboard.GetContentCache(source)
+	if r.JSON != "" {
+		return nil
+	}
+
 	url, err := url.ParseRequestURI(source)
 	if err != nil {
 		return fmt.Errorf("invalid url %v", source)
@@ -264,7 +241,7 @@ func (r *DashboardPipelineImpl) loadDashboardFromURL(source string) error {
 				Code:    resp.StatusCode,
 				Retries: retries + 1,
 			},
-			ContentTimestamp: &metav1.Time{Time: time.Now()},
+			ContentTimestamp: metav1.Time{Time: time.Now()},
 		}
 
 		if err := r.Client.Status().Update(r.Context, r.Dashboard); err != nil {
@@ -305,7 +282,7 @@ func (r *DashboardPipelineImpl) loadDashboardFromURL(source string) error {
 	r.refreshDashboard()
 	r.Dashboard.Status = v1alpha1.GrafanaDashboardStatus{
 		ContentCache:     content,
-		ContentTimestamp: &metav1.Time{Time: time.Now()},
+		ContentTimestamp: metav1.Time{Time: time.Now()},
 		ContentUrl:       source,
 	}
 

controllers/grafanadashboard/grafanadashboard_controller.go

@@ -24,6 +24,7 @@ import (
 	"math"
 	"net/http"
 	"os"
+	"time"
 
 	"github.com/go-logr/logr"
 	grafanav1alpha1 "github.com/grafana-operator/grafana-operator/v4/api/integreatly/v1alpha1"
@@ -38,8 +39,6 @@ import (
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 
-	"time"
-
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -289,7 +288,6 @@ func (r *GrafanaDashboardReconciler) reconcileDashboards(request reconcile.Reque
 		}
 
 		folder, err := grafanaClient.CreateOrUpdateFolder(folderName)
-
 		if err != nil {
 			log.Log.Error(err, "failed to get or create namespace folder for dashboard", "folder", folderName, "dashboard", request.Name)
 			r.manageError(&dashboard, err)
@@ -303,8 +301,9 @@ func (r *GrafanaDashboardReconciler) reconcileDashboards(request reconcile.Reque
 			folderId = *folder.ID
 		}
 
+		// If ContentCacheDuration is not defined at a dashboard level, fallback to the instance-level value
 		if dashboard.Spec.ContentCacheDuration == nil {
-			dashboard.Spec.ContentCacheDuration = r.state.DashboardContentCacheDuration
+			dashboard.Spec.ContentCacheDuration = &r.state.DashboardContentCacheDuration
 		}
 
 		// Process the dashboard. Use the known hash of an existing dashboard
@@ -359,7 +358,7 @@ func (r *GrafanaDashboardReconciler) reconcileDashboards(request reconcile.Reque
 
 		_, err = grafanaClient.CreateOrUpdateDashboard(processed, folderId, folderName)
 		if err != nil {
-			//log.Log.Error(err, "cannot submit dashboard %v/%v", "namespace", dashboard.Namespace, "name", dashboard.Name)
+			// log.Log.Error(err, "cannot submit dashboard %v/%v", "namespace", dashboard.Namespace, "name", dashboard.Name)
 			r.manageError(&dashboard, err)
 
 			continue
@@ -453,7 +452,6 @@ func (r *GrafanaDashboardReconciler) checkNamespaceLabels(dashboard *grafanav1al
 		return false, err
 	}
 	selector, err := metav1.LabelSelectorAsSelector(r.state.DashboardNamespaceSelector)
-
 	if err != nil {
 		return false, err
 	}