Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22806 )
Change subject: [RANGER] KUDU-3661 Ranger policy not honored in Kudu ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/22806/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22806/2//COMMIT_MSG@19 PS2, Line 19: Testing this is not straightforward To me it looks quite straightforward. At least, with the following patch the issue is exposed as-is. Please add a similar test scenario (or this code as-is) -- it should be able to catch a regression in future, if any. [ RUN ] RangerClientTest.FatFingerTypo src/kudu/ranger/ranger_client-test.cc:364: Failure Expected equality of these values: 2 actions.size() Which is: 1 Google Test trace: src/kudu/ranger/ranger_client-test.cc:350: CREATE src/kudu/ranger/ranger_client-test.cc:364: Failure Expected equality of these values: 2 actions.size() Which is: 1 Google Test trace: src/kudu/ranger/ranger_client-test.cc:350: ALTER src/kudu/ranger/ranger_client-test.cc:364: Failure Expected equality of these values: 2 actions.size() Which is: 1 Google Test trace: src/kudu/ranger/ranger_client-test.cc:350: DROP I20250423 13:38:18.878540 4243 test_util.cc:183] ----------------------------------------------- I20250423 13:38:18.878605 4243 test_util.cc:184] Had failures, leaving test files at /tmp/kudutest-0/ranger_client-test.RangerClientTest.FatFingerTypo.1745440698874642-4243-0 [ FAILED ] RangerClientTest.FatFingerTypo (2 ms) [----------] 1 test from RangerClientTest (2 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (2 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] RangerClientTest.FatFingerTypo diff --git a/src/kudu/ranger/ranger_client-test.cc b/src/kudu/ranger/ranger_client-test.cc index 62ee8027a..f141c87ea 100644 --- a/src/kudu/ranger/ranger_client-test.cc +++ b/src/kudu/ranger/ranger_client-test.cc @@ -340,6 +340,31 @@ TEST_F(RangerClientTest, TestAuthorizeActionsAllAuthorized) { ASSERT_EQ(3, actions.size()); } +TEST_F(RangerClientTest, FatFingerTypo) { + for (auto action : { ActionPB::DELETE, + ActionPB::INSERT, + ActionPB::UPDATE, + ActionPB::CREATE, + ActionPB::ALTER, + ActionPB::DROP }) { + SCOPED_TRACE(ActionPB_Name(action)); + // Clear the mock response from prior iteration (if any). + next_response_->clear(); + + // Allow only SELECT and one of the other actions (but not ALL). + Allow("jdoe", action, "default", "foobar"); + Allow("jdoe", ActionPB::SELECT, "default", "foobar"); + unordered_set<ActionPB, ActionHash> actions = { + ActionPB::DELETE, + ActionPB::INSERT, + ActionPB::UPDATE, + ActionPB::SELECT + }; + ASSERT_OK(client_.AuthorizeActions("jdoe", "default", "foobar", /*is_owner=*/false, &actions)); + EXPECT_EQ(2, actions.size()); + } +} + TEST_F(RangerClientTest, TestInvalidJARFails) { FLAGS_ranger_config_path = test_dir_; FLAGS_ranger_jar_path = "/this/is/not/a/real/location/hopefully.jar"; http://gerrit.cloudera.org:8080/#/c/22806/2/src/kudu/master/ranger_authz_provider.cc File src/kudu/master/ranger_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/22806/2/src/kudu/master/ranger_authz_provider.cc@315 PS2, Line 315: if (pb->scan_privilege()) { nit: add a comment to explain why the control returns out of the function here (it's something related to the comment in lines 319-320)? -- To view, visit http://gerrit.cloudera.org:8080/22806 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I635132154d622eb41e993a0a1a818b21b5af6bb7 Gerrit-Change-Number: 22806 Gerrit-PatchSet: 2 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Wed, 23 Apr 2025 20:42:41 +0000 Gerrit-HasComments: Yes
