fix: validate tracing provider completeness after EnvoyProxy merge instead of at admission

api/v1alpha1/envoyproxy_tracing_types.go

@@ -34,19 +34,39 @@ const (
 	TracingProviderTypeDatadog       TracingProviderType = "Datadog"
 )
 
+const (
+	// DefaultTracingProviderType is the provider type applied during translation
+	// when TracingProvider.Type is unset.
+	DefaultTracingProviderType = TracingProviderTypeOpenTelemetry
+	// DefaultTracingProviderPort is the provider port applied during translation
+	// when TracingProvider.Port is unset.
+	DefaultTracingProviderPort int32 = 4317
+)
+
 // TracingProvider defines the tracing provider configuration.
 //
-// +kubebuilder:validation:XValidation:message="host or backendRefs needs to be set",rule="has(self.host) || self.backendRefs.size() > 0"
+// A provider is only required to set host or backendRefs after the
+// GatewayClass-level and Gateway-level EnvoyProxy configs are merged
+// (see EnvoyProxySpec.MergeType), so completeness is checked during
+// translation instead of by a CEL rule here. A provider that is still
+// incomplete after the merge turns tracing off for that Gateway.
+//
 // +kubebuilder:validation:XValidation:message="BackendRefs must be used, backendRef is not supported.",rule="!has(self.backendRef)"
 // +kubebuilder:validation:XValidation:message="BackendRefs only support Service and Backend kind.",rule="has(self.backendRefs) ? self.backendRefs.all(f, f.kind == 'Service' || f.kind == 'Backend') : true"
 // +kubebuilder:validation:XValidation:message="BackendRefs only support Core and gateway.envoyproxy.io group.",rule="has(self.backendRefs) ? (self.backendRefs.all(f, f.group == \"\" || f.group == 'gateway.envoyproxy.io')) : true"
-// +kubebuilder:validation:XValidation:message="openTelemetry can only be used with type OpenTelemetry",rule="has(self.openTelemetry) ? self.type == 'OpenTelemetry' : true"
+// +kubebuilder:validation:XValidation:message="openTelemetry can only be used with type OpenTelemetry",rule="has(self.openTelemetry) ? (!has(self.type) || self.type == 'OpenTelemetry') : true"
 type TracingProvider struct {
 	BackendCluster `json:",inline"`
 	// Type defines the tracing provider type.
+	//
+	// Defaults to OpenTelemetry. The default is applied during translation rather
+	// than by admission, so that a Gateway-level EnvoyProxy overriding only part of
+	// the provider does not replace the type inherited from the GatewayClass level
+	// (see EnvoyProxySpec.MergeType).
+	//
 	// +kubebuilder:validation:Enum=OpenTelemetry;Zipkin;Datadog
-	// +kubebuilder:default=OpenTelemetry
-	Type TracingProviderType `json:"type"`
+	// +optional
+	Type *TracingProviderType `json:"type,omitempty"`
 	// Host define the provider service hostname.
 	//
 	// Deprecated: Use BackendRefs instead.
@@ -55,12 +75,14 @@ type TracingProvider struct {
 	Host *string `json:"host,omitempty"`
 	// Port defines the port the provider service is exposed on.
 	//
+	// Defaults to 4317. The default is applied during translation rather than by
+	// admission, for the same reason as Type.
+	//
 	// Deprecated: Use BackendRefs instead.
 	//
 	// +optional
 	// +kubebuilder:validation:Minimum=0
-	// +kubebuilder:default=4317
-	Port int32 `json:"port,omitempty"`
+	Port *int32 `json:"port,omitempty"`
 	// ServiceName defines the service name to use in tracing configuration.
 	// If not set, Envoy Gateway will use a default service name set as
 	// "name.namespace" (e.g., "my-gateway.default").

charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml

@@ -18921,10 +18921,12 @@ spec:
                                     : true'
                             type: object
                           port:
-                            default: 4317
                             description: |-
                               Port defines the port the provider service is exposed on.
 
+                              Defaults to 4317. The default is applied during translation rather than by
+                              admission, for the same reason as Type.
+
                               Deprecated: Use BackendRefs instead.
                             format: int32
                             minimum: 0
@@ -18942,8 +18944,13 @@ spec:
                             - message: serviceName cannot be empty if provided
                               rule: self != ""
                           type:
-                            default: OpenTelemetry
-                            description: Type defines the tracing provider type.
+                            description: |-
+                              Type defines the tracing provider type.
+
+                              Defaults to OpenTelemetry. The default is applied during translation rather
+                              than by admission, so that a Gateway-level EnvoyProxy overriding only part of
+                              the provider does not replace the type inherited from the GatewayClass level
+                              (see EnvoyProxySpec.MergeType).
                             enum:
                             - OpenTelemetry
                             - Zipkin
@@ -18965,12 +18972,8 @@ spec:
                                   id will be used.
                                 type: boolean
                             type: object
-                        required:
-                        - type
                         type: object
                         x-kubernetes-validations:
-                        - message: host or backendRefs needs to be set
-                          rule: has(self.host) || self.backendRefs.size() > 0
                         - message: BackendRefs must be used, backendRef is not supported.
                           rule: '!has(self.backendRef)'
                         - message: BackendRefs only support Service and Backend kind.
@@ -18982,8 +18985,8 @@ spec:
                             f.group == "" || f.group == ''gateway.envoyproxy.io''))
                             : true'
                         - message: openTelemetry can only be used with type OpenTelemetry
-                          rule: 'has(self.openTelemetry) ? self.type == ''OpenTelemetry''
-                            : true'
+                          rule: 'has(self.openTelemetry) ? (!has(self.type) || self.type
+                            == ''OpenTelemetry'') : true'
                       samplingFraction:
                         description: |-
                           SamplingFraction represents the fraction of requests that should be

charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml

@@ -18920,10 +18920,12 @@ spec:
                                     : true'
                             type: object
                           port:
-                            default: 4317
                             description: |-
                               Port defines the port the provider service is exposed on.
 
+                              Defaults to 4317. The default is applied during translation rather than by
+                              admission, for the same reason as Type.
+
                               Deprecated: Use BackendRefs instead.
                             format: int32
                             minimum: 0
@@ -18941,8 +18943,13 @@ spec:
                             - message: serviceName cannot be empty if provided
                               rule: self != ""
                           type:
-                            default: OpenTelemetry
-                            description: Type defines the tracing provider type.
+                            description: |-
+                              Type defines the tracing provider type.
+
+                              Defaults to OpenTelemetry. The default is applied during translation rather
+                              than by admission, so that a Gateway-level EnvoyProxy overriding only part of
+                              the provider does not replace the type inherited from the GatewayClass level
+                              (see EnvoyProxySpec.MergeType).
                             enum:
                             - OpenTelemetry
                             - Zipkin
@@ -18964,12 +18971,8 @@ spec:
                                   id will be used.
                                 type: boolean
                             type: object
-                        required:
-                        - type
                         type: object
                         x-kubernetes-validations:
-                        - message: host or backendRefs needs to be set
-                          rule: has(self.host) || self.backendRefs.size() > 0
                         - message: BackendRefs must be used, backendRef is not supported.
                           rule: '!has(self.backendRef)'
                         - message: BackendRefs only support Service and Backend kind.
@@ -18981,8 +18984,8 @@ spec:
                             f.group == "" || f.group == ''gateway.envoyproxy.io''))
                             : true'
                         - message: openTelemetry can only be used with type OpenTelemetry
-                          rule: 'has(self.openTelemetry) ? self.type == ''OpenTelemetry''
-                            : true'
+                          rule: 'has(self.openTelemetry) ? (!has(self.type) || self.type
+                            == ''OpenTelemetry'') : true'
                       samplingFraction:
                         description: |-
                           SamplingFraction represents the fraction of requests that should be

internal/gatewayapi/envoyproxy_merge.go

@@ -8,6 +8,8 @@ package gatewayapi
 import (
 	"fmt"
 
+	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+
 	egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
 	"github.com/envoyproxy/gateway/internal/utils"
 )
@@ -90,9 +92,81 @@ func mergeEnvoyProxies(
 		return override, nil
 	}
 
-	merged, err := utils.Merge(*base, *override, mergeType)
+	// The merged object carries the override's metadata, so a backendRef
+	// inherited from base without a namespace would later be resolved in the
+	// override's namespace. Pin it to the namespace it was written in first.
+	merged, err := utils.Merge(*qualifyTelemetryBackendRefs(base), *override, mergeType)
 	if err != nil {
 		return nil, err
 	}
 	return &merged, nil
 }
+
+// qualifyTelemetryBackendRefs returns ep with every telemetry backendRef that
+// omits a namespace set to ep's own namespace, which is where the ref resolves
+// when ep is used on its own. ep itself is not modified: a copy is returned
+// when there is anything to set. An EnvoyProxy without a namespace, such as
+// the one built from the EnvoyGateway default spec, is returned unchanged.
+func qualifyTelemetryBackendRefs(ep *egv1a1.EnvoyProxy) *egv1a1.EnvoyProxy {
+	if ep.Namespace == "" || !hasUnqualifiedTelemetryBackendRef(ep) {
+		return ep
+	}
+
+	ep = ep.DeepCopy()
+	ns := gwapiv1.Namespace(ep.Namespace)
+	for _, cluster := range telemetryBackendClusters(ep) {
+		for i := range cluster.BackendRefs {
+			if cluster.BackendRefs[i].Namespace == nil {
+				cluster.BackendRefs[i].Namespace = new(ns)
+			}
+		}
+	}
+	return ep
+}
+
+func hasUnqualifiedTelemetryBackendRef(ep *egv1a1.EnvoyProxy) bool {
+	for _, cluster := range telemetryBackendClusters(ep) {
+		for i := range cluster.BackendRefs {
+			if cluster.BackendRefs[i].Namespace == nil {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// telemetryBackendClusters returns every BackendCluster under the telemetry
+// settings of ep: the tracing provider, the ALS and OpenTelemetry access log
+// sinks and the OpenTelemetry metric sinks. The returned pointers alias ep.
+func telemetryBackendClusters(ep *egv1a1.EnvoyProxy) []*egv1a1.BackendCluster {
+	telemetry := ep.Spec.Telemetry
+	if telemetry == nil {
+		return nil
+	}
+
+	var clusters []*egv1a1.BackendCluster
+	if telemetry.Tracing != nil {
+		clusters = append(clusters, &telemetry.Tracing.Provider.BackendCluster)
+	}
+	if telemetry.AccessLog != nil {
+		for i := range telemetry.AccessLog.Settings {
+			for j := range telemetry.AccessLog.Settings[i].Sinks {
+				sink := &telemetry.AccessLog.Settings[i].Sinks[j]
+				if sink.ALS != nil {
+					clusters = append(clusters, &sink.ALS.BackendCluster)
+				}
+				if sink.OpenTelemetry != nil {
+					clusters = append(clusters, &sink.OpenTelemetry.BackendCluster)
+				}
+			}
+		}
+	}
+	if telemetry.Metrics != nil {
+		for i := range telemetry.Metrics.Sinks {
+			if sink := telemetry.Metrics.Sinks[i].OpenTelemetry; sink != nil {
+				clusters = append(clusters, &sink.BackendCluster)
+			}
+		}
+	}
+	return clusters
+}