You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/01 22:16:29 UTC

[GitHub] [pulsar] cdbartholomew opened a new pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

cdbartholomew opened a new pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411


   Fixes #8091
   
   ### Motivation
   
   
   In order to use authentication in Presto (and Pulsar SQL), the password authenticators plugin must be present in the distribution. If you configure password authentication and the plugin is not present, the server will not start.
   
   ### Modifications
   
   This update pulls in the password authenticators plugin from the PrestoSQL build and packages as part of the presto distribution used by the `pulsar sql-worker run`  command.
   
   ### Verifying this change
   
   I have built a Docker image with this change and confirmed that file-based password authentication works with this change. Since the plugin comes from the Presto build, I don't think we need any extra tests for it. Adding an integration test would be difficult because configuring authentication in Presto requires HTTPS to also be set up.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): yes. Adds a new dependency on the Presto password plugin.
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - Anything that affects deployment: no
   
   ### Documentation
   
   The plugin can be configured by modifying the files in the `/etc/conf/presto` directory by following the PulsarSQL (now Trino) documentation. For example, this [link](https://trino.io/docs/current/security/password-file.html#) explains has to configure file-based password authentication. 


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] codelipenghui commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-1058893777


   The pr had no activity for 30 days, mark with Stale label.


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] github-actions[bot] commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-1058896691


   @cdbartholomew:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)


-- 
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: commits-unsubscribe@pulsar.apache.org

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



[GitHub] [pulsar] sijie commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-771333984


   @cdbartholomew okay. Can you add an integration test to verify this work?


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] zzzming commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
zzzming commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-771220888


   LGTM


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] zzzming commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
zzzming commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-771220888


   LGTM


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] sijie commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-771333984


   @cdbartholomew okay. Can you add an integration test to verify this work?


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] cdbartholomew commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
cdbartholomew commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-771269210


   Adding the configuration will just cause the server to fail to start, like this: 
   
   ```2021-02-02T00:54:38.790Z	INFO	main	io.prestosql.server.security.PasswordAuthenticatorManager	-- Loading password authenticator --
   2021-02-02T00:54:38.791Z	ERROR	main	io.prestosql.server.Server	Password authenticator 'file' is not registered
   java.lang.IllegalStateException: Password authenticator 'file' is not registered
   ```
   
   To get it to work, you to do item 2,  listed in issue #8091: "include the plugin https://mvnrepository.com/artifact/io.prestosql/presto-password-authenticators in the pulsar image and put in the presto plugin folder."
   
   If you don't believe me, you are welcome to try it.


----------------------------------------------------------------
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.

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



[GitHub] [pulsar] cdbartholomew commented on pull request #9411: [Issue 8091][Pulsar SQL] Add presto password authenticators plugin

Posted by GitBox <gi...@apache.org>.
cdbartholomew commented on pull request #9411:
URL: https://github.com/apache/pulsar/pull/9411#issuecomment-771269210


   Adding the configuration will just cause the server to fail to start, like this: 
   
   ```2021-02-02T00:54:38.790Z	INFO	main	io.prestosql.server.security.PasswordAuthenticatorManager	-- Loading password authenticator --
   2021-02-02T00:54:38.791Z	ERROR	main	io.prestosql.server.Server	Password authenticator 'file' is not registered
   java.lang.IllegalStateException: Password authenticator 'file' is not registered
   ```
   
   To get it to work, you to do item 2,  listed in issue #8091: "include the plugin https://mvnrepository.com/artifact/io.prestosql/presto-password-authenticators in the pulsar image and put in the presto plugin folder."
   
   If you don't believe me, you are welcome to try it.


----------------------------------------------------------------
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.

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