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

Support HTTP proxy with mTLS #679

Closed

Conversation

AlessandroPatti
Copy link
Contributor

Support passing a key and certificate pair to enable mTLS with the http proxy backend

Copy link
Collaborator

@mostynb mostynb left a comment

Choose a reason for hiding this comment

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

Thanks for the feature submission. This looks pretty good, just a few comments...

Once the help text is finalized, we will need to update the help text that's in README.md.

We should also add a test for this feature. We could try to add one to .bazelci/system-test.sh, but it already runs slowly on bazelci/buildkite, so it might be time to refactor that and split it into more jobs. I will try to have a look at that tomorrow.

config/proxy.go Outdated Show resolved Hide resolved
&cli.StringFlag{
Name: "http_proxy.key_file",
Value: "",
Usage: "Path to the key used to autheticate with the proxy backend. Enables mTLS.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

The help text for these two new options should each mention that the other must also be specified.

@mostynb
Copy link
Collaborator

mostynb commented Jul 30, 2023

Merged with some minor adjustments to the help text. I'll have a think about how best to test this, it's probably doable in .bazelci/system-test.sh after some refactoring, now that I have sped it up.

Thanks!

@mostynb mostynb closed this Jul 30, 2023
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.

None yet

3 participants