-
Notifications
You must be signed in to change notification settings - Fork 77
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
Add FetchJWTSVIDs function for workloadapi and jwtSource #187
Conversation
Signed-off-by: Yuhan Li <liyuhan.loveyana@bytedance.com>
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 @loveyana ! Just one small comment.
v2/workloadapi/client.go
Outdated
svids = append(svids, s) | ||
} | ||
|
||
if len(svids) == 0 { |
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 check should move above the for loop and check the length of SVIDs in the response, otherwise, if the response has no SVIDs and firstOnly
is true, we will panic on line 445.
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 point, I directly copied the code of parseX509SVIDs
and modified it. I didn't notice it here. I think parseX509SVIDs
also has this problem, so I modified it at the same time.
In addition, I would like to ask about java-spiffe
, this spiffe/java-spiffe#90 not assigned to reviewer, any idea?
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 would like to ask about
java-spiffe
, this spiffe/java-spiffe#90 not assigned to reviewer, any idea?
@loveyana I'm reviewing it today.
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 would like to ask about
java-spiffe
, this spiffe/java-spiffe#90 not assigned to reviewer, any idea?@loveyana I'm reviewing it today.
In addition, I may need help to look at the HewlettPackard/py-spiffe#105. It seems that the lint in CI on the py-spiffe
side is wrong, but I passed it locally, and it seems that the previous pr also has the same problem. I hope you can help and review it at the same time
Signed-off-by: Yuhan Li <liyuhan.loveyana@bytedance.com>
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, @loveyana !
Signed-off-by: Andrew Harding <aharding@vmware.com>
Signed-off-by: Yuhan Li liyuhan.loveyana@bytedance.com
Add FetchJWTSVIDs method for workloadapi to fetch all jwt-svids, which are provided to the client side for filtering under workload scenarios with multiple registration information.