This is an automated email from the ASF dual-hosted git repository. dbecker pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 0c19f45346f6f41a58f7eb144fdc0938c0a68f3c Author: jichen0919 <[email protected]> AuthorDate: Wed Aug 30 18:00:35 2023 +0800 IMPALA-12397: Fix NullPointerException in SHOW ROLES when there are no roles When there are no roles in Ranger, SHOW ROLES statement hits NullPointerException. The root cause is following statement plugin_.get().getRoles().getRangerRoles(), will return null for latest version of ranger plugin. This issue is not observed in previous version of ranger plugin. This patch will check if null is returned by the method above,if yes, it will return a reference to empty hashset instead. The corresponding unit test and end to end test are added to cover all SHOW ROLE statement cases. Testing: - Pass unit tests. - Pass e2e tests. - Pass core tests with ranger enabled Change-Id: Id80fc2c9152a09194718da1b4266c5f804f0971f Reviewed-on: http://gerrit.cloudera.org:8080/20439 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- .../ranger/RangerImpaladAuthorizationManager.java | 3 + .../RangerImpaladAuthorizationManagerTest.java | 83 ++++++++++++++++++++++ tests/authorization/test_ranger.py | 29 ++++++++ 3 files changed, 115 insertions(+) diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java index beab8a3f5..ea69df169 100644 --- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java +++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java @@ -133,6 +133,9 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager { // The branch for SHOW ROLES. Preconditions.checkState(!params.isIs_show_current_roles()); Set<RangerRole> roles = plugin_.get().getRoles().getRangerRoles(); + if (null == roles) { + roles = Collections.emptySet(); + } roleNames = roles.stream().map(RangerRole::getName).collect(Collectors.toSet()); } diff --git a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManagerTest.java b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManagerTest.java new file mode 100644 index 000000000..e7e16400e --- /dev/null +++ b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManagerTest.java @@ -0,0 +1,83 @@ +/* + * 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.ranger; + +import org.apache.impala.authorization.AuthorizationProvider; +import org.apache.impala.authorization.AuthorizationTestBase; +import org.apache.impala.common.ImpalaException; +import org.apache.impala.thrift.TShowRolesParams; +import org.apache.impala.thrift.TShowRolesResult; + +import org.junit.Assert; +import org.junit.Test; + + +/** + * This class contains unit tests for class RangerImpaladAuthorizationManager. + */ +public class RangerImpaladAuthorizationManagerTest extends AuthorizationTestBase { + private final RangerImpaladAuthorizationManager rangerImpaladAuthzManager_; + + public RangerImpaladAuthorizationManagerTest() throws ImpalaException { + super(AuthorizationProvider.RANGER); + rangerImpaladAuthzManager_ = + (RangerImpaladAuthorizationManager) super.authzFrontend_.getAuthzManager(); + } + + private static TShowRolesParams createTestShowRolesParams(boolean isShowCurrentRole) { + TShowRolesParams showRolesParams = new TShowRolesParams(isShowCurrentRole); + if (!isShowCurrentRole) { + showRolesParams.setRequesting_user("admin"); + } else { + showRolesParams.setRequesting_user("non_owner"); + } + return showRolesParams; + } + + private static TShowRolesParams createTestShowRolesParams(String grantGroup) { + TShowRolesParams showRolesParams = createTestShowRolesParams(false); + showRolesParams.setGrant_group(grantGroup); + return showRolesParams; + } + + private void _testNoExceptionInShowRolesWhenNoRolesInRanger( + TShowRolesParams showRolesParams, String desc) { + try { + TShowRolesResult result = rangerImpaladAuthzManager_.getRoles(showRolesParams); + Assert.assertNotNull(result.getRole_names()); + } catch (ImpalaException ex) { + Assert.fail("No Exception should be thrown for show role statement:" + desc); + } + } + + @Test + public void testNoExceptionInShowRolesWhenNoRolesInRanger() { + // test SHOW ROLES statement. + TShowRolesParams showRolesParams = createTestShowRolesParams(false); + _testNoExceptionInShowRolesWhenNoRolesInRanger(showRolesParams, "SHOW ROLES"); + // test SHOW CURRENT ROLES statement. + TShowRolesParams showCurrentRolesParams = createTestShowRolesParams(true); + _testNoExceptionInShowRolesWhenNoRolesInRanger( + showCurrentRolesParams, "SHOW CURRENT ROLES"); + // test SHOW ROLE GRANT GROUP <group name> statement. + TShowRolesParams showRolesGrantGroupParams = createTestShowRolesParams("admin"); + _testNoExceptionInShowRolesWhenNoRolesInRanger( + showRolesGrantGroupParams, "SHOW ROLE GRANT GROUP admin"); + } +} diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py index 936e9627c..dce1716c0 100644 --- a/tests/authorization/test_ranger.py +++ b/tests/authorization/test_ranger.py @@ -2302,6 +2302,35 @@ class TestRanger(CustomClusterTestSuite): .format(unique_database), user=ADMIN) TestRanger._remove_policy(unique_name) + @pytest.mark.execute_serially + @CustomClusterTestSuite.with_args( + impalad_args=LOCAL_CATALOG_IMPALAD_ARGS, + catalogd_args=LOCAL_CATALOG_CATALOGD_ARGS, + # We additionally set 'reset_ranger' to True, to reset all the policies in the + # Ranger service, so even if there were roles before this test, they will be + # deleted when this test runs. since the Ranger policies are reset before this + # test, we do not have to worry about there could be existing roles when the test + # is running. + reset_ranger=True) + def test_no_exception_in_show_roles_if_no_roles_in_ranger(self, unique_name): + self._test_no_exception_in_show_roles_if_no_roles_in_ranger(unique_name) + + def _test_no_exception_in_show_roles_if_no_roles_in_ranger(self, unique_name): + """ + Ensure that no exception should throw for show roles statement + if there are no roles in ranger. + """ + admin_client = self.create_impala_client() + show_roles_statements = [ + "SHOW ROLES", + "SHOW CURRENT ROLES", + "SHOW ROLE GRANT GROUP admin" + ] + for statement in show_roles_statements: + result = self.execute_query_expect_success(admin_client, statement, + user=ADMIN) + assert len(result.data) == 0 + class TestRangerColumnMaskingTpchNested(CustomClusterTestSuite): """
