cgivre commented on code in PR #3056:
URL: https://github.com/apache/drill/pull/3056#discussion_r3839544015
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java:
##########
@@ -245,6 +245,12 @@ public RelRoot toRel(final SqlNode validatedNode) {
RelNode project = LogicalProject.create(rel.rel,
Collections.emptyList(), expressions, rel.validatedRowType);
rel = RelRoot.of(project, rel.validatedRowType, rel.kind);
}
+
+ // Column-level SELECT authorization check. Done after SqlToRelConverter
has
+ // resolved all column references (so we can trace each to its TableScan)
+ // and before flattenTypes/optimization (so column references are intact).
+ new ColumnAccessChecker(session, drillConfig,
cluster.getMetadataQuery()).check(rel.rel);
Review Comment:
**Planning-time cost is paid on every query, even when Ranger is disabled.**
`check()` walks the whole tree and calls
`RelMetadataQuery.getColumnOrigins()` once per output column of every node
*before* anything consults `authorizer.isEnabled()` — that test lives down in
`ColumnAccessChecker.enforceColumnAccess()`, which only runs after the
traversal has already happened. `getColumnOrigins` is not a cheap metadata
query on wide row types or deep plans.
Since `drill.exec.security.ranger.enabled` defaults to `false`, every
existing Drill deployment pays this on every `toRel` for a feature it isn't
using.
Suggest hoisting the check here:
```java
AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
if (authorizer.isEnabled()) {
new ColumnAccessChecker(session, drillConfig, cluster.getMetadataQuery(),
authorizer).check(rel.rel);
}
```
and passing the resolved `authorizer` into the checker so
`enforceColumnAccess` stops re-resolving it per table.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java:
##########
@@ -103,14 +106,99 @@ void disallowTemporaryTables() {
public Prepare.PreparingTable getTable(List<String> names) {
checkTemporaryTable(names);
Prepare.PreparingTable table = super.getTable(names);
- DrillTable drillTable;
- if (table != null && (drillTable = table.unwrap(DrillTable.class)) !=
null) {
- drillTable.setOptions(session.getOptions());
-
drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names,
drillTable)));
+ if (table != null) {
+ // Ranger SELECT authorization check for ALL table types, including
+ // JDBC storage plugin tables (JdbcTable) that are not DrillTable.
+ checkTableAccess(table, names);
+
+ DrillTable drillTable = table.unwrap(DrillTable.class);
+ if (drillTable != null) {
+ drillTable.setOptions(session.getOptions());
+
drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names,
drillTable)));
+ }
}
return table;
}
+ /**
+ * Checks SELECT permission on the resolved table via the configured {@link
AccessAuthorizer}
+ * (Ranger by default). No-op when authorization is disabled (fail-open).
System schemas
+ * (INFORMATION_SCHEMA, sys) are bypassed inside the authorizer
implementation.
+ *
+ * <p>Extracts datasource/schema/table from the resolved qualified name so
it works
+ * for both DrillTable (native storage plugins) and non-DrillTable (JDBC
storage plugin).
+ * The {@code names} argument is used as a fallback to determine the
datasource when
+ * the qualified name does not expose it.
+ */
+ private void checkTableAccess(Prepare.PreparingTable table, List<String>
names) {
+ AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
+ if (!authorizer.isEnabled()) {
+ return; // fail-open when disabled
+ }
+ // Use the resolved qualified name (includes default schema resolution)
rather than
+ // the raw input names, which may be incomplete when the user omits the
schema.
+ //
+ // Ranger four-level resource model: datasource / schema / table / column.
+ // The first segment of qualifiedName is the datasource (storage plugin
name);
+ // the LAST segment is the table; any segments in between form the schema
path.
+ // For example "mysql.shf.users" -> datasource=mysql, schema=shf,
table=users.
+ // The schema MUST NOT include the datasource prefix, otherwise Ranger
policy
+ // matching fails (policy has schema=shf but request sends
schema=mysql.shf).
+ //
+ // Some backends have no schema concept (e.g. a flat file store). To keep
the
+ // four-level model uniform, we synthesize a default schema per datasource
via
+ // getDefaultSchemaByDataSource(). The default branch returns the
datasource
+ // name itself so each storage plugin gets its own default schema namespace
+ // until an explicit mapping is added.
+ List<String> qualifiedName = table.getQualifiedName();
+ String tableName = qualifiedName.get(qualifiedName.size() - 1);
+ String dataSource;
+ String schemaPath;
+ if (qualifiedName.size() > 2) {
+ // datasource.schema.table OR datasource.subschema.table
+ dataSource = qualifiedName.get(0);
+ schemaPath = SchemaUtilities.getSchemaPath(qualifiedName.subList(1,
qualifiedName.size() - 1));
+ } else if (qualifiedName.size() == 2) {
+ // datasource.table — backend has no schema; synthesize a default so the
+ // four-level resource stays complete (Ranger policy matching requires a
+ // non-null schema key when schema is a mandatory resource).
+ dataSource = qualifiedName.get(0);
+ schemaPath = getDefaultSchemaByDataSource(dataSource);
+ } else {
+ // Single-element qualified name: fall back to the input names list to
find the datasource.
+ dataSource = !names.isEmpty() ? names.get(0) : tableName;
+ schemaPath = getDefaultSchemaByDataSource(dataSource);
+ }
+ String userName = session.getCredentials().getUserName();
+
+ if (!authorizer.checkTableAccess(userName, dataSource, schemaPath,
tableName, AccessTypes.SELECT)) {
+ throw UserException.permissionError()
+ .message("Access denied: user '%s' lacks SELECT privilege on
%s.%s.%s",
+ userName, dataSource, schemaPath, tableName)
+ .build(logger);
+ }
+ }
+
+ /**
+ * Returns the default schema name to use when a table's qualified name does
not
+ * contain an explicit schema segment (i.e. two-segment {@code
datasource.table}
+ * or a single-segment fallback). This keeps the Ranger four-level resource
+ * model complete even for backends that have no native schema concept.
+ *
+ * <p>Add explicit cases below as new storage plugins are integrated. The
+ * {@code default} branch returns the datasource name itself so each plugin
+ * gets a distinct default schema namespace without further
configuration.</p>
+ *
+ * @param dataSource the storage plugin / datasource name
+ * @return a non-null default schema name
+ */
+ static String getDefaultSchemaByDataSource(String dataSource) {
+ return switch (dataSource.toLowerCase()) {
+ case "dfs", "cp" -> "default";
+ default -> dataSource;
+ };
Review Comment:
Hardcoding `dfs` and `cp` here doesn't hold: storage plugin names are chosen
by the operator. `dfs` and `cp` are conventions from the bundled
`bootstrap-storage-plugins.json`, not reserved names — a site can rename them,
delete them, or add a second filesystem plugin under any name, and then the
synthesized schema silently changes from `"default"` to the plugin name and
stops matching the Ranger policy the admin wrote.
Since this value participates in policy matching, it should be configurable
rather than compiled in — e.g. a `drill.exec.security.ranger.default_schema`
map in `drill-module.conf`, defaulting to the plugin name for anything not
listed. That also keeps `DrillCalciteCatalogReader` from having to know
anything about specific plugins.
Minor: the javadoc says "Add explicit cases below as new storage plugins are
integrated" — that's the part I'd like to avoid, since it makes the security
mapping a code change rather than a config change.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/security/ranger/AccessAuthorizerFactory.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.security.ranger;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Singleton factory for {@link AccessAuthorizer}. Follows the same
configuration-driven
+ * reflective loading pattern as
+ * {@link org.apache.drill.exec.rpc.user.security.UserAuthenticatorFactory}.
+ *
+ * <p>When {@code drill.exec.security.ranger.enabled} is {@code false} (the
default),
+ * a {@link NoOpAccessAuthorizer} is returned. When enabled, the
implementation class
+ * named by {@code drill.exec.security.ranger.impl} is loaded reflectively and
+ * initialized with the configured service name.</p>
+ */
+public class AccessAuthorizerFactory {
+ private static final Logger logger =
LoggerFactory.getLogger(AccessAuthorizerFactory.class);
+
+ // Default values used when the corresponding config keys are absent.
+ // Package-private so tests can verify the default-impl selection logic
+ // without triggering RangerAccessAuthorizer.init() (which calls
+ // RangerPluginClassLoader.getInstance() and crashes the JVM in the test
+ // environment).
+ static final String DEFAULT_AUTHORIZER_IMPL =
+ "org.apache.drill.exec.security.ranger.RangerAccessAuthorizer";
+ static final String DEFAULT_SERVICE_NAME = "drill";
+
+ private static volatile AccessAuthorizer instance;
+
+ private AccessAuthorizerFactory() {
+ }
+
+ /**
+ * Returns the singleton {@link AccessAuthorizer} instance, initializing it
from
+ * the given configuration on first call.
+ *
+ * @param config the Drill configuration
+ * @return the authorizer (never {@code null})
+ */
+ public static AccessAuthorizer getAuthorizer(DrillConfig config) {
Review Comment:
**JVM-wide static singleton pinned by whichever `DrillConfig` calls first.**
`instance` is `static`, has no reset, and ignores the `config` argument on
every call after the first. Drill routinely runs more than one Drillbit in a
single JVM — `ClusterFixture` / `BaseTestQuery` spin up multi-node clusters,
and embedded mode shares the process with the client. The first
`getAuthorizer(config)` call wins for the lifetime of the process, so a second
Drillbit configured with Ranger enabled silently gets the
`NoOpAccessAuthorizer` created for the first one (or vice versa).
That also makes this effectively untestable in-process: once any test
touches it, every later test in the same fork sees the cached instance.
`AccessAuthorizerFactoryTest` works around this today, which is a signal.
Suggest scoping the authorizer to `DrillbitContext` instead — construct it
once in `Drillbit.run()` and hand it to `SqlConverter` /
`DrillCalciteCatalogReader` through the context, the same way Drill scopes
`StoragePluginRegistry` and `OptionManager`. That removes the static entirely
and makes lifecycle (including `cleanUp()` on the Ranger plugin, which
currently never gets called) explicit.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/security/ranger/AccessAuthorizerFactory.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.security.ranger;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Singleton factory for {@link AccessAuthorizer}. Follows the same
configuration-driven
+ * reflective loading pattern as
+ * {@link org.apache.drill.exec.rpc.user.security.UserAuthenticatorFactory}.
+ *
+ * <p>When {@code drill.exec.security.ranger.enabled} is {@code false} (the
default),
+ * a {@link NoOpAccessAuthorizer} is returned. When enabled, the
implementation class
+ * named by {@code drill.exec.security.ranger.impl} is loaded reflectively and
+ * initialized with the configured service name.</p>
+ */
+public class AccessAuthorizerFactory {
+ private static final Logger logger =
LoggerFactory.getLogger(AccessAuthorizerFactory.class);
+
+ // Default values used when the corresponding config keys are absent.
+ // Package-private so tests can verify the default-impl selection logic
+ // without triggering RangerAccessAuthorizer.init() (which calls
+ // RangerPluginClassLoader.getInstance() and crashes the JVM in the test
+ // environment).
+ static final String DEFAULT_AUTHORIZER_IMPL =
+ "org.apache.drill.exec.security.ranger.RangerAccessAuthorizer";
+ static final String DEFAULT_SERVICE_NAME = "drill";
+
+ private static volatile AccessAuthorizer instance;
+
+ private AccessAuthorizerFactory() {
+ }
+
+ /**
+ * Returns the singleton {@link AccessAuthorizer} instance, initializing it
from
+ * the given configuration on first call.
+ *
+ * @param config the Drill configuration
+ * @return the authorizer (never {@code null})
+ */
+ public static AccessAuthorizer getAuthorizer(DrillConfig config) {
+ if (instance != null) {
+ return instance;
+ }
+ synchronized (AccessAuthorizerFactory.class) {
+ if (instance != null) {
+ return instance;
+ }
+ instance = createAuthorizer(config);
+ return instance;
+ }
+ }
+
+ /**
+ * Resolves the authorizer implementation class name from configuration.
+ * Package-private for unit testing the selection logic without triggering
+ * {@code RangerAccessAuthorizer.init()}.
+ */
+ static String getImplClassName(DrillConfig config) {
+ return config.hasPath(ExecConstants.RANGER_AUTHORIZER_IMPL)
+ ? config.getString(ExecConstants.RANGER_AUTHORIZER_IMPL)
+ : DEFAULT_AUTHORIZER_IMPL;
+ }
+
+ /**
+ * Resolves the Ranger service name from configuration.
+ * Package-private for unit testing the selection logic.
+ */
+ static String getServiceName(DrillConfig config) {
+ return config.hasPath(ExecConstants.RANGER_SERVICE_NAME)
+ ? config.getString(ExecConstants.RANGER_SERVICE_NAME)
+ : DEFAULT_SERVICE_NAME;
+ }
+
+ private static AccessAuthorizer createAuthorizer(DrillConfig config) {
+ boolean enabled = config.hasPath(ExecConstants.RANGER_AUTH_ENABLED)
+ && config.getBoolean(ExecConstants.RANGER_AUTH_ENABLED);
+ if (!enabled) {
+ logger.info("Ranger authorization is disabled
(drill.exec.security.ranger.enabled=false)");
+ return new NoOpAccessAuthorizer();
+ }
+ String impl = getImplClassName(config);
+ String serviceName = getServiceName(config);
+ logger.info("Initializing Ranger authorizer: impl={}, service={}", impl,
serviceName);
+ try {
+ Class<?> clazz = Class.forName(impl);
+ AccessAuthorizer authorizer = (AccessAuthorizer)
clazz.getDeclaredConstructor().newInstance();
+ authorizer.init(serviceName);
+ logger.info("Ranger authorizer initialized, enabled={}",
authorizer.isEnabled());
+ return authorizer;
+ } catch (Exception e) {
+ logger.error("Failed to initialize Ranger authorizer {}, falling back to
fail-closed", impl, e);
+ throw new RuntimeException("Failed to initialize Ranger authorizer
impl:" + impl
+ + ", serviceName:" + serviceName, e);
+ }
Review Comment:
The log message says "falling back to fail-closed", but nothing falls back —
the next statement rethrows. Propagating is the right behaviour (a Drillbit
that can't initialise its authorizer should refuse to start, and
`Drillbit.run()` will surface it), the message just contradicts the code and
will send an operator looking for a fallback authorizer that doesn't exist.
```java
logger.error("Failed to initialize Ranger authorizer {}; Drillbit startup
will abort", impl, e);
throw new DrillRuntimeException(...);
```
Also worth using `DrillRuntimeException` rather than raw `RuntimeException`
for consistency with the rest of the codebase, and dropping `e.getMessage()`
string-concatenation in favour of the cause (which is already passed).
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/ColumnAccessChecker.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner.sql.conversion;
+
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttleImpl;
+import org.apache.calcite.rel.core.TableScan;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.logical.LogicalSort;
+import org.apache.calcite.rel.metadata.RelColumnOrigin;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexVisitorImpl;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.planner.sql.SchemaUtilities;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.security.ranger.AccessAuthorizer;
+import org.apache.drill.exec.security.ranger.AccessAuthorizerFactory;
+import org.apache.drill.exec.security.ranger.AccessTypes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Visitor that traverses a RelNode tree and enforces column-level SELECT
authorization
+ * via the configured {@link AccessAuthorizer} (Ranger by default).
+ *
+ * <p><b>Design:</b> For every RelNode that carries {@link RexNode} expressions
+ * (Project, Filter, Join, Aggregate, Sort), the visitor collects all {@link
RexInputRef}s
+ * and uses Calcite's {@link RelMetadataQuery#getColumnOrigins(RelNode, int)}
to trace each
+ * referenced column back to its originating {@link TableScan} column index.
This correctly
+ * handles multi-hop projections, filters, joins, and aggregations.</p>
+ *
+ * <p>For a {@link TableScan} that has NO traced column references (e.g.
+ * {@code SELECT * FROM t} with no intervening Project), ALL columns of that
table
+ * are checked.</p>
+ *
+ * <p>System schemas (INFORMATION_SCHEMA, sys) are bypassed inside the
authorizer
+ * implementation. When authorization is disabled, the visitor is a no-op
(fail-open).</p>
+ */
+class ColumnAccessChecker extends RelShuttleImpl {
+
+ private static final Logger logger =
LoggerFactory.getLogger(ColumnAccessChecker.class);
+
+ private final UserSession session;
+ private final DrillConfig drillConfig;
+ private final RelMetadataQuery mq;
+
+ // Records each table's referenced column indices. Uses IdentityHashMap
because
+ // RelOptTable equals/hashCode may be expensive or not identity-based.
+ private final Map<RelOptTable, Set<Integer>> tableToReferencedCols = new
IdentityHashMap<>();
+
+ ColumnAccessChecker(UserSession session, DrillConfig drillConfig,
RelMetadataQuery mq) {
+ this.session = session;
+ this.drillConfig = drillConfig;
+ this.mq = mq;
+ }
+
+ /**
+ * Entry point: traverse the tree and enforce column-level access.
+ */
+ void check(RelNode root) {
+ // Also trace the root node's output columns (covers bare TableScan root or
+ // top-level Project output).
+ traceOutputColumns(root);
+
+ // Walk the tree to collect RexInputRef origins from all
expression-bearing nodes.
+ root.accept(this);
+ }
+
+ // ------------------------------------------------------------------
+ // RelShuttle overrides — collect RexInputRefs from expression-bearing nodes
+ // ------------------------------------------------------------------
+
+ @Override
+ public RelNode visit(LogicalProject project) {
+ collectRefs(project.getProjects(), project.getInput());
+ return super.visit(project);
+ }
+
+ @Override
+ public RelNode visit(LogicalFilter filter) {
+ if (filter.getCondition() != null) {
+ analyzeRex(filter.getCondition(), filter.getInput(), -1, null);
+ }
+ return super.visit(filter);
+ }
+
+ @Override
+ public RelNode visit(LogicalJoin join) {
+ if (join.getCondition() != null) {
+ int leftCount = join.getLeft().getRowType().getFieldCount();
+ analyzeRex(join.getCondition(), join.getRight(), leftCount,
join.getLeft());
+ }
+ return super.visit(join);
+ }
+
+ @Override
+ public RelNode visit(LogicalAggregate aggregate) {
+ for (int i : aggregate.getGroupSet()) {
+ traceColumnOrigin(aggregate.getInput(), i);
+ }
+ return super.visit(aggregate);
+ }
+
+ @Override
+ public RelNode visit(LogicalSort sort) {
+ if (sort.getCollation() != null) {
+ sort.getCollation().getFieldCollations().forEach(fc ->
+ traceColumnOrigin(sort.getInput(), fc.getFieldIndex()));
+ }
+ return super.visit(sort);
+ }
+
+ @Override
+ public RelNode visit(TableScan scan) {
+ RelOptTable table = scan.getTable();
+
+ // Determine which columns to check: traced columns, or ALL if none were
traced
+ // (SELECT * FROM t case).
+ Set<Integer> referencedColIndices = tableToReferencedCols.get(table);
+ List<String> allColumnNames = scan.getRowType().getFieldNames();
+
+ Set<String> columnsToCheck;
+ if (referencedColIndices == null || referencedColIndices.isEmpty()) {
+ // SELECT * — check all columns
+ columnsToCheck = new HashSet<>(allColumnNames);
+ } else {
+ columnsToCheck = new HashSet<>();
+ for (int idx : referencedColIndices) {
+ if (idx >= 0 && idx < allColumnNames.size()) {
+ columnsToCheck.add(allColumnNames.get(idx));
+ }
+ }
+ }
+
+ if (columnsToCheck.isEmpty()) {
+ return scan;
+ }
+
+ enforceColumnAccess(table, columnsToCheck);
+ return scan;
+ }
+
+ /**
+ * Traces the output columns of a RelNode back to their table-scan origins.
+ */
+ private void traceOutputColumns(RelNode node) {
+ if (node == null) {
+ return;
+ }
+ int fieldCount = node.getRowType().getFieldCount();
+ for (int i = 0; i < fieldCount; i++) {
+ traceColumnOrigin(node, i);
+ }
+ }
+
+ /**
+ * Traces a single output column of {@code node} at index {@code
columnIndex} back to
+ * table-scan origins, recording them in {@link #tableToReferencedCols}.
+ */
+ private void traceColumnOrigin(RelNode node, int columnIndex) {
+ if (node == null || mq == null) {
+ return;
+ }
+ Set<RelColumnOrigin> origins;
+ try {
+ origins = mq.getColumnOrigins(node, columnIndex);
+ } catch (Exception e) {
+ logger.debug("getColumnOrigins failed for {} column {}", node,
columnIndex, e);
+ return;
+ }
+ if (origins == null) {
+ return;
+ }
+ for (RelColumnOrigin origin : origins) {
+ RelOptTable originTable = origin.getOriginTable();
+ if (originTable != null) {
+ // Record origins for ALL table types (DrillTable, JdbcTable, etc.).
+ // Previously this only recorded DrillTable origins, which caused
+ // JDBC storage plugin tables (JdbcTable) to be skipped entirely.
+ tableToReferencedCols
+ .computeIfAbsent(originTable, k -> new HashSet<>())
+ .add(origin.getOriginColumnOrdinal());
+ }
+ }
+ }
+
+ /**
+ * Collects RexInputRefs from a list of RexNodes and traces each to its
+ * table-scan origin via the input node's metadata. Also processes any
+ * {@link RexSubQuery} found in the expressions (scalar/IN/EXISTS subqueries)
+ * so that column references inside subqueries are authorized.
+ */
+ private void collectRefs(List<RexNode> rexNodes, RelNode inputNode) {
+ if (rexNodes == null || inputNode == null) {
+ return;
+ }
+ for (RexNode rex : rexNodes) {
+ analyzeRex(rex, inputNode, -1, null);
+ }
+ }
+
+ /**
+ * Analyzes a {@link RexNode} expression, collecting {@link RexInputRef}s and
+ * {@link RexSubQuery}s, tracing each input ref to its table-scan origin and
+ * recursively visiting each subquery's {@link RelNode} tree.
+ *
+ * @param rex the expression to analyze
+ * @param inputNode the input RelNode that RexInputRefs resolve against
+ * @param leftCount if {@code >= 0}, indicates a join condition: refs with
+ * index {@code < leftCount} resolve against {@code
leftInput},
+ * others resolve against {@code inputNode} (the right
input)
+ * with offset {@code leftCount}. If {@code < 0}, all refs
+ * resolve against {@code inputNode}.
+ * @param leftInput the left input of a join, or {@code null} when
+ * {@code leftCount < 0}.
+ */
+ private void analyzeRex(RexNode rex, RelNode inputNode, int leftCount,
RelNode leftInput) {
+ if (rex == null) {
+ return;
+ }
+ Set<Integer> refs = new HashSet<>();
+ List<RexSubQuery> subQueries = new ArrayList<>();
+ rex.accept(new RexRefCollector(refs, subQueries));
+ for (int refIndex : refs) {
+ if (leftCount >= 0 && refIndex < leftCount) {
+ traceColumnOrigin(leftInput, refIndex);
+ } else if (leftCount >= 0) {
+ traceColumnOrigin(inputNode, refIndex - leftCount);
+ } else {
+ traceColumnOrigin(inputNode, refIndex);
+ }
+ }
+ for (RexSubQuery sq : subQueries) {
+ // Trace the subquery's output columns to their table-scan origins.
+ // For scalar subqueries (e.g. SELECT sum(user_id) FROM t), this traces
+ // the aggregate output back to the underlying table column.
+ traceOutputColumns(sq.rel);
+ // Recursively visit the subquery's RelNode tree so that RexInputRefs
+ // inside the subquery (e.g. columns in WHERE/SELECT of the subquery)
+ // are also collected and traced.
+ sq.rel.accept(this);
+ }
+ }
+
+ /**
+ * Enforces column-level access for the given table and column set.
+ */
+ private void enforceColumnAccess(RelOptTable table, Set<String> columns) {
+ AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
+ if (!authorizer.isEnabled()) {
+ return; // fail-open when disabled
+ }
+
+ // Resolve datasource / schema / table from the qualified name, consistent
+ // with DrillCalciteCatalogReader.checkTableAccess(). This works for ALL
+ // table types (DrillTable, JdbcTable, etc.) — previously this method
+ // required a DrillTable and skipped JdbcTable, leaving JDBC storage
+ // plugin tables without column-level authorization.
+ List<String> qualifiedName = table.getQualifiedName();
+ String tableName = qualifiedName.get(qualifiedName.size() - 1);
+ String dataSource;
+ String schemaPath;
+ if (qualifiedName.size() > 2) {
+ dataSource = qualifiedName.get(0);
+ schemaPath = SchemaUtilities.getSchemaPath(qualifiedName.subList(1,
qualifiedName.size() - 1));
+ } else if (qualifiedName.size() == 2) {
+ dataSource = qualifiedName.get(0);
+ schemaPath =
DrillCalciteCatalogReader.getDefaultSchemaByDataSource(dataSource);
+ } else {
+ dataSource = tableName;
+ schemaPath =
DrillCalciteCatalogReader.getDefaultSchemaByDataSource(dataSource);
+ }
+ String userName = session.getCredentials().getUserName();
+
+ if (!authorizer.checkColumnAccess(userName, dataSource, schemaPath,
tableName, columns, AccessTypes.SELECT)) {
+ throw UserException.permissionError()
+ .message("Access denied: user '%s' lacks SELECT privilege on one or
more columns " +
+ "(%s) of table %s.%s.%s", userName, columns, dataSource,
schemaPath, tableName)
+ .build(logger);
+ }
+ }
+
+ /**
+ * RexVisitor that collects all {@link RexInputRef} indices and
+ * {@link RexSubQuery} instances encountered in a {@link RexNode} tree.
+ * <p> dispatches to
+ * {@link #visitSubQuery(RexSubQuery)} (not {@code visitCall}), so a plain
+ * {@code RexInputRef}-only visitor silently skips over subqueries. This
+ * collector overrides {@code visitSubQuery} to capture the subquery and then
+ * continues traversing its operands so that nested {@link RexInputRef}s
+ * (e.g. the left side of {@code x IN (SELECT ...)}) and nested subqueries
+ * are also collected.</p>
+ */
+ private static final class RexRefCollector extends RexVisitorImpl<Void> {
+ private final Set<Integer> refs;
+ private final List<RexSubQuery> subQueries;
+
+ RexRefCollector(Set<Integer> refs, List<RexSubQuery> subQueries) {
+ super(true);
+ this.refs = refs;
+ this.subQueries = subQueries;
+ }
+
+ @Override
+ public Void visitInputRef(RexInputRef ref) {
Review Comment:
**Column-level bypass: correlated references are never authorized.**
`RexRefCollector` recognises `RexInputRef` and `RexSubQuery`. An outer-query
column referenced *from inside* a correlated subquery is neither — Calcite
represents it as a `RexFieldAccess` whose referenceExpr is a
`RexCorrelVariable`. So it is never traced and never checked:
```sql
-- users.ssn is not in any policy, yet this is allowed
SELECT u.name
FROM users u
WHERE EXISTS (SELECT 1 FROM orders o WHERE o.id = u.ssn);
```
Test case 13 in `RangerAuthorization.md` covers the EXISTS shape but uses
`SELECT *` on the outer table, so the outer columns get authorized by the
`SELECT *` path and the hole is masked. A test that projects a narrow
authorized column while correlating on an unauthorized one would fail today.
Fix is an override on the collector plus a way to resolve the correl
variable back to the RelNode it was created from
(`LogicalCorrelate.getCorrelationId()` / `RelOptUtil.getVariablesUsed`, or
track `RexSubQuery` correlation ids as you descend):
```java
@Override
public Void visitFieldAccess(RexFieldAccess fieldAccess) {
RexNode ref = fieldAccess.getReferenceExpr();
if (ref instanceof RexCorrelVariable) {
correlRefs.add(Pair.of(((RexCorrelVariable) ref).id,
fieldAccess.getField().getIndex()));
return null;
}
return super.visitFieldAccess(fieldAccess);
}
```
then trace each collected `(correlId, ordinal)` against the RelNode that
defines that correlation variable.
Please also add a `RangerAuthorization.md` case for the narrow-projection
correlated form so this stays covered.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/ColumnAccessChecker.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner.sql.conversion;
+
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttleImpl;
+import org.apache.calcite.rel.core.TableScan;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.logical.LogicalSort;
+import org.apache.calcite.rel.metadata.RelColumnOrigin;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexVisitorImpl;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.planner.sql.SchemaUtilities;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.security.ranger.AccessAuthorizer;
+import org.apache.drill.exec.security.ranger.AccessAuthorizerFactory;
+import org.apache.drill.exec.security.ranger.AccessTypes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Visitor that traverses a RelNode tree and enforces column-level SELECT
authorization
+ * via the configured {@link AccessAuthorizer} (Ranger by default).
+ *
+ * <p><b>Design:</b> For every RelNode that carries {@link RexNode} expressions
+ * (Project, Filter, Join, Aggregate, Sort), the visitor collects all {@link
RexInputRef}s
+ * and uses Calcite's {@link RelMetadataQuery#getColumnOrigins(RelNode, int)}
to trace each
+ * referenced column back to its originating {@link TableScan} column index.
This correctly
+ * handles multi-hop projections, filters, joins, and aggregations.</p>
+ *
+ * <p>For a {@link TableScan} that has NO traced column references (e.g.
+ * {@code SELECT * FROM t} with no intervening Project), ALL columns of that
table
+ * are checked.</p>
+ *
+ * <p>System schemas (INFORMATION_SCHEMA, sys) are bypassed inside the
authorizer
+ * implementation. When authorization is disabled, the visitor is a no-op
(fail-open).</p>
+ */
+class ColumnAccessChecker extends RelShuttleImpl {
+
+ private static final Logger logger =
LoggerFactory.getLogger(ColumnAccessChecker.class);
+
+ private final UserSession session;
+ private final DrillConfig drillConfig;
+ private final RelMetadataQuery mq;
+
+ // Records each table's referenced column indices. Uses IdentityHashMap
because
+ // RelOptTable equals/hashCode may be expensive or not identity-based.
+ private final Map<RelOptTable, Set<Integer>> tableToReferencedCols = new
IdentityHashMap<>();
+
+ ColumnAccessChecker(UserSession session, DrillConfig drillConfig,
RelMetadataQuery mq) {
+ this.session = session;
+ this.drillConfig = drillConfig;
+ this.mq = mq;
+ }
+
+ /**
+ * Entry point: traverse the tree and enforce column-level access.
+ */
+ void check(RelNode root) {
+ // Also trace the root node's output columns (covers bare TableScan root or
+ // top-level Project output).
+ traceOutputColumns(root);
+
+ // Walk the tree to collect RexInputRef origins from all
expression-bearing nodes.
+ root.accept(this);
+ }
+
+ // ------------------------------------------------------------------
+ // RelShuttle overrides — collect RexInputRefs from expression-bearing nodes
+ // ------------------------------------------------------------------
+
+ @Override
+ public RelNode visit(LogicalProject project) {
+ collectRefs(project.getProjects(), project.getInput());
+ return super.visit(project);
+ }
+
+ @Override
+ public RelNode visit(LogicalFilter filter) {
+ if (filter.getCondition() != null) {
+ analyzeRex(filter.getCondition(), filter.getInput(), -1, null);
+ }
+ return super.visit(filter);
+ }
+
+ @Override
+ public RelNode visit(LogicalJoin join) {
+ if (join.getCondition() != null) {
+ int leftCount = join.getLeft().getRowType().getFieldCount();
+ analyzeRex(join.getCondition(), join.getRight(), leftCount,
join.getLeft());
+ }
+ return super.visit(join);
+ }
+
+ @Override
+ public RelNode visit(LogicalAggregate aggregate) {
+ for (int i : aggregate.getGroupSet()) {
+ traceColumnOrigin(aggregate.getInput(), i);
+ }
+ return super.visit(aggregate);
Review Comment:
**Aggregate call arguments are not traced — only the group set is.**
`getGroupSet()` covers `GROUP BY` keys, but `getAggCallList()` arg ordinals
(the `x` in `SUM(x)`) are skipped. Today `SqlToRelConverter` essentially always
inserts a `LogicalProject` beneath the `LogicalAggregate`, so
`visit(LogicalProject)` happens to catch those columns — but that makes the
correctness of a security check depend on an implicit guarantee about Calcite's
plan shape, which can change across Calcite upgrades (and this repo upgrades
Calcite fairly regularly).
Trace them explicitly:
```java
for (int i : aggregate.getGroupSet()) {
traceColumnOrigin(aggregate.getInput(), i);
}
for (AggregateCall call : aggregate.getAggCallList()) {
for (int arg : call.getArgList()) {
traceColumnOrigin(aggregate.getInput(), arg);
}
if (call.filterArg >= 0) {
traceColumnOrigin(aggregate.getInput(), call.filterArg);
}
for (RelFieldCollation fc : call.getCollation().getFieldCollations()) {
traceColumnOrigin(aggregate.getInput(), fc.getFieldIndex());
}
}
```
Related: neither `visit(LogicalAggregate)` nor `visit(LogicalSort)` calls
`analyzeRex`, so a `RexSubQuery` reachable only from those nodes (e.g. a
subquery in `FILTER (WHERE ...)`, or in `LIMIT`/`OFFSET`) is not descended into.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java:
##########
@@ -103,14 +106,99 @@ void disallowTemporaryTables() {
public Prepare.PreparingTable getTable(List<String> names) {
checkTemporaryTable(names);
Prepare.PreparingTable table = super.getTable(names);
- DrillTable drillTable;
- if (table != null && (drillTable = table.unwrap(DrillTable.class)) !=
null) {
- drillTable.setOptions(session.getOptions());
-
drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names,
drillTable)));
+ if (table != null) {
+ // Ranger SELECT authorization check for ALL table types, including
+ // JDBC storage plugin tables (JdbcTable) that are not DrillTable.
+ checkTableAccess(table, names);
+
+ DrillTable drillTable = table.unwrap(DrillTable.class);
+ if (drillTable != null) {
+ drillTable.setOptions(session.getOptions());
+
drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names,
drillTable)));
+ }
}
return table;
}
+ /**
+ * Checks SELECT permission on the resolved table via the configured {@link
AccessAuthorizer}
+ * (Ranger by default). No-op when authorization is disabled (fail-open).
System schemas
+ * (INFORMATION_SCHEMA, sys) are bypassed inside the authorizer
implementation.
+ *
+ * <p>Extracts datasource/schema/table from the resolved qualified name so
it works
+ * for both DrillTable (native storage plugins) and non-DrillTable (JDBC
storage plugin).
+ * The {@code names} argument is used as a fallback to determine the
datasource when
+ * the qualified name does not expose it.
+ */
+ private void checkTableAccess(Prepare.PreparingTable table, List<String>
names) {
+ AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
+ if (!authorizer.isEnabled()) {
+ return; // fail-open when disabled
+ }
+ // Use the resolved qualified name (includes default schema resolution)
rather than
+ // the raw input names, which may be incomplete when the user omits the
schema.
+ //
+ // Ranger four-level resource model: datasource / schema / table / column.
+ // The first segment of qualifiedName is the datasource (storage plugin
name);
+ // the LAST segment is the table; any segments in between form the schema
path.
+ // For example "mysql.shf.users" -> datasource=mysql, schema=shf,
table=users.
+ // The schema MUST NOT include the datasource prefix, otherwise Ranger
policy
+ // matching fails (policy has schema=shf but request sends
schema=mysql.shf).
+ //
+ // Some backends have no schema concept (e.g. a flat file store). To keep
the
+ // four-level model uniform, we synthesize a default schema per datasource
via
+ // getDefaultSchemaByDataSource(). The default branch returns the
datasource
+ // name itself so each storage plugin gets its own default schema namespace
+ // until an explicit mapping is added.
+ List<String> qualifiedName = table.getQualifiedName();
+ String tableName = qualifiedName.get(qualifiedName.size() - 1);
+ String dataSource;
+ String schemaPath;
+ if (qualifiedName.size() > 2) {
+ // datasource.schema.table OR datasource.subschema.table
+ dataSource = qualifiedName.get(0);
+ schemaPath = SchemaUtilities.getSchemaPath(qualifiedName.subList(1,
qualifiedName.size() - 1));
+ } else if (qualifiedName.size() == 2) {
+ // datasource.table — backend has no schema; synthesize a default so the
+ // four-level resource stays complete (Ranger policy matching requires a
+ // non-null schema key when schema is a mandatory resource).
+ dataSource = qualifiedName.get(0);
+ schemaPath = getDefaultSchemaByDataSource(dataSource);
+ } else {
+ // Single-element qualified name: fall back to the input names list to
find the datasource.
+ dataSource = !names.isEmpty() ? names.get(0) : tableName;
+ schemaPath = getDefaultSchemaByDataSource(dataSource);
+ }
+ String userName = session.getCredentials().getUserName();
+
+ if (!authorizer.checkTableAccess(userName, dataSource, schemaPath,
tableName, AccessTypes.SELECT)) {
Review Comment:
**Blocking: every check in this PR is hardcoded to `SELECT`, so write and
DDL paths are unauthorized.**
`AccessTypes` defines `CREATE`, `INSERT`, `DROP`, `DELETE`, `USE`, `SHOW`
and `DrillAccessType` mirrors them, but no call site ever passes anything other
than `AccessTypes.SELECT`. Two consequences:
1. **`DROP TABLE` is not checked at all.** `DropTableHandler` resolves the
table through `SchemaUtilities.resolveToDrillSchema` /
`SqlHandlerUtil.getTableFromSchema` and calls `AbstractSchema.dropTable`
directly — it never goes through this catalog reader, and it never reaches
`SqlConverter.toRel`, so neither enforcement point fires. Same for
`DropFunctionHandler`, `CreateAliasHandler`, etc.
2. **`INSERT` and `CTAS` are authorized as `SELECT`.** The target table does
resolve through `getTable()`, but the request Ranger sees says
`accessType=SELECT`. A user granted read-only on `mysql.shf.orders` can `INSERT
INTO` it and, via `DROP`, delete it.
For a Ranger integration that's a significant gap — an operator reading the
service-def (which advertises CREATE/DROP/INSERT/DELETE) will reasonably assume
those are enforced.
Two ways forward, either is fine but one is needed before merge:
- **Enforce them.** Thread the statement kind down to this method (the
`SqlKind` is available on the validated node in `SqlConverter`) and add an
explicit check in `DropTableHandler` / `CreateTableHandler` / `InsertHandler`
for `DROP` / `CREATE` / `INSERT`.
- **Scope the PR to SELECT.** Remove the unused constants from `AccessTypes`
and `DrillAccessType`, strip the corresponding access types from
`ranger-servicedef-drill.json`, and state prominently in
`RangerAuthorization.md` that only read paths are governed in this release.
The second is a perfectly reasonable v1 — it just has to be explicit,
because the current shape silently looks like full coverage.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/security/ranger/AccessTypes.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.security.ranger;
+
+/**
+ * Access type string constants used as the {@code accessType} argument to
+ * {@link AccessAuthorizer#checkTableAccess} and
+ * {@link AccessAuthorizer#checkColumnAccess}.
+ *
+ * <p>These constants mirror the values of {@code DrillAccessType} enum in the
+ * {@code drill-ranger-plugin} module. The main classpath cannot see that enum
+ * (it lives behind the {@code RangerPluginClassLoader} isolation boundary),
+ * so these string constants are the only way for Drill core to reference an
+ * access type.</p>
+ *
+ * <p>At runtime, {@code DrillAccessControl.checkTableAccess(..., String)}
+ * converts the string to {@code DrillAccessType} via
+ * {@code DrillAccessType.valueOf(operator.toUpperCase())}. If a constant here
+ * does not match an enum value (e.g. typo or drift after a Ranger upgrade),
+ * the conversion fails and access is denied (fail-closed). This means the
+ * string constants do not need to be kept in perfect lock-step with the enum
+ * — a mismatch is caught at the first call rather than silently allowing
+ * unauthorized access.</p>
+ *
+ * <p>Using a constant class (rather than a dedicated semantic method per
+ * access type like {@code checkTableSelectAccess},
+ * {@code checkTableInsertAccess}, ...) keeps the {@link AccessAuthorizer}
+ * interface compact as new access types are added. A new operation only
+ * requires adding a constant here; callers use the generic
+ * {@code checkTableAccess(..., AccessTypes.INSERT)} form.</p>
+ */
+public final class AccessTypes {
+
+ private AccessTypes() {
+ }
+
+ public static final String SELECT = "SELECT";
+ public static final String CREATE = "CREATE";
+ public static final String INSERT = "INSERT";
+ public static final String DROP = "DROP";
+ public static final String USE = "USE";
+ public static final String DELETE = "DELETE";
+ public static final String SHOW = "SHOW";
Review Comment:
All six of these (`CREATE`, `INSERT`, `DROP`, `USE`, `DELETE`, `SHOW`) are
dead — `SELECT` is the only constant referenced anywhere in the PR. See the
discussion on `DrillCalciteCatalogReader:174`: either wire them up, or remove
them here and from `DrillAccessType` / `ranger-servicedef-drill.json` so the
service-def doesn't advertise permissions Drill never evaluates.
Advertising unenforced access types in the service-def is the part I'd most
like to avoid — an admin who checks "DROP" on a policy will reasonably believe
drops are governed.
##########
drill-ranger/drill-ranger-plugin/src/main/java/org/apache/ranger/authorization/drill/authorizer/RangerDrillPlugin.java:
##########
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.authorization.drill.authorizer;
+
+import org.apache.ranger.plugin.service.RangerBasePlugin;
+
+public class RangerDrillPlugin extends RangerBasePlugin {
+ /**
+ * The Ranger service type. MUST match {@code "name"} in
ranger-servicedef-drill.json
+ * and {@code RangerDrillPlugin.SERVICE_TYPE}.
+ */
+ public final static String SERVICE_TYPE = "drill";
+ public final static String RANGER_PRESTO_APPID = "drill";
Review Comment:
`RANGER_PRESTO_APPID` — copy-paste from the Ranger Presto plugin. The value
is right, the name isn't. Rename to `RANGER_DRILL_APPID` (or just `APP_ID`).
Minor style: this file uses 4-space indentation on the constructor while the
rest of the module uses 2. Checkstyle may not cover `drill-ranger/` yet, but
consistency with the surrounding modules would be good.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/ColumnAccessChecker.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner.sql.conversion;
+
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttleImpl;
+import org.apache.calcite.rel.core.TableScan;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.logical.LogicalSort;
+import org.apache.calcite.rel.metadata.RelColumnOrigin;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexVisitorImpl;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.planner.sql.SchemaUtilities;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.security.ranger.AccessAuthorizer;
+import org.apache.drill.exec.security.ranger.AccessAuthorizerFactory;
+import org.apache.drill.exec.security.ranger.AccessTypes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Visitor that traverses a RelNode tree and enforces column-level SELECT
authorization
+ * via the configured {@link AccessAuthorizer} (Ranger by default).
+ *
+ * <p><b>Design:</b> For every RelNode that carries {@link RexNode} expressions
+ * (Project, Filter, Join, Aggregate, Sort), the visitor collects all {@link
RexInputRef}s
+ * and uses Calcite's {@link RelMetadataQuery#getColumnOrigins(RelNode, int)}
to trace each
+ * referenced column back to its originating {@link TableScan} column index.
This correctly
+ * handles multi-hop projections, filters, joins, and aggregations.</p>
+ *
+ * <p>For a {@link TableScan} that has NO traced column references (e.g.
+ * {@code SELECT * FROM t} with no intervening Project), ALL columns of that
table
+ * are checked.</p>
+ *
+ * <p>System schemas (INFORMATION_SCHEMA, sys) are bypassed inside the
authorizer
+ * implementation. When authorization is disabled, the visitor is a no-op
(fail-open).</p>
+ */
+class ColumnAccessChecker extends RelShuttleImpl {
+
+ private static final Logger logger =
LoggerFactory.getLogger(ColumnAccessChecker.class);
+
+ private final UserSession session;
+ private final DrillConfig drillConfig;
+ private final RelMetadataQuery mq;
+
+ // Records each table's referenced column indices. Uses IdentityHashMap
because
+ // RelOptTable equals/hashCode may be expensive or not identity-based.
+ private final Map<RelOptTable, Set<Integer>> tableToReferencedCols = new
IdentityHashMap<>();
+
+ ColumnAccessChecker(UserSession session, DrillConfig drillConfig,
RelMetadataQuery mq) {
+ this.session = session;
+ this.drillConfig = drillConfig;
+ this.mq = mq;
+ }
+
+ /**
+ * Entry point: traverse the tree and enforce column-level access.
+ */
+ void check(RelNode root) {
+ // Also trace the root node's output columns (covers bare TableScan root or
+ // top-level Project output).
+ traceOutputColumns(root);
+
+ // Walk the tree to collect RexInputRef origins from all
expression-bearing nodes.
+ root.accept(this);
+ }
+
+ // ------------------------------------------------------------------
+ // RelShuttle overrides — collect RexInputRefs from expression-bearing nodes
+ // ------------------------------------------------------------------
+
+ @Override
+ public RelNode visit(LogicalProject project) {
+ collectRefs(project.getProjects(), project.getInput());
+ return super.visit(project);
+ }
+
+ @Override
+ public RelNode visit(LogicalFilter filter) {
+ if (filter.getCondition() != null) {
+ analyzeRex(filter.getCondition(), filter.getInput(), -1, null);
+ }
+ return super.visit(filter);
+ }
+
+ @Override
+ public RelNode visit(LogicalJoin join) {
+ if (join.getCondition() != null) {
+ int leftCount = join.getLeft().getRowType().getFieldCount();
+ analyzeRex(join.getCondition(), join.getRight(), leftCount,
join.getLeft());
+ }
+ return super.visit(join);
+ }
+
+ @Override
+ public RelNode visit(LogicalAggregate aggregate) {
+ for (int i : aggregate.getGroupSet()) {
+ traceColumnOrigin(aggregate.getInput(), i);
+ }
+ return super.visit(aggregate);
+ }
+
+ @Override
+ public RelNode visit(LogicalSort sort) {
+ if (sort.getCollation() != null) {
+ sort.getCollation().getFieldCollations().forEach(fc ->
+ traceColumnOrigin(sort.getInput(), fc.getFieldIndex()));
+ }
+ return super.visit(sort);
+ }
+
+ @Override
+ public RelNode visit(TableScan scan) {
+ RelOptTable table = scan.getTable();
+
+ // Determine which columns to check: traced columns, or ALL if none were
traced
+ // (SELECT * FROM t case).
+ Set<Integer> referencedColIndices = tableToReferencedCols.get(table);
+ List<String> allColumnNames = scan.getRowType().getFieldNames();
+
+ Set<String> columnsToCheck;
+ if (referencedColIndices == null || referencedColIndices.isEmpty()) {
+ // SELECT * — check all columns
+ columnsToCheck = new HashSet<>(allColumnNames);
+ } else {
+ columnsToCheck = new HashSet<>();
+ for (int idx : referencedColIndices) {
+ if (idx >= 0 && idx < allColumnNames.size()) {
+ columnsToCheck.add(allColumnNames.get(idx));
+ }
+ }
+ }
+
+ if (columnsToCheck.isEmpty()) {
+ return scan;
+ }
+
+ enforceColumnAccess(table, columnsToCheck);
+ return scan;
+ }
+
+ /**
+ * Traces the output columns of a RelNode back to their table-scan origins.
+ */
+ private void traceOutputColumns(RelNode node) {
+ if (node == null) {
+ return;
+ }
+ int fieldCount = node.getRowType().getFieldCount();
+ for (int i = 0; i < fieldCount; i++) {
+ traceColumnOrigin(node, i);
+ }
+ }
+
+ /**
+ * Traces a single output column of {@code node} at index {@code
columnIndex} back to
+ * table-scan origins, recording them in {@link #tableToReferencedCols}.
+ */
+ private void traceColumnOrigin(RelNode node, int columnIndex) {
+ if (node == null || mq == null) {
+ return;
+ }
+ Set<RelColumnOrigin> origins;
+ try {
+ origins = mq.getColumnOrigins(node, columnIndex);
+ } catch (Exception e) {
+ logger.debug("getColumnOrigins failed for {} column {}", node,
columnIndex, e);
+ return;
+ }
+ if (origins == null) {
+ return;
+ }
+ for (RelColumnOrigin origin : origins) {
+ RelOptTable originTable = origin.getOriginTable();
+ if (originTable != null) {
+ // Record origins for ALL table types (DrillTable, JdbcTable, etc.).
+ // Previously this only recorded DrillTable origins, which caused
+ // JDBC storage plugin tables (JdbcTable) to be skipped entirely.
+ tableToReferencedCols
+ .computeIfAbsent(originTable, k -> new HashSet<>())
+ .add(origin.getOriginColumnOrdinal());
+ }
+ }
+ }
+
+ /**
+ * Collects RexInputRefs from a list of RexNodes and traces each to its
+ * table-scan origin via the input node's metadata. Also processes any
+ * {@link RexSubQuery} found in the expressions (scalar/IN/EXISTS subqueries)
+ * so that column references inside subqueries are authorized.
+ */
+ private void collectRefs(List<RexNode> rexNodes, RelNode inputNode) {
+ if (rexNodes == null || inputNode == null) {
+ return;
+ }
+ for (RexNode rex : rexNodes) {
+ analyzeRex(rex, inputNode, -1, null);
+ }
+ }
+
+ /**
+ * Analyzes a {@link RexNode} expression, collecting {@link RexInputRef}s and
+ * {@link RexSubQuery}s, tracing each input ref to its table-scan origin and
+ * recursively visiting each subquery's {@link RelNode} tree.
+ *
+ * @param rex the expression to analyze
+ * @param inputNode the input RelNode that RexInputRefs resolve against
+ * @param leftCount if {@code >= 0}, indicates a join condition: refs with
+ * index {@code < leftCount} resolve against {@code
leftInput},
+ * others resolve against {@code inputNode} (the right
input)
+ * with offset {@code leftCount}. If {@code < 0}, all refs
+ * resolve against {@code inputNode}.
+ * @param leftInput the left input of a join, or {@code null} when
+ * {@code leftCount < 0}.
+ */
+ private void analyzeRex(RexNode rex, RelNode inputNode, int leftCount,
RelNode leftInput) {
+ if (rex == null) {
+ return;
+ }
+ Set<Integer> refs = new HashSet<>();
+ List<RexSubQuery> subQueries = new ArrayList<>();
+ rex.accept(new RexRefCollector(refs, subQueries));
+ for (int refIndex : refs) {
+ if (leftCount >= 0 && refIndex < leftCount) {
+ traceColumnOrigin(leftInput, refIndex);
+ } else if (leftCount >= 0) {
+ traceColumnOrigin(inputNode, refIndex - leftCount);
+ } else {
+ traceColumnOrigin(inputNode, refIndex);
+ }
+ }
+ for (RexSubQuery sq : subQueries) {
+ // Trace the subquery's output columns to their table-scan origins.
+ // For scalar subqueries (e.g. SELECT sum(user_id) FROM t), this traces
+ // the aggregate output back to the underlying table column.
+ traceOutputColumns(sq.rel);
+ // Recursively visit the subquery's RelNode tree so that RexInputRefs
+ // inside the subquery (e.g. columns in WHERE/SELECT of the subquery)
+ // are also collected and traced.
+ sq.rel.accept(this);
+ }
+ }
+
+ /**
+ * Enforces column-level access for the given table and column set.
+ */
+ private void enforceColumnAccess(RelOptTable table, Set<String> columns) {
+ AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
+ if (!authorizer.isEnabled()) {
+ return; // fail-open when disabled
+ }
+
+ // Resolve datasource / schema / table from the qualified name, consistent
+ // with DrillCalciteCatalogReader.checkTableAccess(). This works for ALL
+ // table types (DrillTable, JdbcTable, etc.) — previously this method
+ // required a DrillTable and skipped JdbcTable, leaving JDBC storage
+ // plugin tables without column-level authorization.
+ List<String> qualifiedName = table.getQualifiedName();
+ String tableName = qualifiedName.get(qualifiedName.size() - 1);
+ String dataSource;
+ String schemaPath;
+ if (qualifiedName.size() > 2) {
+ dataSource = qualifiedName.get(0);
+ schemaPath = SchemaUtilities.getSchemaPath(qualifiedName.subList(1,
qualifiedName.size() - 1));
+ } else if (qualifiedName.size() == 2) {
+ dataSource = qualifiedName.get(0);
+ schemaPath =
DrillCalciteCatalogReader.getDefaultSchemaByDataSource(dataSource);
+ } else {
+ dataSource = tableName;
+ schemaPath =
DrillCalciteCatalogReader.getDefaultSchemaByDataSource(dataSource);
+ }
+ String userName = session.getCredentials().getUserName();
Review Comment:
**This qualified-name → (datasource, schema, table) split is duplicated in
`DrillCalciteCatalogReader.checkTableAccess`, and the two copies already
disagree.**
In the 1-segment fallback branch:
- here: `dataSource = tableName;`
- in `DrillCalciteCatalogReader` (line ~166): `dataSource = !names.isEmpty()
? names.get(0) : tableName;`
So for the same table, the table-level check and the column-level check can
address two different Ranger resources — one may match a policy while the other
doesn't. Given that Ranger denies by default, that shows up as an inconsistent
allow/deny depending on which check fires first.
Please extract a single helper (e.g. `static DrillRangerResource
resolve(List<String> qualifiedName)` in the `security.ranger` package) and call
it from both sites, so the mapping is defined exactly once and is unit-testable
on its own.
##########
drill-ranger/drill-ranger-service/src/main/java/org/apache/ranger/services/drill/RangerServiceDrill.java:
##########
@@ -0,0 +1,453 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.services.drill;
+
+import org.apache.ranger.plugin.model.RangerPolicy;
+import org.apache.ranger.plugin.model.RangerService;
+import org.apache.ranger.plugin.model.RangerServiceDef;
+import org.apache.ranger.plugin.service.RangerBaseService;
+import org.apache.ranger.plugin.service.ResourceLookupContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Ranger service plugin for Apache Drill.
+ *
+ * <p>Deployed into Ranger Admin (NOT into the Drillbit). Provides:
+ * <ul>
+ * <li>{@link #validateConfig()} - tests connectivity to the Drill cluster
+ * by calling the Drill REST API ({@code POST /query.json}).</li>
+ * <li>{@link #lookupResource(ResourceLookupContext)} - enumerates
+ * datasource / schema / table / column resources via
+ * {@code INFORMATION_SCHEMA} queries through the REST API, so the
+ * Ranger policy editor can auto-complete resource paths.</li>
+ * </ul>
+ *
+ * <p>Uses Drill's REST API (default port 8047) instead of JDBC, so this
+ * module can be compiled with JDK 8 and deployed into a JDK 8 Ranger Admin
+ * without pulling in Drill's JDK 11+ dependencies.
+ *
+ * <p>Connection configuration (must match the service-def JSON
+ * {@code serviceConfigOptions}):
+ * <ul>
+ * <li>{@code username} - Drill user name (required)</li>
+ * <li>{@code password} - Drill user password (optional)</li>
+ * <li>{@code drill.connection.url} - Drill REST endpoint, e.g.
+ * {@code http://host:8047} (required).</li>
+ * </ul>
+ */
+public class RangerServiceDrill extends RangerBaseService {
+
+ private static final Logger logger =
LoggerFactory.getLogger(RangerServiceDrill.class);
+
+ // Service config keys (must match ranger-servicedef-drill.json)
+ private static final String CONFIG_USERNAME = "username";
+ private static final String CONFIG_PASSWORD = "password";
+ private static final String CONFIG_DRILL_URL = "drill.connection.url";
+
+ // Resource names (must match ranger-servicedef-drill.json, lowercase per
Ranger naming rules)
+ private static final String RESOURCE_DATASOURCE = "datasource";
+ private static final String RESOURCE_SCHEMA = "schema";
+ private static final String RESOURCE_TABLE = "table";
+ private static final String RESOURCE_COLUMN = "column";
+
+ // HTTP connect / read timeout (milliseconds)
+ private static final int CONNECT_TIMEOUT_MS = 10_000;
+ private static final int READ_TIMEOUT_MS = 30_000;
+
+ // SQL templates for resource lookup. Each %s is filled via String.format
+ // with the corresponding escaped resource value. TABLES and COLUMNS are
+ // reserved keywords in Drill SQL and must be backtick-quoted.
+ private static final String SQL_VALIDATE_CONNECTION = "SELECT 1";
+
+ private static final String SQL_LOOKUP_DATASOURCE =
+ "SELECT DISTINCT SPLIT_PART(SCHEMA_NAME, '.', 1) AS DATASOURCE "
+ + "FROM INFORMATION_SCHEMA.SCHEMATA "
+ + "WHERE SCHEMA_NAME LIKE '%.%' "
+ + "ORDER BY 1";
+
+ // %s = datasource (e.g. "mysql")
+ private static final String SQL_LOOKUP_SCHEMA =
+ "SELECT SPLIT_PART(SCHEMA_NAME, '.', 2) AS SCHEMA "
+ + "FROM INFORMATION_SCHEMA.SCHEMATA "
+ + "WHERE SCHEMA_NAME LIKE '%s.%%' "
+ + "ORDER BY 1";
+
+ // %s = full table schema (e.g. "mysql.shf")
+ private static final String SQL_LOOKUP_TABLE =
+ "SELECT TABLE_NAME FROM INFORMATION_SCHEMA.`TABLES` "
+ + "WHERE TABLE_SCHEMA = '%s' "
+ + "ORDER BY 1";
+
+ // %1$s = full table schema, %2$s = table name
+ private static final String SQL_LOOKUP_COLUMN =
+ "SELECT COLUMN_NAME FROM INFORMATION_SCHEMA.`COLUMNS` "
+ + "WHERE TABLE_SCHEMA = '%s' "
+ + "AND TABLE_NAME = '%s' "
+ + "ORDER BY 1";
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ @Override
+ public void init(RangerServiceDef serviceDef, RangerService service) {
+ super.init(serviceDef, service);
+ logger.debug("RangerServiceDrill initialized for service={}",
+ service != null ? service.getName() : "null");
+ }
+
+ /**
+ * Validates the service configuration by testing connectivity to Drill.
+ *
+ * @return a map with {@code status} = {@code SUCCESS} or {@code FAILURE}
+ * and a human-readable {@code message}.
+ */
+ @Override
+ public Map<String, Object> validateConfig() throws Exception {
+ Map<String, Object> result = new HashMap<>();
+
+ String username = getConfig(CONFIG_USERNAME);
+ if (isBlank(username)) {
+ return failure(result, "Drill user name is required");
+ }
+ String baseUrl = getConfig(CONFIG_DRILL_URL);
+ if (isBlank(baseUrl)) {
+ return failure(result, "Drill connection URL is required");
+ }
+ String password = getConfig(CONFIG_PASSWORD);
+
+ String normalizedUrl;
+ try {
+ normalizedUrl = buildBaseUrl(baseUrl);
+ } catch (IllegalArgumentException e) {
+ return failure(result, "Invalid drill.connection.url: " +
e.getMessage());
+ }
+
+ logger.info("Validating Drill service connection to {}", normalizedUrl);
+ try {
+ String response = executeQuery(normalizedUrl, username, password,
SQL_VALIDATE_CONNECTION);
+ // A successful query returns JSON with a "rows" array
+ JsonNode root = MAPPER.readTree(response);
+ if (root != null && root.has("rows") && root.get("rows").isArray()) {
+ result.put("status", "SUCCESS");
+ result.put("message", "Connection test succeeded");
+ logger.info("Drill connection validation succeeded for {}",
normalizedUrl);
+ } else {
+ return failure(result, "Unexpected response from Drill: " + response);
+ }
+ } catch (Exception e) {
+ logger.error("Drill connection validation failed for url={}",
normalizedUrl, e);
+ return failure(result, "Connection test failed: " + e.getMessage());
+ }
+ return result;
+ }
+
+ /**
+ * Lists Drill resources for the Ranger policy editor autocomplete.
+ *
+ * <p>Supported resource levels (must match the service-def JSON):
+ * <ul>
+ * <li>{@code datasource} - distinct storage plugins from
+ * {@code INFORMATION_SCHEMA.SCHEMATA}</li>
+ * <li>{@code schema} - schema names filtered by the selected
datasource</li>
+ * <li>{@code table} - table names filtered by datasource + schema</li>
+ * <li>{@code column} - column names filtered by datasource + schema +
table</li>
+ * </ul>
+ *
+ * @param context carries the requested resource name and the
already-selected
+ * parent resources in {@link
ResourceLookupContext#getResources()}
+ * @return a list of matching resource names (never {@code null})
+ */
+ @Override
+ public List<String> lookupResource(ResourceLookupContext context) throws
Exception {
+ if (context == null) {
+ return Collections.emptyList();
+ }
+ String resourceName = context.getResourceName();
+ // getResources() returns Map<String, List<String>> in Ranger 2.8.0:
+ // each parent resource name maps to a list of selected values.
+ Map<String, List<String>> hints = context.getResources() != null
+ ? context.getResources() : Collections.emptyMap();
+
+ if (isBlank(resourceName)) {
+ return Collections.emptyList();
+ }
+
+ String username = getConfig(CONFIG_USERNAME);
+ String password = getConfig(CONFIG_PASSWORD);
+ String baseUrl = buildBaseUrl(getConfig(CONFIG_DRILL_URL));
+
+ logger.debug("lookupResource: resource={}, hints={}", resourceName, hints);
+
+ try {
+ switch (resourceName) {
+ case RESOURCE_DATASOURCE:
+ // Drill's INFORMATION_SCHEMA.SCHEMATA has no STORAGE_PLUGIN column.
+ // The datasource (storage plugin name) is the first segment of
+ // SCHEMA_NAME (e.g. "mysql.shf" -> "mysql"). Use SUBSTR_INDEX to
+ // extract it, then DISTINCT to deduplicate.
+ return extractFirstColumnValues(executeQuery(baseUrl, username,
password,
+ SQL_LOOKUP_DATASOURCE));
+ case RESOURCE_SCHEMA: {
+ String datasource = firstHint(hints, RESOURCE_DATASOURCE);
+ if (isBlank(datasource)) {
+ return Collections.emptyList();
+ }
+ // For a given datasource, list schema names by stripping the
+ // "datasource." prefix from SCHEMA_NAME (e.g. "mysql.shf" -> "shf").
+ // Schemas without a dot (e.g. plain "mysql") are filtered out.
+ String sql = String.format(SQL_LOOKUP_SCHEMA, escapeSql(datasource));
+ return extractFirstColumnValues(executeQuery(baseUrl, username,
password, sql));
+ }
+ case RESOURCE_TABLE: {
+ String datasource = firstHint(hints, RESOURCE_DATASOURCE);
+ String schema = firstHint(hints, RESOURCE_SCHEMA);
+ if (isBlank(datasource) || isBlank(schema)) {
+ return Collections.emptyList();
+ }
+ // TABLE_SCHEMA in INFORMATION_SCHEMA.`TABLES` uses the full
qualified
+ // form "datasource.schema" (e.g. "mysql.shf"), so concatenate the
+ // selected datasource and schema before filtering.
+ // NOTE: TABLES is a reserved keyword in Drill SQL and must be
+ // backtick-quoted; without quotes the parser rejects the query.
+ String tableSchema = escapeSql(datasource) + "." + escapeSql(schema);
+ String sql = String.format(SQL_LOOKUP_TABLE, tableSchema);
+ return extractFirstColumnValues(executeQuery(baseUrl, username,
password, sql));
+ }
+ case RESOURCE_COLUMN: {
+ String datasource = firstHint(hints, RESOURCE_DATASOURCE);
+ String schema = firstHint(hints, RESOURCE_SCHEMA);
+ String table = firstHint(hints, RESOURCE_TABLE);
+ if (isBlank(datasource) || isBlank(schema) || isBlank(table)) {
+ return Collections.emptyList();
+ }
+ // COLUMNS is also a reserved keyword in Drill SQL — backtick-quote
it.
+ String tableSchema = escapeSql(datasource) + "." + escapeSql(schema);
+ String sql = String.format(SQL_LOOKUP_COLUMN, tableSchema,
escapeSql(table));
+ return extractFirstColumnValues(executeQuery(baseUrl, username,
password, sql));
+ }
+ default:
+ logger.warn("Unknown resource name: {}", resourceName);
+ return Collections.emptyList();
+ }
+ } catch (Exception e) {
+ logger.error("lookupResource failed for resource={}, hints={}",
resourceName, hints, e);
+ throw e;
+ }
+ }
+
+ @Override
+ public List<RangerPolicy> getDefaultRangerPolicies() throws Exception {
+ return super.getDefaultRangerPolicies();
+ }
+
+ // ========================================================================
+ // REST API helpers
+ // ========================================================================
+
+ /**
+ * Normalizes the user-supplied {@code drill.connection.url} to a base URL
+ * like {@code http://host:8047}. Trims trailing slashes.
+ */
+ static String buildBaseUrl(String configuredUrl) {
+ if (isBlank(configuredUrl)) {
+ throw new IllegalArgumentException("drill.connection.url is empty");
+ }
+ String url = configuredUrl.trim();
+ // Strip trailing slashes
+ while (url.endsWith("/")) {
+ url = url.substring(0, url.length() - 1);
+ }
+ return url;
Review Comment:
The javadoc (and `RangerAuthorization.md` §3.2, "Bare `host:port` is
normalized to `http://host:port`";) promises normalization that this method
doesn't do — it only trims trailing slashes. A configured value of
`drillbit:8047` produces the endpoint `drillbit:8047/query.json`, which `new
URL(...)` rejects with `MalformedURLException: no protocol`, surfacing as a
generic "Connection test failed" in the Ranger UI.
Either implement the documented normalization or correct the doc and
javadoc. Worth also validating the scheme while you're here — `executeQuery`
sends the Drill password in an HTTP Basic header, so an admin who types
`http://` on a routable network is shipping credentials in the clear. Rejecting
anything that isn't `http`/`https`, and logging a warning for `http` to a
non-loopback host, would be a reasonable guard:
```java
URI uri = URI.create(url);
if (!"http".equals(uri.getScheme()) && !"https".equals(uri.getScheme())) {
throw new IllegalArgumentException("drill.connection.url must use http or
https: " + url);
}
```
##########
drill-ranger/drill-ranger-plugin/src/main/java/org/apache/ranger/authorization/drill/authorizer/DrillAuthorizer.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.authorization.drill.authorizer;
+
+import org.apache.ranger.authorization.drill.resource.DrillAccessResource;
+import org.apache.ranger.authorization.drill.resource.DrillAccessType;
+import org.apache.ranger.authorization.drill.resource.DrillRangerAccessRequest;
+import org.apache.ranger.authorization.drill.resource.DrillResource;
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+
+public class DrillAuthorizer {
+ private static final Logger logger =
LoggerFactory.getLogger(DrillAuthorizer.class);
+ private RangerBaseAuthorizer authorizer;
+
+ /**
+ * Resource validation level enum.
+ * Controls the depth of validation in the {@link
#validateResource(DrillResource, ValidationLevel)}
+ * method. Nested here because it is only used inside this class.
+ */
+ private enum ValidationLevel {
+ /** Validate up to the datasource level (user, dataSource). */
+ DATASOURCE,
+ /** Validate up to the schema level (user, dataSource, schema). */
+ SCHEMA,
+ /** Validate up to the table level (user, dataSource, schema, table). */
+ TABLE,
+ /** Full validation including columns (user, dataSource, schema, table,
columns). */
+ COLUMN
+ }
+
+ public DrillAuthorizer(String serviceName) {
+ authorizer = RangerBaseAuthorizer.getInstance();
+ authorizer.init(serviceName);
+ }
+
+ private boolean checkPermission(DrillRangerAccessRequest request) {
+ return authorizer.isAccessAllowed(request.toRangerRequest());
+ }
+
+ /**
+ * Build a DrillRangerAccessRequest from the given resource and access type,
then check
+ * permission. This abstracts the common logic shared by table-level and
column-level
+ * access checks.
+ *
+ * @param resource the DrillResource providing user, groups, etc.
+ * @param drillAccessResource the DrillAccessResource describing the
accessed entity
+ * @param operator the access type to check
+ * @return the permission check result
+ */
+ private boolean checkAccess(DrillResource resource, DrillAccessResource
drillAccessResource,
+ DrillAccessType operator, RangerAccessRequest.ResourceMatchingScope
scope) {
+ Set<String> groups = new HashSet<>();
+ if (resource.getGroups() != null) {
+ groups.addAll(resource.getGroups());
+ }
+
+ DrillRangerAccessRequest request = DrillRangerAccessRequest.builder()
+ .user(resource.getUser())
+ .groups(groups)
+ .resource(drillAccessResource)
+ .accessType(operator)
+ .resourceMatchingScope(scope)
+ .build();
+
+ return checkPermission(request);
+ }
+
+ public boolean checkTableAccess(DrillResource resource, DrillAccessType
operator) {
+ if (!validateResource(resource, ValidationLevel.TABLE)) {
+ logger.warn("MetaStoreResource validation failed for table access
check");
Review Comment:
`MetaStoreResource` doesn't exist in this codebase — the class is
`DrillResource`. Leftover from whichever plugin this was adapted from. Same
string appears again at line 126 (there labelled "for table access check"
inside `checkColumnAccess`, so that one is doubly wrong).
Also worth including the user and resource in these two warnings — as
written, a validation failure gives an operator nothing to act on.
##########
drill-ranger/drill-ranger-service/src/main/java/org/apache/ranger/services/drill/RangerServiceDrill.java:
##########
@@ -0,0 +1,453 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.services.drill;
+
+import org.apache.ranger.plugin.model.RangerPolicy;
+import org.apache.ranger.plugin.model.RangerService;
+import org.apache.ranger.plugin.model.RangerServiceDef;
+import org.apache.ranger.plugin.service.RangerBaseService;
+import org.apache.ranger.plugin.service.ResourceLookupContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.io.BufferedReader;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Base64;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Ranger service plugin for Apache Drill.
+ *
+ * <p>Deployed into Ranger Admin (NOT into the Drillbit). Provides:
+ * <ul>
+ * <li>{@link #validateConfig()} - tests connectivity to the Drill cluster
+ * by calling the Drill REST API ({@code POST /query.json}).</li>
+ * <li>{@link #lookupResource(ResourceLookupContext)} - enumerates
+ * datasource / schema / table / column resources via
+ * {@code INFORMATION_SCHEMA} queries through the REST API, so the
+ * Ranger policy editor can auto-complete resource paths.</li>
+ * </ul>
+ *
+ * <p>Uses Drill's REST API (default port 8047) instead of JDBC, so this
+ * module can be compiled with JDK 8 and deployed into a JDK 8 Ranger Admin
+ * without pulling in Drill's JDK 11+ dependencies.
+ *
+ * <p>Connection configuration (must match the service-def JSON
+ * {@code serviceConfigOptions}):
+ * <ul>
+ * <li>{@code username} - Drill user name (required)</li>
+ * <li>{@code password} - Drill user password (optional)</li>
+ * <li>{@code drill.connection.url} - Drill REST endpoint, e.g.
+ * {@code http://host:8047} (required).</li>
+ * </ul>
+ */
+public class RangerServiceDrill extends RangerBaseService {
+
+ private static final Logger logger =
LoggerFactory.getLogger(RangerServiceDrill.class);
+
+ // Service config keys (must match ranger-servicedef-drill.json)
+ private static final String CONFIG_USERNAME = "username";
+ private static final String CONFIG_PASSWORD = "password";
+ private static final String CONFIG_DRILL_URL = "drill.connection.url";
+
+ // Resource names (must match ranger-servicedef-drill.json, lowercase per
Ranger naming rules)
+ private static final String RESOURCE_DATASOURCE = "datasource";
+ private static final String RESOURCE_SCHEMA = "schema";
+ private static final String RESOURCE_TABLE = "table";
+ private static final String RESOURCE_COLUMN = "column";
+
+ // HTTP connect / read timeout (milliseconds)
+ private static final int CONNECT_TIMEOUT_MS = 10_000;
+ private static final int READ_TIMEOUT_MS = 30_000;
+
+ // SQL templates for resource lookup. Each %s is filled via String.format
+ // with the corresponding escaped resource value. TABLES and COLUMNS are
+ // reserved keywords in Drill SQL and must be backtick-quoted.
+ private static final String SQL_VALIDATE_CONNECTION = "SELECT 1";
+
+ private static final String SQL_LOOKUP_DATASOURCE =
+ "SELECT DISTINCT SPLIT_PART(SCHEMA_NAME, '.', 1) AS DATASOURCE "
+ + "FROM INFORMATION_SCHEMA.SCHEMATA "
+ + "WHERE SCHEMA_NAME LIKE '%.%' "
+ + "ORDER BY 1";
+
+ // %s = datasource (e.g. "mysql")
+ private static final String SQL_LOOKUP_SCHEMA =
+ "SELECT SPLIT_PART(SCHEMA_NAME, '.', 2) AS SCHEMA "
+ + "FROM INFORMATION_SCHEMA.SCHEMATA "
+ + "WHERE SCHEMA_NAME LIKE '%s.%%' "
+ + "ORDER BY 1";
+
+ // %s = full table schema (e.g. "mysql.shf")
+ private static final String SQL_LOOKUP_TABLE =
+ "SELECT TABLE_NAME FROM INFORMATION_SCHEMA.`TABLES` "
+ + "WHERE TABLE_SCHEMA = '%s' "
+ + "ORDER BY 1";
+
+ // %1$s = full table schema, %2$s = table name
+ private static final String SQL_LOOKUP_COLUMN =
+ "SELECT COLUMN_NAME FROM INFORMATION_SCHEMA.`COLUMNS` "
+ + "WHERE TABLE_SCHEMA = '%s' "
+ + "AND TABLE_NAME = '%s' "
+ + "ORDER BY 1";
+
+ private static final ObjectMapper MAPPER = new ObjectMapper();
+
+ @Override
+ public void init(RangerServiceDef serviceDef, RangerService service) {
+ super.init(serviceDef, service);
+ logger.debug("RangerServiceDrill initialized for service={}",
+ service != null ? service.getName() : "null");
+ }
+
+ /**
+ * Validates the service configuration by testing connectivity to Drill.
+ *
+ * @return a map with {@code status} = {@code SUCCESS} or {@code FAILURE}
+ * and a human-readable {@code message}.
+ */
+ @Override
+ public Map<String, Object> validateConfig() throws Exception {
+ Map<String, Object> result = new HashMap<>();
+
+ String username = getConfig(CONFIG_USERNAME);
+ if (isBlank(username)) {
+ return failure(result, "Drill user name is required");
+ }
+ String baseUrl = getConfig(CONFIG_DRILL_URL);
+ if (isBlank(baseUrl)) {
+ return failure(result, "Drill connection URL is required");
+ }
+ String password = getConfig(CONFIG_PASSWORD);
+
+ String normalizedUrl;
+ try {
+ normalizedUrl = buildBaseUrl(baseUrl);
+ } catch (IllegalArgumentException e) {
+ return failure(result, "Invalid drill.connection.url: " +
e.getMessage());
+ }
+
+ logger.info("Validating Drill service connection to {}", normalizedUrl);
+ try {
+ String response = executeQuery(normalizedUrl, username, password,
SQL_VALIDATE_CONNECTION);
+ // A successful query returns JSON with a "rows" array
+ JsonNode root = MAPPER.readTree(response);
+ if (root != null && root.has("rows") && root.get("rows").isArray()) {
+ result.put("status", "SUCCESS");
+ result.put("message", "Connection test succeeded");
+ logger.info("Drill connection validation succeeded for {}",
normalizedUrl);
+ } else {
+ return failure(result, "Unexpected response from Drill: " + response);
+ }
+ } catch (Exception e) {
+ logger.error("Drill connection validation failed for url={}",
normalizedUrl, e);
+ return failure(result, "Connection test failed: " + e.getMessage());
+ }
+ return result;
+ }
+
+ /**
+ * Lists Drill resources for the Ranger policy editor autocomplete.
+ *
+ * <p>Supported resource levels (must match the service-def JSON):
+ * <ul>
+ * <li>{@code datasource} - distinct storage plugins from
+ * {@code INFORMATION_SCHEMA.SCHEMATA}</li>
+ * <li>{@code schema} - schema names filtered by the selected
datasource</li>
+ * <li>{@code table} - table names filtered by datasource + schema</li>
+ * <li>{@code column} - column names filtered by datasource + schema +
table</li>
+ * </ul>
+ *
+ * @param context carries the requested resource name and the
already-selected
+ * parent resources in {@link
ResourceLookupContext#getResources()}
+ * @return a list of matching resource names (never {@code null})
+ */
+ @Override
+ public List<String> lookupResource(ResourceLookupContext context) throws
Exception {
+ if (context == null) {
+ return Collections.emptyList();
+ }
+ String resourceName = context.getResourceName();
+ // getResources() returns Map<String, List<String>> in Ranger 2.8.0:
+ // each parent resource name maps to a list of selected values.
+ Map<String, List<String>> hints = context.getResources() != null
+ ? context.getResources() : Collections.emptyMap();
+
+ if (isBlank(resourceName)) {
+ return Collections.emptyList();
+ }
+
+ String username = getConfig(CONFIG_USERNAME);
+ String password = getConfig(CONFIG_PASSWORD);
+ String baseUrl = buildBaseUrl(getConfig(CONFIG_DRILL_URL));
+
+ logger.debug("lookupResource: resource={}, hints={}", resourceName, hints);
+
+ try {
+ switch (resourceName) {
+ case RESOURCE_DATASOURCE:
+ // Drill's INFORMATION_SCHEMA.SCHEMATA has no STORAGE_PLUGIN column.
+ // The datasource (storage plugin name) is the first segment of
+ // SCHEMA_NAME (e.g. "mysql.shf" -> "mysql"). Use SUBSTR_INDEX to
+ // extract it, then DISTINCT to deduplicate.
+ return extractFirstColumnValues(executeQuery(baseUrl, username,
password,
+ SQL_LOOKUP_DATASOURCE));
+ case RESOURCE_SCHEMA: {
+ String datasource = firstHint(hints, RESOURCE_DATASOURCE);
+ if (isBlank(datasource)) {
+ return Collections.emptyList();
+ }
+ // For a given datasource, list schema names by stripping the
+ // "datasource." prefix from SCHEMA_NAME (e.g. "mysql.shf" -> "shf").
+ // Schemas without a dot (e.g. plain "mysql") are filtered out.
+ String sql = String.format(SQL_LOOKUP_SCHEMA, escapeSql(datasource));
Review Comment:
`escapeSql` only doubles single quotes, but this value is interpolated into
a `LIKE` pattern (`SQL_LOOKUP_SCHEMA` = `... WHERE SCHEMA_NAME LIKE '%s.%%'`),
so `%` and `_` in the datasource name survive as wildcards. A datasource named
`a_b` matches `axb`, and one containing `%` matches everything.
Low severity — this path runs in Ranger Admin, is read-only against
`INFORMATION_SCHEMA`, and the input comes from a previous lookup rather than
free text. But it's a correctness bug in the policy editor's autocomplete
(wrong schemas offered for the wrong datasource), and it's cheap to fix:
```java
private static String escapeLike(String value) {
return escapeSql(value).replace("\\", "\\\\").replace("%",
"\\%").replace("_", "\\_");
}
```
with a matching `ESCAPE '\'` clause on the `LIKE`. `SQL_LOOKUP_TABLE` /
`SQL_LOOKUP_COLUMN` use `=` rather than `LIKE`, so plain `escapeSql` is fine
there.
##########
drill-ranger/pom.xml:
##########
@@ -0,0 +1,76 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.drill</groupId>
+ <artifactId>drill-root</artifactId>
+ <version>1.23.0-SNAPSHOT</version>
+ </parent>
+
+ <!--
+ Parent pom for the two Ranger / Drill integration modules.
+
+ Split rationale
+ - drill-ranger-plugin : authorization side
+ - drill-ranger-service : service side, loaded by the Ranger Admin JVM.
+ Produces ranger-drill-service-x.x.x.jar, deployed into Ranger Admin's
+ WEB-INF/classes/lib/.
+ Connects to Drill via REST API (HttpURLConnection) for validateConfig
+ / lookupResource; does NOT depend on drill-jdbc (avoids JDK 11 bytecode
+ conflict on Ranger Admin's JDK 8 runtime).
+
+ The two sides have non-overlapping dependencies and run in different JVMs,
+ so they must NOT be packaged together.
+ -->
+ <artifactId>drill-ranger-parent</artifactId>
+ <packaging>pom</packaging>
+ <name>Drill : Ranger Integration Parent</name>
+ <description>
+ Parent module aggregating the Drill Ranger authorization plugin
+ (drill-ranger-plugin) and the Drill Ranger service plugin
+ (drill-ranger-service).
+ </description>
+
+ <properties>
+ <ranger.version>2.8.0</ranger.version>
+ <!--
+ Jersey 2.x version for drill-ranger-plugin. Ranger 2.8.0 ships
+ RangerAdminJersey2RESTClient (in ranger-knox-plugin) which uses the
+ Jersey 2.x / javax.ws.rs.* API. We depend on ranger-knox-plugin to
+ obtain that class (no source copy) and ship Jersey 2.35 client jars
+ in the plugin's isolated classloader directory
(ranger-drill-plugin-impl/).
+
+ This is distinct from drill-root's <jersey.version> (3.1.9, Jersey 3.x
+ for Drill's own REST server, jakarta.ws.rs.* namespace). The two Jersey
+ versions coexist because drill-ranger-plugin is loaded by a dedicated
+ RangerPluginClassLoader that physically isolates org.glassfish.jersey.*
+ 2.35 from Drill's 3.1.9.
+ -->
+ <jersey.ranger.version>2.35</jersey.ranger.version>
+ </properties>
+
+ <modules>
+ <module>drill-ranger-plugin</module>
+ <module>drill-ranger-service</module>
Review Comment:
**Project-level question that should go to dev@ before this merges: all the
Java in these two modules lives under `org.apache.ranger.*`.**
That means Drill would be releasing artifacts that occupy another ASF
project's package namespace. Every comparable integration —
`ranger-hive-plugin`, `ranger-hbase-plugin`, `ranger-kafka-plugin`,
`ranger-presto-plugin` — lives in the Ranger repo and is released by the Ranger
PMC, precisely so the namespace and the release stay with one project. Ranger
also has a documented process for contributing new service plugins.
This isn't a code objection; the code here is reasonable. But it affects who
owns, versions, and CVE-patches these classes, so it needs a decision on
dev@drill and probably a heads-up to dev@ranger rather than being settled in a
PR review. Worth raising before more review effort goes in, since the answer
could move `drill-ranger-plugin` and `drill-ranger-service` out of this repo
entirely and leave only the `AccessAuthorizer` SPI + `ColumnAccessChecker` here
— which, notably, is the part of this PR that's genuinely Drill's.
If the modules do stay, `org.apache.drill.exec.security.ranger` would be the
correct package for them.
Naming nit while I'm here: the directory is `drill-ranger-service` but its
`artifactId` is `ranger-drill-service` (line 30 of that pom), while its sibling
is `drill-ranger-plugin`. Pick one order and use it for both.
##########
drill-ranger/drill-ranger-plugin/src/main/java/org/apache/ranger/authorization/drill/authorizer/RangerBaseAuthorizer.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.authorization.drill.authorizer;
+
+import org.apache.ranger.plugin.audit.RangerDefaultAuditHandler;
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
+import org.apache.ranger.plugin.policyengine.RangerAccessResult;
+import org.apache.ranger.plugin.service.RangerBasePlugin;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Singleton wrapper around {@link RangerBasePlugin} for the Drill service
type.
+ *
+ * <p>Initialized once at Drillbit startup with the service name configured in
+ * {@code ranger-drill-security.xml}. After initialization,
+ * {@link #isAccessAllowed(RangerAccessRequest)}
+ * performs local in-memory policy evaluation (policies are pulled
periodically by the
+ * {@code PolicyRefresher} background thread, identical to the Hive
plugin).</p>
+ */
+public class RangerBaseAuthorizer {
+ private static final Logger logger =
LoggerFactory.getLogger(RangerBaseAuthorizer.class);
+
+ private volatile RangerBasePlugin plugin;
+
+ private RangerBaseAuthorizer() {
+
+ }
+
+ private static class LazyHolder {
+ private static final RangerBaseAuthorizer INSTANCE = new
RangerBaseAuthorizer();
+ }
+
+ public static RangerBaseAuthorizer getInstance() {
+ return LazyHolder.INSTANCE;
+ }
+
+ /**
+ * Initializes the Ranger plugin. The {@code serviceType} MUST be "drill" to
match the
+ * service-def registered in Ranger Admin.
+ *
+ * @param serviceName the service instance name (matches {@code
ranger.plugin.drill.service.name})
+ */
+ public synchronized void init(String serviceName) {
Review Comment:
`plugin` is `volatile`, but the only write is inside a fully `synchronized`
method that already re-checks `plugin != null` under the lock — so the
`volatile` buys nothing here beyond making the unsynchronized read in
`isAccessAllowed` safe (which is the one place it does matter, so keep it, just
noting the intent isn't a double-checked-locking idiom).
The thing I'd actually change: `cleanUp()` is never called.
`RangerBasePlugin.init()` starts a `PolicyRefresher` thread and the audit
provider's background queue; nothing in the Drillbit shutdown path tears them
down, so an embedded/test Drillbit that stops and restarts leaks both. Wiring
`cleanUp()` into `Drillbit.close()` would fix that — and is another argument
for scoping the authorizer to `DrillbitContext` rather than a static (see the
comment on `AccessAuthorizerFactory`).
##########
drill-ranger/drill-ranger-plugin/src/main/java/org/apache/ranger/authorization/drill/authorizer/DrillAuthorizer.java:
##########
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.authorization.drill.authorizer;
+
+import org.apache.ranger.authorization.drill.resource.DrillAccessResource;
+import org.apache.ranger.authorization.drill.resource.DrillAccessType;
+import org.apache.ranger.authorization.drill.resource.DrillRangerAccessRequest;
+import org.apache.ranger.authorization.drill.resource.DrillResource;
+import org.apache.ranger.plugin.policyengine.RangerAccessRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
+
+public class DrillAuthorizer {
+ private static final Logger logger =
LoggerFactory.getLogger(DrillAuthorizer.class);
+ private RangerBaseAuthorizer authorizer;
+
+ /**
+ * Resource validation level enum.
+ * Controls the depth of validation in the {@link
#validateResource(DrillResource, ValidationLevel)}
+ * method. Nested here because it is only used inside this class.
+ */
+ private enum ValidationLevel {
+ /** Validate up to the datasource level (user, dataSource). */
+ DATASOURCE,
+ /** Validate up to the schema level (user, dataSource, schema). */
+ SCHEMA,
+ /** Validate up to the table level (user, dataSource, schema, table). */
+ TABLE,
+ /** Full validation including columns (user, dataSource, schema, table,
columns). */
+ COLUMN
+ }
+
+ public DrillAuthorizer(String serviceName) {
+ authorizer = RangerBaseAuthorizer.getInstance();
+ authorizer.init(serviceName);
+ }
+
+ private boolean checkPermission(DrillRangerAccessRequest request) {
+ return authorizer.isAccessAllowed(request.toRangerRequest());
+ }
+
+ /**
+ * Build a DrillRangerAccessRequest from the given resource and access type,
then check
+ * permission. This abstracts the common logic shared by table-level and
column-level
+ * access checks.
+ *
+ * @param resource the DrillResource providing user, groups, etc.
+ * @param drillAccessResource the DrillAccessResource describing the
accessed entity
+ * @param operator the access type to check
+ * @return the permission check result
+ */
+ private boolean checkAccess(DrillResource resource, DrillAccessResource
drillAccessResource,
+ DrillAccessType operator, RangerAccessRequest.ResourceMatchingScope
scope) {
+ Set<String> groups = new HashSet<>();
+ if (resource.getGroups() != null) {
+ groups.addAll(resource.getGroups());
+ }
+
+ DrillRangerAccessRequest request = DrillRangerAccessRequest.builder()
+ .user(resource.getUser())
+ .groups(groups)
+ .resource(drillAccessResource)
+ .accessType(operator)
+ .resourceMatchingScope(scope)
+ .build();
+
+ return checkPermission(request);
+ }
+
+ public boolean checkTableAccess(DrillResource resource, DrillAccessType
operator) {
+ if (!validateResource(resource, ValidationLevel.TABLE)) {
+ logger.warn("MetaStoreResource validation failed for table access
check");
+ return false;
+ }
+ Optional<String> schema = Optional.ofNullable(resource.getSchema());
+ Optional<String> table = Optional.ofNullable(resource.getTable());
+ DrillAccessResource drillAccessResource = new
DrillAccessResource(resource.getDataSource(),
+ schema, table);
+
+ // Table-level check uses SELF_OR_DESCENDANTS so a request without a column
+ // can still match column-level policies (column is a descendant of table).
+ // This allows a single policy with column=amount to authorize the
table-level
+ // SELECT check that happens during SQL parsing (before columns are
resolved).
+ boolean result = checkAccess(resource, drillAccessResource, operator,
+ RangerAccessRequest.ResourceMatchingScope.SELF_OR_DESCENDANTS);
+ if (logger.isDebugEnabled()) {
+ logger.debug("checkTableAccess result for user={}, datasource={},
schema={}, table={}, " +
+ "operator={}: result={}",
+ resource.getUser(), resource.getDataSource(), resource.getSchema(),
+ resource.getTable(), operator.name(), result);
+ }
+ return result;
+ }
+
+ /**
+ * Validate that required fields of MetaStoreResource are non-empty (full
validation, defaults
+ * to column level)
+ *
+ * @param resource the resource object to validate
+ * @return true if validation passes, false otherwise
+ */
+ public boolean validateResource(DrillResource resource) {
+ return validateResource(resource, ValidationLevel.COLUMN);
+ }
+
+ public boolean checkColumnAccess(DrillResource resource, DrillAccessType
operator) {
+ if (!validateResource(resource, ValidationLevel.COLUMN)) {
+ logger.warn("MetaStoreResource validation failed for table access
check");
+ return false;
+ }
+ Optional<String> schema = Optional.ofNullable(resource.getSchema());
+ Optional<String> table = Optional.ofNullable(resource.getTable());
+
+ for (String column : resource.getColumns()) {
+ Optional<String> columnOpt = Optional.of(column);
+ DrillAccessResource drillAccessResource = new
DrillAccessResource(resource.getDataSource(),
+ schema, table, columnOpt);
+
+ // Column-level check uses SELF for exact column matching: only policies
+ // whose column resource matches the requested column will be applied.
+ boolean allowed = checkAccess(resource, drillAccessResource, operator,
+ RangerAccessRequest.ResourceMatchingScope.SELF);
+ if (logger.isDebugEnabled()) {
+ logger.debug("checkColumnAccess result for user={}, datasource={},
schema={}, table={}, " +
+ "column={}, operator={}: result={}",
+ resource.getUser(), resource.getDataSource(), resource.getSchema(),
+ resource.getTable(), column, operator.name(), allowed);
+ }
+ if (!allowed) {
+ // Fail fast on first denied column — no need to check the rest.
+ logger.warn("Column access denied for user={}, column={}.{}.{}",
+ resource.getUser(), resource.getDataSource(), resource.getSchema(),
Review Comment:
One `isAccessAllowed` call per column means one policy evaluation **and one
audit record** per column per table scan. `SELECT *` on a 200-column table
produces 200 evaluations and 200 audit rows for a single query, which will be
noisy in Solr/HDFS audit and slow in the planner.
`RangerBasePlugin` has a batch form —
`isAccessAllowed(Collection<RangerAccessRequest>)` — which evaluates the set
and emits a single grouped audit event. Building the N requests up front and
issuing one call would collapse this to one round trip:
```java
List<RangerAccessRequest> requests = resource.getColumns().stream()
.map(col -> buildRequest(resource, col, operator))
.collect(Collectors.toList());
Collection<RangerAccessResult> results =
authorizer.isAccessAllowed(requests);
return results.stream().allMatch(r -> r != null && r.getIsAllowed());
```
Note this does change one behaviour: the current code short-circuits on the
first denial, so a denied query only audits up to the first bad column. Batch
evaluation audits the full set — which is arguably what you want for a
compliance trail anyway, but worth a conscious decision.
Small thing on the denial log at line ~146: the message is `"Column access
denied for user={}, column={}.{}.{}"` but the args are datasource/schema/table
— the column name itself, the one piece of information the operator needs,
isn't in the message.
##########
docs/dev/RangerAuthorization.md:
##########
@@ -0,0 +1,346 @@
+# Drill Ranger Authorization Quick Start Guide
+
+This document describes the architecture, configuration, and development
+conventions of the Apache Ranger authorization integration for Drill. It is
+intended for contributors who want to extend or debug the Ranger integration,
+and for operators who want to understand the column-level authorization
+behavior end-to-end.
+
+## 1. Architecture Overview
+
+The Ranger integration spans three layers:
+
+```
++--------------------------------------------------------------+
+| exec/java-exec (Drillbit, JDK 11) |
+| +-------------------------+ +------------------------+ |
+| | SqlConverter (toRel) | ---> | ColumnAccessChecker | |
+| | DrillCalciteCatalogReader| | (RelShuttle, column) | |
+| | Drillbit (startup) | +------------------------+ |
+| +-------------------------+ | |
+| v |
+| +-------------------------------+ +-----------------------+ |
+| | AccessAuthorizerFactory | | DrillAccessControl | |
+| | (singleton, config-driven) | | (static facade) | |
+| +-------------------------------+ +-----------------------+ |
+| | |
+| +----------------------------------------------------------------+
+| | drill-ranger-plugin (JDK 11, deployed to jars/3rdparty/) |
+| | DrillAuthorizer DrillAccessResource DrillRangerAccessRequest|
+| | RangerDrillPlugin RangerBaseAuthorizer |
+| +-----------------------------+--------------------------------+
+| |
+| v
+| +----------------------------------------------------------------+
+| | drill-ranger-service (JDK 8, deployed to Ranger Admin) |
+| | RangerServiceDrill (validateConfig, lookupResource) |
+| | Uses Drill REST API (POST /query.json) — NOT JDBC |
+| +----------------------------------------------------------------+
+```
+
+### 1.1 Modules
+
+| Module | JDK | Deployed To | Responsibility |
+|--------|-----|------------|----------------|
+| `drill-ranger-plugin` | 11 | Drillbit `jars/3rdparty/` | Drillbit-side
authorization: wraps `RangerBasePlugin`, exposes `DrillAccessControl` facade |
+| `drill-ranger-service` | 8 | Ranger Admin `WEB-INF/classes/lib/` | Ranger
Admin-side service plugin: `validateConfig` and `lookupResource` via Drill REST
API |
+| `exec/java-exec` | 11 | Drillbit | Integration hooks:
`AccessAuthorizerFactory`, `ColumnAccessChecker`, `DrillCalciteCatalogReader` |
+
+### 1.2 Why Two Submodules with Different JDK?
+
+Ranger Admin runs on JDK 8. If `drill-ranger-service` were compiled with JDK 11
+bytecode (class major version 55), Ranger Admin would throw
+`UnsupportedClassVersionError`. Conversely, `drill-ranger-plugin` runs inside
+Drillbit which requires JDK 11. The split ensures each jar matches its host
+runtime.
+
+`drill-ranger-service` uses the Drill REST API (`POST /query.json`) instead of
+JDBC precisely to avoid pulling in `drill-jdbc` (JDK 11 bytecode) into the
+Ranger Admin classpath.
+
+## 2. Resource Model
+
+Ranger policies for Drill use a **four-level resource hierarchy**:
+
+```
+datasource → schema → table → column
+```
+
+| Level | Ranger resource key | Example | Notes |
+|-------|--------------------|---------|-------|
+| datasource | `datasource` | `mysql` | Drill storage plugin name |
+| schema | `schema` | `shf` | Schema path WITHOUT datasource prefix |
+| table | `table` | `orders` | Table name |
+| column | `column` | `id`, `amount`, `*` | `*` matches all columns |
+
+**Critical conventions**:
+- Resource keys must be **lowercase** (`datasource`, not `DATASOURCE`). Ranger
+ validates names against `[a-z_-]` only (error code 2022).
+- The `schema` value must NOT include the datasource prefix. Use `shf`, not
+ `mysql.shf`.
+- Access type name in the service-def must exactly match what the code sends —
+ both uppercase `SELECT`.
+
+## 3. Configuration
+
+### 3.1 Drillbit side (`drill-module.conf`)
+
+```hocon
+drill.exec.security.ranger: {
+ enabled: true,
+ service.name: "drill",
+ impl: "org.apache.drill.exec.security.ranger.RangerAccessAuthorizer"
+}
+```
+
+| Key | Default | Description |
+|-----|---------|-------------|
+| `drill.exec.security.ranger.enabled` | `false` | Master switch. `false` →
`NoOpAccessAuthorizer` (fail-open) |
+| `drill.exec.security.ranger.service.name` | `"drill"` | Ranger service name
registered in Ranger Admin |
+| `drill.exec.security.ranger.impl` |
`org.apache.drill.exec.security.ranger.RangerAccessAuthorizer` |
`AccessAuthorizer` implementation class |
+
+### 3.2 Ranger Admin side
+
+Register the Drill service using `ranger-servicedef-drill.json` (located in
+`distribution/src/main/resources/ranger/`). Configure:
+
+- `drill.connection.url` — Drill REST API URL, e.g.
`http://drillbit-host:8047`.
+ Bare `host:port` is normalized to `http://host:port`.
+- `username` / `password` — Drill user for `validateConfig` and
+ `lookupResource` REST calls (HTTP Basic auth).
+
+### 3.3 Deployment Steps
+
+After building the distribution, three deployment actions are required to make
+Ranger Admin recognize Drill as an authorization provider.
+
+#### Step 1: Upload `drill-ranger-service` jar to Ranger Admin
+
+Copy the `drill-ranger-service` jar (the thin jar, NOT the
+`jar-with-dependencies` classifier) into Ranger Admin's per-service plugin
+directory. Create the `drill` subdirectory if it does not exist.
+
+```bash
+# On the Ranger Admin host
+RANGER_ADMIN_HOME=/data/ranger-2.8.1-SNAPSHOT-admin
+TARGET_DIR=$RANGER_ADMIN_HOME/ews/webapp/WEB-INF/classes/ranger-plugins/drill
+
+mkdir -p "$TARGET_DIR"
+cp drill-ranger-service-X.XX.X-SNAPSHOT.jar "$TARGET_DIR/"
+```
+
+#### Step 2: Update Ranger config files in Drill
+
+Copy the Ranger configuration files into Drill's `conf/` directory and edit
+them to match your environment.
+
+```bash
+DRILL_HOME=/opt/drill
+
+cp distribution/src/main/resources/ranger/ranger-drill-security.xml
$DRILL_HOME/conf/
+cp distribution/src/main/resources/ranger/ranger-drill-audit.xml
$DRILL_HOME/conf/
+```
+
+Then edit `$DRILL_HOME/conf/ranger-drill-security.xml`:
+
+| Property | Value to set |
+|----------|--------------|
+| `ranger.plugin.drill.policy.rest.url` | `http://<ranger-admin-host>:6080` |
+| `ranger.plugin.drill.service.name` | The Ranger service name (must match
`drill.exec.security.ranger.service.name` in `drill-override.conf`) |
+
+#### Step 3: Register the Drill service definition in Ranger Admin
+
+Upload `ranger-servicedef-drill.json` to Ranger Admin's REST API. After this
+call succeeds, the "drill" service type appears in Ranger Admin's "Service
+Manager" → "+" dropdown, and you can create a Drill service instance and
+author policies.
+
+```bash
+curl -u user:password -X POST \
+ -H "Accept: application/json" \
+ -H "Content-Type: application/json" \
+ http://ranger-admin-host:port/service/plugins/definitions \
+ -d@distribution/src/main/resources/ranger/ranger-servicedef-drill.json
+```
+### 3.4 Ranger policy files
+
+| File | Location | Purpose |
+|------|----------|---------|
+| `ranger-drill-security.xml` | `distribution/src/main/resources/ranger/` |
Ranger plugin config (policy cache dir, polling interval) |
+| `ranger-drill-audit.xml` | `distribution/src/main/resources/ranger/` | Audit
sink config (HDFS, Solr, etc.) |
+| `ranger-servicedef-drill.json` | `distribution/src/main/resources/ranger/` |
Service definition: resources, access types, config validation |
+
+### 3.5 Audit Log Configuration
+
+By default, Ranger audit records are written to the **Drillbit log** via log4j.
+This is the simplest setup and requires no external dependencies. The default
+values in `ranger-drill-audit.xml` are:
+
+| Property | Default | Description |
+|----------|---------|-------------|
+| `xasecure.audit.is.enabled` | `true` | **Master switch.** Must be `true` for
any audit destination to work. |
+| `xasecure.audit.log4j.is.enabled` | `true` | Audit to log4j (Drillbit log).
**Enabled by default.** |
+| `xasecure.audit.solr.is.enabled` | `false` | Audit to a Solr collection.
Disabled by default. |
+| `xasecure.audit.solr.url` |
`http://ranger-admin-host:6083/solr/ranger_audits` | Solr endpoint (used only
when `solr.is.enabled=true`). |
+| `xasecure.audit.hdfs.is.enabled` | `false` | Audit to HDFS. Disabled by
default. |
+| `xasecure.audit.hdfs.config.directory` | `hdfs://namenode:8020/ranger/audit`
| HDFS audit directory (used only when `hdfs.is.enabled=true`). |
+| `xasecure.audit.hdfs.config.file` | `/etc/hadoop/conf/core-site.xml` |
Hadoop config file for HDFS client (used only when `hdfs.is.enabled=true`). |
+
+> **Property name caveat:** The property names above are verified from
+> `AuditProviderFactory` bytecode in `ranger-audit-core-2.8.0.jar`. The older
+> names `xasecure.audit.is.audit.to.{log4j,solr,hdfs}` are **not** read by
+> `AuditProviderFactory` and have no effect.
+
+When `xasecure.audit.log4j.is.enabled=true`, `AuditProviderFactory` loads
+`org.apache.ranger.audit.provider.Log4jAuditProvider` (from
+`ranger-audit-dest-log4j` jar) via `Class.forName()`. That class logs audit
+events through an SLF4J logger named
+`xaaudit.org.apache.ranger.audit.provider.Log4jAuditProvider`
+(the prefix `xaaudit.` is prepended to the class name in the static
+initializer).
+
+**Important:** For audit records to reach `drillbit.log`, a logger entry for
+this logger name must be present in `logback.xml`. The shipped
+`distribution/src/main/resources/logback.xml` already includes this entry:
+
+```xml
+<logger name="xaaudit.org.apache.ranger.audit.provider.Log4jAuditProvider"
+ additivity="false" level="info">
+ <appender-ref ref="FILE" />
+</logger>
+```
+
+Without this entry, audit events (logged at INFO) fall through to the root
+logger (`error` level, STDOUT only) and are silently dropped.
+
+#### Verifying audit output in drillbit.log
+
+1. **Ranger Admin side** — create a policy that either allows or denies the
+ test user access to a table (e.g. `mysql.shf.orders`). Make sure the policy
+ is saved and the Drillbit has pulled it (default poll interval is 30 s).
+
+2. **Drill side** — run a query that triggers an authorization decision:
+
+ ```sql
+ SELECT id FROM mysql.shf.orders;
+ ```
+
+3. **Check drillbit.log** — look for audit entries from the
+ `xaaudit.org.apache.ranger.audit.provider.Log4jAuditProvider` logger:
+
+ ```bash
+ grep -i "xaaudit\|ranger\|audit\|access" $DRILL_HOME/log/drillbit.log |
tail -20
+ ```
+
+ A typical audit log line looks like:
+
+ ```
+ 2026-08-11 10:30:45,123 [...] INFO
xaaudit.org.apache.ranger.audit.provider.Log4jAuditProvider -
+ accessType=SELECT resource=mysql.shf.orders reqUser=alice ...
+ action=accessAllowed result=1
+ ```
+
+ For a denied query, `action=accessDenied` / `result=0` is logged instead.
+ If nothing appears, verify:
+ - `ranger-audit-dest-log4j-2.8.0.jar` is present in
`$DRILL_HOME/jars/3rdparty/`
+ - `xasecure.audit.is.enabled=true` and
`xasecure.audit.log4j.is.enabled=true`
+ in `$DRILL_HOME/conf/ranger/ranger-drill-audit.xml`
+ - The `logback.xml` entry above is present
+ - The Drillbit was restarted after editing configuration
+
+#### Switching the audit destination
+
+To send audit records to **Solr** instead of (or in addition to) the Drillbit
+log, edit `$DRILL_HOME/conf/ranger/ranger-drill-audit.xml` after deployment:
+
+```xml
+<property>
+ <name>xasecure.audit.solr.is.enabled</name>
+ <value>true</value>
+</property>
+<property>
+ <name>xasecure.audit.solr.url</name>
+ <value>http://your-ranger-admin-host:6083/solr/ranger_audits</value>
+</property>
+```
+
+To send audit records to **HDFS**:
+
+```xml
+<property>
+ <name>xasecure.audit.hdfs.is.enabled</name>
+ <value>true</value>
+</property>
+<property>
+ <name>xasecure.audit.hdfs.config.directory</name>
+ <value>hdfs://your-namenode:8020/ranger/audit</value>
+</property>
+<property>
+ <name>xasecure.audit.hdfs.config.file</name>
+ <value>/etc/hadoop/conf/core-site.xml</value>
+</property>
+```
+
+Multiple sinks can be enabled simultaneously — for example, keep `log4j=true`
+as a local fallback while also forwarding to Solr for centralized search. After
+changing the file, restart the Drillbit for the new settings to take effect.
+
+## 4. Authorization Policy Test Cases
+
+The following test cases document the expected authorization behavior with the
+sample policies below. All SQL runs against tables `mysql.shf.orders` and
+`mysql.shf.users`.
+
+### 4.1 Sample Ranger Policies
+
+**Policy A — users table, all columns**
+
+| Field | Value |
+|-------|-------|
+| datasource | `mysql` |
+| schema | `shf` |
+| table | `users` |
+| column | `*` |
+| access type | `SELECT` |
+| user/group | (authorized user) |
+
+**Policy B — orders table, specific columns only**
+
+| Field | Value |
+|-------|-------|
+| datasource | `mysql` |
+| schema | `shf` |
+| table | `orders` |
+| column | `id`, `amount` |
+| access type | `SELECT` |
+| user/group | (authorized user) |
+
+Under these policies, the `orders.user_id` and `orders.order_date` columns are
+NOT authorized. The `users` table allows all columns via `*`.
+
+### 4.2 Test Cases
Review Comment:
This 22-case behaviour table is the right way to specify an authorization
feature and it's the strongest part of the PR — thank you for writing it. Two
additions would make it complete:
**1. A "Known limitations" section.** Several real gaps are only
discoverable by reading the code:
- `INFORMATION_SCHEMA` and `sys` bypass authorization entirely
(`DrillAccessControl.isSystemSchema`). Any authenticated user can still
enumerate every schema, table and column name across every storage plugin,
including ones they cannot read. That's a defensible v1 position — Ranger's
Hive plugin filters these and it's a lot of extra machinery — but it should be
stated, because "column-level access control" reads as though column *names*
are protected too.
- `DROP TABLE` is not checked, and `INSERT`/`CTAS` are checked as `SELECT`
(see the comment on `DrillCalciteCatalogReader:174`).
- Correlated subquery references are not traced (see the comment on
`ColumnAccessChecker:323`).
**2. Call out case 17 as a known over-denial.** `WITH t AS (SELECT id,
order_date FROM orders) SELECT id FROM t` denying is a defensible consequence
of CTE inlining, and documenting it is exactly right — but it's currently
listed alongside 21 other rows as though it were the intended semantics. A user
who writes a wide CTE and selects one column from it will find this surprising.
Worth a note that the check is deliberately conservative here and why.
Both of these are documentation-only; the behaviours themselves are
reasonable choices for a first cut.
##########
distribution/pom.xml:
##########
@@ -73,6 +73,53 @@
<artifactId>drill-common</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.drill</groupId>
+ <artifactId>drill-ranger-plugin</artifactId>
Review Comment:
**These distribution changes are unconditional, for a feature that defaults
to `enabled: false`.**
Between this dependency block and the `copy-ranger-plugin-isolated-deps`
execution below, every Drill tarball now carries Ranger 2.8.0 plus a complete
second JAX-RS stack: `jersey-client`, `jersey-common`, `jersey-server`,
`jersey-hk2`, `jersey-media-json-jackson`, `jersey-entity-filtering` (2.35),
`hk2-api`/`hk2-locator`/`hk2-utils`, `aopalliance-repackaged`,
`osgi-resource-locator`, and the `javax.*` JAX-RS/annotation/inject APIs —
alongside the Jersey 3.1.9 that Drill's own REST server uses.
Two costs, both borne by every user regardless of whether they run Ranger:
- **Tarball size**, on top of an already large distribution.
- **CVE surface and triage load.** Jersey 2.35 and HK2 2.6.1 are pinned to
old lines here. Every future advisory against them becomes something the Drill
release manager has to answer for, even though the code is dormant in the
default configuration.
Please put the whole thing behind a Maven profile (`-Pranger`, off by
default), covering the dependency, the `dependency-plugin` execution, and the
corresponding `component.xml` dependency sets. Operators who want Ranger opt in
at build time; everyone else gets the current distribution unchanged.
The dual-Jersey coexistence via `RangerPluginClassLoader` is genuinely nice
work and I don't think it's wrong — I'd just rather not ship both stacks to
people who aren't using either one.
##########
exec/java-exec/src/test/java/org/apache/ranger/authorization/drill/authorizer/DrillAccessControl.java:
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.authorization.drill.authorizer;
+
+import java.util.Set;
+
+/**
+ * Test stub for the real {@code DrillAccessControl} class that lives in the
+ * {@code drill-ranger-plugin} module (loaded by the isolated
+ * {@code RangerPluginClassLoader} at runtime).
+ */
+public class DrillAccessControl {
Review Comment:
This is a hand-maintained shadow of `drill-ranger-plugin`'s real
`DrillAccessControl`, placed in `org.apache.ranger.*` inside `java-exec`'s test
tree so `RangerAccessAuthorizer`'s reflective lookup finds *something*.
The problem: the entire point of `RangerAccessAuthorizer` is that it binds
to `DrillAccessControl` by name and signature at runtime through the plugin
classloader. A stub that is compiled separately and updated by hand will drift
from the real class, and when it does, these tests keep passing while
production throws `NoSuchMethodException` on Drillbit startup. That's precisely
the failure the tests exist to catch.
Options, roughly in order of preference:
1. Give `java-exec` a `test`-scoped dependency on `drill-ranger-plugin` and
load the real class. (Check for a module cycle first — `drill-ranger-plugin`
would need to not depend on `java-exec`.)
2. Keep the reflection surface in one place: define the method names and
signatures as constants in a small shared interface that both the real class
and the test double implement, so a signature change breaks compilation.
3. If the stub has to stay, add an integration test in `drill-ranger-plugin`
that asserts every entry of `RangerAccessAuthorizer.METHOD_SIGNATURES` resolves
against the real `DrillAccessControl`. That's a handful of lines and catches
drift directly.
Also note this file has no license header (the ASF RAT check will flag it)
and it lives under `org.apache.ranger` in a Drill module — see the namespace
comment on `drill-ranger/pom.xml`.
##########
drill-ranger/drill-ranger-plugin/src/main/java/org/apache/ranger/authorization/drill/resource/DrillAccessResource.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ranger.authorization.drill.resource;
+
+import org.apache.ranger.plugin.policyengine.RangerAccessResourceImpl;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Map;
+import java.util.Optional;
+
+public class DrillAccessResource extends RangerAccessResourceImpl {
+
+ private static final Logger logger =
LoggerFactory.getLogger(DrillAccessResource.class);
+
+ public DrillAccessResource() {
+ }
+
+ public DrillAccessResource(Map<RangerDrillResource, Optional<String>>
resource) {
+ super();
+ for (Map.Entry<RangerDrillResource, Optional<String>> entry :
resource.entrySet()) {
+ String key = entry.getKey().toString();
+ Optional<String> value = entry.getValue();
+ value.ifPresent(s -> this.setValue(key, s));
+ if (logger.isDebugEnabled()) {
+ logger.debug("AccessResource set value: {} = {}", key, value);
+ }
+ }
+ }
+
+ public DrillAccessResource(String catalogName, Optional<String> schema,
Optional<String> table) {
+ setValue(RangerDrillResource.DATASOURCE.toString(), catalogName);
+ schema.ifPresent(s -> setValue(RangerDrillResource.SCHEMA.toString(), s));
+ table.ifPresent(s -> setValue(RangerDrillResource.TABLE.toString(), s));
+ }
+
+ public DrillAccessResource(String catalogName, Optional<String> schema,
Optional<String> table,
+ Optional<String> column) {
+ setValue(RangerDrillResource.DATASOURCE.toString(), catalogName);
+ schema.ifPresent(s -> setValue(RangerDrillResource.SCHEMA.toString(), s));
+ table.ifPresent(s -> setValue(RangerDrillResource.TABLE.toString(), s));
+ column.ifPresent(s -> setValue(RangerDrillResource.COLUMN.toString(), s));
+ }
+
+ public String getCatalogName() {
+ return (String) getValue(RangerDrillResource.DATASOURCE.toString());
+ }
+
+ public String getTable() {
+ return (String) getValue(RangerDrillResource.TABLE.toString());
+ }
+
+ public String getCatalog() {
+ return (String) getValue(RangerDrillResource.SCHEMA.toString());
+ }
+
+ public String getSchema() {
+ return (String) getValue(RangerDrillResource.SCHEMA.toString());
+ }
Review Comment:
Three accessors, two names, one concept mismatch:
- `getCatalogName()` returns the **datasource**
- `getCatalog()` returns the **schema**
- `getSchema()` returns the **schema** (identical body to `getCatalog()`)
The constructors compound it by naming the first parameter `catalogName` and
storing it under `DATASOURCE`. Given the PR's whole premise is a
clearly-defined four-level model (datasource → schema → table → column), the
accessors should use those four names and nothing else.
`getCatalog()`/`getCatalogName()` look like Presto/Trino vocabulary that leaked
in.
Suggest: `getDataSource()`, `getSchema()`, `getTable()`, `getColumn()`, drop
the duplicates, and rename the constructor params to match. `getColumn()` is
missing entirely today even though the four-arg constructor sets it.
##########
distribution/src/main/resources/drill-config.sh:
##########
@@ -365,6 +365,11 @@ export DRILLBIT_LOG_PATH="${DRILL_LOG_PREFIX}.log"
# Add Drill conf folder at the beginning of the classpath
CP="$DRILL_CONF_DIR"
+# Add Ranger config directory if it exists (for ranger-drill-security.xml etc.)
+if [ -d "$DRILL_CONF_DIR/ranger" ]; then
+ CP="$CP:$DRILL_CONF_DIR/ranger"
+fi
Review Comment:
Guarding on directory existence is the right instinct, so this is harmless
in practice — but it does prepend to `CP` for every Drillbit whether or not
Ranger is enabled, and it's placed before the "Add Drill conf folder at the
beginning of the classpath" block's intent is complete, so `conf/ranger` ends
up ahead of some entries an operator might expect to win.
Two small things:
- `RangerAuthorization.md` §3.3 Step 2 tells the operator to `cp
ranger-drill-security.xml $DRILL_HOME/conf/` (into `conf/`, not
`conf/ranger/`), but §3.5 then refers to
`$DRILL_HOME/conf/ranger/ranger-drill-audit.xml`. This code only adds
`conf/ranger`. Following the doc as written produces a Drillbit that starts
with Ranger enabled and no policy config on the classpath — which, given
`RangerBasePlugin` denies by default with no policies, fails closed but with a
confusing error. Please make the doc and the script agree on one location.
- Consider gating on the same switch as everything else, e.g. only extend
`CP` when the directory exists *and* the operator has opted in, so a stale
`conf/ranger` left over from an experiment can't affect a Drillbit running with
Ranger off.
--
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]
