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 https://github.com/joan38/kubernetes-client/pull/254#discussion_r1403981578 :

@@ -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 https://github.com/joan38/kubernetes-client/pull/254#discussion_r1403981578, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADOXCO67BLUGMO24IWNSTYGA5ANAVCNFSM6AAAAAA7MA6FT6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBXGM3TCNRSGA . You are receiving this because you were mentioned.Message ID: @.***>