Skip to content

Commit 72c5161

Browse files
H34rtUnd3rB14d3H34rtUnd3rB14d3
committed
feat: restrict defaultChildMergeType to top-level Gateway targets
Reject defaultChildMergeType on Listener (sectionName) targets via CEL so the child merge default is declared only at the top-level Gateway. The default that children inherit always comes from the Gateway-level policy Add CEL, unit, and golden test coverage. Signed-off-by: Maksim Kuchkovskiy <K.Maksim.E@yandex.ru>
1 parent ebea924 commit 72c5161

12 files changed

Lines changed: 551 additions & 54 deletions

‎api/v1alpha1/backendtrafficpolicy_types.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ type BackendTrafficPolicy struct {
4646
// +kubebuilder:validation:XValidation:rule="!has(self.compression) || !has(self.compressor)", message="either compression or compressor can be set, not both"
4747
// +kubebuilder:validation:XValidation:rule="!has(self.requestBuffer) || !has(self.httpUpgrade) || self.httpUpgrade.size() == 0", message="requestBuffer cannot be used together with httpUpgrade"
4848
// +kubebuilder:validation:XValidation:rule="!has(self.admissionControl) || ((!has(self.targetRef) || self.targetRef.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute']) && (!has(self.targetRefs) || self.targetRefs.all(ref, ref.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute'])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute'])))", message="admissionControl can only be used with HTTPRoute, GRPCRoute, or Gateway targets"
49-
// +kubebuilder:validation:XValidation:rule="!has(self.defaultChildMergeType) || ((!has(self.targetRef) || self.targetRef.kind == 'Gateway') && (!has(self.targetRefs) || self.targetRefs.all(ref, ref.kind == 'Gateway')) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind == 'Gateway')))", message="defaultChildMergeType can only be used with Gateway targets"
49+
// +kubebuilder:validation:XValidation:rule="!has(self.defaultChildMergeType) || ((!has(self.targetRef) || (self.targetRef.kind == 'Gateway' && !has(self.targetRef.sectionName))) && (!has(self.targetRefs) || self.targetRefs.all(ref, ref.kind == 'Gateway' && !has(ref.sectionName))) && (!has(self.targetSelectors) || self.targetSelectors.all(sel, sel.kind == 'Gateway')))", message="defaultChildMergeType can only be used with Gateway targets without a sectionName"
5050
type BackendTrafficPolicySpec struct {
5151
PolicyTargetReferences `json:",inline"`
5252
ClusterSettings `json:",inline"`
@@ -65,7 +65,9 @@ type BackendTrafficPolicySpec struct {
6565
// an xRoute under this policy's target) that do not set their own mergeType, so a child
6666
// policy merges into this policy instead of replacing it. A child policy can opt out by
6767
// setting mergeType to Replace.
68-
// This field can only be set on policies targeting a parent resource (Gateway).
68+
// This field can only be set on policies targeting an entire Gateway. It is rejected on
69+
// policies targeting a specific Listener (via sectionName), so the default is defined at a
70+
// single top-level parent rather than at multiple intermediate parents.
6971
//
7072
// +kubebuilder:validation:Enum=StrategicMerge;JSONMerge
7173
// +optional

‎charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,9 @@ spec:
519519
an xRoute under this policy's target) that do not set their own mergeType, so a child
520520
policy merges into this policy instead of replacing it. A child policy can opt out by
521521
setting mergeType to Replace.
522-
This field can only be set on policies targeting a parent resource (Gateway).
522+
This field can only be set on policies targeting an entire Gateway. It is rejected on
523+
policies targeting a specific Listener (via sectionName), so the default is defined at a
524+
single top-level parent rather than at multiple intermediate parents.
523525
enum:
524526
- StrategicMerge
525527
- JSONMerge
@@ -3369,10 +3371,12 @@ spec:
33693371
''GRPCRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel,
33703372
sel.kind in [''Gateway'', ''HTTPRoute'', ''GRPCRoute''])))'
33713373
- message: defaultChildMergeType can only be used with Gateway targets
3374+
without a sectionName
33723375
rule: '!has(self.defaultChildMergeType) || ((!has(self.targetRef) ||
3373-
self.targetRef.kind == ''Gateway'') && (!has(self.targetRefs) || self.targetRefs.all(ref,
3374-
ref.kind == ''Gateway'')) && (!has(self.targetSelectors) || self.targetSelectors.all(sel,
3375-
sel.kind == ''Gateway'')))'
3376+
(self.targetRef.kind == ''Gateway'' && !has(self.targetRef.sectionName)))
3377+
&& (!has(self.targetRefs) || self.targetRefs.all(ref, ref.kind ==
3378+
''Gateway'' && !has(ref.sectionName))) && (!has(self.targetSelectors)
3379+
|| self.targetSelectors.all(sel, sel.kind == ''Gateway'')))'
33763380
- message: predictivePercent in preconnect policy only works with RoundRobin
33773381
or Random load balancers
33783382
rule: '!((has(self.connection) && has(self.connection.preconnect) &&

‎charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_backendtrafficpolicies.yaml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,9 @@ spec:
518518
an xRoute under this policy's target) that do not set their own mergeType, so a child
519519
policy merges into this policy instead of replacing it. A child policy can opt out by
520520
setting mergeType to Replace.
521-
This field can only be set on policies targeting a parent resource (Gateway).
521+
This field can only be set on policies targeting an entire Gateway. It is rejected on
522+
policies targeting a specific Listener (via sectionName), so the default is defined at a
523+
single top-level parent rather than at multiple intermediate parents.
522524
enum:
523525
- StrategicMerge
524526
- JSONMerge
@@ -3368,10 +3370,12 @@ spec:
33683370
''GRPCRoute''])) && (!has(self.targetSelectors) || self.targetSelectors.all(sel,
33693371
sel.kind in [''Gateway'', ''HTTPRoute'', ''GRPCRoute''])))'
33703372
- message: defaultChildMergeType can only be used with Gateway targets
3373+
without a sectionName
33713374
rule: '!has(self.defaultChildMergeType) || ((!has(self.targetRef) ||
3372-
self.targetRef.kind == ''Gateway'') && (!has(self.targetRefs) || self.targetRefs.all(ref,
3373-
ref.kind == ''Gateway'')) && (!has(self.targetSelectors) || self.targetSelectors.all(sel,
3374-
sel.kind == ''Gateway'')))'
3375+
(self.targetRef.kind == ''Gateway'' && !has(self.targetRef.sectionName)))
3376+
&& (!has(self.targetRefs) || self.targetRefs.all(ref, ref.kind ==
3377+
''Gateway'' && !has(ref.sectionName))) && (!has(self.targetSelectors)
3378+
|| self.targetSelectors.all(sel, sel.kind == ''Gateway'')))'
33753379
- message: predictivePercent in preconnect policy only works with RoundRobin
33763380
or Random load balancers
33773381
rule: '!((has(self.connection) && has(self.connection.preconnect) &&

‎internal/gatewayapi/backendtrafficpolicy.go‎

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -535,9 +535,14 @@ func (t *Translator) processBackendTrafficPolicyForRoute(
535535
parentPolicy = listenerPolicy
536536
}
537537

538-
// Resolve the effective mergeType: the policy's own value, or the closest
539-
// parent policy's defaultChildMergeType.
540-
mergeType := effectiveMergeType(policy, parentPolicy)
538+
// The default-merge intent comes from the nearest ancestor that declares
539+
// defaultChildMergeType (listener before gateway), independent of which parent
540+
// the child merges into. This keeps a listener-level policy from suppressing the
541+
// gateway-level default.
542+
defaultMergeType := resolveDefaultChildMergeType(listenerPolicy, gwPolicy)
543+
544+
// Resolve the effective mergeType: the policy's own value, or the resolved default.
545+
mergeType := effectiveMergeType(policy, defaultMergeType)
541546
if mergeType == nil || parentPolicy == nil {
542547
// No merge for this gateway: apply the policy standalone.
543548
if err := t.translateBackendTrafficPolicyForRoute(policy, targetedRoute, currTarget, xdsIR, &gwNN, &listener.Name); err != nil {
@@ -1079,25 +1084,39 @@ func (t *Translator) applyTrafficFeatureToRoute(route RouteContext,
10791084

10801085
// effectiveMergeType returns the mergeType to use when merging a route-level policy into its
10811086
// parent policy: the policy's own value if set (Replace meaning "do not merge"), otherwise the
1082-
// parent policy's defaultChildMergeType.
1083-
func effectiveMergeType(policy, parentPolicy *egv1a1.BackendTrafficPolicy) *egv1a1.MergeType {
1087+
// resolved defaultChildMergeType from the nearest ancestor that declares one.
1088+
func effectiveMergeType(policy *egv1a1.BackendTrafficPolicy, defaultMergeType *egv1a1.MergeType) *egv1a1.MergeType {
10841089
if policy.Spec.MergeType != nil {
10851090
if *policy.Spec.MergeType == egv1a1.Replace {
10861091
return nil
10871092
}
10881093
return policy.Spec.MergeType
10891094
}
1090-
if parentPolicy == nil || parentPolicy.Spec.DefaultChildMergeType == nil {
1091-
return nil
1092-
}
1093-
// Defense in depth: the CRD enum restricts DefaultChildMergeType to StrategicMerge/JSONMerge.
1094-
// Ignore anything that is not a real merge so a stray value can never produce a "merged"
1095-
// status while actually replacing the parent.
1096-
if *parentPolicy.Spec.DefaultChildMergeType != egv1a1.StrategicMerge &&
1097-
*parentPolicy.Spec.DefaultChildMergeType != egv1a1.JSONMerge {
1098-
return nil
1095+
return defaultMergeType
1096+
}
1097+
1098+
// resolveDefaultChildMergeType returns the defaultChildMergeType declared by the nearest ancestor
1099+
// policy, checking from the closest parent to the furthest (listener before gateway). CEL restricts
1100+
// defaultChildMergeType to policies targeting an entire Gateway, so the merge target for a child
1101+
// under a listener may be the listener-level policy while the default intent is declared on the
1102+
// gateway-level policy. Resolving the default independently of the merge target keeps a
1103+
// listener-level policy from suppressing the gateway-level default.
1104+
func resolveDefaultChildMergeType(ancestors ...*egv1a1.BackendTrafficPolicy) *egv1a1.MergeType {
1105+
for _, p := range ancestors {
1106+
if p == nil || p.Spec.DefaultChildMergeType == nil {
1107+
continue
1108+
}
1109+
mergeType := p.Spec.DefaultChildMergeType
1110+
// Defense in depth: the CRD enum restricts DefaultChildMergeType to StrategicMerge/JSONMerge.
1111+
// The nearest declarer wins, so if its value is not a real merge, return nil rather than
1112+
// falling through, ensuring a stray value can never produce a "merged" status while actually
1113+
// replacing the parent.
1114+
if *mergeType != egv1a1.StrategicMerge && *mergeType != egv1a1.JSONMerge {
1115+
return nil
1116+
}
1117+
return mergeType
10991118
}
1100-
return parentPolicy.Spec.DefaultChildMergeType
1119+
return nil
11011120
}
11021121

11031122
// anyParentPolicyMergeDefault reports whether any parent policy (gateway- or listener-level) of

‎internal/gatewayapi/backendtrafficpolicy_mergedefault_test.go‎

Lines changed: 50 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,30 +28,64 @@ func TestEffectiveMergeType(t *testing.T) {
2828
Spec: egv1a1.BackendTrafficPolicySpec{MergeType: mt},
2929
}
3030
}
31-
parent := func(defaultMT *egv1a1.MergeType) *egv1a1.BackendTrafficPolicy {
32-
return &egv1a1.BackendTrafficPolicy{
33-
ObjectMeta: metav1.ObjectMeta{Namespace: "app", Name: "parent"},
34-
Spec: egv1a1.BackendTrafficPolicySpec{DefaultChildMergeType: defaultMT},
35-
}
31+
32+
tests := []struct {
33+
name string
34+
pol *egv1a1.BackendTrafficPolicy
35+
defaultMT *egv1a1.MergeType
36+
want *egv1a1.MergeType
37+
}{
38+
{"explicit value wins over default", child(&jsonMerge), &strategic, &jsonMerge},
39+
{"explicit replace opts out of default", child(&replace), &strategic, nil},
40+
{"default applied when unset", child(nil), &strategic, &strategic},
41+
{"no default stays nil", child(nil), nil, nil},
42+
}
43+
44+
for _, tt := range tests {
45+
t.Run(tt.name, func(t *testing.T) {
46+
got := effectiveMergeType(tt.pol, tt.defaultMT)
47+
if tt.want == nil {
48+
assert.Nil(t, got)
49+
return
50+
}
51+
assert.NotNil(t, got)
52+
assert.Equal(t, *tt.want, *got)
53+
})
54+
}
55+
}
56+
57+
// TestResolveDefaultChildMergeType covers resolveDefaultChildMergeType, which returns the
58+
// defaultChildMergeType declared by the nearest ancestor policy (closest first). This decouples
59+
// the default-merge intent (which, after CEL restricts defaultChildMergeType to Gateway targets,
60+
// is only declared on the Gateway-level policy) from the merge target (the closest parent config).
61+
func TestResolveDefaultChildMergeType(t *testing.T) {
62+
strategic := egv1a1.StrategicMerge
63+
jsonMerge := egv1a1.JSONMerge
64+
replace := egv1a1.Replace
65+
66+
withDefault := func(mt *egv1a1.MergeType) *egv1a1.BackendTrafficPolicy {
67+
return &egv1a1.BackendTrafficPolicy{Spec: egv1a1.BackendTrafficPolicySpec{DefaultChildMergeType: mt}}
3668
}
3769

3870
tests := []struct {
39-
name string
40-
pol *egv1a1.BackendTrafficPolicy
41-
parent *egv1a1.BackendTrafficPolicy
42-
want *egv1a1.MergeType
71+
name string
72+
ancestors []*egv1a1.BackendTrafficPolicy
73+
want *egv1a1.MergeType
4374
}{
44-
{"explicit value wins over parent default", child(&jsonMerge), parent(&strategic), &jsonMerge},
45-
{"explicit replace opts out of parent default", child(&replace), parent(&strategic), nil},
46-
{"parent default applied when unset", child(nil), parent(&strategic), &strategic},
47-
{"no parent policy stays nil", child(nil), nil, nil},
48-
{"parent without default stays nil", child(nil), parent(nil), nil},
49-
{"invalid parent default is ignored", child(nil), parent(&replace), nil},
75+
{"no ancestors", nil, nil},
76+
{"nil ancestor", []*egv1a1.BackendTrafficPolicy{nil}, nil},
77+
{"single ancestor with default", []*egv1a1.BackendTrafficPolicy{withDefault(&strategic)}, &strategic},
78+
{"single ancestor without default", []*egv1a1.BackendTrafficPolicy{withDefault(nil)}, nil},
79+
// Reading B: the closest parent (listener) declares no default, so resolution falls through
80+
// to the gateway-level policy that does.
81+
{"closest without default falls through", []*egv1a1.BackendTrafficPolicy{withDefault(nil), withDefault(&strategic)}, &strategic},
82+
{"closest declarer wins", []*egv1a1.BackendTrafficPolicy{withDefault(&jsonMerge), withDefault(&strategic)}, &jsonMerge},
83+
{"invalid nearest default is ignored", []*egv1a1.BackendTrafficPolicy{withDefault(&replace), withDefault(&strategic)}, nil},
5084
}
5185

5286
for _, tt := range tests {
5387
t.Run(tt.name, func(t *testing.T) {
54-
got := effectiveMergeType(tt.pol, tt.parent)
88+
got := resolveDefaultChildMergeType(tt.ancestors...)
5589
if tt.want == nil {
5690
assert.Nil(t, got)
5791
return

‎internal/gatewayapi/testdata/backendtrafficpolicy-default-mergetype-listener-parent.in.yaml‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
gateways:
2+
- apiVersion: gateway.networking.k8s.io/v1
3+
kind: Gateway
4+
metadata:
5+
namespace: envoy-gateway
6+
name: gateway-1
7+
spec:
8+
gatewayClassName: envoy-gateway-class
9+
listeners:
10+
- name: http
11+
protocol: HTTP
12+
port: 80
13+
allowedRoutes:
14+
namespaces:
15+
from: All
16+
httpRoutes:
17+
- apiVersion: gateway.networking.k8s.io/v1
18+
kind: HTTPRoute
19+
metadata:
20+
namespace: default
21+
name: httproute-1
22+
spec:
23+
hostnames:
24+
- gateway.envoyproxy.io
25+
parentRefs:
26+
- namespace: envoy-gateway
27+
name: gateway-1
28+
sectionName: http
29+
rules:
30+
- matches:
31+
- path:
32+
value: "/"
33+
backendRefs:
34+
- name: service-1
35+
port: 8080
36+
backendTrafficPolicies:
37+
# Gateway-level policy declares defaultChildMergeType. It is the only place the default can be
38+
# declared (CEL rejects it on listener targets).
39+
- apiVersion: gateway.envoyproxy.io/v1alpha1
40+
kind: BackendTrafficPolicy
41+
metadata:
42+
namespace: envoy-gateway
43+
name: policy-for-gateway
44+
spec:
45+
targetRef:
46+
group: gateway.networking.k8s.io
47+
kind: Gateway
48+
name: gateway-1
49+
defaultChildMergeType: StrategicMerge
50+
timeout:
51+
tcp:
52+
connectTimeout: 15s
53+
httpUpgrade:
54+
- type: websocket
55+
# Listener-level policy is the closest parent config a child merges into. It sets no
56+
# defaultChildMergeType, so under "closest declared default" resolution the child still inherits
57+
# the gateway-level default and merges into this policy.
58+
- apiVersion: gateway.envoyproxy.io/v1alpha1
59+
kind: BackendTrafficPolicy
60+
metadata:
61+
namespace: envoy-gateway
62+
name: policy-for-listener
63+
spec:
64+
targetRef:
65+
group: gateway.networking.k8s.io
66+
kind: Gateway
67+
name: gateway-1
68+
sectionName: http
69+
timeout:
70+
http:
71+
connectionIdleTimeout: 16s
72+
maxConnectionDuration: 17s
73+
httpUpgrade:
74+
- type: websocket
75+
# Route-level policy with no mergeType: merges into the listener-level policy using the
76+
# gateway-level defaultChildMergeType.
77+
- apiVersion: gateway.envoyproxy.io/v1alpha1
78+
kind: BackendTrafficPolicy
79+
metadata:
80+
namespace: default
81+
name: policy-for-route
82+
spec:
83+
targetRef:
84+
group: gateway.networking.k8s.io
85+
kind: HTTPRoute
86+
name: httproute-1
87+
connection:
88+
bufferLimit: 100M

0 commit comments

Comments
 (0)