Hi all, With four binding +1 votes (Manikumar Reddy, Rajini Sivaram, David Jacot, and Ismael Juma) and one non-binding +1 vote (Ron Dagostino), the vote passes.
Thanks everyone! Kirk On Fri, Oct 15, 2021, at 10:40 AM, Ismael Juma wrote: > Thanks Kirk, I'm overall +1 (binding). I think the timeouts may need a bit of > tweaking still. We can update the KIP and the thread if we decide to do that > as part of the PR review. > > Ismael > > On Thu, Oct 14, 2021 at 11:59 AM Kirk True <k...@mustardgrain.com> wrote: >> __ >> Hi Ismael, >> >> Thanks for reviewing the KIP. I've made a first pass at updating based on >> your feedback. >> >> Questions/comments inline... >> >> On Thu, Oct 14, 2021, at 6:20 AM, Ismael Juma wrote: >>> Hi Kirk, >>> >>> Thanks for the KIP. It looks good overall to me. A few nits: >>> >>> 1. "sasl.login.retry.wait.ms": these configs are typically called `backoff` >>> in Kafka. For example "retry.backoff.ms". The default for `retry.backoff.ms` >>> is 100ms. Is there a reason why we are using a different value for this >>> one? The `sasl.login.retry.max.wait.ms` should be renamed accordingly. >> >> >> Changed to sasl.login.retry.backoff.ms and sasl.login.retry.backoff.max.ms >> and changed the former to 100 ms. >> >>> 2. "sasl.login.attempts": do we need this at all? We have generally moved >>> away from number of retries in favor of timeouts for Kafka (the producer >>> has a retries config partly for historical reasons, but partly due to >>> semantics that are specific to the producer. >> >> Removed this option and now we just retry up to >> sasl.login.retry.backoff.max.ms. >> >>> 3. "sasl.login.read.timeout.ms" : we have two types of kafka timeouts, " >>> default.api.timeout.ms" and "request.timeout.ms". Is this similar to any of >>> the two or is it different? If similar to one of the existing ones, we >>> should name it similarly. >> >> This is specifically for the setReadTimeout on java.net.URLConnection when >> making the call to the OAuth/OIDC provider to retrieve the token. So it is >> SASL specific because reading the response from the external OAuth/OIDC >> provider (likely over WAN) may require a longer timeout. >> >>> 4. "sasl.login.connect.timeout.ms": is this the equivalent of " >>> socket.connection.setup.timeout.ms" in Kafka? I am unsure why we chose such >>> a long name, "connect.timeout.ms" would have been a lot better. However, if >>> it is similar, then we may want to follow the same naming convention. >> >> This is for the setConnectTimeout on java.net.URLConnection, similar to the >> above. >> >>> 5. Should there be a "connect.max.timeout.ms" too? >> >> AFAIK, we don't have that level of control per our use of URLConnection. >> >>> 6. What are the compatibility guarantees offered by the >>> "OAuthCompatibilityTest" CLI tool? Also, can we adjust the name so it's >>> clear that it's a Command versus a test suite? >> >> I changed the name to OAuthCompatibilityTool. Can you elaborate on what >> compatibility guarantees you'd like to see listed? I may just be >> misunderstanding the request. >> >> Thanks, >> Kirk >> >>> >>> Thanks! >>> >>> Ismael >>> >>> On Mon, Sep 27, 2021 at 10:20 AM Kirk True <k...@mustardgrain.com> wrote: >>> >>> > Hi all! >>> > >>> > I'd like to start a vote for KIP-768 that allows Kafka to connect to an >>> > OAuth/OIDC identity provider for authentication and token retrieval: >>> > >>> > >>> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=186877575 >>> > >>> > Thanks! >>> > Kirk >>> >>
