You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by lv...@apache.org on 2019/02/21 01:08:51 UTC
[impala] branch master updated (b8a8edd -> 257fa0c)
This is an automated email from the ASF dual-hosted git repository.
lv pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from b8a8edd IMPALA-8207: Fix query loading for perf and stress tests
new c1274fa IMPALA-8191: Wait for additional breakpad processes during test
new c659b78 IMPALA-8214: Fix bad plan in load_nested.py
new 290e48f IMPALA-7917 (Part 1): Decouple Sentry from Impala
new 0b7c964 Adding hostname to Disk I/O errors.
new 113b2eb Logging current database context when logging analyzing queries.
new 257fa0c IMPALA-8209: Include fragment instance ID in memz/ breakdown
The 6 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/exprs/timezone_db.cc | 3 +-
be/src/runtime/io/disk-io-mgr-test.cc | 10 +-
be/src/runtime/io/disk-io-mgr.cc | 8 +-
be/src/runtime/io/error-converter.cc | 4 +-
be/src/runtime/io/hdfs-file-reader.cc | 8 +-
be/src/runtime/io/hdfs-monitored-ops.cc | 2 +-
be/src/runtime/io/local-file-reader.cc | 6 +-
be/src/runtime/runtime-state.cc | 3 +-
be/src/util/debug-util-test.cc | 2 +-
be/src/util/debug-util.cc | 3 +-
common/thrift/generate_error_codes.py | 2 +-
.../org/apache/impala/analysis/AdminFnStmt.java | 2 +-
.../analysis/AlterTableSetTblProperties.java | 2 +-
.../apache/impala/analysis/AnalysisContext.java | 25 ++-
.../java/org/apache/impala/analysis/Analyzer.java | 23 ++-
.../apache/impala/analysis/AuthorizationStmt.java | 12 +-
.../apache/impala/analysis/CollectionTableRef.java | 2 +-
.../apache/impala/analysis/CopyTestCaseStmt.java | 4 +-
.../impala/analysis/CreateFunctionStmtBase.java | 8 +-
.../apache/impala/analysis/CreateTableStmt.java | 2 +-
.../apache/impala/analysis/DescribeTableStmt.java | 6 +-
.../apache/impala/analysis/DropFunctionStmt.java | 8 +-
.../java/org/apache/impala/analysis/HdfsUri.java | 7 +-
.../org/apache/impala/analysis/InsertStmt.java | 2 +-
.../apache/impala/analysis/ResetMetadataStmt.java | 11 +-
.../{Authorizeable.java => Authorizable.java} | 37 +++--
.../impala/authorization/AuthorizationChecker.java | 175 ++++++---------------
.../impala/authorization/AuthorizationConfig.java | 123 +--------------
.../AuthorizationException.java | 3 +-
.../AuthorizationPolicy.java | 109 +++----------
.../AuthorizationProvider.java} | 15 +-
.../org/apache/impala/authorization/Privilege.java | 103 ++++--------
.../impala/authorization/PrivilegeRequest.java | 42 ++---
.../authorization/PrivilegeRequestBuilder.java | 77 ++++++---
.../impala/authorization/sentry/ImpalaAction.java | 87 ++++++++++
.../{ => sentry}/ImpalaActionFactory.java | 4 +-
.../{ => sentry}/ImpalaPrivilegeModel.java | 3 +-
.../{ => sentry}/SentryAuthProvider.java | 25 +--
.../authorization/sentry/SentryAuthorizable.java | 59 +++++++
.../SentryAuthorizableColumn.java} | 49 ++++--
.../SentryAuthorizableDb.java} | 28 ++--
.../SentryAuthorizableFn.java} | 33 ++--
.../SentryAuthorizableServer.java} | 34 ++--
.../SentryAuthorizableTable.java} | 44 +++---
.../SentryAuthorizableUri.java} | 32 ++--
.../sentry/SentryAuthorizationChecker.java | 127 +++++++++++++++
.../SentryAuthorizationConfig.java} | 48 ++++--
.../sentry/SentryAuthorizationPolicy.java | 102 ++++++++++++
.../authorization/{ => sentry}/SentryConfig.java | 2 +-
.../sentry}/SentryPolicyReaderException.java | 2 +-
.../sentry}/SentryPolicyService.java | 8 +-
.../sentry}/SentryProxy.java | 11 +-
.../sentry}/SentryUnavailableException.java | 2 +-
.../{util => authorization/sentry}/SentryUtil.java | 21 ++-
.../java/org/apache/impala/catalog/Catalog.java | 4 +-
.../impala/catalog/CatalogServiceCatalog.java | 40 +++--
.../java/org/apache/impala/catalog/FeCatalog.java | 1 +
.../org/apache/impala/catalog/ImpaladCatalog.java | 1 +
.../apache/impala/catalog/PrincipalPrivilege.java | 1 +
.../impala/catalog/local/CatalogdMetaProvider.java | 5 +-
.../impala/catalog/local/DirectMetaProvider.java | 2 +-
.../apache/impala/catalog/local/LocalCatalog.java | 2 +-
.../apache/impala/catalog/local/MetaProvider.java | 2 +-
.../apache/impala/service/CatalogOpExecutor.java | 14 +-
.../java/org/apache/impala/service/Frontend.java | 61 +++----
.../java/org/apache/impala/service/JniCatalog.java | 10 +-
.../org/apache/impala/service/JniFrontend.java | 4 +-
.../impala/analysis/AnalyzeAuthStmtsTest.java | 8 +-
.../org/apache/impala/analysis/AuditingTest.java | 11 +-
.../impala/analysis/AuthorizationStmtTest.java | 8 +-
.../apache/impala/analysis/AuthorizationTest.java | 101 ++++++------
.../impala/analysis/StmtMetadataLoaderTest.java | 8 +-
.../{ => sentry}/ImpalaActionFactoryTest.java | 5 +-
.../sentry}/SentryProxyTest.java | 10 +-
.../org/apache/impala/catalog/CatalogTest.java | 1 +
.../events/MetastoreEventsProcessorTest.java | 7 +-
.../org/apache/impala/common/FrontendFixture.java | 7 +-
.../org/apache/impala/common/FrontendTestBase.java | 9 +-
.../org/apache/impala/common/QueryFixture.java | 4 +-
.../impala/testutil/CatalogServiceTestCatalog.java | 17 +-
.../apache/impala/testutil/ImpaladTestCatalog.java | 5 +-
.../impala/testutil/PlannerTestCaseLoader.java | 5 +-
.../impala/testutil/SentryServicePinger.java | 4 +-
fe/src/test/resources/authz-policy.ini.template | 2 +-
testdata/bin/load_nested.py | 6 +-
.../queries/PlannerTest/mt-dop-validation.test | 16 +-
.../queries/PlannerTest/tpch-nested.test | 2 +-
tests/common/impala_cluster.py | 63 ++++++--
tests/custom_cluster/test_breakpad.py | 14 +-
tests/webserver/test_web_pages.py | 14 ++
90 files changed, 1157 insertions(+), 887 deletions(-)
rename fe/src/main/java/org/apache/impala/authorization/{Authorizeable.java => Authorizable.java} (67%)
rename fe/src/main/java/org/apache/impala/{catalog => authorization}/AuthorizationException.java (92%)
rename fe/src/main/java/org/apache/impala/{catalog => authorization}/AuthorizationPolicy.java (86%)
copy fe/src/main/java/org/apache/impala/{util/Visitor.java => authorization/AuthorizationProvider.java} (82%)
create mode 100644 fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaAction.java
rename fe/src/main/java/org/apache/impala/authorization/{ => sentry}/ImpalaActionFactory.java (95%)
rename fe/src/main/java/org/apache/impala/authorization/{ => sentry}/ImpalaPrivilegeModel.java (97%)
rename fe/src/main/java/org/apache/impala/authorization/{ => sentry}/SentryAuthProvider.java (74%)
create mode 100644 fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizable.java
rename fe/src/main/java/org/apache/impala/authorization/{AuthorizeableColumn.java => sentry/SentryAuthorizableColumn.java} (59%)
rename fe/src/main/java/org/apache/impala/authorization/{AuthorizeableDb.java => sentry/SentryAuthorizableDb.java} (68%)
rename fe/src/main/java/org/apache/impala/authorization/{AuthorizeableFn.java => sentry/SentryAuthorizableFn.java} (64%)
rename fe/src/main/java/org/apache/impala/authorization/{AuthorizeableServer.java => sentry/SentryAuthorizableServer.java} (60%)
rename fe/src/main/java/org/apache/impala/authorization/{AuthorizeableTable.java => sentry/SentryAuthorizableTable.java} (64%)
rename fe/src/main/java/org/apache/impala/authorization/{AuthorizeableUri.java => sentry/SentryAuthorizableUri.java} (59%)
create mode 100644 fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
copy fe/src/main/java/org/apache/impala/authorization/{AuthorizationConfig.java => sentry/SentryAuthorizationConfig.java} (80%)
create mode 100644 fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
rename fe/src/main/java/org/apache/impala/authorization/{ => sentry}/SentryConfig.java (98%)
rename fe/src/main/java/org/apache/impala/{common => authorization/sentry}/SentryPolicyReaderException.java (96%)
rename fe/src/main/java/org/apache/impala/{util => authorization/sentry}/SentryPolicyService.java (98%)
rename fe/src/main/java/org/apache/impala/{util => authorization/sentry}/SentryProxy.java (98%)
rename fe/src/main/java/org/apache/impala/{common => authorization/sentry}/SentryUnavailableException.java (96%)
rename fe/src/main/java/org/apache/impala/{util => authorization/sentry}/SentryUtil.java (72%)
rename fe/src/test/java/org/apache/impala/authorization/{ => sentry}/ImpalaActionFactoryTest.java (96%)
rename fe/src/test/java/org/apache/impala/{util => authorization/sentry}/SentryProxyTest.java (98%)
[impala] 02/06: IMPALA-8214: Fix bad plan in load_nested.py
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit c659b78198a767b91c293cbaf77f5c8b269fba39
Author: Tim Armstrong <ta...@cloudera.com>
AuthorDate: Mon Feb 18 22:53:53 2019 -0800
IMPALA-8214: Fix bad plan in load_nested.py
The previous plan had the larger input on the build side of the join and
did a broadcast join, which is very suboptimal.
This speeds up data loading on my minicluster - 18s vs 31s and has a
more significant impact on a real cluster, where queries execute
much faster, the memory requirement is significantly reduced and
the data loading can potentially be broken up into fewer chunks.
I also considered computing stats on the table to let Impala generate
the same plan, but this achieves the same goal more efficiently.
Testing:
Run core tests. Resource estimates in planner tests changed slightly
because of the different distribution of data.
Change-Id: I55e0ca09590a90ba530efe4e8f8bf587dde3eeeb
Reviewed-on: http://gerrit.cloudera.org:8080/12519
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
testdata/bin/load_nested.py | 6 +++---
.../queries/PlannerTest/mt-dop-validation.test | 16 ++++++++--------
.../queries/PlannerTest/tpch-nested.test | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/testdata/bin/load_nested.py b/testdata/bin/load_nested.py
index d391fdb..93d8339 100755
--- a/testdata/bin/load_nested.py
+++ b/testdata/bin/load_nested.py
@@ -104,7 +104,7 @@ def load():
for chunk_idx in xrange(chunks):
sql_params["chunk_idx"] = chunk_idx
tmp_customer_sql = r"""
- SELECT
+ SELECT STRAIGHT_JOIN
c_custkey, c_name, c_address, c_nationkey, c_phone, c_acctbal, c_mktsegment,
c_comment,
GROUP_CONCAT(
@@ -120,8 +120,8 @@ def load():
CAST(lineitems_string AS STRING)
), '\002'
) orders_string
- FROM {source_db}.customer
- LEFT JOIN tmp_orders_string ON c_custkey = o_custkey
+ FROM tmp_orders_string
+ RIGHT JOIN [SHUFFLE] {source_db}.customer ON c_custkey = o_custkey
WHERE c_custkey % {chunks} = {chunk_idx}
GROUP BY 1, 2, 3, 4, 5, 6, 7, 8""".format(**sql_params)
LOG.info("Creating temp customers (chunk {chunk} of {chunks})".format(
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test b/testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
index 5d340f3..f749a4e 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
@@ -219,7 +219,7 @@ from tpch_nested_parquet.customer c, c.c_orders o, o.o_lineitems
where c_custkey < 10 and o_orderkey < 5 and l_linenumber < 3
---- PLAN
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
-| Per-Host Resources: mem-estimate=88.00MB mem-reservation=88.00MB thread-reservation=1
+| Per-Host Resources: mem-estimate=104.00MB mem-reservation=104.00MB thread-reservation=1
PLAN-ROOT SINK
| mem-estimate=0B mem-reservation=0B thread-reservation=0
|
@@ -275,14 +275,14 @@ PLAN-ROOT SINK
stored statistics:
table: rows=150000 size=288.99MB
columns missing stats: c_orders
- extrapolated-rows=disabled max-scan-range-rows=44225
+ extrapolated-rows=disabled max-scan-range-rows=50116
parquet statistics predicates: c_custkey < CAST(10 AS BIGINT)
parquet statistics predicates on o: o_orderkey < CAST(5 AS BIGINT)
parquet statistics predicates on o_lineitems: l_linenumber < CAST(3 AS INT)
parquet dictionary predicates: c_custkey < CAST(10 AS BIGINT)
parquet dictionary predicates on o: o_orderkey < CAST(5 AS BIGINT)
parquet dictionary predicates on o_lineitems: l_linenumber < CAST(3 AS INT)
- mem-estimate=88.00MB mem-reservation=88.00MB thread-reservation=0
+ mem-estimate=104.00MB mem-reservation=104.00MB thread-reservation=0
tuple-ids=0 row-size=230B cardinality=15.00K
in pipelines: 00(GETNEXT)
---- PARALLELPLANS
@@ -297,7 +297,7 @@ PLAN-ROOT SINK
| in pipelines: 00(GETNEXT)
|
F00:PLAN FRAGMENT [RANDOM] hosts=3 instances=9
-Per-Host Resources: mem-estimate=264.00MB mem-reservation=264.00MB thread-reservation=3
+Per-Host Resources: mem-estimate=312.00MB mem-reservation=312.00MB thread-reservation=3
01:SUBPLAN
| mem-estimate=0B mem-reservation=0B thread-reservation=0
| tuple-ids=2,1,0 row-size=482B cardinality=1.50M
@@ -350,14 +350,14 @@ Per-Host Resources: mem-estimate=264.00MB mem-reservation=264.00MB thread-reserv
stored statistics:
table: rows=150000 size=288.99MB
columns missing stats: c_orders
- extrapolated-rows=disabled max-scan-range-rows=44225
+ extrapolated-rows=disabled max-scan-range-rows=50116
parquet statistics predicates: c_custkey < CAST(10 AS BIGINT)
parquet statistics predicates on o: o_orderkey < CAST(5 AS BIGINT)
parquet statistics predicates on o_lineitems: l_linenumber < CAST(3 AS INT)
parquet dictionary predicates: c_custkey < CAST(10 AS BIGINT)
parquet dictionary predicates on o: o_orderkey < CAST(5 AS BIGINT)
parquet dictionary predicates on o_lineitems: l_linenumber < CAST(3 AS INT)
- mem-estimate=88.00MB mem-reservation=88.00MB thread-reservation=0
+ mem-estimate=104.00MB mem-reservation=104.00MB thread-reservation=0
tuple-ids=0 row-size=230B cardinality=15.00K
in pipelines: 00(GETNEXT)
====
@@ -413,7 +413,7 @@ PLAN-ROOT SINK
stored statistics:
table: rows=150000 size=288.99MB
columns missing stats: c_orders, c_orders
- extrapolated-rows=disabled max-scan-range-rows=44225
+ extrapolated-rows=disabled max-scan-range-rows=50116
parquet statistics predicates on o1: o1.o_orderkey < CAST(5 AS BIGINT)
parquet dictionary predicates on o1: o1.o_orderkey < CAST(5 AS BIGINT)
mem-estimate=88.00MB mem-reservation=16.00MB thread-reservation=0
@@ -474,7 +474,7 @@ Per-Host Resources: mem-estimate=269.81MB mem-reservation=53.81MB thread-reserva
stored statistics:
table: rows=150000 size=288.99MB
columns missing stats: c_orders, c_orders
- extrapolated-rows=disabled max-scan-range-rows=44225
+ extrapolated-rows=disabled max-scan-range-rows=50116
parquet statistics predicates on o1: o1.o_orderkey < CAST(5 AS BIGINT)
parquet dictionary predicates on o1: o1.o_orderkey < CAST(5 AS BIGINT)
mem-estimate=88.00MB mem-reservation=16.00MB thread-reservation=0
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
index bd2c6d1..fd6363a 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
@@ -3192,7 +3192,7 @@ PLAN-ROOT SINK
predicates: substr(c_phone, 1, 2) IN ('13', '31', '23', '29', '30', '18', '17')
row-size=47B cardinality=15.00K
---- DISTRIBUTEDPLAN
-Max Per-Host Resource Reservation: Memory=25.94MB Threads=8
+Max Per-Host Resource Reservation: Memory=27.94MB Threads=8
Per-Host Resource Estimates: Memory=688MB
PLAN-ROOT SINK
|
[impala] 05/06: Logging current database context when logging
analyzing queries.
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 113b2eb120ca36777b5f33632deb3a5dcfe3322a
Author: Philip Zeyliger <ph...@cloudera.com>
AuthorDate: Thu Dec 27 14:47:53 2018 -0800
Logging current database context when logging analyzing queries.
The "analyzing query" log message is a reliable way to correlate
queries with ther thread ids, but the session database name
is missing there. Adding it in.
Change-Id: I70886be4685e0d3ae7ca9f6e57e8159dc39c67c7
Reviewed-on: http://gerrit.cloudera.org:8080/12301
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
fe/src/main/java/org/apache/impala/service/Frontend.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 2e61472..5d0ae61 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -1248,7 +1248,8 @@ public class Frontend {
private TExecRequest getTExecRequest(PlanCtx planCtx, EventSequence timeline)
throws ImpalaException {
TQueryCtx queryCtx = planCtx.getQueryContext();
- LOG.info("Analyzing query: " + queryCtx.client_request.stmt);
+ LOG.info("Analyzing query: " + queryCtx.client_request.stmt + " db: "
+ + queryCtx.session.database);
int attempt = 0;
String retryMsg = "";
[impala] 04/06: Adding hostname to Disk I/O errors.
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 0b7c9645451e6c2d30e225a27a89a901df940b09
Author: Philip Zeyliger <ph...@cloudera.com>
AuthorDate: Thu Feb 7 15:31:19 2019 -0800
Adding hostname to Disk I/O errors.
I recently ran into some queries that failed like so:
WARNINGS: Disk I/O error: Could not open file: /data/...: Error(5): Input/output error
These warnings were in the profile, but I had to cross-reference impalad
logs to figure out which machine had the broken disk.
In this commit, I've sprinkled GetBackendString() to include it.
Change-Id: Ib977d2c0983ef81ab1338de090239ed57f3efde2
Reviewed-on: http://gerrit.cloudera.org:8080/12402
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/exprs/timezone_db.cc | 3 ++-
be/src/runtime/io/disk-io-mgr-test.cc | 10 ++++++----
be/src/runtime/io/disk-io-mgr.cc | 8 ++++----
be/src/runtime/io/error-converter.cc | 4 ++--
be/src/runtime/io/hdfs-file-reader.cc | 8 ++++----
be/src/runtime/io/hdfs-monitored-ops.cc | 2 +-
be/src/runtime/io/local-file-reader.cc | 6 +++---
common/thrift/generate_error_codes.py | 2 +-
8 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/be/src/exprs/timezone_db.cc b/be/src/exprs/timezone_db.cc
index 577dea2..ad50e6b 100644
--- a/be/src/exprs/timezone_db.cc
+++ b/be/src/exprs/timezone_db.cc
@@ -30,6 +30,7 @@
#include "gutil/strings/ascii_ctype.h"
#include "gutil/strings/substitute.h"
#include "runtime/hdfs-fs-cache.h"
+#include "util/debug-util.h"
#include "util/filesystem-util.h"
#include "util/hdfs-util.h"
#include "util/string-parser.h"
@@ -387,7 +388,7 @@ Status TimezoneDatabase::LoadZoneAliasesFromHdfs(const string& hdfs_zone_alias_c
current_bytes_read = hdfsRead(hdfs_conn, hdfs_file, buffer.data(), buffer.size());
if (current_bytes_read == 0) break;
if (current_bytes_read < 0) {
- status = Status(TErrorCode::DISK_IO_ERROR,
+ status = Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
GetHdfsErrorMsg("Error reading from HDFS file: ", hdfs_zone_alias_conf));
break;
}
diff --git a/be/src/runtime/io/disk-io-mgr-test.cc b/be/src/runtime/io/disk-io-mgr-test.cc
index 057713a..a8e07b8 100644
--- a/be/src/runtime/io/disk-io-mgr-test.cc
+++ b/be/src/runtime/io/disk-io-mgr-test.cc
@@ -360,8 +360,9 @@ TEST_F(DiskIoMgrTest, InvalidWrite) {
WriteRange::WriteDoneCallback callback =
bind(mem_fn(&DiskIoMgrTest::WriteValidateCallback), this, num_of_writes, new_range,
nullptr, nullptr, nullptr, data,
- Status(TErrorCode::DISK_IO_ERROR, "open() failed for /non-existent/file.txt. "
- "The given path doesn't exist. errno=2"), _1);
+ Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
+ "open() failed for /non-existent/file.txt. "
+ "The given path doesn't exist. errno=2"), _1);
*new_range = pool_.Add(new WriteRange(tmp_file, rand(), 0, callback));
(*new_range)->SetData(reinterpret_cast<uint8_t*>(data), sizeof(int32_t));
@@ -378,7 +379,8 @@ TEST_F(DiskIoMgrTest, InvalidWrite) {
new_range = pool_.Add(new WriteRange*);
callback = bind(mem_fn(&DiskIoMgrTest::WriteValidateCallback), this, num_of_writes,
new_range, nullptr, nullptr, nullptr, data,
- Status(TErrorCode::DISK_IO_ERROR, "fseek() failed for /tmp/disk_io_mgr_test.txt. "
+ Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
+ "fseek() failed for /tmp/disk_io_mgr_test.txt. "
"Invalid inputs. errno=22, offset=-1"), _1);
*new_range = pool_.Add(new WriteRange(tmp_file, -1, 0, callback));
@@ -466,7 +468,7 @@ void DiskIoMgrTest::AddWriteRange(int num_of_writes, int32_t* data,
WriteRange::WriteDoneCallback callback =
bind(mem_fn(&DiskIoMgrTest::WriteValidateCallback), this, num_of_writes,
nullptr, nullptr, nullptr, nullptr, data,
- Status(TErrorCode::DISK_IO_ERROR, expected_output), _1);
+ Status(TErrorCode::DISK_IO_ERROR, GetBackendString(), expected_output), _1);
WriteRange* write_range = pool_.Add(new WriteRange(file_name, offset, 0, callback));
write_range->SetData(reinterpret_cast<uint8_t*>(data), sizeof(int32_t));
EXPECT_OK(writer->AddWriteRange(write_range));
diff --git a/be/src/runtime/io/disk-io-mgr.cc b/be/src/runtime/io/disk-io-mgr.cc
index ce56be0..394745f 100644
--- a/be/src/runtime/io/disk-io-mgr.cc
+++ b/be/src/runtime/io/disk-io-mgr.cc
@@ -293,15 +293,15 @@ void DiskIoMgr::UnregisterContext(RequestContext* reader) {
Status DiskIoMgr::ValidateScanRange(ScanRange* range) {
int disk_id = range->disk_id();
if (disk_id < 0 || disk_id >= disk_queues_.size()) {
- return Status(TErrorCode::DISK_IO_ERROR,
- Substitute("Invalid scan range. Bad disk id: $0", disk_id));
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
+ Substitute("Invalid scan range. Bad disk id: $0", disk_id));
}
if (range->offset() < 0) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Invalid scan range. Negative offset $0", range->offset()));
}
if (range->len() <= 0) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Invalid scan range. Non-positive length $0", range->len()));
}
return Status::OK();
diff --git a/be/src/runtime/io/error-converter.cc b/be/src/runtime/io/error-converter.cc
index 966558d..547f086 100644
--- a/be/src/runtime/io/error-converter.cc
+++ b/be/src/runtime/io/error-converter.cc
@@ -49,8 +49,8 @@ unordered_map<int, string> ErrorConverter::errno_to_error_text_map_(
Status ErrorConverter::GetErrorStatusFromErrno(const string& function_name,
const string& file_path, int err_no, const Params& params) {
- return Status(ErrorMsg(TErrorCode::DISK_IO_ERROR, GetErrorText(function_name,
- file_path, err_no, params)));
+ return Status(ErrorMsg(TErrorCode::DISK_IO_ERROR, GetBackendString(),
+ GetErrorText(function_name, file_path, err_no, params)));
}
string ErrorConverter::GetErrorText(const string& function_name,
diff --git a/be/src/runtime/io/hdfs-file-reader.cc b/be/src/runtime/io/hdfs-file-reader.cc
index b495d61..0bbf984 100644
--- a/be/src/runtime/io/hdfs-file-reader.cc
+++ b/be/src/runtime/io/hdfs-file-reader.cc
@@ -59,7 +59,7 @@ Status HdfsFileReader::Open(bool use_file_handle_cache) {
if (hdfsSeek(hdfs_fs_, exclusive_hdfs_fh_->file(), scan_range_->offset_) != 0) {
// Destroy the file handle
io_mgr->ReleaseExclusiveHdfsFileHandle(std::move(exclusive_hdfs_fh_));
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Error seeking to $0 in file: $1 $2", scan_range_->offset(),
*scan_range_->file_string(), GetHdfsErrorMsg("")));
}
@@ -165,7 +165,7 @@ Status HdfsFileReader::ReadFromPosInternal(hdfsFile hdfs_file, int64_t position_
if (FLAGS_use_hdfs_pread) {
*bytes_read = hdfsPread(hdfs_fs_, hdfs_file, position_in_file, buffer, chunk_size);
if (*bytes_read == -1) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
GetHdfsErrorMsg("Error reading from HDFS file: ",
*scan_range_->file_string()));
}
@@ -174,14 +174,14 @@ Status HdfsFileReader::ReadFromPosInternal(hdfsFile hdfs_file, int64_t position_
// location. Seek to the appropriate location.
if (is_borrowed_fh) {
if (hdfsSeek(hdfs_fs_, hdfs_file, position_in_file) != 0) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Error seeking to $0 in file: $1: $2",
position_in_file, *scan_range_->file_string(), GetHdfsErrorMsg("")));
}
}
*bytes_read = hdfsRead(hdfs_fs_, hdfs_file, buffer, chunk_size);
if (*bytes_read == -1) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
GetHdfsErrorMsg("Error reading from HDFS file: ",
*scan_range_->file_string()));
}
diff --git a/be/src/runtime/io/hdfs-monitored-ops.cc b/be/src/runtime/io/hdfs-monitored-ops.cc
index 3ea5c86..f864b7c 100644
--- a/be/src/runtime/io/hdfs-monitored-ops.cc
+++ b/be/src/runtime/io/hdfs-monitored-ops.cc
@@ -69,7 +69,7 @@ Status OpenHdfsFileOp::Execute() {
if (hdfs_file_ == nullptr) {
// GetHdfsErrorMsg references thread local state to get error information, so it
// must happen in the same thread as the hdfsOpenFile().
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
GetHdfsErrorMsg("Failed to open HDFS file ", fname_));
}
return Status::OK();
diff --git a/be/src/runtime/io/local-file-reader.cc b/be/src/runtime/io/local-file-reader.cc
index 3f88106..9be45a9 100644
--- a/be/src/runtime/io/local-file-reader.cc
+++ b/be/src/runtime/io/local-file-reader.cc
@@ -40,7 +40,7 @@ Status LocalFileReader::Open(bool use_file_handle_cache) {
file_ = fopen(scan_range_->file(), "r");
if (file_ == nullptr) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Could not open file: $0: $1", *scan_range_->file_string(),
GetStrErrMsg()));
}
@@ -68,7 +68,7 @@ Status LocalFileReader::ReadFromPos(int64_t file_offset, uint8_t* buffer,
if (fseek(file_, file_offset, SEEK_SET) == -1) {
fclose(file_);
file_ = nullptr;
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Could not seek to $0 "
"for file: $1: $2", scan_range_->offset(),
*scan_range_->file_string(), GetStrErrMsg()));
@@ -78,7 +78,7 @@ Status LocalFileReader::ReadFromPos(int64_t file_offset, uint8_t* buffer,
DCHECK_LE(*bytes_read, bytes_to_read);
if (*bytes_read < bytes_to_read) {
if (ferror(file_) != 0) {
- return Status(TErrorCode::DISK_IO_ERROR,
+ return Status(TErrorCode::DISK_IO_ERROR, GetBackendString(),
Substitute("Error reading from $0"
"at byte offset: $1: $2", file_,
file_offset, GetStrErrMsg()));
diff --git a/common/thrift/generate_error_codes.py b/common/thrift/generate_error_codes.py
index 93fed7d..10f7d4d 100755
--- a/common/thrift/generate_error_codes.py
+++ b/common/thrift/generate_error_codes.py
@@ -335,7 +335,7 @@ error_codes = (
("THREAD_CREATION_FAILED", 109, "Failed to create thread $0 in category $1: $2"),
- ("DISK_IO_ERROR", 110, "Disk I/O error: $0"),
+ ("DISK_IO_ERROR", 110, "Disk I/O error on $0: $1"),
("DATASTREAM_RECVR_CLOSED", 111,
"DataStreamRecvr for fragment=$0, node=$1 is closed already"),
[impala] 03/06: IMPALA-7917 (Part 1): Decouple Sentry from Impala
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 290e48f5d703202d927da5ae188e1bb5d79e7bfe
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Fri Nov 30 00:24:08 2018 -0600
IMPALA-7917 (Part 1): Decouple Sentry from Impala
The first part of this patch is to provide an initial work to decouple
Sentry from Impala by creating a generic authorization provider
interface that Sentry implements. The idea is to allow more
authorization providers in the future. The patch updates the following:
- Renamed Authorizeable to Authorizable to fix typographical error.
- Moved any clases that uses Sentry specific code to
org.apache.impala.authorization.sentry package and created interfaces
when necessary.
- Moved all generic authorization related classes to
org.apache.impala.authorization package.
- Minor clean up on authorization related code.
In this patch, switching the authorization provider implementation
still requires updating the code in many different places. A follow up
patch will make it easy to switch an authorization provider
implementation.
This patch has no functionality change.
Testing:
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: If1fd1df0b38ddd7cfa41299e95f5827f8a9e9c1f
Reviewed-on: http://gerrit.cloudera.org:8080/12020
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../org/apache/impala/analysis/AdminFnStmt.java | 2 +-
.../analysis/AlterTableSetTblProperties.java | 2 +-
.../apache/impala/analysis/AnalysisContext.java | 25 ++-
.../java/org/apache/impala/analysis/Analyzer.java | 23 ++-
.../apache/impala/analysis/AuthorizationStmt.java | 12 +-
.../apache/impala/analysis/CollectionTableRef.java | 2 +-
.../apache/impala/analysis/CopyTestCaseStmt.java | 4 +-
.../impala/analysis/CreateFunctionStmtBase.java | 8 +-
.../apache/impala/analysis/CreateTableStmt.java | 2 +-
.../apache/impala/analysis/DescribeTableStmt.java | 6 +-
.../apache/impala/analysis/DropFunctionStmt.java | 8 +-
.../java/org/apache/impala/analysis/HdfsUri.java | 7 +-
.../org/apache/impala/analysis/InsertStmt.java | 2 +-
.../apache/impala/analysis/ResetMetadataStmt.java | 11 +-
.../{Authorizeable.java => Authorizable.java} | 37 +++--
.../impala/authorization/AuthorizationChecker.java | 175 ++++++---------------
.../impala/authorization/AuthorizationConfig.java | 123 +--------------
.../AuthorizationException.java | 3 +-
.../AuthorizationPolicy.java | 109 +++----------
.../AuthorizationProvider.java} | 16 +-
.../org/apache/impala/authorization/Privilege.java | 103 ++++--------
.../impala/authorization/PrivilegeRequest.java | 42 ++---
.../authorization/PrivilegeRequestBuilder.java | 77 ++++++---
.../impala/authorization/sentry/ImpalaAction.java | 87 ++++++++++
.../{ => sentry}/ImpalaActionFactory.java | 4 +-
.../{ => sentry}/ImpalaPrivilegeModel.java | 3 +-
.../{ => sentry}/SentryAuthProvider.java | 25 +--
.../authorization/sentry/SentryAuthorizable.java | 59 +++++++
.../SentryAuthorizableColumn.java} | 49 ++++--
.../SentryAuthorizableDb.java} | 28 ++--
.../SentryAuthorizableFn.java} | 33 ++--
.../SentryAuthorizableServer.java} | 34 ++--
.../SentryAuthorizableTable.java} | 44 +++---
.../SentryAuthorizableUri.java} | 32 ++--
.../sentry/SentryAuthorizationChecker.java | 127 +++++++++++++++
.../SentryAuthorizationConfig.java} | 48 ++++--
.../sentry/SentryAuthorizationPolicy.java | 102 ++++++++++++
.../authorization/{ => sentry}/SentryConfig.java | 2 +-
.../sentry}/SentryPolicyReaderException.java | 2 +-
.../sentry}/SentryPolicyService.java | 8 +-
.../sentry}/SentryProxy.java | 11 +-
.../sentry}/SentryUnavailableException.java | 2 +-
.../{util => authorization/sentry}/SentryUtil.java | 21 ++-
.../java/org/apache/impala/catalog/Catalog.java | 4 +-
.../impala/catalog/CatalogServiceCatalog.java | 40 +++--
.../java/org/apache/impala/catalog/FeCatalog.java | 1 +
.../org/apache/impala/catalog/ImpaladCatalog.java | 1 +
.../apache/impala/catalog/PrincipalPrivilege.java | 1 +
.../impala/catalog/local/CatalogdMetaProvider.java | 5 +-
.../impala/catalog/local/DirectMetaProvider.java | 2 +-
.../apache/impala/catalog/local/LocalCatalog.java | 2 +-
.../apache/impala/catalog/local/MetaProvider.java | 2 +-
.../apache/impala/service/CatalogOpExecutor.java | 14 +-
.../java/org/apache/impala/service/Frontend.java | 58 ++++---
.../java/org/apache/impala/service/JniCatalog.java | 10 +-
.../org/apache/impala/service/JniFrontend.java | 4 +-
.../impala/analysis/AnalyzeAuthStmtsTest.java | 8 +-
.../org/apache/impala/analysis/AuditingTest.java | 11 +-
.../impala/analysis/AuthorizationStmtTest.java | 8 +-
.../apache/impala/analysis/AuthorizationTest.java | 101 ++++++------
.../impala/analysis/StmtMetadataLoaderTest.java | 8 +-
.../{ => sentry}/ImpalaActionFactoryTest.java | 5 +-
.../sentry}/SentryProxyTest.java | 10 +-
.../org/apache/impala/catalog/CatalogTest.java | 1 +
.../events/MetastoreEventsProcessorTest.java | 7 +-
.../org/apache/impala/common/FrontendFixture.java | 7 +-
.../org/apache/impala/common/FrontendTestBase.java | 9 +-
.../org/apache/impala/common/QueryFixture.java | 4 +-
.../impala/testutil/CatalogServiceTestCatalog.java | 17 +-
.../apache/impala/testutil/ImpaladTestCatalog.java | 5 +-
.../impala/testutil/PlannerTestCaseLoader.java | 5 +-
.../impala/testutil/SentryServicePinger.java | 4 +-
fe/src/test/resources/authz-policy.ini.template | 2 +-
73 files changed, 1044 insertions(+), 832 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java b/fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
index 5eb900d..fd7e4f9 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
@@ -101,7 +101,7 @@ public class AdminFnStmt extends StatementBase {
String authzServer = analyzer.getAuthzConfig().getServerName();
Preconditions.checkNotNull(authzServer);
analyzer.registerPrivReq(
- new PrivilegeRequestBuilder().onServer(authzServer).all().toRequest());
+ new PrivilegeRequestBuilder().onServer(authzServer).all().build());
}
// TODO: this parsing and type checking logic is specific to the command, similar to
diff --git a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
index 5528316..12819a8 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
@@ -133,7 +133,7 @@ public class AlterTableSetTblProperties extends AlterTableSetStmt {
String authzServer = analyzer.getAuthzConfig().getServerName();
Preconditions.checkNotNull(authzServer);
analyzer.registerPrivReq(new PrivilegeRequestBuilder().onServer(
- authzServer).all().toRequest());
+ authzServer).all().build());
}
}
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
index ecd7ffd..cdbe791 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
@@ -26,13 +26,12 @@ import java.util.Map;
import java.util.Set;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
+import org.apache.impala.authorization.Authorizable;
import org.apache.impala.authorization.AuthorizationChecker;
import org.apache.impala.authorization.AuthorizationConfig;
-import org.apache.impala.authorization.AuthorizeableColumn;
-import org.apache.impala.authorization.AuthorizeableTable;
import org.apache.impala.authorization.Privilege;
import org.apache.impala.authorization.PrivilegeRequest;
-import org.apache.impala.catalog.AuthorizationException;
+import org.apache.impala.authorization.AuthorizationException;
import org.apache.impala.catalog.FeCatalog;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.Type;
@@ -536,7 +535,7 @@ public class AnalysisContext {
List<PrivilegeRequest> otherPrivReqs = new ArrayList<>();
// Group the registered privilege requests based on the table they reference.
for (PrivilegeRequest privReq: analyzer.getPrivilegeReqs()) {
- String tableName = privReq.getAuthorizeable().getFullTableName();
+ String tableName = privReq.getAuthorizable().getFullTableName();
if (tableName == null) {
otherPrivReqs.add(privReq);
} else {
@@ -548,8 +547,8 @@ public class AnalysisContext {
// The table-level SELECT must be the first table-level request, and it
// must precede all column-level privilege requests.
Preconditions.checkState((requests.isEmpty() ||
- !(privReq.getAuthorizeable() instanceof AuthorizeableColumn)) ||
- (requests.get(0).getAuthorizeable() instanceof AuthorizeableTable &&
+ !(privReq.getAuthorizable().getType() == Authorizable.Type.COLUMN)) ||
+ (requests.get(0).getAuthorizable().getType() == Authorizable.Type.TABLE &&
requests.get(0).getPrivilege() == Privilege.SELECT));
requests.add(privReq);
}
@@ -568,7 +567,7 @@ public class AnalysisContext {
} else {
for (PrivilegeRequest privReq: analyzer.getPrivilegeReqs()) {
Preconditions.checkState(
- !(privReq.getAuthorizeable() instanceof AuthorizeableColumn) ||
+ !(privReq.getAuthorizable().getType() == Authorizable.Type.COLUMN) ||
analysisResult_.isSingleColumnPrivStmt());
authorizePrivilegeRequest(authzChecker, privReq);
}
@@ -601,8 +600,8 @@ public class AnalysisContext {
PrivilegeRequest request) throws AuthorizationException, InternalException {
Preconditions.checkNotNull(request);
String dbName = null;
- if (request.getAuthorizeable() != null) {
- dbName = request.getAuthorizeable().getDbName();
+ if (request.getAuthorizable() != null) {
+ dbName = request.getAuthorizable().getDbName();
}
// If this is a system database, some actions should always be allowed
// or disabled, regardless of what is in the auth policy.
@@ -628,7 +627,7 @@ public class AnalysisContext {
boolean hasTableSelectPriv = true;
boolean hasColumnSelectPriv = false;
for (PrivilegeRequest request: requests) {
- if (request.getAuthorizeable() instanceof AuthorizeableTable) {
+ if (request.getAuthorizable().getType() == Authorizable.Type.TABLE) {
try {
authorizePrivilegeRequest(authzChecker, request);
} catch (AuthorizationException e) {
@@ -639,7 +638,7 @@ public class AnalysisContext {
}
} else {
Preconditions.checkState(
- request.getAuthorizeable() instanceof AuthorizeableColumn);
+ request.getAuthorizable().getType() == Authorizable.Type.COLUMN);
if (hasTableSelectPriv) continue;
if (authzChecker.hasAccess(analyzer.getUser(), request)) {
hasColumnSelectPriv = true;
@@ -649,13 +648,13 @@ public class AnalysisContext {
throw new AuthorizationException(String.format("User '%s' does not have " +
"privileges to execute '%s' on: %s", analyzer.getUser().getName(),
request.getPrivilege().toString(),
- request.getAuthorizeable().getFullTableName()));
+ request.getAuthorizable().getFullTableName()));
}
}
if (!hasTableSelectPriv && !hasColumnSelectPriv) {
throw new AuthorizationException(String.format("User '%s' does not have " +
"privileges to execute 'SELECT' on: %s", analyzer.getUser().getName(),
- requests.get(0).getAuthorizeable().getFullTableName()));
+ requests.get(0).getAuthorizable().getFullTableName()));
}
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
index 91bc75e..c1128ef 100644
--- a/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
+++ b/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
@@ -34,7 +34,6 @@ import java.util.Set;
import org.apache.impala.analysis.Path.PathType;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
import org.apache.impala.authorization.AuthorizationConfig;
-import org.apache.impala.authorization.AuthorizeableTable;
import org.apache.impala.authorization.Privilege;
import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.PrivilegeRequestBuilder;
@@ -573,7 +572,7 @@ public class Analyzer {
if (tableRef.requireGrantOption()) {
builder.grantOption();
}
- registerPrivReq(builder.toRequest());
+ registerPrivReq(builder.build());
}
PrivilegeRequestBuilder builder = new PrivilegeRequestBuilder()
.onTable(getDefaultDb(), rawPath.get(0))
@@ -581,7 +580,7 @@ public class Analyzer {
if (tableRef.requireGrantOption()) {
builder.grantOption();
}
- registerPrivReq(builder.toRequest());
+ registerPrivReq(builder.build());
throw e;
} catch (TableLoadingException e) {
throw new AnalysisException(String.format(
@@ -971,7 +970,7 @@ public class Analyzer {
if (column != null) {
registerPrivReq(new PrivilegeRequestBuilder().
allOf(Privilege.SELECT).onColumn(tupleDesc.getTableName().getDb(),
- tupleDesc.getTableName().getTbl(), column.getName()).toRequest());
+ tupleDesc.getTableName().getTbl(), column.getName()).build());
}
}
}
@@ -2430,10 +2429,10 @@ public class Analyzer {
for (Privilege priv : privilege) {
if (priv == Privilege.ANY || addColumnPrivilege) {
registerPrivReq(new PrivilegeRequestBuilder()
- .allOf(priv).onAnyColumn(tableName.getDb(), tableName.getTbl()).toRequest());
+ .allOf(priv).onAnyColumn(tableName.getDb(), tableName.getTbl()).build());
} else {
registerPrivReq(new PrivilegeRequestBuilder()
- .allOf(priv).onTable(tableName.getDb(), tableName.getTbl()).toRequest());
+ .allOf(priv).onTable(tableName.getDb(), tableName.getTbl()).build());
}
}
FeTable table = getTable(tableName.getDb(), tableName.getTbl());
@@ -2515,10 +2514,9 @@ public class Analyzer {
pb.grantOption();
}
if (privilege == Privilege.ANY) {
- registerPrivReq(
- pb.any().onAnyColumn(dbName, AuthorizeableTable.ANY_TABLE_NAME).toRequest());
+ registerPrivReq(pb.any().onAnyColumn(dbName).build());
} else {
- registerPrivReq(pb.allOf(privilege).onDb(dbName).toRequest());
+ registerPrivReq(pb.allOf(privilege).onDb(dbName).build());
}
FeDb db = getDb(dbName, throwIfDoesNotExist);
@@ -2550,7 +2548,7 @@ public class Analyzer {
public boolean dbContainsTable(String dbName, String tableName, Privilege privilege)
throws AnalysisException {
registerPrivReq(new PrivilegeRequestBuilder().allOf(privilege)
- .onTable(dbName, tableName).toRequest());
+ .onTable(dbName, tableName).build());
try {
FeDb db = getCatalog().getDb(dbName);
if (db == null) {
@@ -2636,8 +2634,7 @@ public class Analyzer {
public void registerPrivReq(PrivilegeRequest privReq) {
if (!enablePrivChecks_) return;
if (maskPrivChecks_) {
- globalState_.maskedPrivilegeReqs.add(
- Pair.<PrivilegeRequest, String>create(privReq, authErrorMsg_));
+ globalState_.maskedPrivilegeReqs.add(Pair.create(privReq, authErrorMsg_));
} else {
globalState_.privilegeReqs.add(privReq);
}
@@ -2678,7 +2675,7 @@ public class Analyzer {
if (requireGrantOption) {
builder.grantOption();
}
- registerPrivReq(builder.toRequest());
+ registerPrivReq(builder.build());
}
/**
diff --git a/fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java b/fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
index c0186b3..244ff43 100644
--- a/fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/AuthorizationStmt.java
@@ -17,7 +17,9 @@
package org.apache.impala.analysis;
+import org.apache.impala.authorization.AuthorizationProvider;
import org.apache.impala.authorization.User;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.common.AnalysisException;
import com.google.common.base.Strings;
@@ -36,10 +38,12 @@ public class AuthorizationStmt extends StatementBase {
throw new AnalysisException("Authorization is not enabled. To enable " +
"authorization restart Impala with the --server_name=<name> flag.");
}
- if (analyzer.getAuthzConfig().isFileBasedPolicy()) {
- throw new AnalysisException("Cannot execute authorization statement using a file" +
- " based policy. To disable file based policies, restart Impala without the " +
- "-authorization_policy_file flag set.");
+ if (analyzer.getAuthzConfig().getProvider() == AuthorizationProvider.SENTRY) {
+ if (((SentryAuthorizationConfig) analyzer.getAuthzConfig()).isFileBasedPolicy()) {
+ throw new AnalysisException("Cannot execute authorization statement using a " +
+ "file based policy. To disable file based policies, restart Impala without " +
+ "the -authorization_policy_file flag set.");
+ }
}
if (Strings.isNullOrEmpty(analyzer.getUser().getName())) {
throw new AnalysisException("Cannot execute authorization statement with an " +
diff --git a/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java b/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
index 87f38b7..2496c20 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
@@ -108,7 +108,7 @@ public class CollectionTableRef extends TableRef {
analyzer.registerPrivReq(new PrivilegeRequestBuilder().
allOf(Privilege.SELECT).onColumn(desc_.getTableName().getDb(),
desc_.getTableName().getTbl(), desc_.getPath().getRawPath().get(0))
- .toRequest());
+ .build());
}
isAnalyzed_ = true;
analyzeTableSample(analyzer);
diff --git a/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java b/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
index c503cc5..996a9db 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
@@ -120,11 +120,11 @@ public class CopyTestCaseStmt extends StatementBase {
Pair<Set<FeDb>, Set<FeTable>> referencedObjects = getReferencedCatalogObjects();
for (FeDb db: referencedObjects.first) {
analyzer.registerPrivReq(new PrivilegeRequestBuilder().onDb(db.getName()).allOf
- (Privilege.VIEW_METADATA).toRequest());
+ (Privilege.VIEW_METADATA).build());
}
for (FeTable table: referencedObjects.second) {
analyzer.registerPrivReq(new PrivilegeRequestBuilder().onTable(table.getDb()
- .getName(), table.getName()).allOf(Privilege.VIEW_METADATA).toRequest());
+ .getName(), table.getName()).allOf(Privilege.VIEW_METADATA).build());
}
} else {
hdfsPath_.analyze(analyzer, Privilege.ALL, FsAction.READ, /*registerPrivReq*/ true,
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java b/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
index 5cbdfc6..2620c0a 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateFunctionStmtBase.java
@@ -21,9 +21,8 @@ import java.util.List;
import java.util.Map;
import org.apache.hadoop.fs.permission.FsAction;
-import org.apache.impala.authorization.AuthorizeableFn;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.authorization.PrivilegeRequest;
+import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Type;
@@ -144,8 +143,9 @@ public abstract class CreateFunctionStmtBase extends StatementBase {
fn_ = createFunction(fnName_, null, null, false);
}
- analyzer.registerPrivReq(new PrivilegeRequest(
- new AuthorizeableFn(fn_.dbName(), fn_.signatureString()), Privilege.CREATE));
+ analyzer.registerPrivReq(new PrivilegeRequestBuilder()
+ .onFunction(fn_.dbName(), fn_.signatureString()).allOf(Privilege.CREATE)
+ .build());
db_ = analyzer.getDb(fn_.dbName(), true);
Function existingFn = db_.getFunction(fn_, Function.CompareMode.IS_INDISTINGUISHABLE);
diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
index 8b393f1..06eb125 100644
--- a/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
@@ -256,7 +256,7 @@ public class CreateTableStmt extends StatementBase {
String authzServer = analyzer.getAuthzConfig().getServerName();
Preconditions.checkNotNull(authzServer);
analyzer.registerPrivReq(new PrivilegeRequestBuilder().onServer(
- authzServer).all().toRequest());
+ authzServer).all().build());
}
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java b/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
index 1e7c645..1622656 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
@@ -100,10 +100,10 @@ public class DescribeTableStmt extends StatementBase {
// table/database if the user is not authorized.
if (rawPath_.size() > 1) {
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
- .onTable(rawPath_.get(0), rawPath_.get(1)).any().toRequest());
+ .onTable(rawPath_.get(0), rawPath_.get(1)).any().build());
}
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
- .onTable(analyzer.getDefaultDb(), rawPath_.get(0)).any().toRequest());
+ .onTable(analyzer.getDefaultDb(), rawPath_.get(0)).any().build());
throw ae;
} catch (TableLoadingException tle) {
throw new AnalysisException(tle.getMessage(), tle);
@@ -120,7 +120,7 @@ public class DescribeTableStmt extends StatementBase {
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
.onColumn(path_.getRootTable().getDb().getName(), path_.getRootTable().getName(),
- path_.getRawPath().get(0)).any().toRequest());
+ path_.getRawPath().get(0)).any().build());
if (path_.destType().isComplexType()) {
if (outputStyle_ == TDescribeOutputStyle.FORMATTED ||
diff --git a/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java b/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
index 97111f8..ce3a614 100644
--- a/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
@@ -19,9 +19,8 @@ package org.apache.impala.analysis;
import java.util.ArrayList;
-import org.apache.impala.authorization.AuthorizeableFn;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.authorization.PrivilegeRequest;
+import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.Type;
@@ -85,8 +84,9 @@ public class DropFunctionStmt extends StatementBase {
false);
}
- analyzer.registerPrivReq(new PrivilegeRequest(
- new AuthorizeableFn(desc_.dbName(), desc_.signatureString()), Privilege.DROP));
+ analyzer.registerPrivReq(new PrivilegeRequestBuilder()
+ .onFunction(desc_.dbName(), desc_.signatureString()).allOf(Privilege.DROP)
+ .build());
FeDb db = analyzer.getDb(desc_.dbName(), false);
if (db == null && !ifExists_) {
diff --git a/fe/src/main/java/org/apache/impala/analysis/HdfsUri.java b/fe/src/main/java/org/apache/impala/analysis/HdfsUri.java
index d74545c..89a3a96 100644
--- a/fe/src/main/java/org/apache/impala/analysis/HdfsUri.java
+++ b/fe/src/main/java/org/apache/impala/analysis/HdfsUri.java
@@ -23,9 +23,8 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.fs.permission.FsAction;
-import org.apache.impala.authorization.AuthorizeableUri;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.authorization.PrivilegeRequest;
+import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.FileSystemUtil;
import org.apache.impala.util.FsPermissionChecker;
@@ -121,8 +120,8 @@ public class HdfsUri {
}
if (registerPrivReq) {
- analyzer.registerPrivReq(new PrivilegeRequest(
- new AuthorizeableUri(uriPath_.toString()), privilege));
+ analyzer.registerPrivReq(new PrivilegeRequestBuilder()
+ .onUri(uriPath_.toString()).allOf(privilege).build());
}
}
diff --git a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
index 72c29b3..61d4e64 100644
--- a/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
@@ -422,7 +422,7 @@ public class InsertStmt extends StatementBase {
targetTableName_ = new TableName(table_.getDb().getName(), table_.getName());
PrivilegeRequestBuilder pb = new PrivilegeRequestBuilder();
analyzer.registerPrivReq(pb.onTable(table_.getDb().getName(), table_.getName())
- .allOf(privilegeRequired).toRequest());
+ .allOf(privilegeRequired).build());
}
// We do not support (in|up)serting into views.
diff --git a/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java b/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
index bf47067..b5aacec 100644
--- a/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
@@ -21,7 +21,6 @@ import java.util.List;
import com.google.common.annotations.VisibleForTesting;
import org.apache.impala.authorization.Privilege;
-import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.service.BackendConfig;
@@ -152,7 +151,7 @@ public class ResetMetadataStmt extends StatementBase {
// Verify the user has privileges to access this table.
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
.onTable(dbName, tableName_.getTbl()).allOf(Privilege.REFRESH)
- .toRequest());
+ .build());
}
break;
case REFRESH_AUTHORIZATION:
@@ -160,14 +159,16 @@ public class ResetMetadataStmt extends StatementBase {
throw new AnalysisException("Authorization is not enabled. To enable " +
"authorization restart Impala with the --server_name=<name> flag.");
}
- analyzer.registerPrivReq(new PrivilegeRequest(Privilege.REFRESH));
+ analyzer.registerPrivReq(new PrivilegeRequestBuilder()
+ .onServer(analyzer.getServerName()).allOf(Privilege.REFRESH).build());
break;
case REFRESH_FUNCTIONS:
analyzer.registerPrivReq(new PrivilegeRequestBuilder()
- .onDb(database_).allOf(Privilege.REFRESH).toRequest());
+ .onDb(database_).allOf(Privilege.REFRESH).build());
break;
case INVALIDATE_METADATA_ALL:
- analyzer.registerPrivReq(new PrivilegeRequest(Privilege.REFRESH));
+ analyzer.registerPrivReq(new PrivilegeRequestBuilder()
+ .onServer(analyzer.getServerName()).allOf(Privilege.REFRESH).build());
if (BackendConfig.INSTANCE.getBackendCfg().use_local_catalog) {
throw new AnalysisException("Global INVALIDATE METADATA is not supported " +
"when --use_local_catalog is configured.");
diff --git a/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java b/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
similarity index 67%
rename from fe/src/main/java/org/apache/impala/authorization/Authorizeable.java
rename to fe/src/main/java/org/apache/impala/authorization/Authorizable.java
index 7e30332..1e35ef6 100644
--- a/fe/src/main/java/org/apache/impala/authorization/Authorizeable.java
+++ b/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
@@ -19,31 +19,40 @@ package org.apache.impala.authorization;
import java.util.List;
-import org.apache.sentry.core.model.db.DBModelAuthorizable;
-
/*
- * Abstract class representing an authorizeable object (Table, Db, Column, etc).
+ * Abstract class representing an authorizable object (Table, Db, Column, etc).
*/
-public abstract class Authorizeable {
- /*
- * Returns the list of the Hive "authorizeable" objects in their hierarchical order.
- * For example:
- * [Column] would return Db -> Table -> Column
- * [Table] would return Db -> Table
- * [Db] would return [Db]
- * [URI] would return [URI]
- */
- public abstract List<DBModelAuthorizable> getHiveAuthorizeableHierarchy();
+public abstract class Authorizable {
+ public enum Type {
+ SERVER,
+ DB,
+ TABLE,
+ COLUMN,
+ FUNCTION,
+ URI
+ }
// Returns the name of the object.
public abstract String getName();
+ // Returns the type of the authorizable.
+ public abstract Type getType();
+
// Returns the full table name if applicable, null otherwise.
public String getFullTableName() { return null; }
+ // Returns the table name if applicable, null otherwise.
+ public String getTableName() { return null; }
+
// Returns the database name if applicable, null otherwise.
public String getDbName() { return null; }
+ // Returns the column name if applicable, null otherwise.
+ public String getColumnName() { return null; }
+
+ // Returns the function name if applicable, null otherwise.
+ public String getFnName() { return null; }
+
@Override
public int hashCode() { return getName().hashCode(); }
@@ -51,6 +60,6 @@ public abstract class Authorizeable {
public boolean equals(Object o) {
if (o == null) return false;
if (o.getClass() != this.getClass()) return false;
- return ((Authorizeable) o).getName().equals(this.getName());
+ return ((Authorizable) o).getName().equals(this.getName());
}
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
index a6e6d5d..993d844 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
@@ -17,55 +17,26 @@
package org.apache.impala.authorization;
-import java.util.Collections;
import java.util.EnumSet;
-import java.util.List;
import java.util.Set;
-import org.apache.impala.authorization.Privilege.ImpalaAction;
-import org.apache.impala.catalog.AuthorizationException;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.Authorizable.Type;
import org.apache.impala.common.InternalException;
-import org.apache.sentry.core.common.ActiveRoleSet;
-import org.apache.sentry.core.common.Subject;
-import org.apache.sentry.core.model.db.DBModelAuthorizable;
-import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
-
-import org.apache.impala.util.SentryUtil;
/**
- * Class used to check whether a user has access to a given resource.
+ * A base class used to check whether a user has access to a given resource.
*/
-public class AuthorizationChecker {
- private final ResourceAuthorizationProvider provider_;
- private final AuthorizationConfig config_;
- private final AuthorizeableServer server_;
+public abstract class AuthorizationChecker {
+ protected final AuthorizationConfig config_;
/*
* Creates a new AuthorizationChecker based on the config values.
*/
- public AuthorizationChecker(AuthorizationConfig config, AuthorizationPolicy policy) {
+ public AuthorizationChecker(AuthorizationConfig config) {
Preconditions.checkNotNull(config);
config_ = config;
- if (config.isEnabled()) {
- server_ = new AuthorizeableServer(config.getServerName());
- provider_ = createProvider(config, policy);
- Preconditions.checkNotNull(provider_);
- } else {
- provider_ = null;
- server_ = null;
- }
- }
-
- /*
- * Creates a new ResourceAuthorizationProvider based on the given configuration.
- */
- private static ResourceAuthorizationProvider createProvider(AuthorizationConfig config,
- AuthorizationPolicy policy) {
- return SentryAuthProvider.createProvider(config, policy);
}
/*
@@ -74,24 +45,6 @@ public class AuthorizationChecker {
public AuthorizationConfig getConfig() { return config_; }
/**
- * Returns the set of groups this user belongs to. Uses the GroupMappingService
- * that is in the AuthorizationProvider to properly resolve Hadoop groups or
- * local group mappings.
- */
- public Set<String> getUserGroups(User user) throws InternalException {
- try {
- return provider_.getGroupMapping().getGroups(user.getShortName());
- } catch (Exception e) {
- if (SentryUtil.isSentryGroupNotFound(e)) {
- // Sentry 2.1+ throws exceptions when user does not exist; swallow the
- // exception and just return an empty set for this case.
- return Collections.emptySet();
- }
- throw new RuntimeException(e);
- }
- }
-
- /**
* Authorizes the PrivilegeRequest, throwing an Authorization exception if
* the user does not have sufficient privileges.
*/
@@ -99,42 +52,40 @@ public class AuthorizationChecker {
throws AuthorizationException, InternalException {
Preconditions.checkNotNull(privilegeRequest);
- if (!hasAccess(user, privilegeRequest)) {
- Privilege privilege = privilegeRequest.getPrivilege();
- if (privilegeRequest.getAuthorizeable() instanceof AuthorizeableFn) {
- throw new AuthorizationException(String.format(
- "User '%s' does not have privileges%s to %s functions in: %s",
- user.getName(), grantOption(privilegeRequest.hasGrantOption()), privilege,
- privilegeRequest.getName()));
- }
+ if (hasAccess(user, privilegeRequest)) return;
+
+ Privilege privilege = privilegeRequest.getPrivilege();
+ if (privilegeRequest.getAuthorizable().getType() == Type.FUNCTION) {
+ throw new AuthorizationException(String.format(
+ "User '%s' does not have privileges%s to %s functions in: %s",
+ user.getName(), grantOption(privilegeRequest.hasGrantOption()), privilege,
+ privilegeRequest.getName()));
+ }
- if (EnumSet.of(Privilege.ANY, Privilege.ALL, Privilege.VIEW_METADATA)
- .contains(privilege)) {
- throw new AuthorizationException(String.format(
- "User '%s' does not have privileges%s to access: %s",
- user.getName(), grantOption(privilegeRequest.hasGrantOption()),
- privilegeRequest.getName()));
- } else if (privilege == Privilege.REFRESH) {
- throw new AuthorizationException(String.format(
- "User '%s' does not have privileges%s to execute " +
- "'INVALIDATE METADATA/REFRESH' on: %s", user.getName(),
- grantOption(privilegeRequest.hasGrantOption()), privilegeRequest.getName()));
- } else if (privilege == Privilege.CREATE &&
- privilegeRequest.getAuthorizeable() instanceof AuthorizeableTable) {
- // Creating a table requires CREATE on a database and we shouldn't
- // expose the table name.
- AuthorizeableTable authorizeableTable =
- (AuthorizeableTable) privilegeRequest.getAuthorizeable();
- throw new AuthorizationException(String.format(
- "User '%s' does not have privileges%s to execute '%s' on: %s",
- user.getName(), grantOption(privilegeRequest.hasGrantOption()), privilege,
- authorizeableTable.getDbName()));
- } else {
- throw new AuthorizationException(String.format(
- "User '%s' does not have privileges%s to execute '%s' on: %s",
- user.getName(), grantOption(privilegeRequest.hasGrantOption()), privilege,
- privilegeRequest.getName()));
- }
+ if (EnumSet.of(Privilege.ANY, Privilege.ALL, Privilege.VIEW_METADATA)
+ .contains(privilege)) {
+ throw new AuthorizationException(String.format(
+ "User '%s' does not have privileges%s to access: %s",
+ user.getName(), grantOption(privilegeRequest.hasGrantOption()),
+ privilegeRequest.getName()));
+ } else if (privilege == Privilege.REFRESH) {
+ throw new AuthorizationException(String.format(
+ "User '%s' does not have privileges%s to execute " +
+ "'INVALIDATE METADATA/REFRESH' on: %s", user.getName(),
+ grantOption(privilegeRequest.hasGrantOption()), privilegeRequest.getName()));
+ } else if (privilege == Privilege.CREATE &&
+ privilegeRequest.getAuthorizable().getType() == Type.TABLE) {
+ // Creating a table requires CREATE on a database and we shouldn't
+ // expose the table name.
+ throw new AuthorizationException(String.format(
+ "User '%s' does not have privileges%s to execute '%s' on: %s",
+ user.getName(), grantOption(privilegeRequest.hasGrantOption()), privilege,
+ privilegeRequest.getAuthorizable().getDbName()));
+ } else {
+ throw new AuthorizationException(String.format(
+ "User '%s' does not have privileges%s to execute '%s' on: %s",
+ user.getName(), grantOption(privilegeRequest.hasGrantOption()), privilege,
+ privilegeRequest.getName()));
}
}
@@ -142,51 +93,15 @@ public class AuthorizationChecker {
return hasGrantOption ? " with 'GRANT OPTION'" : "";
}
+ /**
+ * Returns the set of groups this user belongs to.
+ */
+ public abstract Set<String> getUserGroups(User user) throws InternalException;
+
/*
* Returns true if the given user has permission to execute the given
* request, false otherwise. Always returns true if authorization is disabled.
*/
- public boolean hasAccess(User user, PrivilegeRequest request)
- throws InternalException {
- Preconditions.checkNotNull(user);
- Preconditions.checkNotNull(request);
-
- // If authorization is not enabled the user will always have access. If this is
- // an internal request, the user will always have permission.
- if (!config_.isEnabled() || user instanceof ImpalaInternalAdminUser) {
- return true;
- }
-
- EnumSet<ImpalaAction> actions = request.getPrivilege().getSentryActions();
-
- List<DBModelAuthorizable> authorizeables = Lists.newArrayList(
- server_.getHiveAuthorizeableHierarchy());
- // If request.getAuthorizeable() is null, the request is for server-level permission.
- if (request.getAuthorizeable() != null) {
- authorizeables.addAll(request.getAuthorizeable().getHiveAuthorizeableHierarchy());
- }
-
- // The Hive Access API does not currently provide a way to check if the user
- // has any privileges on a given resource.
- if (request.getPrivilege().getAnyOf()) {
- for (ImpalaAction action: actions) {
- if (provider_.hasAccess(new Subject(user.getShortName()), authorizeables,
- EnumSet.of(action), request.hasGrantOption(), ActiveRoleSet.ALL)) {
- return true;
- }
- }
- return false;
- // AuthorizeableFn is special due to Sentry not having the concept of a function in
- // DBModelAuthorizable.AuthorizableType. As a result, the list of authorizables for
- // an AuthorizeableFn only contains the server and database, but not the function
- // itself. So there is no need to remove the last authorizeable here.
- } else if (request.getPrivilege() == Privilege.CREATE && authorizeables.size() > 1 &&
- !(request.getAuthorizeable() instanceof AuthorizeableFn)) {
- // CREATE on an object requires CREATE on the parent,
- // so don't check access on the object we're creating.
- authorizeables.remove(authorizeables.size() - 1);
- }
- return provider_.hasAccess(new Subject(user.getShortName()), authorizeables, actions,
- request.hasGrantOption(), ActiveRoleSet.ALL);
- }
+ public abstract boolean hasAccess(User user, PrivilegeRequest request)
+ throws InternalException;
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java
index 96b65d8..6ab651a 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java
@@ -17,133 +17,22 @@
package org.apache.impala.authorization;
-
-import org.apache.sentry.provider.common.HadoopGroupResourceAuthorizationProvider;
-import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
-
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-
/*
- * Class that contains configuration details for Impala authorization.
+ * Interface that contains configuration details for Impala authorization.
*/
-public class AuthorizationConfig {
- private final String serverName_;
- // Set only if the policy provider is file-based.
- private final String policyFile_;
- private final SentryConfig sentryConfig_;
- private final String policyProviderClassName_;
-
- /**
- * Creates a new authorization configuration object.
- * @param serverName - The name of this Impala server.
- * @param policyFile - The path to the authorization policy file or null if
- * the policy engine is not file based.
- * @param sentryConfigFile - Absolute path and file name of the sentry service.
- * @param policyProviderClassName - Class name of the policy provider to use.
- */
- public AuthorizationConfig(String serverName, String policyFile,
- String sentryConfigFile, String policyProviderClassName) {
- serverName_ = serverName;
- policyFile_ = policyFile;
- sentryConfig_ = new SentryConfig(sentryConfigFile);
- if (!Strings.isNullOrEmpty(policyProviderClassName)) {
- policyProviderClassName = policyProviderClassName.trim();
- }
- policyProviderClassName_ = policyProviderClassName;
- validateConfig();
- }
-
- /**
- * Returns an AuthorizationConfig object that has authorization disabled.
- */
- public static AuthorizationConfig createAuthDisabledConfig() {
- return new AuthorizationConfig(null, null, null, null);
- }
-
- /**
- * Returns an AuthorizationConfig object configured to use Hadoop user->group mappings
- * for the authorization provider.
- */
- public static AuthorizationConfig createHadoopGroupAuthConfig(String serverName,
- String policyFile, String sentryConfigFile) {
- return new AuthorizationConfig(serverName, policyFile, sentryConfigFile,
- HadoopGroupResourceAuthorizationProvider.class.getName());
- }
-
- /*
- * Validates the authorization configuration and throws an AuthorizationException
- * if any problems are found. If authorization is disabled, config checks are skipped.
- */
- private void validateConfig() throws IllegalArgumentException {
- // If authorization is not enabled, config checks are skipped.
- if (!isEnabled()) return;
-
- // Only load the sentry configuration if a sentry-site.xml configuration file was
- // specified. It is optional for impalad.
- if (!Strings.isNullOrEmpty(sentryConfig_.getConfigFile())) {
- sentryConfig_.loadConfig();
- }
-
- if (Strings.isNullOrEmpty(serverName_)) {
- throw new IllegalArgumentException(
- "Authorization is enabled but the server name is null or empty. Set the " +
- "server name using the impalad --server_name flag.");
- }
- if (Strings.isNullOrEmpty(policyProviderClassName_)) {
- throw new IllegalArgumentException("Authorization is enabled but the " +
- "authorization policy provider class name is null or empty. Set the class " +
- "name using the --authorization_policy_provider_class impalad flag.");
- }
-
- Class<?> providerClass = null;
- try {
- // Get the Class object without performing any initialization.
- providerClass = Class.forName(policyProviderClassName_, false,
- this.getClass().getClassLoader());
- } catch (ClassNotFoundException e) {
- throw new IllegalArgumentException(String.format("The authorization policy " +
- "provider class '%s' was not found.", policyProviderClassName_), e);
- }
- Preconditions.checkNotNull(providerClass);
- if (!ResourceAuthorizationProvider.class.isAssignableFrom(providerClass)) {
- throw new IllegalArgumentException(String.format("The authorization policy " +
- "provider class '%s' must be a subclass of '%s'.",
- policyProviderClassName_,
- ResourceAuthorizationProvider.class.getName()));
- }
- }
-
+public interface AuthorizationConfig {
/**
* Returns true if authorization is enabled.
- * If either serverName, policyFile, or sentryServiceConfig_ file is set (not null
- * or empty), authorization is considered enabled.
*/
- public boolean isEnabled() {
- return !Strings.isNullOrEmpty(serverName_) || !Strings.isNullOrEmpty(policyFile_) ||
- !Strings.isNullOrEmpty(sentryConfig_.getConfigFile());
- }
+ boolean isEnabled();
/**
- * Returns true if using an authorization policy from a file in HDFS. If false,
- * uses an authorization policy based on cached metadata sent from the catalog server
- * via the statestore.
+ * Returns the authorization provider type.
*/
- public boolean isFileBasedPolicy() { return !Strings.isNullOrEmpty(policyFile_); }
+ AuthorizationProvider getProvider();
/**
* The server name to secure.
*/
- public String getServerName() { return serverName_; }
-
- /**
- * The policy file path.
- */
- public String getPolicyFile() { return policyFile_; }
-
- /**
- * The Sentry configuration.
- */
- public SentryConfig getSentryConfig() { return sentryConfig_; }
- public String getPolicyProviderClassName() { return policyProviderClassName_; }
+ String getServerName();
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/AuthorizationException.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationException.java
similarity index 92%
copy from fe/src/main/java/org/apache/impala/catalog/AuthorizationException.java
copy to fe/src/main/java/org/apache/impala/authorization/AuthorizationException.java
index 909a75b..70ed2bd 100644
--- a/fe/src/main/java/org/apache/impala/catalog/AuthorizationException.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationException.java
@@ -15,8 +15,9 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.catalog;
+package org.apache.impala.authorization;
+import org.apache.impala.catalog.CatalogException;
/**
* Thrown for authorization errors encountered when accessing Catalog objects.
diff --git a/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java
similarity index 86%
rename from fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
rename to fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java
index 5e9b11d..e33f465 100644
--- a/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.catalog;
+package org.apache.impala.authorization;
import java.util.ArrayList;
import java.util.HashMap;
@@ -25,6 +25,13 @@ import java.util.Map;
import java.util.Set;
import org.apache.commons.net.ntp.TimeStamp;
+import org.apache.impala.catalog.CatalogException;
+import org.apache.impala.catalog.CatalogObjectCache;
+import org.apache.impala.catalog.CatalogObjectVersionSet;
+import org.apache.impala.catalog.Principal;
+import org.apache.impala.catalog.PrincipalPrivilege;
+import org.apache.impala.catalog.Role;
+import org.apache.impala.catalog.Type;
import org.apache.impala.common.AnalysisException;
import org.apache.impala.common.InternalException;
import org.apache.impala.service.Frontend;
@@ -36,8 +43,6 @@ import org.apache.impala.thrift.TResultSet;
import org.apache.impala.thrift.TResultSetMetadata;
import org.apache.impala.util.TResultRowBuilder;
import org.apache.log4j.Logger;
-import org.apache.sentry.core.common.ActiveRoleSet;
-import org.apache.sentry.provider.cache.PrivilegeCache;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
@@ -46,13 +51,12 @@ import com.google.common.collect.Sets;
/**
* A thread safe authorization policy cache, consisting of roles, groups that are
* members of that role, the privileges associated with the role, and users and the
- * privileges associated with the user. The source data this cache is backing is read from
- * the Sentry Policy Service. Writing to the cache will replace any matching items, but
- * will not write back to the Sentry Policy Service.
- * The roleCache_ contains all roles defined in Sentry whereas userCache_ only contains
- * users that have privileges defined in Sentry and does not represent all users in the
- * system. A principal type can have 0 or more privileges and principal types are stored
- * in a map of principal name to principal object. For example:
+ * privileges associated with the user.
+ * The roleCache_ contains all roles defined in authorization provider whereas userCache_
+ * only contains users that have privileges defined in authorization provider and does
+ * not represent all users in the system. A principal type can have 0 or more privileges
+ * and principal types are stored in a map of principal name to principal object.
+ * For example:
* RoleName -> Role -> [PrincipalPriv1, ..., PrincipalPrivN]
* UserName -> User -> [PrincipalPriv1, ..., PrincipalPrivN]
* There is a separate cache for users since we cannot guarantee uniqueness between
@@ -63,12 +67,8 @@ import com.google.common.collect.Sets;
* "principal ID" rather than embedding the principal name. When a privilege is added to
* a principal type, we do a lookup to get the principal ID to probe the principalIds_
* map.
- * Acts as the backing cache for the Sentry cached based provider (which is why
- * PrivilegeCache is implemented).
- * TODO: Instead of calling into Sentry to perform final authorization checks, we
- * should parse/validate the privileges in Impala.
*/
-public class AuthorizationPolicy implements PrivilegeCache {
+public class AuthorizationPolicy {
private static final Logger LOG = Logger.getLogger(AuthorizationPolicy.class);
// Need to keep separate caches of role names and user names since there is
@@ -77,7 +77,8 @@ public class AuthorizationPolicy implements PrivilegeCache {
private final CatalogObjectCache<Role> roleCache_ = new CatalogObjectCache<>();
// Cache of user names (case-sensitive) to user objects.
- private final CatalogObjectCache<User> userCache_ = new CatalogObjectCache<>(false);
+ private final CatalogObjectCache<org.apache.impala.catalog.User> userCache_ =
+ new CatalogObjectCache<>(false);
// Map of principal ID -> user/role name. Used to match privileges to users/roles.
private final Map<Integer, String> principalIds_ = new HashMap<>();
@@ -114,8 +115,8 @@ public class AuthorizationPolicy implements PrivilegeCache {
}
}
if (principal.getPrincipalType() == TPrincipalType.USER) {
- Preconditions.checkArgument(principal instanceof User);
- userCache_.add((User) principal);
+ Preconditions.checkArgument(principal instanceof org.apache.impala.catalog.User);
+ userCache_.add((org.apache.impala.catalog.User) principal);
} else {
Preconditions.checkArgument(principal instanceof Role);
roleCache_.add((Role) principal);
@@ -194,7 +195,7 @@ public class AuthorizationPolicy implements PrivilegeCache {
/**
* Returns all users in the policy. Returns an empty list if no users exist.
*/
- public synchronized List<User> getAllUsers() {
+ public synchronized List<org.apache.impala.catalog.User> getAllUsers() {
return userCache_.getValues();
}
@@ -231,14 +232,14 @@ public class AuthorizationPolicy implements PrivilegeCache {
/**
* Gets a user given a user name. Returns null if no user exist with this name.
*/
- public synchronized User getUser(String userName) {
+ public synchronized org.apache.impala.catalog.User getUser(String userName) {
return userCache_.get(userName);
}
/**
* Gets a user given a user ID. Returns null if no user exists with this ID.
*/
- public synchronized User getUser(int userId) {
+ public synchronized org.apache.impala.catalog.User getUser(int userId) {
String userName = principalIds_.get(userId);
if (userName == null) return null;
return userCache_.get(userName);
@@ -359,8 +360,8 @@ public class AuthorizationPolicy implements PrivilegeCache {
* Removes a user. Returns the removed user or null if no user with
* this name existed.
*/
- public synchronized User removeUser(String userName) {
- User removedUser = userCache_.remove(userName);
+ public synchronized org.apache.impala.catalog.User removeUser(String userName) {
+ org.apache.impala.catalog.User removedUser = userCache_.remove(userName);
if (removedUser == null) return null;
// Cleanup user ID.
principalIds_.remove(removedUser.getId());
@@ -404,66 +405,6 @@ public class AuthorizationPolicy implements PrivilegeCache {
}
/**
- * Returns a set of privilege strings in Sentry format.
- */
- @Override
- public synchronized Set<String> listPrivileges(Set<String> groups,
- ActiveRoleSet roleSet) {
- Set<String> privileges = new HashSet<>();
- if (roleSet != ActiveRoleSet.ALL) {
- throw new UnsupportedOperationException("Impala does not support role subsets.");
- }
-
- // Collect all privileges granted to all roles.
- for (String groupName: groups) {
- List<Role> grantedRoles = getGrantedRoles(groupName);
- for (Role role: grantedRoles) {
- for (PrincipalPrivilege privilege: role.getPrivileges()) {
- String authorizeable = privilege.getName();
- if (authorizeable == null) {
- if (LOG.isTraceEnabled()) {
- LOG.trace("Ignoring invalid privilege: " + privilege.getName());
- }
- continue;
- }
- privileges.add(authorizeable);
- }
- }
- }
- return privileges;
- }
-
- /**
- * Returns a set of privilege strings in Sentry format.
- */
- @Override
- public synchronized Set<String> listPrivileges(Set<String> groups, Set<String> users,
- ActiveRoleSet roleSet) {
- Set<String> privileges = listPrivileges(groups, roleSet);
- for (String userName: users) {
- User user = getUser(userName);
- if (user != null) {
- for (PrincipalPrivilege privilege: user.getPrivileges()) {
- String authorizeable = privilege.getName();
- if (authorizeable == null) {
- if (LOG.isTraceEnabled()) {
- LOG.trace("Ignoring invalid privilege: " + privilege.getName());
- }
- continue;
- }
- privileges.add(authorizeable);
- }
- }
- }
- return privileges;
- }
-
- @Override
- public void close() {
- // Nothing to do, but required by PrivilegeCache.
- }
-
- /**
* Returns the privileges that have been granted to a role as a tabular result set.
* Allows for filtering based on a specific privilege spec or showing all privileges
* granted to the role. Used by the SHOW GRANT ROLE statement.
@@ -559,7 +500,7 @@ public class AuthorizationPolicy implements PrivilegeCache {
throw new AnalysisException(String.format("User '%s' does not exist.",
principalName));
}
- User user = getUser(principalName);
+ org.apache.impala.catalog.User user = getUser(principalName);
if (user != null) {
createShowUserPrivilegesResultRows(result, user.getPrivileges(), filter,
principalName, TPrincipalType.USER);
diff --git a/fe/src/main/java/org/apache/impala/catalog/AuthorizationException.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java
similarity index 70%
rename from fe/src/main/java/org/apache/impala/catalog/AuthorizationException.java
rename to fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java
index 909a75b..a575262 100644
--- a/fe/src/main/java/org/apache/impala/catalog/AuthorizationException.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationProvider.java
@@ -15,18 +15,12 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.catalog;
+package org.apache.impala.authorization;
/**
- * Thrown for authorization errors encountered when accessing Catalog objects.
+ * This enum contains the list of authorization providers supported in Impala.
*/
-public class AuthorizationException extends CatalogException {
- public AuthorizationException(String msg, Throwable cause) {
- super(msg, cause);
- }
-
- public AuthorizationException(String msg) {
- super(msg);
- }
-}
\ No newline at end of file
+public enum AuthorizationProvider {
+ SENTRY
+}
diff --git a/fe/src/main/java/org/apache/impala/authorization/Privilege.java b/fe/src/main/java/org/apache/impala/authorization/Privilege.java
index 877b6ad..f135707 100644
--- a/fe/src/main/java/org/apache/impala/authorization/Privilege.java
+++ b/fe/src/main/java/org/apache/impala/authorization/Privilege.java
@@ -19,97 +19,60 @@ package org.apache.impala.authorization;
import java.util.EnumSet;
-import org.apache.sentry.core.common.Action;
-import org.apache.sentry.core.common.BitFieldAction;
-
/**
- * Maps an Impala Privilege to one or more Impala "Actions".
+ * List of Impala privileges.
*/
public enum Privilege {
- ALL(ImpalaAction.ALL, false),
- ALTER(ImpalaAction.ALTER, false),
- DROP(ImpalaAction.DROP, false),
- CREATE(ImpalaAction.CREATE, false),
- INSERT(ImpalaAction.INSERT, false),
- SELECT(ImpalaAction.SELECT, false),
- REFRESH(ImpalaAction.REFRESH, false),
+ ALL,
+ OWNER,
+ ALTER,
+ DROP,
+ CREATE,
+ INSERT,
+ SELECT,
+ REFRESH,
// Privileges required to view metadata on a server object.
- VIEW_METADATA(EnumSet.of(ImpalaAction.INSERT, ImpalaAction.SELECT,
- ImpalaAction.REFRESH), true),
+ VIEW_METADATA(true),
// Special privilege that is used to determine if the user has any valid privileges
// on a target object.
- ANY(EnumSet.allOf(ImpalaAction.class), true),
- ;
-
- private final EnumSet<ImpalaAction> actions;
+ ANY(true);
+
+ static {
+ ALL.implied_ = EnumSet.of(ALL);
+ OWNER.implied_ = EnumSet.of(OWNER);
+ DROP.implied_ = EnumSet.of(DROP);
+ CREATE.implied_ = EnumSet.of(CREATE);
+ INSERT.implied_ = EnumSet.of(INSERT);
+ SELECT.implied_ = EnumSet.of(SELECT);
+ REFRESH.implied_ = EnumSet.of(REFRESH);
+ VIEW_METADATA.implied_ = EnumSet.of(INSERT, SELECT, REFRESH);
+ ANY.implied_ = EnumSet.of(ALL, OWNER, ALTER, DROP, CREATE, INSERT, SELECT,
+ REFRESH);
+ }
+ private EnumSet<Privilege> implied_;
// Determines whether to check if the user has ANY the privileges defined in the
// actions list or whether to check if the user has ALL of the privileges in the
// actions list.
private final boolean anyOf_;
- /**
- * This enum provides a list of Sentry actions used in Impala.
- */
- public enum ImpalaAction implements Action {
- SELECT("select", 1),
- INSERT("insert", 1 << 2),
- ALTER("alter", 1 << 3),
- CREATE("create", 1 << 4),
- DROP("drop", 1 << 5),
- REFRESH("refresh", 1 << 6),
- ALL("*",
- SELECT.getCode() |
- INSERT.getCode() |
- ALTER.getCode() |
- CREATE.getCode() |
- DROP.getCode() |
- REFRESH.getCode()),
- OWNER("owner", ALL.getCode());
-
- private final BitFieldAction bitFieldAction_;
-
- // In Sentry 1.5.1, BitFieldAction is an abstract class. In Sentry 2.0.0,
- // BitFieldAction is no longer an absract class. To support both versions,
- // we can extend BitFieldAction.
- private static class ImpalaBitFieldAction extends BitFieldAction {
- public ImpalaBitFieldAction(String name, int code) {
- super(name, code);
- }
- }
-
- ImpalaAction(String value, int code) {
- bitFieldAction_ = new ImpalaBitFieldAction(value, code);
- }
-
- @Override
- public String getValue() { return bitFieldAction_.getValue(); }
-
- public int getCode() { return bitFieldAction_.getActionCode(); }
-
- public BitFieldAction getBitFieldAction() { return bitFieldAction_; }
+ Privilege() {
+ anyOf_ = false;
}
- private Privilege(EnumSet<ImpalaAction> actions, boolean anyOf) {
- this.actions = actions;
+ Privilege(boolean anyOf) {
this.anyOf_ = anyOf;
}
- private Privilege(ImpalaAction action, boolean anyOf) {
- this(EnumSet.of(action), anyOf);
- }
-
- /*
- * Returns the set of Sentry Access Actions mapping to this Privilege.
- */
- public EnumSet<ImpalaAction> getSentryActions() {
- return actions;
- }
-
/*
* Determines whether to check if the user has ANY the privileges defined in the
* actions list or whether to check if the user has ALL of the privileges in the
* actions list.
*/
public boolean getAnyOf() { return anyOf_; }
+
+ /**
+ * Gets list of implied privileges for this privilege.
+ */
+ public EnumSet<Privilege> getImpliedPrivileges() { return implied_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java
index e6335d3..7b6ff2d 100644
--- a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java
+++ b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequest.java
@@ -22,51 +22,39 @@ import com.google.common.base.Preconditions;
import java.util.Objects;
/**
- * Represents a privilege request in the context of an Authorizeable object. If no
- * Authorizeable object is provided, it represents a privilege request on the server.
+ * Represents a privilege request in the context of an Authorizable object. If no
+ * Authorizable object is provided, it represents a privilege request on the server.
* For example, SELECT on table Foo in database Bar.
+ *
+ * Use PrivilegeRequestBuilder to create this instance.
*/
public class PrivilegeRequest {
- private final Authorizeable authorizeable_;
+ private final Authorizable authorizable_;
private final Privilege privilege_;
private final boolean grantOption_;
- public PrivilegeRequest(Authorizeable authorizeable, Privilege privilege) {
- this(authorizeable, privilege, false);
- }
-
- public PrivilegeRequest(Authorizeable authorizeable, Privilege privilege,
- boolean grantOption) {
- Preconditions.checkNotNull(authorizeable);
+ PrivilegeRequest(Authorizable authorizable, Privilege privilege, boolean grantOption) {
+ Preconditions.checkNotNull(authorizable);
Preconditions.checkNotNull(privilege);
- authorizeable_ = authorizeable;
+ authorizable_ = authorizable;
privilege_ = privilege;
grantOption_ = grantOption;
}
- public PrivilegeRequest(Privilege privilege) {
- Preconditions.checkNotNull(privilege);
- authorizeable_ = null;
- privilege_ = privilege;
- grantOption_ = false;
- }
-
/*
- * Name of the Authorizeable. Authorizeable refers to the server if it's null.
+ * Name of the Authorizable.
*/
- public String getName() {
- return (authorizeable_ != null) ? authorizeable_.getName() : "server";
- }
+ public String getName() { return authorizable_.getName(); }
/*
- * Requested privilege on the Authorizeable.
+ * Requested privilege on the Authorizable.
*/
public Privilege getPrivilege() { return privilege_; }
/*
- * Returns Authorizeable object. Null if the request is for server-level permission.
+ * Returns Authorizable object. Null if the request is for server-level permission.
*/
- public Authorizeable getAuthorizeable() { return authorizeable_; }
+ public Authorizable getAuthorizable() { return authorizable_; }
/**
* Returns whether the grant option is required or not.
@@ -75,7 +63,7 @@ public class PrivilegeRequest {
@Override
public int hashCode() {
- return Objects.hash(authorizeable_, privilege_, grantOption_);
+ return Objects.hash(authorizable_, privilege_, grantOption_);
}
@Override
@@ -84,7 +72,7 @@ public class PrivilegeRequest {
if (o == null || getClass() != o.getClass()) return false;
PrivilegeRequest that = (PrivilegeRequest) o;
return grantOption_ == that.grantOption_ &&
- Objects.equals(authorizeable_, that.authorizeable_) &&
+ Objects.equals(authorizable_, that.authorizable_) &&
privilege_ == that.privilege_;
}
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
index 6aa71e7..1723219 100644
--- a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
+++ b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
@@ -18,12 +18,18 @@
package org.apache.impala.authorization;
import com.google.common.base.Preconditions;
+import org.apache.impala.authorization.sentry.SentryAuthorizableColumn;
+import org.apache.impala.authorization.sentry.SentryAuthorizableDb;
+import org.apache.impala.authorization.sentry.SentryAuthorizableFn;
+import org.apache.impala.authorization.sentry.SentryAuthorizableServer;
+import org.apache.impala.authorization.sentry.SentryAuthorizableTable;
+import org.apache.impala.authorization.sentry.SentryAuthorizableUri;
/**
* Class that helps build PrivilegeRequest objects.
* For example:
* PrivilegeRequestBuilder builder = new PrivilegeRequestBuilder();
- * PrivilegeRequest = builder.allOf(Privilege.SELECT).onTable("db", "tbl").toRequest();
+ * PrivilegeRequest = builder.allOf(Privilege.SELECT).onTable("db", "tbl").build();
*
* TODO: In the future, this class could be extended to provide the option to specify
* multiple permissions. For example:
@@ -33,40 +39,62 @@ import com.google.common.base.Preconditions;
* builder.anyOf(SELECT, INSERT).onTable(...);
*/
public class PrivilegeRequestBuilder {
- Authorizeable authorizeable_;
- Privilege privilege_;
- boolean grantOption_ = false;
+ private Authorizable authorizable_;
+ private Privilege privilege_;
+ private boolean grantOption_ = false;
/**
- * Sets the authorizeable object to be a column.
+ * Sets the authorizable object to be a URI.
+ */
+ public PrivilegeRequestBuilder onFunction(String dbName, String fnName) {
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableFn(dbName, fnName);
+ return this;
+ }
+
+ /**
+ * Sets the authorizable object to be a URI.
+ */
+ public PrivilegeRequestBuilder onUri(String uriName) {
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableUri(uriName);
+ return this;
+ }
+
+ /**
+ * Sets the authorizable object to be a column.
*/
public PrivilegeRequestBuilder onColumn(String dbName, String tableName,
String columnName) {
- authorizeable_ = new AuthorizeableColumn(dbName, tableName, columnName);
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableColumn(dbName, tableName, columnName);
return this;
}
/**
- * Sets the authorizeable object to be a table.
+ * Sets the authorizable object to be a table.
*/
public PrivilegeRequestBuilder onTable(String dbName, String tableName) {
- authorizeable_ = new AuthorizeableTable(dbName, tableName);
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableTable(dbName, tableName);
return this;
}
/**
- * Sets the authorizeable object to be a server.
+ * Sets the authorizable object to be a server.
*/
public PrivilegeRequestBuilder onServer(String serverName) {
- authorizeable_ = new AuthorizeableServer(serverName);
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableServer(serverName);
return this;
}
/**
- * Sets the authorizeable object to be a database.
+ * Sets the authorizable object to be a database.
*/
public PrivilegeRequestBuilder onDb(String dbName) {
- authorizeable_ = new AuthorizeableDb(dbName);
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableDb(dbName);
return this;
}
@@ -74,14 +102,27 @@ public class PrivilegeRequestBuilder {
* Specifies that permissions on any table in the given database.
*/
public PrivilegeRequestBuilder onAnyTable(String dbName) {
- return onTable(dbName, AuthorizeableTable.ANY_TABLE_NAME);
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableTable(dbName);
+ return this;
}
/**
* Specifies that permissions on any column in the given table.
*/
public PrivilegeRequestBuilder onAnyColumn(String dbName, String tableName) {
- return onColumn(dbName, tableName, AuthorizeableColumn.ANY_COLUMN_NAME);
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableColumn(dbName, tableName);
+ return this;
+ }
+
+ /**
+ * Specifies that permissions on any column in any table.
+ */
+ public PrivilegeRequestBuilder onAnyColumn(String dbName) {
+ Preconditions.checkState(authorizable_ == null);
+ authorizable_ = new SentryAuthorizableColumn(dbName);
+ return this;
}
/**
@@ -117,12 +158,12 @@ public class PrivilegeRequestBuilder {
}
/**
- * Builds a PrivilegeRequest object based on the current Authorizeable object
+ * Builds a PrivilegeRequest object based on the current Authorizable object
* and privilege settings.
*/
- public PrivilegeRequest toRequest() {
- Preconditions.checkNotNull(authorizeable_);
+ public PrivilegeRequest build() {
+ Preconditions.checkNotNull(authorizable_);
Preconditions.checkNotNull(privilege_);
- return new PrivilegeRequest(authorizeable_, privilege_, grantOption_);
+ return new PrivilegeRequest(authorizable_, privilege_, grantOption_);
}
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaAction.java b/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaAction.java
new file mode 100644
index 0000000..152eacb
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaAction.java
@@ -0,0 +1,87 @@
+// 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.impala.authorization.sentry;
+
+import org.apache.impala.authorization.Privilege;
+import org.apache.sentry.core.common.Action;
+import org.apache.sentry.core.common.BitFieldAction;
+
+import java.util.EnumSet;
+import java.util.stream.Collectors;
+
+/**
+ * This enum provides a list of Sentry actions used in Impala.
+ */
+public enum ImpalaAction implements Action {
+ SELECT("select", 1),
+ INSERT("insert", 1 << 2),
+ ALTER("alter", 1 << 3),
+ CREATE("create", 1 << 4),
+ DROP("drop", 1 << 5),
+ REFRESH("refresh", 1 << 6),
+ ALL("*",
+ SELECT.getCode() |
+ INSERT.getCode() |
+ ALTER.getCode() |
+ CREATE.getCode() |
+ DROP.getCode() |
+ REFRESH.getCode()),
+ OWNER("owner", ALL.getCode());
+
+ private final BitFieldAction bitFieldAction_;
+
+ ImpalaAction(String value, int code) {
+ bitFieldAction_ = new BitFieldAction(value, code);
+ }
+
+ @Override
+ public String getValue() { return bitFieldAction_.getValue(); }
+
+ public int getCode() { return bitFieldAction_.getActionCode(); }
+
+ public BitFieldAction getBitFieldAction() { return bitFieldAction_; }
+
+ public static EnumSet<ImpalaAction> from(Privilege privilege) {
+ switch (privilege) {
+ case ALL:
+ return EnumSet.of(ImpalaAction.ALL);
+ case OWNER:
+ return EnumSet.of(ImpalaAction.OWNER);
+ case ALTER:
+ return EnumSet.of(ImpalaAction.ALTER);
+ case DROP:
+ return EnumSet.of(ImpalaAction.DROP);
+ case CREATE:
+ return EnumSet.of(ImpalaAction.CREATE);
+ case INSERT:
+ return EnumSet.of(ImpalaAction.INSERT);
+ case SELECT:
+ return EnumSet.of(ImpalaAction.SELECT);
+ case REFRESH:
+ return EnumSet.of(ImpalaAction.REFRESH);
+ case VIEW_METADATA:
+ case ANY:
+ return EnumSet.copyOf(privilege.getImpliedPrivileges()
+ .stream()
+ .flatMap(p -> from(p).stream())
+ .collect(Collectors.toSet()));
+ default:
+ throw new IllegalArgumentException("Unsupported privilege: " + privilege);
+ }
+ }
+}
diff --git a/fe/src/main/java/org/apache/impala/authorization/ImpalaActionFactory.java b/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaActionFactory.java
similarity index 95%
rename from fe/src/main/java/org/apache/impala/authorization/ImpalaActionFactory.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaActionFactory.java
index c3ef004..87e8a56 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ImpalaActionFactory.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaActionFactory.java
@@ -14,10 +14,10 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.impala.authorization;
+
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
-import org.apache.impala.authorization.Privilege.ImpalaAction;
import org.apache.sentry.core.common.BitFieldAction;
import org.apache.sentry.core.common.BitFieldActionFactory;
diff --git a/fe/src/main/java/org/apache/impala/authorization/ImpalaPrivilegeModel.java b/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaPrivilegeModel.java
similarity index 97%
rename from fe/src/main/java/org/apache/impala/authorization/ImpalaPrivilegeModel.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaPrivilegeModel.java
index 43a194e..5e350a4 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ImpalaPrivilegeModel.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/ImpalaPrivilegeModel.java
@@ -14,7 +14,8 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
+
+package org.apache.impala.authorization.sentry;
import java.util.Map;
diff --git a/fe/src/main/java/org/apache/impala/authorization/SentryAuthProvider.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java
similarity index 74%
rename from fe/src/main/java/org/apache/impala/authorization/SentryAuthProvider.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java
index dc30dff..860015b 100644
--- a/fe/src/main/java/org/apache/impala/authorization/SentryAuthProvider.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java
@@ -15,11 +15,13 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
+package org.apache.impala.authorization.sentry;
import org.apache.commons.lang.reflect.ConstructorUtils;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.sentry.policy.engine.common.CommonPolicyEngine;
+import org.apache.sentry.provider.cache.PrivilegeCache;
import org.apache.sentry.provider.cache.SimpleCacheProviderBackend;
import org.apache.sentry.provider.common.ProviderBackend;
import org.apache.sentry.provider.common.ProviderBackendContext;
@@ -37,20 +39,24 @@ class SentryAuthProvider {
*/
static ResourceAuthorizationProvider createProvider(AuthorizationConfig config,
AuthorizationPolicy policy) {
+ Preconditions.checkArgument(policy instanceof PrivilegeCache);
+ Preconditions.checkArgument(config instanceof SentryAuthorizationConfig);
+ SentryAuthorizationConfig sentryAuthzConfig = (SentryAuthorizationConfig) config;
try {
ProviderBackend providerBe;
// Create the appropriate backend provider.
- if (config.isFileBasedPolicy()) {
- providerBe = new SimpleFileProviderBackend(config.getSentryConfig().getConfig(),
- config.getPolicyFile());
+ if (sentryAuthzConfig.isFileBasedPolicy()) {
+ providerBe = new SimpleFileProviderBackend(
+ sentryAuthzConfig.getSentryConfig().getConfig(),
+ sentryAuthzConfig.getPolicyFile());
ProviderBackendContext context = new ProviderBackendContext();
providerBe.initialize(context);
} else {
// Note: The second parameter to the ProviderBackend is a "resourceFile" path
// which is not used by Impala. We cannot pass 'null' so instead pass an empty
// string.
- providerBe = new SimpleCacheProviderBackend(config.getSentryConfig().getConfig(),
- "");
+ providerBe = new SimpleCacheProviderBackend(
+ sentryAuthzConfig.getSentryConfig().getConfig(), "");
Preconditions.checkNotNull(policy);
ProviderBackendContext context = new ProviderBackendContext();
context.setBindingHandle(policy);
@@ -62,10 +68,11 @@ class SentryAuthProvider {
// Try to create an instance of the specified policy provider class.
// Re-throw any exceptions that are encountered.
- String policyFile = config.getPolicyFile() == null ? "" : config.getPolicyFile();
+ String policyFile = sentryAuthzConfig.getPolicyFile() == null ?
+ "" : sentryAuthzConfig.getPolicyFile();
return (ResourceAuthorizationProvider) ConstructorUtils.invokeConstructor(
- Class.forName(config.getPolicyProviderClassName()),
+ Class.forName(sentryAuthzConfig.getPolicyProviderClassName()),
new Object[] {policyFile, engine, ImpalaPrivilegeModel.INSTANCE});
} catch (Exception e) {
// Re-throw as unchecked exception.
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizable.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizable.java
new file mode 100644
index 0000000..b490f90
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizable.java
@@ -0,0 +1,59 @@
+// 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.impala.authorization.sentry;
+
+import com.google.common.base.Preconditions;
+import org.apache.impala.authorization.Authorizable;
+import org.apache.sentry.core.model.db.DBModelAuthorizable;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * A base class for Authorizable with Sentry specific operations.
+ */
+public abstract class SentryAuthorizable extends Authorizable {
+ /*
+ * Returns the list of the Authorizable objects in their hierarchical order.
+ * For example:
+ * [Column] would return Db -> Table -> Column
+ * [Table] would return Db -> Table
+ * [Db] would return [Db]
+ * [URI] would return [URI]
+ */
+ public abstract List<Authorizable> getAuthorizableHierarchy();
+
+ /**
+ * Returns the list of the Sentry DBModelAuthorizable objects in their hierarchical
+ * order.
+ */
+ public List<DBModelAuthorizable> getDBModelAuthorizableHierarchy() {
+ return getAuthorizableHierarchy().stream()
+ .map(a -> {
+ Preconditions.checkState(a instanceof SentryAuthorizable);
+ return ((SentryAuthorizable) a).getDBModelAuthorizable();
+ })
+ .collect(Collectors.toList());
+ }
+
+ /**
+ * Returns the Sentry DBModelAuthorizable that represents the type
+ * of Authorizable.
+ */
+ public abstract DBModelAuthorizable getDBModelAuthorizable();
+}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableColumn.java
similarity index 59%
rename from fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableColumn.java
index 0af3dfd..46bd354 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableColumn.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableColumn.java
@@ -15,38 +15,46 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-import java.util.List;
-
-import org.apache.sentry.core.model.db.DBModelAuthorizable;
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
+import org.apache.impala.authorization.Authorizable;
+import org.apache.sentry.core.model.db.AccessConstants;
+import org.apache.sentry.core.model.db.DBModelAuthorizable;
+
+import java.util.List;
/**
- * Class used to authorize access to a column.
+ * Class used to authorize access to a column for Sentry.
*/
-public class AuthorizeableColumn extends Authorizeable {
+public class SentryAuthorizableColumn extends SentryAuthorizable {
private final org.apache.sentry.core.model.db.Column column_;
private final org.apache.sentry.core.model.db.Table table_;
private final org.apache.sentry.core.model.db.Database database_;
- public final static String ANY_COLUMN_NAME =
- org.apache.sentry.core.model.db.AccessConstants.ALL;
- public AuthorizeableColumn(String dbName, String tableName, String columnName) {
- Preconditions.checkState(!Strings.isNullOrEmpty(dbName));
- Preconditions.checkState(!Strings.isNullOrEmpty(tableName));
- Preconditions.checkState(!Strings.isNullOrEmpty(columnName));
+ public SentryAuthorizableColumn(String dbName) {
+ this(dbName, AccessConstants.ALL);
+ }
+
+ public SentryAuthorizableColumn(String dbName, String tableName) {
+ this(dbName, tableName, AccessConstants.ALL);
+ }
+
+ public SentryAuthorizableColumn(String dbName, String tableName, String columnName) {
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(dbName));
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(tableName));
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(columnName));
column_ = new org.apache.sentry.core.model.db.Column(columnName);
table_ = new org.apache.sentry.core.model.db.Table(tableName);
database_ = new org.apache.sentry.core.model.db.Database(dbName);
}
@Override
- public List<DBModelAuthorizable> getHiveAuthorizeableHierarchy() {
- return Lists.newArrayList(database_, table_, column_);
+ public List<Authorizable> getAuthorizableHierarchy() {
+ return Lists.newArrayList(new SentryAuthorizableDb(database_.getName()),
+ new SentryAuthorizableTable(database_.getName(), table_.getName()), this);
}
@Override
@@ -54,6 +62,9 @@ public class AuthorizeableColumn extends Authorizeable {
+ column_.getName(); }
@Override
+ public Type getType() { return Type.COLUMN; }
+
+ @Override
public String getFullTableName() {
return database_.getName() + "." + table_.getName();
}
@@ -61,6 +72,12 @@ public class AuthorizeableColumn extends Authorizeable {
@Override
public String getDbName() { return database_.getName(); }
- public String getTblName() { return table_.getName(); }
+ @Override
+ public String getTableName() { return table_.getName(); }
+
+ @Override
public String getColumnName() { return column_.getName(); }
+
+ @Override
+ public DBModelAuthorizable getDBModelAuthorizable() { return column_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableDb.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableDb.java
similarity index 68%
rename from fe/src/main/java/org/apache/impala/authorization/AuthorizeableDb.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableDb.java
index 1f488f3..11b5063 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableDb.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableDb.java
@@ -15,34 +15,40 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-import java.util.List;
-
-import org.apache.sentry.core.model.db.DBModelAuthorizable;
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
import com.google.common.collect.Lists;
+import org.apache.impala.authorization.Authorizable;
+import org.apache.sentry.core.model.db.DBModelAuthorizable;
+
+import java.util.List;
/**
- * Class used to authorize access to a database.
+ * Class used to authorize access to a database for Sentry.
*/
-public class AuthorizeableDb extends Authorizeable {
+public class SentryAuthorizableDb extends SentryAuthorizable {
private final org.apache.sentry.core.model.db.Database database_;
- public AuthorizeableDb(String dbName) {
- Preconditions.checkState(dbName != null && !dbName.isEmpty());
+ public SentryAuthorizableDb(String dbName) {
+ Preconditions.checkArgument(dbName != null && !dbName.isEmpty());
database_ = new org.apache.sentry.core.model.db.Database(dbName);
}
@Override
- public List<DBModelAuthorizable> getHiveAuthorizeableHierarchy() {
- return Lists.newArrayList((DBModelAuthorizable) database_);
+ public List<Authorizable> getAuthorizableHierarchy() {
+ return Lists.newArrayList(this);
}
@Override
public String getName() { return database_.getName(); }
@Override
+ public Type getType() { return Authorizable.Type.DB; }
+
+ @Override
public String getDbName() { return getName(); }
+
+ @Override
+ public DBModelAuthorizable getDBModelAuthorizable() { return database_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFn.java
similarity index 64%
rename from fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFn.java
index e8bc6ef..f91b798 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableFn.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFn.java
@@ -15,40 +15,47 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-import java.util.List;
+package org.apache.impala.authorization.sentry;
+import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
+import com.google.common.collect.Lists;
+import org.apache.impala.authorization.Authorizable;
import org.apache.sentry.core.model.db.DBModelAuthorizable;
-import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
+import java.util.List;
/**
- * Class used to authorize access to a Function.
+ * Class used to authorize access to a Function for Sentry.
*/
-public class AuthorizeableFn extends Authorizeable {
+public class SentryAuthorizableFn extends SentryAuthorizable {
private final String fnName_;
private final org.apache.sentry.core.model.db.Database database_;
- public AuthorizeableFn(String dbName, String fnName) {
- Preconditions.checkState(!Strings.isNullOrEmpty(dbName));
- Preconditions.checkState(!Strings.isNullOrEmpty(fnName));
+ public SentryAuthorizableFn(String dbName, String fnName) {
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(dbName));
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(fnName));
database_ = new org.apache.sentry.core.model.db.Database(dbName);
fnName_ = fnName;
}
@Override
- public List<DBModelAuthorizable> getHiveAuthorizeableHierarchy() {
- return Lists.newArrayList((DBModelAuthorizable) database_);
+ public List<Authorizable> getAuthorizableHierarchy() {
+ return Lists.newArrayList(new SentryAuthorizableDb(database_.getName()));
}
@Override
public String getName() { return database_.getName() + "." + fnName_; }
@Override
+ public Type getType() { return Type.FUNCTION; }
+
+ @Override
public String getDbName() { return database_.getName(); }
- public String getFnName() { return fnName_; };
+ @Override
+ public String getFnName() { return fnName_; }
+
+ @Override
+ public DBModelAuthorizable getDBModelAuthorizable() { return database_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableServer.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableServer.java
similarity index 60%
rename from fe/src/main/java/org/apache/impala/authorization/AuthorizeableServer.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableServer.java
index 3046726..514ec1b 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableServer.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableServer.java
@@ -15,35 +15,39 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-import java.util.List;
+package org.apache.impala.authorization.sentry;
+import com.google.common.collect.Lists;
+import org.apache.impala.authorization.Authorizable;
import org.apache.sentry.core.model.db.DBModelAuthorizable;
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
-import com.google.common.collect.Lists;
+import java.util.List;
/**
- * Class used to authorize access at the catalog level. Generally, all Impala
- * services in the cluster will be configured with the same catalog name.
+ * Class used to authorize access at the catalog level for Sentry. Generally, all
+ * Impala services in the cluster will be configured with the same catalog name.
* What Sentry refers to as a Server maps to our concept of a Catalog, thus
- * the name AuthorizeableServer.
+ * the name AuthorizableServer.
*/
-public class AuthorizeableServer extends Authorizeable {
+public class SentryAuthorizableServer extends SentryAuthorizable {
private final org.apache.sentry.core.model.db.Server server_;
- public AuthorizeableServer(String serverName) {
- Preconditions.checkState(!Strings.isNullOrEmpty(serverName));
- server_ = new org.apache.sentry.core.model.db.Server(serverName);
+ public SentryAuthorizableServer(String serverName) {
+ server_ = new org.apache.sentry.core.model.db.Server(
+ serverName == null ? "server" : serverName);
}
@Override
- public List<DBModelAuthorizable> getHiveAuthorizeableHierarchy() {
- return Lists.newArrayList((DBModelAuthorizable) server_);
+ public List<Authorizable> getAuthorizableHierarchy() {
+ return Lists.newArrayList(this);
}
@Override
public String getName() { return server_.getName(); }
+
+ @Override
+ public Type getType() { return Type.SERVER; }
+
+ @Override
+ public DBModelAuthorizable getDBModelAuthorizable() { return server_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableTable.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableTable.java
similarity index 64%
rename from fe/src/main/java/org/apache/impala/authorization/AuthorizeableTable.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableTable.java
index 9526272..b88f185 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableTable.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableTable.java
@@ -15,49 +15,57 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-import java.util.List;
-
-import org.apache.sentry.core.model.db.DBModelAuthorizable;
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
+import org.apache.impala.authorization.Authorizable;
+import org.apache.sentry.core.model.db.AccessConstants;
+import org.apache.sentry.core.model.db.DBModelAuthorizable;
+
+import java.util.List;
/**
- * Class used to authorize access to a table or view.
+ * Class used to authorize access to a table or view for Sentry.
* Even though Hive's spec includes an authorizable object 'view', we chose
* to treat views the same way as tables for the sake of authorization.
*/
-public class AuthorizeableTable extends Authorizeable {
- // Constant to represent privileges in the policy for "ANY" table in a
- // a database.
- public final static String ANY_TABLE_NAME =
- org.apache.sentry.core.model.db.AccessConstants.ALL;
-
+public class SentryAuthorizableTable extends SentryAuthorizable {
private final org.apache.sentry.core.model.db.Table table_;
private final org.apache.sentry.core.model.db.Database database_;
- public AuthorizeableTable(String dbName, String tableName) {
- Preconditions.checkState(!Strings.isNullOrEmpty(tableName));
- Preconditions.checkState(!Strings.isNullOrEmpty(dbName));
+ public SentryAuthorizableTable(String dbName) {
+ this(dbName, AccessConstants.ALL);
+ }
+
+ public SentryAuthorizableTable(String dbName, String tableName) {
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(tableName));
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(dbName));
table_ = new org.apache.sentry.core.model.db.Table(tableName);
database_ = new org.apache.sentry.core.model.db.Database(dbName);
}
@Override
- public List<DBModelAuthorizable> getHiveAuthorizeableHierarchy() {
- return Lists.newArrayList(database_, table_);
+ public List<Authorizable> getAuthorizableHierarchy() {
+ return Lists.newArrayList(new SentryAuthorizableDb(database_.getName()), this);
}
@Override
public String getName() { return database_.getName() + "." + table_.getName(); }
@Override
+ public Type getType() { return Type.TABLE; }
+
+ @Override
public String getDbName() { return database_.getName(); }
- public String getTblName() { return table_.getName(); }
+
+ @Override
+ public String getTableName() { return table_.getName(); }
@Override
public String getFullTableName() { return getName(); }
+
+ @Override
+ public DBModelAuthorizable getDBModelAuthorizable() { return table_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableUri.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableUri.java
similarity index 59%
rename from fe/src/main/java/org/apache/impala/authorization/AuthorizeableUri.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableUri.java
index cbc6bf6..f359af8 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizeableUri.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableUri.java
@@ -15,33 +15,39 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-import java.util.List;
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
import com.google.common.collect.Lists;
+import org.apache.impala.authorization.Authorizable;
+import org.apache.sentry.core.model.db.AccessURI;
+import org.apache.sentry.core.model.db.DBModelAuthorizable;
+
+import java.util.List;
/**
- * Class used to authorize access to a URI.
+ * Class used to authorize access to a URI for Sentry.
*/
-public class AuthorizeableUri extends Authorizeable {
+public class SentryAuthorizableUri extends SentryAuthorizable {
private final String uriName_;
- public AuthorizeableUri(String uriName) {
- Preconditions.checkNotNull(uriName);
+ public SentryAuthorizableUri(String uriName) {
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(uriName));
uriName_ = uriName;
}
@Override
- public List<org.apache.sentry.core.model.db.DBModelAuthorizable>
- getHiveAuthorizeableHierarchy() {
- org.apache.sentry.core.model.db.AccessURI accessURI =
- new org.apache.sentry.core.model.db.AccessURI(uriName_);
- return Lists.newArrayList(
- (org.apache.sentry.core.model.db.DBModelAuthorizable) accessURI);
+ public List<Authorizable> getAuthorizableHierarchy() {
+ return Lists.newArrayList(this);
}
@Override
public String getName() { return uriName_; }
+
+ @Override
+ public Type getType() { return Type.URI; }
+
+ @Override
+ public DBModelAuthorizable getDBModelAuthorizable() { return new AccessURI(uriName_); }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
new file mode 100644
index 0000000..c2f6f85
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java
@@ -0,0 +1,127 @@
+// 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.impala.authorization.sentry;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import org.apache.impala.authorization.AuthorizationChecker;
+import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.ImpalaInternalAdminUser;
+import org.apache.impala.authorization.Privilege;
+import org.apache.impala.authorization.PrivilegeRequest;
+import org.apache.impala.authorization.User;
+import org.apache.impala.authorization.AuthorizationPolicy;
+import org.apache.impala.common.InternalException;
+import org.apache.sentry.core.common.ActiveRoleSet;
+import org.apache.sentry.core.common.Subject;
+import org.apache.sentry.core.model.db.DBModelAuthorizable;
+import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
+
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * An implementation of AuthorizationChecker that uses Sentry.
+ */
+public class SentryAuthorizationChecker extends AuthorizationChecker {
+ private final ResourceAuthorizationProvider provider_;
+ private final SentryAuthorizableServer server_;
+
+ public SentryAuthorizationChecker(AuthorizationConfig config,
+ AuthorizationPolicy policy) {
+ super(config);
+ if (config.isEnabled()) {
+ server_ = new SentryAuthorizableServer(config.getServerName());
+ provider_ = createProvider(config, policy);
+ Preconditions.checkNotNull(provider_);
+ } else {
+ provider_ = null;
+ server_ = null;
+ }
+ }
+
+ @Override
+ public Set<String> getUserGroups(User user) throws InternalException {
+ try {
+ return provider_.getGroupMapping().getGroups(user.getShortName());
+ } catch (Exception e) {
+ if (SentryUtil.isSentryGroupNotFound(e)) {
+ // Sentry 2.1+ throws exceptions when user does not exist; swallow the
+ // exception and just return an empty set for this case.
+ return Collections.emptySet();
+ }
+ throw new RuntimeException(e);
+ }
+ }
+
+ /*
+ * Creates a new ResourceAuthorizationProvider based on the given configuration.
+ */
+ private static ResourceAuthorizationProvider createProvider(AuthorizationConfig config,
+ AuthorizationPolicy policy) {
+ return SentryAuthProvider.createProvider(config, policy);
+ }
+
+ @Override
+ public boolean hasAccess(User user, PrivilegeRequest request) throws InternalException {
+ Preconditions.checkNotNull(user);
+ Preconditions.checkNotNull(request);
+
+ // If authorization is not enabled the user will always have access. If this is
+ // an internal request, the user will always have permission.
+ if (!config_.isEnabled() || user instanceof ImpalaInternalAdminUser) {
+ return true;
+ }
+
+ EnumSet<ImpalaAction> actions = ImpalaAction.from(request.getPrivilege());
+
+ List<DBModelAuthorizable> authorizables = Lists.newArrayList(
+ server_.getDBModelAuthorizableHierarchy());
+ // If request.getAuthorizable() is null, the request is for server-level permission.
+ if (request.getAuthorizable() != null) {
+ Preconditions.checkState(request.getAuthorizable() instanceof SentryAuthorizable);
+ authorizables.addAll(((SentryAuthorizable) request.getAuthorizable())
+ .getDBModelAuthorizableHierarchy());
+ }
+
+ // The Hive Access API does not currently provide a way to check if the user
+ // has any privileges on a given resource.
+ if (request.getPrivilege().getAnyOf()) {
+ for (ImpalaAction action: actions) {
+ if (provider_.hasAccess(new Subject(user.getShortName()), authorizables,
+ EnumSet.of(action), request.hasGrantOption(), ActiveRoleSet.ALL)) {
+ return true;
+ }
+ }
+ return false;
+ // AuthorizableFn is special due to Sentry not having the concept of a function in
+ // DBModelAuthorizable.AuthorizableType. As a result, the list of authorizables for
+ // an AuthorizableFn only contains the server and database, but not the function
+ // itself. So there is no need to remove the last authorizable here.
+ } else if (request.getPrivilege() == Privilege.CREATE && authorizables.size() > 1 &&
+ !(request.getAuthorizable() instanceof SentryAuthorizableFn)) {
+ // CREATE on an object requires CREATE on the parent,
+ // so don't check access on the object we're creating.
+ authorizables.remove(authorizables.size() - 1);
+ }
+ return provider_.hasAccess(new Subject(user.getShortName()), authorizables, actions,
+ request.hasGrantOption(), ActiveRoleSet.ALL);
+ }
+}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationConfig.java
similarity index 80%
copy from fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java
copy to fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationConfig.java
index 96b65d8..d1aa40a 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationConfig.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationConfig.java
@@ -15,19 +15,19 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
-
-
-import org.apache.sentry.provider.common.HadoopGroupResourceAuthorizationProvider;
-import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
+import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.AuthorizationProvider;
+import org.apache.sentry.provider.common.HadoopGroupResourceAuthorizationProvider;
+import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
-/*
- * Class that contains configuration details for Impala authorization.
+/**
+ * Impala authorization configuration with Sentry.
*/
-public class AuthorizationConfig {
+public class SentryAuthorizationConfig implements AuthorizationConfig {
private final String serverName_;
// Set only if the policy provider is file-based.
private final String policyFile_;
@@ -42,7 +42,7 @@ public class AuthorizationConfig {
* @param sentryConfigFile - Absolute path and file name of the sentry service.
* @param policyProviderClassName - Class name of the policy provider to use.
*/
- public AuthorizationConfig(String serverName, String policyFile,
+ public SentryAuthorizationConfig(String serverName, String policyFile,
String sentryConfigFile, String policyProviderClassName) {
serverName_ = serverName;
policyFile_ = policyFile;
@@ -54,20 +54,27 @@ public class AuthorizationConfig {
validateConfig();
}
+ public SentryAuthorizationConfig(SentryConfig config) {
+ sentryConfig_ = config;
+ serverName_ = null;
+ policyFile_ = null;
+ policyProviderClassName_ = null;
+ }
+
/**
* Returns an AuthorizationConfig object that has authorization disabled.
*/
- public static AuthorizationConfig createAuthDisabledConfig() {
- return new AuthorizationConfig(null, null, null, null);
+ public static SentryAuthorizationConfig createAuthDisabledConfig() {
+ return new SentryAuthorizationConfig(null, null, null, null);
}
/**
* Returns an AuthorizationConfig object configured to use Hadoop user->group mappings
* for the authorization provider.
*/
- public static AuthorizationConfig createHadoopGroupAuthConfig(String serverName,
+ public static SentryAuthorizationConfig createHadoopGroupAuthConfig(String serverName,
String policyFile, String sentryConfigFile) {
- return new AuthorizationConfig(serverName, policyFile, sentryConfigFile,
+ return new SentryAuthorizationConfig(serverName, policyFile, sentryConfigFile,
HadoopGroupResourceAuthorizationProvider.class.getName());
}
@@ -88,7 +95,7 @@ public class AuthorizationConfig {
if (Strings.isNullOrEmpty(serverName_)) {
throw new IllegalArgumentException(
"Authorization is enabled but the server name is null or empty. Set the " +
- "server name using the impalad --server_name flag.");
+ "server name using the impalad --server_name flag.");
}
if (Strings.isNullOrEmpty(policyProviderClassName_)) {
throw new IllegalArgumentException("Authorization is enabled but the " +
@@ -108,7 +115,7 @@ public class AuthorizationConfig {
Preconditions.checkNotNull(providerClass);
if (!ResourceAuthorizationProvider.class.isAssignableFrom(providerClass)) {
throw new IllegalArgumentException(String.format("The authorization policy " +
- "provider class '%s' must be a subclass of '%s'.",
+ "provider class '%s' must be a subclass of '%s'.",
policyProviderClassName_,
ResourceAuthorizationProvider.class.getName()));
}
@@ -116,14 +123,18 @@ public class AuthorizationConfig {
/**
* Returns true if authorization is enabled.
- * If either serverName, policyFile, or sentryServiceConfig_ file is set (not null
+ * If either serverName_, policyFile_, or sentryConfig_ file is set (not null
* or empty), authorization is considered enabled.
*/
+ @Override
public boolean isEnabled() {
return !Strings.isNullOrEmpty(serverName_) || !Strings.isNullOrEmpty(policyFile_) ||
!Strings.isNullOrEmpty(sentryConfig_.getConfigFile());
}
+ @Override
+ public AuthorizationProvider getProvider() { return AuthorizationProvider.SENTRY; }
+
/**
* Returns true if using an authorization policy from a file in HDFS. If false,
* uses an authorization policy based on cached metadata sent from the catalog server
@@ -134,6 +145,7 @@ public class AuthorizationConfig {
/**
* The server name to secure.
*/
+ @Override
public String getServerName() { return serverName_; }
/**
@@ -145,5 +157,9 @@ public class AuthorizationConfig {
* The Sentry configuration.
*/
public SentryConfig getSentryConfig() { return sentryConfig_; }
+
+ /**
+ * Returns Sentry policy provider class name.
+ */
public String getPolicyProviderClassName() { return policyProviderClassName_; }
}
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
new file mode 100644
index 0000000..4609c63
--- /dev/null
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java
@@ -0,0 +1,102 @@
+// 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.impala.authorization.sentry;
+
+import com.google.common.collect.Sets;
+import org.apache.impala.authorization.AuthorizationPolicy;
+import org.apache.impala.catalog.PrincipalPrivilege;
+import org.apache.impala.catalog.Role;
+import org.apache.impala.catalog.User;
+import org.apache.log4j.Logger;
+import org.apache.sentry.core.common.ActiveRoleSet;
+import org.apache.sentry.provider.cache.PrivilegeCache;
+
+import java.util.List;
+import java.util.Set;
+
+/**
+ * The source data this cache is backing is read from the Sentry Policy Service.
+ * Writing to the cache will replace any matching items, but will not write back to the
+ * Sentry Policy Service. Acts as the backing cache for the Sentry cached based provider.
+ * TODO: Instead of calling into Sentry to perform final authorization checks, we
+ * should parse/validate the privileges in Impala.
+ */
+public class SentryAuthorizationPolicy extends AuthorizationPolicy
+ implements PrivilegeCache {
+ private static final Logger LOG = Logger.getLogger(SentryAuthorizationPolicy.class);
+
+ /**
+ * Returns a set of privilege strings in Sentry format.
+ */
+ @Override
+ public synchronized Set<String> listPrivileges(Set<String> groups,
+ ActiveRoleSet roleSet) {
+ Set<String> privileges = Sets.newHashSet();
+ if (roleSet != ActiveRoleSet.ALL) {
+ throw new UnsupportedOperationException("Impala does not support role subsets.");
+ }
+
+ // Collect all privileges granted to all roles.
+ for (String groupName: groups) {
+ List<Role> grantedRoles = getGrantedRoles(groupName);
+ for (Role role: grantedRoles) {
+ for (PrincipalPrivilege privilege: role.getPrivileges()) {
+ String authorizable = privilege.getName();
+ if (authorizable == null) {
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Ignoring invalid privilege: " + privilege.getName());
+ }
+ continue;
+ }
+ privileges.add(authorizable);
+ }
+ }
+ }
+ return privileges;
+ }
+
+ /**
+ * Returns a set of privilege strings in Sentry format.
+ */
+ @Override
+ public synchronized Set<String> listPrivileges(Set<String> groups, Set<String> users,
+ ActiveRoleSet roleSet) {
+ Set<String> privileges = listPrivileges(groups, roleSet);
+ for (String userName: users) {
+ User user = getUser(userName);
+ if (user != null) {
+ for (PrincipalPrivilege privilege: user.getPrivileges()) {
+ String authorizable = privilege.getName();
+ if (authorizable == null) {
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Ignoring invalid privilege: " + privilege.getName());
+ }
+ continue;
+ }
+ privileges.add(authorizable);
+ }
+ }
+ }
+ return privileges;
+ }
+
+ @Override
+ public void close() {
+ // Nothing to do, but required by PrivilegeCache.
+ }
+}
diff --git a/fe/src/main/java/org/apache/impala/authorization/SentryConfig.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryConfig.java
similarity index 98%
rename from fe/src/main/java/org/apache/impala/authorization/SentryConfig.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryConfig.java
index 8ce74d1..991bb11 100644
--- a/fe/src/main/java/org/apache/impala/authorization/SentryConfig.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryConfig.java
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
+package org.apache.impala.authorization.sentry;
import java.io.File;
import java.net.MalformedURLException;
diff --git a/fe/src/main/java/org/apache/impala/common/SentryPolicyReaderException.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyReaderException.java
similarity index 96%
rename from fe/src/main/java/org/apache/impala/common/SentryPolicyReaderException.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyReaderException.java
index b3c27f8..209e47b 100644
--- a/fe/src/main/java/org/apache/impala/common/SentryPolicyReaderException.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyReaderException.java
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.common;
+package org.apache.impala.authorization.sentry;
/**
* Thrown as a generic exception when processing Sentry policy.
diff --git a/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java
similarity index 98%
rename from fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java
index f10f66b..c78ccc7 100644
--- a/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.util;
+package org.apache.impala.authorization.sentry;
import java.util.List;
import java.util.Map;
@@ -33,9 +33,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.impala.analysis.PrivilegeSpec;
-import org.apache.impala.authorization.SentryConfig;
import org.apache.impala.authorization.User;
-import org.apache.impala.catalog.AuthorizationException;
+import org.apache.impala.authorization.AuthorizationException;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.InternalException;
import org.apache.impala.thrift.TPrivilege;
@@ -399,8 +398,7 @@ public class SentryPolicyService {
public List<TSentryRole> listAllRoles(User requestingUser) throws ImpalaException {
SentryServiceClient client = new SentryServiceClient();
try {
- return Lists.newArrayList(SentryUtil.listRoles(client.get(),
- requestingUser.getShortName()));
+ return Lists.newArrayList(client.get().listAllRoles(requestingUser.getShortName()));
} catch (Exception e) {
if (SentryUtil.isSentryAccessDenied(e)) {
throw new AuthorizationException(String.format(ACCESS_DENIED_ERROR_MSG,
diff --git a/fe/src/main/java/org/apache/impala/util/SentryProxy.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java
similarity index 98%
rename from fe/src/main/java/org/apache/impala/util/SentryProxy.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java
index 2b6b564..5e3d8c9 100644
--- a/fe/src/main/java/org/apache/impala/util/SentryProxy.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.util;
+package org.apache.impala.authorization.sentry;
import java.util.ArrayList;
import java.util.List;
@@ -27,15 +27,13 @@ import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import com.google.common.annotations.VisibleForTesting;
-import org.apache.impala.catalog.AuthorizationException;
+import org.apache.impala.authorization.AuthorizationException;
import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.catalog.Principal;
import org.apache.impala.catalog.PrincipalPrivilege;
import org.apache.impala.catalog.Role;
import org.apache.impala.common.Reference;
-import org.apache.impala.common.SentryPolicyReaderException;
-import org.apache.impala.common.SentryUnavailableException;
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TPrincipalType;
import org.apache.log4j.Logger;
@@ -43,7 +41,6 @@ import org.apache.sentry.api.service.thrift.TSentryGroup;
import org.apache.sentry.api.service.thrift.TSentryPrivilege;
import org.apache.sentry.api.service.thrift.TSentryRole;
-import org.apache.impala.authorization.SentryConfig;
import org.apache.impala.authorization.User;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.ImpalaRuntimeException;
@@ -114,8 +111,10 @@ public class SentryProxy {
// The value for the object ownership config.
private final String objectOwnershipConfigValue_;
- public SentryProxy(SentryConfig sentryConfig, CatalogServiceCatalog catalog,
+ public SentryProxy(SentryAuthorizationConfig authzConfig, CatalogServiceCatalog catalog,
String kerberosPrincipal) throws ImpalaException {
+ Preconditions.checkNotNull(authzConfig);
+ SentryConfig sentryConfig = authzConfig.getSentryConfig();
Preconditions.checkNotNull(catalog);
Preconditions.checkNotNull(sentryConfig);
catalog_ = catalog;
diff --git a/fe/src/main/java/org/apache/impala/common/SentryUnavailableException.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryUnavailableException.java
similarity index 96%
rename from fe/src/main/java/org/apache/impala/common/SentryUnavailableException.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryUnavailableException.java
index 2c32a44..d16c291 100644
--- a/fe/src/main/java/org/apache/impala/common/SentryUnavailableException.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryUnavailableException.java
@@ -15,7 +15,7 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.common;
+package org.apache.impala.authorization.sentry;
/**
* Thrown when Sentry is not available.
diff --git a/fe/src/main/java/org/apache/impala/util/SentryUtil.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryUtil.java
similarity index 72%
rename from fe/src/main/java/org/apache/impala/util/SentryUtil.java
rename to fe/src/main/java/org/apache/impala/authorization/sentry/SentryUtil.java
index 3eb7319..fea6f77 100644
--- a/fe/src/main/java/org/apache/impala/util/SentryUtil.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryUtil.java
@@ -15,26 +15,25 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.util;
-
-import java.util.Set;
+package org.apache.impala.authorization.sentry;
import org.apache.sentry.core.common.exception.SentryAccessDeniedException;
import org.apache.sentry.core.common.exception.SentryAlreadyExistsException;
import org.apache.sentry.core.common.exception.SentryGroupNotFoundException;
-import org.apache.sentry.api.service.thrift.SentryPolicyServiceClient;
-import org.apache.sentry.api.service.thrift.TSentryRole;
import org.apache.sentry.core.common.exception.SentryUserException;
+import org.apache.thrift.transport.TTransportException;
/**
- * Wrapper to facilitate differences in Sentry APIs across Sentry versions.
+ * A helper class for Sentry.
*/
public class SentryUtil {
- static boolean isSentryAlreadyExists(Exception e) {
+ private SentryUtil() {}
+
+ public static boolean isSentryAlreadyExists(Exception e) {
return e instanceof SentryAlreadyExistsException;
}
- static boolean isSentryAccessDenied(Exception e) {
+ public static boolean isSentryAccessDenied(Exception e) {
return e instanceof SentryAccessDeniedException;
}
@@ -42,8 +41,8 @@ public class SentryUtil {
return e instanceof SentryGroupNotFoundException;
}
- static Set<TSentryRole> listRoles(SentryPolicyServiceClient client, String username)
- throws SentryUserException {
- return client.listAllRoles(username);
+ public static boolean isSentryUnavailable(Exception e) {
+ return e instanceof SentryUserException &&
+ e.getCause() instanceof TTransportException;
}
}
diff --git a/fe/src/main/java/org/apache/impala/catalog/Catalog.java b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
index 3145686..963d728 100644
--- a/fe/src/main/java/org/apache/impala/catalog/Catalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/Catalog.java
@@ -26,6 +26,8 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicReference;
import org.apache.impala.analysis.FunctionName;
+import org.apache.impala.authorization.AuthorizationPolicy;
+import org.apache.impala.authorization.sentry.SentryAuthorizationPolicy;
import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TFunction;
@@ -67,7 +69,7 @@ public abstract class Catalog implements AutoCloseable {
// Cache of authorization policy metadata. Populated from data retried from the
// Sentry Service, if configured.
- protected AuthorizationPolicy authPolicy_ = new AuthorizationPolicy();
+ protected AuthorizationPolicy authPolicy_ = new SentryAuthorizationPolicy();
// Thread safe cache of database metadata. Uses an AtomicReference so reset()
// operations can atomically swap dbCache_ references.
diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
index 55a854a..89a0e3a 100644
--- a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
@@ -41,7 +41,11 @@ import org.apache.hadoop.hdfs.protocol.CachePoolInfo;
import org.apache.hadoop.hive.metastore.api.CurrentNotificationEventId;
import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
import org.apache.hadoop.hive.metastore.api.UnknownDBException;
-import org.apache.impala.authorization.SentryConfig;
+import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.AuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationProvider;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryProxy;
import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
import org.apache.impala.catalog.events.ExternalEventsProcessor;
import org.apache.impala.catalog.events.MetastoreEventsProcessor;
@@ -79,7 +83,6 @@ import org.apache.impala.thrift.TUniqueId;
import org.apache.impala.thrift.TUpdateTableUsageRequest;
import org.apache.impala.util.FunctionUtils;
import org.apache.impala.util.PatternMatcher;
-import org.apache.impala.util.SentryProxy;
import org.slf4j.Logger;
import org.apache.thrift.TException;
import org.apache.thrift.TSerializer;
@@ -262,9 +265,9 @@ public class CatalogServiceCatalog extends Catalog {
* @throws ImpalaException
*/
public CatalogServiceCatalog(boolean loadInBackground, int numLoadingThreads,
- SentryConfig sentryConfig, TUniqueId catalogServiceId, String kerberosPrincipal,
- String localLibraryPath, MetaStoreClientPool metaStoreClientPool)
- throws ImpalaException {
+ AuthorizationConfig authConfig, TUniqueId catalogServiceId,
+ String kerberosPrincipal, String localLibraryPath,
+ MetaStoreClientPool metaStoreClientPool) throws ImpalaException {
super(metaStoreClientPool);
catalogServiceId_ = catalogServiceId;
tableLoadingMgr_ = new TableLoadingMgr(this, numLoadingThreads);
@@ -279,8 +282,10 @@ public class CatalogServiceCatalog extends Catalog {
} catch (IOException e) {
LOG.error("Couldn't identify the default FS. Cache Pool reader will be disabled.");
}
- if (sentryConfig != null) {
- sentryProxy_ = new SentryProxy(sentryConfig, this, kerberosPrincipal);
+ if (authConfig != null && authConfig.isEnabled() &&
+ authConfig.getProvider() == AuthorizationProvider.SENTRY) {
+ sentryProxy_ = new SentryProxy((SentryAuthorizationConfig) authConfig, this,
+ kerberosPrincipal);
} else {
sentryProxy_ = null;
}
@@ -325,18 +330,19 @@ public class CatalogServiceCatalog extends Catalog {
}
}
- /**
- * Initializes the Catalog using the default MetastoreClientPool impl.
- * @param initialHmsCnxnTimeoutSec Time (in seconds) CatalogServiceCatalog will wait
- * to establish an initial connection to the HMS before giving up.
- */
-
+ /**
+ * Initializes the Catalog using the default MetastoreClientPool impl.
+ * @param initialHmsCnxnTimeoutSec Time (in seconds) CatalogServiceCatalog will wait
+ * to establish an initial connection to the HMS before
+ * giving up.
+ */
public CatalogServiceCatalog(boolean loadInBackground, int numLoadingThreads,
- int initialHmsCnxnTimeoutSec, SentryConfig sentryConfig, TUniqueId catalogServiceId,
- String kerberosPrincipal, String localLibraryPath) throws ImpalaException {
- this(loadInBackground, numLoadingThreads, sentryConfig, catalogServiceId,
+ int initialHmsCnxnTimeoutSec, AuthorizationConfig authConfig,
+ TUniqueId catalogServiceId, String kerberosPrincipal, String localLibraryPath)
+ throws ImpalaException {
+ this(loadInBackground, numLoadingThreads, authConfig, catalogServiceId,
kerberosPrincipal, localLibraryPath, new MetaStoreClientPool(
- INITIAL_META_STORE_CLIENT_POOL_SIZE, initialHmsCnxnTimeoutSec));
+ INITIAL_META_STORE_CLIENT_POOL_SIZE, initialHmsCnxnTimeoutSec));
}
// Timeout for acquiring a table lock
diff --git a/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java b/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
index 999343e..10396f9 100644
--- a/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/FeCatalog.java
@@ -20,6 +20,7 @@ import java.util.List;
import java.util.Set;
import org.apache.impala.analysis.TableName;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.common.InternalException;
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TGetPartitionStatsResponse;
diff --git a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
index 8f97486..4c1f975 100644
--- a/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java
@@ -23,6 +23,7 @@ import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import org.apache.impala.analysis.TableName;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.common.InternalException;
import org.apache.impala.common.Pair;
import org.apache.impala.service.FeSupport;
diff --git a/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java b/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
index 4876367..2b97dc4 100644
--- a/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
+++ b/fe/src/main/java/org/apache/impala/catalog/PrincipalPrivilege.java
@@ -19,6 +19,7 @@ package org.apache.impala.catalog;
import java.util.List;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TCatalogObjectType;
import org.apache.impala.thrift.TPrincipalType;
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
index 0bc04a2..d17f343 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java
@@ -37,7 +37,8 @@ import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.Table;
import org.apache.hadoop.hive.metastore.api.UnknownDBException;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.sentry.SentryAuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.CatalogDeltaLog;
import org.apache.impala.catalog.CatalogException;
@@ -263,7 +264,7 @@ public class CatalogdMetaProvider implements MetaProvider {
* Cache of authorization policy metadata. Populated from data pushed from the
* StateStore. Currently this is _not_ "fetch-on-demand".
*/
- private final AuthorizationPolicy authPolicy_ = new AuthorizationPolicy();
+ private final AuthorizationPolicy authPolicy_ = new SentryAuthorizationPolicy();
public CatalogdMetaProvider(TBackendGflags flags) {
Preconditions.checkArgument(flags.isSetLocal_catalog_cache_expiration_s());
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
index 8bc8996..91deec6 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
@@ -39,7 +39,7 @@ import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.Table;
import org.apache.hadoop.hive.metastore.api.UnknownDBException;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.HdfsPartition.FileDescriptor;
import org.apache.impala.catalog.HdfsTable;
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
index 56d275a..b2f3f9c 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java
@@ -23,7 +23,7 @@ import java.util.Map;
import java.util.Set;
import org.apache.impala.analysis.TableName;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.catalog.BuiltinsDb;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.CatalogException;
diff --git a/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java b/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
index ade99bc..9bfcde5 100644
--- a/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
+++ b/fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java
@@ -27,7 +27,7 @@ import org.apache.hadoop.hive.metastore.api.NoSuchObjectException;
import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.Table;
import org.apache.hadoop.hive.metastore.api.UnknownDBException;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.catalog.Function;
import org.apache.impala.catalog.HdfsPartition.FileDescriptor;
import org.apache.impala.common.Pair;
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 19021b0..b9eeece 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -1143,7 +1143,7 @@ public class CatalogOpExecutor {
*/
private void updateOwnerPrivileges(String databaseName, String tableName,
String serverName, String oldOwner, PrincipalType oldOwnerType, String newOwner,
- PrincipalType newOwnerType, TDdlExecResponse resp) {
+ PrincipalType newOwnerType, TDdlExecResponse resp) throws ImpalaException {
if (catalog_.getSentryProxy() == null || !catalog_.getSentryProxy()
.isObjectOwnershipEnabled()) return;
Preconditions.checkNotNull(serverName);
@@ -1983,7 +1983,7 @@ public class CatalogOpExecutor {
* Create a TPrivilege for an owner of a table for use as a filter.
*/
private TPrivilege createTableOwnerPrivilegeFilter(String databaseName,
- String tableName, String serverName) {
+ String tableName, String serverName) throws ImpalaException {
TPrivilege privilege = createDatabaseOwnerPrivilegeFilter(databaseName, serverName);
privilege.setScope(TPrivilegeScope.TABLE);
privilege.setTable_name(tableName);
@@ -1994,7 +1994,7 @@ public class CatalogOpExecutor {
* Create a TPrivilege for an owner of a database for use as a filter.
*/
private TPrivilege createDatabaseOwnerPrivilegeFilter(String databaseName,
- String serverName) {
+ String serverName) throws ImpalaException {
TPrivilege privilege = new TPrivilege();
privilege.setScope(TPrivilegeScope.DATABASE).setServer_name(serverName)
.setPrivilege_level(TPrivilegeLevel.OWNER)
@@ -3022,7 +3022,7 @@ public class CatalogOpExecutor {
}
private void alterTableOrViewSetOwner(Table tbl, TAlterTableOrViewSetOwnerParams params,
- TDdlExecResponse response) throws ImpalaRuntimeException {
+ TDdlExecResponse response) throws ImpalaException {
org.apache.hadoop.hive.metastore.api.Table msTbl = tbl.getMetaStoreTable().deepCopy();
String oldOwner = msTbl.getOwner();
PrincipalType oldOwnerType = msTbl.getOwnerType();
@@ -3443,7 +3443,7 @@ public class CatalogOpExecutor {
/**
* Checks if with grant is enabled for object ownership in Sentry.
*/
- private boolean isObjectOwnershipGrantEnabled() {
+ private boolean isObjectOwnershipGrantEnabled() throws ImpalaException {
return catalog_.getSentryProxy() == null ? false :
catalog_.getSentryProxy().isObjectOwnershipGrantEnabled();
}
@@ -3913,7 +3913,7 @@ public class CatalogOpExecutor {
}
private void alterDatabase(TAlterDbParams params, TDdlExecResponse response)
- throws CatalogException, ImpalaRuntimeException {
+ throws ImpalaException {
switch (params.getAlter_type()) {
case SET_OWNER:
alterDatabaseSetOwner(params.getDb(), params.getSet_owner_params(), response);
@@ -3925,7 +3925,7 @@ public class CatalogOpExecutor {
}
private void alterDatabaseSetOwner(String dbName, TAlterDbSetOwnerParams params,
- TDdlExecResponse response) throws CatalogException, ImpalaRuntimeException {
+ TDdlExecResponse response) throws ImpalaException {
Db db = catalog_.getDb(dbName);
if (db == null) {
throw new CatalogException("Database: " + dbName + " does not exist.");
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 170cbb9..2e61472 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -65,12 +65,14 @@ import org.apache.impala.analysis.TableName;
import org.apache.impala.analysis.TruncateStmt;
import org.apache.impala.authorization.AuthorizationChecker;
import org.apache.impala.authorization.AuthorizationConfig;
-import org.apache.impala.authorization.AuthorizeableTable;
+import org.apache.impala.authorization.AuthorizationProvider;
import org.apache.impala.authorization.ImpalaInternalAdminUser;
import org.apache.impala.authorization.Privilege;
import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.authorization.User;
+import org.apache.impala.authorization.sentry.SentryAuthorizationChecker;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.Column;
@@ -253,21 +255,25 @@ public class Frontend {
authzConfig_ = authorizationConfig;
catalogManager_ = catalogManager;
- // Load the authorization policy once at startup, initializing
- // authzChecker_. This ensures that, if the policy fails to load,
- // we will throw an exception and fail to start.
- AuthorizationPolicyReader policyReaderTask =
- new AuthorizationPolicyReader(authzConfig_);
- policyReaderTask.run();
-
- // If authorization is enabled, reload the policy on a regular basis.
- if (authzConfig_.isEnabled() && authzConfig_.isFileBasedPolicy()) {
- // Stagger the reads across nodes
- Random randomGen = new Random(UUID.randomUUID().hashCode());
- int delay = AUTHORIZATION_POLICY_RELOAD_INTERVAL_SECS + randomGen.nextInt(60);
-
- policyReader_.scheduleAtFixedRate(policyReaderTask,
- delay, AUTHORIZATION_POLICY_RELOAD_INTERVAL_SECS, TimeUnit.SECONDS);
+ if (authzConfig_.getProvider() == AuthorizationProvider.SENTRY) {
+ SentryAuthorizationConfig sentryAuthzConfig =
+ (SentryAuthorizationConfig) authzConfig_;
+ // Load the authorization policy once at startup, initializing
+ // authzChecker_. This ensures that, if the policy fails to load,
+ // we will throw an exception and fail to start.
+ AuthorizationPolicyReader policyReaderTask =
+ new AuthorizationPolicyReader(sentryAuthzConfig);
+ policyReaderTask.run();
+
+ // If authorization is enabled, reload the policy on a regular basis.
+ if (sentryAuthzConfig.isEnabled() && sentryAuthzConfig.isFileBasedPolicy()) {
+ // Stagger the reads across nodes
+ Random randomGen = new Random(UUID.randomUUID().hashCode());
+ int delay = AUTHORIZATION_POLICY_RELOAD_INTERVAL_SECS + randomGen.nextInt(60);
+
+ policyReader_.scheduleAtFixedRate(policyReaderTask,
+ delay, AUTHORIZATION_POLICY_RELOAD_INTERVAL_SECS, TimeUnit.SECONDS);
+ }
}
impaladTableUsageTracker_ = ImpaladTableUsageTracker.createFromConfig(
BackendConfig.INSTANCE);
@@ -277,9 +283,9 @@ public class Frontend {
* Reads (and caches) an authorization policy from HDFS.
*/
private class AuthorizationPolicyReader implements Runnable {
- private final AuthorizationConfig config_;
+ private final SentryAuthorizationConfig config_;
- public AuthorizationPolicyReader(AuthorizationConfig config) {
+ public AuthorizationPolicyReader(SentryAuthorizationConfig config) {
config_ = config;
}
@@ -287,7 +293,7 @@ public class Frontend {
public void run() {
try {
LOG.info("Reloading authorization policy file from: " + config_.getPolicyFile());
- authzChecker_.set(new AuthorizationChecker(config_,
+ authzChecker_.set(new SentryAuthorizationChecker(config_,
getCatalog().getAuthPolicy()));
} catch (Exception e) {
LOG.error("Error reloading policy file: ", e);
@@ -309,7 +315,7 @@ public class Frontend {
if (!req.is_delta) {
// In the case that it was a non-delta update, the catalog might have reloaded
// itself, and we need to reset the AuthorizationChecker accordingly.
- authzChecker_.set(new AuthorizationChecker(
+ authzChecker_.set(new SentryAuthorizationChecker(
authzConfig_, getCatalog().getAuthPolicy()));
}
return resp;
@@ -783,7 +789,7 @@ public class Frontend {
while (iter.hasNext()) {
String tblName = iter.next();
PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
- .any().onAnyColumn(dbName, tblName).toRequest();
+ .any().onAnyColumn(dbName, tblName).build();
if (!authzChecker_.get().hasAccess(user, privilegeRequest)) {
iter.remove();
}
@@ -807,7 +813,7 @@ public class Frontend {
if (authzConfig_.isEnabled()) {
PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
.any().onColumn(table.getTableName().getDb(), table.getTableName().getTbl(),
- colName).toRequest();
+ colName).build();
if (!authzChecker_.get().hasAccess(user, privilegeRequest)) continue;
}
columns.add(column);
@@ -844,7 +850,7 @@ public class Frontend {
return true;
}
PrivilegeRequest request = new PrivilegeRequestBuilder()
- .any().onAnyColumn(db.getName(), AuthorizeableTable.ANY_TABLE_NAME).toRequest();
+ .any().onAnyColumn(db.getName()).build();
return authzChecker_.get().hasAccess(user, request);
}
@@ -1017,7 +1023,7 @@ public class Frontend {
PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
.allOf(Privilege.VIEW_METADATA).onTable(table.getDb().getName(),
table.getName())
- .toRequest();
+ .build();
if (!authzChecker_.get().hasAccess(user, privilegeRequest)) {
// Filter out columns that the user is not authorized to see.
filteredColumns = new ArrayList<Column>();
@@ -1026,7 +1032,7 @@ public class Frontend {
privilegeRequest = new PrivilegeRequestBuilder()
.allOf(Privilege.VIEW_METADATA)
.onColumn(table.getDb().getName(), table.getName(), colName)
- .toRequest();
+ .build();
if (authzChecker_.get().hasAccess(user, privilegeRequest)) {
filteredColumns.add(col);
}
@@ -1055,7 +1061,7 @@ public class Frontend {
PrivilegeRequest privilegeRequest = new PrivilegeRequestBuilder()
.allOf(Privilege.VIEW_METADATA)
.onTable(table.getDb().getName(),table.getName())
- .toRequest();
+ .build();
// Only filter if the user doesn't have table access.
if (!authzChecker_.get().hasAccess(user, privilegeRequest)) {
List<TResultRow> results = new ArrayList<TResultRow>();
diff --git a/fe/src/main/java/org/apache/impala/service/JniCatalog.java b/fe/src/main/java/org/apache/impala/service/JniCatalog.java
index 457aa78..6e29f42 100644
--- a/fe/src/main/java/org/apache/impala/service/JniCatalog.java
+++ b/fe/src/main/java/org/apache/impala/service/JniCatalog.java
@@ -26,7 +26,8 @@ import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
-import org.apache.impala.authorization.SentryConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryConfig;
import org.apache.impala.authorization.User;
import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.CatalogServiceCatalog;
@@ -107,7 +108,7 @@ public class JniCatalog {
TLogLevel.values()[cfg.non_impala_java_vlog]);
// Check if the Sentry Service is configured. If so, create a configuration object.
- SentryConfig sentryConfig = null;
+ SentryConfig sentryConfig = new SentryConfig(null);
if (!Strings.isNullOrEmpty(cfg.sentry_config)) {
sentryConfig = new SentryConfig(cfg.sentry_config);
sentryConfig.loadConfig();
@@ -115,8 +116,9 @@ public class JniCatalog {
LOG.info(JniUtil.getJavaVersion());
catalog_ = new CatalogServiceCatalog(cfg.load_catalog_in_background,
- cfg.num_metadata_loading_threads, cfg.initial_hms_cnxn_timeout_s, sentryConfig,
- getServiceId(), cfg.principal, cfg.local_library_path);
+ cfg.num_metadata_loading_threads, cfg.initial_hms_cnxn_timeout_s,
+ new SentryAuthorizationConfig(sentryConfig), getServiceId(), cfg.principal,
+ cfg.local_library_path);
try {
catalog_.reset();
} catch (CatalogException e) {
diff --git a/fe/src/main/java/org/apache/impala/service/JniFrontend.java b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
index 0cf5390..1facbe8 100644
--- a/fe/src/main/java/org/apache/impala/service/JniFrontend.java
+++ b/fe/src/main/java/org/apache/impala/service/JniFrontend.java
@@ -44,9 +44,9 @@ import org.apache.hadoop.security.ShellBasedUnixGroupsMapping;
import org.apache.hadoop.security.ShellBasedUnixGroupsNetgroupMapping;
import org.apache.impala.analysis.DescriptorTable;
import org.apache.impala.analysis.ToSqlUtils;
-import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.authorization.ImpalaInternalAdminUser;
import org.apache.impala.authorization.User;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.FeDataSource;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.Function;
@@ -139,7 +139,7 @@ public class JniFrontend {
// Validate the authorization configuration before initializing the Frontend.
// If there are any configuration problems Impala startup will fail.
- AuthorizationConfig authConfig = new AuthorizationConfig(cfg.server_name,
+ SentryAuthorizationConfig authConfig = new SentryAuthorizationConfig(cfg.server_name,
cfg.authorization_policy_file, cfg.sentry_config,
cfg.authorization_policy_provider_class);
if (authConfig.isEnabled()) {
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
index be47a08..994b872 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
@@ -19,7 +19,7 @@ package org.apache.impala.analysis;
import java.util.HashSet;
-import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Role;
import org.apache.impala.catalog.User;
@@ -67,7 +67,7 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
defaultDb, System.getProperty("user.name"));
EventSequence timeline = new EventSequence("Authorization Test");
AnalysisContext analysisCtx = new AnalysisContext(queryCtx,
- AuthorizationConfig.createHadoopGroupAuthConfig("server1", null, null),
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null, null),
timeline);
return analysisCtx;
}
@@ -77,7 +77,7 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
Catalog.DEFAULT_DB, System.getProperty("user.name"));
EventSequence timeline = new EventSequence("Authorization Test");
AnalysisContext analysisCtx = new AnalysisContext(queryCtx,
- AuthorizationConfig.createAuthDisabledConfig(), timeline);
+ SentryAuthorizationConfig.createAuthDisabledConfig(), timeline);
return analysisCtx;
}
@@ -308,7 +308,7 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
Catalog.DEFAULT_DB, "");
EventSequence timeline = new EventSequence("Authorization Test");
AnalysisContext noUserNameCtx = new AnalysisContext(noUserNameQueryCtx,
- AuthorizationConfig.createHadoopGroupAuthConfig("server1", null, null),
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null, null),
timeline);
AnalysisError("GRANT ALL ON SERVER TO myRole", noUserNameCtx,
"Cannot execute authorization statement with an empty username.");
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
index 680efa9..a2e8c57 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java
@@ -19,8 +19,8 @@ package org.apache.impala.analysis;
import java.util.Set;
-import org.apache.impala.authorization.AuthorizationConfig;
-import org.apache.impala.catalog.AuthorizationException;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
+import org.apache.impala.authorization.AuthorizationException;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.common.AnalysisException;
@@ -367,9 +367,10 @@ public class AuditingTest extends FrontendTestBase {
public void TestAccessEventsOnAuthFailure() throws ImpalaException {
// The policy file doesn't exist so all operations will result in
// an AuthorizationError
- AuthorizationConfig config = AuthorizationConfig.createHadoopGroupAuthConfig(
- "server1", "/does/not/exist", System.getenv("IMPALA_HOME") +
- "/fe/src/test/resources/sentry-site.xml");
+ SentryAuthorizationConfig config =
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1",
+ "/does/not/exist", System.getenv("IMPALA_HOME") +
+ "/fe/src/test/resources/sentry-site.xml");
try (ImpaladCatalog catalog = new ImpaladTestCatalog(config)) {
Frontend fe = new Frontend(config, catalog);
AnalysisContext analysisCtx = createAnalysisCtx(config);
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
index d2e7a2a..19befb7 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -29,10 +29,11 @@ import java.util.Set;
import org.apache.commons.lang.ArrayUtils;
import org.apache.impala.analysis.AnalysisContext.AnalysisResult;
-import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.User;
-import org.apache.impala.catalog.AuthorizationException;
+import org.apache.impala.authorization.AuthorizationException;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryPolicyService;
import org.apache.impala.catalog.Role;
import org.apache.impala.catalog.ScalarFunction;
import org.apache.impala.catalog.Type;
@@ -53,7 +54,6 @@ import org.apache.impala.thrift.TPrivilegeScope;
import org.apache.impala.thrift.TQueryOptions;
import org.apache.impala.thrift.TResultRow;
import org.apache.impala.thrift.TTableName;
-import org.apache.impala.util.SentryPolicyService;
import org.apache.sentry.api.service.thrift.TSentryRole;
import org.junit.AfterClass;
import org.junit.Before;
@@ -75,7 +75,7 @@ public class AuthorizationStmtTest extends FrontendTestBase {
private final Frontend authzFrontend_;
public AuthorizationStmtTest() {
- AuthorizationConfig authzConfig = createAuthorizationConfig();
+ SentryAuthorizationConfig authzConfig = createAuthorizationConfig();
analysisContext_ = createAnalysisCtx(authzConfig, USER.getName());
authzCatalog_ = new ImpaladTestCatalog(authzConfig);
authzFrontend_ = new Frontend(authzConfig, authzCatalog_);
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
index 15a06ee..3661fbb 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
@@ -34,9 +34,10 @@ import org.apache.hive.service.rpc.thrift.TGetColumnsReq;
import org.apache.hive.service.rpc.thrift.TGetSchemasReq;
import org.apache.hive.service.rpc.thrift.TGetTablesReq;
import org.apache.impala.authorization.AuthorizationConfig;
-import org.apache.impala.authorization.AuthorizeableTable;
import org.apache.impala.authorization.User;
-import org.apache.impala.catalog.AuthorizationException;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
+import org.apache.impala.authorization.AuthorizationException;
+import org.apache.impala.authorization.sentry.SentryPolicyService;
import org.apache.impala.catalog.FeDb;
import org.apache.impala.catalog.ImpaladCatalog;
import org.apache.impala.common.AnalysisException;
@@ -55,7 +56,7 @@ import org.apache.impala.thrift.TPrivilegeScope;
import org.apache.impala.thrift.TResultSet;
import org.apache.impala.thrift.TSessionState;
import org.apache.impala.util.PatternMatcher;
-import org.apache.impala.util.SentryPolicyService;
+import org.apache.sentry.core.model.db.AccessConstants;
import org.apache.sentry.provider.common.ResourceAuthorizationProvider;
import org.apache.sentry.provider.file.LocalGroupResourceAuthorizationProvider;
import org.junit.After;
@@ -110,9 +111,11 @@ public class AuthorizationTest extends FrontendTestBase {
* Test context whose instances are used to parameterize this test.
*/
private static class TestContext {
- public final AuthorizationConfig authzConfig;
+ public final SentryAuthorizationConfig authzConfig;
public final ImpaladTestCatalog catalog;
- public TestContext(AuthorizationConfig authzConfig, ImpaladTestCatalog catalog) {
+
+ public TestContext(SentryAuthorizationConfig authzConfig,
+ ImpaladTestCatalog catalog) {
this.authzConfig = authzConfig;
this.catalog = catalog;
}
@@ -140,12 +143,12 @@ public class AuthorizationTest extends FrontendTestBase {
static {
testCtxs_ = new ArrayList<>();
// Create and init file based auth config.
- AuthorizationConfig filePolicyAuthzConfig = createPolicyFileAuthzConfig();
+ SentryAuthorizationConfig filePolicyAuthzConfig = createPolicyFileAuthzConfig();
ImpaladTestCatalog filePolicyCatalog = new ImpaladTestCatalog(filePolicyAuthzConfig);
testCtxs_.add(new TestContext(filePolicyAuthzConfig, filePolicyCatalog));
// Create and init sentry service based auth config.
- AuthorizationConfig sentryServiceAuthzConfig;
+ SentryAuthorizationConfig sentryServiceAuthzConfig;
try {
sentryServiceAuthzConfig = createSentryServiceAuthzConfig();
} catch (ImpalaException e) {
@@ -175,23 +178,24 @@ public class AuthorizationTest extends FrontendTestBase {
fe_ = new Frontend(ctx_.authzConfig, ctx_.catalog);
}
- public static AuthorizationConfig createPolicyFileAuthzConfig() {
- AuthorizationConfig result =
- AuthorizationConfig.createHadoopGroupAuthConfig("server1", AUTHZ_POLICY_FILE,
- System.getenv("IMPALA_HOME") + "/fe/src/test/resources/sentry-site.xml");
+ public static SentryAuthorizationConfig createPolicyFileAuthzConfig() {
+ SentryAuthorizationConfig result =
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1",
+ AUTHZ_POLICY_FILE,
+ System.getenv("IMPALA_HOME") + "/fe/src/test/resources/sentry-site.xml");
return result;
}
- public static AuthorizationConfig createSentryServiceAuthzConfig()
+ public static SentryAuthorizationConfig createSentryServiceAuthzConfig()
throws ImpalaException {
- AuthorizationConfig result =
- AuthorizationConfig.createHadoopGroupAuthConfig("server1", null,
+ SentryAuthorizationConfig result =
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null,
System.getenv("IMPALA_HOME") + "/fe/src/test/resources/sentry-site.xml");
setupSentryService(result);
return result;
}
- private static void setupSentryService(AuthorizationConfig authzConfig)
+ private static void setupSentryService(SentryAuthorizationConfig authzConfig)
throws ImpalaException {
SentryPolicyService sentryService = new SentryPolicyService(
authzConfig.getSentryConfig());
@@ -226,7 +230,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.INSERT, TPrivilegeScope.TABLE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional_parquet");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// refresh_functional_text_lzo
@@ -237,7 +241,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.REFRESH, TPrivilegeScope.DATABASE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional_text_lzo");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// refresh_functional_alltypesagg
@@ -292,7 +296,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.INSERT, TPrivilegeScope.DATABASE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional_text_lzo");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// create_functional_text_lzo
@@ -303,7 +307,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.CREATE, TPrivilegeScope.DATABASE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional_text_lzo");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// alter_functional_text_lzo
@@ -314,7 +318,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.ALTER, TPrivilegeScope.DATABASE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional_text_lzo");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// drop_functional_text_lzo
@@ -325,7 +329,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.DROP, TPrivilegeScope.DATABASE, false);
privilege.setServer_name("server1");
privilege.setDb_name("functional_text_lzo");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// alter_functional_alltypeserror
@@ -363,19 +367,19 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.ALL, TPrivilegeScope.URI, false);
privilege.setServer_name("server1");
privilege.setUri("hdfs://localhost:20500/test-warehouse/new_table");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
privilege = new TPrivilege(TPrivilegeLevel.ALL, TPrivilegeScope.URI, false);
privilege.setServer_name("server1");
privilege.setUri("hdfs://localhost:20500/test-warehouse/UPPER_CASE");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
privilege = new TPrivilege(TPrivilegeLevel.ALL, TPrivilegeScope.URI, false);
privilege.setServer_name("server1");
privilege.setUri("hdfs://localhost:20500/test-warehouse/libTestUdfs.so");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// all tpch
@@ -400,7 +404,7 @@ public class AuthorizationTest extends FrontendTestBase {
privilege = new TPrivilege(TPrivilegeLevel.SELECT, TPrivilegeScope.TABLE, false);
privilege.setServer_name("server1");
privilege.setDb_name("tpcds");
- privilege.setTable_name(AuthorizeableTable.ANY_TABLE_NAME);
+ privilege.setTable_name(AccessConstants.ALL);
sentryService.grantRolePrivilege(USER, roleName, privilege);
// select_functional_alltypesagg
@@ -520,8 +524,8 @@ public class AuthorizationTest extends FrontendTestBase {
@Test
public void TestSentryService() throws ImpalaException {
- SentryPolicyService sentryService =
- new SentryPolicyService(ctx_.authzConfig.getSentryConfig());
+ SentryPolicyService sentryService = new SentryPolicyService(
+ ctx_.authzConfig.getSentryConfig());
String roleName = "testRoleName";
roleName = roleName.toLowerCase();
@@ -838,7 +842,7 @@ public class AuthorizationTest extends FrontendTestBase {
// DEFAULT
// </value>
// </property>
- AuthorizationConfig authzConfig = new AuthorizationConfig("server1",
+ SentryAuthorizationConfig authzConfig = new SentryAuthorizationConfig("server1",
AUTHZ_POLICY_FILE, ctx_.authzConfig.getSentryConfig().getConfigFile(),
LocalGroupResourceAuthorizationProvider.class.getName());
try (ImpaladCatalog catalog = new ImpaladTestCatalog(authzConfig)) {
@@ -900,7 +904,7 @@ public class AuthorizationTest extends FrontendTestBase {
public void TestServerNameAuthorized() throws ImpalaException {
if (ctx_.authzConfig.isFileBasedPolicy()) {
// Authorization config that has a different server name from policy file.
- TestWithIncorrectConfig(AuthorizationConfig.createHadoopGroupAuthConfig(
+ TestWithIncorrectConfig(SentryAuthorizationConfig.createHadoopGroupAuthConfig(
"differentServerName", AUTHZ_POLICY_FILE,
ctx_.authzConfig.getSentryConfig().getConfigFile()),
new User(System.getProperty("user.name")));
@@ -915,8 +919,8 @@ public class AuthorizationTest extends FrontendTestBase {
// Validate a non-existent policy file.
// Use a HadoopGroupProvider in this case so the user -> group mappings can still be
// resolved in the absence of the policy file.
- TestWithIncorrectConfig(AuthorizationConfig.createHadoopGroupAuthConfig("server1",
- AUTHZ_POLICY_FILE + "_does_not_exist",
+ TestWithIncorrectConfig(SentryAuthorizationConfig.createHadoopGroupAuthConfig(
+ "server1", AUTHZ_POLICY_FILE + "_does_not_exist",
ctx_.authzConfig.getSentryConfig().getConfigFile()),
new User(System.getProperty("user.name")));
}
@@ -925,12 +929,13 @@ public class AuthorizationTest extends FrontendTestBase {
public void TestConfigValidation() throws InternalException {
String sentryConfig = ctx_.authzConfig.getSentryConfig().getConfigFile();
// Valid configs pass validation.
- AuthorizationConfig config = AuthorizationConfig.createHadoopGroupAuthConfig(
- "server1", AUTHZ_POLICY_FILE, sentryConfig);
+ SentryAuthorizationConfig config =
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1",
+ AUTHZ_POLICY_FILE, sentryConfig);
Assert.assertTrue(config.isEnabled());
Assert.assertTrue(config.isFileBasedPolicy());
- config = AuthorizationConfig.createHadoopGroupAuthConfig("server1", null,
+ config = SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null,
sentryConfig);
Assert.assertTrue(config.isEnabled());
Assert.assertTrue(!config.isFileBasedPolicy());
@@ -938,7 +943,7 @@ public class AuthorizationTest extends FrontendTestBase {
// Invalid configs
// No sentry configuration file.
try {
- config = AuthorizationConfig.createHadoopGroupAuthConfig(
+ config = SentryAuthorizationConfig.createHadoopGroupAuthConfig(
"server1", AUTHZ_POLICY_FILE, null);
Assert.assertTrue(config.isEnabled());
} catch (Exception e) {
@@ -949,7 +954,7 @@ public class AuthorizationTest extends FrontendTestBase {
// Empty / null server name.
try {
- config = AuthorizationConfig.createHadoopGroupAuthConfig(
+ config = SentryAuthorizationConfig.createHadoopGroupAuthConfig(
"", AUTHZ_POLICY_FILE, sentryConfig);
Assert.assertTrue(config.isEnabled());
fail("Expected configuration to fail.");
@@ -960,8 +965,8 @@ public class AuthorizationTest extends FrontendTestBase {
e.getMessage());
}
try {
- config = AuthorizationConfig.createHadoopGroupAuthConfig(null, AUTHZ_POLICY_FILE,
- sentryConfig);
+ config = SentryAuthorizationConfig.createHadoopGroupAuthConfig(null,
+ AUTHZ_POLICY_FILE, sentryConfig);
Assert.assertTrue(config.isEnabled());
fail("Expected configuration to fail.");
} catch (IllegalArgumentException e) {
@@ -973,7 +978,7 @@ public class AuthorizationTest extends FrontendTestBase {
// Sentry config file does not exist.
try {
- config = AuthorizationConfig.createHadoopGroupAuthConfig("server1", "",
+ config = SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", "",
"/path/does/not/exist.xml");
Assert.assertTrue(config.isEnabled());
fail("Expected configuration to fail.");
@@ -985,7 +990,7 @@ public class AuthorizationTest extends FrontendTestBase {
// Invalid ResourcePolicyProvider class name.
try {
- config = new AuthorizationConfig("server1", AUTHZ_POLICY_FILE, sentryConfig,
+ config = new SentryAuthorizationConfig("server1", AUTHZ_POLICY_FILE, sentryConfig,
"ClassDoesNotExist");
Assert.assertTrue(config.isEnabled()); fail("Expected configuration to fail.");
} catch (IllegalArgumentException e) {
@@ -996,7 +1001,7 @@ public class AuthorizationTest extends FrontendTestBase {
// Valid class name, but class is not derived from ResourcePolicyProvider
try {
- config = new AuthorizationConfig("server1", AUTHZ_POLICY_FILE, sentryConfig,
+ config = new SentryAuthorizationConfig("server1", AUTHZ_POLICY_FILE, sentryConfig,
this.getClass().getName());
Assert.assertTrue(config.isEnabled()); fail("Expected configuration to fail.");
} catch (IllegalArgumentException e) {
@@ -1007,13 +1012,13 @@ public class AuthorizationTest extends FrontendTestBase {
}
// Config validations skipped if authorization disabled
- config = new AuthorizationConfig("", "", "", "");
+ config = new SentryAuthorizationConfig("", "", "", "");
Assert.assertFalse(config.isEnabled());
- config = new AuthorizationConfig(null, "", "", null);
+ config = new SentryAuthorizationConfig(null, "", "", null);
Assert.assertFalse(config.isEnabled());
- config = new AuthorizationConfig("", null, "", "");
+ config = new SentryAuthorizationConfig("", null, "", "");
Assert.assertFalse(config.isEnabled());
- config = new AuthorizationConfig(null, null, null, null);
+ config = new SentryAuthorizationConfig(null, null, null, null);
Assert.assertFalse(config.isEnabled());
}
@@ -1022,7 +1027,7 @@ public class AuthorizationTest extends FrontendTestBase {
if (!ctx_.authzConfig.isFileBasedPolicy()) return;
// Use an authorization configuration that uses the
// LocalGroupResourceAuthorizationProvider.
- AuthorizationConfig authzConfig = new AuthorizationConfig("server1",
+ SentryAuthorizationConfig authzConfig = new SentryAuthorizationConfig("server1",
AUTHZ_POLICY_FILE, ctx_.authzConfig.getSentryConfig().getConfigFile(),
LocalGroupResourceAuthorizationProvider.class.getName());
try (ImpaladCatalog catalog = new ImpaladTestCatalog(authzConfig)) {
@@ -1118,8 +1123,4 @@ public class AuthorizationTest extends FrontendTestBase {
return new TSessionState(null, null,
defaultDb, user.getName(), new TNetworkAddress("", 0));
}
-
- private SentryPolicyService createSentryService() {
- return new SentryPolicyService(ctx_.authzConfig.getSentryConfig());
- }
}
diff --git a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
index 1b8556e..08ab02c 100644
--- a/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
@@ -20,7 +20,7 @@ package org.apache.impala.analysis;
import java.util.Arrays;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
-import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.FeTable;
import org.apache.impala.common.ImpalaException;
@@ -38,7 +38,8 @@ public class StmtMetadataLoaderTest {
String[] expectedDbs, String[] expectedTables)
throws ImpalaException {
try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) {
- Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
+ Frontend fe = new Frontend(SentryAuthorizationConfig.createAuthDisabledConfig(),
+ catalog);
StatementBase stmt = Parser.parse(stmtStr);
// Catalog is fresh and no tables are cached.
validateUncached(stmt, fe, expectedNumLoadRequests, expectedNumCatalogUpdates,
@@ -50,7 +51,8 @@ public class StmtMetadataLoaderTest {
private void testNoLoad(String stmtStr) throws ImpalaException {
try (ImpaladTestCatalog catalog = new ImpaladTestCatalog()) {
- Frontend fe = new Frontend(AuthorizationConfig.createAuthDisabledConfig(), catalog);
+ Frontend fe = new Frontend(SentryAuthorizationConfig.createAuthDisabledConfig(),
+ catalog);
StatementBase stmt = Parser.parse(stmtStr);
validateCached(stmt, fe, new String[]{}, new String[]{});
}
diff --git a/fe/src/test/java/org/apache/impala/authorization/ImpalaActionFactoryTest.java b/fe/src/test/java/org/apache/impala/authorization/sentry/ImpalaActionFactoryTest.java
similarity index 96%
rename from fe/src/test/java/org/apache/impala/authorization/ImpalaActionFactoryTest.java
rename to fe/src/test/java/org/apache/impala/authorization/sentry/ImpalaActionFactoryTest.java
index 9d0ecea..2961705 100644
--- a/fe/src/test/java/org/apache/impala/authorization/ImpalaActionFactoryTest.java
+++ b/fe/src/test/java/org/apache/impala/authorization/sentry/ImpalaActionFactoryTest.java
@@ -14,10 +14,11 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.authorization;
+package org.apache.impala.authorization.sentry;
import com.google.common.collect.Lists;
-import org.apache.impala.authorization.Privilege.ImpalaAction;
+import org.apache.impala.authorization.sentry.ImpalaAction;
+import org.apache.impala.authorization.sentry.ImpalaActionFactory;
import org.apache.sentry.core.common.BitFieldAction;
import org.junit.Test;
diff --git a/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java b/fe/src/test/java/org/apache/impala/authorization/sentry/SentryProxyTest.java
similarity index 98%
rename from fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
rename to fe/src/test/java/org/apache/impala/authorization/sentry/SentryProxyTest.java
index 9d24f62..d09f0fa 100644
--- a/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
+++ b/fe/src/test/java/org/apache/impala/authorization/sentry/SentryProxyTest.java
@@ -15,13 +15,11 @@
// specific language governing permissions and limitations
// under the License.
-package org.apache.impala.util;
+package org.apache.impala.authorization.sentry;
import com.google.common.base.Preconditions;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
-import org.apache.impala.authorization.AuthorizationConfig;
-import org.apache.impala.authorization.SentryConfig;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.catalog.Role;
import org.apache.impala.catalog.User;
@@ -32,7 +30,7 @@ import org.apache.impala.thrift.TPrincipalType;
import org.apache.impala.thrift.TPrivilege;
import org.apache.impala.thrift.TPrivilegeLevel;
import org.apache.impala.thrift.TPrivilegeScope;
-import org.apache.impala.util.SentryProxy.AuthorizationDelta;
+import org.apache.impala.authorization.sentry.SentryProxy.AuthorizationDelta;
import org.apache.sentry.api.service.thrift.TSentryPrivilege;
import org.apache.sentry.api.service.thrift.TSentryRole;
import org.junit.After;
@@ -55,10 +53,10 @@ public class SentryProxyTest {
private static final org.apache.impala.authorization.User USER =
new org.apache.impala.authorization.User(System.getProperty("user.name"));
private final SentryPolicyService sentryService_;
- private final AuthorizationConfig authzConfig_;
+ private final SentryAuthorizationConfig authzConfig_;
public SentryProxyTest() {
- authzConfig_ = AuthorizationConfig.createHadoopGroupAuthConfig(
+ authzConfig_ = SentryAuthorizationConfig.createHadoopGroupAuthConfig(
SENTRY_SERVER, null, System.getenv("IMPALA_HOME") +
"/fe/src/test/resources/sentry-site.xml");
sentryService_ = new SentryPolicyService(authzConfig_.getSentryConfig());
diff --git a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
index d6ca5b8..b7caea0 100644
--- a/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
@@ -49,6 +49,7 @@ import org.apache.hadoop.hive.metastore.api.ColumnStatisticsData;
import org.apache.impala.analysis.FunctionName;
import org.apache.impala.analysis.LiteralExpr;
import org.apache.impala.analysis.NumericLiteral;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.catalog.MetaStoreClientPool.MetaStoreClient;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.Reference;
diff --git a/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java b/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
index be891f5..e6977ce 100644
--- a/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
+++ b/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java
@@ -44,7 +44,8 @@ import org.apache.hadoop.hive.metastore.api.PrincipalType;
import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
import org.apache.hadoop.hive.metastore.client.builder.PartitionBuilder;
import org.apache.hadoop.hive.metastore.client.builder.TableBuilder;
-import org.apache.impala.authorization.SentryConfig;
+import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryConfig;
import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.catalog.DatabaseNotFoundException;
@@ -796,10 +797,10 @@ public class MetastoreEventsProcessorTest {
private String tblName_;
private FakeCatalogServiceCatalogForFlagTests(boolean loadInBackground,
- int numLoadingThreads, SentryConfig sentryConfig, TUniqueId catalogServiceId,
+ int numLoadingThreads, AuthorizationConfig authConfig, TUniqueId catalogServiceId,
String kerberosPrincipal, String localLibraryPath,
MetaStoreClientPool metaStoreClientPool) throws ImpalaException {
- super(loadInBackground, numLoadingThreads, sentryConfig, catalogServiceId,
+ super(loadInBackground, numLoadingThreads, authConfig, catalogServiceId,
kerberosPrincipal, localLibraryPath, metaStoreClientPool);
}
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendFixture.java b/fe/src/test/java/org/apache/impala/common/FrontendFixture.java
index 1f696f0..0c8e648 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendFixture.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendFixture.java
@@ -37,6 +37,7 @@ import org.apache.impala.analysis.StatementBase;
import org.apache.impala.analysis.StmtMetadataLoader;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.AggregateFunction;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.CatalogException;
@@ -91,7 +92,7 @@ public class FrontendFixture {
// The actual Impala frontend that backs this fixture.
protected final Frontend frontend_ = new Frontend(
- AuthorizationConfig.createAuthDisabledConfig(), catalog_);
+ SentryAuthorizationConfig.createAuthDisabledConfig(), catalog_);
// Test-local list of test databases and tables.
protected final List<Db> testDbs_ = new ArrayList<>();
@@ -293,7 +294,7 @@ public class FrontendFixture {
defaultDb, System.getProperty("user.name"));
EventSequence timeline = new EventSequence("Frontend Test Timeline");
AnalysisContext analysisCtx = new AnalysisContext(queryCtx,
- AuthorizationConfig.createAuthDisabledConfig(), timeline);
+ SentryAuthorizationConfig.createAuthDisabledConfig(), timeline);
return analysisCtx;
}
@@ -302,7 +303,7 @@ public class FrontendFixture {
queryCtx.client_request.query_options = queryOptions;
EventSequence timeline = new EventSequence("Frontend Test Timeline");
AnalysisContext analysisCtx = new AnalysisContext(queryCtx,
- AuthorizationConfig.createAuthDisabledConfig(), timeline);
+ SentryAuthorizationConfig.createAuthDisabledConfig(), timeline);
return analysisCtx;
}
diff --git a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
index 837fd8c..d7d49c0 100644
--- a/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
+++ b/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
@@ -31,6 +31,7 @@ import org.apache.impala.analysis.StatementBase;
import org.apache.impala.analysis.StmtMetadataLoader;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.catalog.Db;
import org.apache.impala.catalog.Function;
@@ -286,10 +287,10 @@ public class FrontendTestBase extends AbstractFrontendTest {
* Creates an authorization config for creating an AnalysisContext with
* authorization enabled.
*/
- protected AuthorizationConfig createAuthorizationConfig() {
- AuthorizationConfig authzConfig = AuthorizationConfig.createHadoopGroupAuthConfig(
- "server1", null, System.getenv("IMPALA_HOME") +
- "/fe/src/test/resources/sentry-site.xml");
+ protected SentryAuthorizationConfig createAuthorizationConfig() {
+ SentryAuthorizationConfig authzConfig =
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null,
+ System.getenv("IMPALA_HOME") + "/fe/src/test/resources/sentry-site.xml");
return authzConfig;
}
}
diff --git a/fe/src/test/java/org/apache/impala/common/QueryFixture.java b/fe/src/test/java/org/apache/impala/common/QueryFixture.java
index 88c9d57..70db975 100644
--- a/fe/src/test/java/org/apache/impala/common/QueryFixture.java
+++ b/fe/src/test/java/org/apache/impala/common/QueryFixture.java
@@ -33,7 +33,7 @@ import org.apache.impala.analysis.SqlScanner;
import org.apache.impala.analysis.StatementBase;
import org.apache.impala.analysis.StmtMetadataLoader;
import org.apache.impala.analysis.StmtMetadataLoader.StmtTableCache;
-import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.testutil.TestUtils;
import org.apache.impala.thrift.TQueryCtx;
import org.apache.impala.thrift.TQueryOptions;
@@ -95,7 +95,7 @@ public class QueryFixture {
protected AnalysisContext makeAnalysisContext() {
EventSequence timeline = new EventSequence("Frontend Test Timeline");
return new AnalysisContext(queryCtx_,
- AuthorizationConfig.createAuthDisabledConfig(), timeline);
+ SentryAuthorizationConfig.createAuthDisabledConfig(), timeline);
}
/**
diff --git a/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java b/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
index ba36fdc..090d730 100644
--- a/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
+++ b/fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
@@ -17,8 +17,10 @@
package org.apache.impala.testutil;
-import org.apache.impala.authorization.SentryConfig;
-import org.apache.impala.catalog.AuthorizationPolicy;
+import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryConfig;
+import org.apache.impala.authorization.AuthorizationPolicy;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.catalog.MetaStoreClientPool;
import org.apache.impala.common.ImpalaException;
@@ -34,11 +36,10 @@ import java.util.UUID;
* for testing.
*/
public class CatalogServiceTestCatalog extends CatalogServiceCatalog {
-
public CatalogServiceTestCatalog(boolean loadInBackground, int numLoadingThreads,
- SentryConfig sentryConfig, TUniqueId catalogServiceId,
+ AuthorizationConfig authConfig, TUniqueId catalogServiceId,
MetaStoreClientPool metaStoreClientPool) throws ImpalaException {
- super(loadInBackground, numLoadingThreads, sentryConfig, catalogServiceId, null,
+ super(loadInBackground, numLoadingThreads, authConfig, catalogServiceId, null,
System.getProperty("java.io.tmpdir"), metaStoreClientPool);
// Cache pools are typically loaded asynchronously, but as there is no fixed execution
@@ -49,7 +50,7 @@ public class CatalogServiceTestCatalog extends CatalogServiceCatalog {
}
public static CatalogServiceCatalog create() {
- return createWithAuth(null);
+ return createWithAuth(new SentryConfig(null));
}
/**
@@ -60,8 +61,8 @@ public class CatalogServiceTestCatalog extends CatalogServiceCatalog {
FeSupport.loadLibrary();
CatalogServiceCatalog cs;
try {
- cs = new CatalogServiceTestCatalog(false, 16, config, new TUniqueId(), new
- MetaStoreClientPool(0, 0));
+ cs = new CatalogServiceTestCatalog(false, 16, new SentryAuthorizationConfig(config),
+ new TUniqueId(), new MetaStoreClientPool(0, 0));
cs.reset();
} catch (ImpalaException e) {
throw new IllegalStateException(e.getMessage(), e);
diff --git a/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java b/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
index 1683b2c..0b160e6 100644
--- a/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
+++ b/fe/src/test/java/org/apache/impala/testutil/ImpaladTestCatalog.java
@@ -21,6 +21,7 @@ import com.google.common.base.Preconditions;
import org.apache.impala.analysis.TableName;
import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.catalog.BuiltinsDb;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.catalog.Db;
@@ -47,13 +48,13 @@ public class ImpaladTestCatalog extends ImpaladCatalog {
private final CatalogServiceCatalog srcCatalog_;
public ImpaladTestCatalog() {
- this(AuthorizationConfig.createAuthDisabledConfig());
+ this(SentryAuthorizationConfig.createAuthDisabledConfig());
}
/**
* Takes an AuthorizationConfig to bootstrap the backing CatalogServiceCatalog.
*/
- public ImpaladTestCatalog(AuthorizationConfig authzConfig) {
+ public ImpaladTestCatalog(SentryAuthorizationConfig authzConfig) {
super("127.0.0.1");
CatalogServiceCatalog catalogServerCatalog = authzConfig.isEnabled() ?
CatalogServiceTestCatalog.createWithAuth(authzConfig.getSentryConfig()) :
diff --git a/fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java b/fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java
index 27a0604..9ae61fc 100644
--- a/fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java
+++ b/fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java
@@ -18,12 +18,11 @@
package org.apache.impala.testutil;
import org.apache.impala.authorization.AuthorizationConfig;
+import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.Catalog;
import org.apache.impala.common.ImpalaException;
-import org.apache.impala.service.BackendConfig;
import org.apache.impala.service.CatalogOpExecutor;
import org.apache.impala.service.Frontend;
-import org.apache.impala.thrift.TBackendGflags;
import org.apache.impala.thrift.TCatalogUpdateResult;
import org.apache.impala.thrift.TCopyTestCaseReq;
import org.apache.impala.thrift.TDdlExecResponse;
@@ -56,7 +55,7 @@ public class PlannerTestCaseLoader implements AutoCloseable {
private final Frontend frontend_;
public PlannerTestCaseLoader() throws ImpalaException {
- AuthorizationConfig config = AuthorizationConfig.createAuthDisabledConfig();
+ AuthorizationConfig config = SentryAuthorizationConfig.createAuthDisabledConfig();
catalog_ = new ImpaladTestCatalog(
CatalogServiceTestCatalog.createTransientTestCatalog());
frontend_ = new Frontend(config, catalog_);
diff --git a/fe/src/test/java/org/apache/impala/testutil/SentryServicePinger.java b/fe/src/test/java/org/apache/impala/testutil/SentryServicePinger.java
index 4361ef9..97c67e3 100644
--- a/fe/src/test/java/org/apache/impala/testutil/SentryServicePinger.java
+++ b/fe/src/test/java/org/apache/impala/testutil/SentryServicePinger.java
@@ -21,9 +21,9 @@ import org.apache.commons.cli.BasicParser;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.OptionBuilder;
import org.apache.commons.cli.Options;
-import org.apache.impala.authorization.SentryConfig;
+import org.apache.impala.authorization.sentry.SentryConfig;
import org.apache.impala.authorization.User;
-import org.apache.impala.util.SentryPolicyService;
+import org.apache.impala.authorization.sentry.SentryPolicyService;
import org.apache.log4j.Level;
import org.apache.sentry.core.common.transport.SentryTransportFactory;
import org.slf4j.Logger;
diff --git a/fe/src/test/resources/authz-policy.ini.template b/fe/src/test/resources/authz-policy.ini.template
index 80ac068..b7cd07e 100644
--- a/fe/src/test/resources/authz-policy.ini.template
+++ b/fe/src/test/resources/authz-policy.ini.template
@@ -34,7 +34,7 @@ ${USER} = all_tpch, all_newdb, all_functional_seq_snap, select_tpcds,\
auth_to_local_group = test_role
server_admin = all_server
-# The [roles] section defines privileges for objects in the authorizeable hierarchy.
+# The [roles] section defines privileges for objects in the authorizable hierarchy.
# Each role must start at the server-level and go down. If no "action" is specified, the
# role will have all permissions at that level. For more information see the Hive Access
# documentation.
[impala] 06/06: IMPALA-8209: Include fragment instance ID in memz/
breakdown
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 257fa0c68bb4e64880a64844d8d4023c54645230
Author: Lars Volker <lv...@cloudera.com>
AuthorDate: Tue Feb 19 10:12:52 2019 -0800
IMPALA-8209: Include fragment instance ID in memz/ breakdown
The change for IMPALA-7694 had accidentally removed the fragment
instance ID from the memz/ breakdown. This change puts it back and adds
a test to make sure it's there.
This change also pads query IDs with zeros when printing them in the
backend.
Change-Id: I73bf06bf95c88186b16fd03243de9bac946c5cc8
Reviewed-on: http://gerrit.cloudera.org:8080/12524
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/runtime/runtime-state.cc | 3 ++-
be/src/util/debug-util-test.cc | 2 +-
be/src/util/debug-util.cc | 3 ++-
tests/webserver/test_web_pages.py | 14 ++++++++++++++
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/be/src/runtime/runtime-state.cc b/be/src/runtime/runtime-state.cc
index 9ecaafb..8ab60b7 100644
--- a/be/src/runtime/runtime-state.cc
+++ b/be/src/runtime/runtime-state.cc
@@ -73,7 +73,8 @@ RuntimeState::RuntimeState(QueryState* query_state, const TPlanFragmentCtx& frag
utc_timestamp_(new TimestampValue(
TimestampValue::Parse(query_state->query_ctx().utc_timestamp_string))),
local_time_zone_(&TimezoneDatabase::GetUtcTimezone()),
- profile_(RuntimeProfile::Create(obj_pool(), "<fragment instance>")),
+ profile_(RuntimeProfile::Create(
+ obj_pool(), "Fragment " + PrintId(instance_ctx.fragment_instance_id))),
instance_buffer_reservation_(new ReservationTracker) {
Init();
}
diff --git a/be/src/util/debug-util-test.cc b/be/src/util/debug-util-test.cc
index bf430c7..0b92c27 100644
--- a/be/src/util/debug-util-test.cc
+++ b/be/src/util/debug-util-test.cc
@@ -34,7 +34,7 @@ TEST(DebugUtil, UniqueID) {
std::string str("feedbeeff00d7777:2020202020202020");
EXPECT_EQ(str, PrintId(unique_id));
unique_id.lo = 0x20ULL;
- EXPECT_EQ("feedbeeff00d7777:20", PrintId(unique_id));
+ EXPECT_EQ("feedbeeff00d7777:0000000000000020", PrintId(unique_id));
}
string RecursionStack(int level) {
diff --git a/be/src/util/debug-util.cc b/be/src/util/debug-util.cc
index 052d70d..f2729b4 100644
--- a/be/src/util/debug-util.cc
+++ b/be/src/util/debug-util.cc
@@ -88,7 +88,8 @@ PRINT_THRIFT_ENUM_IMPL(TUnit)
string PrintId(const TUniqueId& id, const string& separator) {
stringstream out;
- out << hex << id.hi << separator << id.lo;
+ // Outputting the separator string resets the stream width.
+ out << hex << setfill('0') << setw(16) << id.hi << separator << setw(16) << id.lo;
return out.str();
}
diff --git a/tests/webserver/test_web_pages.py b/tests/webserver/test_web_pages.py
index c7fa741..99724b4 100644
--- a/tests/webserver/test_web_pages.py
+++ b/tests/webserver/test_web_pages.py
@@ -22,6 +22,7 @@ from tests.common.impala_cluster import ImpalaCluster
from tests.common.impala_test_suite import ImpalaTestSuite
import json
import pytest
+import re
import requests
@@ -40,6 +41,7 @@ class TestWebPage(ImpalaTestSuite):
QUERY_FINSTANCES_URL = "http://localhost:{0}/query_finstances"
RPCZ_URL = "http://localhost:{0}/rpcz"
THREAD_GROUP_URL = "http://localhost:{0}/thread-group"
+ MEMZ_URL = "http://localhost:{0}/memz"
METRICS_URL = "http://localhost:{0}/metrics"
JMX_URL = "http://localhost:{0}/jmx"
ADMISSION_URL = "http://localhost:{0}/admission"
@@ -111,6 +113,18 @@ class TestWebPage(ImpalaTestSuite):
page = requests.get("http://localhost:25020/memz")
assert page.status_code == requests.codes.ok
+ def test_memz_shows_fragment_instance_id(self):
+ """Tests that the memory breakdown on memz shows fragment instance IDs."""
+ query = "select count(*) from functional_parquet.alltypes where bool_col = sleep(100)"
+ query_handle = self.client.execute_async(query)
+ try:
+ self.wait_for_state(query_handle, self.client.QUERY_STATES['RUNNING'], 1000)
+ memz_breakdown = self.get_debug_page(self.MEMZ_URL)['detailed']
+ finstance_re = re.compile("Fragment [0-9a-f]{16}:[0-9a-f]{16}")
+ assert finstance_re.search(memz_breakdown), memz_breakdown
+ finally:
+ self.client.close_query(query_handle)
+
def test_query_profile_encoded_unknown_query_id(self):
"""Test that /query_profile_encoded error message starts with the expected line in
case of missing query and does not contain any leading whitespace.
[impala] 01/06: IMPALA-8191: Wait for additional breakpad processes
during test
Posted by lv...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lv pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit c1274fafb04de1b9b7c3a17e209814b8c4346311
Author: Lars Volker <lv...@cloudera.com>
AuthorDate: Fri Feb 15 05:49:56 2019 -0800
IMPALA-8191: Wait for additional breakpad processes during test
The Breakpad signal handler forks off a process to write a minidump.
During the breakpad tests we send signals to the Impala daemons and then
wait for all processes to go away. Prior to this change we did this by
waiting on the PID returned by process.get_pid(). It is determined by
iterating over psutil.get_pid_list() which is an ordered list of PIDs
running on the system. We return the first process in the list with a
matching command line. In cases where the PID space rolled over, this
could have been the forked off breakpad process and we'd wait on that
one. During the subsequent check that all processes are indeed gone, we
could then pick up the original Impala daemon that had forked off to
write the minidump and was still in the process of shutting down.
To fix this, we wait for every process twice. Processes are identified
by their command and iterating through them twice makes sure we catch
both the original daemon and it's breakpad child.
This change also contains improvements to the logging of processes in
our tests. This should make it easier to identify similar issues in the
future.
Testing: I ran the breakpad tests in exhaustive mode. I didn't try to
exercise it around a PID roll-over, but we shouldn't see the issue in
IMPALA-8191 again.
Change-Id: Ia4dcc5fecb9b5f38ae1504aae40f099837cf1bca
Reviewed-on: http://gerrit.cloudera.org:8080/12501
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
tests/common/impala_cluster.py | 63 +++++++++++++++++++++++++----------
tests/custom_cluster/test_breakpad.py | 14 +++++---
2 files changed, 55 insertions(+), 22 deletions(-)
diff --git a/tests/common/impala_cluster.py b/tests/common/impala_cluster.py
index 751c792..1596741 100644
--- a/tests/common/impala_cluster.py
+++ b/tests/common/impala_cluster.py
@@ -287,39 +287,71 @@ class Process(object):
self.container_id = container_id
self.port_map = port_map
+ def __class_name(self):
+ return self.__class__.__name__
+
+ def __str__(self):
+ return "<%s PID: %s (%s)>" % (self.__class_name(), self.__get_pid(),
+ ' '.join(self.cmd))
+
+ def __repr__(self):
+ return str(self)
+
def get_pid(self):
"""Gets the PID of the process. Returns None if the PID cannot be determined"""
- LOG.info("Attempting to find PID for %s" % ' '.join(self.cmd))
- return self.__get_pid()
+ pid = self.__get_pid()
+ if pid:
+ LOG.info("Found PID %s for %s" % (pid, " ".join(self.cmd)))
+ else:
+ LOG.info("No PID found for process cmdline: %s. Process is dead?" %
+ " ".join(self.cmd))
+ return pid
+
+ def get_pids(self):
+ """Gets the PIDs of the process. In some circumstances, a process can run multiple
+ times, e.g. when it forks in the Breakpad crash handler. Returns an empty list if no
+ PIDs can be determined."""
+ pids = self.__get_pids()
+ if pids:
+ LOG.info("Found PIDs %s for %s" % (", ".join(map(str, pids)), " ".join(self.cmd)))
+ else:
+ LOG.info("No PID found for process cmdline: %s. Process is dead?" %
+ " ".join(self.cmd))
+ return pids
def __get_pid(self):
+ pids = self.__get_pids()
+ assert len(pids) < 2, "Expected single pid but found %s" % ", ".join(map(str, pids))
+ return len(pids) == 1 and pids[0] or None
+
+ def __get_pids(self):
if self.container_id is not None:
container_info = self._get_container_info(self.container_id)
if container_info["State"]["Status"] != "running":
- return None
- return container_info["State"]["Status"]["Pid"]
+ return []
+ return [container_info["State"]["Status"]["Pid"]]
# In non-containerised case, search for process based on matching command lines.
+ pids = []
for pid in psutil.get_pid_list():
try:
process = psutil.Process(pid)
if set(self.cmd) == set(process.cmdline):
- return pid
- except psutil.NoSuchProcess, e:
- # A process from get_pid_list() no longer exists, continue.
- LOG.info(e)
- LOG.info("No PID found for process cmdline: %s. Process is dead?" % self.cmd)
- return None
+ pids.append(pid)
+ except psutil.NoSuchProcess:
+ # A process from get_pid_list() no longer exists, continue. We don't log this
+ # error since it can refer to arbitrary processes outside of our testing code.
+ pass
+ return pids
def kill(self, signal=SIGKILL):
"""
Kills the given processes.
"""
if self.container_id is None:
- pid = self.get_pid()
- if pid is None:
- assert 0, "No processes %s found" % self.cmd
- LOG.info('Killing: %s (PID: %d) with signal %s' % (' '.join(self.cmd), pid, signal))
+ pid = self.__get_pid()
+ assert pid is not None, "No processes for %s" % self
+ LOG.info('Killing %s with signal %s' % (self, signal))
exec_process("kill -%d %d" % (signal, pid))
else:
LOG.info("Stopping container: {0}".format(self.container_id))
@@ -349,9 +381,6 @@ class Process(object):
while self.__get_pid() is not None:
sleep(0.01)
- def __str__(self):
- return "Command: %s PID: %s" % (self.cmd, self.get_pid())
-
# Base class for all Impala processes
class BaseImpalaProcess(Process):
diff --git a/tests/custom_cluster/test_breakpad.py b/tests/custom_cluster/test_breakpad.py
index 91f79d9..1c2d6bf 100644
--- a/tests/custom_cluster/test_breakpad.py
+++ b/tests/custom_cluster/test_breakpad.py
@@ -88,11 +88,15 @@ class TestBreakpadBase(CustomClusterTestSuite):
def wait_for_all_processes_dead(self, processes, timeout=300):
for process in processes:
try:
- pid = process.get_pid()
- if not pid:
- continue
- psutil_process = psutil.Process(pid)
- psutil_process.wait(timeout)
+ # For every process in the list we might see the original Impala process plus a
+ # forked off child that is writing the minidump. We need to catch both.
+ for pid in process.get_pids():
+ print "Checking pid %s" % pid
+ psutil_process = psutil.Process(pid)
+ psutil_process.wait(timeout)
+ except psutil.NoSuchProcess:
+ # Process has exited in the meantime
+ pass
except psutil.TimeoutExpired:
raise RuntimeError("Unable to kill %s (pid %d) after %d seconds." %
(psutil_process.name, psutil_process.pid, timeout))