Skip to content

ZEP-0032: support ipv6#31

Merged
AustinAbro321 merged 6 commits intozarf-dev:mainfrom
touchardv:add-ipv6-zep
Jul 8, 2025
Merged

ZEP-0032: support ipv6#31
AustinAbro321 merged 6 commits intozarf-dev:mainfrom
touchardv:add-ipv6-zep

Conversation

@touchardv
Copy link
Copy Markdown
Contributor

@touchardv touchardv commented Jun 5, 2025

  • One-line PR description: adding new ZEP
  • Other comments:

Signed-off-by: Vincent Touchard <touchardv@gmail.com>
@touchardv touchardv mentioned this pull request Jun 6, 2025
3 tasks
@touchardv touchardv changed the title ZEP-0022 Support ipv6 ZEP-0032 Support ipv6 Jun 6, 2025
Comment thread 0032-support-ipv6/zep.yaml Outdated
Signed-off-by: Vincent Touchard <touchardv@gmail.com>
@touchardv touchardv marked this pull request as ready for review June 10, 2025 19:51
@touchardv touchardv changed the title ZEP-0032 Support ipv6 ZEP-0032: support ipv6 Jun 11, 2025
Copy link
Copy Markdown
Member

@AustinAbro321 AustinAbro321 left a comment

Choose a reason for hiding this comment

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

Good start. Added some clarifying questions and requests for additional content

Comment thread 0032-support-ipv6/README.md Outdated
Comment thread 0032-support-ipv6/README.md
Comment thread 0032-support-ipv6/README.md Outdated
Comment thread 0032-support-ipv6/README.md
How will security be reviewed, and by whom?

How will UX be reviewed, and by whom?
-->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need info in this section:
Does the new daemonset introduce any additional attack vectors? Could someone shell into that container?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added some info here.

Copy link
Copy Markdown
Member

@AustinAbro321 AustinAbro321 Jun 18, 2025

Choose a reason for hiding this comment

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

Not exactly sure what include the minimal amount of binaries to prevent shell access would mean in action. Are we planning to remake the socat image with less binaries?

It's not a deal breaker if someone with the proper k8s access can shell into the container, but it is worth documenting here as a risk. Especially since this pod will need hostNetwork: true or some other way to access the host namespace.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think that there we have many options. In the initial PR we used a basic socat image to keep things simple. Any other more secured/simple container image would do fine. Also, another track that could be considered: implementing the proxy using Rust/Go (a few lines of code) and use the same mechanism as the Rust injector...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Gotcha that makes sense. Could you put your intended path under the design details section and add the alternative plan in the alternative section. I'd lean towards saying socat is adequate for the first release, but I'd like to get thoughts from @brandtkeller who is more security minded.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated 👍

Comment thread 0032-support-ipv6/README.md Outdated
Signed-off-by: Vincent Touchard <touchardv@gmail.com>
Signed-off-by: Vincent Touchard <touchardv@gmail.com>
@touchardv touchardv requested a review from AustinAbro321 June 16, 2025 16:04
Comment thread 0032-support-ipv6/README.md Outdated
make use of the proposal?
-->

In the event of a change of networking configuration in the Kubernetes cluster, the adminstrator should be able to simply re-run `zarf init` with or without the `--ipv6` command line flag.
Copy link
Copy Markdown
Member

@AustinAbro321 AustinAbro321 Jun 18, 2025

Choose a reason for hiding this comment

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

We should be more clear here. Documenting these cases help us test these paths in the implementation.

If a user with an existing cluster dual stack cluster wants their cluster to use the ipv6 setup then they can run zarf init --ipv6

If a user wants to stop using the ipv6 setup then they run zarf init without the ipv6 flag and their cluster will go back to the ipv4 setup.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

Signed-off-by: Vincent Touchard <touchardv@gmail.com>
Signed-off-by: Vincent Touchard <touchardv@gmail.com>
@touchardv touchardv requested a review from AustinAbro321 June 26, 2025 16:44
@AustinAbro321
Copy link
Copy Markdown
Member

AustinAbro321 commented Jun 27, 2025

Hey @touchardv providing an update. I've been evaluating the security risks of this solution and testing it out with different setups and clusters, still more testing to do. The proxy solution would be valuable in a broader context than just IPV6. For example, this will allow NFtables to work, and potentially could allow distros and CNIs that block the route localnet nodeport to work. We have several issues around this problem (including but not limited to zarf-dev/zarf#2383, zarf-dev/zarf#2146, zarf-dev/zarf#3684).

One idea I'm working on right now is replacing the flag --ipv6 with something like --registry-proxy or --hostport that would enable the proxy daemonset for any setup regardless if they are ipv6. We can then check the IP family of the cluster automatically. If an ipv6 only cluster tries to use zarf init without --registry-proxy we would error. I may make a PR to this proposal and your implementation. This seems to work well so far.

Really appreciate the work you've put in so far, and I'm excited to see where this goes. Expect some more updates next week.

@AustinAbro321
Copy link
Copy Markdown
Member

Hello @touchardv I've been using this as a starting point for #37. The idea being to take the solution here and apply it more broadly and more securely. For example, instead of --ipv6 the flag will be --registry-proxy. In ipv4 or dual stacks we'll use hostPort over hostNetwork for better security posture. Additionally, we're going to look at doing this with mTLS from the proxy to the registry.

Really appreciate the work you've done so far. I'm going to merge this, then create another PR that will move the status to replaced, and continue the work in #37, this way we keep the historical context

Copy link
Copy Markdown
Member

@AustinAbro321 AustinAbro321 left a comment

Choose a reason for hiding this comment

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

This is a great start. The registry proxy will likely be a more broad and long term solution than just for IPv6. I'm merging this so we have the historical record here. I'll make some updates to replace this and point it to #37 in a future PR. Work towards bringing IPv6 compatibility to Zarf will continue in that PR / ZEP.

@AustinAbro321 AustinAbro321 merged commit 72e090b into zarf-dev:main Jul 8, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants