Skip to content

Commit 30d23a4

Browse files
author
Dayakar Maruboena
committed
feat: Add TLS support for Perses-Tempo datasource
1 parent 6d140e8 commit 30d23a4

File tree

9 files changed

+394
-59
lines changed

9 files changed

+394
-59
lines changed

‎api/services/v1alpha1/monitoring_types.go‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ type Traces struct {
8787
// +kubebuilder:validation:Pattern="^(0(\\.[0-9]+)?|1(\\.0+)?)$"
8888
SampleRatio string `json:"sampleRatio,omitempty"`
8989
// TLS configuration for Tempo gRPC connections
90+
// +optional
91+
// +nullable
9092
TLS *TracesTLS `json:"tls,omitempty"`
9193
// Exporters defines custom trace exporters for sending traces to external observability tools.
9294
// Each key represents the exporter name, and the value contains the exporter configuration.
@@ -95,10 +97,10 @@ type Traces struct {
9597
Exporters map[string]runtime.RawExtension `json:"exporters,omitempty"`
9698
}
9799

98-
// TracesTLS defines TLS configuration for traces collection
100+
// TracesTLS defines TLS configuration for trace ingestion and query APIs
99101
type TracesTLS struct {
100-
// Enabled enables TLS for Tempo gRPC connections
101-
// +kubebuilder:default=true
102+
// Enabled enables TLS for Tempo OTLP ingestion (gRPC/HTTP) and query APIs (HTTP)
103+
// TLS is disabled by default to maintain backward compatibility
102104
Enabled bool `json:"enabled,omitempty"`
103105
// CertificateSecret specifies the name of the secret containing TLS certificates
104106
// If not specified, OpenShift service serving certificates will be used
@@ -108,8 +110,18 @@ type TracesTLS struct {
108110
CAConfigMap string `json:"caConfigMap,omitempty"`
109111
}
110112

111-
// TracesStorage defines the storage configuration for tracing.
112-
// +kubebuilder:validation:XValidation:rule="self.backend != 'pv' ? has(self.secret) : true", message="When backend is s3 or gcs, the 'secret' field must be specified and non-empty"
113+
// Storage backend type constants
114+
const (
115+
// StorageBackendPV represents persistent volume storage backend
116+
StorageBackendPV = "pv"
117+
// StorageBackendS3 represents S3-compatible storage backend
118+
StorageBackendS3 = "s3"
119+
// StorageBackendGCS represents Google Cloud Storage backend
120+
StorageBackendGCS = "gcs"
121+
)
122+
123+
// TracesStorage defines the storage configuration for tracing
124+
// +kubebuilder:validation:XValidation:rule="self.backend != 'pv' ? (has(self.secret) && self.secret != '') : true", message="When backend is s3 or gcs, the 'secret' field must be specified and non-empty"
113125
// +kubebuilder:validation:XValidation:rule="self.backend != 'pv' ? !has(self.size) : true", message="Size is supported when backend is pv only"
114126
type TracesStorage struct {
115127
// Backend defines the storage backend type.

‎docs/api-overview.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3039,7 +3039,7 @@ _Appears in:_
30393039

30403040

30413041

3042-
TracesStorage defines the storage configuration for tracing.
3042+
TracesStorage defines the storage configuration for tracing
30433043

30443044

30453045

@@ -3058,7 +3058,7 @@ _Appears in:_
30583058

30593059

30603060

3061-
TracesTLS defines TLS configuration for traces collection
3061+
TracesTLS defines TLS configuration for trace ingestion and query APIs
30623062

30633063

30643064

@@ -3067,7 +3067,7 @@ _Appears in:_
30673067

30683068
| Field | Description | Default | Validation |
30693069
| --- | --- | --- | --- |
3070-
| `enabled` _boolean_ | Enabled enables TLS for Tempo gRPC connections | true | |
3070+
| `enabled` _boolean_ | Enabled enables TLS for Tempo OTLP ingestion (gRPC/HTTP) and query APIs (HTTP)<br />TLS is disabled by default to maintain backward compatibility | | |
30713071
| `certificateSecret` _string_ | CertificateSecret specifies the name of the secret containing TLS certificates<br />If not specified, OpenShift service serving certificates will be used | | |
30723072
| `caConfigMap` _string_ | CAConfigMap specifies the name of the ConfigMap containing the CA certificate<br />Required for mutual TLS authentication | | |
30733073

‎internal/controller/services/monitoring/monitoring_controller_actions.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const (
4646
PersesTempoDashboardTemplate = "resources/perses-tempo-dashboard.tmpl.yaml"
4747
PersesDatasourcePrometheusTemplate = "resources/perses-datasource-prometheus.tmpl.yaml"
4848
PrometheusClusterProxyTemplate = "resources/data-science-prometheus-cluster-proxy.tmpl.yaml"
49+
TempoServiceCAConfigMapTemplate = "resources/tempo-service-ca-configmap.tmpl.yaml"
4950

5051
// Resource names.
5152
PersesTempoDatasourceName = "tempo-datasource"
@@ -558,6 +559,10 @@ func deployPersesTempoIntegration(ctx context.Context, rr *odhtypes.Reconciliati
558559
FS: resourcesFS,
559560
Path: PersesTempoDatasourceTemplate,
560561
},
562+
{
563+
FS: resourcesFS,
564+
Path: TempoServiceCAConfigMapTemplate,
565+
},
561566
}
562567

563568
// Only deploy dashboard template if its CRD exists

‎internal/controller/services/monitoring/monitoring_controller_support.go‎

Lines changed: 31 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,38 @@ func addTracesTemplateData(templateData map[string]any, traces *serviceApi.Trace
215215
// Add retention for all backends (both TempoMonolithic and TempoStack)
216216
templateData["TracesRetention"] = traces.Storage.Retention.Duration.String()
217217

218+
// Determine TLS enabled state for query endpoints (reuses existing TLS configuration)
219+
tlsEnabled := determineTLSEnabled(traces)
220+
templateData["TempoTLSEnabled"] = tlsEnabled
221+
222+
// Set TLS certificate configuration
223+
if tlsEnabled {
224+
// traces.TLS is guaranteed non-nil here since determineTLSEnabled returns false when TLS is nil
225+
templateData["TempoCertificateSecret"] = traces.TLS.CertificateSecret
226+
templateData["TempoCAConfigMap"] = traces.TLS.CAConfigMap
227+
} else {
228+
// Set empty values to avoid template missing key errors
229+
templateData["TempoCertificateSecret"] = ""
230+
templateData["TempoCAConfigMap"] = ""
231+
}
232+
233+
// Use HTTPS for query endpoints when TLS is enabled (defaults to disabled)
234+
protocol := "http"
235+
if tlsEnabled {
236+
protocol = "https"
237+
}
238+
218239
// Add tempo-related data from traces.Storage fields (Storage is a struct, not a pointer)
219240
switch traces.Storage.Backend {
220-
case "pv":
241+
case serviceApi.StorageBackendPV:
221242
templateData["TempoEndpoint"] = fmt.Sprintf("tempo-data-science-tempomonolithic.%s.svc.cluster.local:4317", namespace)
222-
// Perses datasource needs HTTP query endpoint (port 3200)
223-
templateData["TempoQueryEndpoint"] = fmt.Sprintf("http://tempo-data-science-tempomonolithic.%s.svc.cluster.local:3200", namespace)
243+
// Perses datasource query endpoint (port 3200) - uses HTTPS when TLS is enabled
244+
templateData["TempoQueryEndpoint"] = fmt.Sprintf("%s://tempo-data-science-tempomonolithic.%s.svc.cluster.local:3200", protocol, namespace)
224245
templateData["Size"] = traces.Storage.Size
225-
case "s3", "gcs":
246+
case serviceApi.StorageBackendS3, serviceApi.StorageBackendGCS:
226247
templateData["TempoEndpoint"] = fmt.Sprintf("tempo-data-science-tempostack-gateway.%s.svc.cluster.local:4317", namespace)
227-
// Perses datasource needs HTTP query endpoint via gateway (port 8080)
228-
templateData["TempoQueryEndpoint"] = fmt.Sprintf("http://tempo-data-science-tempostack-gateway.%s.svc.cluster.local:8080", namespace)
248+
// Perses datasource query endpoint via gateway (port 8080) - uses HTTPS when TLS is enabled
249+
templateData["TempoQueryEndpoint"] = fmt.Sprintf("%s://tempo-data-science-tempostack-gateway.%s.svc.cluster.local:8080", protocol, namespace)
229250
templateData["Secret"] = traces.Storage.Secret
230251
}
231252

@@ -328,7 +349,6 @@ func getTemplateData(ctx context.Context, rr *odhtypes.ReconciliationRequest) (m
328349

329350
// Add traces-related data if traces are configured
330351
if traces := monitoring.Spec.Traces; traces != nil {
331-
addTracesData(traces, monitoring.Spec.Namespace, templateData)
332352
if err := addTracesTemplateData(templateData, traces, monitoring.Spec.Namespace); err != nil {
333353
return nil, err
334354
}
@@ -538,48 +558,15 @@ func addExportersData(metrics *serviceApi.Metrics, templateData map[string]any)
538558
return nil
539559
}
540560

541-
// addTracesData adds traces configuration data to the template data map.
542-
func addTracesData(traces *serviceApi.Traces, namespace string, templateData map[string]any) {
543-
templateData["OtlpEndpoint"] = fmt.Sprintf("http://data-science-collector.%s.svc.cluster.local:4317", namespace)
544-
templateData["SampleRatio"] = traces.SampleRatio
545-
templateData["Backend"] = traces.Storage.Backend // backend has default "pv" set in API
546-
547-
tlsEnabled := determineTLSEnabled(traces)
548-
templateData["TempoTLSEnabled"] = tlsEnabled
549-
550-
if tlsEnabled && traces.TLS != nil {
551-
templateData["TempoCertificateSecret"] = traces.TLS.CertificateSecret
552-
templateData["TempoCAConfigMap"] = traces.TLS.CAConfigMap
553-
} else {
554-
// Set empty values to avoid template missing key errors
555-
templateData["TempoCertificateSecret"] = ""
556-
templateData["TempoCAConfigMap"] = ""
557-
}
558-
559-
templateData["TracesRetention"] = traces.Storage.Retention.Duration.String()
560-
561-
setTempoEndpointAndStorageData(traces, namespace, templateData)
562-
}
563-
564561
// determineTLSEnabled determines if TLS should be enabled for traces.
562+
// TLS must be explicitly enabled via traces.TLS.Enabled field.
563+
// Default is false to avoid Tempo operator certificate provisioning issues.
565564
func determineTLSEnabled(traces *serviceApi.Traces) bool {
566565
if traces.TLS != nil {
567566
return traces.TLS.Enabled
568567
}
569-
return traces.Storage.Backend == "pv"
570-
}
571-
572-
// setTempoEndpointAndStorageData sets the tempo endpoint and storage-specific data.
573-
func setTempoEndpointAndStorageData(traces *serviceApi.Traces, namespace string, templateData map[string]any) {
574-
switch traces.Storage.Backend {
575-
case "pv":
576-
templateData["TempoEndpoint"] = fmt.Sprintf("tempo-data-science-tempomonolithic.%s.svc.cluster.local:4317", namespace)
577-
templateData["Size"] = traces.Storage.Size
578-
case "s3", "gcs":
579-
// Always use gateway endpoint for S3/GCS backends (required for OpenShift mode)
580-
templateData["TempoEndpoint"] = fmt.Sprintf("tempo-data-science-tempostack-gateway.%s.svc.cluster.local:4317", namespace)
581-
templateData["Secret"] = traces.Storage.Secret
582-
}
568+
// Default to false - user must explicitly enable TLS
569+
return false
583570
}
584571

585572
// getResourceValueOrDefault returns the resource value or a default if empty or zero.

‎internal/controller/services/monitoring/monitoring_controller_support_test.go‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,186 @@ func TestMonitoringStackThanosQuerierIntegration(t *testing.T) {
964964
}
965965
}
966966

967+
func TestDetermineTLSEnabled(t *testing.T) {
968+
tests := []struct {
969+
name string
970+
traces *serviceApi.Traces
971+
expected bool
972+
}{
973+
{
974+
name: "TLS explicitly enabled",
975+
traces: &serviceApi.Traces{
976+
TLS: &serviceApi.TracesTLS{
977+
Enabled: true,
978+
},
979+
Storage: serviceApi.TracesStorage{
980+
Backend: "pv",
981+
},
982+
},
983+
expected: true,
984+
},
985+
{
986+
name: "TLS explicitly disabled",
987+
traces: &serviceApi.Traces{
988+
TLS: &serviceApi.TracesTLS{
989+
Enabled: false,
990+
},
991+
Storage: serviceApi.TracesStorage{
992+
Backend: "pv",
993+
},
994+
},
995+
expected: false,
996+
},
997+
{
998+
name: "TLS nil - PV backend defaults to false",
999+
traces: &serviceApi.Traces{
1000+
Storage: serviceApi.TracesStorage{
1001+
Backend: "pv",
1002+
},
1003+
},
1004+
expected: false,
1005+
},
1006+
{
1007+
name: "TLS nil - S3 backend defaults to false",
1008+
traces: &serviceApi.Traces{
1009+
Storage: serviceApi.TracesStorage{
1010+
Backend: "s3",
1011+
},
1012+
},
1013+
expected: false,
1014+
},
1015+
{
1016+
name: "TLS nil - GCS backend defaults to false",
1017+
traces: &serviceApi.Traces{
1018+
Storage: serviceApi.TracesStorage{
1019+
Backend: "gcs",
1020+
},
1021+
},
1022+
expected: false,
1023+
},
1024+
}
1025+
1026+
for _, tt := range tests {
1027+
t.Run(tt.name, func(t *testing.T) {
1028+
result := determineTLSEnabled(tt.traces)
1029+
if result != tt.expected {
1030+
t.Errorf("determineTLSEnabled() = %v, expected %v", result, tt.expected)
1031+
}
1032+
})
1033+
}
1034+
}
1035+
1036+
func TestAddTracesTemplateData_TLS(t *testing.T) {
1037+
tests := []struct {
1038+
name string
1039+
traces *serviceApi.Traces
1040+
namespace string
1041+
expectedTLSEnabled bool
1042+
expectedHTTPProtocol string
1043+
expectedPVEndpoint string
1044+
expectedS3Endpoint string
1045+
}{
1046+
{
1047+
name: "PV backend with TLS disabled (default)",
1048+
traces: &serviceApi.Traces{
1049+
SampleRatio: "0.1",
1050+
Storage: serviceApi.TracesStorage{
1051+
Backend: "pv",
1052+
Size: "5Gi",
1053+
Retention: metav1.Duration{Duration: 90 * 24 * 60 * 60 * 1000000000}, // 90 days in nanoseconds
1054+
},
1055+
},
1056+
namespace: "test-namespace",
1057+
expectedTLSEnabled: false,
1058+
expectedHTTPProtocol: "http",
1059+
expectedPVEndpoint: "http://tempo-data-science-tempomonolithic.test-namespace.svc.cluster.local:3200",
1060+
},
1061+
{
1062+
name: "PV backend with TLS explicitly disabled",
1063+
traces: &serviceApi.Traces{
1064+
SampleRatio: "0.1",
1065+
TLS: &serviceApi.TracesTLS{
1066+
Enabled: false,
1067+
},
1068+
Storage: serviceApi.TracesStorage{
1069+
Backend: "pv",
1070+
Size: "5Gi",
1071+
Retention: metav1.Duration{Duration: 90 * 24 * 60 * 60 * 1000000000},
1072+
},
1073+
},
1074+
namespace: "test-namespace",
1075+
expectedTLSEnabled: false,
1076+
expectedHTTPProtocol: "http",
1077+
expectedPVEndpoint: "http://tempo-data-science-tempomonolithic.test-namespace.svc.cluster.local:3200",
1078+
},
1079+
{
1080+
name: "S3 backend with TLS disabled (default)",
1081+
traces: &serviceApi.Traces{
1082+
SampleRatio: "0.1",
1083+
Storage: serviceApi.TracesStorage{
1084+
Backend: "s3",
1085+
Secret: "s3-secret",
1086+
Retention: metav1.Duration{Duration: 90 * 24 * 60 * 60 * 1000000000},
1087+
},
1088+
},
1089+
namespace: "test-namespace",
1090+
expectedTLSEnabled: false,
1091+
expectedHTTPProtocol: "http",
1092+
expectedS3Endpoint: "http://tempo-data-science-tempostack-gateway.test-namespace.svc.cluster.local:8080",
1093+
},
1094+
{
1095+
name: "S3 backend with TLS explicitly enabled",
1096+
traces: &serviceApi.Traces{
1097+
SampleRatio: "0.1",
1098+
TLS: &serviceApi.TracesTLS{
1099+
Enabled: true,
1100+
},
1101+
Storage: serviceApi.TracesStorage{
1102+
Backend: "s3",
1103+
Secret: "s3-secret",
1104+
Retention: metav1.Duration{Duration: 90 * 24 * 60 * 60 * 1000000000},
1105+
},
1106+
},
1107+
namespace: "test-namespace",
1108+
expectedTLSEnabled: true,
1109+
expectedHTTPProtocol: "https",
1110+
expectedS3Endpoint: "https://tempo-data-science-tempostack-gateway.test-namespace.svc.cluster.local:8080",
1111+
},
1112+
}
1113+
1114+
for _, tt := range tests {
1115+
t.Run(tt.name, func(t *testing.T) {
1116+
g := NewWithT(t)
1117+
1118+
templateData := make(map[string]any)
1119+
err := addTracesTemplateData(templateData, tt.traces, tt.namespace)
1120+
g.Expect(err).ShouldNot(HaveOccurred())
1121+
1122+
// Verify TLS enabled flag
1123+
tlsEnabled, exists := templateData["TempoTLSEnabled"]
1124+
g.Expect(exists).Should(BeTrue(), "TempoTLSEnabled should be set")
1125+
g.Expect(tlsEnabled).Should(Equal(tt.expectedTLSEnabled))
1126+
1127+
// Verify query endpoint URL based on backend
1128+
queryEndpoint, exists := templateData["TempoQueryEndpoint"]
1129+
g.Expect(exists).Should(BeTrue(), "TempoQueryEndpoint should be set")
1130+
1131+
switch tt.traces.Storage.Backend {
1132+
case serviceApi.StorageBackendPV:
1133+
g.Expect(queryEndpoint).Should(Equal(tt.expectedPVEndpoint))
1134+
case serviceApi.StorageBackendS3, serviceApi.StorageBackendGCS:
1135+
g.Expect(queryEndpoint).Should(Equal(tt.expectedS3Endpoint))
1136+
}
1137+
1138+
// Verify other template data fields are set
1139+
g.Expect(templateData).Should(HaveKey("OtlpEndpoint"))
1140+
g.Expect(templateData).Should(HaveKey("SampleRatio"))
1141+
g.Expect(templateData).Should(HaveKey("Backend"))
1142+
g.Expect(templateData).Should(HaveKey("TracesRetention"))
1143+
})
1144+
}
1145+
}
1146+
9671147
func TestIsLocalServiceEndpoint(t *testing.T) {
9681148
tests := []struct {
9691149
name string

‎internal/controller/services/monitoring/resources/perses-tempo-datasource.tmpl.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,14 @@ spec:
1616
kind: HTTPProxy
1717
spec:
1818
url: {{.TempoQueryEndpoint}}
19+
{{- if .TempoTLSEnabled }}
20+
client:
21+
tls:
22+
enable: true
23+
caCert:
24+
type: configmap
25+
name: tempo-service-ca
26+
certPath: service-ca.crt
27+
{{- end }}
1928
headers:
2029
X-Scope-OrgID: {{.Namespace}}

0 commit comments

Comments
 (0)