Before I explain what the patch does, here’s what the code is currently doing (which was’t super-obvious to me at first):
- Read in the cached token, if enabled
- If we have a token, make a connection to the server to test if the token is still valid (i.e. hasn’t expired or been tampered with)
- If it’s not valid, then let’s delete it
- If we don’t have a token (or it was deleted in the previous step), get a new one from the Server using Kerberos or Pseudo auth (which uses a new connection under the covers)
- If we now have a new token, write it to the cache file (if enabled)
- Create a new connection using the token and return it up to the caller, which may further modify it and then use it
As you can see, we may make 2-3 connections per REST call. This is partly due to how the code is setup where the auth client code passes the connection to other code, which doesn’t know about auth. While that could be changed, it would require a lot of refactoring and changing how the OozieClient and AuthOozieClient are setup.
The main new thing is the else statement with the big comment. Most of the other stuff is some minor refactoring, some helpful comments, and an optimization that also prevents a rare race condition. Here’s a summary of what I changed:
I think checking the expiration time locally should only be a problem if the local clock is way off (in which case other things like Kerberos won’t work anyway). If the clock is wrong, either it won’t delete the token in which case the check to the server will take care of it, or it will delete the token early in which case we’ll just get a new one. So I think this should be fine, but if others don’t think we should do this, I’m fine with removing it.
I wasn't able to get MiniKDC working. Having the test working with Kerberos would be good though, but I don't think it should block getting this fix in. Once this is in, I'll create another JIRA and upload what I have so far there. Maybe someone else can figure it out.