Browse Source

AMBARI-10236. Principal and Keytab configuration specifications are ignored when disabling Kerberos (rlevas)

Robert Levas 10 years ago
parent
commit
fe39b6460b
26 changed files with 769 additions and 514 deletions
  1. 29 25
      ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
  2. 126 64
      ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
  3. 1 1
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java
  4. 5 5
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileWriter.java
  5. 5 9
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java
  6. 1 1
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
  7. 0 115
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileBuilder.java
  8. 1 1
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFile.java
  9. 3 3
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileReader.java
  10. 44 0
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileReaderFactory.java
  11. 5 7
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileWriter.java
  12. 44 0
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileWriterFactory.java
  13. 48 0
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosDataFile.java
  14. 5 5
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFile.java
  15. 6 6
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileReader.java
  16. 45 0
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileReaderFactory.java
  17. 100 0
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileWriter.java
  18. 44 0
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileWriterFactory.java
  19. 22 17
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
  20. 22 47
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/UpdateKerberosConfigsServerAction.java
  21. 26 30
      ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java
  22. 43 0
      ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
  23. 30 27
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileTest.java
  24. 74 79
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileTest.java
  25. 8 8
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerActionTest.java
  26. 32 64
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/UpdateKerberosConfigsServerActionTest.java

+ 29 - 25
ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java

@@ -53,8 +53,8 @@ import org.apache.ambari.server.events.publishers.AmbariEventPublisher;
 import org.apache.ambari.server.events.publishers.VersionEventPublisher;
 import org.apache.ambari.server.metadata.ActionMetadata;
 import org.apache.ambari.server.orm.dao.KerberosPrincipalHostDAO;
-import org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile;
-import org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileReader;
+import org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReader;
+import org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReaderFactory;
 import org.apache.ambari.server.serveraction.kerberos.KerberosServerAction;
 import org.apache.ambari.server.state.AgentVersion;
 import org.apache.ambari.server.state.Alert;
@@ -162,6 +162,12 @@ public class HeartBeatHandler {
   @Inject
   private KerberosPrincipalHostDAO kerberosPrincipalHostDAO;
 
+  /**
+   * KerberosIdentityDataFileReaderFactory used to create KerberosIdentityDataFileReader instances
+   */
+  @Inject
+  private KerberosIdentityDataFileReaderFactory kerberosIdentityDataFileReaderFactory;
+
   private Map<String, Long> hostResponseIds = new ConcurrentHashMap<String, Long>();
 
   private Map<String, HeartBeatResponse> hostResponses = new ConcurrentHashMap<String, HeartBeatResponse>();
@@ -1050,18 +1056,18 @@ public class HeartBeatHandler {
   void injectKeytab(ExecutionCommand ec, String command, String targetHost) throws AmbariException {
     List<Map<String, String>> kcp = ec.getKerberosCommandParams();
     String dataDir = ec.getCommandParams().get(KerberosServerAction.DATA_DIRECTORY);
-    KerberosActionDataFileReader reader = null;
+    KerberosIdentityDataFileReader reader = null;
 
     try {
-      reader = new KerberosActionDataFileReader(new File(dataDir, KerberosActionDataFile.DATA_FILE_NAME));
+      reader = kerberosIdentityDataFileReaderFactory.createKerberosIdentityDataFileReader(new File(dataDir, KerberosIdentityDataFileReader.DATA_FILE_NAME));
 
       for (Map<String, String> record : reader) {
-        String hostName = record.get(KerberosActionDataFile.HOSTNAME);
+        String hostName = record.get(KerberosIdentityDataFileReader.HOSTNAME);
 
         if (targetHost.equalsIgnoreCase(hostName)) {
 
           if ("SET_KEYTAB".equalsIgnoreCase(command)) {
-            String keytabFilePath = record.get(KerberosActionDataFile.KEYTAB_FILE_PATH);
+            String keytabFilePath = record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH);
 
             if (keytabFilePath != null) {
 
@@ -1070,20 +1076,18 @@ public class HeartBeatHandler {
 
               if (keytabFile.canRead()) {
                 Map<String, String> keytabMap = new HashMap<String, String>();
-                String principal = record.get(KerberosActionDataFile.PRINCIPAL);
-                String isService = record.get(KerberosActionDataFile.SERVICE);
-
-                keytabMap.put(KerberosActionDataFile.HOSTNAME, hostName);
-                keytabMap.put(KerberosActionDataFile.SERVICE, isService);
-                keytabMap.put(KerberosActionDataFile.COMPONENT, record.get(KerberosActionDataFile.COMPONENT));
-                keytabMap.put(KerberosActionDataFile.PRINCIPAL, principal);
-                keytabMap.put(KerberosActionDataFile.PRINCIPAL_CONFIGURATION, record.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-                keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_PATH, keytabFilePath);
-                keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME, record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-                keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS, record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-                keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME, record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-                keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS, record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-                keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION, record.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
+                String principal = record.get(KerberosIdentityDataFileReader.PRINCIPAL);
+                String isService = record.get(KerberosIdentityDataFileReader.SERVICE);
+
+                keytabMap.put(KerberosIdentityDataFileReader.HOSTNAME, hostName);
+                keytabMap.put(KerberosIdentityDataFileReader.SERVICE, isService);
+                keytabMap.put(KerberosIdentityDataFileReader.COMPONENT, record.get(KerberosIdentityDataFileReader.COMPONENT));
+                keytabMap.put(KerberosIdentityDataFileReader.PRINCIPAL, principal);
+                keytabMap.put(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH, keytabFilePath);
+                keytabMap.put(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_NAME, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_NAME));
+                keytabMap.put(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_ACCESS, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_ACCESS));
+                keytabMap.put(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_NAME, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_NAME));
+                keytabMap.put(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_ACCESS, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_ACCESS));
 
                 BufferedInputStream bufferedIn = new BufferedInputStream(new FileInputStream(keytabFile));
                 byte[] keytabContent = IOUtils.toByteArray(bufferedIn);
@@ -1096,11 +1100,11 @@ public class HeartBeatHandler {
           } else if ("REMOVE_KEYTAB".equalsIgnoreCase(command)) {
             Map<String, String> keytabMap = new HashMap<String, String>();
 
-            keytabMap.put(KerberosActionDataFile.HOSTNAME, hostName);
-            keytabMap.put(KerberosActionDataFile.SERVICE, record.get(KerberosActionDataFile.SERVICE));
-            keytabMap.put(KerberosActionDataFile.COMPONENT, record.get(KerberosActionDataFile.COMPONENT));
-            keytabMap.put(KerberosActionDataFile.PRINCIPAL, record.get(KerberosActionDataFile.PRINCIPAL));
-            keytabMap.put(KerberosActionDataFile.KEYTAB_FILE_PATH, record.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
+            keytabMap.put(KerberosIdentityDataFileReader.HOSTNAME, hostName);
+            keytabMap.put(KerberosIdentityDataFileReader.SERVICE, record.get(KerberosIdentityDataFileReader.SERVICE));
+            keytabMap.put(KerberosIdentityDataFileReader.COMPONENT, record.get(KerberosIdentityDataFileReader.COMPONENT));
+            keytabMap.put(KerberosIdentityDataFileReader.PRINCIPAL, record.get(KerberosIdentityDataFileReader.PRINCIPAL));
+            keytabMap.put(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH));
 
             kcp.add(keytabMap);
           }

+ 126 - 64
ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java

@@ -51,12 +51,12 @@ import org.apache.ambari.server.serveraction.kerberos.CreatePrincipalsServerActi
 import org.apache.ambari.server.serveraction.kerberos.DestroyPrincipalsServerAction;
 import org.apache.ambari.server.serveraction.kerberos.FinalizeKerberosServerAction;
 import org.apache.ambari.server.serveraction.kerberos.KDCType;
-import org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile;
-import org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileBuilder;
+import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileWriter;
+import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileWriterFactory;
+import org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileWriter;
 import org.apache.ambari.server.serveraction.kerberos.KerberosAdminAuthenticationException;
-import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFile;
-import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileBuilder;
 import org.apache.ambari.server.serveraction.kerberos.KerberosCredential;
+import org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileWriterFactory;
 import org.apache.ambari.server.serveraction.kerberos.KerberosInvalidConfigurationException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosKDCConnectionException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosLDAPContainerException;
@@ -164,6 +164,12 @@ public class KerberosHelper {
   @Inject
   private KerberosDescriptorFactory kerberosDescriptorFactory;
 
+  @Inject
+  private KerberosIdentityDataFileWriterFactory kerberosIdentityDataFileWriterFactory;
+
+  @Inject
+  private KerberosConfigDataFileWriterFactory kerberosConfigDataFileWriterFactory;
+
   /**
    * Used to get kerberos descriptors associated with the cluster or stack.
    * Currently not available via injection.
@@ -686,7 +692,7 @@ public class KerberosHelper {
       if ((hosts != null) && !hosts.isEmpty()) {
         List<ServiceComponentHost> serviceComponentHostsToProcess = new ArrayList<ServiceComponentHost>();
         KerberosDescriptor kerberosDescriptor = getKerberosDescriptor(cluster);
-        KerberosActionDataFileBuilder kerberosActionDataFileBuilder = null;
+        KerberosIdentityDataFileWriter kerberosIdentityDataFileWriter = null;
         Map<String, String> kerberosDescriptorProperties = kerberosDescriptor.getProperties();
         Map<String, Map<String, String>> kerberosConfigurations = new HashMap<String, Map<String, String>>();
 
@@ -708,7 +714,7 @@ public class KerberosHelper {
         File dataDirectory = createTemporaryDirectory();
 
         // Create the file used to store details about principals and keytabs to create
-        File indexFile = new File(dataDirectory, KerberosActionDataFile.DATA_FILE_NAME);
+        File identityDataFile = new File(dataDirectory, KerberosIdentityDataFileWriter.DATA_FILE_NAME);
 
         try {
           // Iterate over the hosts in the cluster to find the components installed in each.  For each
@@ -750,14 +756,14 @@ public class KerberosHelper {
                     int identitiesAdded = 0;
                     List<KerberosIdentityDescriptor> serviceIdentities = serviceDescriptor.getIdentities(true);
 
-                    // Lazily create the KerberosActionDataFileBuilder instance...
-                    if (kerberosActionDataFileBuilder == null) {
-                      kerberosActionDataFileBuilder = new KerberosActionDataFileBuilder(indexFile);
+                    // Lazily create the KerberosIdentityDataFileWriter instance...
+                    if (kerberosIdentityDataFileWriter == null) {
+                      kerberosIdentityDataFileWriter = kerberosIdentityDataFileWriterFactory.createKerberosIdentityDataFileWriter(identityDataFile);
                     }
 
                     // Add service-level principals (and keytabs)
-                    identitiesAdded += addIdentities(kerberosActionDataFileBuilder, serviceIdentities,
-                        identityFilter, hostname, serviceName, componentName, configurations);
+                    identitiesAdded += addIdentities(kerberosIdentityDataFileWriter, serviceIdentities,
+                        identityFilter, hostname, serviceName, componentName, kerberosConfigurations, configurations);
 
                     // If there is no filter or the filter contains the current component name,
                     // test to see if this component should be process by querying the handler...
@@ -773,8 +779,8 @@ public class KerberosHelper {
                             componentDescriptor.getConfigurations(true), configurations);
 
                         // Add component-level principals (and keytabs)
-                        identitiesAdded += addIdentities(kerberosActionDataFileBuilder, componentIdentities,
-                            identityFilter, hostname, serviceName, componentName, configurations);
+                        identitiesAdded += addIdentities(kerberosIdentityDataFileWriter, componentIdentities,
+                            identityFilter, hostname, serviceName, componentName, kerberosConfigurations, configurations);
                       }
                     }
 
@@ -787,14 +793,14 @@ public class KerberosHelper {
             }
           }
         } catch (IOException e) {
-          String message = String.format("Failed to write index file - %s", indexFile.getAbsolutePath());
+          String message = String.format("Failed to write index file - %s", identityDataFile.getAbsolutePath());
           LOG.error(message);
           throw new AmbariException(message, e);
         } finally {
-          if (kerberosActionDataFileBuilder != null) {
+          if (kerberosIdentityDataFileWriter != null) {
             // Make sure the data file is closed
             try {
-              kerberosActionDataFileBuilder.close();
+              kerberosIdentityDataFileWriter.close();
             } catch (IOException e) {
               LOG.warn("Failed to close the index file writer", e);
             }
@@ -930,7 +936,7 @@ public class KerberosHelper {
       if ((hosts != null) && !hosts.isEmpty()) {
         List<ServiceComponentHost> serviceComponentHostsToProcess = new ArrayList<ServiceComponentHost>();
         KerberosDescriptor kerberosDescriptor = getKerberosDescriptor(cluster);
-        KerberosActionDataFileBuilder kerberosActionDataFileBuilder = null;
+        KerberosIdentityDataFileWriter kerberosIdentityDataFileWriter = null;
         Map<String, String> kerberosDescriptorProperties = kerberosDescriptor.getProperties();
 
         // While iterating over all the ServiceComponentHosts find hosts that have KERBEROS_CLIENT
@@ -945,7 +951,7 @@ public class KerberosHelper {
         File dataDirectory = createTemporaryDirectory();
 
         // Create the file used to store details about principals and keytabs to create
-        File indexFile = new File(dataDirectory, KerberosActionDataFile.DATA_FILE_NAME);
+        File identityDataFile = new File(dataDirectory, KerberosIdentityDataFileWriter.DATA_FILE_NAME);
 
         // Create a special identity for the test user
         KerberosIdentityDescriptor identity = new KerberosIdentityDescriptor(new HashMap<String, Object>() {
@@ -1016,14 +1022,14 @@ public class KerberosHelper {
 
                   int identitiesAdded = 0;
 
-                  // Lazily create the KerberosActionDataFileBuilder instance...
-                  if (kerberosActionDataFileBuilder == null) {
-                    kerberosActionDataFileBuilder = new KerberosActionDataFileBuilder(indexFile);
+                  // Lazily create the KerberosIdentityDataFileWriter instance...
+                  if (kerberosIdentityDataFileWriter == null) {
+                    kerberosIdentityDataFileWriter = kerberosIdentityDataFileWriterFactory.createKerberosIdentityDataFileWriter(identityDataFile);
                   }
 
                   // Add service-level principals (and keytabs)
-                  identitiesAdded += addIdentities(kerberosActionDataFileBuilder, Collections.singleton(identity),
-                      null, hostname, serviceName, componentName, configurations);
+                  identitiesAdded += addIdentities(kerberosIdentityDataFileWriter, Collections.singleton(identity),
+                      null, hostname, serviceName, componentName, null, configurations);
 
                   if (identitiesAdded > 0) {
                     // Add the relevant principal name and keytab file data to the command params state
@@ -1041,14 +1047,14 @@ public class KerberosHelper {
             }
           }
         } catch (IOException e) {
-          String message = String.format("Failed to write index file - %s", indexFile.getAbsolutePath());
+          String message = String.format("Failed to write index file - %s", identityDataFile.getAbsolutePath());
           LOG.error(message);
           throw new AmbariException(message, e);
         } finally {
-          if (kerberosActionDataFileBuilder != null) {
+          if (kerberosIdentityDataFileWriter != null) {
             // Make sure the data file is closed
             try {
-              kerberosActionDataFileBuilder.close();
+              kerberosIdentityDataFileWriter.close();
             } catch (IOException e) {
               LOG.warn("Failed to close the index file writer", e);
             }
@@ -1369,6 +1375,48 @@ public class KerberosHelper {
     return configurations;
   }
 
+  /**
+   * Merges the specified configuration property in a map of configuration types.
+   * The supplied property is processed to replace variables using the replacement Map.
+   * <p/>
+   * See {@link org.apache.ambari.server.state.kerberos.KerberosDescriptor#replaceVariables(String, java.util.Map)}
+   * for information on variable replacement.
+   *
+   * @param configurations             the Map of configuration types to update
+   * @param configurationSpecification the config-type/property_name value specifying the property to set
+   * @param value                      the value of the property to set
+   * @param replacements               a Map of (grouped) replacement values
+   * @throws AmbariException
+   */
+  private void mergeConfiguration(Map<String, Map<String, String>> configurations,
+                                  String configurationSpecification,
+                                  String value,
+                                  Map<String, Map<String, String>> replacements) throws AmbariException {
+
+    if (configurationSpecification != null) {
+      String[] parts = configurationSpecification.split("/");
+      if (parts.length == 2) {
+        String type = parts[0];
+        String property = parts[1];
+
+        mergeConfigurations(configurations, type, Collections.singletonMap(property, value), replacements);
+      }
+    }
+  }
+
+  /**
+   * Merges configuration from a Map of configuration updates into a main configurations Map.  Each
+   * property in the updates Map is processed to replace variables using the replacement Map.
+   * <p/>
+   * See {@link org.apache.ambari.server.state.kerberos.KerberosDescriptor#replaceVariables(String, java.util.Map)}
+   * for information on variable replacement.
+   *
+   * @param configurations a Map of configurations
+   * @param type           the configuration type
+   * @param updates        a Map of property updates
+   * @param replacements   a Map of (grouped) replacement values
+   * @throws AmbariException
+   */
   private void mergeConfigurations(Map<String, Map<String, String>> configurations, String type,
                                    Map<String, String> updates,
                                    Map<String, Map<String, String>> replacements) throws AmbariException {
@@ -1389,27 +1437,30 @@ public class KerberosHelper {
   }
 
   /**
-   * Adds identities to the KerberosActionDataFileBuilder.
+   * Adds identities to the KerberosIdentityDataFileWriter.
    *
-   * @param kerberosActionDataFileBuilder a KerberosActionDataFileBuilder to use for storing identity
-   *                                      records
-   * @param identities                    a List of KerberosIdentityDescriptors to add to the data
-   *                                      file
-   * @param identityFilter                a Collection of identity names indicating the relevant identities -
-   *                                      if null, no filter is relevant; if empty, the filter indicates no
-   *                                      relevant identities
-   * @param hostname                      the relevant hostname
-   * @param serviceName                   the relevant service name
-   * @param componentName                 the relevant component name
-   * @param configurations                a Map of configurations to use a replacements for variables
-   *                                      in identity fields
+   * @param kerberosIdentityDataFileWriter a KerberosIdentityDataFileWriter to use for storing identity
+   *                                        records
+   * @param identities                      a List of KerberosIdentityDescriptors to add to the data
+   *                                        file
+   * @param identityFilter                  a Collection of identity names indicating the relevant identities -
+   *                                        if null, no filter is relevant; if empty, the filter indicates no
+   *                                        relevant identities
+   * @param hostname                        the relevant hostname
+   * @param serviceName                     the relevant service name
+   * @param componentName                   the relevant component name
+   * @param kerberosConfigurations          a map of the configurations to update with identity-specific
+   *                                        values
+   * @param configurations                  a Map of configurations to use a replacements for variables
+   *                                        in identity fields
    * @return an integer indicating the number of identities added to the data file
    * @throws java.io.IOException if an error occurs while writing a record to the data file
    */
-  private int addIdentities(KerberosActionDataFileBuilder kerberosActionDataFileBuilder,
+  private int addIdentities(KerberosIdentityDataFileWriter kerberosIdentityDataFileWriter,
                             Collection<KerberosIdentityDescriptor> identities,
                             Collection<String> identityFilter, String hostname, String serviceName,
-                            String componentName, Map<String, Map<String, String>> configurations)
+                            String componentName, Map<String, Map<String, String>> kerberosConfigurations,
+                            Map<String, Map<String, String>> configurations)
       throws IOException {
     int identitiesAdded = 0;
 
@@ -1436,6 +1487,7 @@ public class KerberosHelper {
             String keytabFileGroupName = null;
             String keytabFileGroupAccess = null;
             String keytabFileConfiguration = null;
+            boolean keytabIsCachable = false;
 
             if (keytabDescriptor != null) {
               keytabFilePath = KerberosDescriptor.replaceVariables(keytabDescriptor.getFile(), configurations);
@@ -1444,23 +1496,28 @@ public class KerberosHelper {
               keytabFileGroupName = KerberosDescriptor.replaceVariables(keytabDescriptor.getGroupName(), configurations);
               keytabFileGroupAccess = KerberosDescriptor.replaceVariables(keytabDescriptor.getGroupAccess(), configurations);
               keytabFileConfiguration = KerberosDescriptor.replaceVariables(keytabDescriptor.getConfiguration(), configurations);
+              keytabIsCachable = keytabDescriptor.isCachable();
             }
 
             // Append an entry to the action data file builder...
-            kerberosActionDataFileBuilder.addRecord(
+            kerberosIdentityDataFileWriter.writeRecord(
                 hostname,
                 serviceName,
                 componentName,
                 principal,
                 principalType,
-                principalConfiguration,
                 keytabFilePath,
                 keytabFileOwnerName,
                 keytabFileOwnerAccess,
                 keytabFileGroupName,
                 keytabFileGroupAccess,
-                keytabFileConfiguration,
-                (keytabDescriptor.isCachable()) ? "true" : "false");
+                (keytabIsCachable) ? "true" : "false");
+
+            // Add the principal-related configuration to the map of configurations
+            mergeConfiguration(kerberosConfigurations, principalConfiguration, principal, null);
+
+            // Add the keytab-related configuration to the map of configurations
+            mergeConfiguration(kerberosConfigurations, keytabFileConfiguration, keytabFilePath, null);
 
             identitiesAdded++;
           }
@@ -2240,10 +2297,10 @@ public class KerberosHelper {
       // If there are configurations to set, create a (temporary) data file to store the configuration
       // updates and fill it will the relevant configurations.
       if (!kerberosConfigurations.isEmpty()) {
-        File configFile = new File(dataDirectory, KerberosConfigDataFile.DATA_FILE_NAME);
-        KerberosConfigDataFileBuilder kerberosConfDataFileBuilder = null;
+        File configFile = new File(dataDirectory, KerberosConfigDataFileWriter.DATA_FILE_NAME);
+        KerberosConfigDataFileWriter kerberosConfDataFileWriter = null;
         try {
-          kerberosConfDataFileBuilder = new KerberosConfigDataFileBuilder(configFile);
+          kerberosConfDataFileWriter = kerberosConfigDataFileWriterFactory.createKerberosConfigDataFileWriter(configFile);
 
           for (Map.Entry<String, Map<String, String>> entry : kerberosConfigurations.entrySet()) {
             String type = entry.getKey();
@@ -2251,10 +2308,10 @@ public class KerberosHelper {
 
             if (properties != null) {
               for (Map.Entry<String, String> configTypeEntry : properties.entrySet()) {
-                kerberosConfDataFileBuilder.addRecord(type,
+                kerberosConfDataFileWriter.addRecord(type,
                     configTypeEntry.getKey(),
                     configTypeEntry.getValue(),
-                    KerberosConfigDataFile.OPERATION_TYPE_SET);
+                    KerberosConfigDataFileWriter.OPERATION_TYPE_SET);
               }
             }
           }
@@ -2263,9 +2320,9 @@ public class KerberosHelper {
           LOG.error(message);
           throw new AmbariException(message, e);
         } finally {
-          if (kerberosConfDataFileBuilder != null) {
+          if (kerberosConfDataFileWriter != null) {
             try {
-              kerberosConfDataFileBuilder.close();
+              kerberosConfDataFileWriter.close();
             } catch (IOException e) {
               LOG.warn("Failed to close the kerberos configurations file writer", e);
             }
@@ -2375,8 +2432,8 @@ public class KerberosHelper {
       // updates and fill it will the relevant configurations.
       if (!kerberosConfigurations.isEmpty()) {
         Map<String, Collection<String>> configurationsToRemove = new HashMap<String, Collection<String>>();
-        File configFile = new File(dataDirectory, KerberosConfigDataFile.DATA_FILE_NAME);
-        KerberosConfigDataFileBuilder kerberosConfDataFileBuilder = null;
+        File configFile = new File(dataDirectory, KerberosConfigDataFileWriter.DATA_FILE_NAME);
+        KerberosConfigDataFileWriter kerberosConfDataFileWriter = null;
 
         // Fill the configurationsToRemove map with all Kerberos-related configurations.  Values
         // needed to be kept will have new values from the stack definition and thus pruned from
@@ -2430,21 +2487,26 @@ public class KerberosHelper {
         }
 
         try {
-          kerberosConfDataFileBuilder = new KerberosConfigDataFileBuilder(configFile);
+          kerberosConfDataFileWriter = kerberosConfigDataFileWriterFactory.createKerberosConfigDataFileWriter(configFile);
 
           for (Map.Entry<String, Map<String, String>> entry : kerberosConfigurations.entrySet()) {
             String type = entry.getKey();
             Map<String, String> properties = entry.getValue();
+            Collection<String> propertiesToRemove = configurationsToRemove.get(type);
 
             if (properties != null) {
               for (Map.Entry<String, String> configTypeEntry : properties.entrySet()) {
-                String value = configTypeEntry.getValue();
+                String propertyName = configTypeEntry.getKey();
 
-                kerberosConfDataFileBuilder.addRecord(type,
-                    configTypeEntry.getKey(),
-                    value,
-                    (value == null) ? KerberosConfigDataFile.OPERATION_TYPE_REMOVE : KerberosConfigDataFile.OPERATION_TYPE_SET
-                );
+                // Ignore properties that should be removed
+                if ((propertiesToRemove == null) || !propertiesToRemove.contains(propertyName)) {
+                  String value = configTypeEntry.getValue();
+                  String operation = (value == null)
+                      ? KerberosConfigDataFileWriter.OPERATION_TYPE_REMOVE
+                      : KerberosConfigDataFileWriter.OPERATION_TYPE_SET;
+
+                  kerberosConfDataFileWriter.addRecord(type, propertyName, value, operation);
+                }
               }
             }
           }
@@ -2456,7 +2518,7 @@ public class KerberosHelper {
 
             if (properties != null) {
               for (String propertyName : properties) {
-                kerberosConfDataFileBuilder.addRecord(type, propertyName, null, KerberosConfigDataFile.OPERATION_TYPE_REMOVE);
+                kerberosConfDataFileWriter.addRecord(type, propertyName, null, KerberosConfigDataFileWriter.OPERATION_TYPE_REMOVE);
               }
             }
           }
@@ -2465,9 +2527,9 @@ public class KerberosHelper {
           LOG.error(message);
           throw new AmbariException(message, e);
         } finally {
-          if (kerberosConfDataFileBuilder != null) {
+          if (kerberosConfDataFileWriter != null) {
             try {
-              kerberosConfDataFileBuilder.close();
+              kerberosConfDataFileWriter.close();
             } catch (IOException e) {
               LOG.warn("Failed to close the kerberos configurations file writer", e);
             }

+ 1 - 1
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileReader.java

@@ -58,7 +58,7 @@ public abstract class AbstractKerberosDataFileReader implements Iterable<Map<Str
    * This may be called multiple times and the appropriate action will occur depending on if the
    * file has been previously opened or closed.
    *
-   * @throws java.io.IOException
+   * @throws java.io.IOException if an error occurs while accessing the file
    */
   public void open() throws IOException {
     if (isClosed()) {

+ 5 - 5
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileBuilder.java → ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/AbstractKerberosDataFileWriter.java

@@ -26,18 +26,18 @@ import java.io.FileWriter;
 import java.io.IOException;
 
 /**
- * AbstractKerberosDataFileBuilder provides a generic facility to write a data file using some
+ * AbstractKerberosDataFileWriter provides a generic facility to write a data file using some
  * underlying record-based file writer.
  * <p/>
  * This class encapsulates a {@link org.apache.commons.csv.CSVPrinter} to create a CSV-formatted file.
  */
-public abstract class AbstractKerberosDataFileBuilder {
+public abstract class AbstractKerberosDataFileWriter {
 
   private File file;
   private CSVPrinter csvPrinter;
 
   /**
-   * Creates a new KerberosConfigDataFileBuilder
+   * Creates a new KerberosConfigDataFileWriter
    * <p/>
    * The file is opened upon creation, so there is no need to manually open it unless manually
    * closed before using.
@@ -45,7 +45,7 @@ public abstract class AbstractKerberosDataFileBuilder {
    * @param file a File declaring where to write the data
    * @throws java.io.IOException
    */
-  public AbstractKerberosDataFileBuilder(File file) throws IOException {
+  public AbstractKerberosDataFileWriter(File file) throws IOException {
     this.file = file;
     open();
   }
@@ -77,7 +77,7 @@ public abstract class AbstractKerberosDataFileBuilder {
   }
 
   /**
-   * Tests this KerberosConfigDataFileBuilder to see if the data file is closed.
+   * Tests this KerberosConfigDataFileWriter to see if the data file is closed.
    *
    * @return true if closed; otherwise false
    */

+ 5 - 9
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java

@@ -39,10 +39,6 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentMap;
 
-import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.HOSTNAME;
-import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.KEYTAB_FILE_IS_CACHABLE;
-import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.KEYTAB_FILE_PATH;
-
 /**
  * CreateKeytabFilesServerAction is a ServerAction implementation that creates keytab files as
  * instructed.
@@ -111,9 +107,9 @@ public class CreateKeytabFilesServerAction extends KerberosServerAction {
    * {@link org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandler} to generate
    * the keytab file. To help avoid filename collisions and to build a structure that is easy to
    * discover, each keytab file is stored in host-specific
-   * ({@link org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile#HOSTNAME})
+   * ({@link org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReader#HOSTNAME})
    * directory using the SHA1 hash of its destination file path
-   * ({@link org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile#KEYTAB_FILE_PATH})
+   * ({@link org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReader#KEYTAB_FILE_PATH})
    * <p/>
    * <pre>
    *   data_directory
@@ -154,8 +150,8 @@ public class CreateKeytabFilesServerAction extends KerberosServerAction {
         Map<String, String> principalPasswordMap = getPrincipalPasswordMap(requestSharedDataContext);
         Map<String, Integer> principalKeyNumberMap = getPrincipalKeyNumberMap(requestSharedDataContext);
 
-        String host = identityRecord.get(HOSTNAME);
-        String keytabFilePath = identityRecord.get(KEYTAB_FILE_PATH);
+        String host = identityRecord.get(KerberosIdentityDataFileReader.HOSTNAME);
+        String keytabFilePath = identityRecord.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH);
 
         if ((host != null) && !host.isEmpty() && (keytabFilePath != null) && !keytabFilePath.isEmpty()) {
           Set<String> visitedPrincipalKeys = visitedIdentities.get(evaluatedPrincipal);
@@ -246,7 +242,7 @@ public class CreateKeytabFilesServerAction extends KerberosServerAction {
                     // and store that location so it can be reused rather than recreate it.
                     KerberosPrincipalEntity principalEntity = kerberosPrincipalDAO.find(evaluatedPrincipal);
                     if (principalEntity != null) {
-                      if (!principalEntity.isService() && ("true".equalsIgnoreCase(identityRecord.get(KEYTAB_FILE_IS_CACHABLE)))) {
+                      if (!principalEntity.isService() && ("true".equalsIgnoreCase(identityRecord.get(KerberosIdentityDataFileReader.KEYTAB_FILE_IS_CACHABLE)))) {
                         File cachedKeytabFile = cacheKeytab(evaluatedPrincipal, keytab);
                         String previousCachedFilePath = principalEntity.getCachedKeytabPath();
                         String cachedKeytabFilePath = ((cachedKeytabFile == null) || !cachedKeytabFile.exists())

+ 1 - 1
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java

@@ -117,7 +117,7 @@ public class CreatePrincipalsServerAction extends KerberosServerAction {
         password = operationHandler.createSecurePassword();
 
         try {
-          boolean servicePrincipal = "service".equalsIgnoreCase(identityRecord.get(KerberosActionDataFile.PRINCIPAL_TYPE));
+          boolean servicePrincipal = "service".equalsIgnoreCase(identityRecord.get(KerberosIdentityDataFileReader.PRINCIPAL_TYPE));
 
           if (operationHandler.principalExists(evaluatedPrincipal)) {
             // Create a new password since we need to know what it is.

+ 0 - 115
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileBuilder.java

@@ -1,115 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.ambari.server.serveraction.kerberos;
-
-import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.*;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.Arrays;
-
-/**
- * KerberosActionDataFileBuilder is an implementation of a KerberosActionDataFile that is used to
- * create a new KerberosActionDataFile.
- * <p/>
- * This class encapsulates a {@link org.apache.commons.csv.CSVPrinter} to create a CSV-formatted file.
- */
-public class KerberosActionDataFileBuilder extends AbstractKerberosDataFileBuilder {
-
-  /**
-   * Creates a new KerberosActionDataFileBuilder
-   * <p/>
-   * The file is opened upon creation, so there is no need to manually open it unless manually
-   * closed before using.
-   *
-   * @param file a File declaring where to write the data
-   * @throws IOException
-   */
-  public KerberosActionDataFileBuilder(File file) throws IOException {
-    super(file);
-  }
-
-
-  /**
-   * Appends a new record to the data file
-   *
-   * @param hostName                a String containing the hostname column data
-   * @param serviceName             a String containing the service name column data
-   * @param serviceComponentName    a String containing the component name column data
-   * @param principal               a String containing the (raw, non-evaluated) principal "pattern"
-   *                                column data
-   * @param principalType           a String declaring the principal type - expecting "service" or "user"
-   * @param principalConfiguration  a String containing the principal's configuration property column data
-   *                                (expected to be the type and name of the configuration property
-   *                                to use to store the evaluated principal data in
-   *                                - i.e., config-type/property)
-   * @param keytabFilePath          a String containing the destination keytab file path column data
-   * @param keytabFileOwnerName     a String containing the keytab file owner name column data
-   * @param keytabFileOwnerAccess   a String containing the keytab file owner access column data
-   *                                (expected to be "r" or "rw")
-   * @param keytabFileGroupName     a String containing the keytab file group name column data
-   * @param keytabFileGroupAccess   a String containing the keytab file group access column data
-   *                                (expected to be "r", "rw", or "")
-   * @param keytabFileConfiguration a String containing the keytab's configuration property column data
-   *                                (expected to be the type and name of the configuration property
-   *                                to use to store the keytab file's absolute path in
-   *                                - i.e., config-type/property)
-   * @param keytabFileCanCache      a String containing a boolean value (true, false) indicating
-   *                                whether the generated keytab can be cached or not
-   * @throws IOException
-   */
-  public void addRecord(String hostName, String serviceName, String serviceComponentName,
-                        String principal, String principalType, String principalConfiguration,
-                        String keytabFilePath, String keytabFileOwnerName,
-                        String keytabFileOwnerAccess, String keytabFileGroupName,
-                        String keytabFileGroupAccess, String keytabFileConfiguration,
-                        String keytabFileCanCache)
-      throws IOException {
-    super.appendRecord(hostName,
-        serviceName,
-        serviceComponentName,
-        principal,
-        principalType,
-        principalConfiguration,
-        keytabFilePath,
-        keytabFileOwnerName,
-        keytabFileOwnerAccess,
-        keytabFileGroupName,
-        keytabFileGroupAccess,
-        keytabFileConfiguration,
-        keytabFileCanCache);
-  }
-
-  @Override
-  protected Iterable<String> getHeaderRecord() {
-    return Arrays.asList(HOSTNAME,
-        SERVICE,
-        COMPONENT,
-        PRINCIPAL,
-        PRINCIPAL_TYPE,
-        PRINCIPAL_CONFIGURATION,
-        KEYTAB_FILE_PATH,
-        KEYTAB_FILE_OWNER_NAME,
-        KEYTAB_FILE_OWNER_ACCESS,
-        KEYTAB_FILE_GROUP_NAME,
-        KEYTAB_FILE_GROUP_ACCESS,
-        KEYTAB_FILE_CONFIGURATION,
-        KEYTAB_FILE_IS_CACHABLE);
-  }
-}

+ 1 - 1
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFile.java

@@ -22,7 +22,7 @@ package org.apache.ambari.server.serveraction.kerberos;
  * KerberosConfigDataFile declares the default data file name and the common record column names
  * for the Kerberos configuration data files.
  */
-public class KerberosConfigDataFile {
+public interface KerberosConfigDataFile extends KerberosDataFile {
   public static final String DATA_FILE_NAME = "configs.dat";
 
   public static final String CONFIGURATION_TYPE = "config";

+ 3 - 3
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileReader.java

@@ -27,7 +27,7 @@ import java.io.IOException;
  * <p/>
  * This class encapsulates a {@link org.apache.commons.csv.CSVParser} to read a CSV-formatted file.
  */
-public class KerberosConfigDataFileReader extends AbstractKerberosDataFileReader {
+public class KerberosConfigDataFileReader extends AbstractKerberosDataFileReader implements KerberosConfigDataFile{
 
   /**
    * Creates a new KerberosConfigDataFileReader
@@ -36,9 +36,9 @@ public class KerberosConfigDataFileReader extends AbstractKerberosDataFileReader
    * closed before using.
    *
    * @param file a File declaring where to write the data
-   * @throws java.io.IOException
+   * @throws java.io.IOException if an error occurs while accessing the file
    */
-  public KerberosConfigDataFileReader(File file) throws IOException {
+  KerberosConfigDataFileReader(File file) throws IOException {
     super(file);
   }
 }

+ 44 - 0
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileReaderFactory.java

@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.serveraction.kerberos;
+
+import com.google.inject.Singleton;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * KerberosConfigDataFileReaderFactory creates KerberosConfigDataFileReader instances.
+ */
+@Singleton
+public class KerberosConfigDataFileReaderFactory {
+  /**
+   * Creates a new KerberosConfigDataFileReader
+   * <p/>
+   * The file is opened upon creation, so there is no need to manually open it unless manually
+   * closed before using.
+   *
+   * @param file a File declaring where to write the data
+   * @return the created KerberosConfigDataFileReader
+   * @throws java.io.IOException if an error occurs while accessing the file
+   */
+  public KerberosConfigDataFileReader createKerberosConfigDataFileReader(File file) throws IOException {
+    return new KerberosConfigDataFileReader(file);
+  }
+}

+ 5 - 7
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileBuilder.java → ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileWriter.java

@@ -18,22 +18,20 @@
 
 package org.apache.ambari.server.serveraction.kerberos;
 
-import static org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFile.*;
-
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
 
 /**
- * KerberosConfigDataFileBuilder is an implementation of a KerberosConfigDataFile that is used to
- * create a new KerberosConfigDataFile.
+ * KerberosConfigDataFileWriter is an implementation of a KerberosConfigDataFile that is used to
+ * create a new KerberosConfigDataFileWriter.
  * <p/>
  * This class encapsulates a {@link org.apache.commons.csv.CSVPrinter} to create a CSV-formatted file.
  */
-public class KerberosConfigDataFileBuilder extends AbstractKerberosDataFileBuilder {
+public class KerberosConfigDataFileWriter extends AbstractKerberosDataFileWriter implements KerberosConfigDataFile {
 
   /**
-   * Creates a new KerberosConfigDataFileBuilder
+   * Creates a new KerberosConfigDataFileWriter
    * <p/>
    * The file is opened upon creation, so there is no need to manually open it unless manually
    * closed before using.
@@ -41,7 +39,7 @@ public class KerberosConfigDataFileBuilder extends AbstractKerberosDataFileBuild
    * @param file a File declaring where to write the data
    * @throws java.io.IOException
    */
-  public KerberosConfigDataFileBuilder(File file) throws IOException {
+  KerberosConfigDataFileWriter(File file) throws IOException {
     super(file);
   }
 

+ 44 - 0
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileWriterFactory.java

@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.serveraction.kerberos;
+
+import com.google.inject.Singleton;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * KerberosConfigDataFileWriterFactory creates KerberosConfigDataFileWriter instances.
+ */
+@Singleton
+public class KerberosConfigDataFileWriterFactory {
+  /**
+   * Creates a new KerberosConfigDataFileWriter
+   * <p/>
+   * The file is opened upon creation, so there is no need to manually open it unless manually
+   * closed before using.
+   *
+   * @param file a File declaring where to write the data
+   * @return the created KerberosConfigDataFileWriter
+   * @throws java.io.IOException if an error occurs while accessing the file
+   */
+  public KerberosConfigDataFileWriter createKerberosConfigDataFileWriter(File file) throws IOException {
+    return new KerberosConfigDataFileWriter(file);
+  }
+}

+ 48 - 0
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosDataFile.java

@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.serveraction.kerberos;
+
+import java.io.IOException;
+
+/**
+ * KerberosDataFile is an interfaced expected to be implemented by all Kerberos data file
+ * implementations
+ */
+public interface KerberosDataFile {
+
+  /**
+   * Opens the data file.
+   * <p/>
+   * This may be called multiple times and the appropriate action should occur depending on if the
+   * file has been previously opened or closed.
+   *
+   * @throws java.io.IOException if an error occurs while opening the file
+   */
+  void open() throws IOException;
+
+  /**
+   * Closes the data file.
+   * <p/>
+   * This may be called multiple times and the appropriate action should occur depending on if the
+   * file has been previously opened or closed.
+   *
+   * @throws java.io.IOException if an error occurs while closing the file
+   */
+  void close() throws IOException;
+}

+ 5 - 5
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFile.java → ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFile.java

@@ -19,23 +19,23 @@
 package org.apache.ambari.server.serveraction.kerberos;
 
 /**
- * KerberosActionDataFile declares the default data file name and the common record column names
+ * KerberosIdentityDataFile declares the default data file name and the common record column names
  * for the Kerberos action (metadata) data files.
  */
-public class KerberosActionDataFile {
-  public static final String DATA_FILE_NAME = "index.dat";
+public interface KerberosIdentityDataFile extends KerberosDataFile {
+  public static final String DATA_FILE_NAME = "identity.dat";
 
   public static final String HOSTNAME = "hostname";
   public static final String SERVICE = "service";
   public static final String COMPONENT = "component";
   public static final String PRINCIPAL = "principal";
   public static final String PRINCIPAL_TYPE = "principal_type";
-  public static final String PRINCIPAL_CONFIGURATION = "principal_configuration";
   public static final String KEYTAB_FILE_PATH = "keytab_file_path";
   public static final String KEYTAB_FILE_OWNER_NAME = "keytab_file_owner_name";
   public static final String KEYTAB_FILE_OWNER_ACCESS = "keytab_file_owner_access";
   public static final String KEYTAB_FILE_GROUP_NAME = "keytab_file_group_name";
   public static final String KEYTAB_FILE_GROUP_ACCESS = "keytab_file_group_access";
-  public static final String KEYTAB_FILE_CONFIGURATION = "keytab_file_configuration";
   public static final String KEYTAB_FILE_IS_CACHABLE = "keytab_file_is_cachable";
+
+
 }

+ 6 - 6
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileReader.java → ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileReader.java

@@ -22,23 +22,23 @@ import java.io.File;
 import java.io.IOException;
 
 /**
- * KerberosActionDataFileReader is an implementation of a KerberosActionDataFile that is used to
- * read existing KerberosActionDataFiles.
+ * KerberosIdentityDataFileReader is an implementation of an AbstractKerberosDataFileReader that is
+ * used to read existing Kerberos identity data files.
  * <p/>
  * This class encapsulates a {@link org.apache.commons.csv.CSVParser} to read a CSV-formatted file.
  */
-public class KerberosActionDataFileReader extends AbstractKerberosDataFileReader {
+public class KerberosIdentityDataFileReader extends AbstractKerberosDataFileReader implements KerberosIdentityDataFile {
 
   /**
-   * Creates a new KerberosActionDataFileReader
+   * Creates a new KerberosIdentityDataFileReader
    * <p/>
    * The file is opened upon creation, so there is no need to manually open it unless manually
    * closed before using.
    *
    * @param file a File declaring where to write the data
-   * @throws IOException
+   * @throws java.io.IOException if an error occurs while accessing the file
    */
-  public KerberosActionDataFileReader(File file) throws IOException {
+  KerberosIdentityDataFileReader(File file) throws IOException {
     super(file);
   }
 }

+ 45 - 0
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileReaderFactory.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.serveraction.kerberos;
+
+import com.google.inject.Singleton;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * KerberosIdentityDataFileReaderFactory creates KerberosIdentityDataFileReader instances.
+ */
+@Singleton
+public class KerberosIdentityDataFileReaderFactory {
+
+  /**
+   * Creates a new KerberosIdentityDataFileReader
+   * <p/>
+   * The file is opened upon creation, so there is no need to manually open it unless manually
+   * closed before using.
+   *
+   * @param file a File declaring where to write the data
+   * @return the created KerberosIdentityDataFileReader
+   * @throws java.io.IOException if an error occurs while accessing the file
+   */
+  public KerberosIdentityDataFileReader createKerberosIdentityDataFileReader(File file) throws IOException {
+    return new KerberosIdentityDataFileReader(file);
+  }
+}

+ 100 - 0
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileWriter.java

@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.serveraction.kerberos;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+
+/**
+ * KerberosIdentityDataFileWriter is an implementation of an AbstractKerberosDataFileWriter that
+ * is used to create a new Kerberos identity data file.
+ * <p/>
+ * This class encapsulates a {@link org.apache.commons.csv.CSVPrinter} to create a CSV-formatted file.
+ */
+public class KerberosIdentityDataFileWriter extends AbstractKerberosDataFileWriter implements KerberosIdentityDataFile {
+
+  /**
+   * Creates a new KerberosIdentityDataFileWriter
+   * <p/>
+   * The file is opened upon creation, so there is no need to manually open it unless manually
+   * closed before using.
+   *
+   * @param file a File declaring where to write the data
+   * @throws IOException
+   */
+  KerberosIdentityDataFileWriter(File file) throws IOException {
+    super(file);
+  }
+
+
+  /**
+   * Appends a new record to the data file
+   *
+   * @param hostName              a String containing the hostname column data
+   * @param serviceName           a String containing the service name column data
+   * @param serviceComponentName  a String containing the component name column data
+   * @param principal             a String containing the (raw, non-evaluated) principal "pattern"
+   *                              column data
+   * @param principalType         a String declaring the principal type - expecting "service" or "user"
+   * @param keytabFilePath        a String containing the destination keytab file path column data
+   * @param keytabFileOwnerName   a String containing the keytab file owner name column data
+   * @param keytabFileOwnerAccess a String containing the keytab file owner access column data
+   *                              (expected to be "r" or "rw")
+   * @param keytabFileGroupName   a String containing the keytab file group name column data
+   * @param keytabFileGroupAccess a String containing the keytab file group access column data
+   *                              (expected to be "r", "rw", or "")
+   * @param keytabFileCanCache    a String containing a boolean value (true, false) indicating
+   *                              whether the generated keytab can be cached or not
+   * @throws IOException
+   */
+  public void writeRecord(String hostName, String serviceName, String serviceComponentName,
+                          String principal, String principalType,
+                          String keytabFilePath, String keytabFileOwnerName,
+                          String keytabFileOwnerAccess, String keytabFileGroupName,
+                          String keytabFileGroupAccess, String keytabFileCanCache)
+      throws IOException {
+    super.appendRecord(hostName,
+        serviceName,
+        serviceComponentName,
+        principal,
+        principalType,
+        keytabFilePath,
+        keytabFileOwnerName,
+        keytabFileOwnerAccess,
+        keytabFileGroupName,
+        keytabFileGroupAccess,
+        keytabFileCanCache);
+  }
+
+  @Override
+  protected Iterable<String> getHeaderRecord() {
+    return Arrays.asList(HOSTNAME,
+        SERVICE,
+        COMPONENT,
+        PRINCIPAL,
+        PRINCIPAL_TYPE,
+        KEYTAB_FILE_PATH,
+        KEYTAB_FILE_OWNER_NAME,
+        KEYTAB_FILE_OWNER_ACCESS,
+        KEYTAB_FILE_GROUP_NAME,
+        KEYTAB_FILE_GROUP_ACCESS,
+        KEYTAB_FILE_IS_CACHABLE);
+  }
+}

+ 44 - 0
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileWriterFactory.java

@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ambari.server.serveraction.kerberos;
+
+import com.google.inject.Singleton;
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ * KerberosIdentityDataFileWriterFactory creates KerberosIdentityDataFileWriter  instances.
+ */
+@Singleton
+public class KerberosIdentityDataFileWriterFactory {
+  /**
+   * Creates a new createKerberosIdentityDataFileWriter
+   * <p/>
+   * The file is opened upon creation, so there is no need to manually open it unless manually
+   * closed before using.
+   *
+   * @param file a File declaring where to write the data
+   * @return the created KerberosIdentityDataFileWriter
+   * @throws java.io.IOException if an error occurs while accessing the file
+   */
+  public KerberosIdentityDataFileWriter createKerberosIdentityDataFileWriter(File file) throws IOException {
+    return new KerberosIdentityDataFileWriter(file);
+  }
+}

+ 22 - 17
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java

@@ -29,7 +29,7 @@ import org.apache.ambari.server.state.Clusters;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile.DATA_FILE_NAME;
+import static org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReader.DATA_FILE_NAME;
 
 import java.io.File;
 import java.io.IOException;
@@ -42,7 +42,7 @@ import java.util.Map;
  * <p/>
  * This class provides helper methods used to get common properties from the command parameters map
  * and iterate through the Kerberos identity metadata file
- * (see {@link org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile}).
+ * (see {@link org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReader}).
  */
 public abstract class KerberosServerAction extends AbstractServerAction {
   /**
@@ -117,6 +117,11 @@ public abstract class KerberosServerAction extends AbstractServerAction {
   @Inject
   private KerberosOperationHandlerFactory kerberosOperationHandlerFactory;
 
+  /**
+   * The KerberosIdentityDataFileReaderFactory to use to obtain KerberosIdentityDataFileReader instances
+   */
+  @Inject
+  private KerberosIdentityDataFileReaderFactory kerberosIdentityDataFileReaderFactory;
 
   /**
    * Given a (command parameter) Map and a property name, attempts to safely retrieve the requested
@@ -305,12 +310,12 @@ public abstract class KerberosServerAction extends AbstractServerAction {
 
   /**
    * Iterates through the Kerberos identity metadata from the
-   * {@link org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile} and calls the
-   * implementing class to handle each identity found.
+   * {@link org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileReader} and calls
+   * the implementing class to handle each identity found.
    * <p/>
    * Using the "data_directory" value from this action's command parameters map, creates a
-   * {@link org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileReader} to parse
-   * the relative index.dat file and iterate through its "records".  Each "record" is process using
+   * {@link KerberosIdentityDataFileReader} to parse
+   * the relative identity.dat file and iterate through its "records".  Each "record" is process using
    * {@link #processRecord(java.util.Map, String, KerberosOperationHandler, java.util.Map)}.
    *
    * @param requestSharedDataContext a Map to be used a shared data among all ServerActions related
@@ -345,14 +350,14 @@ public abstract class KerberosServerAction extends AbstractServerAction {
             LOG.error(message);
             throw new AmbariException(message);
           }
-          // The "index" file may or may not exist in the data directory, depending on if there
-          // is work to do or not.
-          File indexFile = new File(dataDirectory, DATA_FILE_NAME);
+          // The "identity data" file may or may not exist in the data directory, depending on if
+          // there is work to do or not.
+          File identityDataFile = new File(dataDirectory, DATA_FILE_NAME);
 
-          if (indexFile.exists()) {
-            if (!indexFile.canRead()) {
+          if (identityDataFile.exists()) {
+            if (!identityDataFile.canRead()) {
               String message = String.format("Failed to process the identities, cannot read the index file: %s",
-                  indexFile.getAbsolutePath());
+                  identityDataFile.getAbsolutePath());
               actionLog.writeStdErr(message);
               LOG.error(message);
               throw new AmbariException(message);
@@ -378,9 +383,9 @@ public abstract class KerberosServerAction extends AbstractServerAction {
             }
 
             // Create the data file reader to parse and iterate through the records
-            KerberosActionDataFileReader reader = null;
+            KerberosIdentityDataFileReader reader = null;
             try {
-              reader = new KerberosActionDataFileReader(indexFile);
+              reader = kerberosIdentityDataFileReaderFactory.createKerberosIdentityDataFileReader(identityDataFile);
               for (Map<String, String> record : reader) {
                 // Process the current record
                 commandReport = processRecord(record, defaultRealm, handler, requestSharedDataContext);
@@ -397,7 +402,7 @@ public abstract class KerberosServerAction extends AbstractServerAction {
               throw new AmbariException(e.getMessage(), e);
             } catch (IOException e) {
               String message = String.format("Failed to process the identities, cannot read the index file: %s",
-                  indexFile.getAbsolutePath());
+                  identityDataFile.getAbsolutePath());
               actionLog.writeStdErr(message);
               LOG.error(message, e);
               throw new AmbariException(message, e);
@@ -483,8 +488,8 @@ public abstract class KerberosServerAction extends AbstractServerAction {
     CommandReport commandReport = null;
 
     if (record != null) {
-      String principal = record.get(KerberosActionDataFile.PRINCIPAL);
-      String host = record.get(KerberosActionDataFile.HOSTNAME);
+      String principal = record.get(KerberosIdentityDataFileReader.PRINCIPAL);
+      String host = record.get(KerberosIdentityDataFileReader.HOSTNAME);
 
       if (principal != null) {
         // Evaluate the principal "pattern" found in the record to generate the "evaluated principal"

+ 22 - 47
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/UpdateKerberosConfigsServerAction.java

@@ -37,6 +37,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentMap;
 
 /**
@@ -53,6 +54,12 @@ public class UpdateKerberosConfigsServerAction extends AbstractServerAction {
   @Inject
   private ConfigHelper configHelper;
 
+  /**
+   * The KerberosConfigDataFileReaderFactory to use to obtain KerberosConfigDataFileReader instances
+   */
+  @Inject
+  private KerberosConfigDataFileReaderFactory kerberosConfigDataFileReaderFactory;
+
   /**
    * Executes this ServerAction
    * <p/>
@@ -87,65 +94,40 @@ public class UpdateKerberosConfigsServerAction extends AbstractServerAction {
 
       // If the data directory exists, attempt to process further, else assume there is no work to do
       if (dataDirectory.exists()) {
-        KerberosActionDataFileReader indexReader = null;
         KerberosConfigDataFileReader configReader = null;
+        Set<String> configTypes = new HashSet<String>();
 
         try {
-          // If the action data file exists, iterate over the records to find the identity-specific
-          // configuration settings to update
-          File indexFile = new File(dataDirectory, KerberosActionDataFile.DATA_FILE_NAME);
-          if (indexFile.exists()) {
-            indexReader = new KerberosActionDataFileReader(indexFile);
-
-            for (Map<String, String> record : indexReader) {
-              String principal = record.get(KerberosActionDataFile.PRINCIPAL);
-              String principalConfig = record.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION);
-              String[] principalTokens = principalConfig.split("/");
-              if (principalTokens.length == 2) {
-                String principalConfigType = principalTokens[0];
-                String principalConfigProp = principalTokens[1];
-                addConfigTypePropVal(propertiesToSet, principalConfigType, principalConfigProp, principal);
-              }
-
-              String keytabPath = record.get(KerberosActionDataFile.KEYTAB_FILE_PATH);
-              String keytabConfig = record.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION);
-              String[] keytabTokens = keytabConfig.split("/");
-              if (keytabTokens.length == 2) {
-                String keytabConfigType = keytabTokens[0];
-                String keytabConfigProp = keytabTokens[1];
-                addConfigTypePropVal(propertiesToSet, keytabConfigType, keytabConfigProp, keytabPath);
-              }
-            }
-          }
-
           // If the config data file exists, iterate over the records to find the (explicit)
           // configuration settings to update
-          File configFile = new File(dataDirectory, KerberosConfigDataFile.DATA_FILE_NAME);
+          File configFile = new File(dataDirectory, KerberosConfigDataFileReader.DATA_FILE_NAME);
           if (configFile.exists()) {
-            configReader = new KerberosConfigDataFileReader(configFile);
+            configReader = kerberosConfigDataFileReaderFactory.createKerberosConfigDataFileReader(configFile);
             for (Map<String, String> record : configReader) {
-              String configType = record.get(KerberosConfigDataFile.CONFIGURATION_TYPE);
-              String configKey = record.get(KerberosConfigDataFile.KEY);
-              String configVal = record.get(KerberosConfigDataFile.VALUE);
-              String configOp = record.get(KerberosConfigDataFile.OPERATION);
+              String configType = record.get(KerberosConfigDataFileReader.CONFIGURATION_TYPE);
+              String configKey = record.get(KerberosConfigDataFileReader.KEY);
+              String configOp = record.get(KerberosConfigDataFileReader.OPERATION);
+
+              configTypes.add(configType);
 
-              if (KerberosConfigDataFile.OPERATION_TYPE_REMOVE.equals(configOp)) {
+              if (KerberosConfigDataFileReader.OPERATION_TYPE_REMOVE.equals(configOp)) {
                 removeConfigTypeProp(propertiesToRemove, configType, configKey);
               } else {
+                String configVal = record.get(KerberosConfigDataFileReader.VALUE);
                 addConfigTypePropVal(propertiesToSet, configType, configKey, configVal);
               }
             }
           }
 
-          if (!propertiesToSet.isEmpty()) {
+          if (!configTypes.isEmpty()) {
             String configNote = cluster.getSecurityType() == SecurityType.KERBEROS
                 ? "Enabling Kerberos"
                 : "Disabling Kerberos";
 
-            for (Map.Entry<String, Map<String, String>> entry : propertiesToSet.entrySet()) {
-              String type = entry.getKey();
-
-              configHelper.updateConfigType(cluster, controller, type, entry.getValue(), propertiesToRemove.get(type),
+            for (String configType : configTypes) {
+              configHelper.updateConfigType(cluster, controller, configType,
+                  propertiesToSet.get(configType),
+                  propertiesToRemove.get(configType),
                   authenticatedUserName, configNote);
             }
           }
@@ -156,13 +138,6 @@ public class UpdateKerberosConfigsServerAction extends AbstractServerAction {
           commandReport = createCommandReport(1, HostRoleStatus.FAILED, "{}", actionLog.getStdOut(),
               actionLog.getStdErr());
         } finally {
-          if (indexReader != null && !indexReader.isClosed()) {
-            try {
-              indexReader.close();
-            } catch (Throwable t) {
-              // ignored
-            }
-          }
           if (configReader != null && !configReader.isClosed()) {
             try {
               configReader.close();

+ 26 - 30
ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java

@@ -84,8 +84,8 @@ import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
 import org.apache.ambari.server.orm.OrmTestHelper;
 import org.apache.ambari.server.orm.dao.RepositoryVersionDAO;
 import org.apache.ambari.server.orm.entities.RepositoryVersionEntity;
-import org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFile;
-import org.apache.ambari.server.serveraction.kerberos.KerberosActionDataFileBuilder;
+import org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileWriter;
+import org.apache.ambari.server.serveraction.kerberos.KerberosIdentityDataFileWriterFactory;
 import org.apache.ambari.server.serveraction.kerberos.KerberosServerAction;
 import org.apache.ambari.server.state.Alert;
 import org.apache.ambari.server.state.AlertState;
@@ -2494,17 +2494,15 @@ public class TestHeartbeatHandler {
 
     properties = kcp.get(0);
     Assert.assertNotNull(properties);
-    Assert.assertEquals("c6403.ambari.apache.org", properties.get(KerberosActionDataFile.HOSTNAME));
-    Assert.assertEquals("HDFS", properties.get(KerberosActionDataFile.SERVICE));
-    Assert.assertEquals("DATANODE", properties.get(KerberosActionDataFile.COMPONENT));
-    Assert.assertEquals("dn/_HOST@_REALM", properties.get(KerberosActionDataFile.PRINCIPAL));
-    Assert.assertEquals("hdfs-site/dfs.namenode.kerberos.principal", properties.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-    Assert.assertEquals("/etc/security/keytabs/dn.service.keytab", properties.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
-    Assert.assertEquals("hdfs", properties.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-    Assert.assertEquals("r", properties.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-    Assert.assertEquals("hadoop", properties.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-    Assert.assertEquals("", properties.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-    Assert.assertEquals("hdfs-site/dfs.namenode.keytab.file", properties.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
+    Assert.assertEquals("c6403.ambari.apache.org", properties.get(KerberosIdentityDataFileWriter.HOSTNAME));
+    Assert.assertEquals("HDFS", properties.get(KerberosIdentityDataFileWriter.SERVICE));
+    Assert.assertEquals("DATANODE", properties.get(KerberosIdentityDataFileWriter.COMPONENT));
+    Assert.assertEquals("dn/_HOST@_REALM", properties.get(KerberosIdentityDataFileWriter.PRINCIPAL));
+    Assert.assertEquals("/etc/security/keytabs/dn.service.keytab", properties.get(KerberosIdentityDataFileWriter.KEYTAB_FILE_PATH));
+    Assert.assertEquals("hdfs", properties.get(KerberosIdentityDataFileWriter.KEYTAB_FILE_OWNER_NAME));
+    Assert.assertEquals("r", properties.get(KerberosIdentityDataFileWriter.KEYTAB_FILE_OWNER_ACCESS));
+    Assert.assertEquals("hadoop", properties.get(KerberosIdentityDataFileWriter.KEYTAB_FILE_GROUP_NAME));
+    Assert.assertEquals("", properties.get(KerberosIdentityDataFileWriter.KEYTAB_FILE_GROUP_ACCESS));
 
     Assert.assertEquals(Base64.encodeBase64String("hello".getBytes()), kcp.get(0).get(KerberosServerAction.KEYTAB_CONTENT_BASE64));
 
@@ -2516,17 +2514,15 @@ public class TestHeartbeatHandler {
 
     properties = kcp.get(0);
     Assert.assertNotNull(properties);
-    Assert.assertEquals("c6403.ambari.apache.org", properties.get(KerberosActionDataFile.HOSTNAME));
-    Assert.assertEquals("HDFS", properties.get(KerberosActionDataFile.SERVICE));
-    Assert.assertEquals("DATANODE", properties.get(KerberosActionDataFile.COMPONENT));
-    Assert.assertEquals("dn/_HOST@_REALM", properties.get(KerberosActionDataFile.PRINCIPAL));
-    Assert.assertFalse(properties.containsKey(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-    Assert.assertEquals("/etc/security/keytabs/dn.service.keytab", properties.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
-    Assert.assertFalse(properties.containsKey(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-    Assert.assertFalse(properties.containsKey(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-    Assert.assertFalse(properties.containsKey(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-    Assert.assertFalse(properties.containsKey(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-    Assert.assertFalse(properties.containsKey(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
+    Assert.assertEquals("c6403.ambari.apache.org", properties.get(KerberosIdentityDataFileWriter.HOSTNAME));
+    Assert.assertEquals("HDFS", properties.get(KerberosIdentityDataFileWriter.SERVICE));
+    Assert.assertEquals("DATANODE", properties.get(KerberosIdentityDataFileWriter.COMPONENT));
+    Assert.assertEquals("dn/_HOST@_REALM", properties.get(KerberosIdentityDataFileWriter.PRINCIPAL));
+    Assert.assertEquals("/etc/security/keytabs/dn.service.keytab", properties.get(KerberosIdentityDataFileWriter.KEYTAB_FILE_PATH));
+    Assert.assertFalse(properties.containsKey(KerberosIdentityDataFileWriter.KEYTAB_FILE_OWNER_NAME));
+    Assert.assertFalse(properties.containsKey(KerberosIdentityDataFileWriter.KEYTAB_FILE_OWNER_ACCESS));
+    Assert.assertFalse(properties.containsKey(KerberosIdentityDataFileWriter.KEYTAB_FILE_GROUP_NAME));
+    Assert.assertFalse(properties.containsKey(KerberosIdentityDataFileWriter.KEYTAB_FILE_GROUP_ACCESS));
     Assert.assertFalse(properties.containsKey(KerberosServerAction.KEYTAB_CONTENT_BASE64));
   }
 
@@ -2606,8 +2602,8 @@ public class TestHeartbeatHandler {
 
   private File createTestKeytabData() throws Exception {
     File dataDirectory = temporaryFolder.newFolder();
-    File indexFile = new File(dataDirectory, KerberosActionDataFile.DATA_FILE_NAME);
-    KerberosActionDataFileBuilder kerberosActionDataFileBuilder = new KerberosActionDataFileBuilder(indexFile);
+    File identityDataFile = new File(dataDirectory, KerberosIdentityDataFileWriter.DATA_FILE_NAME);
+    KerberosIdentityDataFileWriter kerberosIdentityDataFileWriter = injector.getInstance(KerberosIdentityDataFileWriterFactory.class).createKerberosIdentityDataFileWriter(identityDataFile);
     File hostDirectory = new File(dataDirectory, "c6403.ambari.apache.org");
 
     File keytabFile;
@@ -2616,12 +2612,12 @@ public class TestHeartbeatHandler {
     else
       throw new Exception("Failed to create " + hostDirectory.getAbsolutePath());
 
-    kerberosActionDataFileBuilder.addRecord("c6403.ambari.apache.org", "HDFS", "DATANODE",
-        "dn/_HOST@_REALM", "service", "hdfs-site/dfs.namenode.kerberos.principal",
+    kerberosIdentityDataFileWriter.writeRecord("c6403.ambari.apache.org", "HDFS", "DATANODE",
+        "dn/_HOST@_REALM", "service",
         "/etc/security/keytabs/dn.service.keytab",
-        "hdfs", "r", "hadoop", "", "hdfs-site/dfs.namenode.keytab.file", "false");
+        "hdfs", "r", "hadoop", "", "false");
 
-    kerberosActionDataFileBuilder.close();
+    kerberosIdentityDataFileWriter.close();
 
     // Ensure the host directory exists...
     FileWriter fw = new FileWriter(keytabFile);

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

@@ -41,6 +41,8 @@ import org.apache.ambari.server.metadata.RoleCommandOrder;
 import org.apache.ambari.server.orm.DBAccessor;
 import org.apache.ambari.server.security.SecurityHelper;
 import org.apache.ambari.server.serveraction.kerberos.KDCType;
+import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileWriter;
+import org.apache.ambari.server.serveraction.kerberos.KerberosConfigDataFileWriterFactory;
 import org.apache.ambari.server.serveraction.kerberos.KerberosCredential;
 import org.apache.ambari.server.serveraction.kerberos.KerberosMissingAdminCredentialsException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationException;
@@ -80,6 +82,7 @@ import org.junit.Test;
 
 import javax.persistence.EntityManager;
 
+import java.io.File;
 import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -100,6 +103,7 @@ public class KerberosHelperTest extends EasyMockSupport {
   private static Injector injector;
   private final ClusterController clusterController = createStrictMock(ClusterController.class);
   private final KerberosDescriptorFactory kerberosDescriptorFactory = createStrictMock(KerberosDescriptorFactory.class);
+  private final KerberosConfigDataFileWriterFactory kerberosConfigDataFileWriterFactory = createStrictMock(KerberosConfigDataFileWriterFactory.class);
   private final AmbariMetaInfo metaInfo = createNiceMock(AmbariMetaInfo.class);
 
   @Before
@@ -166,6 +170,7 @@ public class KerberosHelperTest extends EasyMockSupport {
         bind(KerberosOperationHandlerFactory.class).toInstance(kerberosOperationHandlerFactory);
         bind(ClusterController.class).toInstance(clusterController);
         bind(KerberosDescriptorFactory.class).toInstance(kerberosDescriptorFactory);
+        bind(KerberosConfigDataFileWriterFactory.class).toInstance(kerberosConfigDataFileWriterFactory);
       }
     });
 
@@ -379,6 +384,25 @@ public class KerberosHelperTest extends EasyMockSupport {
 
     KerberosHelper kerberosHelper = injector.getInstance(KerberosHelper.class);
 
+    KerberosConfigDataFileWriter kerberosConfigDataFileWriter = createMock(KerberosConfigDataFileWriter.class);
+    kerberosConfigDataFileWriter.addRecord("cluster-env", "security_enabled", "true", "SET");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service1-site", "component1.kerberos.principal", "component1/_HOST@${realm}", "SET");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service1-site", "component1.keytab.file", "${keytab_dir}/service1.keytab", "SET");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service2-site", "component2.kerberos.principal", "component2/host1@${realm}", "SET");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service2-site", "component2.keytab.file", "${keytab_dir}/service2.keytab", "SET");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.close();
+    expectLastCall().times(1);
+
+    KerberosConfigDataFileWriterFactory factory = injector.getInstance(KerberosConfigDataFileWriterFactory.class);
+    expect(factory.createKerberosConfigDataFileWriter(anyObject(File.class)))
+        .andReturn(kerberosConfigDataFileWriter)
+        .times(1);
+
     final StackId stackVersion = createNiceMock(StackId.class);
 
     final ServiceComponentHost schKerberosClient = createMock(ServiceComponentHost.class);
@@ -658,6 +682,25 @@ public class KerberosHelperTest extends EasyMockSupport {
 
     KerberosHelper kerberosHelper = injector.getInstance(KerberosHelper.class);
 
+    KerberosConfigDataFileWriter kerberosConfigDataFileWriter = createMock(KerberosConfigDataFileWriter.class);
+    kerberosConfigDataFileWriter.addRecord("cluster-env", "security_enabled", "false", "SET");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service1-site", "component1.kerberos.principal", null, "REMOVE");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service1-site", "component1.keytab.file", null, "REMOVE");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service2-site", "component2.kerberos.principal", null, "REMOVE");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.addRecord("service2-site", "component2.keytab.file", null, "REMOVE");
+    expectLastCall().times(1);
+    kerberosConfigDataFileWriter.close();
+    expectLastCall().times(1);
+
+    KerberosConfigDataFileWriterFactory factory = injector.getInstance(KerberosConfigDataFileWriterFactory.class);
+    expect(factory.createKerberosConfigDataFileWriter(anyObject(File.class)))
+        .andReturn(kerberosConfigDataFileWriter)
+        .times(1);
+
     final StackId stackVersion = createNiceMock(StackId.class);
 
     final ServiceComponentHost schKerberosClient = createMock(ServiceComponentHost.class);

+ 30 - 27
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosConfigDataFileTest.java

@@ -28,7 +28,7 @@ import java.util.Iterator;
 import java.util.Map;
 
 /**
- * This is a test to see how well the KerberosConfigDataFileBuilder and KerberosConfigDataFileReader
+ * This is a test to see how well the KerberosConfigDataFileWriter and KerberosConfigDataFileReader
  * work when the data temporaryDirectory is opened, close, reopened, and appended to.
  */
 public class KerberosConfigDataFileTest {
@@ -36,27 +36,30 @@ public class KerberosConfigDataFileTest {
   @Rule
   public TemporaryFolder folder = new TemporaryFolder();
 
+  private KerberosConfigDataFileReaderFactory kerberosConfigDataFileReaderFactory = new KerberosConfigDataFileReaderFactory();
+  private KerberosConfigDataFileWriterFactory kerberosConfigDataFileWriterFactory = new KerberosConfigDataFileWriterFactory();
+
   @Test
   public void testKerberosConfigDataFile() throws Exception {
     File file = folder.newFile();
     Assert.assertNotNull(file);
 
     // Write the data
-    KerberosConfigDataFileBuilder builder = new KerberosConfigDataFileBuilder(file);
-    Assert.assertFalse(builder.isClosed());
+    KerberosConfigDataFileWriter writer = kerberosConfigDataFileWriterFactory.createKerberosConfigDataFileWriter(file);
+    Assert.assertFalse(writer.isClosed());
 
     for (int i = 0; i < 10; i++) {
-      builder.addRecord("config-type" + i, "key" + i, "value" + i, KerberosConfigDataFile.OPERATION_TYPE_SET);
+      writer.addRecord("config-type" + i, "key" + i, "value" + i, KerberosConfigDataFileWriter.OPERATION_TYPE_SET);
     }
     for (int i = 10; i < 15; i++) {
-      builder.addRecord("config-type" + i, "key" + i, "value" + i, KerberosConfigDataFile.OPERATION_TYPE_REMOVE);
+      writer.addRecord("config-type" + i, "key" + i, "value" + i, KerberosConfigDataFileWriter.OPERATION_TYPE_REMOVE);
     }
 
-    builder.close();
-    Assert.assertTrue(builder.isClosed());
+    writer.close();
+    Assert.assertTrue(writer.isClosed());
 
     // Read the data...
-    KerberosConfigDataFileReader reader = new KerberosConfigDataFileReader(file);
+    KerberosConfigDataFileReader reader = kerberosConfigDataFileReaderFactory.createKerberosConfigDataFileReader(file);
     Assert.assertFalse(reader.isClosed());
 
     Iterator<Map<String, String>> iterator = reader.iterator();
@@ -68,15 +71,15 @@ public class KerberosConfigDataFileTest {
       Map<String, String> record = iterator.next();
 
       if (i < 15) {
-        Assert.assertEquals("config-type" + i, record.get(KerberosConfigDataFile.CONFIGURATION_TYPE));
-        Assert.assertEquals("key" + i, record.get(KerberosConfigDataFile.KEY));
-        Assert.assertEquals("value" + i, record.get(KerberosConfigDataFile.VALUE));
+        Assert.assertEquals("config-type" + i, record.get(KerberosConfigDataFileReader.CONFIGURATION_TYPE));
+        Assert.assertEquals("key" + i, record.get(KerberosConfigDataFileReader.KEY));
+        Assert.assertEquals("value" + i, record.get(KerberosConfigDataFileReader.VALUE));
 
         if(i<10) {
-          Assert.assertEquals("SET", record.get(KerberosConfigDataFile.OPERATION));
+          Assert.assertEquals("SET", record.get(KerberosConfigDataFileReader.OPERATION));
         }
         else {
-          Assert.assertEquals("REMOVE", record.get(KerberosConfigDataFile.OPERATION));
+          Assert.assertEquals("REMOVE", record.get(KerberosConfigDataFileReader.OPERATION));
         }
       }
 
@@ -93,9 +96,9 @@ public class KerberosConfigDataFileTest {
     i = 0;
     for (Map<String, String> record : reader) {
       if (i < 10) {
-        Assert.assertEquals("config-type" + i, record.get(KerberosConfigDataFile.CONFIGURATION_TYPE));
-        Assert.assertEquals("key" + i, record.get(KerberosConfigDataFile.KEY));
-        Assert.assertEquals("value" + i, record.get(KerberosConfigDataFile.VALUE));
+        Assert.assertEquals("config-type" + i, record.get(KerberosConfigDataFileReader.CONFIGURATION_TYPE));
+        Assert.assertEquals("key" + i, record.get(KerberosConfigDataFileReader.KEY));
+        Assert.assertEquals("value" + i, record.get(KerberosConfigDataFileReader.VALUE));
       }
 
       i++;
@@ -107,15 +110,15 @@ public class KerberosConfigDataFileTest {
     Assert.assertTrue(reader.isClosed());
 
     // Add an additional record
-    builder.open();
-    Assert.assertFalse(builder.isClosed());
+    writer.open();
+    Assert.assertFalse(writer.isClosed());
 
-    builder.addRecord("config-type", "key", "value", KerberosConfigDataFile.OPERATION_TYPE_SET);
+    writer.addRecord("config-type", "key", "value", KerberosConfigDataFileReader.OPERATION_TYPE_SET);
 
-    builder.close();
-    Assert.assertTrue(builder.isClosed());
+    writer.close();
+    Assert.assertTrue(writer.isClosed());
 
-    reader = new KerberosConfigDataFileReader(file);
+    reader = kerberosConfigDataFileReaderFactory.createKerberosConfigDataFileReader(file);
     Assert.assertFalse(reader.isClosed());
 
     i = 0;
@@ -129,13 +132,13 @@ public class KerberosConfigDataFileTest {
     Assert.assertTrue(reader.isClosed());
 
     // Add an additional record
-    builder = new KerberosConfigDataFileBuilder(file);
-    Assert.assertFalse(builder.isClosed());
+    writer = kerberosConfigDataFileWriterFactory.createKerberosConfigDataFileWriter(file);
+    Assert.assertFalse(writer.isClosed());
 
-    builder.addRecord("config-type", "key", "value", KerberosConfigDataFile.OPERATION_TYPE_REMOVE);
+    writer.addRecord("config-type", "key", "value", KerberosConfigDataFileReader.OPERATION_TYPE_REMOVE);
 
-    builder.close();
-    Assert.assertTrue(builder.isClosed());
+    writer.close();
+    Assert.assertTrue(writer.isClosed());
 
     reader.open();
     Assert.assertFalse(reader.isClosed());

+ 74 - 79
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosActionDataFileTest.java → ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosIdentityDataFileTest.java

@@ -28,43 +28,46 @@ import java.util.Iterator;
 import java.util.Map;
 
 /**
- * This is a test to see how well the KerberosActionDataFileBuilder and KerberosActionDataFileReader
+ * This is a test to see how well the KerberosIdentityDataFileWriter and KerberosIdentityDataFileReader
  * work when the data temporaryDirectory is opened, close, reopened, and appended to.
  */
-public class KerberosActionDataFileTest {
+public class KerberosIdentityDataFileTest {
 
   @Rule
   public TemporaryFolder folder = new TemporaryFolder();
 
+  private KerberosIdentityDataFileReaderFactory kerberosIdentityDataFileReaderFactory = new KerberosIdentityDataFileReaderFactory();
+  private KerberosIdentityDataFileWriterFactory kerberosIdentityDataFileWriterFactory = new KerberosIdentityDataFileWriterFactory();
+
   @Test
-  public void testKerberosActionDataFile() throws Exception {
+  public void testKerberosIdentityDataFile() throws Exception {
     File file = folder.newFile();
     Assert.assertNotNull(file);
 
     // Write the data
-    KerberosActionDataFileBuilder builder = new KerberosActionDataFileBuilder(file);
-    Assert.assertFalse(builder.isClosed());
+    KerberosIdentityDataFileWriter writer = kerberosIdentityDataFileWriterFactory.createKerberosIdentityDataFileWriter(file);
+    Assert.assertFalse(writer.isClosed());
 
     for (int i = 0; i < 10; i++) {
-      builder.addRecord("hostName" + i, "serviceName" + i, "serviceComponentName" + i,
-          "principal" + i, "principal_type" + i, "principalConfiguration" + i, "keytabFilePath" + i,
+      writer.writeRecord("hostName" + i, "serviceName" + i, "serviceComponentName" + i,
+          "principal" + i, "principal_type" + i, "keytabFilePath" + i,
           "keytabFileOwnerName" + i, "keytabFileOwnerAccess" + i,
           "keytabFileGroupName" + i, "keytabFileGroupAccess" + i,
-          "keytabFileConfiguration" + i, "false");
+          "false");
     }
 
     // Add some odd characters
-    builder.addRecord("hostName's", "serviceName#", "serviceComponentName\"",
-        "principal", "principal_type", "principalConfiguration", "keytabFilePath",
+    writer.writeRecord("hostName's", "serviceName#", "serviceComponentName\"",
+        "principal", "principal_type", "keytabFilePath",
         "'keytabFileOwnerName'", "<keytabFileOwnerAccess>",
         "\"keytabFileGroupName\"", "keytab,File,Group,Access",
-        "\"keytab,'File',Configuration\"", "false");
+        "false");
 
-    builder.close();
-    Assert.assertTrue(builder.isClosed());
+    writer.close();
+    Assert.assertTrue(writer.isClosed());
 
     // Read the data...
-    KerberosActionDataFileReader reader = new KerberosActionDataFileReader(file);
+    KerberosIdentityDataFileReader reader = kerberosIdentityDataFileReaderFactory.createKerberosIdentityDataFileReader(file);
     Assert.assertFalse(reader.isClosed());
 
     Iterator<Map<String, String>> iterator = reader.iterator();
@@ -76,33 +79,29 @@ public class KerberosActionDataFileTest {
       Map<String, String> record = iterator.next();
 
       if (i < 10) {
-        Assert.assertEquals("hostName" + i, record.get(KerberosActionDataFile.HOSTNAME));
-        Assert.assertEquals("serviceName" + i, record.get(KerberosActionDataFile.SERVICE));
-        Assert.assertEquals("serviceComponentName" + i, record.get(KerberosActionDataFile.COMPONENT));
-        Assert.assertEquals("principal" + i, record.get(KerberosActionDataFile.PRINCIPAL));
-        Assert.assertEquals("principal_type" + i, record.get(KerberosActionDataFile.PRINCIPAL_TYPE));
-        Assert.assertEquals("principalConfiguration" + i, record.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-        Assert.assertEquals("keytabFilePath" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
-        Assert.assertEquals("keytabFileOwnerName" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-        Assert.assertEquals("keytabFileOwnerAccess" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-        Assert.assertEquals("keytabFileGroupName" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-        Assert.assertEquals("keytabFileGroupAccess" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-        Assert.assertEquals("keytabFileConfiguration" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
-        Assert.assertEquals("false", record.get(KerberosActionDataFile.KEYTAB_FILE_IS_CACHABLE));
+        Assert.assertEquals("hostName" + i, record.get(KerberosIdentityDataFileReader.HOSTNAME));
+        Assert.assertEquals("serviceName" + i, record.get(KerberosIdentityDataFileReader.SERVICE));
+        Assert.assertEquals("serviceComponentName" + i, record.get(KerberosIdentityDataFileReader.COMPONENT));
+        Assert.assertEquals("principal" + i, record.get(KerberosIdentityDataFileReader.PRINCIPAL));
+        Assert.assertEquals("principal_type" + i, record.get(KerberosIdentityDataFileReader.PRINCIPAL_TYPE));
+        Assert.assertEquals("keytabFilePath" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH));
+        Assert.assertEquals("keytabFileOwnerName" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_NAME));
+        Assert.assertEquals("keytabFileOwnerAccess" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_ACCESS));
+        Assert.assertEquals("keytabFileGroupName" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_NAME));
+        Assert.assertEquals("keytabFileGroupAccess" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_ACCESS));
+        Assert.assertEquals("false", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_IS_CACHABLE));
       } else {
-        Assert.assertEquals("hostName's", record.get(KerberosActionDataFile.HOSTNAME));
-        Assert.assertEquals("serviceName#", record.get(KerberosActionDataFile.SERVICE));
-        Assert.assertEquals("serviceComponentName\"", record.get(KerberosActionDataFile.COMPONENT));
-        Assert.assertEquals("principal", record.get(KerberosActionDataFile.PRINCIPAL));
-        Assert.assertEquals("principal_type", record.get(KerberosActionDataFile.PRINCIPAL_TYPE));
-        Assert.assertEquals("principalConfiguration", record.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-        Assert.assertEquals("keytabFilePath", record.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
-        Assert.assertEquals("'keytabFileOwnerName'", record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-        Assert.assertEquals("<keytabFileOwnerAccess>", record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-        Assert.assertEquals("\"keytabFileGroupName\"", record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-        Assert.assertEquals("keytab,File,Group,Access", record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-        Assert.assertEquals("\"keytab,'File',Configuration\"", record.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
-        Assert.assertEquals("false", record.get(KerberosActionDataFile.KEYTAB_FILE_IS_CACHABLE));
+        Assert.assertEquals("hostName's", record.get(KerberosIdentityDataFileReader.HOSTNAME));
+        Assert.assertEquals("serviceName#", record.get(KerberosIdentityDataFileReader.SERVICE));
+        Assert.assertEquals("serviceComponentName\"", record.get(KerberosIdentityDataFileReader.COMPONENT));
+        Assert.assertEquals("principal", record.get(KerberosIdentityDataFileReader.PRINCIPAL));
+        Assert.assertEquals("principal_type", record.get(KerberosIdentityDataFileReader.PRINCIPAL_TYPE));
+        Assert.assertEquals("keytabFilePath", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH));
+        Assert.assertEquals("'keytabFileOwnerName'", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_NAME));
+        Assert.assertEquals("<keytabFileOwnerAccess>", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_ACCESS));
+        Assert.assertEquals("\"keytabFileGroupName\"", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_NAME));
+        Assert.assertEquals("keytab,File,Group,Access", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_ACCESS));
+        Assert.assertEquals("false", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_IS_CACHABLE));
       }
 
       i++;
@@ -116,31 +115,27 @@ public class KerberosActionDataFileTest {
     i = 0;
     for (Map<String, String> record : reader) {
       if (i < 10) {
-        Assert.assertEquals("hostName" + i, record.get(KerberosActionDataFile.HOSTNAME));
-        Assert.assertEquals("serviceName" + i, record.get(KerberosActionDataFile.SERVICE));
-        Assert.assertEquals("serviceComponentName" + i, record.get(KerberosActionDataFile.COMPONENT));
-        Assert.assertEquals("principal" + i, record.get(KerberosActionDataFile.PRINCIPAL));
-        Assert.assertEquals("principal_type" + i, record.get(KerberosActionDataFile.PRINCIPAL_TYPE));
-        Assert.assertEquals("principalConfiguration" + i, record.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-        Assert.assertEquals("keytabFilePath" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
-        Assert.assertEquals("keytabFileOwnerName" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-        Assert.assertEquals("keytabFileOwnerAccess" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-        Assert.assertEquals("keytabFileGroupName" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-        Assert.assertEquals("keytabFileGroupAccess" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-        Assert.assertEquals("keytabFileConfiguration" + i, record.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
+        Assert.assertEquals("hostName" + i, record.get(KerberosIdentityDataFileReader.HOSTNAME));
+        Assert.assertEquals("serviceName" + i, record.get(KerberosIdentityDataFileReader.SERVICE));
+        Assert.assertEquals("serviceComponentName" + i, record.get(KerberosIdentityDataFileReader.COMPONENT));
+        Assert.assertEquals("principal" + i, record.get(KerberosIdentityDataFileReader.PRINCIPAL));
+        Assert.assertEquals("principal_type" + i, record.get(KerberosIdentityDataFileReader.PRINCIPAL_TYPE));
+        Assert.assertEquals("keytabFilePath" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH));
+        Assert.assertEquals("keytabFileOwnerName" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_NAME));
+        Assert.assertEquals("keytabFileOwnerAccess" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_ACCESS));
+        Assert.assertEquals("keytabFileGroupName" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_NAME));
+        Assert.assertEquals("keytabFileGroupAccess" + i, record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_ACCESS));
       } else {
-        Assert.assertEquals("hostName's", record.get(KerberosActionDataFile.HOSTNAME));
-        Assert.assertEquals("serviceName#", record.get(KerberosActionDataFile.SERVICE));
-        Assert.assertEquals("serviceComponentName\"", record.get(KerberosActionDataFile.COMPONENT));
-        Assert.assertEquals("principal", record.get(KerberosActionDataFile.PRINCIPAL));
-        Assert.assertEquals("principal_type", record.get(KerberosActionDataFile.PRINCIPAL_TYPE));
-        Assert.assertEquals("principalConfiguration", record.get(KerberosActionDataFile.PRINCIPAL_CONFIGURATION));
-        Assert.assertEquals("keytabFilePath", record.get(KerberosActionDataFile.KEYTAB_FILE_PATH));
-        Assert.assertEquals("'keytabFileOwnerName'", record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_NAME));
-        Assert.assertEquals("<keytabFileOwnerAccess>", record.get(KerberosActionDataFile.KEYTAB_FILE_OWNER_ACCESS));
-        Assert.assertEquals("\"keytabFileGroupName\"", record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_NAME));
-        Assert.assertEquals("keytab,File,Group,Access", record.get(KerberosActionDataFile.KEYTAB_FILE_GROUP_ACCESS));
-        Assert.assertEquals("\"keytab,'File',Configuration\"", record.get(KerberosActionDataFile.KEYTAB_FILE_CONFIGURATION));
+        Assert.assertEquals("hostName's", record.get(KerberosIdentityDataFileReader.HOSTNAME));
+        Assert.assertEquals("serviceName#", record.get(KerberosIdentityDataFileReader.SERVICE));
+        Assert.assertEquals("serviceComponentName\"", record.get(KerberosIdentityDataFileReader.COMPONENT));
+        Assert.assertEquals("principal", record.get(KerberosIdentityDataFileReader.PRINCIPAL));
+        Assert.assertEquals("principal_type", record.get(KerberosIdentityDataFileReader.PRINCIPAL_TYPE));
+        Assert.assertEquals("keytabFilePath", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_PATH));
+        Assert.assertEquals("'keytabFileOwnerName'", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_NAME));
+        Assert.assertEquals("<keytabFileOwnerAccess>", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_OWNER_ACCESS));
+        Assert.assertEquals("\"keytabFileGroupName\"", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_NAME));
+        Assert.assertEquals("keytab,File,Group,Access", record.get(KerberosIdentityDataFileReader.KEYTAB_FILE_GROUP_ACCESS));
       }
 
       i++;
@@ -150,19 +145,19 @@ public class KerberosActionDataFileTest {
     Assert.assertTrue(reader.isClosed());
 
     // Add an additional record
-    builder.open();
-    Assert.assertFalse(builder.isClosed());
+    writer.open();
+    Assert.assertFalse(writer.isClosed());
 
-    builder.addRecord("hostName", "serviceName", "serviceComponentName",
-        "principal","principal_type", "principalConfiguration", "keytabFilePath",
+    writer.writeRecord("hostName", "serviceName", "serviceComponentName",
+        "principal", "principal_type", "keytabFilePath",
         "keytabFileOwnerName", "keytabFileOwnerAccess",
         "keytabFileGroupName", "keytabFileGroupAccess",
-        "keytabFileConfiguration", "true");
+        "true");
 
-    builder.close();
-    Assert.assertTrue(builder.isClosed());
+    writer.close();
+    Assert.assertTrue(writer.isClosed());
 
-    reader = new KerberosActionDataFileReader(file);
+    reader = kerberosIdentityDataFileReaderFactory.createKerberosIdentityDataFileReader(file);
     Assert.assertFalse(reader.isClosed());
 
     i = 0;
@@ -176,17 +171,17 @@ public class KerberosActionDataFileTest {
     Assert.assertTrue(reader.isClosed());
 
     // Add an additional record
-    builder = new KerberosActionDataFileBuilder(file);
-    Assert.assertFalse(builder.isClosed());
+    writer = kerberosIdentityDataFileWriterFactory.createKerberosIdentityDataFileWriter(file);
+    Assert.assertFalse(writer.isClosed());
 
-    builder.addRecord("hostName", "serviceName", "serviceComponentName",
-        "principal", "principal_type", "principalConfiguration", "keytabFilePath",
+    writer.writeRecord("hostName", "serviceName", "serviceComponentName",
+        "principal", "principal_type", "keytabFilePath",
         "keytabFileOwnerName", "keytabFileOwnerAccess",
         "keytabFileGroupName", "keytabFileGroupAccess",
-        "keytabFileConfiguration", "true");
+        "true");
 
-    builder.close();
-    Assert.assertTrue(builder.isClosed());
+    writer.close();
+    Assert.assertTrue(writer.isClosed());
 
     reader.open();
     Assert.assertFalse(reader.isClosed());

+ 8 - 8
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerActionTest.java

@@ -80,7 +80,7 @@ public class KerberosServerActionTest {
             if (requestSharedDataContext.get("FAIL") != null) {
               return createCommandReport(1, HostRoleStatus.FAILED, "{}", "ERROR", "ERROR");
             } else {
-              requestSharedDataContext.put(identityRecord.get(KerberosActionDataFile.PRINCIPAL), evaluatedPrincipal);
+              requestSharedDataContext.put(identityRecord.get(KerberosIdentityDataFileReader.PRINCIPAL), evaluatedPrincipal);
               return null;
             }
           }
@@ -103,16 +103,16 @@ public class KerberosServerActionTest {
     Assert.assertTrue(temporaryDirectory.mkdirs());
 
     // Create a data file
-    KerberosActionDataFileBuilder builder =
-        new KerberosActionDataFileBuilder(new File(temporaryDirectory, KerberosActionDataFile.DATA_FILE_NAME));
+    KerberosIdentityDataFileWriter writer =
+        new KerberosIdentityDataFileWriter(new File(temporaryDirectory, KerberosIdentityDataFileWriter.DATA_FILE_NAME));
     for (int i = 0; i < 10; i++) {
-      builder.addRecord("hostName", "serviceName" + i, "serviceComponentName" + i,
-          "principal|_HOST|_REALM" + i, "principal_type", "principalConfiguration" + i, "keytabFilePath" + i,
+      writer.writeRecord("hostName", "serviceName" + i, "serviceComponentName" + i,
+          "principal|_HOST|_REALM" + i, "principal_type", "keytabFilePath" + i,
           "keytabFileOwnerName" + i, "keytabFileOwnerAccess" + i,
           "keytabFileGroupName" + i, "keytabFileGroupAccess" + i,
-          "keytabFileConfiguration" + i, "false");
+          "false");
     }
-    builder.close();
+    writer.close();
 
     commandParams.put(KerberosServerAction.DATA_DIRECTORY, temporaryDirectory.getAbsolutePath());
     commandParams.put(KerberosServerAction.DEFAULT_REALM, "REALM.COM");
@@ -133,7 +133,7 @@ public class KerberosServerActionTest {
   @After
   public void tearDown() throws Exception {
     if (temporaryDirectory != null) {
-      new File(temporaryDirectory, KerberosActionDataFile.DATA_FILE_NAME).delete();
+      new File(temporaryDirectory, KerberosIdentityDataFileWriter.DATA_FILE_NAME).delete();
       temporaryDirectory.delete();
     }
   }

+ 32 - 64
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/UpdateKerberosConfigsServerActionTest.java

@@ -23,29 +23,23 @@ import com.google.inject.Guice;
 import com.google.inject.Injector;
 import org.apache.ambari.server.agent.ExecutionCommand;
 import org.apache.ambari.server.controller.AmbariManagementController;
-import org.apache.ambari.server.controller.ConfigurationRequest;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
 import org.apache.ambari.server.state.ConfigHelper;
-import org.apache.commons.codec.digest.DigestUtils;
-import org.easymock.EasyMock;
-import org.junit.After;
+import org.easymock.EasyMockSupport;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import java.io.BufferedWriter;
 import java.io.File;
-import java.io.FileWriter;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.concurrent.ConcurrentMap;
 
 import static org.easymock.EasyMock.*;
 
-public class UpdateKerberosConfigsServerActionTest {
+public class UpdateKerberosConfigsServerActionTest extends EasyMockSupport{
 
   @Rule
   public TemporaryFolder testFolder = new TemporaryFolder();
@@ -53,95 +47,61 @@ public class UpdateKerberosConfigsServerActionTest {
   private Injector injector;
   private UpdateKerberosConfigsServerAction action;
 
-  private final AmbariManagementController controller = EasyMock.createNiceMock(AmbariManagementController.class);
-  private final ConfigHelper configHelper = createNiceMock(ConfigHelper.class);
-  private final Clusters clusters = EasyMock.createNiceMock(Clusters.class);
-  private final Cluster cluster = EasyMock.createNiceMock(Cluster.class);
 
   @Before
   public void setup() throws Exception {
-    setupIndexDat();
-    setupConfigDat();
+    final AmbariManagementController controller = createNiceMock(AmbariManagementController.class);
+    final Clusters clusters = createNiceMock(Clusters.class);
+    final Cluster cluster = createNiceMock(Cluster.class);
 
     expect(controller.getClusters()).andReturn(clusters).once();
-    replay(controller);
-
-    configHelper.updateConfigType(anyObject(Cluster.class), anyObject(AmbariManagementController.class),
-        anyObject(String.class), anyObject(Map.class), anyObject(Collection.class), anyObject(String.class), anyObject(String.class));
-    expectLastCall().atLeastOnce();
-    replay(configHelper);
-
-    replay(cluster);
 
     expect(clusters.getCluster(anyObject(String.class))).andReturn(cluster).once();
-    replay(clusters);
 
     injector = Guice.createInjector(new AbstractModule() {
 
       @Override
       protected void configure() {
         bind(AmbariManagementController.class).toInstance(controller);
-        bind(ConfigHelper.class).toInstance(configHelper);
+        bind(ConfigHelper.class).toInstance(createNiceMock(ConfigHelper.class));
       }
     });
-    action = injector.getInstance(UpdateKerberosConfigsServerAction.class);
-  }
-
-  private void setupIndexDat() throws Exception {
-
-    File indexFile;
-    KerberosActionDataFileBuilder kerberosActionDataFileBuilder = null;
 
     dataDir = testFolder.getRoot().getAbsolutePath();
 
-    indexFile = new File(dataDir, KerberosActionDataFile.DATA_FILE_NAME);
-    kerberosActionDataFileBuilder = new KerberosActionDataFileBuilder(indexFile);
-
-    kerberosActionDataFileBuilder.addRecord("c6403.ambari.apache.org", "HDFS", "DATANODE",
-      "dn/_HOST@_REALM", "service", "hdfs-site/dfs.namenode.kerberos.principal",
-      "/etc/security/keytabs/dn.service.keytab",
-      "hdfs", "r", "hadoop", "", "hdfs-site/dfs.namenode.keytab.file", "false");
-
-    kerberosActionDataFileBuilder.close();
-    File hostDirectory = new File(dataDir, "c6403.ambari.apache.org");
-
-    // Ensure the host directory exists...
-    if (hostDirectory.exists() || hostDirectory.mkdirs()) {
-      File file = new File(hostDirectory, DigestUtils.sha1Hex("/etc/security/keytabs/dn.service.keytab"));
-      if (!file.exists()) {
-        file.createNewFile();
-      }
+    setupConfigDat();
 
-      FileWriter fw = new FileWriter(file.getAbsoluteFile());
-      BufferedWriter bw = new BufferedWriter(fw);
-      bw.write("hello");
-      bw.close();
-    }
+    action = injector.getInstance(UpdateKerberosConfigsServerAction.class);
   }
 
   private void setupConfigDat() throws Exception {
-    File configFile = new File(dataDir, KerberosConfigDataFile.DATA_FILE_NAME);
-    FileWriter fw = new FileWriter(configFile.getAbsoluteFile());
-    BufferedWriter bw = new BufferedWriter(fw);
-    bw.write("config,key,value\n");
-    bw.write("hdfs-site,hadoop.security.authentication,kerberos");
-    bw.close();
+    File configFile = new File(dataDir, KerberosConfigDataFileWriter.DATA_FILE_NAME);
+    KerberosConfigDataFileWriterFactory factory = injector.getInstance(KerberosConfigDataFileWriterFactory.class);
+    KerberosConfigDataFileWriter writer = factory.createKerberosConfigDataFileWriter(configFile);
+    writer.addRecord("hdfs-site", "hadoop.security.authentication", "kerberos", KerberosConfigDataFileWriter.OPERATION_TYPE_SET);
+    writer.addRecord("hdfs-site", "remove.me", null, KerberosConfigDataFileWriter.OPERATION_TYPE_REMOVE);
+    writer.close();
   }
 
   @Test
   public void testUpdateConfig() throws Exception {
-    ExecutionCommand executionCommand = new ExecutionCommand();
     Map<String, String> commandParams = new HashMap<String, String>();
     commandParams.put(KerberosServerAction.DATA_DIRECTORY, dataDir);
+
+    ExecutionCommand executionCommand = new ExecutionCommand();
     executionCommand.setCommandParams(commandParams);
 
-    action.setExecutionCommand(executionCommand);
+    ConfigHelper configHelper = injector.getInstance(ConfigHelper.class);
+    configHelper.updateConfigType(anyObject(Cluster.class), anyObject(AmbariManagementController.class),
+        anyObject(String.class), anyObject(Map.class), anyObject(Collection.class), anyObject(String.class), anyObject(String.class));
+    expectLastCall().atLeastOnce();
 
-    ConcurrentMap<String, Object> requestSharedDataContext = null;
+    replayAll();
 
-    action.execute(requestSharedDataContext);
+    action.setExecutionCommand(executionCommand);
+    action.execute(null);
 
-    verify(controller, clusters, cluster, configHelper);
+    verifyAll();
   }
 
   @Test
@@ -150,8 +110,12 @@ public class UpdateKerberosConfigsServerActionTest {
     Map<String, String> commandParams = new HashMap<String, String>();
     executionCommand.setCommandParams(commandParams);
 
+    replayAll();
+
     action.setExecutionCommand(executionCommand);
     action.execute(null);
+
+    verifyAll();
   }
 
   @Test
@@ -161,7 +125,11 @@ public class UpdateKerberosConfigsServerActionTest {
     commandParams.put(KerberosServerAction.DATA_DIRECTORY, testFolder.newFolder().getAbsolutePath());
     executionCommand.setCommandParams(commandParams);
 
+    replayAll();
+
     action.setExecutionCommand(executionCommand);
     action.execute(null);
+
+    verifyAll();
   }
 }