-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add CLI params for TLS cert and key - serves over HTTPS #1354
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1354 +/- ##
==========================================
+ Coverage 80.67% 80.69% +0.01%
==========================================
Files 24 24
Lines 2251 2258 +7
Branches 345 346 +1
==========================================
+ Hits 1816 1822 +6
- Misses 343 344 +1
Partials 92 92
Continue to review full report at Codecov.
|
@@ -125,5 +125,5 @@ def create_web_ui(self, host="", port=8089, auth_credentials=None): | |||
:param port: Port that the web server should listen to | |||
:param auth_credentials: If provided (in format "username:password") basic auth will be enabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add docstrings for the tls_cert
and tls_cert
params to the create_web_ui()
method? (Since this method is part of the public API, and included in the API docs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops yeah sure thing, missed this one.
This looks good! Do we need to say something about the expected format of the cert and key, or is that standardised nowadays? |
I'm certainly no expert but I think most cert types are handled fine out of the box. I tested with an existing LetsEncrypt cert I had, a new cert using |
Ok, great! Thanks! |
This adds support for serving the web UI over HTTPS. This shouldn't be the default behavior probably but if you have a cert I think you should be able to use it. Especially if basic auth is enabled.
This was first attempted and closed several years ago in #282. I'm hoping that it can be readdressed today though. Most of the web is moving to HTTPS and I believe anything that ships a web server should also come with the ability to run it securely - especially with a package as widely used as Locust is.
Comments/feedback/suggestions welcome, happy to iterate and make this work however we can! I love the Locust project and am happy to help improve it.