Skip to content

Commit

Permalink
Chore(Internal): Refactor pointer bools (#1776)
Browse files Browse the repository at this point in the history
* refactor: Change AllowCrossNamespaceImport from *bool to bool

* test: Add notification policy and contactpoint

* chore: Delete duplicate annotation

* chore: Generate crds/docs

---------

Co-authored-by: Igor Beliakov <[email protected]>
  • Loading branch information
Baarsgaard and weisdd authored Dec 18, 2024
1 parent 34511cd commit 80f4b26
Show file tree
Hide file tree
Showing 23 changed files with 71 additions and 25 deletions.
3 changes: 2 additions & 1 deletion api/v1beta1/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ type GrafanaCommonSpec struct {

// Allow the Operator to match this resource with Grafanas outside the current namespace
// +optional
AllowCrossNamespaceImport *bool `json:"allowCrossNamespaceImport,omitempty"`
// +kubebuilder:default=false
AllowCrossNamespaceImport bool `json:"allowCrossNamespaceImport,omitempty"`
}

// Common Functions that all CRs should implement, excluding Grafana
Expand Down
5 changes: 1 addition & 4 deletions api/v1beta1/grafanadashboard_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,7 @@ func (in *GrafanaDashboardStatus) getContentCache(url string, cacheDuration time
}

func (in *GrafanaDashboard) IsAllowCrossNamespaceImport() bool {
if in.Spec.AllowCrossNamespaceImport != nil {
return *in.Spec.AllowCrossNamespaceImport
}
return false
return in.Spec.AllowCrossNamespaceImport
}

func (in *GrafanaDashboard) IsUpdatedUID(uid string) bool {
Expand Down
5 changes: 1 addition & 4 deletions api/v1beta1/grafanadatasource_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ func (in *GrafanaDatasource) CustomUIDOrUID() string {
}

func (in *GrafanaDatasource) IsAllowCrossNamespaceImport() bool {
if in.Spec.AllowCrossNamespaceImport != nil {
return *in.Spec.AllowCrossNamespaceImport
}
return false
return in.Spec.AllowCrossNamespaceImport
}

func (in *GrafanaDatasourceList) Find(namespace string, name string) *GrafanaDatasource {
Expand Down
5 changes: 1 addition & 4 deletions api/v1beta1/grafanafolder_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,5 @@ func (in *GrafanaFolder) MatchNamespace() string {
}

func (in *GrafanaFolder) AllowCrossNamespace() bool {
if in.Spec.AllowCrossNamespaceImport != nil {
return *in.Spec.AllowCrossNamespaceImport
}
return false
return in.Spec.AllowCrossNamespaceImport
}
5 changes: 0 additions & 5 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ spec:
description: GrafanaAlertRuleGroupSpec defines the desired state of GrafanaAlertRuleGroup
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ spec:
description: GrafanaContactPointSpec defines the desired state of GrafanaContactPoint
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ spec:
description: GrafanaDashboardSpec defines the desired state of GrafanaDashboard
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ spec:
description: GrafanaDatasourceSpec defines the desired state of GrafanaDatasource
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ spec:
description: GrafanaFolderSpec defines the desired state of GrafanaFolder
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ spec:
GrafanaNotificationPolicy
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
6 changes: 2 additions & 4 deletions controllers/controller_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,6 @@ func TestLabelsSatisfyMatchExpressions(t *testing.T) {
}

var _ = Describe("GetMatchingInstances functions", Ordered, func() {
refTrue := true
refFalse := false
namespace := corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "get-matching-test",
Expand All @@ -253,7 +251,7 @@ var _ = Describe("GetMatchingInstances functions", Ordered, func() {
},
Spec: v1beta1.GrafanaFolderSpec{
GrafanaCommonSpec: v1beta1.GrafanaCommonSpec{
AllowCrossNamespaceImport: &refTrue,
AllowCrossNamespaceImport: true,
InstanceSelector: &v1.LabelSelector{
MatchLabels: map[string]string{
"test": "folder",
Expand All @@ -265,7 +263,7 @@ var _ = Describe("GetMatchingInstances functions", Ordered, func() {
// Create duplicate resources, changing key fields
denyFolder := allowFolder.DeepCopy()
denyFolder.Name = "deny-cross-namespace"
denyFolder.Spec.AllowCrossNamespaceImport = &refFalse
denyFolder.Spec.AllowCrossNamespaceImport = false

matchAllFolder := allowFolder.DeepCopy()
matchAllFolder.Name = "invalid-match-labels"
Expand Down
3 changes: 1 addition & 2 deletions controllers/grafanaalertrulegroup_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ type GrafanaAlertRuleGroupReconciler struct {

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
//+kubebuilder:rbac:groups=grafana.integreatly.org,resources=grafanaalertrulegroups/finalizers,verbs=update

func (r *GrafanaAlertRuleGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
controllerLog := log.FromContext(ctx).WithName("GrafanaAlertRuleGroupReconciler")
Expand Down Expand Up @@ -336,7 +335,7 @@ func (r *GrafanaAlertRuleGroupReconciler) GetMatchingInstances(ctx context.Conte
if err != nil || len(instances.Items) == 0 {
return nil, err
}
if group.Spec.AllowCrossNamespaceImport != nil && *group.Spec.AllowCrossNamespaceImport {
if group.Spec.AllowCrossNamespaceImport {
return instances.Items, nil
}
items := []grafanav1beta1.Grafana{}
Expand Down
2 changes: 1 addition & 1 deletion controllers/grafanacontactpoint_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func (r *GrafanaContactPointReconciler) GetMatchingInstances(ctx context.Context
if err != nil || len(instances.Items) == 0 {
return nil, err
}
if contactPoint.Spec.AllowCrossNamespaceImport != nil && *contactPoint.Spec.AllowCrossNamespaceImport {
if contactPoint.Spec.AllowCrossNamespaceImport {
return instances.Items, nil
}
items := []grafanav1beta1.Grafana{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ spec:
description: GrafanaAlertRuleGroupSpec defines the desired state of GrafanaAlertRuleGroup
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ spec:
description: GrafanaContactPointSpec defines the desired state of GrafanaContactPoint
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ spec:
description: GrafanaDashboardSpec defines the desired state of GrafanaDashboard
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ spec:
description: GrafanaDatasourceSpec defines the desired state of GrafanaDatasource
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ spec:
description: GrafanaFolderSpec defines the desired state of GrafanaFolder
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ spec:
GrafanaNotificationPolicy
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
6 changes: 6 additions & 0 deletions deploy/kustomize/base/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ spec:
description: GrafanaAlertRuleGroupSpec defines the desired state of GrafanaAlertRuleGroup
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down Expand Up @@ -358,6 +359,7 @@ spec:
description: GrafanaContactPointSpec defines the desired state of GrafanaContactPoint
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down Expand Up @@ -654,6 +656,7 @@ spec:
description: GrafanaDashboardSpec defines the desired state of GrafanaDashboard
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down Expand Up @@ -1149,6 +1152,7 @@ spec:
description: GrafanaDatasourceSpec defines the desired state of GrafanaDatasource
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down Expand Up @@ -1414,6 +1418,7 @@ spec:
description: GrafanaFolderSpec defines the desired state of GrafanaFolder
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down Expand Up @@ -1629,6 +1634,7 @@ spec:
GrafanaNotificationPolicy
properties:
allowCrossNamespaceImport:
default: false
description: Allow the Operator to match this resource with Grafanas
outside the current namespace
type: boolean
Expand Down
12 changes: 12 additions & 0 deletions docs/docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ GrafanaAlertRuleGroupSpec defines the desired state of GrafanaAlertRuleGroup
<td>boolean</td>
<td>
Allow the Operator to match this resource with Grafanas outside the current namespace<br/>
<br/>
<i>Default</i>: false<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down Expand Up @@ -721,6 +723,8 @@ GrafanaContactPointSpec defines the desired state of GrafanaContactPoint
<td>boolean</td>
<td>
Allow the Operator to match this resource with Grafanas outside the current namespace<br/>
<br/>
<i>Default</i>: false<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down Expand Up @@ -1205,6 +1209,8 @@ GrafanaDashboardSpec defines the desired state of GrafanaDashboard
<td>boolean</td>
<td>
Allow the Operator to match this resource with Grafanas outside the current namespace<br/>
<br/>
<i>Default</i>: false<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down Expand Up @@ -2321,6 +2327,8 @@ GrafanaDatasourceSpec defines the desired state of GrafanaDatasource
<td>boolean</td>
<td>
Allow the Operator to match this resource with Grafanas outside the current namespace<br/>
<br/>
<i>Default</i>: false<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down Expand Up @@ -2902,6 +2910,8 @@ GrafanaFolderSpec defines the desired state of GrafanaFolder
<td>boolean</td>
<td>
Allow the Operator to match this resource with Grafanas outside the current namespace<br/>
<br/>
<i>Default</i>: false<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down Expand Up @@ -3257,6 +3267,8 @@ GrafanaNotificationPolicySpec defines the desired state of GrafanaNotificationPo
<td>boolean</td>
<td>
Allow the Operator to match this resource with Grafanas outside the current namespace<br/>
<br/>
<i>Default</i>: false<br/>
</td>
<td>false</td>
</tr><tr>
Expand Down
32 changes: 32 additions & 0 deletions tests/example-resources.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,38 @@ spec:
admin_password: secret
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaContactPoint
metadata:
name: testdata
labels:
test: "testdata"
spec:
name: testdata
type: email
instanceSelector:
matchLabels:
test: "testdata"
settings:
addresses: "[email protected]"
subject: 'Grafana Alert'
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaNotificationPolicy
metadata:
name: testdata
labels:
test: "testdata"
spec:
instanceSelector:
matchLabels:
test: "testdata"
route:
receiver: testdata
group_by:
- grafana_folder
- alertname
---
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaFolder
metadata:
name: testdata
Expand Down

0 comments on commit 80f4b26

Please sign in to comment.