From 5969bef1fab1df11f6062d4637b747ea7e6b9838 Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Sun, 2 Aug 2026 20:04:25 +0300 Subject: [PATCH 1/7] fix: validate tracing provider completeness after EnvoyProxy merge Drop the 'host or backendRefs needs to be set' CEL rule from TracingProvider so a per-Gateway EnvoyProxy can override a single field (e.g. serviceName) and inherit the rest via mergeType. The completeness check now runs in processTracing after the GatewayClass-level and Gateway-level configs are merged, surfacing an InvalidParameters Gateway condition instead of an admission error. Fixes #9527 Co-Authored-By: Claude Fable 5 Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- api/v1alpha1/envoyproxy_tracing_types.go | 6 ++++- .../gateway.envoyproxy.io_envoyproxies.yaml | 2 -- .../gateway.envoyproxy.io_envoyproxies.yaml | 2 -- internal/gatewayapi/listener.go | 10 ++++--- internal/gatewayapi/listener_test.go | 26 +++++++++++++++++++ .../bug_fixes/9527-tracing-cel-merge.md | 1 + test/cel-validation/envoyproxy_test.go | 9 ++++--- 7 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 release-notes/current/bug_fixes/9527-tracing-cel-merge.md diff --git a/api/v1alpha1/envoyproxy_tracing_types.go b/api/v1alpha1/envoyproxy_tracing_types.go index 11eaeb686d9..7db261f0b60 100644 --- a/api/v1alpha1/envoyproxy_tracing_types.go +++ b/api/v1alpha1/envoyproxy_tracing_types.go @@ -36,7 +36,11 @@ const ( // 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 validated during +// translation instead of by a CEL rule here. +// // +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" diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml index a9630d033a6..09646ccaba1 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -18969,8 +18969,6 @@ spec: - 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. diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index 76159da3d20..4d80f188e80 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -18968,8 +18968,6 @@ spec: - 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. diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go index 3d8376553ad..0e17b7d9f44 100644 --- a/internal/gatewayapi/listener.go +++ b/internal/gatewayapi/listener.go @@ -1080,11 +1080,13 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En // fallback to host and port // TODO: remove support for Host/Port in v1.2 if len(ds) == 0 { - var host string - var port uint32 - if tracing.Provider.Host != nil { - host, port = *tracing.Provider.Host, uint32(tracing.Provider.Port) + // Validated here instead of by a CRD CEL rule so that a partial provider + // (e.g. only serviceName) can be completed by the GatewayClass-level and + // Gateway-level EnvoyProxy merge before the check runs. + if tracing.Provider.Host == nil { + return nil, fmt.Errorf("host or backendRefs needs to be set on the tracing provider after merging EnvoyProxy configs") } + host, port := *tracing.Provider.Host, uint32(tracing.Provider.Port) ds = destinationSettingFromHostAndPort(settingName, host, port) authority = host } diff --git a/internal/gatewayapi/listener_test.go b/internal/gatewayapi/listener_test.go index 0ca43b8b362..cd1c575d9a7 100644 --- a/internal/gatewayapi/listener_test.go +++ b/internal/gatewayapi/listener_test.go @@ -1088,6 +1088,32 @@ func TestProcessTracingServiceName(t *testing.T) { mergeGateways: true, expectedServiceName: "test-gateway-class", // Should use gateway class name when merging }, + { + name: "tracing provider without backendRefs or host", + gateway: &gwapiv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test-namespace", + }, + }, + envoyProxy: &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proxy", + Namespace: "test-namespace", + }, + Spec: egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ + Type: egv1a1.TracingProviderTypeOpenTelemetry, + ServiceName: new("only-name-overridden"), + }, + }, + }, + }, + }, + expectError: true, + }, } for _, tc := range cases { diff --git a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md new file mode 100644 index 00000000000..bcce3b24d4c --- /dev/null +++ b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md @@ -0,0 +1 @@ +Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest: the host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, surfacing an `InvalidParameters` Gateway condition instead of blocking admission. diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index 406b0bb518e..cf8667e0d7c 100644 --- a/test/cel-validation/envoyproxy_test.go +++ b/test/cel-validation/envoyproxy_test.go @@ -1311,19 +1311,22 @@ func TestEnvoyProxyProvider(t *testing.T) { }, }, { - desc: "tracing-empty-backend", + // A partial provider must be accepted at admission so it can be + // completed by the GatewayClass/Gateway EnvoyProxy merge; completeness + // is validated during translation instead. + desc: "tracing-partial-provider-for-merge", mutate: func(envoy *egv1a1.EnvoyProxy) { envoy.Spec = egv1a1.EnvoyProxySpec{ Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: egv1a1.TracingProviderTypeOpenTelemetry, + ServiceName: new("my-override"), }, }, }, } }, - wantErrors: []string{"host or backendRefs needs to be set"}, }, { desc: "valid-tracing-service-name", From edb169c7c8920326077e4fe56c6207d4704d8d6d Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Sun, 2 Aug 2026 20:20:54 +0300 Subject: [PATCH 2/7] docs: regenerate API reference for TracingProvider comment change Co-Authored-By: Claude Fable 5 Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- site/content/en/latest/api/extension_types.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 5d5f90e9259..f1a54f47747 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -6687,6 +6687,11 @@ _Appears in:_ TracingProvider defines the tracing provider configuration. +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 validated during +translation instead of by a CEL rule here. + _Appears in:_ - [ProxyTracing](#proxytracing) From 03392a3fa3fe9575aa3d955706f0cd8e0367fc05 Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Mon, 3 Aug 2026 11:16:52 +0300 Subject: [PATCH 3/7] Regenerate gateway-crds-helm template snapshots Co-Authored-By: Claude Fable 5 Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- test/helm/gateway-crds-helm/all.out.yaml | 2 -- test/helm/gateway-crds-helm/e2e.out.yaml | 2 -- test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml | 2 -- 3 files changed, 6 deletions(-) diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 197aec2547f..dec5aca77ac 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -53063,8 +53063,6 @@ spec: - 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. diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 32f54c37669..1aac8c5acb7 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -29001,8 +29001,6 @@ spec: - 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. diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index db1da4ef9c1..92b844a9f40 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -29001,8 +29001,6 @@ spec: - 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. From a784c401f039388a3c046bde68e52ddaead33af2 Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Thu, 6 Aug 2026 11:33:01 +0300 Subject: [PATCH 4/7] Skip tracing with a log message instead of failing the Gateway An incomplete tracing provider now turns tracing off for that Gateway instead of setting Accepted=False, so an observability misconfiguration does not stop the proxy from being provisioned. Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- api/v1alpha1/envoyproxy_tracing_types.go | 5 +++-- internal/gatewayapi/listener.go | 11 ++++++++--- internal/gatewayapi/listener_test.go | 11 +++-------- .../current/bug_fixes/9527-tracing-cel-merge.md | 2 +- site/content/en/latest/api/extension_types.md | 5 +++-- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/api/v1alpha1/envoyproxy_tracing_types.go b/api/v1alpha1/envoyproxy_tracing_types.go index 7db261f0b60..f4fcfb15e53 100644 --- a/api/v1alpha1/envoyproxy_tracing_types.go +++ b/api/v1alpha1/envoyproxy_tracing_types.go @@ -38,8 +38,9 @@ const ( // // 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 validated during -// translation instead of by a CEL rule here. +// (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" diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go index 0e17b7d9f44..728202def74 100644 --- a/internal/gatewayapi/listener.go +++ b/internal/gatewayapi/listener.go @@ -1080,11 +1080,16 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En // fallback to host and port // TODO: remove support for Host/Port in v1.2 if len(ds) == 0 { - // Validated here instead of by a CRD CEL rule so that a partial provider + // Checked here instead of by a CRD CEL rule so that a partial provider // (e.g. only serviceName) can be completed by the GatewayClass-level and - // Gateway-level EnvoyProxy merge before the check runs. + // Gateway-level EnvoyProxy merge before the check runs. An incomplete + // provider only turns tracing off, it does not stop the Gateway from + // being provisioned. if tracing.Provider.Host == nil { - return nil, fmt.Errorf("host or backendRefs needs to be set on the tracing provider after merging EnvoyProxy configs") + t.Logger.Info("Disabling tracing because the merged tracing provider sets neither host nor backendRefs", + "gateway", utils.NamespacedName(gwCtx.Gateway).String(), + "envoyProxy", utils.NamespacedName(envoyproxy).String()) + return nil, nil } host, port := *tracing.Provider.Host, uint32(tracing.Provider.Port) ds = destinationSettingFromHostAndPort(settingName, host, port) diff --git a/internal/gatewayapi/listener_test.go b/internal/gatewayapi/listener_test.go index cd1c575d9a7..1392541c5ce 100644 --- a/internal/gatewayapi/listener_test.go +++ b/internal/gatewayapi/listener_test.go @@ -916,7 +916,6 @@ func TestProcessTracingServiceName(t *testing.T) { envoyProxy *egv1a1.EnvoyProxy mergeGateways bool expectedServiceName string - expectError bool }{ { name: "no tracing configuration", @@ -1089,7 +1088,7 @@ func TestProcessTracingServiceName(t *testing.T) { expectedServiceName: "test-gateway-class", // Should use gateway class name when merging }, { - name: "tracing provider without backendRefs or host", + name: "tracing provider without backendRefs or host disables tracing", gateway: &gwapiv1.Gateway{ ObjectMeta: metav1.ObjectMeta{ Name: "test-gateway", @@ -1112,7 +1111,8 @@ func TestProcessTracingServiceName(t *testing.T) { }, }, }, - expectError: true, + // An empty expectedServiceName asserts that no tracing config is built. + expectedServiceName: "", }, } @@ -1177,11 +1177,6 @@ func TestProcessTracingServiceName(t *testing.T) { Gateway: tc.gateway, }, tc.envoyProxy, tc.mergeGateways, resources) - if tc.expectError { - assert.Error(t, err) - return - } - require.NoError(t, err) if tc.expectedServiceName == "" { diff --git a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md index bcce3b24d4c..680cd1374e2 100644 --- a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md +++ b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md @@ -1 +1 @@ -Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest: the host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, surfacing an `InvalidParameters` Gateway condition instead of blocking admission. +Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest. The host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, and a provider that is still incomplete turns tracing off with a log message instead of blocking admission or the Gateway. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index f1a54f47747..5799dba0fba 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -6689,8 +6689,9 @@ TracingProvider defines the tracing provider configuration. 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 validated during -translation instead of by a CEL rule here. +(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. _Appears in:_ - [ProxyTracing](#proxytracing) From b64b761220a6e839adc2d0482b4a6da190b7aa73 Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Fri, 21 Aug 2026 14:24:51 +0300 Subject: [PATCH 5/7] fix: stop defaulting the tracing provider type and port at admission A Gateway-level EnvoyProxy that overrides only part of the tracing provider still had `type` and `port` materialized by admission, because both carried a kubebuilder default. Once those values are stored they are indistinguishable from an explicit override, so StrategicMerge and JSONMerge replaced the type and port inherited from the GatewayClass level: a Datadog provider became OpenTelemetry, and an OpenTelemetry provider on a custom port such as 4318 was redirected to 4317. Both fields are now optional, and the defaults are applied during translation instead, where the merge has already happened. The defaults themselves are unchanged (OpenTelemetry and 4317) and live next to the type as DefaultTracingProviderType and DefaultTracingProviderPort. The openTelemetry CEL rule is relaxed to tolerate an absent type, and the xDS translator resolves the default again so an IR built without an explicit provider type keeps working. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- api/v1alpha1/envoyproxy_tracing_types.go | 27 ++++-- api/v1alpha1/zz_generated.deepcopy.go | 10 +++ .../gateway.envoyproxy.io_envoyproxies.yaml | 19 ++-- .../gateway.envoyproxy.io_envoyproxies.yaml | 19 ++-- internal/gatewayapi/listener.go | 31 +++++-- internal/gatewayapi/listener_test.go | 87 +++++++++++++++++-- ...envoyproxy-otel-backend-custom-ca.out.yaml | 1 + ...l-backend-tls-per-resource-secret.out.yaml | 1 + .../envoyproxy-otel-backend-tls.out.yaml | 1 + .../envoyproxy-otel-backendtlspolicy.out.yaml | 1 + .../envoyproxy-tracing-backend-uds.out.yaml | 1 + ...proxy-tracing-backend-with-hc-log.out.yaml | 1 + .../envoyproxy-tracing-backend.out.yaml | 1 + .../envoyproxy-tracing-sampler.out.yaml | 1 + .../provider/kubernetes/predicates_test.go | 3 +- internal/xds/translator/tracing.go | 8 +- internal/xds/translator/tracing_test.go | 5 +- .../bug_fixes/9527-tracing-cel-merge.md | 2 +- site/content/en/latest/api/extension_types.md | 4 +- test/cel-validation/envoyproxy_test.go | 31 +++---- test/helm/gateway-crds-helm/all.out.yaml | 19 ++-- test/helm/gateway-crds-helm/e2e.out.yaml | 19 ++-- .../envoy-gateway-crds.out.yaml | 19 ++-- 23 files changed, 234 insertions(+), 77 deletions(-) diff --git a/api/v1alpha1/envoyproxy_tracing_types.go b/api/v1alpha1/envoyproxy_tracing_types.go index f4fcfb15e53..0dfa8bd36ab 100644 --- a/api/v1alpha1/envoyproxy_tracing_types.go +++ b/api/v1alpha1/envoyproxy_tracing_types.go @@ -34,6 +34,15 @@ 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. // // A provider is only required to set host or backendRefs after the @@ -45,13 +54,19 @@ const ( // +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. @@ -60,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"). diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2a4014dfa8a..54a64efd178 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -8981,11 +8981,21 @@ func (in *Tracing) DeepCopy() *Tracing { func (in *TracingProvider) DeepCopyInto(out *TracingProvider) { *out = *in in.BackendCluster.DeepCopyInto(&out.BackendCluster) + if in.Type != nil { + in, out := &in.Type, &out.Type + *out = new(TracingProviderType) + **out = **in + } if in.Host != nil { in, out := &in.Host, &out.Host *out = new(string) **out = **in } + if in.Port != nil { + in, out := &in.Port, &out.Port + *out = new(int32) + **out = **in + } if in.ServiceName != nil { in, out := &in.ServiceName, &out.ServiceName *out = new(string) diff --git a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml index 09646ccaba1..9d7830b3086 100644 --- a/charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/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,8 +18972,6 @@ spec: id will be used. type: boolean type: object - required: - - type type: object x-kubernetes-validations: - message: BackendRefs must be used, backendRef is not supported. @@ -18980,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 diff --git a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index 4d80f188e80..a59fc47abbe 100644 --- a/charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/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,8 +18971,6 @@ spec: id will be used. type: boolean type: object - required: - - type type: object x-kubernetes-validations: - message: BackendRefs must be used, backendRef is not supported. @@ -18979,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 diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go index 728202def74..d29601acc4f 100644 --- a/internal/gatewayapi/listener.go +++ b/internal/gatewayapi/listener.go @@ -1061,14 +1061,27 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En } tracing := envoyproxy.Spec.Telemetry.Tracing + // Type and Port are defaulted here rather than by admission so that a + // Gateway-level EnvoyProxy overriding only part of the provider does not + // replace the values inherited from the GatewayClass level: a defaulted field + // is indistinguishable from an explicit one once the merge patch is built. + // The copy keeps the EnvoyProxy resource itself untouched. + provider := tracing.Provider + if provider.Type == nil { + provider.Type = ptr.To(egv1a1.DefaultTracingProviderType) + } + if provider.Port == nil { + provider.Port = ptr.To(egv1a1.DefaultTracingProviderPort) + } + // TODO: rename this, so that we can share backend with accesslog? destName := "tracing" settingName := irDestinationSettingName(destName, -1) - ds, traffic, err := t.processBackendRefsForTelemetry(settingName, tracing.Provider.BackendCluster, envoyproxy.Namespace, resources, envoyproxy, gwCtx) + ds, traffic, err := t.processBackendRefsForTelemetry(settingName, provider.BackendCluster, envoyproxy.Namespace, resources, envoyproxy, gwCtx) if err != nil { return nil, err } - if tracing.Provider.Type == egv1a1.TracingProviderTypeOpenTelemetry { + if *provider.Type == egv1a1.TracingProviderTypeOpenTelemetry { // TODO: update when OTLP/HTTP is completely supported (logs, traces, metrics) for _, d := range ds { d.Protocol = ir.GRPC @@ -1085,13 +1098,13 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En // Gateway-level EnvoyProxy merge before the check runs. An incomplete // provider only turns tracing off, it does not stop the Gateway from // being provisioned. - if tracing.Provider.Host == nil { + if provider.Host == nil { t.Logger.Info("Disabling tracing because the merged tracing provider sets neither host nor backendRefs", "gateway", utils.NamespacedName(gwCtx.Gateway).String(), "envoyProxy", utils.NamespacedName(envoyproxy).String()) return nil, nil } - host, port := *tracing.Provider.Host, uint32(tracing.Provider.Port) + host, port := *provider.Host, uint32(*provider.Port) ds = destinationSettingFromHostAndPort(settingName, host, port) authority = host } @@ -1103,8 +1116,8 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En } // Use configured service name if provided - if tracing.Provider.ServiceName != nil { - serviceName = *tracing.Provider.ServiceName + if provider.ServiceName != nil { + serviceName = *provider.ServiceName } return &ir.Tracing{ @@ -1115,15 +1128,15 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En OverallSamplingRate: proxySamplingFractionPtr(tracing.OverallSamplingFraction), CustomTags: ir.CustomTagMapToSlice(tracing.CustomTags), Tags: ir.MapToSlice(tracing.Tags), - ResourceAttributes: ir.MapToSlice(getOpenTelemetryTracingResourceAttributes(&tracing.Provider)), + ResourceAttributes: ir.MapToSlice(getOpenTelemetryTracingResourceAttributes(&provider)), Destination: ir.RouteDestination{ Name: destName, Settings: ds, Metadata: buildResourceMetadata(envoyproxy, nil), }, - Provider: tracing.Provider, + Provider: provider, Traffic: traffic, - Headers: getOpenTelemetryTracingHeaders(&tracing.Provider), + Headers: getOpenTelemetryTracingHeaders(&provider), SpanName: tracing.SpanName, }, nil } diff --git a/internal/gatewayapi/listener_test.go b/internal/gatewayapi/listener_test.go index 1392541c5ce..e78e7302e5b 100644 --- a/internal/gatewayapi/listener_test.go +++ b/internal/gatewayapi/listener_test.go @@ -950,7 +950,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -986,7 +986,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1026,7 +1026,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1067,7 +1067,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1104,7 +1104,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), ServiceName: new("only-name-overridden"), }, }, @@ -1190,6 +1190,83 @@ func TestProcessTracingServiceName(t *testing.T) { } } +// TestProcessTracingProviderDefaults guards the defaults that used to be applied +// by admission. Applying them there made a partial Gateway-level override +// indistinguishable from an explicit one, so an inherited type or port was +// silently replaced by the defaulted value during the EnvoyProxy merge. +func TestProcessTracingProviderDefaults(t *testing.T) { + cases := []struct { + name string + provider egv1a1.TracingProvider + expectedType egv1a1.TracingProviderType + expectedPort uint32 + }{ + { + name: "unset type and port fall back to the documented defaults", + provider: egv1a1.TracingProvider{ + Host: ptr.To("otel-collector.monitoring.svc.cluster.local"), + }, + expectedType: egv1a1.TracingProviderTypeOpenTelemetry, + expectedPort: 4317, + }, + { + name: "type and port inherited from the GatewayClass level are kept", + provider: egv1a1.TracingProvider{ + Host: ptr.To("datadog-agent.monitoring.svc.cluster.local"), + Type: ptr.To(egv1a1.TracingProviderTypeDatadog), + Port: ptr.To(int32(8126)), + }, + expectedType: egv1a1.TracingProviderTypeDatadog, + expectedPort: 8126, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + translator := &Translator{} + envoyProxy := &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-proxy", + Namespace: "test-namespace", + }, + Spec: egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: tc.provider, + }, + }, + }, + } + + result, err := translator.processTracing(&GatewayContext{ + Gateway: &gwapiv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "test-namespace", + }, + }, + }, envoyProxy, false, &resource.Resources{}) + require.NoError(t, err) + require.NotNil(t, result) + + require.NotNil(t, result.Provider.Type) + assert.Equal(t, tc.expectedType, *result.Provider.Type) + require.NotNil(t, result.Provider.Port) + assert.Equal(t, tc.expectedPort, uint32(*result.Provider.Port)) + + // The host/port fallback destination has to agree with the provider port. + require.Len(t, result.Destination.Settings, 1) + require.Len(t, result.Destination.Settings[0].Endpoints, 1) + assert.Equal(t, tc.expectedPort, result.Destination.Settings[0].Endpoints[0].Port) + + // Defaulting works on a copy, so the EnvoyProxy resource keeps whatever + // the user actually set. + assert.Equal(t, tc.provider.Type, envoyProxy.Spec.Telemetry.Tracing.Provider.Type) + assert.Equal(t, tc.provider.Port, envoyProxy.Spec.Telemetry.Tracing.Provider.Port) + }) + } +} + func TestProcessAccessLog(t *testing.T) { tests := []struct { name string diff --git a/internal/gatewayapi/testdata/envoyproxy-otel-backend-custom-ca.out.yaml b/internal/gatewayapi/testdata/envoyproxy-otel-backend-custom-ca.out.yaml index ff5b757e2c6..7926e6d61cf 100644 --- a/internal/gatewayapi/testdata/envoyproxy-otel-backend-custom-ca.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-otel-backend-custom-ca.out.yaml @@ -264,6 +264,7 @@ xdsIR: kind: Backend name: otel-collector namespace: envoy-gateway + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls-per-resource-secret.out.yaml b/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls-per-resource-secret.out.yaml index 6527350a901..c555a091b13 100644 --- a/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls-per-resource-secret.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls-per-resource-secret.out.yaml @@ -261,6 +261,7 @@ xdsIR: kind: Backend name: otel-collector namespace: envoy-gateway + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls.out.yaml b/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls.out.yaml index 9c95ca2bf3d..2c12f551a83 100644 --- a/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-otel-backend-tls.out.yaml @@ -261,6 +261,7 @@ xdsIR: kind: Backend name: otel-collector namespace: envoy-gateway + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-otel-backendtlspolicy.out.yaml b/internal/gatewayapi/testdata/envoyproxy-otel-backendtlspolicy.out.yaml index e15df74b14e..f7018a15b09 100644 --- a/internal/gatewayapi/testdata/envoyproxy-otel-backendtlspolicy.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-otel-backendtlspolicy.out.yaml @@ -292,6 +292,7 @@ xdsIR: kind: Backend name: otel-collector namespace: envoy-gateway + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-backend-uds.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-backend-uds.out.yaml index c84261d0697..c5e6bcce11c 100644 --- a/internal/gatewayapi/testdata/envoyproxy-tracing-backend-uds.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-tracing-backend-uds.out.yaml @@ -269,6 +269,7 @@ xdsIR: timeout: tcp: connectTimeout: 15s + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-backend-with-hc-log.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-backend-with-hc-log.out.yaml index db36129747c..d96f1b97ea0 100644 --- a/internal/gatewayapi/testdata/envoyproxy-tracing-backend-with-hc-log.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-tracing-backend-with-hc-log.out.yaml @@ -245,6 +245,7 @@ xdsIR: timeout: 500ms type: HTTP unhealthyThreshold: 3 + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-backend.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-backend.out.yaml index 66cc26c5040..f9b09b665f1 100644 --- a/internal/gatewayapi/testdata/envoyproxy-tracing-backend.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-tracing-backend.out.yaml @@ -248,6 +248,7 @@ xdsIR: timeout: tcp: connectTimeout: 15s + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-sampler.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-sampler.out.yaml index 2393104ece4..9c1f5c2ff41 100644 --- a/internal/gatewayapi/testdata/envoyproxy-tracing-sampler.out.yaml +++ b/internal/gatewayapi/testdata/envoyproxy-tracing-sampler.out.yaml @@ -163,6 +163,7 @@ xdsIR: samplingPercentage: numerator: 50 type: ParentBasedTraceIdRatio + port: 4317 type: OpenTelemetry samplingRate: 100 serviceName: gateway-1.envoy-gateway diff --git a/internal/provider/kubernetes/predicates_test.go b/internal/provider/kubernetes/predicates_test.go index f1f6ec1e082..b1015aa1a3a 100644 --- a/internal/provider/kubernetes/predicates_test.go +++ b/internal/provider/kubernetes/predicates_test.go @@ -17,6 +17,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -1380,7 +1381,7 @@ func TestValidateServiceForReconcile(t *testing.T) { }, Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { diff --git a/internal/xds/translator/tracing.go b/internal/xds/translator/tracing.go index 203c82bf781..0269439b9eb 100644 --- a/internal/xds/translator/tracing.go +++ b/internal/xds/translator/tracing.go @@ -44,7 +44,11 @@ func buildHCMTracing(tracing *ir.Tracing) (*hcm.HttpConnectionManager_Tracing, e var providerName string var providerConfig typConfigGen - switch tracing.Provider.Type { + // Defaulted here as well as in the Gateway API translation, so that an IR built + // without an explicit provider type still resolves to the documented default. + providerType := ptr.Deref(tracing.Provider.Type, egv1a1.DefaultTracingProviderType) + + switch providerType { case egv1a1.TracingProviderTypeDatadog: providerName = envoyDatadog @@ -99,7 +103,7 @@ func buildHCMTracing(tracing *ir.Tracing) (*hcm.HttpConnectionManager_Tracing, e return proto.ToAnyWithValidation(config) } default: - return nil, fmt.Errorf("unknown tracing provider type: %s", tracing.Provider.Type) + return nil, fmt.Errorf("unknown tracing provider type: %s", providerType) } ocAny, err := providerConfig() diff --git a/internal/xds/translator/tracing_test.go b/internal/xds/translator/tracing_test.go index 932a8d0263c..e8d84aabac5 100644 --- a/internal/xds/translator/tracing_test.go +++ b/internal/xds/translator/tracing_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -34,7 +35,7 @@ func TestBuildHCMTracingSampling(t *testing.T) { Name: "tracing", }, Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), }, }, expectedRandomSampling: 10.0, @@ -50,7 +51,7 @@ func TestBuildHCMTracingSampling(t *testing.T) { Name: "tracing", }, Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), }, }, expectedRandomSampling: 10.0, diff --git a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md index 680cd1374e2..29bd301a2c2 100644 --- a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md +++ b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md @@ -1 +1 @@ -Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest. The host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, and a provider that is still incomplete turns tracing off with a log message instead of blocking admission or the Gateway. +Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest. The host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, and a provider that is still incomplete turns tracing off with a log message instead of blocking admission or the Gateway. The tracing provider `type` and `port` are no longer defaulted by admission either, so a partial override no longer replaces the type or port inherited from the GatewayClass level; both still default to `OpenTelemetry` and `4317` during translation. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 5799dba0fba..28a76b9563e 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -6701,9 +6701,9 @@ _Appears in:_ | `backendRef` | _[BackendObjectReference](https://gateway-api.sigs.k8s.io/reference/api-spec/1.5/spec/#backendobjectreference)_ | false | | BackendRef references a Kubernetes object that represents the
backend server to which the authorization request will be sent.
Deprecated: Use BackendRefs instead. | | `backendRefs` | _[BackendRef](#backendref) array_ | false | | BackendRefs references a Kubernetes object that represents the
backend server to which the authorization request will be sent. | | `backendSettings` | _[ClusterSettings](#clustersettings)_ | false | | BackendSettings holds configuration for managing the connection
to the backend. | -| `type` | _[TracingProviderType](#tracingprovidertype)_ | true | OpenTelemetry | Type defines the tracing provider type. | +| `type` | _[TracingProviderType](#tracingprovidertype)_ | false | | 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). | | `host` | _string_ | false | | Host define the provider service hostname.
Deprecated: Use BackendRefs instead. | -| `port` | _integer_ | false | 4317 | Port defines the port the provider service is exposed on.
Deprecated: Use BackendRefs instead. | +| `port` | _integer_ | false | | 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. | | `serviceName` | _string_ | false | | 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").
Note: This field is only supported for OpenTelemetry and Datadog tracing providers.
For Zipkin, the service name in traces is always derived from the Envoy --service-cluster flag
(typically "namespace/name" format). Setting this field has no effect for Zipkin. | | `zipkin` | _[ZipkinTracingProvider](#zipkintracingprovider)_ | false | | Zipkin defines the Zipkin tracing provider configuration | | `openTelemetry` | _[OpenTelemetryTracingProvider](#opentelemetrytracingprovider)_ | false | | OpenTelemetry defines the OpenTelemetry tracing provider configuration | diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index cf8667e0d7c..6a60c2be5b2 100644 --- a/test/cel-validation/envoyproxy_test.go +++ b/test/cel-validation/envoyproxy_test.go @@ -17,6 +17,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -1243,7 +1244,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1268,7 +1269,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1292,7 +1293,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1320,7 +1321,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), ServiceName: new("my-override"), }, }, @@ -1335,7 +1336,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1361,7 +1362,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1682,7 +1683,7 @@ func TestEnvoyProxyProvider(t *testing.T) { }, }, }, - Type: egv1a1.TracingProviderTypeZipkin, + Type: ptr.To(egv1a1.TracingProviderTypeZipkin), Zipkin: &egv1a1.ZipkinTracingProvider{}, }, }, @@ -1700,7 +1701,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1728,7 +1729,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1759,7 +1760,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1790,7 +1791,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1824,7 +1825,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1858,7 +1859,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1892,7 +1893,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1926,7 +1927,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: egv1a1.TracingProviderTypeOpenTelemetry, + Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index dec5aca77ac..4c449976d56 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -53015,10 +53015,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 @@ -53036,8 +53038,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 @@ -53059,8 +53066,6 @@ spec: id will be used. type: boolean type: object - required: - - type type: object x-kubernetes-validations: - message: BackendRefs must be used, backendRef is not supported. @@ -53074,8 +53079,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 diff --git a/test/helm/gateway-crds-helm/e2e.out.yaml b/test/helm/gateway-crds-helm/e2e.out.yaml index 1aac8c5acb7..04bf8e27a6d 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -28953,10 +28953,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 @@ -28974,8 +28976,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 @@ -28997,8 +29004,6 @@ spec: id will be used. type: boolean type: object - required: - - type type: object x-kubernetes-validations: - message: BackendRefs must be used, backendRef is not supported. @@ -29012,8 +29017,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 diff --git a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml index 92b844a9f40..2fc8d004861 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -28953,10 +28953,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 @@ -28974,8 +28976,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 @@ -28997,8 +29004,6 @@ spec: id will be used. type: boolean type: object - required: - - type type: object x-kubernetes-validations: - message: BackendRefs must be used, backendRef is not supported. @@ -29012,8 +29017,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 From 65fea69b6abb466b22afb5709bccb029d92f0703 Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:07:02 +0300 Subject: [PATCH 6/7] chore: use new() instead of ptr.To in the tracing changes main now rejects ptr.To through forbidigo in favour of the new(expr) builtin, so the tracing provider defaults and their tests follow suit. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DbWe5JJNQYR3HvVFAPheNP Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- internal/gatewayapi/listener.go | 4 +-- internal/gatewayapi/listener_test.go | 18 +++++------ .../provider/kubernetes/predicates_test.go | 3 +- internal/xds/translator/tracing_test.go | 5 ++- test/cel-validation/envoyproxy_test.go | 31 +++++++++---------- 5 files changed, 29 insertions(+), 32 deletions(-) diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go index d29601acc4f..b5cc660d72f 100644 --- a/internal/gatewayapi/listener.go +++ b/internal/gatewayapi/listener.go @@ -1068,10 +1068,10 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En // The copy keeps the EnvoyProxy resource itself untouched. provider := tracing.Provider if provider.Type == nil { - provider.Type = ptr.To(egv1a1.DefaultTracingProviderType) + provider.Type = new(egv1a1.DefaultTracingProviderType) } if provider.Port == nil { - provider.Port = ptr.To(egv1a1.DefaultTracingProviderPort) + provider.Port = new(egv1a1.DefaultTracingProviderPort) } // TODO: rename this, so that we can share backend with accesslog? diff --git a/internal/gatewayapi/listener_test.go b/internal/gatewayapi/listener_test.go index e78e7302e5b..e99470cb7ab 100644 --- a/internal/gatewayapi/listener_test.go +++ b/internal/gatewayapi/listener_test.go @@ -950,7 +950,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -986,7 +986,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1026,7 +1026,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1067,7 +1067,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1104,7 +1104,7 @@ func TestProcessTracingServiceName(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), ServiceName: new("only-name-overridden"), }, }, @@ -1204,7 +1204,7 @@ func TestProcessTracingProviderDefaults(t *testing.T) { { name: "unset type and port fall back to the documented defaults", provider: egv1a1.TracingProvider{ - Host: ptr.To("otel-collector.monitoring.svc.cluster.local"), + Host: new("otel-collector.monitoring.svc.cluster.local"), }, expectedType: egv1a1.TracingProviderTypeOpenTelemetry, expectedPort: 4317, @@ -1212,9 +1212,9 @@ func TestProcessTracingProviderDefaults(t *testing.T) { { name: "type and port inherited from the GatewayClass level are kept", provider: egv1a1.TracingProvider{ - Host: ptr.To("datadog-agent.monitoring.svc.cluster.local"), - Type: ptr.To(egv1a1.TracingProviderTypeDatadog), - Port: ptr.To(int32(8126)), + Host: new("datadog-agent.monitoring.svc.cluster.local"), + Type: new(egv1a1.TracingProviderTypeDatadog), + Port: new(int32(8126)), }, expectedType: egv1a1.TracingProviderTypeDatadog, expectedPort: 8126, diff --git a/internal/provider/kubernetes/predicates_test.go b/internal/provider/kubernetes/predicates_test.go index b1015aa1a3a..35f908af204 100644 --- a/internal/provider/kubernetes/predicates_test.go +++ b/internal/provider/kubernetes/predicates_test.go @@ -17,7 +17,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -1381,7 +1380,7 @@ func TestValidateServiceForReconcile(t *testing.T) { }, Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { diff --git a/internal/xds/translator/tracing_test.go b/internal/xds/translator/tracing_test.go index e8d84aabac5..383a43b8d3c 100644 --- a/internal/xds/translator/tracing_test.go +++ b/internal/xds/translator/tracing_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/stretchr/testify/require" - "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -35,7 +34,7 @@ func TestBuildHCMTracingSampling(t *testing.T) { Name: "tracing", }, Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), }, }, expectedRandomSampling: 10.0, @@ -51,7 +50,7 @@ func TestBuildHCMTracingSampling(t *testing.T) { Name: "tracing", }, Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), }, }, expectedRandomSampling: 10.0, diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index 6a60c2be5b2..508c7e2387d 100644 --- a/test/cel-validation/envoyproxy_test.go +++ b/test/cel-validation/envoyproxy_test.go @@ -17,7 +17,6 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "k8s.io/utils/ptr" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -1244,7 +1243,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1269,7 +1268,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1293,7 +1292,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1321,7 +1320,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), ServiceName: new("my-override"), }, }, @@ -1336,7 +1335,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1362,7 +1361,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1683,7 +1682,7 @@ func TestEnvoyProxyProvider(t *testing.T) { }, }, }, - Type: ptr.To(egv1a1.TracingProviderTypeZipkin), + Type: new(egv1a1.TracingProviderTypeZipkin), Zipkin: &egv1a1.ZipkinTracingProvider{}, }, }, @@ -1701,7 +1700,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1729,7 +1728,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1760,7 +1759,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1791,7 +1790,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1825,7 +1824,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1859,7 +1858,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1893,7 +1892,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { @@ -1927,7 +1926,7 @@ func TestEnvoyProxyProvider(t *testing.T) { Telemetry: &egv1a1.ProxyTelemetry{ Tracing: &egv1a1.ProxyTracing{ Provider: egv1a1.TracingProvider{ - Type: ptr.To(egv1a1.TracingProviderTypeOpenTelemetry), + Type: new(egv1a1.TracingProviderTypeOpenTelemetry), BackendCluster: egv1a1.BackendCluster{ BackendRefs: []egv1a1.BackendRef{ { From 6fdac977e93380c39514b46bb881fdc6bed712c1 Mon Sep 17 00:00:00 2001 From: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> Date: Thu, 3 Sep 2026 02:07:03 +0300 Subject: [PATCH 7/7] fix: resolve inherited telemetry backendRefs in the namespace they were defined in The merged EnvoyProxy carries the Gateway-level object's metadata, so a backendRef inherited from the GatewayClass-level EnvoyProxy without an explicit namespace was looked up in the Gateway's namespace, and the Gateway was marked invalid even though the class-level reference was valid. This affected the tracing provider as well as the accessLog and metrics sinks. Before merging, every telemetry backendRef of the base EnvoyProxy that omits a namespace is now set to the base's own namespace, which is where it resolves when that EnvoyProxy is used on its own. Refs written on the Gateway-level EnvoyProxy are untouched and keep resolving in the Gateway's namespace. The shared GatewayClass object is not modified; a copy is merged instead. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DbWe5JJNQYR3HvVFAPheNP Signed-off-by: Kadir Can Yildirim <252162627+kadircanyildirm-crypto@users.noreply.github.com> --- internal/gatewayapi/envoyproxy_merge.go | 76 ++++- internal/gatewayapi/envoyproxy_merge_test.go | 179 ++++++++++++ ...rategic-merge-inherited-backendref.in.yaml | 89 ++++++ ...ategic-merge-inherited-backendref.out.yaml | 268 ++++++++++++++++++ .../bug_fixes/9527-tracing-cel-merge.md | 2 +- 5 files changed, 612 insertions(+), 2 deletions(-) create mode 100644 internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.in.yaml create mode 100644 internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.out.yaml diff --git a/internal/gatewayapi/envoyproxy_merge.go b/internal/gatewayapi/envoyproxy_merge.go index 53ab2d4f8b2..9d1ba015c0d 100644 --- a/internal/gatewayapi/envoyproxy_merge.go +++ b/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 +} diff --git a/internal/gatewayapi/envoyproxy_merge_test.go b/internal/gatewayapi/envoyproxy_merge_test.go index 7642662c093..7303b7e6b73 100644 --- a/internal/gatewayapi/envoyproxy_merge_test.go +++ b/internal/gatewayapi/envoyproxy_merge_test.go @@ -9,6 +9,8 @@ import ( "testing" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" ) @@ -272,3 +274,180 @@ func TestMergeEnvoyProxyConfigs(t *testing.T) { }) } } + +// TestMergeEnvoyProxyConfigsTelemetryBackendRefNamespace checks that telemetry +// backendRefs inherited from the GatewayClass-level EnvoyProxy keep resolving in +// that EnvoyProxy's namespace after the Gateway-level EnvoyProxy is merged over it. +func TestMergeEnvoyProxyConfigsTelemetryBackendRefNamespace(t *testing.T) { + const ( + classNS = "envoy-gateway-system" + gatewayNS = "app-ns" + ) + + ref := func(name string, ns *string) egv1a1.BackendRef { + r := egv1a1.BackendRef{ + BackendObjectReference: gwapiv1.BackendObjectReference{ + Name: gwapiv1.ObjectName(name), + Port: new(gwapiv1.PortNumber(4317)), + }, + } + if ns != nil { + r.Namespace = new(gwapiv1.Namespace(*ns)) + } + return r + } + nsOf := func(r egv1a1.BackendRef) string { + if r.Namespace == nil { + return "" + } + return string(*r.Namespace) + } + + newClassProxy := func() *egv1a1.EnvoyProxy { + return &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{Namespace: classNS, Name: "class"}, + Spec: egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ + Type: new(egv1a1.TracingProviderTypeDatadog), + BackendCluster: egv1a1.BackendCluster{ + BackendRefs: []egv1a1.BackendRef{ + ref("datadog-agent", nil), + ref("otel-collector", new("monitoring")), + }, + }, + }, + }, + AccessLog: &egv1a1.ProxyAccessLog{ + Settings: []egv1a1.ProxyAccessLogSetting{{ + Sinks: []egv1a1.ProxyAccessLogSink{ + { + Type: egv1a1.ProxyAccessLogSinkTypeALS, + ALS: &egv1a1.ALSEnvoyProxyAccessLog{ + Type: egv1a1.ALSEnvoyProxyAccessLogTypeHTTP, + BackendCluster: egv1a1.BackendCluster{BackendRefs: []egv1a1.BackendRef{ref("als", nil)}}, + }, + }, + { + Type: egv1a1.ProxyAccessLogSinkTypeOpenTelemetry, + OpenTelemetry: &egv1a1.OpenTelemetryEnvoyProxyAccessLog{ + BackendCluster: egv1a1.BackendCluster{BackendRefs: []egv1a1.BackendRef{ref("otel-logs", nil)}}, + }, + }, + }, + }}, + }, + Metrics: &egv1a1.ProxyMetrics{ + Sinks: []egv1a1.ProxyMetricSink{{ + Type: egv1a1.MetricSinkTypeOpenTelemetry, + OpenTelemetry: &egv1a1.ProxyOpenTelemetrySink{ + BackendCluster: egv1a1.BackendCluster{BackendRefs: []egv1a1.BackendRef{ref("otel-metrics", nil)}}, + }, + }}, + }, + }, + }, + } + } + newGatewayProxy := func(mergeType *egv1a1.MergeType, tracingRefs ...egv1a1.BackendRef) *egv1a1.EnvoyProxy { + return &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{Namespace: gatewayNS, Name: "gateway"}, + Spec: egv1a1.EnvoyProxySpec{ + MergeType: mergeType, + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ + ServiceName: new("my-custom-service"), + BackendCluster: egv1a1.BackendCluster{BackendRefs: tracingRefs}, + }, + }, + }, + }, + } + } + + for _, mergeType := range []egv1a1.MergeType{egv1a1.StrategicMerge, egv1a1.JSONMerge} { + t.Run(string(mergeType)+" pins inherited refs to the GatewayClass EnvoyProxy namespace", func(t *testing.T) { + classProxy := newClassProxy() + merged, err := MergeEnvoyProxyConfigs(nil, classProxy, newGatewayProxy(new(mergeType))) + require.NoError(t, err) + + require.Equal(t, gatewayNS, merged.Namespace) + telemetry := merged.Spec.Telemetry + require.Equal(t, "my-custom-service", *telemetry.Tracing.Provider.ServiceName) + require.Equal(t, egv1a1.TracingProviderTypeDatadog, *telemetry.Tracing.Provider.Type) + + tracingRefs := telemetry.Tracing.Provider.BackendRefs + require.Len(t, tracingRefs, 2) + require.Equal(t, classNS, nsOf(tracingRefs[0]), "inherited ref without a namespace") + require.Equal(t, "monitoring", nsOf(tracingRefs[1]), "explicit namespace is kept") + require.Equal(t, classNS, nsOf(telemetry.AccessLog.Settings[0].Sinks[0].ALS.BackendRefs[0])) + require.Equal(t, classNS, nsOf(telemetry.AccessLog.Settings[0].Sinks[1].OpenTelemetry.BackendRefs[0])) + require.Equal(t, classNS, nsOf(telemetry.Metrics.Sinks[0].OpenTelemetry.BackendRefs[0])) + + // The GatewayClass EnvoyProxy is shared across Gateways and must not be modified. + require.Equal(t, newClassProxy(), classProxy) + }) + } + + t.Run("refs set by the Gateway EnvoyProxy keep resolving in its own namespace", func(t *testing.T) { + gatewayProxy := newGatewayProxy(new(egv1a1.StrategicMerge), ref("gateway-collector", nil)) + merged, err := MergeEnvoyProxyConfigs(nil, newClassProxy(), gatewayProxy) + require.NoError(t, err) + + tracingRefs := merged.Spec.Telemetry.Tracing.Provider.BackendRefs + require.Len(t, tracingRefs, 1) + require.Equal(t, "gateway-collector", string(tracingRefs[0].Name)) + require.Empty(t, nsOf(tracingRefs[0])) + // Sinks the Gateway EnvoyProxy did not touch are still inherited and pinned. + require.Equal(t, classNS, nsOf(merged.Spec.Telemetry.Metrics.Sinks[0].OpenTelemetry.BackendRefs[0])) + }) + + t.Run("replace mode leaves the Gateway EnvoyProxy untouched", func(t *testing.T) { + gatewayProxy := newGatewayProxy(nil, ref("gateway-collector", nil)) + merged, err := MergeEnvoyProxyConfigs(nil, newClassProxy(), gatewayProxy) + require.NoError(t, err) + + require.Same(t, gatewayProxy, merged) + require.Empty(t, nsOf(merged.Spec.Telemetry.Tracing.Provider.BackendRefs[0])) + }) + + t.Run("no Gateway EnvoyProxy leaves the GatewayClass EnvoyProxy untouched", func(t *testing.T) { + classProxy := newClassProxy() + merged, err := MergeEnvoyProxyConfigs(nil, classProxy, nil) + require.NoError(t, err) + + require.Same(t, classProxy, merged) + require.Empty(t, nsOf(merged.Spec.Telemetry.Tracing.Provider.BackendRefs[0])) + }) + + t.Run("default spec refs have no namespace to pin", func(t *testing.T) { + defaultSpec := &egv1a1.EnvoyProxySpec{ + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ + BackendCluster: egv1a1.BackendCluster{BackendRefs: []egv1a1.BackendRef{ref("default-collector", nil)}}, + }, + }, + }, + } + classProxy := &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{Namespace: classNS, Name: "class"}, + Spec: egv1a1.EnvoyProxySpec{ + MergeType: new(egv1a1.StrategicMerge), + Telemetry: &egv1a1.ProxyTelemetry{ + Tracing: &egv1a1.ProxyTracing{ + Provider: egv1a1.TracingProvider{ServiceName: new("class-service")}, + }, + }, + }, + } + merged, err := MergeEnvoyProxyConfigs(defaultSpec, classProxy, nil) + require.NoError(t, err) + + tracingRefs := merged.Spec.Telemetry.Tracing.Provider.BackendRefs + require.Len(t, tracingRefs, 1) + require.Empty(t, nsOf(tracingRefs[0])) + }) +} diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.in.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.in.yaml new file mode 100644 index 00000000000..2de33feca5b --- /dev/null +++ b/internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.in.yaml @@ -0,0 +1,89 @@ +gatewayClass: + apiVersion: gateway.networking.k8s.io/v1 + kind: GatewayClass + metadata: + name: envoy-gateway-class + spec: + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: class-proxy + namespace: envoy-gateway-system +envoyProxyForGatewayClass: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + namespace: envoy-gateway-system + name: class-proxy + spec: + telemetry: + tracing: + provider: + type: Datadog + backendRefs: + - name: datadog-agent + port: 8126 +envoyProxiesForGateways: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + namespace: envoy-gateway + name: gateway-proxy + spec: + mergeType: StrategicMerge + telemetry: + tracing: + provider: + serviceName: my-custom-service +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: gateway-proxy + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: Same +services: +- apiVersion: v1 + kind: Service + metadata: + name: datadog-agent + namespace: envoy-gateway-system + spec: + clusterIP: 10.11.12.13 + ports: + - name: traces + port: 8126 + protocol: TCP + targetPort: 8126 +endpointSlices: +- apiVersion: discovery.k8s.io/v1 + kind: EndpointSlice + metadata: + name: endpointslice-datadog-agent + namespace: envoy-gateway-system + labels: + kubernetes.io/service-name: datadog-agent + addressType: IPv4 + ports: + - name: traces + protocol: TCP + port: 8126 + endpoints: + - addresses: + - 7.7.7.7 + conditions: + ready: true diff --git a/internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.out.yaml b/internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.out.yaml new file mode 100644 index 00000000000..fc1a907c7fa --- /dev/null +++ b/internal/gatewayapi/testdata/envoyproxy-tracing-strategic-merge-inherited-backendref.out.yaml @@ -0,0 +1,268 @@ +envoyProxiesForGateways: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + name: gateway-proxy + namespace: envoy-gateway + spec: + logging: {} + mergeType: StrategicMerge + telemetry: + tracing: + provider: + backendRefs: + - name: datadog-agent + namespace: envoy-gateway-system + port: 8126 + serviceName: my-custom-service + type: Datadog + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + conditions: + - lastTransitionTime: null + message: EnvoyProxy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: EnvoyProxy has been accepted. + reason: Accepted + status: "True" + type: Accepted +envoyProxyForGatewayClass: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + name: class-proxy + namespace: envoy-gateway-system + spec: + logging: {} + telemetry: + tracing: + provider: + backendRefs: + - name: datadog-agent + port: 8126 + type: Datadog + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + conditions: + - lastTransitionTime: null + message: EnvoyProxy has been accepted. + reason: Accepted + status: "True" + type: Accepted +gatewayClass: + apiVersion: gateway.networking.k8s.io/v1 + kind: GatewayClass + metadata: + name: envoy-gateway-class + spec: + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: class-proxy + namespace: envoy-gateway-system + status: + conditions: + - lastTransitionTime: null + message: Valid GatewayClass + reason: Accepted + status: "True" + type: Accepted +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + infrastructure: + parametersRef: + group: gateway.envoyproxy.io + kind: EnvoyProxy + name: gateway-proxy + listeners: + - allowedRoutes: + namespaces: + from: Same + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 0 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +infraIR: + envoy-gateway/gateway-1: + proxy: + config: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + name: gateway-proxy + namespace: envoy-gateway + spec: + logging: {} + mergeType: StrategicMerge + telemetry: + tracing: + provider: + backendRefs: + - name: datadog-agent + namespace: envoy-gateway-system + port: 8126 + serviceName: my-custom-service + type: Datadog + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: GatewayClass + name: envoy-gateway-class + conditions: + - lastTransitionTime: null + message: EnvoyProxy has been accepted. + reason: Accepted + status: "True" + type: Accepted + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: EnvoyProxy has been accepted. + reason: Accepted + status: "True" + type: Accepted + listeners: + - name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 + tracing: + authority: datadog-agent.envoy-gateway-system.svc + destination: + metadata: + kind: EnvoyProxy + name: gateway-proxy + namespace: envoy-gateway + name: tracing + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8126 + metadata: + kind: Service + name: datadog-agent + namespace: envoy-gateway-system + sectionName: "8126" + name: tracing/backend/-1 + protocol: TCP + provider: + backendRefs: + - name: datadog-agent + namespace: envoy-gateway-system + port: 8126 + port: 4317 + serviceName: my-custom-service + type: Datadog + samplingRate: 100 + serviceName: my-custom-service diff --git a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md index 29bd301a2c2..fda8c0a98d1 100644 --- a/release-notes/current/bug_fixes/9527-tracing-cel-merge.md +++ b/release-notes/current/bug_fixes/9527-tracing-cel-merge.md @@ -1 +1 @@ -Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest. The host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, and a provider that is still incomplete turns tracing off with a log message instead of blocking admission or the Gateway. The tracing provider `type` and `port` are no longer defaulted by admission either, so a partial override no longer replaces the type or port inherited from the GatewayClass level; both still default to `OpenTelemetry` and `4317` during translation. +Fixed CRD validation rejecting a per-Gateway EnvoyProxy that overrides only part of the tracing provider (e.g. `serviceName`) when relying on `mergeType` to inherit the rest. The host/backendRefs completeness check now runs after the GatewayClass-level and Gateway-level configs are merged, and a provider that is still incomplete turns tracing off with a log message instead of blocking admission or the Gateway. The tracing provider `type` and `port` are no longer defaulted by admission either, so a partial override no longer replaces the type or port inherited from the GatewayClass level; both still default to `OpenTelemetry` and `4317` during translation. Telemetry `backendRefs` (tracing provider, accessLog and metrics sinks) inherited from the GatewayClass-level EnvoyProxy that do not set a namespace are now resolved in that EnvoyProxy's namespace instead of the Gateway-level EnvoyProxy's namespace.