Unify directional HTTP route policies in config v2#2680
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2680 +/- ##
==========================================
- Coverage 69.88% 69.74% -0.14%
==========================================
Files 359 360 +1
Lines 51142 51535 +393
==========================================
+ Hits 35739 35945 +206
- Misses 13172 13341 +169
- Partials 2231 2249 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mariomac
left a comment
There was a problem hiding this comment.
Looks great! I wonder if there is a way to simplify the code:
- Remove the code from old, non-directional route decorator (
provideRoutes) and keep only the directional route decoratorprovideDirectionalRoutes - If the user defines the non-directional routing patterns, copy them to both
IncomingandOutgoingconfiguration so the directional routing code will behave as the old, global, non-directional route decorator.
Give global and per-service HTTP route settings one directional policy shape so incoming and outgoing traffic can configure patterns, ignored routes, and unmatched behavior consistently. Preserve the v1 configuration surface while translating its flat global policy into both v2 directions. Document inheritance and migration semantics and cover conversion and runtime precedence with tests.
Turn valid include-rule route refinements into an active directional route configuration even when global policies are omitted. This allows per-service route and ignore settings to reach request processing while leaving complete configurations without global policies inactive. Cover partial and complete v2 inputs to preserve inherited defaults and the explicit unset baseline.
Directional route policies belong to the HTTP configuration surface. Applying them to every protocol allowed broad ignored patterns to suppress unrelated gRPC and database telemetry. Return before policy selection for non-HTTP events and cover incoming and outgoing protocols with regression assertions.
Rule-only v2 route refinements need a baseline for inheritance without turning that baseline into a global policy. Treating the baseline as global allowed harvested routes to appear on unrelated services and directions. Record the rule-only state through conversion, attach compiled policies only where overrides exist, and preserve the absence of global policies during export.
Complete v2 imports now build global policies without inheriting v1 runtime defaults. They also retain direction presence so omitted policies stay inactive through service discovery, routing, and v2 export. Per-service refinements can still activate a direction without a global policy, preserving the intended override behavior.
Rule policy activation previously rebuilt the runtime route config and discarded which global directions were present. An omitted direction then became active whenever any per-service route refinement existed. Retain the existing config while installing the directional baseline so global absence survives discovery, routing, and v2 export.
Treat a zero maximum as disabling trie-based collapsing, matching the documented configuration contract. Cover both the trie itself and directional route processing so future policy changes preserve uncapped routes.
Keep global matchers on the router and attach compiled policies only when a service rule overrides that direction. Retain independent mutable tries for enabled low-cardinality policies so one service cannot consume another service's cardinality budget.
Require empty or one nonzero ASCII byte in both converter validation and the published schema, matching the byte-based route classifier. Reject Unicode and NUL values before policy construction and document the supported surface.
91793c4 to
ca8e20a
Compare
|
Thanks, Mario. I agree that a single directional path would be a cleaner end state. I’m hesitant to take it in this PR. The non-directional decorator exists solely for config v1 compatibility, and the plan is to retire that surface once v2 has proven stable. Converting legacy configuration into the directional representation now would add a transient compatibility layer and broaden this into a v1 behavioral-parity exercise—particularly around service-level routing precedence—only to remove it with the v1 path. I’d keep this PR focused on making v2 correct and stable, then delete the non-directional implementation as part of the v1 sunset. If you think it is worth retaining the idea, I’m happy to open a follow-up issue so we can revisit it if the v1 support horizon changes or the duplicate logic causes a concrete maintenance issue. Let me know what you think. |
|
Ok @MrAlias your strategy sounds reasonable to me. |
Bring the pull request onto the current base so it includes the receiver test correction and all intervening repository changes. This creates a fresh pull request revision for CI instead of rerunning the stale merge.
Bring this pull request up to date with the latest base branch changes so it can be tested and reviewed against the current codebase.
Use the unset unmatched-route baseline for every direction without a global v2 policy. This prevents pattern-only service refinements from falling back to the legacy wildcard behavior. Cover the mixed global and per-service case so partial directional configuration stays consistent with rule-only routing.
Closes #747.
Summary
Configuration semantics
Per-service route policies override global policies field by field. Omitted fields inherit the global value, while explicitly configured arrays replace the inherited array, including an empty array that clears it. Route evaluation applies ignored patterns first, then configured or discovered routes, then unmatched-route behavior.
Validation
make generatemake check-config-schemamake check-config-v2-artifactsmake lint-markdownpython3 devdocs/config/version-2.0/validate_example.pygo test -race ./...golangci-lintacross every modified Go package