[BUG] External command only gets called once

[andrei-trandafir]

Describe the bug

For services that use the ExternalCommand auth type with the admin server, the command execution only happens once when the flyteidl client is instantiated.

This behaviour was introduced in issue #5606 (PR #5686) when optimisations were made to reduce the number of calls to the admin authentication metadata endpoints. These changes also resulted in putting the creation of the token source provider for the client behind a Once synchronization (link).

As the external command gets executed in the token source provider (link), this only gets executed once, resulting in errors if the provided token expires or needs to be refreshed for any reason.

This bug broke our propeller instance when upgrading from version 1.11.0 to 1.14.1 as our external command returns a token that expires afters 45 minutes and results in a Unauthenticated error being returned from the admin instance.

Expected behavior

The external command should be re-executed whenever authentication fails. This ensures that if the external command expires or depends on some external configuration that might change, the command is re-run and authentication can continue.

This can be implemented by creating a custom externalCommandTokenSource that runs the command instead of the token source provider. Given that the token would be saved into the token cache, this command wouldn't be called until the token would expire, effectively providing the same experience for deployments that have non-expiring tokens.

This approach keeps the same behaviour for other authentication methods and the same behaviour of caching the authentication metadata that was originally introduced in the PR mentioned earlier.

Additional context to reproduce

1. Deploy a flyte cluster where the propeller uses ExternalCommand authentication where the command returns a token that will expire.
2. Wait until the token expires

Screenshots

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[wild-endeavor]

@andrei-trandafir just to be clear this is what you're referring to correct? https://github.com/flyteorg/flyte/pull/6271/files

[andrei-trandafir]

@wild-endeavor that's exactly it, I've already opened a PR myself:
#6255

[wild-endeavor]

Oh sorry... I completely missed that. I'm not used to not seeing the pr badge. Mind adding some unit tests to your PR and testing it?

I did my best to go through all the calls to Token() last night, I don't think there's anything crazy, but is there any chance you can test the change against your backend, just to make sure it's making the number of calls you expect?

[andrei-trandafir]

Yeah, I'll be adding some tests soon, sorry for the delay on it.

We've already added this change to our instance and so far it seems to be working as expected.

[wild-endeavor]

thanks! let me know when it's ready for review and i'll close my pr.