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

Expose HTTP-based paths through Connect proxy #6446

Merged
merged 84 commits into from
Sep 26, 2019
Merged

Expose HTTP-based paths through Connect proxy #6446

merged 84 commits into from
Sep 26, 2019

Conversation

freddygv
Copy link
Contributor

@freddygv freddygv commented Sep 5, 2019

Fixes: #5396

This PR adds a proxy configuration stanza called expose. This registered listeners in Connect sidecar proxies to allow requests to specific paths from outside of the node. This allows services to protect themselves by only listening on the loopback interface, while still accepting traffic from non Connect-enabled services.

Under expose there is a boolean checks flag that would automatically expose all registered HTTP and gRPC check paths.

This stanza also accepts a paths list to expose individual paths. The primary use case for this functionality would be to expose paths for third parties like Prometheus or the kubelet.

Listeners for requests to exposed paths will be configured dynamically at run-time. Any time a proxy, or check can be registered, a listener can also be created.

In this initial implementation requests to these paths will not be authenticated/encrypted.


Change overview:

  • Added a cache-type for service checks so that Envoy can be notified any time it needs to modify check listeners. This might be if we enable/disable exposing checks or if checks are added/removed.

  • Rerouted the addr used by (CheckHTTP|CheckGRPC).check() based on whether checks are exposed. If checks are exposed, then ProxyGRPC|ProxyHTTP are set to point at the proxy, and outbound checks will use that as their target. The port for the listener is generated dynamically from a configurable range, and all listener ports are tracked to avoid collisions.

  • Created an Envoy listener for every path to expose. Otherwise distinguishing between HTTP1/2 traffic with Envoy (< v1.12) isn't possible. This also enables us to set a TLS context for each path later on.

  • Created an Envoy cluster any time the path's target port doesn't match the local_app's port.


Remaining TODO:

  • Envoy cluster/listener creation tests
  • Docs
  • Create a separate WIP branch for securing exposed paths with TLS
  • Fix TestManager_BasicLifecycle

@johncowen
Copy link
Contributor

Hey 👋 , sorry for the lateness of this. I was looking over the documentation here for our UI integration for this today and noticed there are various places in

https://github.com/hashicorp/consul/blob/expose-urls/website/source/docs/connect/registration/service-registration.html.md

..which say:

The following is a complete example showing <insert something here>

Wondering if this needs the Expose property/key adding in these places for it to be 'complete'

Tiny nit I know, but thought it would be worth mentioning just incase, it might help someone in future who is scanning the docs quickly for how to do something.

@freddygv
Copy link
Contributor Author

Good catch @johncowen, just added the stanza to the full config example that I missed.

@rboyer
Copy link
Member

rboyer commented Sep 24, 2019

Were you still planning on keeping the TLS bits in here, or was that still a TODO to shift that stuff into a different PR?

@freddygv
Copy link
Contributor Author

@rboyer I was in the process of removing the TLS stuff.

All that's living here now: https://github.com/hashicorp/consul/tree/expose-paths-tls

Copy link
Collaborator

@judithpatudith judithpatudith left a comment

Choose a reason for hiding this comment

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

Last couple tweaks. LGTM!

@freddygv freddygv merged commit fdd10dd into master Sep 26, 2019
@freddygv freddygv deleted the expose-urls branch September 26, 2019 02:55
freddygv added a commit that referenced this pull request Sep 27, 2019
Fixes: #5396

This PR adds a proxy configuration stanza called expose. These flags register
listeners in Connect sidecar proxies to allow requests to specific HTTP paths from outside of the node. This allows services to protect themselves by only
listening on the loopback interface, while still accepting traffic from non
Connect-enabled services.

Under expose there is a boolean checks flag that would automatically expose all
registered HTTP and gRPC check paths.

This stanza also accepts a paths list to expose individual paths. The primary
use case for this functionality would be to expose paths for third parties like
Prometheus or the kubelet.

Listeners for requests to exposed paths are be configured dynamically at run
time. Any time a proxy, or check can be registered, a listener can also be
created.

In this initial implementation requests to these paths are not
authenticated/encrypted.
@johncowen johncowen mentioned this pull request Oct 3, 2019
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.

Connect: Expose HTTP health checks
6 participants