----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33225/#review80203 -----------------------------------------------------------
agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json <https://reviews.apache.org/r/33225/#comment129983> With isRecursive==false shouldn't this be false? agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json <https://reviews.apache.org/r/33225/#comment129984> I understand why these 2 cases true because that is how eager pattern matcher would work -- but they are counter intutive. It would good to have a test case added where value is "/public/*test/" to contrast it with this. Presence of trailing / restricts wildcard only to sub-directory names of /public. agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json <https://reviews.apache.org/r/33225/#comment129987> It would be good to a input where *test is not a parent of *result, to highlight that * before result can include / too, e.g. "/public/a-test/foo/bar/a-result" agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json <https://reviews.apache.org/r/33225/#comment129997> Because of the way we treat wildcard there is no way for user to say: - allow /public/1test/1result but disallow /public/blah/1test/1result and /public/1test/blah/1result. Today user can't do that. I wonder if wildcard * should match anything except the path seperator to make this matching intutive to user. agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json <https://reviews.apache.org/r/33225/#comment129992> The answer depends on how do the two following patterns differ: root.default.mycompany* root.default.mycompany*. it should be false if pattern if the second one and it will be, if you were to add a test. While the behavior is counter intutive, if user is able to control the behavior them we are good. agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json <https://reviews.apache.org/r/33225/#comment129993> "roots.default" would/should be false - Alok Lal On April 15, 2015, 8:43 a.m., Madhan Neethiraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33225/ > ----------------------------------------------------------- > > (Updated April 15, 2015, 8:43 a.m.) > > > Review request for ranger, Alok Lal, Don Bosco Durai, Gautam Borad, Abhay > Kulkarni, Ramesh Mani, Selvamohan Neethiraj, and Velmurugan Periasamy. > > > Bugs: RANGER-400 > https://issues.apache.org/jira/browse/RANGER-400 > > > Repository: ranger > > > Description > ------- > > Path-matcher handling of isRecursive updated to fix the issue. Added unit > tests. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerDefaultPolicyEvaluator.java > b6c98f7 > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerAbstractResourceMatcher.java > 646cbc5 > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerDefaultResourceMatcher.java > 007fc42 > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerPathResourceMatcher.java > fffdbfc > > agents-common/src/main/java/org/apache/ranger/plugin/resourcematcher/RangerResourceMatcher.java > 4a846b5 > > agents-common/src/test/java/org/apache/ranger/plugin/resourcematcher/TestResourceMatcher.java > PRE-CREATION > > agents-common/src/test/resources/resourcematcher/test_resourcematcher_default.json > PRE-CREATION > > agents-common/src/test/resources/resourcematcher/test_resourcematcher_path.json > PRE-CREATION > > Diff: https://reviews.apache.org/r/33225/diff/ > > > Testing > ------- > > All existing and new unit tests pass > > > Thanks, > > Madhan Neethiraj > >
