Implement spec.uid for GrafanaDatasource

api/v1beta1/grafanadatasource_types.go

@@ -17,16 +17,15 @@ limitations under the License.
 package v1beta1
 
 import (
-	"bytes"
 	"encoding/json"
-	"errors"
-	"fmt"
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 type GrafanaDatasourceInternal struct {
+	// Deprecated field, use spec.uid instead
+	// +optional
 	UID           string `json:"uid,omitempty"`
 	Name          string `json:"name,omitempty"`
 	Type          string `json:"type,omitempty"`
@@ -40,7 +39,9 @@ type GrafanaDatasourceInternal struct {
 
 	// Deprecated field, it has no effect
 	OrgID *int64 `json:"orgId,omitempty"`
-	// Deprecated field, it has no effect
+
+	// Whether to enable/disable editing of the datasource in Grafana UI
+	// +optional
 	Editable *bool `json:"editable,omitempty"`
 
 	// +kubebuilder:validation:Schemaless
@@ -57,7 +58,13 @@ type GrafanaDatasourceInternal struct {
 }
 
 // GrafanaDatasourceSpec defines the desired state of GrafanaDatasource
+// +kubebuilder:validation:XValidation:rule="((!has(oldSelf.uid) && !has(self.uid)) || (has(oldSelf.uid) && has(self.uid)))", message="spec.uid is immutable"
 type GrafanaDatasourceSpec struct {
+	// The UID, for the datasource, fallback to the deprecated spec.datasource.uid and metadata.uid
+	// +optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.uid is immutable"
+	CustomUID string `json:"uid,omitempty"`
+
 	Datasource *GrafanaDatasourceInternal `json:"datasource"`
 
 	// selects Grafana instances for import
@@ -146,37 +153,26 @@ func (in *GrafanaDatasource) Unchanged(hash string) bool {
 	return in.Status.Hash == hash
 }
 
-func (in *GrafanaDatasource) IsUpdatedUID(uid string) bool {
+func (in *GrafanaDatasource) IsUpdatedUID() bool {
 	// Datasource has just been created, status is not yet updated
 	if in.Status.UID == "" {
 		return false
 	}
 
-	if uid == "" {
-		uid = string(in.UID)
-	}
-
-	return in.Status.UID != uid
+	return in.Status.UID != in.CustomUIDOrUID()
 }
 
-func (in *GrafanaDatasource) ExpandVariables(variables map[string][]byte) ([]byte, error) {
-	if in.Spec.Datasource == nil {
-		return nil, errors.New("data source is empty, can't expand variables")
-	}
-
-	raw, err := json.Marshal(in.Spec.Datasource)
-	if err != nil {
-		return nil, err
+// Wrapper around CustomUID, datasourcelUID or default metadata.uid
+func (in *GrafanaDatasource) CustomUIDOrUID() string {
+	if in.Spec.CustomUID != "" {
+		return in.Spec.CustomUID
 	}
 
-	for key, value := range variables {
-		patterns := []string{fmt.Sprintf("$%v", key), fmt.Sprintf("${%v}", key)}
-		for _, pattern := range patterns {
-			raw = bytes.ReplaceAll(raw, []byte(pattern), value)
-		}
+	if in.Spec.Datasource.UID != "" {
+		return in.Spec.Datasource.UID
 	}
 
-	return raw, nil
+	return string(in.ObjectMeta.UID)
 }
 
 func (in *GrafanaDatasource) IsAllowCrossNamespaceImport() bool {

api/v1beta1/grafanadatasource_types_test.go

@@ -1,62 +1,70 @@
 package v1beta1
 
 import (
-	"bytes"
-	"fmt"
-	"testing"
-)
+	"context"
 
-func TestGrafanaDatasources_expandVariables(t *testing.T) {
-	type testcase struct {
-		name      string
-		variables map[string][]byte
-		in        GrafanaDatasource
-		out       []byte
-	}
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
 
-	testcases := []testcase{
-		{
-			name: "basic replacement",
-			variables: map[string][]byte{
-				"PROMETHEUS_USERNAME": []byte("root"),
-			},
-			in: GrafanaDatasource{
-				Spec: GrafanaDatasourceSpec{
-					Datasource: &GrafanaDatasourceInternal{
-						Name: "prometheus",
-						User: "${PROMETHEUS_USERNAME}",
-					},
-				},
-			},
-			out: []byte("{\"name\":\"prometheus\",\"user\":\"root\"}"),
+func newDatasource(name string, uid string) *GrafanaDatasource {
+	return &GrafanaDatasource{
+		TypeMeta: v1.TypeMeta{
+			APIVersion: APIVersion,
+			Kind:       "GrafanaDatasource",
 		},
-		{
-			name: "replacement without curly braces",
-			variables: map[string][]byte{
-				"PROMETHEUS_USERNAME": []byte("root"),
-			},
-			in: GrafanaDatasource{
-				Spec: GrafanaDatasourceSpec{
-					Datasource: &GrafanaDatasourceInternal{
-						Name: "prometheus",
-						User: "$PROMETHEUS_USERNAME",
-					},
+		ObjectMeta: v1.ObjectMeta{
+			Name:      name,
+			Namespace: "default",
+		},
+		Spec: GrafanaDatasourceSpec{
+			CustomUID: uid,
+			InstanceSelector: &v1.LabelSelector{
+				MatchLabels: map[string]string{
+					"test": "datasource",
 				},
 			},
-			out: []byte("{\"name\":\"prometheus\",\"user\":\"root\"}"),
+			Datasource: &GrafanaDatasourceInternal{
+				Name:   "testdata",
+				Type:   "grafana-testdata-datasource",
+				Access: "proxy",
+			},
 		},
 	}
+}
 
-	for _, tc := range testcases {
-		t.Run(tc.name, func(t *testing.T) {
-			b, err := tc.in.ExpandVariables(tc.variables)
-			if err != nil {
-				t.Fatal(err)
-			}
+var _ = Describe("Datasource type", func() {
+	Context("Ensure Datasource spec.uid is immutable", func() {
+		ctx := context.Background()
+		It("Should block adding uid field when missing", func() {
+			ds := newDatasource("missing-uid", "")
+			By("Create new Datasource without uid")
+			Expect(k8sClient.Create(ctx, ds)).To(Succeed())
 
-			if !bytes.Equal(b, tc.out) {
-				t.Error(fmt.Errorf("expected %v, but got %v", string(tc.out), string(b)))
-			}
+			By("Adding a uid")
+			ds.Spec.CustomUID = "new-uid"
+			Expect(k8sClient.Update(ctx, ds)).To(HaveOccurred())
 		})
-	}
-}
+
+		It("Should block removing uid field when set", func() {
+			ds := newDatasource("existing-uid", "existing-uid")
+			By("Creating Datasource with existing UID")
+			Expect(k8sClient.Create(ctx, ds)).To(Succeed())
+
+			By("And setting UID to ''")
+			ds.Spec.CustomUID = ""
+			Expect(k8sClient.Update(ctx, ds)).To(HaveOccurred())
+		})
+
+		It("Should block changing value of uid", func() {
+			ds := newDatasource("removing-uid", "existing-uid")
+			By("Create new Datasource with existing UID")
+			Expect(k8sClient.Create(ctx, ds)).To(Succeed())
+
+			By("Changing the existing UID")
+			ds.Spec.CustomUID = "new-uid"
+			Expect(k8sClient.Update(ctx, ds)).To(HaveOccurred())
+		})
+	})
+})

api/v1beta1/suite_test.go

@@ -0,0 +1,80 @@
+/*
+Copyright 2022.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"path/filepath"
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/envtest"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	//+kubebuilder:scaffold:imports
+)
+
+// These tests use Ginkgo (BDD-style Go testing framework). Refer to
+// http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
+
+const (
+	APIVersion = "grafana.integreatly.org/v1beta1"
+)
+
+var (
+	cfg       *rest.Config
+	k8sClient client.Client
+	testEnv   *envtest.Environment
+)
+
+func TestAPIs(t *testing.T) {
+	RegisterFailHandler(Fail)
+
+	RunSpecs(t, "CRDs Suite")
+}
+
+var _ = BeforeSuite(func() {
+	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
+
+	By("bootstrapping test environment")
+	testEnv = &envtest.Environment{
+		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases")},
+		ErrorIfCRDPathMissing: true,
+	}
+
+	cfg, err := testEnv.Start()
+	Expect(err).NotTo(HaveOccurred())
+	Expect(cfg).NotTo(BeNil())
+
+	err = AddToScheme(scheme.Scheme)
+	Expect(err).NotTo(HaveOccurred())
+
+	//+kubebuilder:scaffold:scheme
+
+	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
+	Expect(err).NotTo(HaveOccurred())
+	Expect(k8sClient).NotTo(BeNil())
+})
+
+var _ = AfterSuite(func() {
+	By("tearing down the test environment")
+	err := testEnv.Stop()
+	Expect(err).NotTo(HaveOccurred())
+})

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

@@ -67,7 +67,8 @@ spec:
                   database:
                     type: string
                   editable:
-                    description: Deprecated field, it has no effect
+                    description: Whether to enable/disable editing of the datasource
+                      in Grafana UI
                     type: boolean
                   isDefault:
                     type: boolean
@@ -86,6 +87,7 @@ spec:
                   type:
                     type: string
                   uid:
+                    description: Deprecated field, use spec.uid instead
                     type: string
                   url:
                     type: string
@@ -161,6 +163,13 @@ spec:
                 format: duration
                 pattern: ^([0-9]+(\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$
                 type: string
+              uid:
+                description: The UID, for the datasource, fallback to the deprecated
+                  spec.datasource.uid and metadata.uid
+                type: string
+                x-kubernetes-validations:
+                - message: spec.uid is immutable
+                  rule: self == oldSelf
               valuesFrom:
                 description: environments variables from secrets or config maps
                 items:
@@ -231,6 +240,10 @@ spec:
             - datasource
             - instanceSelector
             type: object
+            x-kubernetes-validations:
+            - message: spec.uid is immutable
+              rule: ((!has(oldSelf.uid) && !has(self.uid)) || (has(oldSelf.uid) &&
+                has(self.uid)))
           status:
             description: GrafanaDatasourceStatus defines the observed state of GrafanaDatasource
             properties:

controllers/datasource_controller.go

@@ -182,6 +182,9 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{}, nil
 	}
 
+	// Overwrite OrgID to ensure the field is useless
+	cr.Spec.Datasource.OrgID = nil
+
 	instances, err := r.GetMatchingDatasourceInstances(ctx, cr, r.Client)
 	if err != nil {
 		controllerLog.Error(err, "could not find matching instances", "name", cr.Name, "namespace", cr.Namespace)
@@ -196,7 +199,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		return ctrl.Result{RequeueAfter: RequeueDelay}, err
 	}
 
-	if cr.IsUpdatedUID(datasource.UID) {
+	if cr.IsUpdatedUID() {
 		controllerLog.Info("datasource uid got updated, deleting datasources with the old uid")
 		err = r.onDatasourceDeleted(ctx, req.Namespace, req.Name)
 		if err != nil {
@@ -258,7 +261,7 @@ func (r *GrafanaDatasourceReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		if cr.ResyncPeriodHasElapsed() {
 			cr.Status.LastResync = metav1.Time{Time: time.Now()}
 		}
-		cr.Status.UID = datasource.UID
+		cr.Status.UID = cr.CustomUIDOrUID()
 		return ctrl.Result{RequeueAfter: cr.GetResyncPeriod()}, r.Client.Status().Update(ctx, cr)
 	} else {
 		// if there was an issue with the datasource, update the status
@@ -445,9 +448,7 @@ func (r *GrafanaDatasourceReconciler) getDatasourceContent(ctx context.Context,
 		return nil, "", err
 	}
 
-	if cr.Spec.Datasource.UID == "" {
-		simpleContent.Set("uid", string(cr.UID))
-	}
+	simpleContent.Set("uid", cr.CustomUIDOrUID())
 
 	for _, ref := range cr.Spec.ValuesFrom {
 		ref := ref