> On July 14, 2017, 3:14 p.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelper.java > > Line 313 (original), 313 (patched) > > <https://reviews.apache.org/r/60878/diff/1/?file=1776859#file1776859line313> > > > > Can we document why both stacks (original and new) must contain the key > > before merging in the user value? It might not be clear from this > > if-statement. > > Robert Levas wrote: > Acutually, *must* is a strong word here. The bug is that there is no need > for this; yet the code expected it. The fix checks to see if the identity > exists in both the new and old stack Kerberos descriptors. If so, then it > tries to reconcile them along with the matching user-supplied Kerberos > identity. Else if there is no need to reconcile the identities, then we > simply keep the user-defined identitiy as-is.
Retinking this (due to your comment), the fix does not properly address the case where a user adds an identitiy before an upgrade that happens to exist in the new stack. The current fix may loose the additions to the identitiy that the new stack might have added. Fixing... - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60878/#review180571 ----------------------------------------------------------- On July 14, 2017, 3:09 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60878/ > ----------------------------------------------------------- > > (Updated July 14, 2017, 3:09 p.m.) > > > Review request for Ambari, Attila Magyar, Eugene Chekanskiy, Jonathan Hurley, > Laszlo Puskas, Nate Cole, Sumit Mohanty, Sebastian Toader, and Sid Wagle. > > > Bugs: AMBARI-21480 > https://issues.apache.org/jira/browse/AMBARI-21480 > > > Repository: ambari > > > Description > ------- > > Ambari-server.log:- > ``` > java.lang.NullPointerException > at > org.apache.ambari.server.state.kerberos.KerberosDescriptorUpdateHelper.processIdentity(KerberosDescriptorUpdateHelper.java:360) > at > org.apache.ambari.server.state.kerberos.KerberosDescriptorUpdateHelper.processIdentities(KerberosDescriptorUpdateHelper.java:321) > at > org.apache.ambari.server.state.kerberos.KerberosDescriptorUpdateHelper.processComponent(KerberosDescriptorUpdateHelper.java:230) > at > org.apache.ambari.server.state.kerberos.KerberosDescriptorUpdateHelper.processService(KerberosDescriptorUpdateHelper.java:195) > at > org.apache.ambari.server.state.kerberos.KerberosDescriptorUpdateHelper.processServices(KerberosDescriptorUpdateHelper.java:122) > at > org.apache.ambari.server.state.kerberos.KerberosDescriptorUpdateHelper.updateUserKerberosDescriptor(KerberosDescriptorUpdateHelper.java:78) > at > org.apache.ambari.server.serveraction.upgrades.UpgradeUserKerberosDescriptor.execute(UpgradeUserKerberosDescriptor.java:139) > at > org.apache.ambari.server.serveraction.ServerActionExecutor$Worker.execute(ServerActionExecutor.java:517) > at > org.apache.ambari.server.serveraction.ServerActionExecutor$Worker.run(ServerActionExecutor.java:454) > at java.lang.Thread.run(Thread.java:748) > ``` > > # Cause > This is caused by having a _custom/unexpected_ identity in the user-supplied > Kerberos descriptor that is not in the Kerberos descriptor from the initial > stack. > > # Solution > Ignore the _custom/unexpected_ identity since the user must have added that > manually and it is expected that it should remain untouched after the upgrade > process. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelper.java > f05b62b41c > > ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorUpdateHelperTest.java > 247d17ec09 > > > Diff: https://reviews.apache.org/r/60878/diff/1/ > > > Testing > ------- > > manual tests > > # Local test results: PENDING > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
