joshelser commented on a change in pull request #2320: URL: https://github.com/apache/hbase/pull/2320#discussion_r693092411
########## File path: src/main/asciidoc/_chapters/security.adoc ########## @@ -271,14 +271,14 @@ Since 2.2.0, client can specify the following configurations in `hbase-site.xml` </property> <property> - <name>hbase.client.keytab.principal</name> + <name>hbase.client.kerberos.principal</name> <value>[email protected]</value> </property> ---- Then application can automatically do the login and credential renewal jobs without client interference. It's optional feature, client, who upgrades to 2.2.0, can still keep their login and credential renewal logic already did in older version, as long as keeping `hbase.client.keytab.file` -and `hbase.client.keytab.principal` are unset. +and `hbase.client.kerberos.principal` are unset. Review comment: Would be nice to add a comment here about the previous (wrong) configuration property so folks know for certain what happened. ########## File path: hbase-common/src/main/java/org/apache/hadoop/hbase/AuthUtil.java ########## @@ -87,13 +87,35 @@ /** Client keytab file */ public static final String HBASE_CLIENT_KEYTAB_FILE = "hbase.client.keytab.file"; - /** Client principal */ + /** + * Client principal + * @deprecated Replaced by {@link #HBASE_CLIENT_KERBEROS_PRINCIPAL_VALUE}. + * This constant incorrectly defined the value as "hbase.client.keytab.principal" + * which does not match the value used before version 2.2.0. + */ + @Deprecated public static final String HBASE_CLIENT_KERBEROS_PRINCIPAL = "hbase.client.keytab.principal"; + /** Client principal: "hbase.client.kerberos.principal" */ + public static final String HBASE_CLIENT_KERBEROS_PRINCIPAL_VALUE = + "hbase.client.kerberos.principal"; + private AuthUtil() { super(); } + private static Configuration adjustConf(Configuration conf) { + if (conf != null) { + String principal = conf.get(HBASE_CLIENT_KERBEROS_PRINCIPAL); + if (principal != null) { + // replace deprecated principal key + conf.set(HBASE_CLIENT_KERBEROS_PRINCIPAL_VALUE, principal); + conf.unset(HBASE_CLIENT_KERBEROS_PRINCIPAL); Review comment: IMO, it's a smell to make modifications to the configuration which the caller passed in. What about making a copy of the configuration and updating that configuration-copy? I know that `Configuration` also has some mechanism to handle deprecations, but it might be overkill for this situation :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
