Question on Refresh behavior

[sol-0]

Question

Hi @edwarnicke ,

I was checking how refresh mechanism works in NSM during issue #839 investigation, and I think refresh might be broken in the case when different mesh elements (nsmgr, forwarders, etc.) have different token expiry.

I checked your changes in this PR and saw that, e.g. in NSMgr server refresh was removed, so after that I also checked your discussion with Vladimir and found your response:
"...We could do as you suggested and compute that in refresh. I'm tempted though to do it in updatetoken by clamping down expiretime to the smallest in the path on the return direction of the path." - could you please share whether anything was done here? As I couldn't find a place in the code where this case is handled.

Also, if you don't mind, could you please describe how refresh mechanism is working currently?

[denis-tingaikin]

/сс @edwarnicke

[edwarnicke]

@sol-0 Delighted to talk you through it :)

Let's start with the Client Chain. It includes a opts.refreshClient in the chain. By default, that opts.refreshClient is refresh.NewClient.

However, if the user passes client.WithoutRefresh() then opts.refreshClient is set to null.NewClient which is a noop.

This brings us around to the first central feature of refresh: It should only be performed by non-passthrough clients.
If a client is part of a passthrough chain element (like NSMgr, or a Forwarder, or cmd-nse-firewall-vpp it should be using the client.WithoutRefresh() option to not do refresh. Refresh is driven from the start of the chain, not the middle.

The refresh chain element on Request computes the time after which it should send a refresh request which is roughly 1/3 the time remaining till expiration plus a little bit of noise.

The refresh chain element creates a cancelCtx cancels any old cancelCtx from previous request and stores the new cancelCtx.

From there a go routine is spawned to send the refresh at the computed time and, if it does not succeed, keep trying to send it.

Note: if cancelCtx has been canceled (either by Close or a new Request) the go routine exits.

The comment you so astutely noted is that we should probably clamp down the expireTime to the earliest time in the chain.
IMHO the best way to do that is to have updatetoken compare its expireTime to the expireTime of the next PathSegment after it in the Path, and pick the earliest.

[denis-tingaikin]

The comment you so astutely noted is that we should probably clamp down the expireTime to the earliest time in the chain.
IMHO the best way to do that is to have updatetoken compare its expireTime to the expireTime of the next PathSegment after it in the Path, and pick the earliest.

I think we need to implement this. I see that it can be super useful for interdomain scenarios where two or more clusters can have a totally different spire setup.

@edwarnicke Do you mind if we'll create an issue and describe a potential technical solution for this?

[edwarnicke]

@denis-tingaikin Please do

[sol-0]

@edwarnicke ,
thanks a lot for really helpful and in-depth explanation!

@denis-tingaikin ,
I've added a new issue #1189 based on this discussion.