diff --git a/api/v1alpha1/envoyproxy_tracing_types.go b/api/v1alpha1/envoyproxy_tracing_types.go index 11eaeb686d..f4fcfb15e5 100644 --- a/api/v1alpha1/envoyproxy_tracing_types.go +++ b/api/v1alpha1/envoyproxy_tracing_types.go @@ -36,7 +36,12 @@ 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 checked during +// translation instead of by a CEL rule here. A provider that is still +// incomplete after the merge turns tracing off for that Gateway. +// // +kubebuilder:validation:XValidation:message="BackendRefs must be used, backendRef is not supported.",rule="!has(self.backendRef)" // +kubebuilder:validation:XValidation:message="BackendRefs only support Service and Backend kind.",rule="has(self.backendRefs) ? self.backendRefs.all(f, f.kind == 'Service' || f.kind == 'Backend') : true" // +kubebuilder:validation:XValidation:message="BackendRefs only support Core and gateway.envoyproxy.io group.",rule="has(self.backendRefs) ? (self.backendRefs.all(f, f.group == \"\" || f.group == 'gateway.envoyproxy.io')) : true" 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 9e52118274..75cce2c965 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 @@ -18193,8 +18193,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 256ae54218..d0c88ab131 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 @@ -18192,8 +18192,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 b7f6ca661c..7768b24ca9 100644 --- a/internal/gatewayapi/listener.go +++ b/internal/gatewayapi/listener.go @@ -994,11 +994,18 @@ 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) + // 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. An incomplete + // provider only turns tracing off, it does not stop the Gateway from + // being provisioned. + if tracing.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) ds = destinationSettingFromHostAndPort(settingName, host, port) authority = host } diff --git a/internal/gatewayapi/listener_test.go b/internal/gatewayapi/listener_test.go index 13005bcaf7..451a45fcd3 100644 --- a/internal/gatewayapi/listener_test.go +++ b/internal/gatewayapi/listener_test.go @@ -762,7 +762,6 @@ func TestProcessTracingServiceName(t *testing.T) { envoyProxy *egv1a1.EnvoyProxy mergeGateways bool expectedServiceName string - expectError bool }{ { name: "no tracing configuration", @@ -934,6 +933,33 @@ 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 disables tracing", + 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"), + }, + }, + }, + }, + }, + // An empty expectedServiceName asserts that no tracing config is built. + expectedServiceName: "", + }, } for _, tc := range cases { @@ -997,11 +1023,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 new file mode 100644 index 0000000000..680cd1374e --- /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, 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 5115c98ca5..a29cfe7295 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -6525,6 +6525,12 @@ _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 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) diff --git a/test/cel-validation/envoyproxy_test.go b/test/cel-validation/envoyproxy_test.go index 770dcc6494..c157a62afb 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", diff --git a/test/helm/gateway-crds-helm/all.out.yaml b/test/helm/gateway-crds-helm/all.out.yaml index 5cefe156f4..999f646811 100644 --- a/test/helm/gateway-crds-helm/all.out.yaml +++ b/test/helm/gateway-crds-helm/all.out.yaml @@ -52059,8 +52059,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 25daff866b..6d93ffbd6b 100644 --- a/test/helm/gateway-crds-helm/e2e.out.yaml +++ b/test/helm/gateway-crds-helm/e2e.out.yaml @@ -27997,8 +27997,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 9fa651b4d8..aa17b2b566 100644 --- a/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml +++ b/test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml @@ -27997,8 +27997,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.