Skip to content

Commit 41bd22c

Browse files
ChrisJBurnsclaude
andcommitted
Move endpoint validation to Validate(), fix smart quote corruption
The gci linter was replacing ASCII quotes with Unicode smart quotes (U+201C/U+201D) in kubebuilder annotations, breaking controller-gen CRD generation. Additionally, the CEL rule for endpoint validation generated double-quoted YAML that the Helm wrapper mishandled. Move the endpoint-requires-signals validation from a CEL rule to the programmatic Validate() method, which avoids both issues and provides clearer error messages. Add test case for this validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5ae1d5f commit 41bd22c

File tree

4 files changed

+34
-7
lines changed

4 files changed

+34
-7
lines changed

cmd/thv-operator/api/v1alpha1/mcptelemetryconfig_types.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ type SensitiveHeader struct {
3131
// - Adds ResourceAttributes for shared OTel resource attributes
3232
//
3333
// +kubebuilder:validation:XValidation:rule="!has(self.headers) || !has(self.sensitiveHeaders) || self.sensitiveHeaders.all(sh, !(sh.name in self.headers))",message="a header name cannot appear in both headers and sensitiveHeaders"
34-
// +kubebuilder:validation:XValidation:rule=”!has(self.endpoint) || size(self.endpoint) == 0 || (has(self.tracing) && self.tracing.enabled) || (has(self.metrics) && self.metrics.enabled)”,message=”endpoint requires at least one of tracing or metrics to be enabled”
3534
//
3635
//nolint:lll // CEL validation rules exceed line length limit
3736
type MCPTelemetryOTelConfig struct {
@@ -178,9 +177,30 @@ type MCPTelemetryConfigReference struct {
178177
// CEL catches issues at API admission time, but this method also validates
179178
// stored objects to catch any that bypassed CEL or were stored before CEL rules were added.
180179
func (r *MCPTelemetryConfig) Validate() error {
180+
if err := r.validateEndpointRequiresSignals(); err != nil {
181+
return err
182+
}
181183
return r.validateSensitiveHeaders()
182184
}
183185

186+
// validateEndpointRequiresSignals rejects an endpoint when neither tracing nor metrics is enabled.
187+
// Without this check the config would pass CRD validation but fail at runtime in telemetry.NewProvider.
188+
func (r *MCPTelemetryConfig) validateEndpointRequiresSignals() error {
189+
if r.Spec.OpenTelemetry == nil {
190+
return nil
191+
}
192+
otel := r.Spec.OpenTelemetry
193+
if otel.Endpoint == "" {
194+
return nil
195+
}
196+
tracingEnabled := otel.Tracing != nil && otel.Tracing.Enabled
197+
metricsEnabled := otel.Metrics != nil && otel.Metrics.Enabled
198+
if !tracingEnabled && !metricsEnabled {
199+
return fmt.Errorf("endpoint requires at least one of tracing or metrics to be enabled")
200+
}
201+
return nil
202+
}
203+
184204
// validateSensitiveHeaders validates sensitive header entries and checks for overlap with plaintext headers.
185205
func (r *MCPTelemetryConfig) validateSensitiveHeaders() error {
186206
if r.Spec.OpenTelemetry == nil {

cmd/thv-operator/controllers/mcptelemetryconfig_controller_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,19 @@ func TestMCPTelemetryConfig_Validate(t *testing.T) {
498498
},
499499
expectError: true,
500500
},
501+
{
502+
name: "invalid endpoint without tracing or metrics",
503+
config: &mcpv1alpha1.MCPTelemetryConfig{
504+
Spec: mcpv1alpha1.MCPTelemetryConfigSpec{
505+
OpenTelemetry: &mcpv1alpha1.MCPTelemetryOTelConfig{
506+
Enabled: true,
507+
Endpoint: "otel-collector:4317",
508+
// No Tracing or Metrics configured
509+
},
510+
},
511+
},
512+
expectError: true,
513+
},
501514
{
502515
name: "invalid empty secret ref name",
503516
config: &mcpv1alpha1.MCPTelemetryConfig{

deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcptelemetryconfigs.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,6 @@ spec:
163163
- message: a header name cannot appear in both headers and sensitiveHeaders
164164
rule: '!has(self.headers) || !has(self.sensitiveHeaders) || self.sensitiveHeaders.all(sh,
165165
!(sh.name in self.headers))'
166-
- message: endpoint requires at least one of tracing or metrics to be enabled
167-
rule: '!has(self.endpoint) || size(self.endpoint) == 0 || (has(self.tracing)
168-
&& self.tracing.enabled) || (has(self.metrics) && self.metrics.enabled)'
169166
prometheus:
170167
description: Prometheus defines Prometheus-specific configuration
171168
properties:

deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcptelemetryconfigs.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,6 @@ spec:
166166
- message: a header name cannot appear in both headers and sensitiveHeaders
167167
rule: '!has(self.headers) || !has(self.sensitiveHeaders) || self.sensitiveHeaders.all(sh,
168168
!(sh.name in self.headers))'
169-
- message: endpoint requires at least one of tracing or metrics to be enabled
170-
rule: '!has(self.endpoint) || size(self.endpoint) == 0 || (has(self.tracing)
171-
&& self.tracing.enabled) || (has(self.metrics) && self.metrics.enabled)'
172169
prometheus:
173170
description: Prometheus defines Prometheus-specific configuration
174171
properties:

0 commit comments

Comments
 (0)