pan3793 commented on PR #7418: URL: https://github.com/apache/kyuubi/pull/7418#issuecomment-4350391087
@aajisaka I recall the background. The security feature is requested and implemented by @turboFei from eBay. At the beginning, Kyuubi only supports interactive mode, there is no communication between Kyuubi server instances, so security applies to `server <=> engine` communication, and the feature is named as `kyuubi.engine.security.enabled`. Later, Kyuubi added support for batch mode, and the HA implementation contains the HTTP request between Kyuubi server instances, it reuses `kyuubi.engine.security.enabled` config, and expands the scope to all internal communications - both `server <=> server` and `server <=> engine`. What's confusing? 1. the config name, obviously, `kyuubi.engine.security.*` is misleading, we might change it to `kyuubi.internal.security.*` and add a fallback to avoid breaking changes. 2. `kyuubi.engine.security.enabled` must be set to `true` to enable batch HA, this was strongly insisted on by the feature author @turboFei, for security reasons. But I think we should remove such a requirement. (as a workaround, I introduced `SimpleEngineSecuritySecretProviderImpl` to allow admin to configure pre-share secret key on `kyuubi-defaults.conf`, to avoid introducing a zk dependency to store the secret key). This should be documented and simplified, otherwise, users are not able to enable the batch HA without looking at the code in depth. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
