AMBARI-25173. Regenerate keytab skips some keytabs on repaired host (… #2839
Conversation
ghost
requested review from
|
Refer to this link for build results (access rights to CI server needed): |
| false); | ||
|
|
||
| List<KerberosIdentityDescriptor> globalIdentities = new ArrayList<>(); | ||
| if (!identities.isEmpty()) { |
There was a problem hiding this comment.
Technically, this is unnecessary... but its ok.
| for (ResolvedKerberosKeytab rkk : kerberosKeytabController.getFilteredKeytabs(serviceIdentities, getHostFilter(),getIdentityFilter())) { | ||
| final Collection<KerberosIdentityDescriptor> identities = serviceComponentFilter == null ? null : kerberosKeytabController.getServiceIdentities(getClusterName(), serviceComponentFilter.keySet()); | ||
| if (identities != null && getOperationType(getCommandParameters()) == OperationType.RECREATE_ALL){ | ||
| identities.addAll(kerberosHelper.getGlobalActiveIdentities(getClusterName())); |
There was a problem hiding this comment.
If getGlobalActiveIdentities is a superset of the identities returned by getServiceIdentities then why do both. Wouldn't it be better to do something lie
if (getOperationType(getCommandParameters()) == OperationType.RECREATE_ALL) {
identities = kerberosHelper.getGlobalActiveIdentities(getClusterName());
}
else {
identities = serviceComponentFilter == null ? null : kerberosKeytabController.getServiceIdentities(getClusterName(), serviceComponentFilter.keySet())
}
There was a problem hiding this comment.
getGlobalActiveIdentities is not a superset. It just contains smokeuser and spnego. The identities branch that contains them is on the same level of hierarchy with services branch. If the name is misleading, maybe there is a proper name for them?
There was a problem hiding this comment.
Thanks for the clarification. Can you update the Javadoc for getGlobalActiveIdentities to indicate that? Naming is hard, so I am not sure if there is a better name, now that I know what the method is supposed to do.
|
Already fixed by AMBARI-24672 on latest branch-2.7 |
ghost
closed this
…dlysnichenko)
What changes were proposed in this pull request?
Looks like AMBARI-24319 added this bug when trying to fix another bug. The mentioned bugfix addressed the situation when we regenerate keytabs for some service, and accidentally regenerate referenced shared keytabs. As a result, other services that are using these shared keytabs are facing issue with authentication.
Existing implementation never regenerates referenced keytabs. Since code only goes through "services" at kerberosDescriptor, and never descends into "identities", entries at "identities" are never regenerated.
Current patch will check if a request is "regenerate_keytabs=all", and in this case, it would add entries from "identities" to a list of keytabs that are regenerated
How was this patch tested?
unit tests, manual check on a live cluster
NOTE: Current patch intersects with changes from #2802 . I've rebased against these changes, and am currently checking if the patch still works