-
Notifications
You must be signed in to change notification settings - Fork 376
Add Configuration variable to allow using custom Authorization header #710
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?
Add Configuration variable to allow using custom Authorization header #710
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
d0e6447
to
e32531f
Compare
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.
Pull Request Overview
Adds support for custom authorization headers by introducing a new environment variable BAZELISK_AUTH_HEADER
, enabling the use of Bearer tokens and other authentication methods beyond basic auth for downloading Bazel releases.
- Introduces
BAZELISK_AUTH_HEADER
environment variable for custom authorization headers - Modifies authentication logic to prioritize custom auth header over netrc credentials
- Documents the new configuration option with usage examples
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
httputil/httputil.go | Adds AuthHeader global variable and updates download logic to use custom auth when available |
core/core.go | Implements getAuthHeader function to retrieve configuration value and sets httputil.AuthHeader |
README.md | Documents the new BAZELISK_AUTH_HEADER environment variable with usage example |
Comments suppressed due to low confidence (1)
core/core.go:305
- Variable name 'auth_header' uses snake_case which is inconsistent with Go naming conventions. It should be renamed to 'authHeader' to follow camelCase convention.
auth_header := config.Get("BAZELISK_AUTH_HEADER")
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
93623b9
to
086d75a
Compare
if err == nil { | ||
// successfully parsed netrc for given host | ||
auth = t | ||
if AuthHeader != "" { |
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.
I think we should raise an error if both AuthHeader is set and a .netrc file exists.
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.
Nope,
.netrc file can exists but does not have the host or does have the host and the password.
But for the specific endpoint to download you need to use the token.
This is the actual case in my infrastructure.
I have .netrc but I can't use the password in it as a basic auth header. I need a seperate Access Token for baselisk.
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 @fweikert
Do you have any other notes on this review ?
Hello,
I have a use case that I can't use Basic Auth to authenticate to the download server, but I can only use Bearer tokens.
so I made a new configuration variable to allow passing any custom string to Authorization header, including Bearer tokens.
I hope this helps other people.
Thanks