feat(irtranslator): attach xRoute source metadata to Envoy routes#13620
Conversation
Signed-off-by: Mihir Dixit <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds standard Envoy route filter_metadata identifying the originating Gateway API xRoute (and rule) to improve observability/debuggability (tracing correlation, access-log filtering) across generated routes.
Changes:
- Introduces
addRouteSourceMetadataandrouteSourceMetadataKeyto attachio.kgateway.route_sourcefilter metadata onto Envoy routes. - Hooks metadata attachment into route generation (
envoyRoutes) prior to route plugin execution. - Adds unit coverage for the metadata helper and refreshes goldens to include the new metadata.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/kgateway/translator/irtranslator/policy.go | Adds helper + key constant to populate per-route filter_metadata describing route source. |
| pkg/kgateway/translator/irtranslator/route.go | Calls helper during Envoy route construction so metadata is attached consistently. |
| pkg/kgateway/translator/irtranslator/route_test.go | Adds table-driven tests validating metadata behavior (including preservation of existing metadata). |
You can also share your feedback on Copilot code review. Take the survey.
Signed-off-by: Mihir Dixit <[email protected]>
374a7fe to
3b22fcc
Compare
Signed-off-by: Mihir Dixit <[email protected]>
bb07225 to
08dacc4
Compare
andy-fong
left a comment
There was a problem hiding this comment.
I am not ultimately familiar with these but on search for envoyroutev3 on the codebase, a couple areas comes up that might need further investigation:
- in this route.go, there is a case for setFallBackConfig(), do we need metadata there?
- in the directresponse plugin, it also seems to generate route, check and test if we need metadata there as well.
|
Also, can you test out what is the output if merging of route happens? How does the metadata look like? |
- Change route source metadata key prefix from io.kgateway to dev.kgateway - Add comments explaining why setFallBackConfig and directresponse plugins currently do not inject route metadata Signed-off-by: mihir-dixit2k27 <[email protected]>
Tested this. When two route rules merge into a single Envoy route, metadata.FilterMetadata[routeSourceMetadataKey] = &structpb.Struct{Fields: fields}the last rule processed wins. Actual test output: {
"filter_metadata": {
"dev.kgateway.route_source": {
"group": "gateway.networking.k8s.io",
"kind": "HTTPRoute",
"name": "test-route",
"namespace": "default",
"rule": "rule2-merged"
}
}
}So if |
Gate addRouteSourceMetadata behind a KGW_ROUTE_SOURCE_METADATA_ENABLED environment variable (default: false). This makes the dev.kgateway.route_source filter metadata attachment opt-in, addressing reviewer feedback on PR kgateway-dev#13620. Changes: - api/settings/settings.go: add RouteSourceMetadataEnabled bool field - pkg/kgateway/translator/irtranslator/route.go: gate addRouteSourceMetadata call - pkg/kgateway/translator/irtranslator/gateway.go: thread flag through Translator struct - pkg/kgateway/translator/translator.go: wire setting from Settings - install/helm/kgateway/values.yaml: add routeSourceMetadataEnabled: false - install/helm/kgateway/templates/deployment.yaml: add KGW_ROUTE_SOURCE_METADATA_ENABLED env var No golden files changed as the flag is off by default. Signed-off-by: Mihir Dixit <[email protected]>
andy-fong
left a comment
There was a problem hiding this comment.
@mihir-dixit2k27 Thank you again for the contribution. Can you add translator test to this now that we don't have to modify all the golden file.
TestRouteSourceMetadataFlag covers both states of routeSourceMetadataEnabled without modifying any golden files: - off (default): no dev.kgateway.route_source key present in filter metadata - on: all five fields (kind, group, name, namespace, rule) are populated The test replicates the flag gate in envoyRoutes() directly, so it runs without a fully-wired GatewayIR or backend list. Signed-off-by: Mihir Dixit <[email protected]>
Signed-off-by: mihir-dixit2k27 <[email protected]>
The function and its constant were dropped when main was merged into the branch. Also restores TestAddRouteSourceMetadata and TestRouteSourceMetadataFlag which cover the helper and flag gate. Signed-off-by: Mihir Dixit <[email protected]>
TestEnvVarCoverage enforces that every Settings field has a non-default value in allEnvVarsSet(). Add KGW_ROUTE_SOURCE_METADATA_ENABLED and wire RouteSourceMetadataEnabled into the all-values-set expected struct. Signed-off-by: Mihir Dixit <[email protected]>
b93bfe2 to
9f746fd
Compare
Signed-off-by: mihir-dixit2k27 <[email protected]>
Ready for re-review @andy-fong |
TestAddRouteSourceMetadata and TestRouteSourceMetadataFlag were dropped during a main merge conflict resolution, leaving three imports unused and causing both lint and unit test failures. Restore both test functions on top of the current branch state which already contains all main changes. Signed-off-by: Mihir Dixit <[email protected]>
…ml merge conflict Upstream main added KGW_ENABLE_AWS_EC2_DISCOVERY and KGW_AWS_EC2_REFRESH_INTERVAL in the same location as our KGW_ROUTE_SOURCE_METADATA_ENABLED, causing a GitHub PR merge conflict. Include both sets of changes and add the corresponding values.yaml defaults. Signed-off-by: Mihir Dixit <[email protected]>
28953c2 to
ef790b1
Compare
Signed-off-by: mihir-dixit2k27 <[email protected]>
…route-metadata-only Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
|
re: will route replacement carry metadata? It depends
When a single route is invalid (bad builtin filter, invalid config, validateRoutePreEnvoy
When a vhost-plugin fails, strict batch validation fails, or route-config plugins fail This is correct because the fallback route isn't produced by any xRoute — it's a synthetic error route that kgateway emits because a listener/vhost/RC-level policy failed |
Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
Signed-off-by: omar <[email protected]>
Description
Motivation: Envoy routes generated by kgateway currently carry no
information about the originating xRoute resource. This makes
debugging, distributed tracing correlation, and access-log filtering
against route origin difficult. This was requested in #[issue number].
What changed:
irtranslator/policy.go— addedaddRouteSourceMetadatahelper androuteSourceMetadataKeyconstant. The helper attaches adev.kgateway.route_sourcefilter metadata entry to every Envoy routecarrying the originating xRoute's
kind,group,name,namespace, and rulename. All fields are nil-safe — empty stringsand nil Parents are no-ops.
irtranslator/route.go— callsaddRouteSourceMetadatainsideenvoyRoutesafterinitRoutes, before plugin execution, so everygenerated route gets metadata regardless of which plugins run.
irtranslator/route_test.go— addedTestAddRouteSourceMetadatawith table-driven cases: full metadata, missing kind/rule fields, nil
Parent (no-op), and empty rule name (field omitted).
Golden files updated via
REFRESH_GOLDEN=trueto include the newmetadata in translator snapshot output.
Output example — new metadata on a generated Envoy route:
Related issues: Fixes #13563
Phase scope: This is Phase 1 — automatic source metadata only.
User-defined custom metadata via
TrafficPolicyor annotations isexplicitly out of scope and can build on this foundation.
Change Type
Changelog
Additional Notes
TestDiscoveryNamespaceSelectorhas a pre-existing flake onmainunrelated to these changes — verified by running the test suite on
main before this branch was created.
structpbandenvoycorev3are already transitive dependencies in
go.mod.dev.kgateway.route_sourcefollows the existingfilter metadata namespace convention used elsewhere in the codebase.