feat: merge support for eep - #9638
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 #9638 +/- ##
==========================================
- Coverage 76.04% 75.98% -0.06%
==========================================
Files 259 259
Lines 43275 43473 +198
==========================================
+ Hits 32907 33034 +127
- Misses 8178 8236 +58
- Partials 2190 2203 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
e8f426e to
a10fbcc
Compare
|
@codex review |
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: a10fbccc56
ℹ️ 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".
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7b54392d5
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7b54392d5
ℹ️ 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".
| parentPolicy, parentScope = p, gatewayScope(gwNN) | ||
| } | ||
| } else { | ||
| ancestorRef = getAncestorRefForPolicy(gwNN, &listener.Name) |
There was a problem hiding this comment.
When a route parentRef omits sectionName, ancestorRefs above contains the Gateway ancestor without a section, but the merge branch reports merge successes and translation failures on a different listener-scoped ancestor synthesized from listener.Name here. In that common attach-to-Gateway scenario, the policy can end up with Accepted=True on the actual ancestor from SetAcceptedForPolicyAncestors while Merged or Accepted=False is recorded only under a separate listener ancestor, so users watching the route-policy ancestor miss whether the merge succeeded or failed; the merge path should update the same ancestor refs consistently.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This Issue has already existed in other policies.
so if we should fix this, I want to fix in follow-up PR.
| // MergeType determines how this configuration is merged with existing EnvoyExtensionPolicy | ||
| // configurations targeting a parent resource. When set, this configuration will be merged | ||
| // into the closest parent EnvoyExtensionPolicy in the route's attachment hierarchy (for | ||
| // example, one targeting a Gateway, Gateway listener, ListenerSet, or ListenerSet | ||
| // listener). | ||
| // Currently, this field can only be set when targeting xRoute resources. | ||
| // 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" | ||
| // +optional | ||
| MergeType *MergeType `json:"mergeType,omitempty"` |
There was a problem hiding this comment.
what will happen if there're multiple merge EEP target to same xRoute? only the first one will work?
3 participants
What this PR does / why we need it:
Support mergeType in EEP.
This PR's scope contains only supporting
mergeTypesimilar to other policies.Route policies merge Gateway or ListenerSet policies.
Which issue(s) this PR fixes:
Fixes #6733
PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.