Skip to content

Commit ac71f9f

Browse files
kadircanyildirm-cryptozirain
authored andcommitted
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>
1 parent 6757380 commit ac71f9f

5 files changed

Lines changed: 18 additions & 16 deletions

File tree

api/v1alpha1/envoyproxy_tracing_types.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ const (
3838
//
3939
// A provider is only required to set host or backendRefs after the
4040
// GatewayClass-level and Gateway-level EnvoyProxy configs are merged
41-
// (see EnvoyProxySpec.MergeType), so completeness is validated during
42-
// translation instead of by a CEL rule here.
41+
// (see EnvoyProxySpec.MergeType), so completeness is checked during
42+
// translation instead of by a CEL rule here. A provider that is still
43+
// incomplete after the merge turns tracing off for that Gateway.
4344
//
4445
// +kubebuilder:validation:XValidation:message="BackendRefs must be used, backendRef is not supported.",rule="!has(self.backendRef)"
4546
// +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"

internal/gatewayapi/listener.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,11 +1055,16 @@ func (t *Translator) processTracing(gwCtx *GatewayContext, envoyproxy *egv1a1.En
10551055
// fallback to host and port
10561056
// TODO: remove support for Host/Port in v1.2
10571057
if len(ds) == 0 {
1058-
// Validated here instead of by a CRD CEL rule so that a partial provider
1058+
// Checked here instead of by a CRD CEL rule so that a partial provider
10591059
// (e.g. only serviceName) can be completed by the GatewayClass-level and
1060-
// Gateway-level EnvoyProxy merge before the check runs.
1060+
// Gateway-level EnvoyProxy merge before the check runs. An incomplete
1061+
// provider only turns tracing off, it does not stop the Gateway from
1062+
// being provisioned.
10611063
if tracing.Provider.Host == nil {
1062-
return nil, fmt.Errorf("host or backendRefs needs to be set on the tracing provider after merging EnvoyProxy configs")
1064+
t.Logger.Info("Disabling tracing because the merged tracing provider sets neither host nor backendRefs",
1065+
"gateway", utils.NamespacedName(gwCtx.Gateway).String(),
1066+
"envoyProxy", utils.NamespacedName(envoyproxy).String())
1067+
return nil, nil
10631068
}
10641069
host, port := *tracing.Provider.Host, uint32(tracing.Provider.Port)
10651070
ds = destinationSettingFromHostAndPort(settingName, host, port)

internal/gatewayapi/listener_test.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,6 @@ func TestProcessTracingServiceName(t *testing.T) {
916916
envoyProxy *egv1a1.EnvoyProxy
917917
mergeGateways bool
918918
expectedServiceName string
919-
expectError bool
920919
}{
921920
{
922921
name: "no tracing configuration",
@@ -1089,7 +1088,7 @@ func TestProcessTracingServiceName(t *testing.T) {
10891088
expectedServiceName: "test-gateway-class", // Should use gateway class name when merging
10901089
},
10911090
{
1092-
name: "tracing provider without backendRefs or host",
1091+
name: "tracing provider without backendRefs or host disables tracing",
10931092
gateway: &gwapiv1.Gateway{
10941093
ObjectMeta: metav1.ObjectMeta{
10951094
Name: "test-gateway",
@@ -1112,7 +1111,8 @@ func TestProcessTracingServiceName(t *testing.T) {
11121111
},
11131112
},
11141113
},
1115-
expectError: true,
1114+
// An empty expectedServiceName asserts that no tracing config is built.
1115+
expectedServiceName: "",
11161116
},
11171117
}
11181118

@@ -1177,11 +1177,6 @@ func TestProcessTracingServiceName(t *testing.T) {
11771177
Gateway: tc.gateway,
11781178
}, tc.envoyProxy, tc.mergeGateways, resources)
11791179

1180-
if tc.expectError {
1181-
assert.Error(t, err)
1182-
return
1183-
}
1184-
11851180
require.NoError(t, err)
11861181

11871182
if tc.expectedServiceName == "" {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +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.
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.

site/content/en/latest/api/extension_types.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6674,8 +6674,9 @@ TracingProvider defines the tracing provider configuration.
66746674

66756675
A provider is only required to set host or backendRefs after the
66766676
GatewayClass-level and Gateway-level EnvoyProxy configs are merged
6677-
(see EnvoyProxySpec.MergeType), so completeness is validated during
6678-
translation instead of by a CEL rule here.
6677+
(see EnvoyProxySpec.MergeType), so completeness is checked during
6678+
translation instead of by a CEL rule here. A provider that is still
6679+
incomplete after the merge turns tracing off for that Gateway.
66796680

66806681
_Appears in:_
66816682
- [ProxyTracing](#proxytracing)

0 commit comments

Comments
 (0)