AMBARI-9022. Preserve existing kerberos auth_to_local properties when scaling cluster

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

@@ -18,54 +18,72 @@
 
 package org.apache.ambari.server.controller;
 
-import java.util.Map;
-import java.util.TreeMap;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 /**
  * AuthToLocalBuilder helps to create auth_to_local rules for use in configuration files like
- * core-site.xml.
+ * core-site.xml.  No duplicate rules will be generated.
  * <p/>
- * For each principal appended to the rule set, parse out the primary value and match it to a local
- * username.  Then when done appending all principals, generate the rules where each entry yields
- * one of the following rule:
+ * Allows previously existing rules to be added verbatim.  Also allows new rules to be generated
+ * based on a principal and local username.  For each principal added to the builder, generate
+ * a rule conforming to one of the following formats:
  * <p/>
- * Qualified Principal: RULE:[2:$1@$0](PRIMARY@REALM)s/.*\/LOCAL_USERNAME/
+ * Qualified Principal (the principal contains a user and host):
+ * RULE:[2:$1@$0](PRIMARY@REALM)s/.*\/LOCAL_USERNAME/
  * <p/>
- * Unqualified Principal: RULE:[1:$1@$0](PRIMARY@REALM)s/.*\/LOCAL_USERNAME/
+ * Unqualified Principal (only user is specified):
+ * RULE:[1:$1@$0](PRIMARY@REALM)s/.*\/LOCAL_USERNAME/
+ * <p>
+ * Additionally, for each realm included in the rule set, generate a default realm rule
+ * in the format: RULE:[1:$1@$0](.*@REALM)s/@.{@literal *}//
+ * <p>
+ * Ordering guarantees for the generated rule string are as follows:
+ * <ul>
+ *   <li>Rules with the same expected component count are ordered according to match component count</li>
+ *   <li>Rules with different expected component count are ordered according to the default string ordering</li>
+ *   <li>Rules in the form of .*@REALM are ordered after all other rules with the same expected component count</li>
+ * </ul>
+ *
  */
 public class AuthToLocalBuilder {
 
   /**
-   * A Regular expression declaring a qualified principal such that the principal is in the following format:
-   * primary/instance@REALM
-   */
-  private static final Pattern PATTERN_QUALIFIED_PRINCIPAL = Pattern.compile("(\\w+)/.*@.*");
-
-  /**
-   * A Regular expression declaring an un qualified principal such that the principal is in the following format:
-   * primary@REALM
+   * Ordered set of rules which have been added to the builder.
    */
-  private static final Pattern PATTERN_UNQUALIFIED_PRINCIPAL = Pattern.compile("(\\w+)@.*");
+  private Set<Rule> setRules = new TreeSet<Rule>();
 
-  /**
-   * A map of qualified principal names (primary/instance@REALM, with instance and @REALM removed).
-   * <p/>
-   * A TreeMap is used to help generate deterministic ordering of rules for testing.
-   */
-  private Map<String, String> qualifiedAuthToLocalMap = new TreeMap<String, String>();
 
   /**
-   * A map of unqualified principal names (primary@REALM, with @REALM removed).
-   * <p/>
-   * A TreeMap is used to help generate deterministic ordering of rules for testing.
+   * Add existing rules from the given authToLocal configuration property.
+   * The rules are added verbatim.
+   *
+   * @param authToLocalRules config property value containing the existing rules
    */
-  private Map<String, String> unqualifiedAuthToLocalMap = new TreeMap<String, String>();
+  public void addRules(String authToLocalRules) {
+    if (authToLocalRules != null && ! authToLocalRules.isEmpty()) {
+      String[] rules = authToLocalRules.split("RULE:|DEFAULT");
+      for (String r : rules) {
+        r = r.trim();
+        if (! r.isEmpty()) {
+          Rule rule = createRule(r);
+          setRules.add(rule);
+          // ensure that a default rule is added for each realm
+          addDefaultRealmRule(rule.getPrincipal());
+        }
+      }
+    }
+  }
 
 
   /**
-   * Appends a principal and local username mapping to the builder.
+   * Adds a rule for the given principal and local user.
+   * The principal must contain a realm component.
    * <p/>
    * The supplied principal is parsed to determine if it is qualified or unqualified and stored
    * accordingly so that when the mapping rules are generated the appropriate rule is generated.
@@ -76,58 +94,401 @@ public class AuthToLocalBuilder {
    * <p/>
    * If the principal does not match one of the two expected patterns, it will be ignored.
    *
-   * @param principal     a String containing the full principal to append
-   * @param localUsername a String declaring that local username to map the principal to
+   * @param principal     a string containing the full principal
+   * @param localUsername a string declaring that local username to map the principal to
+   * @throws IllegalArgumentException if the provided principal doesn't contain a realm element
    */
-  public void append(String principal, String localUsername) {
-    if ((principal != null) && (localUsername != null) && !principal.isEmpty() && !localUsername.isEmpty()) {
-      // Determine if the principal is contains an instance declaration
-      Matcher matcher;
-
-      matcher = PATTERN_QUALIFIED_PRINCIPAL.matcher(principal);
-      if (matcher.matches()) {
-        qualifiedAuthToLocalMap.put(matcher.group(1), localUsername);
-      } else {
-        matcher = PATTERN_UNQUALIFIED_PRINCIPAL.matcher(principal);
-        if (matcher.matches()) {
-          unqualifiedAuthToLocalMap.put(matcher.group(1), localUsername);
-        }
+  public void addRule(String principal, String localUsername) {
+    if ((principal != null) && (localUsername != null) &&
+        !principal.isEmpty() && !localUsername.isEmpty()) {
+
+      Principal p = new Principal(principal);
+      if (p.getRealm() == null) {
+        throw new IllegalArgumentException(
+            "Attempted to add a rule for a principal with no realm: " + principal);
       }
+
+      Rule rule = createHostAgnosticRule(p, localUsername);
+      setRules.add(rule);
+      addDefaultRealmRule(rule.getPrincipal());
     }
   }
 
   /**
    * Generates the auth_to_local rules used by configuration settings such as core-site/auth_to_local.
    *
-   * @param realm a String declaring the realm to use in rule set
+   * @param realm a string declaring the realm to use in rule set
    *
    */
   public String generate(String realm) {
-
     StringBuilder builder = new StringBuilder();
+    // ensure that a default rule is added for this realm
+    setRules.add(createDefaultRealmRule(realm));
 
-    for (Map.Entry<String, String> entry : qualifiedAuthToLocalMap.entrySet()) {
-      // RULE:[2:$1@$0](PRIMARY@REALM)s/.*/LOCAL_USERNAME/
-      appendRule(builder, String.format("RULE:[2:$1@$0](%s@%s)s/.*/%s/", entry.getKey(), realm, entry.getValue()));
+    for (Rule rule : setRules) {
+      appendRule(builder, rule.toString());
     }
 
-    for (Map.Entry<String, String> entry : unqualifiedAuthToLocalMap.entrySet()) {
-      // RULE:[1:$1@$0](PRIMARY@REALM)s/.*/LOCAL_USERNAME/
-      appendRule(builder, String.format("RULE:[1:$1@$0](%s@%s)s/.*/%s/", entry.getKey(), realm, entry.getValue()));
-    }
-
-    // RULE:[1:$1@$0](.*@YOUR.REALM)s/@.*//
-    appendRule(builder, String.format("RULE:[1:$1@$0](.*@%s)s/@.*//", realm));
-
     appendRule(builder, "DEFAULT");
-
     return builder.toString();
   }
 
+  /**
+   * Append a rule to the given string builder.
+   *
+   * @param stringBuilder  string builder to which rule is added
+   * @param rule           rule to add
+   */
   private void appendRule(StringBuilder stringBuilder, String rule) {
     if (stringBuilder.length() > 0) {
       stringBuilder.append('\n');
     }
     stringBuilder.append(rule);
   }
+
+  /**
+   * Add a default realm rule for the realm associated with a principal.
+   * If the realm is null or is a wildcard ".*" then no rule id added.
+   *
+   * @param principal  principal which contains the realm
+   */
+  private void addDefaultRealmRule(Principal principal) {
+    String realm = principal.getRealm();
+    if (realm != null && ! realm.equals(".*")) {
+      setRules.add(createDefaultRealmRule(realm));
+    }
+  }
+
+  /**
+   * Create a rule that expects 2 components in the principal and ignores hostname in the comparison.
+   *
+   * @param principal  principal
+   * @param localUser  local user
+   *
+   * @return a new rule that ignores hostname in the comparison
+   */
+  private Rule createHostAgnosticRule(Principal principal, String localUser) {
+    List<String> principalComponents = principal.getComponents();
+    int componentCount = principalComponents.size();
+
+    return new Rule(principal, componentCount, 1, String.format(
+        "RULE:[%d:$1@$0](%s@%s)s/.*/%s/", componentCount,
+        principal.getComponent(1), principal.getRealm(), localUser));
+  }
+
+  /**
+   * Create a default rule for a realm which matches all principals with 1 component and the same realm.
+   *
+   * @param realm  realm that the rule is being created for
+   *
+   * @return  a new default realm rule
+   */
+  private Rule createDefaultRealmRule(String realm) {
+    return new Rule(new Principal(String.format(".*@%s", realm)),
+        1, 1, String.format("RULE:[1:$1@$0](.*@%s)s/@.*//", realm));
+  }
+
+  /**
+   * Create a rule from an existing string representation.
+   * @param rule  string representation of a rule
+   *
+   * @return  a new rule which matches the provided string representation
+   */
+  private Rule createRule(String rule) {
+    return new Rule(rule.startsWith("RULE:") ? rule : String.format("RULE:%s", rule));
+  }
+
+
+  /**
+   * Rule implementation.
+   */
+  private static class Rule implements Comparable<Rule> {
+    /**
+     * pattern used to parse existing rules
+     */
+    private static final Pattern PATTERN_RULE_PARSE =
+        Pattern.compile("RULE:\\s*\\[\\s*(\\d)\\s*:\\s*(.+?)(?:@(.+?))??\\s*\\]\\s*\\((.+?)\\)\\s*(.*)");
+
+    /**
+     * associated principal
+     */
+    private Principal principal;
+
+    /**
+     * string representation of the rule
+     */
+    private String rule;
+
+    /**
+     * expected component count
+     */
+    private int expectedComponentCount;
+
+    /**
+     * number of components being matched in the rule
+     */
+    private int matchComponentCount;
+
+    /**
+     * Constructor.
+     *
+     * @param principal               principal
+     * @param expectedComponentCount  number of components needed by a principal to match
+     * @param matchComponentCount     number of components which are included in the rule evaluation
+     * @param rule                    string representation of the rule
+     */
+    public Rule(Principal principal, int expectedComponentCount, int matchComponentCount, String rule) {
+      this.principal = principal;
+      this.expectedComponentCount = expectedComponentCount;
+      this.matchComponentCount = matchComponentCount;
+      this.rule = rule;
+    }
+
+    /**
+     * Constructor.
+     *
+     * @param rule  string representation of the rule
+     */
+    public Rule(String rule) {
+      //this.rule = rule;
+      Matcher m = PATTERN_RULE_PARSE.matcher(rule);
+      if (! m.matches()) {
+        throw new IllegalArgumentException("Invalid rule: " + rule);
+      }
+      expectedComponentCount = Integer.valueOf(m.group(1));
+
+      String matchPattern = m.group(2);
+      matchComponentCount = (matchPattern.startsWith("$") ?
+          matchPattern.substring(1) :
+          matchPattern).
+            split("\\$").length;
+      String patternRealm = m.group(3);
+      principal = new Principal(m.group(4));
+      String replacementRule = m.group(5);
+      if (patternRealm != null) {
+        this.rule = String.format("RULE:[%d:%s@%s](%s)%s",
+            expectedComponentCount, matchPattern, patternRealm,
+            principal.toString(), replacementRule);
+      } else {
+        this.rule = String.format("RULE:[%d:%s](%s)%s",
+            expectedComponentCount, matchPattern,
+            principal.toString(), replacementRule);
+      }
+    }
+
+    /**
+     * Get the associated principal.
+     *
+     * @return associated principal
+     */
+    public Principal getPrincipal() {
+      return principal;
+    }
+
+    /**
+     * Get the expected component count.  This specified the number of components
+     * that a principal must contain to match this rule.
+     *
+     * @return the expected component count
+     */
+    public int getExpectedComponentCount() {
+      return expectedComponentCount;
+    }
+
+    /**
+     * Get the match component count.  This is the number of components that are evaluated
+     * when attempting to match a principal to the rule.
+     *
+     * @return the match component count
+     */
+    public int getMatchComponentCount() {
+      return matchComponentCount;
+    }
+
+    /**
+     * String representation of the rule in the form
+     * RULE:[componentCount:matchString](me@foo.com)s/pattern/localUser/
+     *
+     * @return string representation of the rule
+     */
+    @Override
+    public String toString() {
+      return rule;
+    }
+
+    /**
+     * Compares rules.
+     * <p>
+     * For rules with different expected component counts, the default string comparison is used.
+     * For rules with the same expected component count rules are ordered so that rules with a higher
+     * match component count occur first.
+     * <p>
+     * For rules with the same expected component count, default realm rules in the form of
+     * .*@myRealm.com are ordered last.
+     *
+     * @param other  the other rule to compare
+     *
+     * @return a negative integer, zero, or a positive integer as this object is less than,
+     *         equal to, or greater than the specified object
+     */
+    @Override
+    public int compareTo(Rule other) {
+      Principal thatPrincipal = other.getPrincipal();
+      //todo: better implementation that recursively evaluates realm and all components
+      if (expectedComponentCount != other.getExpectedComponentCount()) {
+        return rule.compareTo(other.rule);
+      } else {
+        if (matchComponentCount != other.getMatchComponentCount()) {
+          return other.getMatchComponentCount() - matchComponentCount;
+        } else {
+          if (principal.equals(thatPrincipal)) {
+            return rule.compareTo(other.rule);
+          } else {
+            // check for wildcard realms '.*'
+            String realm = principal.getRealm();
+            String thatRealm = thatPrincipal.getRealm();
+            if (realm == null ? thatRealm != null : ! realm.equals(thatRealm)) {
+              if (realm != null && realm.equals(".*")) {
+                return 1;
+              } else if (thatRealm != null && thatRealm.equals(".*")) {
+                return -1;
+              }
+            }
+            // check for wildcard component 1
+            String component1 = principal.getComponent(1);
+            String thatComponent1 = thatPrincipal.getComponent(1);
+            if (component1 != null && component1.equals(".*")) {
+              return 1;
+            } else if(thatComponent1 != null && thatComponent1.equals(".*")) {
+              return -1;
+            } else {
+              return rule.compareTo(other.rule);
+            }
+          }
+        }
+      }
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      return this == o || o instanceof Rule && rule.equals(((Rule) o).rule);
+    }
+
+    @Override
+    public int hashCode() {
+      return rule.hashCode();
+    }
+  }
+
+  /**
+   * Principal implementation.
+   */
+  private static class Principal {
+
+    /**
+     * principal pattern which allows for null realm
+     */
+    private static final Pattern p = Pattern.compile("([^@]+)(?:@(.*))?");
+
+    /**
+     * string representation
+     */
+    private String principal;
+
+    /**
+     * associated realm
+     */
+    private String realm;
+
+    /**
+     * list of components in the principal not including the realm
+     */
+    private List<String> components;
+
+    /**
+     * Constructor.
+     *
+     * @param principal  string representation of the principal
+     */
+    public Principal(String principal) {
+      this.principal = principal;
+
+      Matcher m = p.matcher(principal);
+
+      if (m.matches()) {
+        String allComponents = m.group(1);
+        if (allComponents == null) {
+          components = Collections.emptyList();
+        } else {
+          allComponents = allComponents.startsWith("/") ? allComponents.substring(1) : allComponents;
+          components = Arrays.asList(allComponents.split("/"));
+        }
+        realm = m.group(2);
+      } else {
+        throw new IllegalArgumentException("Invalid Principal: " + principal);
+      }
+    }
+
+    /**
+     * Get all of the components which make up the principal.
+     *
+     * @return list of principal components
+     */
+    public List<String> getComponents() {
+      return components;
+    }
+
+    /**
+     * Get the component at the specified location.
+     * Uses the range 1-n to match the notation used in the rule.
+     *
+     * @param position position of the component in the range 1-n
+     *
+     * @return the component at the specified location or null
+     */
+    public String getComponent(int position) {
+      if (position > components.size()) {
+        return null;
+      } else {
+        return components.get(position - 1);
+      }
+    }
+
+    /**
+     * Get the associated realm.
+     *
+     * @return the associated realm
+     */
+    public String getRealm() {
+      return realm;
+    }
+
+    @Override
+    public String toString() {
+      return principal;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+
+      Principal principal1 = (Principal) o;
+
+      return components.equals(principal1.components) &&
+             principal.equals(principal1.principal) &&
+             !(realm != null ?
+                 !realm.equals(principal1.realm) :
+                 principal1.realm != null);
+
+    }
+
+    @Override
+    public int hashCode() {
+      int result = principal.hashCode();
+      result = 31 * result + (realm != null ? realm.hashCode() : 0);
+      result = 31 * result + components.hashCode();
+      return result;
+    }
+  }
 }

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

@@ -366,6 +366,11 @@ public class KerberosHelper {
               // Calculate the current host-specific configurations. These will be used to replace
               // variables within the Kerberos descriptor data
               Map<String, Map<String, String>> configurations = calculateConfigurations(cluster, hostname);
+              // add existing kerberos auth_to_local rules to builder
+              if (configurations.containsKey("core-site")) {
+                authToLocalBuilder.addRules(
+                    configurations.get("core-site").get("hadoop.security.auth_to_local"));
+              }
 
               // A map to hold un-categorized properties.  This may come from the KerberosDescriptor
               // and will also contain a value for the current host
@@ -1023,7 +1028,7 @@ public class KerberosHelper {
         if ((identityFilter == null) || identityFilter.contains(identity.getName())) {
           KerberosPrincipalDescriptor principalDescriptor = identity.getPrincipalDescriptor();
           if (principalDescriptor != null) {
-            authToLocalBuilder.append(
+            authToLocalBuilder.addRule(
                 KerberosDescriptor.replaceVariables(principalDescriptor.getValue(), configurations),
                 KerberosDescriptor.replaceVariables(principalDescriptor.getLocalUsername(), configurations));
           }

ambari-server/src/main/resources/common-services/HDFS/2.1.0.2.0/configuration/core-site.xml

@@ -136,11 +136,6 @@
   <property>
     <name>hadoop.security.auth_to_local</name>
     <value>
-        RULE:[2:$1@$0]([rn]m@.*)s/.*/yarn/
-        RULE:[2:$1@$0](jhs@.*)s/.*/mapred/
-        RULE:[2:$1@$0]([nd]n@.*)s/.*/hdfs/
-        RULE:[2:$1@$0](hm@.*)s/.*/hbase/
-        RULE:[2:$1@$0](rs@.*)s/.*/hbase/
         DEFAULT
     </value>
 <description>The mapping from kerberos principal names to local OS mapreduce.job.user.names.

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

@@ -25,64 +25,215 @@ import static org.junit.Assert.*;
 public class AuthToLocalBuilderTest {
 
   @Test
-  public void testExpectedRules() {
+  public void testRuleGeneration() {
     AuthToLocalBuilder builder = new AuthToLocalBuilder();
 
-    builder.append("nn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
     // Duplicate principal for secondary namenode, should be filtered out...
-    builder.append("nn/_HOST@EXAMPLE.COM", "hdfs");
-    builder.append("dn/_HOST@EXAMPLE.COM", "hdfs");
-    builder.append("jn/_HOST@EXAMPLE.COM", "hdfs");
-    builder.append("rm/_HOST@EXAMPLE.COM", "yarn");
-    builder.append("jhs/_HOST@EXAMPLE.COM", "mapred");
-    builder.append("hm/_HOST@EXAMPLE.COM", "hbase");
-    builder.append("rs/_HOST@EXAMPLE.COM", "hbase");
-
-    builder.append("foobar@EXAMPLE.COM", "hdfs");
-
-    assertEquals("RULE:[2:$1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[2:$1@$0](hm@EXAMPLE.COM)s/.*/hbase/\n" +
-            "RULE:[2:$1@$0](jhs@EXAMPLE.COM)s/.*/mapred/\n" +
-            "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[2:$1@$0](nn@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[2:$1@$0](rm@EXAMPLE.COM)s/.*/yarn/\n" +
-            "RULE:[2:$1@$0](rs@EXAMPLE.COM)s/.*/hbase/\n" +
-            "RULE:[1:$1@$0](foobar@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
-            "DEFAULT",
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("dn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("jn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("rm/_HOST@EXAMPLE.COM", "yarn");
+    builder.addRule("jhs/_HOST@EXAMPLE.COM", "mapred");
+    builder.addRule("hm/_HOST@EXAMPLE.COM", "hbase");
+    builder.addRule("rs/_HOST@EXAMPLE.COM", "hbase");
+
+    builder.addRule("foobar@EXAMPLE.COM", "hdfs");
+
+    assertEquals(
+        "RULE:[1:$1@$0](foobar@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "RULE:[2:$1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](hm@EXAMPLE.COM)s/.*/hbase/\n" +
+        "RULE:[2:$1@$0](jhs@EXAMPLE.COM)s/.*/mapred/\n" +
+        "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](nn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](rm@EXAMPLE.COM)s/.*/yarn/\n" +
+        "RULE:[2:$1@$0](rs@EXAMPLE.COM)s/.*/hbase/\n" +
+        "DEFAULT",
+      builder.generate("EXAMPLE.COM"));
+  }
+
+  @Test
+  public void testRuleGeneration_ExistingRules() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    // previously generated non-host specific rules
+    builder.addRule("foobar@EXAMPLE.COM", "hdfs");
+    // doesn't exist in latter generation
+    builder.addRule("hm/_HOST@EXAMPLE.COM", "hbase");
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
+    String existingRules = builder.generate("EXAMPLE.COM");
+
+    builder = new AuthToLocalBuilder();
+    // set previously existing rules
+    builder.addRules(existingRules);
+
+    builder.addRule("dn/_HOST@EXAMPLE.COM", "hdfs");
+    // Duplicate of existing rule should not result in duplicate rule generation
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
+    // duplicated again in this builder should not result in duplicate rule generation
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("jn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("rm/_HOST@EXAMPLE.COM", "yarn");
+    builder.addRule("jhs/_HOST@EXAMPLE.COM", "mapred");
+    builder.addRule("rs/_HOST@EXAMPLE.COM", "hbase");
+
+    assertEquals(
+        "RULE:[1:$1@$0](foobar@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "RULE:[2:$1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](hm@EXAMPLE.COM)s/.*/hbase/\n" +
+        "RULE:[2:$1@$0](jhs@EXAMPLE.COM)s/.*/mapred/\n" +
+        "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](nn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](rm@EXAMPLE.COM)s/.*/yarn/\n" +
+        "RULE:[2:$1@$0](rs@EXAMPLE.COM)s/.*/hbase/\n" +
+        "DEFAULT",
         builder.generate("EXAMPLE.COM"));
   }
 
-  public void testUnexpectedRules() {
+  @Test
+  public void testRuleGeneration_ExistingRules_existingMoreSpecificRule() {
     AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    // previously generated non-host specific rules
+    builder.addRule("foobar@EXAMPLE.COM", "hdfs");
+    builder.addRule("hm/_HOST@EXAMPLE.COM", "hbase");
+    builder.addRule("jn/_HOST@EXAMPLE.COM", "hdfs");
+    String existingRules = builder.generate("EXAMPLE.COM");
+    // prepend host specific rule
+    existingRules = "RULE:[2:$1/$2@$0](dn/somehost.com@EXAMPLE.COM)s/.*/hdfs/\n" + existingRules;
+    // append default realm rule for additional realm
+    existingRules += "\nRULE:[1:$1@$0](.*@OTHER_REALM.COM)s/@.*//";
 
-    builder.append("nn/c6501.ambari.apache.org", "hdfs");
+    builder = new AuthToLocalBuilder();
+    // set previously existing rules
+    builder.addRules(existingRules);
+    // more specific host qualifed rule exists for dn
+    // non-host specific rule should still be generated but occur later in generated string
+    builder.addRule("dn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
     // Duplicate principal for secondary namenode, should be filtered out...
-    builder.append("nn/c6502.ambari.apache.org@EXAMPLE.COM", "hdfs");
-    builder.append("dn/c6501.ambari.apache.org@EXAMPLE.COM", "hdfs");
-    builder.append("jn/c6501.ambari.apache.org@EXAMPLE.COM", "hdfs");
-    builder.append("rm/c6501.ambari.apache.org@EXAMPLE.COM", "yarn");
-    builder.append("jhs/c6501.ambari.apache.org@EXAMPLE.COM", "mapred");
-    builder.append("hm/c6501.ambari.apache.org@EXAMPLE.COM", "hbase");
-    builder.append("rs/c6501.ambari.apache.org@EXAMPLE.COM", "hbase");
-
-    builder.append("hdfs@EXAMPLE.COM", "hdfs");
-    builder.append("hdfs/admin@EXAMPLE.COM", "hdfs");
-
-    // This is an unexpected invalid principal format, it should be ignored
-    builder.append("hdfs:admin@EXAMPLE.COM", "hdfs");
-
-    assertEquals("RULE:[2:$1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[2:$1@$0](hm@EXAMPLE.COM)s/.*/hbase/\n" +
-            "RULE:[2:$1@$0](jhs@EXAMPLE.COM)s/.*/mapred/\n" +
-            "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[2:$1@$0](nn@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[2:$1@$0](rm@EXAMPLE.COM)s/.*/yarn/\n" +
-            "RULE:[2:$1@$0](rs@EXAMPLE.COM)s/.*/hbase/\n" +
-            "RULE:[1:$1@$0](hdfs@EXAMPLE.COM)s/.*/hdfs/\n" +
-            "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
-            "DEFAULT",
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
+    // duplicate of existing rule
+    builder.addRule("jn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("rm/_HOST@EXAMPLE.COM", "yarn");
+    builder.addRule("jhs/_HOST@EXAMPLE.COM", "mapred");
+    builder.addRule("rs/_HOST@EXAMPLE.COM", "hbase");
+
+
+    assertEquals(
+        "RULE:[1:$1@$0](foobar@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "RULE:[1:$1@$0](.*@OTHER_REALM.COM)s/@.*//\n" +
+        "RULE:[2:$1/$2@$0](dn/somehost.com@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](hm@EXAMPLE.COM)s/.*/hbase/\n" +
+        "RULE:[2:$1@$0](jhs@EXAMPLE.COM)s/.*/mapred/\n" +
+        "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](nn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](rm@EXAMPLE.COM)s/.*/yarn/\n" +
+        "RULE:[2:$1@$0](rs@EXAMPLE.COM)s/.*/hbase/\n" +
+        "DEFAULT",
+        builder.generate("EXAMPLE.COM"));
+  }
+
+  @Test
+  public void testAddNullExistingRule() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    builder.addRules(null);
+
+    assertEquals(
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "DEFAULT",
+        builder.generate("EXAMPLE.COM")
+    );
+  }
+
+
+  @Test
+  public void testRulesWithWhitespace() {
+    String rulesWithWhitespace =
+        "RULE:   [1:$1@$0](foobar@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[  1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "RULE:[2:   $1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0   ](hm@EXAMPLE.COM)s/.*/hbase/\n" +
+        "RULE:[2:$1@$0]   (jhs@EXAMPLE.COM)s/.*/mapred/\n" +
+        "RULE:[2:$1@$0](jn@EXAMPLE.COM)   s/.*/hdfs/\n";
+
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    builder.addRules(rulesWithWhitespace);
+
+    assertEquals(
+        "RULE:[1:$1@$0](foobar@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "RULE:[2:$1@$0](dn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](hm@EXAMPLE.COM)s/.*/hbase/\n" +
+        "RULE:[2:$1@$0](jhs@EXAMPLE.COM)s/.*/mapred/\n" +
+        "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "DEFAULT",
         builder.generate("EXAMPLE.COM"));
+
   }
 
+  @Test
+  public void testExistingRuleWithNoRealm() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    builder.addRules("RULE:[1:$1](foobar)s/.*/hdfs/");
+
+    assertEquals(
+        "RULE:[1:$1](foobar)s/.*/hdfs/\n" +
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "DEFAULT",
+        builder.generate("EXAMPLE.COM"));
+  }
+
+  @Test
+  public void testExistingRuleWithNoRealm2() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    builder.addRules("RULE:[1:$1/$2](foobar/someHost)s/.*/hdfs/");
+
+    assertEquals(
+        "RULE:[1:$1/$2](foobar/someHost)s/.*/hdfs/\n" +
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "DEFAULT",
+        builder.generate("EXAMPLE.COM"));
+  }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testAddNewRuleWithNoRealm() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+
+    builder.addRule("someUser", "hdfs");
+  }
+
+  @Test(expected=IllegalArgumentException.class)
+  public void testAddNewRuleWithNoRealm2() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+
+    builder.addRule("someUser/someHost", "hdfs");
+  }
+
+  @Test
+  public void testExistingWildcardRealm() {
+    AuthToLocalBuilder builder = new AuthToLocalBuilder();
+    builder.addRules("RULE:[2:$1@$0]([rn]m@.*)s/.*/yarn/\n" +
+                     "RULE:[2:$1@$0]([nd]n@.*)s/.*/hdfs/\n" +
+                     "RULE:[2:$1@$0](.*@EXAMPLE.COM)s/.*/yarn/\n" +
+                     "DEFAULT");
+    builder.addRule("nn/_HOST@EXAMPLE.COM", "hdfs");
+    builder.addRule("jn/_HOST@EXAMPLE.COM", "hdfs");
+
+    // ensure that no default realm rule is generated for .* realm and
+    // also that that .* realm rules are ordered last in relation to
+    // other rules with the same number of expected principal components
+    assertEquals(
+        "RULE:[1:$1@$0](.*@EXAMPLE.COM)s/@.*//\n" +
+        "RULE:[2:$1@$0](jn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](nn@EXAMPLE.COM)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0](.*@EXAMPLE.COM)s/.*/yarn/\n" +
+        "RULE:[2:$1@$0]([nd]n@.*)s/.*/hdfs/\n" +
+        "RULE:[2:$1@$0]([rn]m@.*)s/.*/yarn/\n" +
+        "DEFAULT",
+        builder.generate("EXAMPLE.COM"));
+  }
 }