-
Notifications
You must be signed in to change notification settings - Fork 630
policy: generalize merging API #11675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR generalizes the policy merging API by letting parent resources specify how child and parent policies should be merged (shallow vs. deep, parent- vs. child-preferred). Key changes include:
- Introduce a generic
MergePoliciesfunction in the plugin SDK with four merge strategies. - Replace delegation-specific annotations with a unified
kgateway.dev/inherited-policy-priorityannotation. - Update translators and plugins to honor the new merge strategies and add deep-merge support for transformation policies.
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/kubernetes/e2e/features/route_delegation/types.go | Register new policy_merging.yaml manifest in the e2e suite |
| test/kubernetes/e2e/features/route_delegation/testdata/policy_merging.yaml | Add test fixture covering all merge-strategy scenarios |
| test/kubernetes/e2e/features/route_delegation/suite.go | Add TestPolicyMerging to validate merge strategy behavior |
| pkg/pluginsdk/policy/merge.go | Add generic merge API (MergePolicies, GetMergeStrategy) |
| pkg/pluginsdk/ir/iface.go | Add InheritedPolicyPriority field to RouteContext |
| pkg/pluginsdk/ir/gw.go | Store inherited priority on PolicyAtt, update merge origins |
| internal/kgateway/translator/irtranslator/route.go | Build and merge policies using new hierarchical priorities |
| internal/kgateway/translator/gateway/* | Propagate MergeOptions through all built-in and trafficpolicy plugins |
| api/annotations/policy.go | Define the new InheritedPolicyPriority annotation values |
| api/annotations/delegation.go | Remove delegation-specific policy priority annotations |
| internal/kgateway/krtcollections/policy.go | Read and propagate new annotation for policy inheritance |
| .golangci.yaml | Add alias for the new api/annotations package |
Comments suppressed due to low confidence (5)
test/kubernetes/e2e/features/route_delegation/testdata/policy_merging.yaml:57
- Remove the trailing whitespace on this annotation value to avoid unnecessary diffs.
kgateway.dev/inherited-policy-priority: DeepMergePreferParent
pkg/pluginsdk/policy/merge.go:118
- Add a GoDoc comment for
MergePoliciesexplaining its purpose, parameters, and return value.
func MergePolicies[T comparable](
pkg/pluginsdk/policy/merge.go:64
- Add a GoDoc for
GetMergeStrategyto describe how it maps annotation values and the hierarchy flag to aMergeStrategy.
func GetMergeStrategy(
pkg/pluginsdk/ir/gw.go:179
- Add a comment describing how
AppendWithPrioritydiffers fromAppendand how theHierarchicalPriorityparameter affects merge ordering.
func (a *AttachedPolicies) AppendWithPriority(HierarchicalPriority int, l ...AttachedPolicies) {
pkg/pluginsdk/policy/merge.go:64
- Add unit tests for
GetMergeStrategyto cover allInheritedPolicyPriorityValuecases and verify correct merge-strategy selection.
func GetMergeStrategy(
53efe7b to
3654e62
Compare
Description - Generalizes inherited policy merging so that a resource higher in the config hierarchy can define the merge strategy to use. - 4 merging strategies are supported: ShallowMergePreferChild(default), ShallowMergePreferParent, DeepMergePreferChild, DeepMergePreferParent. - The DeepMerge* variants are only implemented for TrafficPolicy's `transformation` field. Depending on the merge strategy, the higher priority transformations are ordered first. - `policy.MergePolicies` implements a generic variant for merging policies within the same hierarchy, and then across hierarchies. - Implements policy inheritance for builtin policies, i.e. a parent route's inherited policy priority is honored unlike before. - Uses the generic `kgateway.dev/inherited-policy-priority` annotation and removes the delegation specific annotation. - Makes ShallowMergePreferChild the default inherited policy for delegation so that child route's policies can override parent policies by default. This is a breaking change. Change Type ``` /kind breaking_change /kind new_feature ``` Changelog ```release-note Use `kgateway.dev/inherited-policy-priority: ShallowMergePreferParent` instead of `delegation.kgateway.dev/inherited-policy-priority: PreferParent` and `kgateway.dev/inherited-policy-priority: ShallowMergePreferChild` instead of `delegation.kgateway.dev/inherited-policy-priority: PreferChild`, as annotations to define inherited policy priority for delegated routes. By default, child HTTPRoute policies take precedence over parent HTTPRoute policies for delegated routes. ``` Additional Notes Part of kgateway-dev#11642 Signed-off-by: Shashank Ram <[email protected]>
Signed-off-by: Shashank Ram <[email protected]>
lgadban
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 🎉 🎉
Description
Generalizes inherited policy merging so that a resource higher in the config hierarchy can define the merge strategy to use.
4 merging strategies are supported: ShallowMergePreferChild(default), ShallowMergePreferParent, DeepMergePreferChild, DeepMergePreferParent.
The DeepMerge* variants are only implemented for TrafficPolicy's
transformationfield. Depending on the merge strategy, the higher priority transformations are ordered first.policy.MergePoliciesimplements a generic variant for merging policies within the same hierarchy, and then across hierarchies.Implements policy inheritance for builtin policies, i.e. a parent route's inherited policy priority is honored unlike before.
Uses the generic
kgateway.dev/inherited-policy-priorityannotation and removes the delegation specific annotation.Makes ShallowMergePreferChild the default inherited policy for delegation so that child route's policies can override parent policies by default. This is a breaking change.
Change Type
Changelog
Additional Notes
Part of #11642