Allow override arbitrary tempo config (#718)

* Allow override arbitrary tempo config Signed-off-by: Ruben Vargas <[email protected]> * Replace custom config type with apiextensionsv1.JSONN Signed-off-by: Ruben Vargas <[email protected]> * Update apis/tempo/v1alpha1/tempostack_types.go Co-authored-by: Andreas Gerstmayr <[email protected]> * Some simplificiations, and warning webhook added Signed-off-by: Ruben Vargas <[email protected]> * Fix tests Signed-off-by: Ruben Vargas <[email protected]> * Update docs Signed-off-by: Ruben Vargas <[email protected]> * Convert from pointer to normal object Signed-off-by: Ruben Vargas <[email protected]> * Fix wrong override Signed-off-by: Ruben Vargas <[email protected]> * Update apis/tempo/v1alpha1/tempostack_webhook.go Co-authored-by: Andreas Gerstmayr <[email protected]> --------- Signed-off-by: Ruben Vargas <[email protected]> Co-authored-by: Andreas Gerstmayr <[email protected]>
grafana · Jan 8, 2024 · cbe369b · cbe369b
1 parent 105a022
commit cbe369b
Show file tree

Hide file tree

Showing 16 changed files with 465 additions and 7 deletions.
diff --git a/.chloggen/free_form_config.yaml b/.chloggen/free_form_config.yaml
@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. operator, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Allow override arbitrary tempo configurations
+
+# One or more tracking issues related to the change
+issues: [629]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
diff --git a/apis/tempo/v1alpha1/tempostack_types.go b/apis/tempo/v1alpha1/tempostack_types.go
@@ -2,6 +2,7 @@ package v1alpha1
 
 import (
 	corev1 "k8s.io/api/core/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
@@ -122,6 +123,18 @@ type TempoStackSpec struct {
 	// +kubebuilder:validation:Optional
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Observability"
 	Observability ObservabilitySpec `json:"observability,omitempty"`
+
+	// +optional
+	// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Extra Configurations"
+	ExtraConfig *ExtraConfigSpec `json:"extraConfig,omitempty"`
+}
+
+// ExtraConfigSpec defines extra configurations for tempo that will be merged with the operator generated, configurations defined here
+// has precedence and could override generated config.
+type ExtraConfigSpec struct {
+	// +optional
+	// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Tempo Extra Configurations"
+	Tempo apiextensionsv1.JSON `json:"tempo,omitempty"`
 }
 
 // ObservabilitySpec defines how telemetry data gets handled.

diff --git a/apis/tempo/v1alpha1/tempostack_webhook.go b/apis/tempo/v1alpha1/tempostack_webhook.go
@@ -450,6 +450,13 @@ func (v *validator) validate(ctx context.Context, obj runtime.Object) (admission
 		allErrors = append(allErrors, errors...)
 	}
 
+	if tempo.Spec.ExtraConfig != nil && len(tempo.Spec.ExtraConfig.Tempo.Raw) > 0 {
+		allWarnings = append(allWarnings, admission.Warnings{
+			"override tempo configuration could potentially break the stack, use it carefully",
+		}...)
+
+	}
+
 	allErrors = append(allErrors, v.validateReplicationFactor(*tempo)...)
 	allErrors = append(allErrors, v.validateQueryFrontend(*tempo)...)
 	allErrors = append(allErrors, v.validateGateway(*tempo)...)

diff --git a/apis/tempo/v1alpha1/tempostack_webhook_test.go b/apis/tempo/v1alpha1/tempostack_webhook_test.go
@@ -8,13 +8,15 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/grafana/tempo-operator/apis/config/v1alpha1"
 	"github.com/grafana/tempo-operator/internal/manifests/naming"
@@ -1578,10 +1580,125 @@ func TestValidateReceiverTLSAndGateway(t *testing.T) {
 	}
 }
 
+func TestWarning(t *testing.T) {
+	gvType := metav1.TypeMeta{
+		APIVersion: "testv1",
+		Kind:       "something",
+	}
+
+	tests := []struct {
+		name     string
+		input    runtime.Object
+		expected admission.Warnings
+		client   client.Client
+	}{
+		{
+			name: "no secret exists",
+			input: &TempoStack{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obj",
+					Namespace: "abc",
+				},
+				TypeMeta: gvType,
+				Spec: TempoStackSpec{
+					ServiceAccount: naming.DefaultServiceAccountName("test-obj"),
+					Storage: ObjectStorageSpec{
+						Secret: ObjectStorageSecretSpec{
+							Name: "not-found",
+						},
+					},
+					Template: TempoTemplateSpec{
+						Ingester: TempoComponentSpec{
+							Replicas: func(i int32) *int32 { return &i }(1),
+						},
+					},
+				},
+			},
+			client:   &k8sFake{},
+			expected: admission.Warnings{"Secret 'not-found' does not exist"},
+		},
+		{
+			name: "warning for use extra config",
+			input: &TempoStack{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obj",
+					Namespace: "abc",
+				},
+				TypeMeta: gvType,
+				Spec: TempoStackSpec{
+					ServiceAccount: naming.DefaultServiceAccountName("test-obj"),
+					Storage: ObjectStorageSpec{
+						Secret: ObjectStorageSecretSpec{
+							Name: "not-found",
+						},
+					},
+					Template: TempoTemplateSpec{
+						Ingester: TempoComponentSpec{
+							Replicas: func(i int32) *int32 { return &i }(1),
+						},
+					},
+					ExtraConfig: &ExtraConfigSpec{
+						Tempo: v1.JSON{Raw: []byte("{}")},
+					},
+				},
+			},
+			client: &k8sFake{
+				secret: &corev1.Secret{},
+			},
+			expected: admission.Warnings{
+				"override tempo configuration could potentially break the stack, use it carefully",
+			},
+		},
+		{
+			name: "no extra config used",
+			input: &TempoStack{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-obj",
+					Namespace: "abc",
+				},
+				TypeMeta: gvType,
+				Spec: TempoStackSpec{
+					ServiceAccount: naming.DefaultServiceAccountName("test-obj"),
+					Storage: ObjectStorageSpec{
+						Secret: ObjectStorageSecretSpec{
+							Name: "not-found",
+						},
+					},
+					Template: TempoTemplateSpec{
+						Ingester: TempoComponentSpec{
+							Replicas: func(i int32) *int32 { return &i }(1),
+						},
+					},
+				},
+			},
+			client: &k8sFake{
+				secret: &corev1.Secret{},
+			},
+			expected: admission.Warnings{},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			v := &validator{ctrlConfig: v1alpha1.ProjectConfig{}, client: test.client}
+			wrgs, _ := v.validate(context.Background(), test.input)
+			assert.Equal(t, test.expected, wrgs)
+		})
+	}
+}
+
 type k8sFake struct {
+	secret *corev1.Secret
 	client.Client
 }
 
-func (*k8sFake) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+func (k *k8sFake) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
+	if k.secret != nil {
+		if obj.GetObjectKind().GroupVersionKind().Kind == k.secret.Kind {
+			obj = k.secret
+			return nil
+		}
+	}
+
 	return fmt.Errorf("mock: fails always")
 }
diff --git a/apis/tempo/v1alpha1/zz_generated.deepcopy.go b/apis/tempo/v1alpha1/zz_generated.deepcopy.go
diff --git a/bundle/community/manifests/tempo-operator.clusterserviceversion.yaml b/bundle/community/manifests/tempo-operator.clusterserviceversion.yaml
@@ -42,7 +42,7 @@ metadata:
     capabilities: Deep Insights
     categories: Logging & Tracing,Monitoring
     containerImage: ghcr.io/grafana/tempo-operator/tempo-operator
-    createdAt: "2023-12-13T18:24:45Z"
+    createdAt: "2023-12-31T18:14:24Z"
     description: Create and manage deployments of Tempo, a high-scale distributed
       tracing backend.
     operatorframework.io/cluster-monitoring: "true"
@@ -87,6 +87,10 @@ spec:
         name: ""
         version: v1
       specDescriptors:
+      - displayName: Extra Configurations
+        path: extraConfig
+      - displayName: Tempo Extra Configurations
+        path: extraConfig.tempo
       - description: HashRing defines the spec for the distributed hash ring configuration.
         displayName: Hash Ring
         path: hashRing

diff --git a/bundle/community/manifests/tempo.grafana.com_tempostacks.yaml b/bundle/community/manifests/tempo.grafana.com_tempostacks.yaml
@@ -53,6 +53,14 @@ spec:
           spec:
             description: TempoStackSpec defines the desired state of TempoStack.
             properties:
+              extraConfig:
+                description: ExtraConfigSpec defines extra configurations for tempo
+                  that will be merged with the operator generated, configurations
+                  defined here has precedence and could override generated config.
+                properties:
+                  tempo:
+                    x-kubernetes-preserve-unknown-fields: true
+                type: object
               hashRing:
                 description: HashRing defines the spec for the distributed hash ring
                   configuration.

diff --git a/bundle/openshift/manifests/tempo-operator.clusterserviceversion.yaml b/bundle/openshift/manifests/tempo-operator.clusterserviceversion.yaml
@@ -42,7 +42,7 @@ metadata:
     capabilities: Deep Insights
     categories: Logging & Tracing,Monitoring
     containerImage: ghcr.io/grafana/tempo-operator/tempo-operator
-    createdAt: "2023-12-13T18:24:44Z"
+    createdAt: "2023-12-31T18:14:22Z"
     description: Create and manage deployments of Tempo, a high-scale distributed
       tracing backend.
     operatorframework.io/cluster-monitoring: "true"
@@ -87,6 +87,10 @@ spec:
         name: ""
         version: v1
       specDescriptors:
+      - displayName: Extra Configurations
+        path: extraConfig
+      - displayName: Tempo Extra Configurations
+        path: extraConfig.tempo
       - description: HashRing defines the spec for the distributed hash ring configuration.
         displayName: Hash Ring
         path: hashRing

diff --git a/bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml b/bundle/openshift/manifests/tempo.grafana.com_tempostacks.yaml
@@ -53,6 +53,14 @@ spec:
           spec:
             description: TempoStackSpec defines the desired state of TempoStack.
             properties:
+              extraConfig:
+                description: ExtraConfigSpec defines extra configurations for tempo
+                  that will be merged with the operator generated, configurations
+                  defined here has precedence and could override generated config.
+                properties:
+                  tempo:
+                    x-kubernetes-preserve-unknown-fields: true
+                type: object
               hashRing:
                 description: HashRing defines the spec for the distributed hash ring
                   configuration.

diff --git a/config/crd/bases/tempo.grafana.com_tempostacks.yaml b/config/crd/bases/tempo.grafana.com_tempostacks.yaml
@@ -50,6 +50,14 @@ spec:
           spec:
             description: TempoStackSpec defines the desired state of TempoStack.
             properties:
+              extraConfig:
+                description: ExtraConfigSpec defines extra configurations for tempo
+                  that will be merged with the operator generated, configurations
+                  defined here has precedence and could override generated config.
+                properties:
+                  tempo:
+                    x-kubernetes-preserve-unknown-fields: true
+                type: object
               hashRing:
                 description: HashRing defines the spec for the distributed hash ring
                   configuration.

diff --git a/config/manifests/community/bases/tempo-operator.clusterserviceversion.yaml b/config/manifests/community/bases/tempo-operator.clusterserviceversion.yaml
@@ -48,6 +48,10 @@ spec:
         name: ""
         version: v1
       specDescriptors:
+      - displayName: Extra Configurations
+        path: extraConfig
+      - displayName: Tempo Extra Configurations
+        path: extraConfig.tempo
       - description: HashRing defines the spec for the distributed hash ring configuration.
         displayName: Hash Ring
         path: hashRing

diff --git a/config/manifests/openshift/bases/tempo-operator.clusterserviceversion.yaml b/config/manifests/openshift/bases/tempo-operator.clusterserviceversion.yaml
@@ -48,6 +48,10 @@ spec:
         name: ""
         version: v1
       specDescriptors:
+      - displayName: Extra Configurations
+        path: extraConfig
+      - displayName: Tempo Extra Configurations
+        path: extraConfig.tempo
       - description: HashRing defines the spec for the distributed hash ring configuration.
         displayName: Hash Ring
         path: hashRing