Skip to content

Commit

Permalink
Implement spec.uid for GrafanaDatasource (#1735)
Browse files Browse the repository at this point in the history
* feat: spec.uid for Datasource

* test: update example test resources

* refactor: Update datasource field descriptions

* test: Datasource immutability with envtest

dep: upgrade ginkgo to v2
  • Loading branch information
Baarsgaard authored Nov 11, 2024
1 parent e1fa46a commit 3b42b31
Show file tree
Hide file tree
Showing 14 changed files with 242 additions and 108 deletions.
44 changes: 20 additions & 24 deletions api/v1beta1/grafanadatasource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
106 changes: 57 additions & 49 deletions api/v1beta1/grafanadatasource_types_test.go
Original file line number Diff line number Diff line change
@@ -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())
})
})
})
80 changes: 80 additions & 0 deletions api/v1beta1/suite_test.go
Original file line number Diff line number Diff line change
@@ -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())
})
15 changes: 14 additions & 1 deletion config/crd/bases/grafana.integreatly.org_grafanadatasources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -86,6 +87,7 @@ spec:
type:
type: string
uid:
description: Deprecated field, use spec.uid instead
type: string
url:
type: string
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
11 changes: 6 additions & 5 deletions controllers/datasource_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 3b42b31

Please sign in to comment.