-
Notifications
You must be signed in to change notification settings - Fork 140
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
enhancement: make security test timeout configurable via env variable #512
Conversation
Security Test timeouts are currently hardcoded in API's config.yml. Provide user options to use a environment variable HUSKYCI_CLIENT_TESTS_TIMEOUT in the client code. All requests send to the API henceforth will contain that timeout. On the API side, if a non zero custom timeout is detected, it shall override the default timeouts for the security tests. Signed-off-by: Dwaipayan Ray <[email protected]>
@rafaveira3 Review please! :) |
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.
Hello, @raydwaipayan! Thanks a lot for your time working on this issue. I have just checked out your branch and was not able to limit the time securityTests are running. This is what I did:
export HUSKYCI_CLIENT_TESTS_TIMEOUT="1"
. .env && make run-client
And the securityTests did not exit after 1 second. Am I doing something wrong? 🙃
I updated the issue with some new tips on how you can tackle this using the timeout
command similar with what we are doing with Gitleaks:
Hi @rafaveira3, The waitContainer function in api.go takes the Is it perhaps an unimplemented feature? |
Indeed, @raydwaipayan! You are totally right. You rock! 🚀 Unfortunately, we are not using this feature with this input yet. We need to work on this later on. 😅 However, for this particular issue, we need to provide our clients (not the API) the ability to set the timeout on their side. We kinda did this (hard-coded at this moment) using the I was wondering if could use the same logic we are using to handle this |
@rafaveira3 Since the docker api is being called directly by the program, I think we can't use the timeout command after the api server is started. Probably we have to use something which is supplied by docker's api. Or write our own timeout handler that is. So the best bet is to either harcode everything or just fix the docker's timeout handling. You could open an issue on that. After it's done we could check this again and it should probably run fine :) |
@raydwaipayan, I believe that what @rafaveira3 is trying to say is that we can substitute the In this scenario, we should create another method ( Other than that, we should also do a timeout error handling, such as this one. After that, another change you may need to do is to add some logic, similar to this one, so that the security test can handle properly the timeout output. What do you say we start adding these changes by replacing how GitLeaks does it since it's already hard coded? 🙂 |
@Krlier True I looked into it and it seems doable. I will try to draw something up :) |
Hey, @raydwaipayan! How's it going? Did you come up with something? Do you need any help? |
Security Test timeouts are currently hardcoded in API's
config.yml.
Provide user options to use a environment variable
HUSKYCI_CLIENT_TESTS_TIMEOUT in the client code. All requests
send to the API henceforth will contain that timeout.
On the API side, if a non zero custom timeout is detected, it
shall override the default timeouts for the security tests.
Signed-off-by: Dwaipayan Ray [email protected]
Description
Adds support for setting Security Test timeout from the client side
using environment variables.
Closes #509
Proposed Changes
Add custom timeout handling logic in both the api and client. Timeout is
completely optional and no function signatures were changed. So, there
are no breaking changes.
Testing