gateway: make tempo-query forwarding on gateway optional

.chloggen/allow_gateway_without_jaeger_ui.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: Make Tempo-Query forwarding on gateway optional
+
+# One or more tracking issues related to the change
+issues: [628]
+
+# (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:

apis/tempo/v1alpha1/tempostack_webhook.go

@@ -258,13 +258,6 @@ func (v *validator) validateQueryFrontend(tempo TempoStack) field.ErrorList {
 func (v *validator) validateGateway(tempo TempoStack) field.ErrorList {
 	path := field.NewPath("spec").Child("template").Child("gateway").Child("enabled")
 	if tempo.Spec.Template.Gateway.Enabled {
-		if !tempo.Spec.Template.QueryFrontend.JaegerQuery.Enabled {
-			return field.ErrorList{
-				field.Invalid(path, tempo.Spec.Template.Gateway.Enabled,
-					"to use the gateway, please enable jaegerQuery",
-				)}
-		}
-
 		if tempo.Spec.Template.QueryFrontend.JaegerQuery.Ingress.Type != IngressTypeNone {
 			return field.ErrorList{
 				field.Invalid(path, tempo.Spec.Template.Gateway.Enabled,

apis/tempo/v1alpha1/tempostack_webhook_test.go

@@ -780,32 +780,6 @@ func TestValidateGatewayAndJaegerQuery(t *testing.T) {
 			},
 			expected: nil,
 		},
-		{
-			name: "invalid config disable jaegerQuery",
-			input: TempoStack{
-				Spec: TempoStackSpec{
-					ReplicationFactor: 3,
-					Template: TempoTemplateSpec{
-						QueryFrontend: TempoQueryFrontendSpec{
-							JaegerQuery: JaegerQuerySpec{
-								Enabled: false,
-								Ingress: IngressSpec{
-									Type: "ingress",
-								},
-							},
-						},
-						Gateway: TempoGatewaySpec{
-							Enabled: true,
-						},
-					},
-				},
-			},
-			expected: field.ErrorList{
-				field.Invalid(path, true,
-					"to use the gateway, please enable jaegerQuery",
-				),
-			},
-		},
 		{
 			name: "invalid configuration, ingress and gateway enabled",
 			input: TempoStack{

internal/manifests/gateway/gateway.go

@@ -98,6 +98,11 @@ func BuildGateway(params manifestutils.Params) ([]client.Object, error) {
 		objs = append(objs, routeObj)
 	}
 
+	dep.Spec.Template, err = patchTraceReadEndpoint(params, dep.Spec.Template)
+	if err != nil {
+		return nil, err
+	}
+
 	dep.Spec.Template, err = patchTracing(params.Tempo, dep.Spec.Template)
 	if err != nil {
 		return nil, err
@@ -114,6 +119,8 @@ func httpScheme(tls bool) string {
 	return "http"
 }
 
+const containerNameTempoGateway = "tempo-gateway"
+
 func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash string) *appsv1.Deployment {
 	tempo := params.Tempo
 	labels := manifestutils.ComponentLabels(manifestutils.GatewayComponentName, tempo.Name)
@@ -164,16 +171,14 @@ func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash
 					Tolerations:        cfg.Tolerations,
 					Containers: []corev1.Container{
 						{
-							Name:  "tempo-gateway",
+							Name:  containerNameTempoGateway,
 							Image: image,
 							Env:   proxy.ReadProxyVarsFromEnv(),
 							Args: append([]string{
 								fmt.Sprintf("--traces.tenant-header=%s", manifestutils.TenantHeader),
 								fmt.Sprintf("--web.listen=0.0.0.0:%d", portPublic),
 								fmt.Sprintf("--web.internal.listen=0.0.0.0:%d", portInternal),
 								fmt.Sprintf("--traces.write.endpoint=%s:%d", naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.DistributorComponentName), manifestutils.PortOtlpGrpcServer),
-								fmt.Sprintf("--traces.read.endpoint=%s://%s:%d", httpScheme(params.CtrlConfig.Gates.HTTPEncryption),
-									naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortJaegerQuery),
 								fmt.Sprintf("--traces.tempo.endpoint=%s://%s:%d", httpScheme(params.CtrlConfig.Gates.HTTPEncryption),
 									naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortHTTPServer),
 								fmt.Sprintf("--grpc.listen=0.0.0.0:%d", portGRPC),
@@ -279,6 +284,30 @@ func deployment(params manifestutils.Params, rbacCfgHash string, tenantsCfgHash
 	return dep
 }
 
+func patchTraceReadEndpoint(params manifestutils.Params, pod corev1.PodTemplateSpec) (corev1.PodTemplateSpec, error) {
+	if !params.Tempo.Spec.Template.QueryFrontend.JaegerQuery.Enabled {
+		return pod, nil
+	}
+
+	container := corev1.Container{
+		Args: []string{
+			fmt.Sprintf("--traces.read.endpoint=%s://%s:%d", httpScheme(params.CtrlConfig.Gates.HTTPEncryption),
+				naming.ServiceFqdn(params.Tempo.Namespace, params.Tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortJaegerQuery),
+		},
+	}
+
+	for i := range pod.Spec.Containers {
+		if pod.Spec.Containers[i].Name != containerNameTempoGateway {
+			continue
+		}
+		if err := mergo.Merge(&pod.Spec.Containers[i], container, mergo.WithAppendSlice); err != nil {
+			return corev1.PodTemplateSpec{}, err
+		}
+	}
+
+	return pod, nil
+}
+
 func patchTracing(tempo v1alpha1.TempoStack, pod corev1.PodTemplateSpec) (corev1.PodTemplateSpec, error) {
 	if tempo.Spec.Observability.Tracing.SamplingFraction == "" {
 		return pod, nil

internal/manifests/gateway/gateway_test.go

@@ -374,6 +374,126 @@ func TestPatchTracing(t *testing.T) {
 	}
 }
 
+func TestPatchTraceReadEndpoint(t *testing.T) {
+	tt := []struct {
+		name        string
+		inputParams manifestutils.Params
+		inputPod    corev1.PodTemplateSpec
+		expectPod   corev1.PodTemplateSpec
+		expectErr   error
+	}{
+		{
+			name: "with trace read endpoint",
+			inputParams: manifestutils.Params{
+				Tempo: v1alpha1.TempoStack{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "name",
+						Namespace: "default",
+					},
+					Spec: v1alpha1.TempoStackSpec{
+						Template: v1alpha1.TempoTemplateSpec{
+							QueryFrontend: v1alpha1.TempoQueryFrontendSpec{
+								JaegerQuery: v1alpha1.JaegerQuerySpec{
+									Enabled: true,
+								},
+							},
+						},
+					},
+				},
+			},
+			inputPod: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name: containerNameTempoGateway,
+							Args: []string{
+								"--abc",
+							},
+						},
+						{
+							Name: "second",
+							Args: []string{
+								"--xyz",
+							},
+						},
+					},
+				},
+			},
+			expectPod: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name: containerNameTempoGateway,
+							Args: []string{
+								"--abc",
+								"--traces.read.endpoint=http://tempo-name-query-frontend.default.svc.cluster.local:16686",
+							},
+						},
+						{
+							Name: "second",
+							Args: []string{
+								"--xyz",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "without trace read endpoint",
+			inputParams: manifestutils.Params{
+				Tempo: v1alpha1.TempoStack{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "name",
+						Namespace: "default",
+					},
+					Spec: v1alpha1.TempoStackSpec{
+						Template: v1alpha1.TempoTemplateSpec{
+							QueryFrontend: v1alpha1.TempoQueryFrontendSpec{
+								JaegerQuery: v1alpha1.JaegerQuerySpec{
+									Enabled: false,
+								},
+							},
+						},
+					},
+				},
+			},
+			inputPod: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name: containerNameTempoGateway,
+							Args: []string{
+								"--abc",
+							},
+						},
+					},
+				},
+			},
+			expectPod: corev1.PodTemplateSpec{
+				Spec: corev1.PodSpec{
+					Containers: []corev1.Container{
+						{
+							Name: containerNameTempoGateway,
+							Args: []string{
+								"--abc",
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			pod, err := patchTraceReadEndpoint(tc.inputParams, tc.inputPod)
+			require.Equal(t, tc.expectErr, err)
+			assert.Equal(t, tc.expectPod, pod)
+		})
+	}
+}
+
 func TestTLSParameters(t *testing.T) {
 	tempo := v1alpha1.TempoStack{
 		ObjectMeta: metav1.ObjectMeta{
@@ -410,6 +530,11 @@ func TestTLSParameters(t *testing.T) {
 				},
 			},
 			Template: v1alpha1.TempoTemplateSpec{
+				QueryFrontend: v1alpha1.TempoQueryFrontendSpec{
+					JaegerQuery: v1alpha1.JaegerQuerySpec{
+						Enabled: true,
+					},
+				},
 				Gateway: v1alpha1.TempoGatewaySpec{
 					Enabled: true,
 				},
@@ -444,8 +569,6 @@ func TestTLSParameters(t *testing.T) {
 	assert.Contains(t, args, fmt.Sprintf("--traces.tls.key-file=%s/tls.key", manifestutils.TempoServerTLSDir()))
 	assert.Contains(t, args, fmt.Sprintf("--traces.tls.cert-file=%s/tls.crt", manifestutils.TempoServerTLSDir()))
 	assert.Contains(t, args, fmt.Sprintf("--traces.tls.ca-file=%s/service-ca.crt", manifestutils.CABundleDir))
-	assert.Contains(t, args, fmt.Sprintf("--traces.read.endpoint=https://%s:16686",
-		naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName)))
 	assert.Contains(t, args, fmt.Sprintf("--traces.tempo.endpoint=https://%s:%d",
 		naming.ServiceFqdn(tempo.Namespace, tempo.Name, manifestutils.QueryFrontendComponentName), manifestutils.PortHTTPServer))
 	assert.Equal(t, corev1.URISchemeHTTPS, dep.Spec.Template.Spec.Containers[0].ReadinessProbe.HTTPGet.Scheme)

tests/e2e/gateway/02-assert.yaml

@@ -139,3 +139,19 @@ spec:
 
 status:
   readyReplicas: 1
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  labels:
+    app.kubernetes.io/component: query-frontend
+    app.kubernetes.io/instance: foo
+    app.kubernetes.io/managed-by: tempo-operator
+    app.kubernetes.io/name: tempo
+  name: tempo-foo-query-frontend
+spec:
+  template:
+    spec:
+      containers:
+        - name: tempo
+        - name: tempo-query

tests/e2e/gateway/03-if-ui-exists-error.yaml

@@ -0,0 +1,5 @@
+---
+apiVersion: kuttl.dev/v1beta1
+kind: TestStep
+commands:
+  - script: "while true; do container_names=$(kubectl get deployment/tempo-foo-query-frontend -n $NAMESPACE -o=jsonpath='{.spec.template.spec.containers[*].name}'); if [[ -n $(echo \"$container_names\" | grep \"tempo-query\") ]]; then echo \"tempo-query container still exists. Retrying...\"; else echo \"tempo-query container no longer exists. Test passed.\"; exit 0; fi; sleep 5; done"

tests/e2e/gateway/03-install-disable-jaeger-query.yaml

@@ -0,0 +1,47 @@
+---
+apiVersion: tempo.grafana.com/v1alpha1
+kind: TempoStack
+metadata:
+  name: foo
+spec:
+  template:
+    gateway:
+      enabled: true
+    queryFrontend:
+      jaegerQuery:
+        enabled: false
+  storage:
+    secret:
+      type: s3
+      name: minio-test
+  storageSize: 200M
+  tenants:
+    mode: static
+    authentication:
+      - tenantName: test-oidc
+        tenantId: test-oidc
+        oidc:
+          issuerURL: http://dex.svc:30556/dex
+          redirectURL: http://tempo-foo-gateway.svc:8080/oidc/test-oidc/callback
+          usernameClaim: email
+          secret:
+            name: oidc-test
+    authorization:
+      roleBindings:
+      - name: "test"
+        roles:
+        - read-write
+        subjects:
+        - kind: user
+          name: "[email protected]"
+      roles:
+      - name: read-write
+        permissions:
+        - read
+        - write
+        resources:
+        - logs
+        - metrics
+        - traces
+        tenants:
+        - test-oidc