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

architecture: add ztunnel.md #46472

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Conversation

howardjohn
Copy link
Member

@howardjohn howardjohn commented Aug 10, 2023

This introduces a new doc giving an overview of ztunnel. This is all around in various places, but spread out through like 20 design docs...

@howardjohn howardjohn requested review from a team as code owners August 10, 2023 23:50
@howardjohn howardjohn added the release-notes-none Indicates a PR that does not require release notes. label Aug 10, 2023
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 10, 2023
architecture/ambient/ztunnel.md Show resolved Hide resolved
architecture/ambient/ztunnel.md Show resolved Hide resolved
architecture/ambient/ztunnel.md Show resolved Hide resolved
architecture/ambient/ztunnel.md Outdated Show resolved Hide resolved
architecture/ambient/ztunnel.md Outdated Show resolved Hide resolved
architecture/ambient/ztunnel.md Outdated Show resolved Hide resolved
architecture/ambient/ztunnel.md Outdated Show resolved Hide resolved
architecture/ambient/ztunnel.md Show resolved Hide resolved
Copy link
Member

@linsun linsun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as a first pass, great work John!

@howardjohn
Copy link
Member Author

/retest

@keithmattix
Copy link
Contributor

/retest


This document provides an overview of the architecture and design decisions around Ztunnel, the node-proxy component in ambient mode.

## Background and motivation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may help new folks if there was a bit more of a description of the ambient architecture, so they know where the ztunnel fits. This section motivates some of the reasoning, but when we get to the Goals section, I don't think it will be clear to new folks how it fits in. I think explaining the secure overlay and L7 enforcement layers may make it clearer--but perhaps there should just be a top-level doc in this directory that covers that...

I'm happy to suggest some edits or do a follow-on PR, if you agree.


TODO: fill in implementation details of how redirection is actually implemented.

## HBONE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all good information. But since sidecars also support HBONE, should we create a separate doc and then point to that from here?

Copy link
Member

@justinpettit justinpettit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for writing this--it's helpful. I was a little surprised to see the file living here, since there's a separate repo for Ztunnel. Is the way to think about it that Ztunnel is something generic, and this is how Istio ambient uses it? Regardless, it may be useful to provide a pointer to the other repo in this doc to clarify where it lives.

@howardjohn
Copy link
Member Author

ping @istio/wg-networking-maintainers-pilot @istio/wg-networking-maintainers-ztunnel

@istio-testing istio-testing merged commit 481d116 into istio:master Sep 13, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes-none Indicates a PR that does not require release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants