-
Notifications
You must be signed in to change notification settings - Fork 67
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
Support create token with headers #1455
Conversation
82a8401
to
a506f4c
Compare
a506f4c
to
a7087a8
Compare
What's the difference between this and the method in the https://github.com/streamnative/pulsarctl/pull/1455/files#diff-c61908f43ade6604eaae204d799b4b61f0ea0f98df937b11b8d8caacc73df0b0R45? |
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.
Seems we already support the API with header:
pulsarctl/pkg/cmdutils/token.go
Line 94 in 79b2d46
func (t *token) CreateToken( |
what’s the difference between them? Maybe we can use the existing API to extend more options for the jwt claims?
The pulsarctl/pkg/cmdutils/token.go Lines 76 to 92 in 79b2d46
|
0c76b3b
to
199591f
Compare
199591f
to
cd82391
Compare
@zymap I let cmd call the |
pkg/ctl/token/create.go
Outdated
@@ -126,6 +135,8 @@ func create(vc *cmdutils.VerbCmd) { | |||
"The expire time for a token. e.g. 1s, 1m, 1h") | |||
set.BoolVar(&args.base64Encoded, "base64", false, | |||
"The secret key is base64 encoded or not.") | |||
set.StringVar(&args.headers, "headers", "", |
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.
There is a method set.StringToStringVar()
to receive a map value from the command line. We can use that instead of parse the map value by ourselves.
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.
Good idea, updated.
### Motivation Currently, the tool that generates the token cannot attach headers. We should support it. For example, it makes sense to attach a kid field for JWKS. (cherry picked from commit c7fdea5)
### Motivation Currently, the tool that generates the token cannot attach headers. We should support it. For example, it makes sense to attach a kid field for JWKS. (cherry picked from commit c7fdea5)
Master Issue: #
Motivation
Currently, the tool that generates the token cannot attach headers. We should support it. For example, it makes sense to attach a kid field for JWKS.
Modifications
Support create token with headers
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation
Check the box below.
Need to update docs?
doc-required
(If you need help on updating docs, create a doc issue)
no-need-doc
(Please explain why)
doc
(If this PR contains doc changes)