.*: Fix Azure MSI token refresh #4611
Conversation
phillebaba
force-pushed
the
fix/msi-token-refresh
branch
from
96d5abb
to
6fe5af9
Compare
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
phillebaba
force-pushed
the
fix/msi-token-refresh
branch
2 times, most recently
from
6c9665b
to
b1fb8d4
Compare
| var tokenRefresher blob.TokenRefresher = func(credential blob.TokenCredential) time.Duration { | ||
| err := azureServicePrincipalToken.Refresh() | ||
| if err != nil { | ||
| // Retry later as the error can be related to API throttling |
There was a problem hiding this comment.
I would 100% add metric or error log to indicate that we hit this and why (err)
pkg/objstore/azure/helpers.go
Outdated
| azureServicePrincipalToken, err := msiConfig.ServicePrincipalToken() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Get a new token. | ||
| var tokenRefresher blob.TokenRefresher = func(credential blob.TokenCredential) time.Duration { |
There was a problem hiding this comment.
what about defining this function outside? (normally) ;p
There was a problem hiding this comment.
Cant really do that as it need access to the spt.
phillebaba
force-pushed
the
fix/msi-token-refresh
branch
from
b1fb8d4
to
def81e7
Compare
Signed-off-by: Philip Laine <philip.laine@xenit.se>
phillebaba
force-pushed
the
fix/msi-token-refresh
branch
from
def81e7
to
447e04c
Compare
|
@bwplotka hopefully this solves all comments and issues. I will leave a build of this running for at least 12 hours to make sure that the token refresh works. The only thing I am not super happy about is the token refresh failure but it might be something that we need to re visit in the future instead. |
|
Nice, thanks. Can we add issue to the library we use, so they are aware? |
|
Created an issue in the library, linking it here to make it easier to track. |
Signed-off-by: Philip Laine <philip.laine@xenit.se>
Changes
The changes made in #4607 was missing a token refresh as I expected that to be done by the client. This was not the case and after leaving things running over the weekend the token eventually expired and everything stopped working. This change adds a token refresh function which renews the token before it expires.
Verification
Have tested a build of this branch in an AKS cluster, and it works. I will leave it running for another day just to make sure that the token refresh actually works.