You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "rajinisivaram (via GitHub)" <gi...@apache.org> on 2023/02/07 11:15:05 UTC

[GitHub] [kafka] rajinisivaram opened a new pull request, #13211: KAFKA-14676: Include all SASL configs in login cache key to ensure clients in a JVM can use different OAuth configs

rajinisivaram opened a new pull request, #13211:
URL: https://github.com/apache/kafka/pull/13211

   We currently cache login managers in static maps for both static JAAS config using system property and for JAAS config specified using Kafka config `sasl.jaas.config`. In addition to the JAAS config, the login manager callback handler is included in the key, but all other configs are ignored. This implementation is based on the assumption clients that require different logins (e.g. username/password) use different JAAS configs, because login properties are included in the JAAS config rather than as separate top-level configs. The OIDC support added in KIP-768 only allows configuration of token endpoint URL as a top-level config. This results in two clients in a JVM configured with different token endpoint URLs to incorrectly share a login.
   
   This PR includes all SASL configs prefixed with `sasl.` to be included in the key so that logins are only shared if all the sasl configs are identical.
   
   Two rejected approaches:
   1) Only include `sasl.oauthbearer.token.endpoint.url` in the key. Rejected because there may be other configs in future for which we want different logins. It may also be useful to have a mechanism to force different logins when using custom configs, `sasl.` prefix would work in general.
   2) Include all configs in the key. Kafka brokers create multiple inter-broker clients with different configs, e.g. client.id. We want brokers to create only one login, hence chose a subset based on the `sasl.` prefix.
   
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] kirktrue commented on pull request #13211: KAFKA-14676: Include all SASL configs in login cache key to ensure clients in a JVM can use different OAuth configs

Posted by "kirktrue (via GitHub)" <gi...@apache.org>.
kirktrue commented on PR #13211:
URL: https://github.com/apache/kafka/pull/13211#issuecomment-1421255523

   @rajinisivaram - LGTM.
   
   It took me a second to verify that the `configs` that was passed into the `LoginMetadata ` wasn't being used directly in the `hashCode` and `equals` methods.
   
   Thanks for the fix!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] rajinisivaram commented on pull request #13211: KAFKA-14676: Include all SASL configs in login cache key to ensure clients in a JVM can use different OAuth configs

Posted by "rajinisivaram (via GitHub)" <gi...@apache.org>.
rajinisivaram commented on PR #13211:
URL: https://github.com/apache/kafka/pull/13211#issuecomment-1427116236

   @omkreddy @kirktrue Thanks for the reviews. Test failures not related, merging to trunk.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] rajinisivaram merged pull request #13211: KAFKA-14676: Include all SASL configs in login cache key to ensure clients in a JVM can use different OAuth configs

Posted by "rajinisivaram (via GitHub)" <gi...@apache.org>.
rajinisivaram merged PR #13211:
URL: https://github.com/apache/kafka/pull/13211


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] rajinisivaram commented on pull request #13211: KAFKA-14676: Include all SASL configs in login cache key to ensure clients in a JVM can use different OAuth configs

Posted by "rajinisivaram (via GitHub)" <gi...@apache.org>.
rajinisivaram commented on PR #13211:
URL: https://github.com/apache/kafka/pull/13211#issuecomment-1427120216

   Also cherry-picked to 3.4 and 3.3 branches.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org