Refactor: Common opts structs and ResyncPeriod (#1765)

* feat: New common struct * refactor: Embed GrafanaCommonSpec into all crds except Grafanas * refactor: Immutability tests for all crds * docs: Generate * refactor: Use of ResyncPeriod in all reconcilers --------- Co-authored-by: Igor Beliakov <[email protected]>
grafana · Nov 26, 2024 · ffdc532 · ffdc532
1 parent 7ab226c
commit ffdc532
Show file tree

Hide file tree

Showing 32 changed files with 292 additions and 349 deletions.
diff --git a/api/v1beta1/common.go b/api/v1beta1/common.go
@@ -1,6 +1,9 @@
 package v1beta1
 
-import v1 "k8s.io/api/core/v1"
+import (
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
 
 type ValueFrom struct {
 	TargetPath string          `json:"targetPath"`
@@ -16,3 +19,23 @@ type ValueFromSource struct {
 	// +optional
 	SecretKeyRef *v1.SecretKeySelector `json:"secretKeyRef,omitempty"`
 }
+
+// Common Options that all CRs should embed, excluding GrafanaSpec
+// Ensure alignment on handling ResyncPeriod, InstanceSelector, and AllowCrossNamespaceImport
+type GrafanaCommonSpec struct {
+	// How often the resource is synced, defaults to 10m0s if not set
+	// +optional
+	// +kubebuilder:validation:Type=string
+	// +kubebuilder:validation:Format=duration
+	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
+	// +kubebuilder:default="10m0s"
+	ResyncPeriod metav1.Duration `json:"resyncPeriod,omitempty"`
+
+	// Selects Grafana instances for import
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.instanceSelector is immutable"
+	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
+
+	// Allow the Operator to match this resource with Grafanas outside the current namespace
+	// +optional
+	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
+}
diff --git a/api/v1beta1/grafanaalertrulegroup_types.go b/api/v1beta1/grafanaalertrulegroup_types.go
@@ -27,21 +27,12 @@ import (
 // +kubebuilder:validation:XValidation:rule="(has(self.folderUID) && !(has(self.folderRef))) || (has(self.folderRef) && !(has(self.folderUID)))", message="Only one of FolderUID or FolderRef can be set"
 // +kubebuilder:validation:XValidation:rule="((!has(oldSelf.editable) && !has(self.editable)) || (has(oldSelf.editable) && has(self.editable)))", message="spec.editable is immutable"
 type GrafanaAlertRuleGroupSpec struct {
+	GrafanaCommonSpec `json:",inline"`
+
 	// +optional
 	// Name of the alert rule group. If not specified, the resource name will be used.
 	Name string `json:"name,omitempty"`
 
-	// +optional
-	// +kubebuilder:validation:Type=string
-	// +kubebuilder:validation:Format=duration
-	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
-	// +kubebuilder:default="10m"
-	ResyncPeriod metav1.Duration `json:"resyncPeriod,omitempty"`
-
-	// selects Grafanas for import
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
-
 	// UID of the folder containing this rule group
 	// Overrides the FolderSelector
 	FolderUID string `json:"folderUID,omitempty"`
@@ -57,9 +48,6 @@ type GrafanaAlertRuleGroupSpec struct {
 	// +kubebuilder:validation:Required
 	Interval metav1.Duration `json:"interval"`
 
-	// +optional
-	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
-
 	// Whether to enable or disable editing of the alert rule group in Grafana UI
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	// +optional

diff --git a/api/v1beta1/grafanaalertrulegroup_types_test.go b/api/v1beta1/grafanaalertrulegroup_types_test.go
@@ -22,9 +22,11 @@ func newAlertRuleGroup(name string, editable *bool) *GrafanaAlertRuleGroup {
 			Name:      name,
 			Editable:  editable,
 			FolderRef: "DummyFolderRef",
-			InstanceSelector: &v1.LabelSelector{
-				MatchLabels: map[string]string{
-					"test": "alertrulegroup",
+			GrafanaCommonSpec: GrafanaCommonSpec{
+				InstanceSelector: &v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "alertrulegroup",
+					},
 				},
 			},
 			Rules: []AlertRule{},

diff --git a/api/v1beta1/grafanacontactpoint_types.go b/api/v1beta1/grafanacontactpoint_types.go
@@ -27,22 +27,13 @@ import (
 // GrafanaContactPointSpec defines the desired state of GrafanaContactPoint
 // +kubebuilder:validation:XValidation:rule="((!has(oldSelf.uid) && !has(self.uid)) || (has(oldSelf.uid) && has(self.uid)))", message="spec.uid is immutable"
 type GrafanaContactPointSpec struct {
+	GrafanaCommonSpec `json:",inline"`
+
 	// Manually specify the UID the Contact Point is created with
 	// +optional
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.uid is immutable"
 	CustomUID string `json:"uid,omitempty"`
 
-	// +optional
-	// +kubebuilder:validation:Type=string
-	// +kubebuilder:validation:Format=duration
-	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
-	// +kubebuilder:default="10m"
-	ResyncPeriod metav1.Duration `json:"resyncPeriod,omitempty"`
-
-	// selects Grafanas for import
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
-
 	// +optional
 	DisableResolveMessage bool `json:"disableResolveMessage,omitempty"`
 
@@ -56,9 +47,6 @@ type GrafanaContactPointSpec struct {
 
 	// +kubebuilder:validation:Enum=alertmanager;prometheus-alertmanager;dingding;discord;email;googlechat;kafka;line;opsgenie;pagerduty;pushover;sensugo;sensu;slack;teams;telegram;threema;victorops;webhook;wecom;hipchat;oncall
 	Type string `json:"type,omitempty"`
-
-	// +optional
-	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
 }
 
 // GrafanaContactPointStatus defines the observed state of GrafanaContactPoint

diff --git a/api/v1beta1/grafanacontactpoint_types_test.go b/api/v1beta1/grafanacontactpoint_types_test.go
@@ -27,9 +27,11 @@ func newContactPoint(name string, uid string) *GrafanaContactPoint {
 		},
 		Spec: GrafanaContactPointSpec{
 			CustomUID: uid,
-			InstanceSelector: &v1.LabelSelector{
-				MatchLabels: map[string]string{
-					"test": "datasource",
+			GrafanaCommonSpec: GrafanaCommonSpec{
+				InstanceSelector: &v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "datasource",
+					},
 				},
 			},
 			Settings: settings,

diff --git a/api/v1beta1/grafanadashboard_types.go b/api/v1beta1/grafanadashboard_types.go
@@ -37,7 +37,6 @@ const (
 	DashboardSourceTypeJsonnet    DashboardSourceType = "jsonnet"
 	DashboardSourceTypeGrafanaCom DashboardSourceType = "grafana"
 	DashboardSourceConfigMap      DashboardSourceType = "configmap"
-	DefaultResyncPeriod                               = "5m"
 )
 
 type GrafanaDashboardDatasource struct {
@@ -62,6 +61,8 @@ type GrafanaDashboardUrlAuthorization struct {
 // +kubebuilder:validation:XValidation:rule="(has(self.folder) && !(has(self.folderRef) || has(self.folderUID))) || !(has(self.folder))", message="folder field cannot be set when folderUID or folderRef is already declared"
 // +kubebuilder:validation:XValidation:rule="((!has(oldSelf.uid) && !has(self.uid)) || (has(oldSelf.uid) && has(self.uid)))", message="spec.uid is immutable"
 type GrafanaDashboardSpec struct {
+	GrafanaCommonSpec `json:",inline"`
+
 	// Manually specify the uid for the dashboard, overwrites uids already present in the json model
 	// +optional
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.uid is immutable"
@@ -98,10 +99,6 @@ type GrafanaDashboardSpec struct {
 	// +optional
 	ConfigMapRef *v1.ConfigMapKeySelector `json:"configMapRef,omitempty"`
 
-	// selects Grafanas for import
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
-
 	// folder assignment for dashboard
 	// +optional
 	FolderTitle string `json:"folder,omitempty"`
@@ -122,22 +119,10 @@ type GrafanaDashboardSpec struct {
 	// +optional
 	ContentCacheDuration metav1.Duration `json:"contentCacheDuration,omitempty"`
 
-	// how often the dashboard is refreshed, defaults to 5m if not set
-	// +optional
-	// +kubebuilder:validation:Type=string
-	// +kubebuilder:validation:Format=duration
-	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
-	// +kubebuilder:default="5m"
-	ResyncPeriod string `json:"resyncPeriod,omitempty"`
-
 	// maps required data sources to existing ones
 	// +optional
 	Datasources []GrafanaDashboardDatasource `json:"datasources,omitempty"`
 
-	// allow to import this resources from an operator in a different namespace
-	// +optional
-	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
-
 	// environments variables as a map
 	// +optional
 	Envs []GrafanaDashboardEnv `json:"envs,omitempty"`
@@ -261,25 +246,10 @@ func (in *GrafanaDashboard) Unchanged(hash string) bool {
 }
 
 func (in *GrafanaDashboard) ResyncPeriodHasElapsed() bool {
-	deadline := in.Status.LastResync.Add(in.GetResyncPeriod())
+	deadline := in.Status.LastResync.Add(in.Spec.ResyncPeriod.Duration)
 	return time.Now().After(deadline)
 }
 
-func (in *GrafanaDashboard) GetResyncPeriod() time.Duration {
-	if in.Spec.ResyncPeriod == "" {
-		in.Spec.ResyncPeriod = DefaultResyncPeriod
-		return in.GetResyncPeriod()
-	}
-
-	duration, err := time.ParseDuration(in.Spec.ResyncPeriod)
-	if err != nil {
-		in.Spec.ResyncPeriod = DefaultResyncPeriod
-		return in.GetResyncPeriod()
-	}
-
-	return duration
-}
-
 func (in *GrafanaDashboard) GetSourceTypes() []DashboardSourceType {
 	var sourceTypes []DashboardSourceType
 

diff --git a/api/v1beta1/grafanadashboard_types_test.go b/api/v1beta1/grafanadashboard_types_test.go
@@ -265,9 +265,11 @@ func getDashboardCR(t *testing.T, crUID string, statusUID string, specUID string
 			UID:       types.UID(crUID),
 		},
 		Spec: GrafanaDashboardSpec{
-			InstanceSelector: &metav1.LabelSelector{
-				MatchLabels: map[string]string{
-					"dashboard": "grafana",
+			GrafanaCommonSpec: GrafanaCommonSpec{
+				InstanceSelector: &metav1.LabelSelector{
+					MatchLabels: map[string]string{
+						"dashboard": "grafana",
+					},
 				},
 			},
 			CustomUID: specUID,
@@ -293,9 +295,11 @@ func newDashboard(name string, uid string) *GrafanaDashboard {
 		},
 		Spec: GrafanaDashboardSpec{
 			CustomUID: uid,
-			InstanceSelector: &v1.LabelSelector{
-				MatchLabels: map[string]string{
-					"test": "datasource",
+			GrafanaCommonSpec: GrafanaCommonSpec{
+				InstanceSelector: &v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "datasource",
+					},
 				},
 			},
 			Json: "",

diff --git a/api/v1beta1/grafanadatasource_types.go b/api/v1beta1/grafanadatasource_types.go
@@ -60,17 +60,15 @@ 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 {
+	GrafanaCommonSpec `json:",inline"`
+
 	// 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
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
-
 	// plugins
 	// +optional
 	Plugins PluginList `json:"plugins,omitempty"`
@@ -79,18 +77,6 @@ type GrafanaDatasourceSpec struct {
 	// +optional
 	// +kubebuilder:validation:MaxItems=99
 	ValuesFrom []ValueFrom `json:"valuesFrom,omitempty"`
-
-	// how often the datasource is refreshed, defaults to 5m if not set
-	// +optional
-	// +kubebuilder:validation:Type=string
-	// +kubebuilder:validation:Format=duration
-	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
-	// +kubebuilder:default="5m"
-	ResyncPeriod string `json:"resyncPeriod,omitempty"`
-
-	// allow to import this resources from an operator in a different namespace
-	// +optional
-	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
 }
 
 // GrafanaDatasourceStatus defines the observed state of GrafanaDatasource
@@ -129,23 +115,8 @@ type GrafanaDatasourceList struct {
 	Items           []GrafanaDatasource `json:"items"`
 }
 
-func (in *GrafanaDatasource) GetResyncPeriod() time.Duration {
-	if in.Spec.ResyncPeriod == "" {
-		in.Spec.ResyncPeriod = DefaultResyncPeriod
-		return in.GetResyncPeriod()
-	}
-
-	duration, err := time.ParseDuration(in.Spec.ResyncPeriod)
-	if err != nil {
-		in.Spec.ResyncPeriod = DefaultResyncPeriod
-		return in.GetResyncPeriod()
-	}
-
-	return duration
-}
-
 func (in *GrafanaDatasource) ResyncPeriodHasElapsed() bool {
-	deadline := in.Status.LastResync.Add(in.GetResyncPeriod())
+	deadline := in.Status.LastResync.Add(in.Spec.ResyncPeriod.Duration)
 	return time.Now().After(deadline)
 }
 

diff --git a/api/v1beta1/grafanadatasource_types_test.go b/api/v1beta1/grafanadatasource_types_test.go
@@ -20,9 +20,11 @@ func newDatasource(name string, uid string) *GrafanaDatasource {
 		},
 		Spec: GrafanaDatasourceSpec{
 			CustomUID: uid,
-			InstanceSelector: &v1.LabelSelector{
-				MatchLabels: map[string]string{
-					"test": "datasource",
+			GrafanaCommonSpec: GrafanaCommonSpec{
+				InstanceSelector: &v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "datasource",
+					},
 				},
 			},
 			Datasource: &GrafanaDatasourceInternal{

diff --git a/api/v1beta1/grafanafolder_types.go b/api/v1beta1/grafanafolder_types.go
@@ -32,6 +32,8 @@ import (
 // +kubebuilder:validation:XValidation:rule="(has(self.parentFolderUID) && !(has(self.parentFolderRef))) || (has(self.parentFolderRef) && !(has(self.parentFolderUID))) || !(has(self.parentFolderRef) && (has(self.parentFolderUID)))", message="Only one of parentFolderUID or parentFolderRef can be set"
 // +kubebuilder:validation:XValidation:rule="((!has(oldSelf.uid) && !has(self.uid)) || (has(oldSelf.uid) && has(self.uid)))", message="spec.uid is immutable"
 type GrafanaFolderSpec struct {
+	GrafanaCommonSpec `json:",inline"`
+
 	// Manually specify the UID the Folder is created with
 	// +optional
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="spec.uid is immutable"
@@ -45,29 +47,13 @@ type GrafanaFolderSpec struct {
 	// +optional
 	Permissions string `json:"permissions,omitempty"`
 
-	// Selects Grafanas for import
-	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	InstanceSelector *metav1.LabelSelector `json:"instanceSelector"`
-
-	// Enable matching Grafana instances outside the current namespace
-	// +optional
-	AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
-
 	// UID of the folder in which the current folder should be created
 	// +optional
 	ParentFolderUID string `json:"parentFolderUID,omitempty"`
 
 	// Reference to an existing GrafanaFolder CR in the same namespace
 	// +optional
 	ParentFolderRef string `json:"parentFolderRef,omitempty"`
-
-	// How often the folder is synced, defaults to 5m if not set
-	// +optional
-	// +kubebuilder:validation:Type=string
-	// +kubebuilder:validation:Format=duration
-	// +kubebuilder:validation:Pattern="^([0-9]+(\\.[0-9]+)?(ns|us|µs|ms|s|m|h))+$"
-	// +kubebuilder:default="5m"
-	ResyncPeriod string `json:"resyncPeriod,omitempty"`
 }
 
 // GrafanaFolderStatus defines the observed state of GrafanaFolder
@@ -180,22 +166,7 @@ func (in *GrafanaFolder) GetTitle() string {
 	return in.Name
 }
 
-func (in *GrafanaFolder) GetResyncPeriod() time.Duration {
-	if in.Spec.ResyncPeriod == "" {
-		in.Spec.ResyncPeriod = DefaultResyncPeriod
-		return in.GetResyncPeriod()
-	}
-
-	duration, err := time.ParseDuration(in.Spec.ResyncPeriod)
-	if err != nil {
-		in.Spec.ResyncPeriod = DefaultResyncPeriod
-		return in.GetResyncPeriod()
-	}
-
-	return duration
-}
-
 func (in *GrafanaFolder) ResyncPeriodHasElapsed() bool {
-	deadline := in.Status.LastResync.Add(in.GetResyncPeriod())
+	deadline := in.Status.LastResync.Add(in.Spec.ResyncPeriod.Duration)
 	return time.Now().After(deadline)
 }
diff --git a/api/v1beta1/grafanafolder_types_test.go b/api/v1beta1/grafanafolder_types_test.go
@@ -89,9 +89,11 @@ func newFolder(name string, uid string) *GrafanaFolder {
 		},
 		Spec: GrafanaFolderSpec{
 			CustomUID: uid,
-			InstanceSelector: &v1.LabelSelector{
-				MatchLabels: map[string]string{
-					"test": "folder",
+			GrafanaCommonSpec: GrafanaCommonSpec{
+				InstanceSelector: &v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"test": "folder",
+					},
 				},
 			},
 		},