flyte icon indicating copy to clipboard operation
flyte copied to clipboard

[Housekeeping] flytekit's AuthUnaryInterceptor should not have to try to refresh credentials on grpc.StatusCode.UNKNOWN

Open fg91 opened this issue 1 year ago • 5 comments

Describe the issue

Currently, flytekit's AuthUnaryInterceptor lazily tries to refresh credentials, not only when a call to flyteadmin fails with 401 or grpc status 16 (UNAUTHENTICATED) but also grpc status 2 (UNKOWN):

class AuthUnaryInterceptor(grpc.UnaryUnaryClientInterceptor, grpc.UnaryStreamClientInterceptor):

    def intercept_unary_unary(
        ...
    ):
            if e.code() == grpc.StatusCode.UNAUTHENTICATED or e.code() == grpc.StatusCode.UNKNOWN:
                self._authenticator.refresh_credentials()

The reason is that the python grpc client does not correctly map status codes if the server responds with a content-type other than application/grpc.

We reported the issue here.

What if we do not do this?

Uncleanliness. This is a housekeeping note that this can be removed in the future if the underlying issue is fixed in the grpc python client.

Related component(s)

flytekit

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

  • [X] Yes

Have you read the Code of Conduct?

  • [X] Yes

fg91 avatar Apr 10 '24 20:04 fg91

@wild-endeavor fyi

fg91 avatar Apr 10 '24 20:04 fg91

Wait @fg91 I thought you tried this again and said it still wasn't working?

wild-endeavor avatar Apr 11 '24 17:04 wild-endeavor

Had a brief discussion, thank you again @fg91 for staying on top of this. Unfortunately the underlying issue is still not resolved so there's nothing to do at this time (circa Q2 2024).

Basically the backstory to this is that between grpcio 1.53.0 and 1.61.x the grpc channel completely crashed when the server responded with content-type != application/grpc. Then, after 1.62.0 the channel doesn’t crash and you can try to refresh credentials and try again with the same channel. But even with 1.62.0 and onwards is that the content-type is not correct, the error is grpc status 2 (unknown) even if it actually was 401/status-code 16.

So we currently still need to || e.code() == grpc.StatusCode.UNKNOWN in the auth interceptor.

Will keep this ticket open.

wild-endeavor avatar Apr 11 '24 20:04 wild-endeavor