-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[feat] Add URL-based secret-source
#5413
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
base: master
Are you sure you want to change the base?
Conversation
|
Test:
|
|
I'm reviewing this, but added you @mstoykov as a reviewer too, as I really want your eyes on this 🙇🏻 |
ankur22
left a comment
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.
LGTM 🚀 I don't have the full context of the changes, but from I can see and the issue description it makes sense with this approach.
internal/secretsource/url/url.go
Outdated
| } | ||
| defer func() { _ = response.Body.Close() }() | ||
|
|
||
| if response.StatusCode != http.StatusOK { |
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.
Is there a plan to retry server errors? At the moment the iteration will fail when it sees a 5xx, subsequent iterations might pass though if the error is temporary. I guess with retries, the issue is that it will affect the timings of the iteration.
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.
Thanks, I added retries in d625ce9
|
ankur22
left a comment
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.
LGMT 🚀
What?
Implements #5412. Adds a new
urlsecret-source that fetches secrets from HTTP endpoints. This allows k6 to retrieve secrets from any HTTP-based secret management service using configurable URL templates.Why?
Checklist
make check) and all pass.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)
https://github.com/grafana/k6-cloud/issues/4038
Closes #5412