Repository: ambari
Updated Branches:
  refs/heads/trunk 7a082ecd7 -> 65393a52e


AMBARI-9055. Pass LDAP URL and Principal container DN to Active Directory 
operations handler (rlevas)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/65393a52
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/65393a52
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/65393a52

Branch: refs/heads/trunk
Commit: 65393a52eb38ff6ac779efed3536c837f5a17ae1
Parents: 7a082ec
Author: Robert Levas <[email protected]>
Authored: Thu Jan 15 06:42:06 2015 -0500
Committer: Robert Levas <[email protected]>
Committed: Thu Jan 15 06:42:17 2015 -0500

----------------------------------------------------------------------
 .../server/controller/KerberosHelper.java       | 81 ++++++++++++++++----
 .../kerberos/ADKerberosOperationHandler.java    | 49 +++++++++---
 .../kerberos/CreatePrincipalsServerAction.java  |  3 +-
 .../kerberos/KerberosOperationHandler.java      |  9 ++-
 .../KerberosOperationHandlerFactory.java        | 19 ++---
 .../kerberos/KerberosServerAction.java          | 29 ++++++-
 .../kerberos/MITKerberosOperationHandler.java   | 48 +++++++++---
 .../server/controller/KerberosHelperTest.java   | 15 +++-
 .../ADKerberosOperationHandlerTest.java         | 18 +++--
 .../KerberosOperationHandlerFactoryTest.java    |  4 +
 .../kerberos/KerberosOperationHandlerTest.java  |  4 +-
 .../MITKerberosOperationHandlerTest.java        | 10 ++-
 .../KerberosPrincipalDescriptorTest.java        | 13 ++--
 13 files changed, 229 insertions(+), 73 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
index 5e3e612..6d0e159 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
@@ -184,8 +184,26 @@ public class KerberosHelper {
         throw new AmbariException(message);
       }
 
+      Config configKerberosEnv = 
cluster.getDesiredConfigByType("kerberos-env");
+      if (configKerberosEnv == null) {
+        String message = "The 'kerberos-env' configuration is not available";
+        LOG.error(message);
+        throw new AmbariException(message);
+      }
+
+      Map<String, String> kerberosEnvProperties = 
configKerberosEnv.getProperties();
+      if (kerberosEnvProperties == null) {
+        String message = "The 'kerberos-env' configuration properties are not 
available";
+        LOG.error(message);
+        throw new AmbariException(message);
+      }
+
       KDCType kdcType = null;
-      String kdcTypeProperty = krb5ConfProperties.get("kdc_type");
+      String kdcTypeProperty = kerberosEnvProperties.get("kdc_type");
+      if (kdcTypeProperty == null) {
+        // TODO: (rlevas) Only pull from kerberos-env, this is only for 
transitional purposes (AMBARI 9121)
+        kdcTypeProperty = krb5ConfProperties.get("kdc_type");
+      }
       if (kdcTypeProperty != null) {
         try {
           kdcType = KDCType.translate(kdcTypeProperty);
@@ -201,12 +219,18 @@ public class KerberosHelper {
         kdcType = KDCType.MIT_KDC;
       }
 
+      KDCDetails kdcDetails = new KDCDetails(
+          kdcType,
+          (kerberosEnvProperties == null) ? null : 
kerberosEnvProperties.get("ldap_url"),
+          (kerberosEnvProperties == null) ? null : 
kerberosEnvProperties.get("container_dn")
+      );
+
       if ("true".equalsIgnoreCase(securityEnabled)) {
         LOG.info("Configuring Kerberos for realm {} on cluster, {}", 
defaultRealm, cluster.getClusterName());
-        requestStageContainer = handle(cluster, kerberosDescriptor, 
defaultRealm, kdcType, requestStageContainer, enableKerberosHandler);
+        requestStageContainer = handle(cluster, kerberosDescriptor, 
defaultRealm, kdcDetails, requestStageContainer, enableKerberosHandler);
       } else if ("false".equalsIgnoreCase(securityEnabled)) {
         LOG.info("Disabling Kerberos from cluster, {}", 
cluster.getClusterName());
-        requestStageContainer = handle(cluster, kerberosDescriptor, 
defaultRealm, kdcType, requestStageContainer, disableKerberosHandler);
+        requestStageContainer = handle(cluster, kerberosDescriptor, 
defaultRealm, kdcDetails, requestStageContainer, disableKerberosHandler);
       } else {
         String message = String.format("Invalid value for `security_enabled` 
property of cluster-env: %s", securityEnabled);
         LOG.error(message);
@@ -229,7 +253,7 @@ public class KerberosHelper {
    * @param cluster               the relevant Cluster
    * @param kerberosDescriptor    the (derived) KerberosDescriptor
    * @param realm                 the default Kerberos realm for the Cluster
-   * @param kdcType               the relevant KDC type (MIT KDC or Active 
Directory)
+   * @param kdcDetails            a KDCDetails containing information about 
relevant KDC
    * @param requestStageContainer a RequestStageContainer to place generated 
stages, if needed -
    *                              if null a new RequestStageContainer will be 
created.
    * @return the updated or a new RequestStageContainer containing the stages 
that need to be
@@ -239,7 +263,7 @@ public class KerberosHelper {
   @Transactional
   private RequestStageContainer handle(Cluster cluster,
                                        KerberosDescriptor kerberosDescriptor,
-                                       String realm, KDCType kdcType,
+                                       String realm, KDCDetails kdcDetails,
                                        RequestStageContainer 
requestStageContainer,
                                        Handler handler) throws AmbariException 
{
 
@@ -401,7 +425,7 @@ public class KerberosHelper {
                       "}"
               );
             } else {
-              KerberosOperationHandler operationHandler = 
kerberosOperationHandlerFactory.getKerberosOperationHandler(kdcType);
+              KerberosOperationHandler operationHandler = 
kerberosOperationHandlerFactory.getKerberosOperationHandler(kdcDetails.getKdcType());
 
               if (operationHandler == null) {
                 throw new AmbariException("Failed to get an appropriate 
Kerberos operation handler.");
@@ -410,7 +434,7 @@ public class KerberosHelper {
                 KerberosCredential kerberosCredentials = 
KerberosCredential.decrypt(credentials, key);
 
                 try {
-                  operationHandler.open(kerberosCredentials, realm);
+                  operationHandler.open(kerberosCredentials, realm, 
kdcDetails.getLdapUrl(), kdcDetails.getPrincipalContainerDn());
                   if (!operationHandler.testAdministratorCredentials()) {
                     throw new IllegalArgumentException(
                         "Invalid KDC administrator credentials.\n" +
@@ -502,7 +526,7 @@ public class KerberosHelper {
 
         // Use the handler implementation to setup the relevant stages.
         int lastStageId = handler.createStages(cluster, hosts, 
kerberosConfigurations,
-            clusterHostInfoJson, hostParamsJson, event, roleCommandOrder, 
realm, kdcType.toString(),
+            clusterHostInfoJson, hostParamsJson, event, roleCommandOrder, 
realm, kdcDetails,
             dataDirectory, requestStageContainer, 
serviceComponentHostsToProcess);
 
         // Add the cleanup stage...
@@ -993,7 +1017,7 @@ public class KerberosHelper {
      * @param event                  a ServiceComponentHostServerActionEvent 
to pass to any created tasks
      * @param roleCommandOrder       the RoleCommandOrder to use to generate 
the RoleGraph for any newly created Stages
      * @param realm                  a String declaring the cluster's Kerberos 
realm
-     * @param kdcType                a relevant KDCType
+     * @param kdcDetails             a KDCDetails containing the information 
about the relevant KDC
      * @param dataDirectory          a File pointing to the (temporary) data 
directory
      * @param requestStageContainer  a RequestStageContainer to store the new 
stages in, if null a
      *                               new RequestStageContainer will be created
@@ -1006,7 +1030,7 @@ public class KerberosHelper {
                      String clusterHostInfo, String hostParams,
                      ServiceComponentHostServerActionEvent event,
                      RoleCommandOrder roleCommandOrder,
-                     String realm, String kdcType, File dataDirectory,
+                     String realm, KDCDetails kdcDetails, File dataDirectory,
                      RequestStageContainer requestStageContainer,
                      List<ServiceComponentHost> serviceComponentHosts)
         throws AmbariException;
@@ -1059,7 +1083,7 @@ public class KerberosHelper {
                             Map<String, Map<String, String>> 
kerberosConfigurations,
                             String clusterHostInfoJson, String hostParamsJson,
                             ServiceComponentHostServerActionEvent event,
-                            RoleCommandOrder roleCommandOrder, String realm, 
String kdcType,
+                            RoleCommandOrder roleCommandOrder, String realm, 
KDCDetails kdcDetails,
                             File dataDirectory, RequestStageContainer 
requestStageContainer,
                             List<ServiceComponentHost> serviceComponentHosts)
         throws AmbariException {
@@ -1121,7 +1145,9 @@ public class KerberosHelper {
       Map<String, String> commandParameters = new HashMap<String, String>();
       commandParameters.put(KerberosServerAction.DATA_DIRECTORY, 
dataDirectory.getAbsolutePath());
       commandParameters.put(KerberosServerAction.DEFAULT_REALM, realm);
-      commandParameters.put(KerberosServerAction.KDC_TYPE, kdcType);
+      commandParameters.put(KerberosServerAction.KDC_TYPE, 
kdcDetails.getKdcType().name());
+      commandParameters.put(KerberosServerAction.KDC_LDAP_URL, 
kdcDetails.getLdapUrl());
+      commandParameters.put(KerberosServerAction.KDC_PRINCIPAL_CONTAINER_DN, 
kdcDetails.getPrincipalContainerDn());
       commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, 
getEncryptedAdministratorCredentials(cluster));
 
       // *****************************************************************
@@ -1258,7 +1284,7 @@ public class KerberosHelper {
                             Map<String, Map<String, String>> 
kerberosConfigurations,
                             String clusterHostInfoJson, String hostParamsJson,
                             ServiceComponentHostServerActionEvent event,
-                            RoleCommandOrder roleCommandOrder, String realm, 
String kdcType,
+                            RoleCommandOrder roleCommandOrder, String realm, 
KDCDetails kdcDetails,
                             File dataDirectory, RequestStageContainer 
requestStageContainer,
                             List<ServiceComponentHost> serviceComponentHosts) {
       // TODO (rlevas): If there are principals, keytabs, and configurations 
to process, setup the following sages:
@@ -1269,4 +1295,33 @@ public class KerberosHelper {
       return -1;
     }
   }
+
+
+  /**
+   * KDCDetails is a helper class to hold the details of the relevant KDC so 
they may be passed
+   * around more easily.
+   */
+  private static class KDCDetails {
+    private final KDCType kdcType;
+    private final String ldapUrl;
+    private final String principalContainerDn;
+
+    public KDCDetails(KDCType kdcType, String ldapUrl, String 
principalContainerDn) {
+      this.kdcType = kdcType;
+      this.ldapUrl = ldapUrl;
+      this.principalContainerDn = principalContainerDn;
+    }
+
+    public KDCType getKdcType() {
+      return kdcType;
+    }
+
+    public String getLdapUrl() {
+      return ldapUrl;
+    }
+
+    public String getPrincipalContainerDn() {
+      return principalContainerDn;
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java
index 01913e4..dce980d 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java
@@ -145,7 +145,7 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
   @Override
   public boolean principalExists(String principal) throws 
KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
@@ -154,7 +154,7 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
     try {
       searchResultEnum = ldapContext.search(
           principalContainerDn,
-          "(cn=" + principal + ")",
+          "(userPrincipalName=" + principal + ")",
           searchControls);
       if (searchResultEnum.hasMore()) {
         return true;
@@ -180,21 +180,36 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
    *
    * @param principal a String containing the principal to add
    * @param password  a String containing the password to use when creating 
the principal
+   * @param service   a boolean value indicating whether the principal is to 
be created as a service principal or not
    * @return an Integer declaring the generated key number
    * @throws KerberosOperationException
    */
   @Override
-  public Integer createServicePrincipal(String principal, String password)
+  public Integer createPrincipal(String principal, String password, boolean 
service)
       throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
+
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
     }
     if (password == null) {
       throw new KerberosOperationException("principal password is null");
     }
+
+    // TODO: (rlevas) pass components and realm in separately (AMBARI-9122)
+    String realm = getDefaultRealm();
+    int atIndex = principal.indexOf("@");
+    if (atIndex >= 0) {
+      realm = principal.substring(atIndex + 1);
+      principal = principal.substring(0, atIndex);
+    }
+
+    if (realm == null) {
+      realm = "";
+    }
+
     Attributes attributes = new BasicAttributes();
 
     Attribute objectClass = new BasicAttribute("objectClass");
@@ -206,12 +221,14 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
     attributes.put(cn);
 
     Attribute upn = new BasicAttribute("userPrincipalName");
-    upn.add(String.format("%s@%s", principal, 
getDefaultRealm().toLowerCase()));
+    upn.add(String.format("%s@%s", principal, realm.toLowerCase()));
     attributes.put(upn);
 
-    Attribute spn = new BasicAttribute("servicePrincipalName");
-    spn.add(principal);
-    attributes.put(spn);
+    if (service) {
+      Attribute spn = new BasicAttribute("servicePrincipalName");
+      spn.add(principal);
+      attributes.put(spn);
+    }
 
     Attribute uac = new BasicAttribute("userAccountControl");  // 
userAccountControl
     uac.add("512");
@@ -248,7 +265,7 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
   @Override
   public Integer setPrincipalPassword(String principal, String password) 
throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
@@ -289,9 +306,9 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
    * @throws KerberosOperationException
    */
   @Override
-  public boolean removeServicePrincipal(String principal) throws 
KerberosOperationException {
+  public boolean removePrincipal(String principal) throws 
KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
@@ -313,6 +330,16 @@ public class ADKerberosOperationHandler extends 
KerberosOperationHandler {
     return true;
   }
 
+  @Override
+  public boolean testAdministratorCredentials() throws 
KerberosOperationException {
+    if (!isOpen()) {
+      throw new KerberosOperationException("This operation handler has not 
been opened");
+    }
+    // If this KerberosOperationHandler was successfully opened, successful 
authentication has
+    // already occurred.
+    return true;
+  }
+
   /**
    * Helper method to create the LDAP context needed to interact with the 
Active Directory.
    *

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
index 38ca320..947b033 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
@@ -114,7 +114,8 @@ public class CreatePrincipalsServerAction extends 
KerberosServerAction {
           }
         } else {
           LOG.debug("Creating new principal - {}", evaluatedPrincipal);
-          Integer keyNumber = 
operationHandler.createServicePrincipal(evaluatedPrincipal, password);
+          boolean servicePrincipal = 
"service".equalsIgnoreCase(identityRecord.get(KerberosActionDataFile.PRINCIPAL_TYPE));
+          Integer keyNumber = 
operationHandler.createPrincipal(evaluatedPrincipal, password, 
servicePrincipal);
 
           if (keyNumber != null) {
             principalPasswordMap.put(evaluatedPrincipal, password);

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java
index 5a1310d..49f573e 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java
@@ -178,10 +178,11 @@ public abstract class KerberosOperationHandler {
    *
    * @param principal a String containing the principal to add
    * @param password  a String containing the password to use when creating 
the principal
+   * @param service a boolean value indicating whether the principal is to be 
created as a service principal or not
    * @return an Integer declaring the generated key number
    * @throws KerberosOperationException
    */
-  public abstract Integer createServicePrincipal(String principal, String 
password)
+  public abstract Integer createPrincipal(String principal, String password, 
boolean service)
       throws KerberosOperationException;
 
   /**
@@ -206,7 +207,7 @@ public abstract class KerberosOperationHandler {
    * @return true if the principal was successfully removed; otherwise false
    * @throws KerberosOperationException
    */
-  public abstract boolean removeServicePrincipal(String principal)
+  public abstract boolean removePrincipal(String principal)
       throws KerberosOperationException;
 
   /**
@@ -218,6 +219,10 @@ public abstract class KerberosOperationHandler {
    *                                    administrator credentials
    */
   public boolean testAdministratorCredentials() throws 
KerberosOperationException {
+    if (!isOpen()) {
+      throw new KerberosOperationException("This operation handler has not 
been opened");
+    }
+
     KerberosCredential credentials = getAdministratorCredentials();
     if (credentials == null) {
       throw new KerberosOperationException("Missing KDC administrator 
credentials");

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java
index a717b90..6639b69 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java
@@ -27,32 +27,27 @@ import com.google.inject.Singleton;
 public class KerberosOperationHandlerFactory {
 
   /**
-   * Gets a relevant KerberosOperationHandler give some KDCType.
+   * Gets the relevant KerberosOperationHandler for some KDCType.
    * <p/>
    * If no KDCType is specified, {@link 
org.apache.ambari.server.serveraction.kerberos.KDCType#MIT_KDC}
    * will be assumed.
    *
    * @param kdcType the relevant KDCType
    * @return a KerberosOperationHandler
+   * @throws java.lang.IllegalArgumentException if kdcType is null or the 
KDCType is an unexpected value
    */
   public KerberosOperationHandler getKerberosOperationHandler(KDCType kdcType) 
{
-    KerberosOperationHandler handler = null;
-
-    // If not specified, use KDCType.MIT_KDC as a default
     if (kdcType == null) {
-      kdcType = KDCType.MIT_KDC;
+      throw new IllegalArgumentException("kdcType may not be null");
     }
 
     switch (kdcType) {
-
       case MIT_KDC:
-        handler = new MITKerberosOperationHandler();
-        break;
+        return new MITKerberosOperationHandler();
       case ACTIVE_DIRECTORY:
-        handler = new ADKerberosOperationHandler();
-        break;
+        return new ADKerberosOperationHandler();
+      default:
+        throw new IllegalArgumentException(String.format("Unexpected kdcType 
value: %s", kdcType.name()));
     }
-
-    return handler;
   }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
index 7edd889..b3592e6 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
@@ -70,6 +70,18 @@ public abstract class KerberosServerAction extends 
AbstractServerAction {
   public static final String KDC_TYPE = "kdc_type";
 
   /**
+   * A (command parameter) property name used to hold the URL for the LDAP 
interface to the KDC.
+   * This value may be null.
+   */
+  public static final String KDC_LDAP_URL = "kdc_ldap_url";
+
+  /**
+   * A (command parameter) property name used to hold the distinguished name 
(DN) of the container
+   * in which to store principals within the KDC.  This value may be null.
+   */
+  public static final String KDC_PRINCIPAL_CONTAINER_DN = 
"kdc_principal_container_dn";
+
+  /**
    * The prefix to use for the data directory name.
    */
   public static final String DATA_DIRECTORY_PREFIX = ".ambari_";
@@ -360,11 +372,20 @@ public abstract class KerberosServerAction extends 
AbstractServerAction {
               throw new AmbariException(message);
             }
 
+            String ldapUrl = getCommandParameterValue(KDC_LDAP_URL);
+            String principalContainerDn = 
getCommandParameterValue(KDC_PRINCIPAL_CONTAINER_DN);
+            try {
+              handler.open(administratorCredential, defaultRealm, ldapUrl, 
principalContainerDn);
+            } catch (KerberosOperationException e) {
+              String message = String.format("Failed to process the 
identities, could not properly open the KDC operation handler: %s",
+                  e.getMessage());
+              LOG.error(message);
+              throw new AmbariException(message, e);
+            }
+
             // Create the data file reader to parse and iterate through the 
records
             KerberosActionDataFileReader reader = null;
             try {
-              handler.open(administratorCredential, defaultRealm);
-
               reader = new KerberosActionDataFileReader(indexFile);
               for (Map<String, String> record : reader) {
                 // Process the current record
@@ -376,7 +397,9 @@ public abstract class KerberosServerAction extends 
AbstractServerAction {
                   break;
                 }
               }
-            } catch (KerberosOperationException e) {
+            } catch (AmbariException e) {
+              // Catch this separately from IOException since the reason it 
was thrown was not the same
+              // Note: AmbariException is an IOException, so there may be some 
confusion
               throw new AmbariException(e.getMessage(), e);
             } catch (IOException e) {
               String message = String.format("Failed to process the 
identities, cannot read the index file: %s",

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
index a70b412..23eed35 100644
--- 
a/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
+++ 
b/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
@@ -26,6 +26,7 @@ import java.io.File;
 import java.text.NumberFormat;
 import java.text.ParseException;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -79,7 +80,7 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
       throws KerberosOperationException {
 
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
 
     if (principal == null) {
@@ -112,6 +113,7 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
    *
    * @param principal a String containing the principal add
    * @param password  a String containing the password to use when creating 
the principal
+   * @param service a boolean value indicating whether the principal is to be 
created as a service principal or not
    * @return an Integer declaring the generated key number
    * @throws KerberosKDCConnectionException       if a connection to the KDC 
cannot be made
    * @throws KerberosAdminAuthenticationException if the administrator 
credentials fail to authenticate
@@ -119,11 +121,11 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
    * @throws KerberosOperationException           if an unexpected error 
occurred
    */
   @Override
-  public Integer createServicePrincipal(String principal, String password)
+  public Integer createPrincipal(String principal, String password, boolean 
service)
       throws KerberosOperationException {
 
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -168,7 +170,7 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
   @Override
   public Integer setPrincipalPassword(String principal, String password) 
throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -201,9 +203,9 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
    * @throws KerberosOperationException           if an unexpected error 
occurred
    */
   @Override
-  public boolean removeServicePrincipal(String principal) throws 
KerberosOperationException {
+  public boolean removePrincipal(String principal) throws 
KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -237,7 +239,7 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
    */
   private Integer getKeyNumber(String principal) throws 
KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be 
opened");
+      throw new KerberosOperationException("This operation handler has not 
been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -357,8 +359,36 @@ public class MITKerberosOperationHandler extends 
KerberosOperationHandler {
       result = executeCommand(command.toArray(new String[command.size()]));
 
       if (!result.isSuccessful()) {
-        String message = String.format("Failed to execute kadmin:\n\tExitCode: 
%s\n\tSTDOUT: %s\n\tSTDERR: %s",
-            result.getExitCode(), result.getStdout(), result.getStderr());
+        // Build command string, replacing administrator password with 
"********"
+        StringBuilder cleanCommand = new StringBuilder();
+        Iterator<String> iterator = command.iterator();
+
+        if(iterator.hasNext())
+          cleanCommand.append(iterator.next());
+
+        while(iterator.hasNext()){
+          String part = iterator.next();
+
+          cleanCommand.append(' ');
+
+          if(part.contains(" ")) {
+            cleanCommand.append('"');
+            cleanCommand.append(part);
+            cleanCommand.append('"');
+          }
+          else {
+            cleanCommand.append(part);
+          }
+
+          if("-w".equals(part)) {
+            // Skip the password and use "********" instead
+            if(iterator.hasNext())
+              iterator.next();
+            cleanCommand.append(" ********");
+          }
+        }
+        String message = String.format("Failed to execute kadmin:\n\tCommand: 
%s\n\tExitCode: %s\n\tSTDOUT: %s\n\tSTDERR: %s",
+            cleanCommand.toString(), result.getExitCode(), result.getStdout(), 
result.getStderr());
         LOG.warn(message);
 
         // Test STDERR to see of any "expected" error conditions were 
encountered...

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
index e32524a..8900a26 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
@@ -37,7 +37,6 @@ import org.apache.ambari.server.serveraction.kerberos.KDCType;
 import org.apache.ambari.server.serveraction.kerberos.KerberosCredential;
 import 
org.apache.ambari.server.serveraction.kerberos.KerberosOperationException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandler;
-import 
org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandlerTest;
 import 
org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandlerFactory;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
@@ -92,6 +91,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           public void open(KerberosCredential administratorCredentials, String 
defaultRealm) throws KerberosOperationException {
             setAdministratorCredentials(administratorCredentials);
             setDefaultRealm(defaultRealm);
+            setOpen(true);
           }
 
           @Override
@@ -105,7 +105,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           }
 
           @Override
-          public Integer createServicePrincipal(String principal, String 
password) throws KerberosOperationException {
+          public Integer createPrincipal(String principal, String password, 
boolean service) throws KerberosOperationException {
             return null;
           }
 
@@ -115,7 +115,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           }
 
           @Override
-          public boolean removeServicePrincipal(String principal) throws 
KerberosOperationException {
+          public boolean removePrincipal(String principal) throws 
KerberosOperationException {
             return false;
           }
         })
@@ -251,10 +251,18 @@ public class KerberosHelperTest extends EasyMockSupport {
     final Config clusterEnvConfig = createNiceMock(Config.class);
     
expect(clusterEnvConfig.getProperties()).andReturn(clusterEnvProperties).once();
 
+    final Map<String, String> kerberosEnvProperties = 
createNiceMock(Map.class);
+    // TODO: (rlevas) Add when AMBARI 9121 is complete
+    // 
expect(kerberosEnvProperties.get("kdc_type")).andReturn("mit-kdc").once();
+
+    final Config kerberosEnvConfig = createNiceMock(Config.class);
+    
expect(kerberosEnvConfig.getProperties()).andReturn(kerberosEnvProperties).once();
+
     final Map<String, String> krb5ConfProperties = createNiceMock(Map.class);
     expect(krb5ConfProperties.get("kdc_type")).andReturn("mit-kdc").once();
 
     final Config krb5ConfConfig = createNiceMock(Config.class);
+    // TODO: (rlevas) Remove when AMBARI 9121 is complete
     
expect(krb5ConfConfig.getProperties()).andReturn(krb5ConfProperties).once();
 
     final MaintenanceStateHelper maintenanceStateHelper = 
injector.getInstance(MaintenanceStateHelper.class);
@@ -264,6 +272,7 @@ public class KerberosHelperTest extends EasyMockSupport {
     final Cluster cluster = createNiceMock(Cluster.class);
     
expect(cluster.getDesiredConfigByType("cluster-env")).andReturn(clusterEnvConfig).once();
     
expect(cluster.getDesiredConfigByType("krb5-conf")).andReturn(krb5ConfConfig).once();
+    
expect(cluster.getDesiredConfigByType("kerberos-env")).andReturn(kerberosEnvConfig).once();
     expect(cluster.getClusterName()).andReturn("c1").anyTimes();
     expect(cluster.getServices())
         .andReturn(new HashMap<String, Service>() {

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java
index d95dc5b..aad325a 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java
@@ -38,11 +38,11 @@ import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 
 public class ADKerberosOperationHandlerTest extends EasyMockSupport {
-  private static final String DEFAULT_ADMIN_PRINCIPAL = "[email protected]";
-  private static final String DEFAULT_ADMIN_PASSWORD = "hadoop";
-  private static final String DEFAULT_LDAP_URL = "ldaps://ad.example.com";
-  private static final String DEFAULT_PRINCIPAL_CONTAINER_DN = 
"ou=cluster,dc=example,dc=com";
-  private static final String DEFAULT_REALM = "EXAMPLE.COM";
+  private static final String DEFAULT_ADMIN_PRINCIPAL = 
"[email protected]";
+  private static final String DEFAULT_ADMIN_PASSWORD = "Hadoop12345";
+  private static final String DEFAULT_LDAP_URL = "ldaps://10.0.100.4";
+  private static final String DEFAULT_PRINCIPAL_CONTAINER_DN = 
"ou=HDP,DC=HDP01,DC=LOCAL";
+  private static final String DEFAULT_REALM = "HDP01.LOCAL";
 
   @Test(expected = KerberosKDCConnectionException.class)
   public void testOpenExceptionLdapUrlNotProvided() throws Exception {
@@ -220,21 +220,23 @@ public class ADKerberosOperationHandlerTest extends 
EasyMockSupport {
     }
 
     if (containerDN == null) {
-      containerDN = "";
+      containerDN = DEFAULT_PRINCIPAL_CONTAINER_DN;
     }
 
     KerberosCredential credentials = new KerberosCredential(principal, 
password, null);
 
     handler.open(credentials, realm, ldapUrl, containerDN);
 
+    System.out.println("Test Admin Credentials: " + 
handler.testAdministratorCredentials());
     // does the principal already exist?
     System.out.println("Principal exists: " + 
handler.principalExists("nn/c1508.ambari.apache.org"));
 
     //create principal
-    handler.createServicePrincipal("nn/c1508.ambari.apache.org", "welcome");
+    handler.createPrincipal("nn/c1508.ambari.apache.org@" + 
DEFAULT_REALM.toLowerCase(), handler.createSecurePassword(), true);
+    handler.createPrincipal("nn/c1508.ambari.apache.org", 
handler.createSecurePassword(), true);
 
     //update the password
-    handler.setPrincipalPassword("nn/c1508.ambari.apache.org", "welcome10");
+    handler.setPrincipalPassword("nn/c1508.ambari.apache.org", 
handler.createSecurePassword());
 
     // remove the principal
     // handler.removeServicePrincipal("nn/c1508.ambari.apache.org");

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java
index 5ec519d..7cb37be 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java
@@ -36,4 +36,8 @@ public class KerberosOperationHandlerFactoryTest {
       new 
KerberosOperationHandlerFactory().getKerberosOperationHandler(KDCType.ACTIVE_DIRECTORY).getClass());
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testForNull() {
+    Assert.assertNull(new 
KerberosOperationHandlerFactory().getKerberosOperationHandler(null));
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java
index e86656d..40686d6 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java
@@ -220,7 +220,7 @@ public abstract class KerberosOperationHandlerTest {
       }
 
       @Override
-      public Integer createServicePrincipal(String principal, String password) 
throws KerberosOperationException {
+      public Integer createPrincipal(String principal, String password, 
boolean serivce) throws KerberosOperationException{
         return 0;
       }
 
@@ -230,7 +230,7 @@ public abstract class KerberosOperationHandlerTest {
       }
 
       @Override
-      public boolean removeServicePrincipal(String principal) throws 
KerberosOperationException {
+      public boolean removePrincipal(String principal) throws 
KerberosOperationException {
         return false;
       }
     };

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java
index 4a1e399..bd0e938 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java
@@ -51,6 +51,8 @@ public class MITKerberosOperationHandlerTest extends 
EasyMockSupport {
     try {
       handler.setPrincipalPassword(DEFAULT_ADMIN_PRINCIPAL, "");
       Assert.fail("KerberosOperationException not thrown for empty password");
+      handler.createPrincipal("", "1234", false);
+      Assert.fail("AmbariException not thrown for empty principal");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
@@ -76,28 +78,28 @@ public class MITKerberosOperationHandlerTest extends 
EasyMockSupport {
     handler.open(new KerberosCredential(DEFAULT_ADMIN_PRINCIPAL, 
DEFAULT_ADMIN_PASSWORD, null), DEFAULT_REALM);
 
     try {
-      handler.createServicePrincipal(DEFAULT_ADMIN_PRINCIPAL, null);
+      handler.createPrincipal(DEFAULT_ADMIN_PRINCIPAL, null, false);
       Assert.fail("KerberosOperationException not thrown for null password");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
 
     try {
-      handler.createServicePrincipal(DEFAULT_ADMIN_PRINCIPAL, "");
+      handler.createPrincipal(DEFAULT_ADMIN_PRINCIPAL, "", false);
       Assert.fail("KerberosOperationException not thrown for empty password");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
 
     try {
-      handler.createServicePrincipal(null, DEFAULT_ADMIN_PASSWORD);
+      handler.createPrincipal(null, DEFAULT_ADMIN_PASSWORD, false);
       Assert.fail("KerberosOperationException not thrown for null principal");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
 
     try {
-      handler.createServicePrincipal("", DEFAULT_ADMIN_PASSWORD);
+      handler.createPrincipal("", DEFAULT_ADMIN_PASSWORD, false);
       Assert.fail("KerberosOperationException not thrown for empty principal");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());

http://git-wip-us.apache.org/repos/asf/ambari/blob/65393a52/ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java
----------------------------------------------------------------------
diff --git 
a/ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java
 
b/ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java
index 3bbb220..9a4a042 100644
--- 
a/ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java
+++ 
b/ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java
@@ -29,17 +29,17 @@ public class KerberosPrincipalDescriptorTest {
   public static final String JSON_VALUE =
       "{" +
           "\"value\": \"service/_HOST@_REALM\"," +
-          "\"type\": \"service\"," +
           "\"configuration\": 
\"service-site/service.component.kerberos.principal\"," +
+          "\"type\": \"service\"," +
           "\"local_username\": \"localUser\"" +
           "}";
 
   public static final Map<String, Object> MAP_VALUE =
       new HashMap<String, Object>() {
         {
-          put("value", "HTTP/_HOST@_REALM");
-          put("type", "service");
+          put("value", "user@_REALM");
           put("configuration", 
"service-site/service.component.kerberos.https.principal");
+          put("type", "user");
           put("local_username", null);
         }
       };
@@ -49,21 +49,24 @@ public class KerberosPrincipalDescriptorTest {
     Assert.assertFalse(principalDescriptor.isContainer());
     Assert.assertEquals("service/_HOST@_REALM", 
principalDescriptor.getValue());
     Assert.assertEquals("service-site/service.component.kerberos.principal", 
principalDescriptor.getConfiguration());
+    Assert.assertEquals(KerberosPrincipalType.SERVICE, 
principalDescriptor.getType());
     Assert.assertEquals("localUser", principalDescriptor.getLocalUsername());
   }
 
   public static void validateFromMap(KerberosPrincipalDescriptor 
principalDescriptor) {
     Assert.assertNotNull(principalDescriptor);
     Assert.assertFalse(principalDescriptor.isContainer());
-    Assert.assertEquals("HTTP/_HOST@_REALM", principalDescriptor.getValue());
+    Assert.assertEquals("user@_REALM", principalDescriptor.getValue());
     
Assert.assertEquals("service-site/service.component.kerberos.https.principal", 
principalDescriptor.getConfiguration());
+    Assert.assertEquals(KerberosPrincipalType.USER, 
principalDescriptor.getType());
     Assert.assertNull(principalDescriptor.getLocalUsername());
   }
 
   public static void validateUpdatedData(KerberosPrincipalDescriptor 
principalDescriptor) {
     Assert.assertNotNull(principalDescriptor);
-    Assert.assertEquals("HTTP/_HOST@_REALM", principalDescriptor.getValue());
+    Assert.assertEquals("user@_REALM", principalDescriptor.getValue());
     
Assert.assertEquals("service-site/service.component.kerberos.https.principal", 
principalDescriptor.getConfiguration());
+    Assert.assertEquals(KerberosPrincipalType.USER, 
principalDescriptor.getType());
     Assert.assertEquals("localUser", principalDescriptor.getLocalUsername());
   }
 

Reply via email to