Skip to content

Commit 0e3e12d

Browse files
H34rtUnd3rB14d3H34rtUnd3rB14d3
committed
refactor: nest policy defaults under policyDefaults with per-policy MergeSettings
Signed-off-by: Maksim Kuchkovskiy <K.Maksim.E@yandex.ru>
1 parent f71b766 commit 0e3e12d

24 files changed

Lines changed: 326 additions & 195 deletions

‎api/v1alpha1/envoyproxy_types.go‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,24 +217,35 @@ type EnvoyProxySpec struct {
217217
// +optional
218218
MergeType *MergeType `json:"mergeType,omitempty"`
219219

220-
// BackendTrafficPolicy defines defaults applied to BackendTrafficPolicy resources
221-
// attached to Gateways that use this EnvoyProxy.
220+
// PolicyDefaults defines defaults applied to Envoy Gateway policies attached to
221+
// Gateways that use this EnvoyProxy.
222222
// +optional
223-
BackendTrafficPolicy *PolicyDefaults `json:"backendTrafficPolicy,omitempty"`
223+
PolicyDefaults *PolicyDefaults `json:"policyDefaults,omitempty"`
224224
}
225225

226-
// PolicyDefaults defines default settings shared by Envoy Gateway xPolicies (e.g. BackendTrafficPolicy)
227-
// attached to Gateways that use this EnvoyProxy.
226+
// PolicyDefaults defines defaults applied to Envoy Gateway policies, keyed by policy kind.
228227
type PolicyDefaults struct {
229-
// DefaultMergeType is the mergeType used for a policy that does not set one,
228+
// BackendTrafficPolicy defines defaults applied to BackendTrafficPolicy resources.
229+
// +optional
230+
BackendTrafficPolicy *BackendTrafficPolicyDefaults `json:"backendTrafficPolicy,omitempty"`
231+
}
232+
233+
// BackendTrafficPolicyDefaults defines defaults applied to BackendTrafficPolicy resources.
234+
type BackendTrafficPolicyDefaults struct {
235+
MergeSettings `json:",inline"`
236+
}
237+
238+
// MergeSettings defines how an Envoy Gateway policy that does not set a mergeType is merged by default.
239+
type MergeSettings struct {
240+
// MergeType is the mergeType applied to a policy that does not set one,
230241
// so a route-level policy merges into its parent instead of replacing it.
231242
// +kubebuilder:validation:Enum=StrategicMerge;JSONMerge
232243
// +optional
233-
DefaultMergeType *MergeType `json:"defaultMergeType,omitempty"`
244+
MergeType *MergeType `json:"mergeType,omitempty"`
234245

235-
// ExcludeLabel, when present on a policy, opts that policy out of DefaultMergeType.
246+
// MergeExcludeLabel, when present on a policy, opts that policy out of the default MergeType.
236247
// +optional
237-
ExcludeLabel *string `json:"excludeLabel,omitempty"`
248+
MergeExcludeLabel *string `json:"mergeExcludeLabel,omitempty"`
238249
}
239250

240251
// EnvoyProxyGeoIP defines shared GeoIP provider settings for EnvoyProxy.

‎api/v1alpha1/validation/envoygateway_validate.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,16 @@ func ValidateEnvoyGateway(eg *egv1a1.EnvoyGateway) error {
8282
// enforced by CRD validation for EnvoyProxy resources but not when the spec is provided inline as
8383
// the EnvoyGateway default, since that path does not go through CRD admission.
8484
func validateEnvoyGatewayDefaultEnvoyProxy(spec *egv1a1.EnvoyProxySpec) error {
85-
if spec == nil || spec.BackendTrafficPolicy == nil || spec.BackendTrafficPolicy.DefaultMergeType == nil {
85+
if spec == nil || spec.PolicyDefaults == nil || spec.PolicyDefaults.BackendTrafficPolicy == nil ||
86+
spec.PolicyDefaults.BackendTrafficPolicy.MergeType == nil {
8687
return nil
8788
}
88-
switch *spec.BackendTrafficPolicy.DefaultMergeType {
89+
switch *spec.PolicyDefaults.BackendTrafficPolicy.MergeType {
8990
case egv1a1.StrategicMerge, egv1a1.JSONMerge:
9091
return nil
9192
default:
92-
return fmt.Errorf("envoyProxy.backendTrafficPolicy.defaultMergeType must be one of StrategicMerge or JSONMerge, got %q",
93-
*spec.BackendTrafficPolicy.DefaultMergeType)
93+
return fmt.Errorf("envoyProxy.policyDefaults.backendTrafficPolicy.mergeType must be one of StrategicMerge or JSONMerge, got %q",
94+
*spec.PolicyDefaults.BackendTrafficPolicy.MergeType)
9495
}
9596
}
9697

‎api/v1alpha1/validation/envoygateway_validate_test.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,29 +1055,33 @@ func TestValidateEnvoyGateway(t *testing.T) {
10551055
expect: true,
10561056
},
10571057
{
1058-
name: "default EnvoyProxy with valid backendTrafficPolicy.defaultMergeType",
1058+
name: "default EnvoyProxy with valid backendTrafficPolicy.mergeType",
10591059
eg: &egv1a1.EnvoyGateway{
10601060
EnvoyGatewaySpec: egv1a1.EnvoyGatewaySpec{
10611061
Gateway: egv1a1.DefaultGateway(),
10621062
Provider: egv1a1.DefaultEnvoyGatewayProvider(),
10631063
EnvoyProxy: &egv1a1.EnvoyProxySpec{
1064-
BackendTrafficPolicy: &egv1a1.PolicyDefaults{
1065-
DefaultMergeType: new(egv1a1.StrategicMerge),
1064+
PolicyDefaults: &egv1a1.PolicyDefaults{
1065+
BackendTrafficPolicy: &egv1a1.BackendTrafficPolicyDefaults{MergeSettings: egv1a1.MergeSettings{
1066+
MergeType: new(egv1a1.StrategicMerge),
1067+
}},
10661068
},
10671069
},
10681070
},
10691071
},
10701072
expect: true,
10711073
},
10721074
{
1073-
name: "default EnvoyProxy with invalid backendTrafficPolicy.defaultMergeType",
1075+
name: "default EnvoyProxy with invalid backendTrafficPolicy.mergeType",
10741076
eg: &egv1a1.EnvoyGateway{
10751077
EnvoyGatewaySpec: egv1a1.EnvoyGatewaySpec{
10761078
Gateway: egv1a1.DefaultGateway(),
10771079
Provider: egv1a1.DefaultEnvoyGatewayProvider(),
10781080
EnvoyProxy: &egv1a1.EnvoyProxySpec{
1079-
BackendTrafficPolicy: &egv1a1.PolicyDefaults{
1080-
DefaultMergeType: new(egv1a1.Replace),
1081+
PolicyDefaults: &egv1a1.PolicyDefaults{
1082+
BackendTrafficPolicy: &egv1a1.BackendTrafficPolicyDefaults{MergeSettings: egv1a1.MergeSettings{
1083+
MergeType: new(egv1a1.Replace),
1084+
}},
10811085
},
10821086
},
10831087
},

‎api/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 47 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -236,24 +236,6 @@ spec:
236236
<= {"1.0":1,"1.1":2,"1.2":3,"1.3":4,"Auto":5}[self.maxVersion]
237237
: !has(self.minVersion) && has(self.maxVersion) ? 3 <= {"1.0":1,"1.1":2,"1.2":3,"1.3":4,"Auto":5}[self.maxVersion]
238238
: true'
239-
backendTrafficPolicy:
240-
description: |-
241-
BackendTrafficPolicy defines defaults applied to BackendTrafficPolicy resources
242-
attached to Gateways that use this EnvoyProxy.
243-
properties:
244-
defaultMergeType:
245-
description: |-
246-
DefaultMergeType is the mergeType used for a policy that does not set one,
247-
so a route-level policy merges into its parent instead of replacing it.
248-
enum:
249-
- StrategicMerge
250-
- JSONMerge
251-
type: string
252-
excludeLabel:
253-
description: ExcludeLabel, when present on a policy, opts that
254-
policy out of DefaultMergeType.
255-
type: string
256-
type: object
257239
bootstrap:
258240
description: |-
259241
Bootstrap defines the Envoy Bootstrap as a YAML string.
@@ -812,6 +794,29 @@ spec:
812794
- StrategicMerge
813795
- JSONMerge
814796
type: string
797+
policyDefaults:
798+
description: |-
799+
PolicyDefaults defines defaults applied to Envoy Gateway policies attached to
800+
Gateways that use this EnvoyProxy.
801+
properties:
802+
backendTrafficPolicy:
803+
description: BackendTrafficPolicy defines defaults applied to
804+
BackendTrafficPolicy resources.
805+
properties:
806+
mergeExcludeLabel:
807+
description: MergeExcludeLabel, when present on a policy,
808+
opts that policy out of the default MergeType.
809+
type: string
810+
mergeType:
811+
description: |-
812+
MergeType is the mergeType applied to a policy that does not set one,
813+
so a route-level policy merges into its parent instead of replacing it.
814+
enum:
815+
- StrategicMerge
816+
- JSONMerge
817+
type: string
818+
type: object
819+
type: object
815820
preserveRouteOrder:
816821
description: |-
817822
PreserveRouteOrder determines if the order of matching for HTTPRoutes is determined by Gateway-API

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

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -235,24 +235,6 @@ spec:
235235
<= {"1.0":1,"1.1":2,"1.2":3,"1.3":4,"Auto":5}[self.maxVersion]
236236
: !has(self.minVersion) && has(self.maxVersion) ? 3 <= {"1.0":1,"1.1":2,"1.2":3,"1.3":4,"Auto":5}[self.maxVersion]
237237
: true'
238-
backendTrafficPolicy:
239-
description: |-
240-
BackendTrafficPolicy defines defaults applied to BackendTrafficPolicy resources
241-
attached to Gateways that use this EnvoyProxy.
242-
properties:
243-
defaultMergeType:
244-
description: |-
245-
DefaultMergeType is the mergeType used for a policy that does not set one,
246-
so a route-level policy merges into its parent instead of replacing it.
247-
enum:
248-
- StrategicMerge
249-
- JSONMerge
250-
type: string
251-
excludeLabel:
252-
description: ExcludeLabel, when present on a policy, opts that
253-
policy out of DefaultMergeType.
254-
type: string
255-
type: object
256238
bootstrap:
257239
description: |-
258240
Bootstrap defines the Envoy Bootstrap as a YAML string.
@@ -811,6 +793,29 @@ spec:
811793
- StrategicMerge
812794
- JSONMerge
813795
type: string
796+
policyDefaults:
797+
description: |-
798+
PolicyDefaults defines defaults applied to Envoy Gateway policies attached to
799+
Gateways that use this EnvoyProxy.
800+
properties:
801+
backendTrafficPolicy:
802+
description: BackendTrafficPolicy defines defaults applied to
803+
BackendTrafficPolicy resources.
804+
properties:
805+
mergeExcludeLabel:
806+
description: MergeExcludeLabel, when present on a policy,
807+
opts that policy out of the default MergeType.
808+
type: string
809+
mergeType:
810+
description: |-
811+
MergeType is the mergeType applied to a policy that does not set one,
812+
so a route-level policy merges into its parent instead of replacing it.
813+
enum:
814+
- StrategicMerge
815+
- JSONMerge
816+
type: string
817+
type: object
818+
type: object
814819
preserveRouteOrder:
815820
description: |-
816821
PreserveRouteOrder determines if the order of matching for HTTPRoutes is determined by Gateway-API

‎internal/gatewayapi/backendtrafficpolicy.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,26 +1080,27 @@ func (t *Translator) effectiveMergeType(policy *egv1a1.BackendTrafficPolicy, ep
10801080
if policy.Spec.MergeType != nil {
10811081
return policy.Spec.MergeType
10821082
}
1083-
if ep == nil || ep.Spec.BackendTrafficPolicy == nil || ep.Spec.BackendTrafficPolicy.DefaultMergeType == nil {
1083+
if ep == nil || ep.Spec.PolicyDefaults == nil || ep.Spec.PolicyDefaults.BackendTrafficPolicy == nil ||
1084+
ep.Spec.PolicyDefaults.BackendTrafficPolicy.MergeType == nil {
10841085
return nil
10851086
}
10861087
if policy.Namespace == t.ControllerNamespace {
10871088
return nil
10881089
}
1089-
d := ep.Spec.BackendTrafficPolicy
1090-
if label := ptr.Deref(d.ExcludeLabel, ""); label != "" {
1090+
d := ep.Spec.PolicyDefaults.BackendTrafficPolicy
1091+
if label := ptr.Deref(d.MergeExcludeLabel, ""); label != "" {
10911092
if _, ok := policy.Labels[label]; ok {
10921093
return nil
10931094
}
10941095
}
1095-
// Defense in depth: the CRD enum restricts DefaultMergeType to StrategicMerge/JSONMerge, but the
1096+
// Defense in depth: the CRD enum restricts MergeType to StrategicMerge/JSONMerge, but the
10961097
// EnvoyGateway default EnvoyProxySpec is not subject to CRD validation. Ignore anything that is
10971098
// not a real merge so a stray value (e.g. Replace) can never produce a "merged" status while
10981099
// actually replacing the parent.
1099-
if *d.DefaultMergeType != egv1a1.StrategicMerge && *d.DefaultMergeType != egv1a1.JSONMerge {
1100+
if *d.MergeType != egv1a1.StrategicMerge && *d.MergeType != egv1a1.JSONMerge {
11001101
return nil
11011102
}
1102-
return d.DefaultMergeType
1103+
return d.MergeType
11031104
}
11041105

11051106
// anyGatewayMergeDefault reports whether any of the route's parent gateways supplies a default

0 commit comments

Comments
 (0)