Skip to content
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

Expression/ATC router in KIC #3730

Closed
1 of 3 tasks
mflendrich opened this issue Mar 15, 2023 · 2 comments
Closed
1 of 3 tasks

Expression/ATC router in KIC #3730

mflendrich opened this issue Mar 15, 2023 · 2 comments
Assignees
Milestone

Comments

@mflendrich
Copy link
Contributor

mflendrich commented Mar 15, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Problem Statement

KIC today can work with Kong's traditional and traditional-compatible router flavors today.
In particular, (KIC at the moment of writing - 2.9.0-rc.1) does not work with the expression/atc router flavor.
This issue tracks the design phase of supporting the atc router flavor in KIC.

Proposed Solution

One possible implementation known to us today:

Abstract the part of the translate logic that builds Route objects away.
Introduce a second implementation (alongside the existing one that yields Route match criteria in Route fields) that builds the expression.

Additional information

Sub-issues:

Implement translators

Acceptance Criteria

  • A design doc for the migration path towards the atc router exists (introducing the alternative implementation, switching over, likely also phasing old the old one)
  • A set of well-defined issues exists that track the implementation work based on the design doc above. These issues have well-defined acceptance criteria.
@rainest
Copy link
Contributor

rainest commented Mar 29, 2023

Functionality without expression support planned

There are several features we do not intend to support with expressions or may skip if possible:

  • Expressions should not support KongIngress override fields. All these are supported via annotations and KongIngress is deprecated. We can reasonably say that you need to migrate off KongIngress to use the new system.
  • The gateway does not support providing your own expression routes when using traditional_compatible. This would prevent us from adding translators piecemeal. We wish to determine if gateway could support this, since it's ostensibly performing its own internal conversion to expressions and should theoretically be able to apply the expression without that translation step.
  • We may wish to skip support for TCPIngress and UDPIngress in favor of supporting only the GWAPI equivalents. However, this is difficult if we cannot gateway support for using mixed expression and traditional routes. If not, however, these translators are hopefully simple to implement, as they have simple rules with limited available routing criteria.
  • Ditto Knative, but we'd need to have a plan to deprecate it (there will be no GWAPI replacement).
  • Ditto Ingress V1Beta1. No need to support this at all--like KongIngress, we plan to just require upgrading to v1.
  • No support for non-combined routes. This is kind of a moot point since expressions lack the inherent limitations that originally made non-combined routes simpler to implement.

Split expression building for rules and annotations

The way we split route construction between the translators and annotation system introduces some complication to expression building. Rather than having a dedicated field we can set independently for, say, methods, we need to combine a matcher we built earlier with additional criteria.

This should be fine (though we do need to keep the original matcher around, not convert it to a string immediately), since annotation criteria must apply to the entire route resource, whereas the translators build off rules within it, and can AND the annotation criteria with an ORed set of rule expressions.

You'd get something like

((method == GET || method == POST) AND (path == /AAA || path == /BBB))

Implementation strategy

To implement translation to expressions, Tao and I developed two approaches in our PoCs:

I built a partial implementation that builds matchers (the struct addedin #3780) immediately, and composes them over time as they are returned up the stack and passed on to further steps. In the example, it builds an ORed set of each HTTPRouteRoute in a rule, and would then OR each compatible (same service, etc.) rule together, and would later AND each set of rules with any annotation criteria (the latter two of these steps still need to be built).

Tao built an additional intermediate MatchRule struct that can then build and combine predicates and matchers at the end of the route generation process.

Our initial thoughts are that directly building the match expressions is a bit simpler (we do not have to manage both translation to the intermediate and from the intermediate to matchers), but would present a problem if we found something that required modifying matchers within the tree. Although we're not converting to strings immediately, once we've returned a complete matcher from some part of the path, we lose the original source structure that resulted in it. We need further experimentation to determine if this is something we would need to do in practice: something like annotations do not need to apply piecemeal to only a subset of rules sourced from an Ingress.

@mheap mheap added the epic label May 5, 2023
@mheap mheap changed the title Spike: Expression/ATC router in KIC Expression/ATC router in KIC May 5, 2023
@mflendrich
Copy link
Contributor Author

Closing this issue - KIC 2.10 is shipping basic feature-gated expression router support for L7 routes; further versions will take care of setting priorities (#3958), L4 proxying (blocked by Gateway not supporting stream listeners in ATC router mode) and graduation to GA.

@mflendrich mflendrich closed this as not planned Won't fix, can't repro, duplicate, stale May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants