-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
feat: add bearer token auth #21462
base: master
Are you sure you want to change the base?
feat: add bearer token auth #21462
Conversation
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21462 +/- ##
==========================================
+ Coverage 55.21% 55.30% +0.08%
==========================================
Files 337 337
Lines 56840 56903 +63
==========================================
+ Hits 31387 31470 +83
+ Misses 22757 22751 -6
+ Partials 2696 2682 -14 ☔ View full report in Codecov by Sentry. |
Following he discussion in the contributors meeting, bearer token auth is only relevant for Git repos at this point, so I will add CLI changes that display a not supported message if used with Helm repo, and a UI change that would not display this field for Helm repo. |
0756543
to
606bd5b
Compare
Signed-off-by: reggie-k <[email protected]>
✅ Preview Environment deployed on Bunnyshell
See: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
CLI and UI are now enforcing that the bearer token can be passed only for Git repositories (on the backend side, there is one struct with all the possible params so impossible to separate). The field is not displayed for HTTPS Helm repos, only for Git ones. |
Signed-off-by: reggie-k <[email protected]>
@@ -34,6 +34,7 @@ func AddRepoFlags(command *cobra.Command, opts *RepoOptions) { | |||
command.Flags().StringVar(&opts.Repo.Project, "project", "", "project of the repository") | |||
command.Flags().StringVar(&opts.Repo.Username, "username", "", "username to the repository") | |||
command.Flags().StringVar(&opts.Repo.Password, "password", "", "password to the repository") | |||
command.Flags().StringVar(&opts.Repo.BearerToken, "bearer-token", "", "bearer token to the Git repository") |
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.
should we change it to bearer token to the repository, so it will be consistent with username and password?
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.
This is on purpose, since It is only relevant for Git, at least at the moment. When support for Helm would be added, this would be a good time to make it consistent.
Left few small comments, also will do manual QA and will let you know |
Signed-off-by: reggie-k <[email protected]>
…into bb-bearer-token
Signed-off-by: reggie-k <[email protected]>
Closes #9667
The PR is adding a new param,
BearerToken
, it can be used when the Bearer Auth is needed instead of Basic Auth, like for example for BitBucket Project token, which requires Bearer Auth.The relevant docs, CLI and UI changes are also present, along with unit tests.
Checklist: