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

Fixes #20930 - expose path for HTTPS endpoints #21134

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

vijayraghav-io
Copy link
Contributor

@vijayraghav-io vijayraghav-io commented May 19, 2024

Description

fixes #20930

Added 3 parameters CAFile, CertFile, KeyFile to ServiceDefinition -> Proxy -> Expose -> Path.
In case of HTTPS path , these 3 parameters are used to configure envoy and allow HTTPS -> HTTPS traffic for external service path to be exposed.

Testing & Reproduction steps

refer #20930 for reproduction scenario

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@vijayraghav-io
Copy link
Contributor Author

Hi @david-yu / @huikang / @jkirschner-hashicorp ,

Kindly take this PR for review.

@Cottand

@vijayraghav-io vijayraghav-io requested a review from a team as a code owner May 19, 2024 14:21
@Cottand
Copy link

Cottand commented May 19, 2024

@vijayraghav-io thanks for the PR.

Do we need these fields to be set in config for HTTPS -> HTTPS ? I would expect it should be possible to simply not terminate TLS at all.

And if it does terminate TLS, why do we need the keyFile? Can we not have envoy trust the provided CA and expose HTTP? As in HTTP -> HTTPS

@vijayraghav-io
Copy link
Contributor Author

vijayraghav-io commented May 20, 2024

@Cottand Thanks for your comments

Yes, as an option, HTTPS passthrough i.e. without terminating TLS and using normal TCP_Proxy can be provided. Updated to accommodate this. For this protocol must be provided as "tcp" (Service -> Proxy -> Expose -> Path -> Protocol = "tcp") , no need to provide certfile or keyfile path in this case.

For terminating TLS, yes its a good suggestion to have envoy trust the provided CA, let me get a collective feedback from other reviewers as well if any before updating.

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.

in Connect, I cannot expose path for HTTPS endpoints inside the container
2 participants