DRILL-8548: Integrate Apache Ranger authorization for Drill - #3056
DRILL-8548: Integrate Apache Ranger authorization for Drill#3056shfshihuafeng wants to merge 1 commit into
Conversation
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-shade-plugin</artifactId> | ||
| <version>3.2.4</version> |
There was a problem hiding this comment.
this version is very old - 3.6.2 exists
There was a problem hiding this comment.
@pjfanning Sorry for the confusion – dependency-reduced-pom.xml was a leftover from the shade plugin and was accidentally committed in the initial commit. It will be removed.
| <dependency> | ||
| <groupId>io.netty</groupId> | ||
| <artifactId>netty-handler</artifactId> | ||
| <version>4.1.118.Final</version> |
There was a problem hiding this comment.
very old - security contingent too
We have this in the main pom.xml.
<netty.version>4.1.135.Final</netty.version>
You should be able to omit the version here and with other other netty jars (below).
| <dependency> | ||
| <groupId>org.apache.hadoop</groupId> | ||
| <artifactId>hadoop-common</artifactId> | ||
| <version>3.3.4</version> |
There was a problem hiding this comment.
again - you should not be including versions here - the version control should be in the main pom.xml
| <dependency> | ||
| <groupId>org.slf4j</groupId> | ||
| <artifactId>slf4j-api</artifactId> | ||
| <version>2.0.6</version> |
There was a problem hiding this comment.
no versions here - and why do you keep using ancient versions?
| <dependency> | ||
| <groupId>ch.qos.logback</groupId> | ||
| <artifactId>logback-classic</artifactId> | ||
| <version>1.5.25</version> |
There was a problem hiding this comment.
all these version numbers should be in main pom.xml
| <dependency> | ||
| <groupId>com.sun.jersey</groupId> | ||
| <artifactId>jersey-client</artifactId> | ||
| <version>1.19.4</version> |
There was a problem hiding this comment.
this jar version is not maintained and riddled with security issues
There was a problem hiding this comment.
Thanks for flagging this issue in the review — it's an important one. Two points in response:
Constrained by Ranger 2.8.0 (the highest resolvable release). Ranger 2.8.0's RangerAdminRESTClient / RangerRESTClient hard-code the com.sun.jersey.api.client.* API, so the plugin must ship Jersey 1.x client/core. Ranger 3.x migrated to Jersey 2.x, but Ranger 3.0.0 artifacts cannot be resolved from the configured repositories, so 2.8.0 is effectively the latest release we can consume right now. Upgrading Ranger is the only path to drop Jersey 1.x.
Tracked as a follow-up, not blocking this PR. I've left a SECURITY NOTE block in drill-ranger/drill-ranger-plugin/pom.xml marking this for when Ranger is upgraded.
Separately, I'll take a closer look at the Ranger source later to see whether there's a way to avoid shipping the Jersey 1.x client classes entirely. Do you have any other suggestions or a better approach in mind?
There was a problem hiding this comment.
Thanks for flagging this. You're right — Jersey 1.19.4 is unmaintained and carries known CVEs. I have reviewed the Ranger code and reworked the approach ,and would like to validate the approach before pushing the changes.
- Why not simply upgrade to Jersey 2.35
Ranger 2.8.0 ships RangerAdminJersey2RESTClient (in ranger-knox-plugin), which is built on Jersey 2.x (javax.ws.rs.* API + org.glassfish.jersey.* implementation). However, Drill's own REST server depends on Jersey 3.1.9 (jakarta.ws.rs.* API + org.glassfish.jersey.* implementation).
Jersey 2.x and 3.x share the same org.glassfish.jersey.* package namespace but are binary-incompatible due to the Jakarta EE 8→9 namespace migration (javax.ws.rs.* → jakarta.ws.rs.*). Putting both on the same classpath causes ClassCastException / NoSuchMethodError at runtime. So Jersey 2.35 cannot sit on Drill's main classpath alongside Jersey 3.1.9.
- Isolation approach
We use Ranger's built-in RangerPluginClassLoader — a child-first (parent-last) classloader that isolates the plugin's Jersey 2.x dependencies from the Drillbit's main classpath:
Jersey 2.35 and its transitive deps (HK2 2.6.1, jakarta.ws.rs-api:2.1.6, jakarta.annotation-api:1.3.5) are placed in a dedicated directory jars/ranger-drill-plugin-impl/, loaded only by RangerPluginClassLoader.
The Drillbit main classpath (jars/3rdparty/ + jars/classb/) keeps only Jersey 3.1.9; Jersey 2.x jars are excluded via version-pinned assembly rules and Maven dependency exclusions.
RangerAccessAuthorizer (in drill-java-exec) loads DrillAccessControl reflectively through the isolated classloader, switching the TCCL (Thread Context ClassLoader) around each invocation. This avoids any compile-scope dependency from drill-java-exec on Jersey 2.x.
ranger-plugin-classloader is the only Ranger artifact on the main classpath — it must be there to bootstrap the isolated classloader
Is this classloader-isolation approach acceptable to the community? If so, I'll push the implementation.
There was a problem hiding this comment.
if 2 can be made to work, that seems good to me
There was a problem hiding this comment.
@pjfanning I'll go with option 2, implement it, run thorough tests, and then submit the PR
There was a problem hiding this comment.
@pjfanning I've just completed the implementation and pushed the commit. Ready for re-review
cgivre
added
enhancement
|
@shfshihuafeng Do you use the Drill slack channel? I had a question for you that does not pertain to this pull request. |
|
6d7d66c to
4cf3cd7
Compare
|
@shfshihuafeng Can you please rebase to latest master. There is a fix for flaky Splunk tests that were breaking the CI. |
4cf3cd7 to
59ab5cb
Compare
done |
59ab5cb to
5c46a5d
Compare
I sent you an email. |
@cgivre Thank you for your email – I received it. I’m glad to accept the your invitation. I had already replied to your previous invitation email a few days ago, but I'm not sure if you received it. Could you please confirm? I will resend my reply right now |
@cgivre I have resent the email, but I still haven't heard back from you, so I would like to confirm whether you received my previous email. |
5c46a5d to
147f260
Compare
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.glassfish.jersey.media.multipart; |
There was a problem hiding this comment.
shouldn't we get classes like this from a dependency lib?
There was a problem hiding this comment.
@pjfanning
Background: Upgrading Ranger to Jersey 2.35 and the Necessity of ClassLoader Isolation
Ranger 2.8.0 默认 uses Jersey 1.x (com.sun.jersey.), whose package name doesn't overlap with Drill's Jersey 3.1.9 (org.glassfish.jersey.) — no isolation needed. We upgraded to RangerAdminJersey2RESTClient (Jersey 2.35) to drop the EOL Jersey 1.x and its jersey-bundle fat jar. But Jersey 2.x moved its implementation to org.glassfish.jersey.* — the same package name as Drill's 3.1.9, with a different API namespace (javax.ws.rs.* vs jakarta.ws.rs.*). They cannot coexist on one classpath, so RangerPluginClassLoader isolation is required. This is the trade-off for avoiding Jersey 1.x's legacy baggage.
Drill (Jersey 3.1.9, jakarta.ws.rs.) and Ranger 2.8.0 (Jersey 2.35, javax.ws.rs.) require two binary-incompatible Jersey versions — isolated via RangerPluginClassLoader.
1.why a shim is needed
RangerPluginClassLoader.getResources() merges the parent's 3.1.9 SPI entry → ServiceFinder loads the 3.1.9 MultiPartFeatureAutodiscoverable → cast to 2.35 ForcedAutoDiscoverable fails → ClassCastException. Since this class does not exist in Jersey 2.x (verified via jar tf), it cannot be pulled from a dependency — the shim is the only way to provide a 2.35-compatible impl with the same FQN(Fully Qualified Name).
2.The shim is safe and scoped to the Ranger plugin only
The shim lives exclusively in the Ranger plugin's isolated directory (jars/ranger-drill-plugin-impl/). It is invisible to Drill's 3.1.9 REST server, so there is no functional risk or unintended side‑effect on Drill's main codebase.
Given the above, do you have any concerns about this approach, or any suggestions for further improvements?
There was a problem hiding this comment.
@pjfanning @cgivre I implemented DrillRangerPluginClassLoader to resolve the conflict between Drill's Jersey 3.1.9 and Ranger's Jersey 2.35.
92d121c to
bf541b7
Compare
cgivre
left a comment
cgivre
left a comment
There was a problem hiding this comment.
Thanks for this. I started the review and have a few questions:
- Can we move the ranger code to the
contrib/folder? - What is the user experience when they are denied access to something?
- Drill supports table aliasing. Is that handled here?
| limitations under the License. | ||
| --> | ||
| <configuration> | ||
| <!-- Audit log destination. Set to Solr or HDFS for persistent audit storage. |
There was a problem hiding this comment.
By default, I think we should set the logging to the Drillbit log. We should however include this in the documentation so that a user would know how to configure the Ranger logging.
There was a problem hiding this comment.
@cgivre we have set the default logging to the Drillbit log, as you suggested. We have also updated the documentation to guide users on how to configure Ranger logging. The relevant section is RangerAuthorization.md#3.5 Audit Log Configuration.
I have successfully tested the audit logging feature for Drill, and it works as expected. as following:
] INFO x.o.a.r.a.p.Log4jAuditProvider - {"repoType":207,"repo":"drill","reqUser":"root","evtTime":"2026-08-12 12:40:39.349","access":"SELECT","resource":"mysql/shf/orders/id","resType":"column","action":"SELECT","result":1,"agent":"drill","policy":4,"reason":null,"enforcer":"ranger-acl","sess":null,"cliType":null,"cliIP":null,"reqData":null,"agentHost":"localhost.localdomain","logType":"RangerAudit","id":"263169cc-6b63-4799-8fd1-dd011e22ad65-1","seq_num":3,"event_count":1,"event_dur_ms":0,"tags":[],"datasets":null,"projects":null,"additional_info":null,"cluster_name":"","zone_name":null,"policy_version":7}
| */ | ||
| public class DrillAccessControl { | ||
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(DrillAccessControl.class); |
There was a problem hiding this comment.
Nit: It is a Drill convention to call the logger logger.
| * Resource validation level enum. | ||
| * Controls the depth of validation in the validateResource method. | ||
| */ | ||
| public enum ValidationLevel { |
There was a problem hiding this comment.
Does this need to exist as a separate class or is there some other place for it that would make sense?
| try { | ||
| accessType = DrillAccessType.valueOf(operator.toUpperCase()); | ||
| } catch (Exception e) { | ||
| LOG.error("Unsupported access type '{}', denied table access for user={}, schema={}, table={}", |
There was a problem hiding this comment.
What does the user see if they are trying to do something that is not allowed?
There was a problem hiding this comment.
When access is denied, DrillAccessControl returns false, and the caller throws a UserException.permissionError() with a clear message.
For table-level denial (in DrillCalciteCatalogReader):
Access denied: user 'alice' lacks SELECT privilege on mysql.shf.orders
For column-level denial (in ColumnAccessChecker):
Access denied: user 'alice' lacks SELECT privilege on one or more columns
([order_date]) of table mysql.shf.orders
| import java.util.Set; | ||
|
|
||
| public class DrillAuthorizer { | ||
| private static final Logger LOG = LoggerFactory.getLogger(DrillAuthorizer.class); |
There was a problem hiding this comment.
Nit: Drill convention is to name the logger logger. Here and elsewhere.
| */ | ||
| package org.apache.ranger.authorization.drill.resource; | ||
|
|
||
| public enum DrillAccessType { |
There was a problem hiding this comment.
CTEs don't require a dedicated DrillAccessType — they resolve to SELECT on the underlying tables. No additional handling is needed.RangerAuthorization.md#4.test case
bf541b7 to
880634d
Compare
|
880634d to
3a0ef48
Compare
There was a problem hiding this comment.
A few comments: Enforcing at DrillCalciteCatalogReader.getTable() for table-level plus a RelShuttle over RelMetadataQuery.getColumnOrigins() for column-level is the right approach, doing it before optimization is the right place, and keeping Ranger behind an AccessAuthorizer SPI with a No-Op default is good structure. The RangerPluginClassLoader work to make Jersey 2.35 and 3.1.9 coexist is genuinely hard-won, and the 22-case behaviour table in RangerAuthorization.md is the best kind of spec for a feature like this.
I've left inline comments on specifics. The ones I'd want resolved before merge:
1. Only SELECT is ever enforced (DrillCalciteCatalogReader:174). AccessTypes and the service-def advertise CREATE/DROP/INSERT/DELETE, but every call site passes SELECT. DROP TABLE bypasses both enforcement points entirely (DropTableHandler goes straight to AbstractSchema.dropTable), and INSERT/CTAS are authorized as reads. So a read-only user can drop and overwrite tables. Either wire the access types through, or scope the PR to SELECT and remove the unenforced types from the service-def — the second is a fine v1, it just needs to be explicit rather than implied.
2. Correlated subqueries bypass the column check (ColumnAccessChecker:323). RexRefCollector handles RexInputRef and RexSubQuery, but an outer column referenced from inside a subquery is a RexFieldAccess over RexCorrelVariable and is never traced. Doc case 13 uses SELECT * on the outer table, which masks it.
3. The checker runs on every query even when Ranger is off (SqlConverter:252). The full tree walk and all the getColumnOrigins calls happen before anything consults isEnabled(), so every existing deployment pays planning cost for a feature defaulting to false.
4. AccessAuthorizerFactory is a non-resettable JVM-wide static pinned by the first DrillConfig to call it. Drill runs multi-Drillbit clusters in one JVM in its own test fixtures; scoping this to DrillbitContext would fix that and also give RangerBasePlugin.cleanUp() somewhere to be called from (nothing calls it today, so the policy-refresher and audit threads leak on restart).
5. Two project-level questions that belong on dev@ rather than in this PR — I'd raise them early, since the answers could reshape the module layout:
- All the Java in
drill-ranger/lives underorg.apache.ranger.*. Drill would be publishing into another PMC's namespace; every comparable plugin (Hive, HBase, Kafka, Presto) ships from the Ranger repo instead. - The distribution changes are unconditional, so every Drill tarball gains Ranger plus a full second Jersey 2.35 + HK2 stack for a feature that's off by default. A
-Prangerprofile would keep that off the default build.
The remainder — LIKE-wildcard escaping in the service lookup, buildBaseUrl not doing what its javadoc and the docs promise, per-column Ranger calls generating one audit record per column, the MetaStoreResource/RANGER_PRESTO_APPID copy-paste leftovers, the getCatalog/getSchema accessor confusion, and the hand-maintained DrillAccessControl stub in java-exec's test tree — are all inline and mostly small.
Marking this as a comment rather than request-changes since items 1 and 5 are as much scoping decisions as defects; happy to re-review once you've had a chance to work through them.
| // 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); |
There was a problem hiding this 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:
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.
| 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) { |
There was a problem hiding this 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:
-- 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):
@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.
| public RelNode visit(LogicalAggregate aggregate) { | ||
| for (int i : aggregate.getGroupSet()) { | ||
| traceColumnOrigin(aggregate.getInput(), i); | ||
| } | ||
| return super.visit(aggregate); |
There was a problem hiding this 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:
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.
| 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(); |
There was a problem hiding this 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.
| } | ||
| String userName = session.getCredentials().getUserName(); | ||
|
|
||
| if (!authorizer.checkTableAccess(userName, dataSource, schemaPath, tableName, AccessTypes.SELECT)) { |
There was a problem hiding this 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:
DROP TABLEis not checked at all.DropTableHandlerresolves the table throughSchemaUtilities.resolveToDrillSchema/SqlHandlerUtil.getTableFromSchemaand callsAbstractSchema.dropTabledirectly — it never goes through this catalog reader, and it never reachesSqlConverter.toRel, so neither enforcement point fires. Same forDropFunctionHandler,CreateAliasHandler, etc.INSERTandCTASare authorized asSELECT. The target table does resolve throughgetTable(), but the request Ranger sees saysaccessType=SELECT. A user granted read-only onmysql.shf.orderscanINSERT INTOit and, viaDROP, 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
SqlKindis available on the validated node inSqlConverter) and add an explicit check inDropTableHandler/CreateTableHandler/InsertHandlerforDROP/CREATE/INSERT. - Scope the PR to SELECT. Remove the unused constants from
AccessTypesandDrillAccessType, strip the corresponding access types fromranger-servicedef-drill.json, and state prominently inRangerAuthorization.mdthat 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.
| * {@code drill-ranger-plugin} module (loaded by the isolated | ||
| * {@code RangerPluginClassLoader} at runtime). | ||
| */ | ||
| public class DrillAccessControl { |
There was a problem hiding this 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:
- Give
java-execatest-scoped dependency ondrill-ranger-pluginand load the real class. (Check for a module cycle first —drill-ranger-pluginwould need to not depend onjava-exec.) - 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.
- If the stub has to stay, add an integration test in
drill-ranger-pluginthat asserts every entry ofRangerAccessAuthorizer.METHOD_SIGNATURESresolves against the realDrillAccessControl. 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.
| <module>drill-ranger-plugin</module> | ||
| <module>drill-ranger-service</module> |
There was a problem hiding this 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.
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.drill</groupId> | ||
| <artifactId>drill-ranger-plugin</artifactId> |
There was a problem hiding this 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.
| # 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 |
There was a problem hiding this 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 tocp ranger-drill-security.xml $DRILL_HOME/conf/(intoconf/, notconf/ranger/), but §3.5 then refers to$DRILL_HOME/conf/ranger/ranger-drill-audit.xml. This code only addsconf/ranger. Following the doc as written produces a Drillbit that starts with Ranger enabled and no policy config on the classpath — which, givenRangerBasePlugindenies 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
CPwhen the directory exists and the operator has opted in, so a staleconf/rangerleft over from an experiment can't affect a Drillbit running with Ranger off.
| ## 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 |
There was a problem hiding this 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_SCHEMAandsysbypass 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 TABLEis not checked, andINSERT/CTASare checked asSELECT(see the comment onDrillCalciteCatalogReader: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.
cgivre
left a comment
cgivre
left a comment
There was a problem hiding this comment.
I left some comments as well. Thanks for submitting this.
3 participants
DRILL-8548: Integrate Apache Ranger authorization for Drill
Description
Motivation
Apache Drill is a federated query engine that concurrently accesses heterogeneous data sources such as relational databases (MySQL), file systems (local/HDFS), and search engines (Elasticsearch). However, each data source has its own independent permission model with varying granularities – relational databases support schema/table/column level controls, file systems often lack column‑level privileges, and Elasticsearch only provides index‑level authorization. This forces administrators to maintain separate permission schemes for each data source individually, resulting in cumbersome configuration, high operational overhead, and the inability to achieve cross‑source unified auditing. Therefore, it is necessary to build a unified, fine‑grained permission governance system for Drill, which is primarily embodied as follows:
Unified Policy Management: All data sources accessible via Drill can be governed through a single Ranger console. A single Ranger policy applies simultaneously across multiple heterogeneous sources, eliminating the need for per‑storage‑system configuration and greatly simplifying administrative complexity.
Column‑Level Fine‑Grained Access Control: Permissions can be restricted down to individual columns within a table, satisfying the need to isolate sensitive fields (e.g., ID numbers, salaries, phone numbers) and achieving true field‑level security protection.
Consistent Enforcement at the Federation Layer: Permission checks are performed uniformly during Drill's
toRelphase (before logical plan generation), ensuring that the same set of policies takes effect regardless of whether the query is a single‑table scan, a cross‑source JOIN, or a nested subquery. There is no possibility of bypassing Ranger through other storage plugins, guaranteeing complete integrity of mandatory enforcement.Audit & Compliance Readiness: Every access decision is recorded and pushed into Ranger's audit pipeline, providing a complete audit trail of "who queried what and when". This natively supports compliance reporting requirements.
Introduction
To address the resource hierarchy discrepancies among different data sources (e.g., relational databases support database/table/column levels, whereas file systems lack the Schema concept), I designed a four‑level logical resource model (DataSource → Schema → Table → Column) that uniformly maps the structures of heterogeneous data sources onto this hierarchy, thereby providing a unified resource addressing foundation for Ranger policy configuration.
This PR introduces Apache Ranger as a pluggable authorization framework for Drill, enabling centralized table-level and column-level access control for Drill queries. It is a substantial feature spanning three layers: a new
drill-rangermodule (authorization plugin + Ranger Admin service plugin), integration hooks inexec/java-exec, and distribution packaging. The design follows Drill's existingAccessAuthorizerSPI and Calcite'sRelShuttlemechanism so that column-level checks happen in thetoRelphase before physical planning.Documentation
This PR ships a developer-facing quick start guide, RangerAuthorization.md, covering the end-to-end architecture, configuration, deployment, and authorization behavior of the Ranger integration. It is intended both for contributors extending/debugging the integration and for operators who want to understand the column-level authorization flow.
Testing
drill-ranger-service(1 file, 28 cases, JUnit 5 + Mockito)RangerServiceDrillTest—buildBaseUrlnormalization,escapeSqlSQL-injection guard,firstHint/extractFirstColumnValuesJSON parsing,lookupResource/validateConfigedge cases.exec/java-exec(4 files, 27 cases, JUnit 4 + Mockito, extendsBaseTest)AccessAuthorizerFactoryTest— config-driven reflective loading, caching, failure modes.NoOpAccessAuthorizerTest— fail-open contract.RangerAccessAuthorizerTest— delegation toDrillAccessControlwithDrillAccessTypeenum normalization (null/unknown →SELECT).DrillCalciteCatalogReaderTest—getDefaultSchemaByDataSourcepackage-private static helper.All tests pass on Windows with
-DforkCount=0(the defaultforkCount=1fails withCreateProcess error=5on locked-down Windows hosts).