> On June 6, 2015, 10:56 a.m., Robert Levas wrote: > > This looks good, but for some reason the number of `/` characters seems to > > be excessive. We should check to make sure this is correct. However it > > could be that the pattern is compressed due to unneeded data: > > `RULE:[1:$1@$0](.*@%s)s/@.*/(empty string replacment)/(no regex flags, > > usually g might go here)/L` > > > > Is 'L' a regex flag in this case or a special flag for the Hadoop > > auth-to-local rules processor? If testing was a success, than I guess the > > current format is correct.
Not sure but I would think it's part of the regex, testing was successful, but will test some more to confirm format is correct. Nice point > On June 6, 2015, 10:56 a.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java, > > lines 370-371 > > <https://reviews.apache.org/r/35073/diff/2/?file=979950#file979950line370> > > > > This could be problematic in the event > > `kerberos-env/case_insensitive_username_rules` is not set (or `null`). > > Maybe use something like > > ``` > > boolean caseInsensitiveUser = > > "true".equalsIgnoreCase(existingConfigurations.get("kerberos-env").get("case_insensitive_username_rules")) > > ``` "kerberos-env" is expected to be available at this point as it was previously validated within the execution flow. I could double validate if you are concerned. If "case_insensitive_username_rules" however is null it shouldn't be a problem as Boolean.valueOf(null) should return false, which is correct. > On June 6, 2015, 10:56 a.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java, > > line 243 > > <https://reviews.apache.org/r/35073/diff/2/?file=979949#file979949line243> > > > > This seems redundant. Could the flag be pulled out into a variable and > > appened as either and empty string or '//L' depending on the value of > > caseInsensitiveUser? Or maybe simply add the opation to the end if needed? Yes your approach would make the code a little more readable. - Emil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35073/#review86908 ----------------------------------------------------------- On June 5, 2015, 1:51 p.m., Emil Anca wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35073/ > ----------------------------------------------------------- > > (Updated June 5, 2015, 1:51 p.m.) > > > Review request for Ambari, Robert Levas and Vitalyi Brodetskyi. > > > Bugs: AMBARI-11687 > https://issues.apache.org/jira/browse/AMBARI-11687 > > > Repository: ambari > > > Description > ------- > > Force principals names to resolve to lowercase local usernames in > auth-to-local rules. This will help when the KDC is an MIT KDC or an Active > Directory and user accounts have uppercase letters that need to be converted > to lowercase letters. For example: {{USER1234@REALM}} should resolve to > {{user1234}}. > > *Solution* > # Provide a kerberos-env configuration to optionally create case-insensitive > rules > # If creating case-insensitive rules, _generic_ auth-to-local rules should > contain the {{L}} option, as in: > > ~~~ > RULE:[1:$1@$0](.*@REALM)s/@.*///L > ~~~ > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/AuthToLocalBuilder.java > 89d0b55 > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java > 76054b7 > > ambari-server/src/main/resources/common-services/KERBEROS/1.10.3-10/configuration/kerberos-env.xml > 6d720a0 > > ambari-server/src/test/java/org/apache/ambari/server/controller/AuthToLocalBuilderTest.java > d1a2bd1 > > ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java > 5744b53 > ambari-web/app/data/HDP2/site_properties.js d6ab273 > > Diff: https://reviews.apache.org/r/35073/diff/ > > > Testing > ------- > > * mvn clean test -pl AuthToLocalBuilderTest > * Kerbernized/dekerbenized prop with / without prop > * Added service on kerberized cluster > > > Thanks, > > Emil Anca > >
