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

Only consider virtual IPs for transparent proxies #10162

Merged
merged 3 commits into from
May 3, 2021

Conversation

freddygv
Copy link
Contributor

Initially we were loading every potential upstream address into Envoy
and then routing traffic to the logical upstream service. The downside
of this behavior is that traffic meant to go to a specific instance
would be load balanced across ALL instances.

Traffic to specific instance IPs should be forwarded to the original
destination and if it's a destination in the mesh then we should ensure
the appropriate certificates are used.

This PR makes transparent proxying a Kubernetes-only feature for now
since support for other environments requires generating virtual IPs,
and Consul does not do that at the moment.

Initially we were loading every potential upstream address into Envoy
and then routing traffic to the logical upstream service. The downside
of this behavior is that traffic meant to go to a specific instance
would be load balanced across ALL instances.

Traffic to specific instance IPs should be forwarded to the original
destination and if it's a destination in the mesh then we should ensure
the appropriate certificates are used.
@freddygv freddygv added this to the 1.10.0-beta2 milestone Apr 30, 2021
@freddygv freddygv requested a review from a team April 30, 2021 22:41
@github-actions github-actions bot added theme/envoy/xds Related to Envoy support type/docs Documentation needs to be created/updated/clarified labels Apr 30, 2021
@hashicorp-ci
Copy link
Contributor

🤔 This PR has changes in the website/ directory but does not have a type/docs-cherrypick label. If the changes are for the next version, this can be ignored. If they are updates to current docs, attach the label to auto cherrypick to the stable-website branch after merging.

@vercel vercel bot temporarily deployed to Preview – consul-ui-staging May 3, 2021 15:34 Inactive
@vercel vercel bot temporarily deployed to Preview – consul May 3, 2021 15:34 Inactive
@@ -32,6 +32,8 @@ import (
"github.com/hashicorp/consul/sdk/iptables"
)

const virtualIPTag = "virtual"
Copy link
Member

Choose a reason for hiding this comment

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

Would this be better off in agent/structs?

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't need to export it now, I think it's desirable to be able to keep it locally unexported. We can always move it and export it later, but if we don't need to it's one less thing that has to be changed when we need to split up the agent/structs package.

Copy link
Member

@rboyer rboyer left a comment

Choose a reason for hiding this comment

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

LGTM minus the one comment.

@vercel vercel bot temporarily deployed to Preview – consul-ui-staging May 3, 2021 19:00 Inactive
@vercel vercel bot temporarily deployed to Preview – consul May 3, 2021 19:00 Inactive
@freddygv freddygv merged commit 2ca3f48 into master May 3, 2021
@freddygv freddygv deleted the tproxy/only-consider-vip branch May 3, 2021 20:15
@hc-github-team-consul-core
Copy link
Collaborator

🍒 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/361369.

@hc-github-team-consul-core
Copy link
Collaborator

🍒❌ Cherry pick of commit 2ca3f48 onto release/1.10.x failed! Build Log

freddygv added a commit that referenced this pull request May 3, 2021
Initially we were loading every potential upstream address into Envoy
and then routing traffic to the logical upstream service. The downside
of this behavior is that traffic meant to go to a specific instance
would be load balanced across ALL instances.

Traffic to specific instance IPs should be forwarded to the original
destination and if it's a destination in the mesh then we should ensure
the appropriate certificates are used.

This PR makes transparent proxying a Kubernetes-only feature for now
since support for other environments requires generating virtual IPs,
and Consul does not do that at the moment.
@freddygv freddygv restored the tproxy/only-consider-vip branch May 3, 2021 21:36
@freddygv freddygv deleted the tproxy/only-consider-vip branch May 3, 2021 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/envoy/xds Related to Envoy support type/docs Documentation needs to be created/updated/clarified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants