AMBARI-17993. Kerberos identity definitions in Kerberos descriptors should explicitly declare a reference (rlevas)

ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java

@@ -1367,6 +1367,7 @@ public class KerberosHelperImpl implements KerberosHelper {
 
                     hostActiveIdentities.put(uniqueKey, new KerberosIdentityDescriptor(
                         identity.getName(),
+                        identity.getReference(),
                         resolvedPrincipalDescriptor,
                         resolvedKeytabDescriptor,
                         identity.getWhen()));

ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/AbstractKerberosDescriptorContainer.java

@@ -225,28 +225,8 @@ public abstract class AbstractKerberosDescriptorContainer extends AbstractKerber
         KerberosIdentityDescriptor identityToAdd;
 
         if (resolveReferences) {
-          // Copy this KerberosIdentityDescriptor and then attempt to find the referenced one.
-          // * If a reference is found, copy that, update it with the initial KerberosIdentityDescriptor
-          //   and then add it to the list.
-          // * If a reference is not found, simply add the initial KerberosIdentityDescriptor to the list
-          KerberosIdentityDescriptor referencedIdentity;
-          try {
-            referencedIdentity = getReferencedIdentityDescriptor(identity.getName());
-          } catch (AmbariException e) {
-            throw new AmbariException(String.format("Invalid Kerberos identity reference: %s", identity.getName()), e);
-          }
-
-          // Detach this identity from the tree...
-          identity = new KerberosIdentityDescriptor(identity.toMap());
-
-          if (referencedIdentity != null) {
-            KerberosIdentityDescriptor detachedIdentity = new KerberosIdentityDescriptor(referencedIdentity.toMap());
-            detachedIdentity.update(identity);
-
-            identityToAdd = detachedIdentity;
-          } else {
-            identityToAdd = identity;
-          }
+          // Dereference this KerberosIdentityDescriptor, if necessary
+          identityToAdd = dereferenceIdentity(identity);
         } else {
           identityToAdd = identity;
         }
@@ -771,4 +751,51 @@ public abstract class AbstractKerberosDescriptorContainer extends AbstractKerber
       return false;
     }
   }
+
+  /**
+   * Recursively dereference a referenced identity.
+   * <p>
+   * Follows the path of references such that the top-most identity definition (one with no pointer
+   * to a referenced identity) contains the base  information which is copied and updated with the
+   * referencing identity's data. The composite identity is then update with the next referencing
+   * identity's data, and so on until the initial identity is encountered.
+   *
+   * @param identity the initial identity to dereference
+   * @return a (disconnected) {@link KerberosIdentityDescriptor} built by traversing the identity
+   * references; or the input identity if it does not reference any other identities.
+   * @throws AmbariException
+   */
+  private KerberosIdentityDescriptor dereferenceIdentity(KerberosIdentityDescriptor identity) throws AmbariException {
+    KerberosIdentityDescriptor dereferencedIdentity = null;
+
+    if(identity != null) {
+      KerberosIdentityDescriptor referencedIdentity;
+      try {
+        if (identity.getReference() != null) {
+          referencedIdentity = getReferencedIdentityDescriptor(identity.getReference());
+        } else {
+          // For backwards compatibility, see if the identity's name indicates a reference...
+          referencedIdentity = getReferencedIdentityDescriptor(identity.getName());
+        }
+      } catch (AmbariException e) {
+        throw new AmbariException(String.format("Invalid Kerberos identity reference: %s", identity.getReference()), e);
+      }
+
+      if (referencedIdentity != null) {
+        dereferencedIdentity = dereferenceIdentity(referencedIdentity);  // Dereference the "parent"...
+
+        if (dereferencedIdentity != null) {
+          dereferencedIdentity.update(identity);
+        } else {
+          dereferencedIdentity = new KerberosIdentityDescriptor(referencedIdentity.toMap());
+          dereferencedIdentity.update(identity);
+        }
+      }
+      else {
+        dereferencedIdentity = new KerberosIdentityDescriptor(identity.toMap());
+      }
+    }
+
+    return dereferencedIdentity;
+  }
 }

ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/KerberosIdentityDescriptor.java

@@ -71,6 +71,11 @@ import java.util.Map;
  */
 public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
 
+  /**
+   * The path to the Kerberos Identity definitions this {@link KerberosIdentityDescriptor} references
+   */
+  private String reference = null;
+
   /**
    * The KerberosPrincipalDescriptor containing the principal details for this Kerberos identity
    */
@@ -99,12 +104,14 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
    * Creates a new KerberosIdentityDescriptor
    *
    * @param name the name of this identity descriptor
+   * @param reference an optional path to a referenced KerberosIdentityDescriptor
    * @param principal a KerberosPrincipalDescriptor
    * @param keytab a KerberosKeytabDescriptor
    * @param when a predicate
    */
-  public KerberosIdentityDescriptor(String name, KerberosPrincipalDescriptor principal, KerberosKeytabDescriptor keytab, Predicate when) {
+  public KerberosIdentityDescriptor(String name, String reference, KerberosPrincipalDescriptor principal, KerberosKeytabDescriptor keytab, Predicate when) {
     setName(name);
+    setReference(reference);
     setPrincipalDescriptor(principal);
     setKeytabDescriptor(keytab);
     setWhen(when);
@@ -124,6 +131,8 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
     // This is not automatically set by the super classes.
     setName(getStringValue(data, "name"));
 
+    setReference(getStringValue(data, "reference"));
+
     if (data != null) {
       Object item;
 
@@ -146,6 +155,25 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
     }
   }
 
+  /**
+   * Gets the path to the referenced Kerberos identity definition
+   *
+   * @return the path to the referenced Kerberos identity definition or <code>null</code> if not set
+   */
+  public String getReference() {
+    return reference;
+  }
+
+  /**
+   * Sets the path to the referenced Kerberos identity definition
+   *
+   * @param reference the path to the referenced Kerberos identity definition or <code>null</code>
+   *                  to indicate no reference
+   */
+  public void setReference(String reference) {
+    this.reference = reference;
+  }
+
   /**
    * Gets the KerberosPrincipalDescriptor related to this KerberosIdentityDescriptor
    *
@@ -263,6 +291,8 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
     if (updates != null) {
       setName(updates.getName());
 
+      setReference(updates.getReference());
+
       setPassword(updates.getPassword());
 
       KerberosPrincipalDescriptor existingPrincipal = getPrincipalDescriptor();
@@ -298,6 +328,10 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
   public Map<String, Object> toMap() {
     Map<String, Object> dataMap = super.toMap();
 
+    if (reference != null) {
+      dataMap.put("reference", reference);
+    }
+
     if (principal != null) {
       dataMap.put(Type.PRINCIPAL.getDescriptorName(), principal.toMap());
     }
@@ -320,6 +354,9 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
   @Override
   public int hashCode() {
     return super.hashCode() +
+        ((getReference() == null)
+            ? 0
+            : getReference().hashCode()) +
         ((getPrincipalDescriptor() == null)
             ? 0
             : getPrincipalDescriptor().hashCode()) +
@@ -340,6 +377,11 @@ public class KerberosIdentityDescriptor extends AbstractKerberosDescriptor {
     } else if (object.getClass() == KerberosIdentityDescriptor.class) {
       KerberosIdentityDescriptor descriptor = (KerberosIdentityDescriptor) object;
       return super.equals(object) &&
+          (
+              (getReference() == null)
+                  ? (descriptor.getReference() == null)
+                  : getReference().equals(descriptor.getReference())
+          ) &&
           (
               (getPrincipalDescriptor() == null)
                   ? (descriptor.getPrincipalDescriptor() == null)

ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java

@@ -2542,7 +2542,7 @@ public class UpgradeCatalog240 extends AbstractUpgradeCatalog {
                 componentDescriptor.removeIdentity("hbase_queryserver_hbase");
 
                 // Add the new identity
-                componentDescriptor.putIdentity(new KerberosIdentityDescriptor("/spnego", newPrincipalDescriptor, newKeytabDescriptor, null));
+                componentDescriptor.putIdentity(new KerberosIdentityDescriptor("phoenix_spnego", "/spnego", newPrincipalDescriptor, newKeytabDescriptor, null));
 
                 artifactEntity.setArtifactData(kerberosDescriptor.toMap());
                 artifactDAO.merge(artifactEntity);

ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java

@@ -3682,6 +3682,7 @@ public class KerberosHelperTest extends EasyMockSupport {
 
     final KerberosIdentityDescriptor identityDescriptor1 = createMock(KerberosIdentityDescriptor.class);
     expect(identityDescriptor1.getName()).andReturn("identity1").anyTimes();
+    expect(identityDescriptor1.getReference()).andReturn(null).anyTimes();
     expect(identityDescriptor1.getPrincipalDescriptor()).andReturn(principalDescriptor1).anyTimes();
     expect(identityDescriptor1.getKeytabDescriptor()).andReturn(keytabDescriptor1).anyTimes();
     expect(identityDescriptor1.shouldInclude(anyObject(Map.class))).andReturn(true).anyTimes();
@@ -3689,6 +3690,7 @@ public class KerberosHelperTest extends EasyMockSupport {
 
     final KerberosIdentityDescriptor identityDescriptor2 = createMock(KerberosIdentityDescriptor.class);
     expect(identityDescriptor2.getName()).andReturn("identity2").anyTimes();
+    expect(identityDescriptor2.getReference()).andReturn(null).anyTimes();
     expect(identityDescriptor2.getPrincipalDescriptor()).andReturn(principalDescriptor2).anyTimes();
     expect(identityDescriptor2.getKeytabDescriptor()).andReturn(keytabDescriptor2).anyTimes();
     expect(identityDescriptor2.shouldInclude(anyObject(Map.class))).andReturn(true).anyTimes();
@@ -3696,6 +3698,7 @@ public class KerberosHelperTest extends EasyMockSupport {
 
     final KerberosIdentityDescriptor identityDescriptorService1 = createMock(KerberosIdentityDescriptor.class);
     expect(identityDescriptorService1.getName()).andReturn("identity3").anyTimes();
+    expect(identityDescriptorService1.getReference()).andReturn(null).anyTimes();
     expect(identityDescriptorService1.getPrincipalDescriptor()).andReturn(principalDescriptorService1).anyTimes();
     expect(identityDescriptorService1.getKeytabDescriptor()).andReturn(keytabDescriptorService1).anyTimes();
     expect(identityDescriptorService1.shouldInclude(anyObject(Map.class))).andReturn(true).anyTimes();

ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosDescriptorTest.java

@@ -169,7 +169,7 @@ public class KerberosDescriptorTest {
       Assert.assertEquals(3, resolvedIdentities.size());
 
       for (KerberosIdentityDescriptor item : resolvedIdentities) {
-        if ("/shared".equals(item.getName())) {
+        if ("/shared".equals(item.getReference())) {
           resolvedIdentity = item;
           break;
         }
@@ -180,7 +180,7 @@ public class KerberosDescriptorTest {
       Assert.assertNotNull(identities);
       Assert.assertEquals(3, identities.size());
 
-      KerberosIdentityDescriptor identityReference = component.getIdentity("/shared");
+      KerberosIdentityDescriptor identityReference = component.getIdentity("shared_identity");
       Assert.assertNotNull(identityReference);
 
       KerberosIdentityDescriptor referencedIdentity = descriptor.getIdentity("shared");
@@ -410,4 +410,64 @@ public class KerberosDescriptorTest {
     identity = serviceDescriptor.getReferencedIdentityDescriptor("../service2_identity");
     Assert.assertNull(identity);
   }
+
+  @Test
+  public void testGetReferencedIdentityDescriptor_Recursive() throws IOException {
+    boolean identityFound = false;
+    List<KerberosIdentityDescriptor> identities;
+
+    URL systemResourceURL = ClassLoader.getSystemResource("kerberos/test_get_referenced_identity_descriptor.json");
+    Assert.assertNotNull(systemResourceURL);
+
+    KerberosDescriptor descriptor = KERBEROS_DESCRIPTOR_FACTORY.createInstance(new File(systemResourceURL.getFile()));
+    Assert.assertNotNull(descriptor);
+
+    KerberosServiceDescriptor serviceDescriptor = descriptor.getService("SERVICE2");
+    Assert.assertNotNull(serviceDescriptor);
+
+    identities = serviceDescriptor.getIdentities(true, null);
+    Assert.assertNotNull(identities);
+
+    identityFound = false;
+    for(KerberosIdentityDescriptor identity : identities) {
+      if("service2_stack_reference".equals(identity.getName())) {
+
+        // From base identity
+        Assert.assertEquals("stack@${realm}", identity.getPrincipalDescriptor().getValue());
+
+        // Overwritten by the "local" identity
+        Assert.assertEquals("${keytab_dir}/service2_stack.keytab", identity.getKeytabDescriptor().getFile());
+        Assert.assertEquals("/stack_identity", identity.getReference());
+        Assert.assertEquals("service2/property1_principal", identity.getPrincipalDescriptor().getConfiguration());
+
+        identityFound = true;
+      }
+    }
+    Assert.assertTrue(identityFound);
+
+    KerberosComponentDescriptor componentDescriptor =  serviceDescriptor.getComponent("SERVICE2_COMPONENT1");
+    Assert.assertNotNull(componentDescriptor);
+
+    identities = componentDescriptor.getIdentities(true, null);
+    Assert.assertNotNull(identities);
+
+    identityFound = false;
+    for(KerberosIdentityDescriptor identity : identities) {
+      if("component1_service2_stack_reference".equals(identity.getName())) {
+
+        // From base identity
+        Assert.assertEquals("stack@${realm}", identity.getPrincipalDescriptor().getValue());
+
+        // Overwritten by the "referenced" identity
+        Assert.assertEquals("${keytab_dir}/service2_stack.keytab", identity.getKeytabDescriptor().getFile());
+
+        // Overwritten by the "local" identity
+        Assert.assertEquals("/SERVICE2/service2_stack_reference", identity.getReference());
+        Assert.assertEquals("component1_service2/property1_principal", identity.getPrincipalDescriptor().getConfiguration());
+
+        identityFound = true;
+      }
+    }
+    Assert.assertTrue(identityFound);
+  }
 }

ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosIdentityDescriptorTest.java

@@ -63,7 +63,8 @@ public class KerberosIdentityDescriptorTest {
   public static final Map<String, Object> MAP_VALUE_REFERENCE =
       new HashMap<String, Object>() {
         {
-          put("name", "/shared");
+          put("name", "shared_identity");
+          put("reference", "/shared");
           put("keytab", new HashMap<String, Object>() {
             {
               put("file", "/home/user/me/subject.service.keytab");

ambari-server/src/test/resources/kerberos/test_get_referenced_identity_descriptor.json

@@ -30,7 +30,7 @@
       }
     }
   ],
-  "services" : [
+  "services": [
     {
       "name": "SERVICE1",
       "identities": [
@@ -82,6 +82,17 @@
             }
           }
         },
+        {
+          "name": "service2_stack_reference",
+          "reference": "/stack_identity",
+          "principal": {
+            "configuration": "service2/property1_principal"
+          },
+          "keytab": {
+            "configuration": "service2/property1_keytab",
+            "file": "${keytab_dir}/service2_stack.keytab"
+          }
+        },
         {
           "name": "collision",
           "principal": {
@@ -90,7 +101,7 @@
           }
         }
       ],
-      "components" : [
+      "components": [
         {
           "name": "SERVICE2_COMPONENT1",
           "identities": [
@@ -112,6 +123,16 @@
                 }
               }
             },
+            {
+              "name": "component1_service2_stack_reference",
+              "reference": "/SERVICE2/service2_stack_reference",
+              "principal": {
+                "configuration": "component1_service2/property1_principal"
+              },
+              "keytab": {
+                "configuration": "component1_service2/property1_keytab"
+              }
+            },
             {
               "name": "collision",
               "principal": {