feat: mergeType in envoy extension policy#9047
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9047 +/- ##
==========================================
- Coverage 74.78% 74.77% -0.02%
==========================================
Files 252 252
Lines 40567 40696 +129
==========================================
+ Hits 30340 30430 +90
- Misses 8157 8190 +33
- Partials 2070 2076 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
gateway/internal/gatewayapi/envoyextensionpolicy.go
Lines 542 to 544 in a31dda1
Reject mergeType on Gateway-targeted policies
The new API field documents that mergeType is only valid for route-targeted EnvoyExtensionPolicies, but Gateway/Listener-targeted policies are still translated normally with no validation gate. In practice, a misconfigured policy with mergeType + Gateway target will be accepted and applied instead of being marked invalid, which violates the API contract and makes configuration errors hard to detect.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return "" | ||
| } | ||
|
|
||
| func getOverriddenTargetsMessageForGateway( |
There was a problem hiding this comment.
now, this function isn't used by anyplace.
|
/retest |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a31dda10c0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // This field cannot be set when targeting a parent resource (Gateway). | ||
| // If unset, no merging occurs, and only the most specific configuration takes effect. | ||
| // | ||
| // +kubebuilder:validation:XValidation:rule="self != 'Replace'",message="Replace is not a valid MergeType for EnvoyExtensionPolicy" |
There was a problem hiding this comment.
The spec comment added for MergeType says it "cannot be set when targeting a parent resource (Gateway)", but the only validation here is self != 'Replace', so Gateway-targeted EnvoyExtensionPolicy objects with mergeType: StrategicMerge/JSONMerge are still admitted. At runtime those policies are processed by processEnvoyExtensionPolicyForGateway without any MergeType guard (so merge intent is effectively ignored), which creates a silent misconfiguration path for users. Please add a CEL/runtime validation that forbids mergeType when any target ref resolves to kind: Gateway.
Useful? React with 👍 / 👎.
|
what will happen if I have multiple EEPs target to a same HTTPRoute? |
currently, we don't support merging same resource level in all policies. |
|
zirain and I are discussing about future merge logic in slack. |
2 participants
What this PR does / why we need it:
Support
mergeTypein EEP.Before implementing #8092 , I'd like to support this.
Which issue(s) this PR fixes:
Fixes #6733
Release Notes: Yes