Core: add an option to renew a token in advance according to a configured duration

[kayrus]

OpenStack services internal communications usually reuse a user token, and asynchronous operations can fail although client received a successful response code. A couple of examples:

• glance -> swift service communication
• cinder -> glance -> swift communication
• nova -> cinder communication

On the OpenStack service side this limitation can be solved by using a service token: https://docs.openstack.org/cinder/latest/configuration/block-storage/service-token.html
However this solution is not always secure enough and this option is disabled by default (and deprecated in glance https://wiki.openstack.org/wiki/OSSN/OSSN-0060), especially in case of CADF audit events.
For long running operations like cinder -> glance -> swift, this may be not an option, because the whole operation can take more than a token TTL, but in cases like nova -> cinder this may do a trick.

I propose to introduce a provider client option, e.g. ReauthBeforeTokenExpiration time.Duration, and run a ReauthFunc in advance before the token expiration.
See kubernetes/cloud-provider-openstack#1394 for details
@jtopjian suggestions are welcome, especially on a variable name.

[jtopjian]

Is there something preventing client.Reauthenticate from being manually triggered?

[kayrus]

@jtopjian no, but it should be called before each API call. You can imaging the mess in app.

[jtopjian]

I think I see where my view differed.

My view of the issue you linked was that there are a handful of API calls that will trigger actions which may last a long time. During these specific API calls, someone might want to obtain a new token in order to increase the probability that the token would not expire during the call.

But, if I understand correctly, this feature would conditionally renew a token if any API call was made during a certain window. Is this correct?

If so, one thing to keep in mind is that this is not an absolute fix for the problem being described. It really just increases the probability of success. There could be some outside issue that prolongs the API call and still causes the token to expire.

I wonder if adding support for pre and post call hooks would be a better general solution?

[kayrus]

But, if I understand correctly, this feature would conditionally renew a token if any API call was made during a certain window. Is this correct?

currently a client token renewal happens, when API call returns 401, which is kinda hacky from my perspective, but it worked for ages. I propose checking the token expiration in advance (token.expiry_date - time.Duration) inside the provider do method. A token renewal without even causing the 401 would be a nicer logic.

[kayrus]

@jtopjian here is a nice example from swift code of the logic I propose to introduce: https://opendev.org/openstack/glance_store/src/commit/d05024afc3bed9deca1acc49f4a8d6faf14b0545/glance_store/_drivers/swift/connection_manager.py#L75

[jtopjian]

currently a client token renewal happens, when API call returns 401, which is kinda hacky from my perspective, but it worked for ages

IMO, the reauth implementation has always felt too complicated for what it does. I'm happy that we've gotten enough support over the years to make it work well, though.

The trigger on a 401 is actually the simple part. Unless I'm mistaken, the implementation that you're talking about would just be a similar, but instead of firing after a 401, it would conditionally fire before the actual HTTP request is made. It would still call the same reauth code, though.

I'm not opposed to this idea at all. I just want to take the opportunity to see if instead of implementing this specific feature we should look at adding generic support for a pre-call hook, where devs/users can define a function to do anything before each API call.

[kayrus]

The trigger on a 401 is actually the simple part. Unless I'm mistaken, the implementation that you're talking about would just be a similar, but instead of firing after a 401, it would conditionally fire before the actual HTTP request is made. It would still call the same reauth code, though.

Exactly.

I'm not opposed to this idea at all. I just want to take the opportunity to see if instead of implementing this specific feature we should look at adding generic support for a pre-call hook, where devs/users can define a function to do anything before each API call.

The only area I see is to collect metrics for each API call. But I'm not sure whether we need to mix reauth and pre/post request hooks. IMHO this is something different and requires more detailed discussion, e.g. which values to share with the func, etc.

[jtopjian]

IMHO this is something different and requires more detailed discussion, e.g. which values to share with the func, etc.

It might be different, but it's worth putting some due diligence into and exploring. Please feel free to open a PR with this feature. We can review it and see if it's easy to abstract into a generic pre-call hook.

It's quite possible it's not easy to abstract. I think it would be incredibly difficult to abstract the 401 reauth into a generic post-call hook and so the same may be true here. But let's explore it so we're not in a situation where we implement this preemptive reauth and then later end up implementing pre-call hooks in the next line of code.

[kayrus]

One of the ideas of post-call hooks is to retry a call, when a server returned an error, e.g. 502 net/http timeout.