[
https://issues.apache.org/jira/browse/DRILL-5664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16083290#comment-16083290
]
ASF GitHub Bot commented on DRILL-5664:
---------------------------------------
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/870#discussion_r126840721
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/store/StoragePluginRegistryImpl.java
---
@@ -146,23 +148,37 @@ public void init() throws DrillbitStartupException {
String pluginsData = Resources.toString(url, Charsets.UTF_8);
StoragePlugins plugins =
lpPersistence.getMapper().readValue(pluginsData, StoragePlugins.class);
for (Map.Entry<String, StoragePluginConfig> config : plugins) {
- if (!definePluginConfig(config.getKey(), config.getValue()))
{
+
+ final String pluginName = config.getKey();
+ final StoragePluginConfig pluginConfig = config.getValue();
+
+ if (!definePluginConfig(pluginName, pluginConfig)) {
logger.warn("Duplicate plugin instance '{}' defined in
[{}, {}], ignoring the later one.",
- config.getKey(), pluginURLMap.get(config.getKey()),
url);
+ config.getKey(), pluginURLMap.get(pluginName), url);
continue;
}
- pluginURLMap.put(config.getKey(), url);
+ pluginURLMap.put(pluginName, url);
}
}
} else {
throw new IOException("Failure finding " +
ExecConstants.BOOTSTRAP_STORAGE_PLUGINS_FILE);
}
}
- Map<String, StoragePlugin> activePlugins = new HashMap<String,
StoragePlugin>();
+ final Map<String, StoragePlugin> activePlugins = new HashMap<String,
StoragePlugin>();
for (Map.Entry<String, StoragePluginConfig> entry :
Lists.newArrayList(pluginSystemTable.getAll())) {
- String name = entry.getKey();
- StoragePluginConfig config = entry.getValue();
+ final String name = entry.getKey();
+ final StoragePluginConfig config = entry.getValue();
+
+ // Update the security setting inside StoragePluginConfig based on
secureStoragePlugin flag
--- End diff --
This doesn't seem right at all... We are updating in ZK the value of a
plugin property based on something defined in config. All this can ever be is
wrong an out-of-date. If the user changes the plugin after start, do we honor
the boot option or the storage plugin option? Will they understand when we
change the option back on next restart? A big mess...
Better: add a new `configure(PluginContext context)` method to the base
plugin class. One hopes that there is an abstract base class that can provide
the default implementation.
Then, define the `PluginContext` class with information of interest to the
plugins. This might include the `DrillConfig` and your security setting.
Plugins are then obligated to use the provided value; not one stored in the
plugin config.
But, this has a separate problem. What if I have two Hive servers: one
which is secure, and one which is open? The same rule must be applied to both.
So, this argument says that security of a Hive server is *not* a Drill
boot-time config setting, it is instead and attribute of the remote system and
*should* be part of the storage plugin config, and should *not* be altered by
Drill.
> Enable security for Drill HiveStoragePlugin based on a config parameter
> -----------------------------------------------------------------------
>
> Key: DRILL-5664
> URL: https://issues.apache.org/jira/browse/DRILL-5664
> Project: Apache Drill
> Issue Type: Improvement
> Affects Versions: 1.11.0
> Reporter: Sorabh Hamirwasia
> Assignee: Sorabh Hamirwasia
>
> For enabling security on DrillClient to Drillbit and Drillbit to Drillbit
> channel we have a configuration. But this doesn't ensure that Storage Plugin
> channel is also configured with security turned on. For example: When
> security is enabled on Drill side then HiveStoragePlugin which Drill uses
> doesn't open secure channel to HiveMetastore by default unless someone
> manually change the HiveStoragePluginConfig.
> With this JIRA we are introducing a new config option
> _security.storage_plugin.enabled: false_ based on which Drill can update the
> StoragePlugin config's to enable/disable security. When this config is set to
> true/false then for now Drill will update the HiveStoragePlugin config to set
> the value of _hive.metastore.sasl.enabled_ as true/false. So that when Drill
> connects to Metastore it does so in secured way. But if an user tries to
> update the config later which is opposite of what the Drill config says then
> we will log a warning before updating.
> Later the same login can be extended for all the other storage plugin's as
> well to do respective setting change based on the configuration on Drill side.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
