bugfix: refresh before token expires, not after

[timbertson]

If you set refreshBeforeExpiration to 1 minute, the current code refreshes the token only after the token has expired for at least a minute. Time maths are hard, I had to add the test to actually verify my hunch 😆

[joan38]

./mill __.style, push the style fix and we should be good to go.
Thanks!

[timbertson]

Done 👍

[yurique]

Oh interesting 🤔
What a nasty bug and a nice catch!

It indeed looks like a plusSeconds vs minusSeconds problem here:

val shouldRenew =  cached.expirationTimestamp.exists(_.isBefore(now.minusSeconds(refreshBeforeExpiration.toSeconds)))
                                                                     ~~~~~~~~ 

it should be like this (with .plusSeconds), right? (time maths are hard :) )

don't refresh
TIME ----------|-----------|-----------------------------|--------------->
               \ now       \ now+                        \ token
                             refreshBefore                 expiration
                                 \------------------------NOT isBefore

do refresh
TIME ----------|-----------|----------------------------|--------------->
               \ now       \ token                      \ now+                        
                             expiration                   refreshBefore 
                                 isBefore---------------------/


do refresh
TIME ---------|---------------------|-------------------|--------------->
              \ token               \ now               \ now+                        
                expiration                                refreshBefore 
                   isBefore---------------------------------/ 

[timbertson]

Sorry I got waylaid, feel free to push whatever you need to my branch to sort it out.

…

On Fri, 24 Nov 2023, 5:28 pm Joan Goyeau, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In kubernetes-client/src/com/goyeau/kubernetes/client/util/cache/AuthorizationCache.scala <#254 (comment)> : > @@ -38,8 +38,8 @@ object AuthorizationCache { case Some(cached) => F.realTimeInstant .flatMap { now => - val shouldRenew = - cached.expirationTimestamp.exists(_.isBefore(now.minusSeconds(refreshBeforeExpiration.toSeconds))) + val minExpiry = now.plusSeconds(refreshBeforeExpiration.toSeconds) We need: https://github.com/scala/scala-java8-compat ivy"org.scala-lang.modules::scala-java8-compat:1.0.2" — Reply to this email directly, view it on GitHub <#254 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAADOXCO67BLUGMO24IWNSTYGA5ANAVCNFSM6AAAAAA7MA6FT6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBXGM3TCNRSGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>