-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add IPv6/dual-stack support #350
Conversation
1e795ad
to
7f5007b
Compare
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Such an exciting change :D I left a few comments.
NOTE: as you're probably aware of, I have no way of testing this atm since my env's been reduced to nothingness.
proxy-init/cmd/root.go
Outdated
return cmd | ||
} | ||
|
||
// BuildFirewallConfiguration returns an iptables FirewallConfiguration suitable to use to configure iptables. | ||
func BuildFirewallConfiguration(options *RootOptions) (*iptables.FirewallConfiguration, error) { | ||
func BuildFirewallConfiguration(options *RootOptions, ipv6 bool) (*iptables.FirewallConfiguration, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm qq here: if root options already have IPv6 is it necessary to take it as a value in the function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above.
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
Followup to linkerd/linkerd2-proxy-init#350 In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next. *Do not merge yet*: We're pulling the images `ghcr.io/alpeb/proxy-init:ipv6` and `ghcr.io/alpeb/cni-plugin:ipv6` as temporary builds for linkerd/linkerd2-proxy-init#350, while that gets released.
In support of the new proxy-init flags `--iptables-mode` and `--ipv6`: - For the linkerd-control-plane chart added the values.yaml entry `enableIPv6` (defaults to true). The `proxyInit.iptablesMode` was already there, but we interpret it now slightly differently in `_proxy-init.tpl`. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `enableIPv6` (defaults to true). Note this allows routing IPv6 traffic to the proxy, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come up next.
628b5ed
to
94489ef
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a quick comment on how I think we could refactor the options. We could copy it by dereferencing the value. I put together an example to illustrate what I mean:
Hey @mateiidavid thanks for the detailed review 🌮 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for incorporating the feedback. I'm super happy with how this turned out and excited to get it going :D
## Flags Changes This replaces the proxy-init flags `--firewall-bin-path` and `--firewall-save-bin-path` with the flag `--iptables-mode` (with possible values `legacy` and `nft`). Also the `--ipv6` flag has been added (default `true`). Proxy-init won't be relying just on the iptables commands family (iptables-legacy, iptables-legacy-save, iptables-nft, iptable-nft-save), but also on the ip6tables command family, so it's better to know the mode we're in (legacy or nft) and whether ipv6 is enabled, to determine all the commands that need to be used instead of directly passing them as arguments. After the set of rules run via iptables are processed, if `--ipv6` is true (which is the default), the same set of rules will be run via ip6tables. Analog changes were applied to linkerd-cni as well. ## Backwards-Compatibility This is backwards-compatible with older control planes as long as the older flags are not used. If those flags are used, an explanatory error is thrown (better than showing a deprecation message and failing later when legacy/nft iptables don't work). If `--ipv6` is not passed (and thus defaults to true), this doesn't impact operation even if the cluster doesn't support IPv6; the ip6tables rules are applied but they're innocuous. OTOH if there's no kernel support for IPv6 then the ip6tables command will fail but we'll just log the failure and not fail the linkerd-init container (nor the `add` command for linkerd-cni). This avoids having to explicitly set `--ipv6=false`, but it can be set if the user is aware of such limitations and wants to get rid of the errors. ## Linkerd IPv6 Support This allows routing IPv6 traffic to the proxy, but is just the first step towards IPv6/dual-stack support. Control plane and proxy changes will come up next.
After going over this off-github we realized it'd be best to have the upcoming control plane updates remain compatible with older proxy-inits, so in d156e1a I've brought back the |
- For the linkerd-control-plane chart added the values.yaml entry `disableIPv6`, defaulting to false. If we wan't to explicitly set `disableIPv6=true`, the proxy-init v2.3.0 (containing linkerd/linkerd2-proxy-init#350) needs to be used, otherwise proxy-init will error out because of the unknown flag. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `disableIPv6` (defaults to false). These flags can still be passed to the current proxy-init version (without linkerd/linkerd2-proxy-init#350) and will simply be ignored. Note this allows routing IPv6 traffic to the proxy by default, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come next.
Followup to linkerd/linkerd2-proxy-init#350 - For the linkerd-control-plane chart added the values.yaml entry `disableIPv6`, defaulting to false. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `disableIPv6` (defaults to false). Note this allows routing IPv6 traffic to the proxy by default, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come next. Co-authored-by: Oliver Gould <ver@buoyant.io>
Followup to linkerd/linkerd2-proxy-init#350 - For the linkerd-control-plane chart added the values.yaml entry `disableIPv6`, defaulting to false. - For the linkerd2-cni chart added the entries `iptablesMode` (defaults to "legacy") and `disableIPv6` (defaults to false). Note this allows routing IPv6 traffic to the proxy by default, but it's just the first step towards IPv6/dual-stack support. More control plane and proxy changes will come next. Co-authored-by: Oliver Gould <ver@buoyant.io> Signed-off-by: Mark S <the@wondersmith.dev>
Flags Changes
This adds the proxy-init flag
--iptables-mode
(with possible valueslegacy
andnft
), which supersedes--firewal-bin-path
andfirewall-save-bin-path
(which still remain supported).Also the
--ipv6
flag has been added (defaulttrue
).After the set of rules run via iptables are processed, if
--ipv6
is true (which is the default), the same set of rules will be run via ip6tables.Analog changes were applied to linkerd-cni as well.
Backwards-Compatibility
This is backwards-compatible with older control planes and upcoming control planes.
If
--ipv6
is not passed (and thus defaults to true), this doesn't impact operation even if the cluster doesn't support IPv6; the ip6tables rules are applied but they're innocuous.OTOH if there's no kernel support for IPv6 (which is the case for github runners*) then the ip6tables command will fail but we'll just log the failure and not fail the linkerd-init container (nor the
add
command for linkerd-cni). This avoids having to explicitly set--ipv6=false
, but it can be set if the user is aware of such limitations and wants to get rid of the errors.Testing Improvements
The cni-plugin-integration workflow has been simplified by using a matrix strategy, and enhanced by parameterizing the iptables-mode config.
Linkerd IPv6 Support
This allows routing IPv6 traffic to the proxy, but is just the first step towards IPv6/dual-stack support. Control plane and proxy changes will come up next.
(*) Github Runners IPv6 Support
Even though
modinfo
signals support for IPv6,ip6tables
commands throw modprobe errors. Indeed, according to actions/runner-images#668 support is not there yet.Also, according to actions/runner#3138 there are issues with hosted runners as well, but that might not affect us if we still expose an IPv4 interface to interact with github. Something to take into account when we get to IPv6 integration testing.