cli: Add new consul connect redirect-traffic command for applying traffic redirection rules when Transparent Proxy is enabled.
#9910
Conversation
github-actions
bot
added
pr/dependencies
ishustava
force-pushed
the
ishustava/tproxy-basic-iptables
branch
from
481cc7e
to
c4e3e7a
Compare
ishustava
force-pushed
the
ishustava/tproxy-basic-iptables
branch
from
c4e3e7a
to
b63ebe0
Compare
ishustava
changed the title
consul connect redirect-traffic command for applying traffic redirection rules when Transparent Proxy is enabled.
There was a problem hiding this comment.
This is looking good @ishustava, nice work! I added questions/comments in line.
| cfg.ProxyInboundPort = svc.Port | ||
|
|
||
| // todo: change once it's configurable | ||
| cfg.ProxyOutboundPort = xds.TProxyOutboundPort |
There was a problem hiding this comment.
This is more of a note for myself, but looking at this maybe the TProxyOutboundPort constant should be moved to the SDK so that consul-k8s doesn't need to import the xds package. We won't default this value in proxy registrations.
There was a problem hiding this comment.
I was just coming here to say that the command package probably shouldn't import agent/xds
There was a problem hiding this comment.
💯 Agree. Sorry, I think misunderstood how defaulting should work and thought this const will go away eventually and so was using it temporarily. After syncing with Freddy, I've moved it to the iptables package instead.
| // based on the configuration provided in cfg. | ||
| // This implementation was inspired by | ||
| // https://github.com/openservicemesh/osm/blob/650a1a1dcf081ae90825f3b5dba6f30a0e532725/pkg/injector/iptables.go | ||
| func Setup(cfg Config) error { |
There was a problem hiding this comment.
Is this idempotent? What happens if it's run twice? Either with the same or different Config.
Should we attempt to drop the chains before adding them?
There was a problem hiding this comment.
Yeah, that's a great call-out. I've been thinking about your comment above about documenting that ApplyRules is not atomic, and I think it makes sense to make that one atomic and this one idempotent. I'm thinking to address it in a follow-up PR because most likely I'll switch to using iptables-save and iptables-restore to do that.
| all traffic to go through the [Envoy proxy](https://envoyproxy.io) when using [Consul | ||
| Connect](/docs/connect/) in the Transparent Proxy mode. |
There was a problem hiding this comment.
| all traffic to go through the [Envoy proxy](https://envoyproxy.io) when using [Consul | |
| Connect](/docs/connect/) in the Transparent Proxy mode. | |
| all traffic to go through the [Envoy proxy](https://envoyproxy.io) when using [Consul | |
| service mesh](/docs/connect/) in the Transparent Proxy mode. |
Co-authored-by: Freddy <freddygv@users.noreply.github.com>
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/347969. |
This PR proposes the following changes:
consul connect redirect-trafficcommand for applying traffic redirection rules when Transparent Proxy is enabled.iptablespackage for applying traffic redirection rules with iptables.