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

Add CORS functionality to Locust #1793

Merged
merged 2 commits into from
Jun 22, 2021
Merged

Add CORS functionality to Locust #1793

merged 2 commits into from
Jun 22, 2021

Conversation

KasimAhmic
Copy link
Contributor

I'm in the process of integrating Locust with other services at my work and we recently ran into CORS issues when trying to access the Locust API. We had circumvented it initially by using a CORS proxy server but I've since come back and updated Locust to be able to handle CORS internally using Flask-Cors. Figured it was a useful enough feature to share!

This is my first PR here so I'm not 100% sure if everything was done properly; happy to receive any and all feedback!

Changes:

  • Add Flask-Cors >= 3.0.10 as a dependency
  • Add enable_cors argument to WebUI class
  • Add --enable-cors command line argument to argument_parser.py

Add Flask-Cors >= 3.0.10 as a dependency, add enable_cors argument to WebUI, add --enable-cors command line argument to argument_parser
@cyberw
Copy link
Collaborator

cyberw commented Jun 21, 2021

Hi! is there any down side to having this always enabled (one less command line option to worry about)?

@KasimAhmic
Copy link
Contributor Author

Hi! is there any down side to having this always enabled (one less command line option to worry about)?

Hey! The only downside is if someone happens to guess the IP address Locust is running on (very unlikely) they would be able to spy on and control your test unless you also had Basic Auth turned on. Though now that I'm thinking about it, perhaps that's not the biggest issue in the world. Thoughts?

@cyberw
Copy link
Collaborator

cyberw commented Jun 21, 2021

Hi! is there any down side to having this always enabled (one less command line option to worry about)?

Hey! The only downside is if someone happens to guess the IP address Locust is running on (very unlikely) they would be able to spy on and control your test unless you also had Basic Auth turned on. Though now that I'm thinking about it, perhaps that's not the biggest issue in the world. Thoughts?

Isnt that already the case? (if they just open the URL)

@KasimAhmic
Copy link
Contributor Author

Hi! is there any down side to having this always enabled (one less command line option to worry about)?

Hey! The only downside is if someone happens to guess the IP address Locust is running on (very unlikely) they would be able to spy on and control your test unless you also had Basic Auth turned on. Though now that I'm thinking about it, perhaps that's not the biggest issue in the world. Thoughts?

Isnt that already the case? (if they just open the URL)

Yup, you're absolutely correct; I had forgotten about that lol I can push some changes later today to make it always on.

Also, were the failing tests caused by me somehow? The line below might suggest otherwise but I have no clue honestly

ERROR: InvocationError for command /home/runner/work/locust/locust/.tox/py39/bin/black --check . (exited with code 1)

@cyberw
Copy link
Collaborator

cyberw commented Jun 21, 2021

Yup, you're absolutely correct; I had forgotten about that lol I can push some changes later today to make it always on.

Also, were the failing tests caused by me somehow? The line below might suggest otherwise but I have no clue honestly

ERROR: InvocationError for command /home/runner/work/locust/locust/.tox/py39/bin/black --check . (exited with code 1)

👍

The error is that your code is not formatted correctly. Install and run black to autoformat your code (or add it to your editor and use auto format on save for an even better experience)

@KasimAhmic
Copy link
Contributor Author

Yup, you're absolutely correct; I had forgotten about that lol I can push some changes later today to make it always on.

Also, were the failing tests caused by me somehow? The line below might suggest otherwise but I have no clue honestly

ERROR: InvocationError for command /home/runner/work/locust/locust/.tox/py39/bin/black --check . (exited with code 1)

👍

The error is that your code is not formatted correctly. Install and run black to autoformat your code (or add it to your editor and use auto format on save for an even better experience)

Ah, so it was my fault after all lol Okay, I'll get that fixed up as well and push an update later today!

@KasimAhmic
Copy link
Contributor Author

Okay, a bit late but the changes have been made.

  • Cross-Origin Resource Sharing has been enabled by default
  • Arguments to toggle it have been removed
  • Formatting issues (trailing commas as it turns out) have been fixed

@cyberw
Copy link
Collaborator

cyberw commented Jun 22, 2021

Cool stuff! Can you try it out with flask 2 so we know it is compatible for our coming upgrade?

@KasimAhmic
Copy link
Contributor Author

Cool stuff! Can you try it out with flask 2 so we know it is compatible for our coming upgrade?

After replicating the changes found in #1764, everything continues to work as expected!

@cyberw cyberw merged commit 6ac8257 into locustio:master Jun 22, 2021
@cyberw
Copy link
Collaborator

cyberw commented Jun 22, 2021

Thx!

@KasimAhmic KasimAhmic deleted the enable-cors branch June 22, 2021 13:21
@KasimAhmic
Copy link
Contributor Author

No problem!

As for the coming upgrade, is there any rough estimate on the release date? If not, no worries

@cyberw
Copy link
Collaborator

cyberw commented Jun 22, 2021

Not really, sorry

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.

2 participants