Add hint support #242
Add hint support #242
Conversation
Signed-off-by: Kevin Fox <[email protected]>
Co-authored-by: Marcos Yacob <[email protected]> Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: kfox1111 <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
Signed-off-by: Kevin Fox <[email protected]>
| return os.WriteFile(filePath, []byte(jwtSVID.Marshal()), jwtSVIDFileMode) | ||
| } | ||
|
|
||
| func getJWTByHint(jwtSVIDs []*jwtsvid.SVID, hint string) *jwtsvid.SVID { // Choose a better name :D |
There was a problem hiding this comment.
The workload api has a WithDefaultJWTSVIDPicker filter. What do you think of structuring this slightly differently by using this to filter svids by hint through the worklaod api. Then the logic in this library will be unchanged and simpler just to write to everything to disk.
There was a problem hiding this comment.
When I was digging through the code, I didn't find an easy way to apply that filter. Maybe I missed something though. Do you know where in the code we could do that?
There was a problem hiding this comment.
Yea youre right. Its not available through the APIs we are using.
There was a problem hiding this comment.
Marcos was talking about maybe a major refactoring at some point of the spiffe-helper. Maybe that would be a good time to consider refactoring that part of this PR?
There was a problem hiding this comment.
Would you be able to move the logic to filter by hint to pkg/sidecar.go? I think thats a better place for this code to live. If not I can do that after merging this.
There was a problem hiding this comment.
I looked into this a little bit. Would totally have to rewrite the sidecar tests / disk tests which is a good chunk of work. I'm ok doing that, but it would be nice to be able to start using this soon as well. Can we push this pr through and then rework it after release?
Co-authored-by: Faisal Memon <[email protected]> Signed-off-by: kfox1111 <[email protected]>
Fixes: #113