vvysotskyi commented on pull request #2162: URL: https://github.com/apache/drill/pull/2162#issuecomment-775383960
Hello @JohnOmernik Thanks a lot for reviewing the document and your comments, they all are major. Doing these changes I had some similar objections, but finally decided to do it as it is. The problem we trying to solve here is to add functionality for hiding credentials from the user that creates the storage plugin. Users can only specify how to access credentials, but it doesn't mean that the user knows actual values stored there, and he shouldn't. Actual credentials may change, but references may leave the same without affecting target users. Providing some functionality for storing credentials automatically means providing functionality for displaying their values, but as mentioned above, I don't think we want to do that. Having `HadoopCredentialsProvider` is consistent with the Hadoop way of providing configuration, but not more. It may be useful for the case when we have some pre-defined credentials that may be used in storage plugins, but it is not aimed for the case of frequently added/updated credentials. `EnvCredentialsProvider` has a similar use case, but for the k8s world. Some pre-defined credentials may be exposed into the container as env variables, so Drill can use them, it may also be useful, but for limited cases as the earlier one. I would expect `VaultCredentialProvider` to be used more compared to the two previous ones. I assumed the flow would be the following: administrator configures Drill with specified vault address and token. After that, users either ask the admin or use some tool for adding/viewing/updating credentials. Currently, Drill differentiates only two user kinds: admins and regular users. Admins can create storage plugins, and all plugins created by one admin are visible to all other admins. This PR doesn't aim to improve Drill user management, it just provides minimal functionality for storage plugins credentials, so currently we use the same Vault token for all users. There is also a discussion for improving Drill roles and permissions: https://lists.apache.org/thread.html/rbb4a83a67ee92b081d0ce3a17533dce5bfaf31b4dcc10ee9fd3cff33%40%3Cdev.drill.apache.org%3E. One of the ideas @vdiravka proposed is integrating with Vault. So in this case, the user will already be associated with some Vault token and this value may be used instead of providing the token in configs, hope we will follow that way. ---------------------------------------------------------------- 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: [email protected]
