proposal: extensions

[yuval-k]

Allow extending the behaviour of ztunnel by invoking it as a library with extensions.

[istio-testing]

Hi @yuval-k. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[dhawton]

/ok-to-test

[yuval-k]

/retest

[yuval-k]

/retest

[yuval-k]

/retest

[yuval-k]

/retest

[ymesika]

You might want to use the logging macros instead

[ymesika]

I think it should be placed after the match below

[yuval-k]

this part of was moved as is from main.rs

[howardjohn]

I think with any extensibility mechanism, we need:

• Very very clear and explicit bounds, to avoid this creeping from 4 callbacks to 400
• Multiple usages by different vendors/users that validate the abstraction picked is correct

So far I have chatted with 3 folks about this (including you and myself), and 2/3 of them would not be supported by this model. Its perfectly fine to have folks need customization beyond what extensibility we provide out of the box - there will always be folks with super bespoke use cases that we can't and should not bend over backwards to meet.

However, I would like to at least see some other user that could implement there desired extensions via this mechanism. If not, it may make sense to wait until we have reached that point before prematurely merging anything.

I do think in general this seems like a plausible extension mechanism, though

[linsun]

@howardjohn can you explain the scenarios of extensions from 2/3 you mentioned that would not fit?

[howardjohn]

More invasive changes like different transport protocols, custom tweaks to how we do HBONE, different copying mechanisms, etc. Its a bit vague, though, for all involved I think - which is another reason not to rush things

[kyessenov]

Is the extended featureset so broad that the waypoints cannot capture all datapaths? Normally, network functions could be chained, which is essentially what waypoints do to enhance L7 processing. Why not adopt the similar model for ztunnels and run traffic through a chain of coprocessors? The protocol is sufficient to express it, and the running out of process has many flexibility benefits.

[yuval-k]

I think that some extensions (like tweaks to HBONE) will require ztunnel extensions vs out of process ones.

As far as the API itself, these extensions are not an istio user-facing api, but rather an istio vendor-facing api (similar to envoy filters in that regard).

As such, I think it's ok to treat extensions as an internal API, and allow it to be re-factored/broken without notice (as long as the same use-cases are still enabled, i.e. downstream code will still work after it adapts to the new API).

This is similar to envoy's extension development model (i.e. the stable config is the XDS protos, not Http::StreamDecoderFilter).

[kyessenov]

HBONE is the only stable specification we have here, so I'm hoping we don't make vendor-specific customizations to it.
Otherwise, having well-documented internal interfaces seems like a good practice, as long as they are not treated as an extension model.

[istio-testing]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bleggett]

(xposting some comments from #251)

1. I think to @kyessenov 's and @yuval-k's points we will need 2 kinds of integrations, of which this proposal is the latter.

   • A) Documented, end-user facing integrations - "bring your own X and we will play with it" - these are the kinds of things John is mentioning in Define complete set of integration points #251 - the list John has there makes sense, and I agree these need to be limited and formalized, as we will have to document and support them here in this repo -> we (this repo/Istio) bear the cost of supporting those integration points.

   • B) Internal interfaces/extension points around typical TCP proxy lifecycle hooks that make it easier to hack/extend/experiment with ztunnel proper, but which Istio doesn't make stability, compatibility, or functionality guarantees about and which we don't support as user-facing - if you want to use one of these you either need to fork the repo (and bear that cost yourself) or wrap ztunnel with your own code (and bear the cost of keeping in sync with upstream) -> you (vendor/entity hacking on ztunnel) bear the cost of supporting those integration points, and we document accordingly.

2. I think the best way to constrain ourselves is to keep ztunnel a relatively ignorant TCP proxy that routes packets and ignores protocols - if that is all it does, then that is a pretty effective hard limit on how it can be extended, and what we choose to allow as far as upstreamed extension points.

3. Given that this proposal is category B), none of the hooks here need to be set in stone, are subject to change as we see fit, and the onus is on vendors/users of those internal interfaces to construct/propose/shop around a more formalized/standardized proposal for A) if they don't like this state of affairs.

[costinm]

A different perspective: I think we have close to consensus that we should keep ztunnel minimal in the Istio distribution and have a stable common protocol. This will allow integrations and interop - including other implementations of the protocol, for example in proxyless or native implementations in different languages or non-istio implementations ( and outside of K8S ).

While I don't think a hook would help - having a relatively stable API in ztunnel that allows different parts to be reused is not bad. For example a rust application outside of k8s ( or on a k8s cluster where user doesn't have permissions to set ztunnel/cni) should be able to reuse the ztunnel code for connecting to XDS and getting PTR-DS, as well as the code to
add metadata.

It is not clear if such code should be ztunnel or another crate that is focused on providing a library-like API and used
in rust-based applications using HBONE and/or PTR-DS, but I don't see why someone would need to duplicate the
code in ztunnel just to provide a stable API. For other languages - separate libraries will likely be needed for full
proxyless/native ambient support.

With a stable API - it should be possible to customize and hook with far more flexibility and still use common code
for integration with the XDS server.

[bleggett]

A different perspective: I think we have close to consensus that we should keep ztunnel minimal in the Istio distribution and have a stable common protocol. This will allow integrations and interop - including other implementations of the protocol, for example in proxyless or native implementations in different languages or non-istio implementations ( and outside of K8S ).

While I don't think a hook would help...

One thing I would note is that we are already pretty near to hitting the clippy too_many_arguments lint flag in e.g. handle_inbound (one of the core functions we proposed adding an extension hook to) on master already, due to dependency injection: https://github.com/istio/ztunnel/blob/master/src/proxy/inbound.rs#L144

There are other ways to fix that specific issue of course, but ultimately I think a hook API there makes a lot more sense than piling more and more stuff into those same handlers for whatever the next ztunnel feature that comes along is - which is the current trajectory.

Arguably L4 metrics could have been implemented with some variant of the extension API proposed here. If more things like the L4 metrics implementation come along, I think we might need to revisit the "extension hooks don't simplify core handlers" argument before we proceed to jam more things into the core TCP connection handlers every time we want to add a feature that needs to hook into the TCP connection primitives.

...having a relatively stable API in ztunnel that allows different parts to be reused is not bad. For example a rust application outside of k8s ( or on a k8s cluster where user doesn't have permissions to set ztunnel/cni) should be able to reuse the ztunnel code for connecting to XDS and getting PTR-DS, as well as the code to add metadata.

Not against that - my personal stance on librarifying existing apps is "don't maintain separate libraries unless and until you have real consumers of those separate libraries" - if we have a ztunnel-external dep (or someone can make a compelling argument for why we need to split out an external lib to support one) - then let's do it.