AMBARI-11202. templeton.hive.properties property value substitution should be done by ambari-server (rlevas)

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

@@ -92,6 +92,7 @@ import org.apache.ambari.server.state.kerberos.KerberosIdentityDescriptor;
 import org.apache.ambari.server.state.kerberos.KerberosKeytabDescriptor;
 import org.apache.ambari.server.state.kerberos.KerberosPrincipalDescriptor;
 import org.apache.ambari.server.state.kerberos.KerberosServiceDescriptor;
+import org.apache.ambari.server.state.kerberos.VariableReplacementHelper;
 import org.apache.ambari.server.state.svccomphost.ServiceComponentHostServerActionEvent;
 import org.apache.ambari.server.utils.StageUtils;
 import org.apache.commons.io.FileUtils;
@@ -180,6 +181,9 @@ public class KerberosHelper {
   @Inject
   private KerberosConfigDataFileWriterFactory kerberosConfigDataFileWriterFactory;
 
+  @Inject
+  private VariableReplacementHelper variableReplacementHelper;
+
   /**
    * Used to get kerberos descriptors associated with the cluster or stack.
    * Currently not available via injection.
@@ -1085,9 +1089,9 @@ public class KerberosHelper {
                       // Add the relevant principal name and keytab file data to the command params state
                       if (!commandParameters.containsKey("principal_name") || !commandParameters.containsKey("keytab_file")) {
                         commandParameters.put("principal_name",
-                            KerberosDescriptor.replaceVariables(identity.getPrincipalDescriptor().getValue(), configurations));
+                            variableReplacementHelper.replaceVariables(identity.getPrincipalDescriptor().getValue(), configurations));
                         commandParameters.put("keytab_file",
-                            KerberosDescriptor.replaceVariables(identity.getKeytabDescriptor().getFile(), configurations));
+                            variableReplacementHelper.replaceVariables(identity.getKeytabDescriptor().getFile(), configurations));
                       }
 
                       serviceComponentHostsToProcess.add(sch);
@@ -1400,7 +1404,7 @@ public class KerberosHelper {
    * 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)}
+   * See {@link VariableReplacementHelper#replaceVariables(String, Map)}
    * for information on variable replacement.
    *
    * @param configurations a Map of configurations
@@ -1437,7 +1441,7 @@ public class KerberosHelper {
    * 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)}
+   * See {@link VariableReplacementHelper#replaceVariables(String, Map)}
    * for information on variable replacement.
    *
    * @param configurations             the Map of configuration types to update
@@ -1466,7 +1470,7 @@ public class KerberosHelper {
    * 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)}
+   * See {@link VariableReplacementHelper#replaceVariables(String, Map)}
    * for information on variable replacement.
    *
    * @param configurations a Map of configurations
@@ -1487,8 +1491,8 @@ public class KerberosHelper {
 
       for (Map.Entry<String, String> property : updates.entrySet()) {
         existingProperties.put(
-            KerberosDescriptor.replaceVariables(property.getKey(), replacements),
-            KerberosDescriptor.replaceVariables(property.getValue(), replacements)
+            variableReplacementHelper.replaceVariables(property.getKey(), replacements),
+            variableReplacementHelper.replaceVariables(property.getValue(), replacements)
         );
       }
     }
@@ -1532,9 +1536,9 @@ public class KerberosHelper {
           String principalConfiguration = null;
 
           if (principalDescriptor != null) {
-            principal = KerberosDescriptor.replaceVariables(principalDescriptor.getValue(), configurations);
+            principal = variableReplacementHelper.replaceVariables(principalDescriptor.getValue(), configurations);
             principalType = principalDescriptor.getType().name().toLowerCase();
-            principalConfiguration = KerberosDescriptor.replaceVariables(principalDescriptor.getConfiguration(), configurations);
+            principalConfiguration = variableReplacementHelper.replaceVariables(principalDescriptor.getConfiguration(), configurations);
           }
 
           if (principal != null) {
@@ -1548,12 +1552,12 @@ public class KerberosHelper {
             boolean keytabIsCachable = false;
 
             if (keytabDescriptor != null) {
-              keytabFilePath = KerberosDescriptor.replaceVariables(keytabDescriptor.getFile(), configurations);
-              keytabFileOwnerName = KerberosDescriptor.replaceVariables(keytabDescriptor.getOwnerName(), configurations);
-              keytabFileOwnerAccess = KerberosDescriptor.replaceVariables(keytabDescriptor.getOwnerAccess(), configurations);
-              keytabFileGroupName = KerberosDescriptor.replaceVariables(keytabDescriptor.getGroupName(), configurations);
-              keytabFileGroupAccess = KerberosDescriptor.replaceVariables(keytabDescriptor.getGroupAccess(), configurations);
-              keytabFileConfiguration = KerberosDescriptor.replaceVariables(keytabDescriptor.getConfiguration(), configurations);
+              keytabFilePath = variableReplacementHelper.replaceVariables(keytabDescriptor.getFile(), configurations);
+              keytabFileOwnerName = variableReplacementHelper.replaceVariables(keytabDescriptor.getOwnerName(), configurations);
+              keytabFileOwnerAccess = variableReplacementHelper.replaceVariables(keytabDescriptor.getOwnerAccess(), configurations);
+              keytabFileGroupName = variableReplacementHelper.replaceVariables(keytabDescriptor.getGroupName(), configurations);
+              keytabFileGroupAccess = variableReplacementHelper.replaceVariables(keytabDescriptor.getGroupAccess(), configurations);
+              keytabFileConfiguration = variableReplacementHelper.replaceVariables(keytabDescriptor.getConfiguration(), configurations);
               keytabIsCachable = keytabDescriptor.isCachable();
             }
 
@@ -1608,8 +1612,8 @@ public class KerberosHelper {
           KerberosPrincipalDescriptor principalDescriptor = identity.getPrincipalDescriptor();
           if (principalDescriptor != null) {
             authToLocalBuilder.addRule(
-                KerberosDescriptor.replaceVariables(principalDescriptor.getValue(), configurations),
-                KerberosDescriptor.replaceVariables(principalDescriptor.getLocalUsername(), configurations));
+                variableReplacementHelper.replaceVariables(principalDescriptor.getValue(), configurations),
+                variableReplacementHelper.replaceVariables(principalDescriptor.getLocalUsername(), configurations));
           }
         }
       }
@@ -2073,7 +2077,7 @@ public class KerberosHelper {
               String principal = null;
 
               if (principalDescriptor != null) {
-                principal = KerberosDescriptor.replaceVariables(principalDescriptor.getValue(), configurations);
+                principal = variableReplacementHelper.replaceVariables(principalDescriptor.getValue(), configurations);
               }
 
               if (principal != null) {
@@ -2081,7 +2085,7 @@ public class KerberosHelper {
                 String keytabFile = null;
 
                 if (keytabDescriptor != null) {
-                  keytabFile = KerberosDescriptor.replaceVariables(keytabDescriptor.getFile(), configurations);
+                  keytabFile = variableReplacementHelper.replaceVariables(keytabDescriptor.getFile(), configurations);
                 }
 
                 if (replaceHostNames) {
@@ -2094,8 +2098,8 @@ public class KerberosHelper {
                   KerberosPrincipalDescriptor resolvedPrincipalDescriptor =
                       new KerberosPrincipalDescriptor(principal,
                           principalDescriptor.getType(),
-                          KerberosDescriptor.replaceVariables(principalDescriptor.getConfiguration(), configurations),
-                          KerberosDescriptor.replaceVariables(principalDescriptor.getLocalUsername(), configurations));
+                          variableReplacementHelper.replaceVariables(principalDescriptor.getConfiguration(), configurations),
+                          variableReplacementHelper.replaceVariables(principalDescriptor.getLocalUsername(), configurations));
 
                   KerberosKeytabDescriptor resolvedKeytabDescriptor;
 
@@ -2105,11 +2109,11 @@ public class KerberosHelper {
                     resolvedKeytabDescriptor =
                         new KerberosKeytabDescriptor(
                             keytabFile,
-                            KerberosDescriptor.replaceVariables(keytabDescriptor.getOwnerName(), configurations),
-                            KerberosDescriptor.replaceVariables(keytabDescriptor.getOwnerAccess(), configurations),
-                            KerberosDescriptor.replaceVariables(keytabDescriptor.getGroupName(), configurations),
-                            KerberosDescriptor.replaceVariables(keytabDescriptor.getGroupAccess(), configurations),
-                            KerberosDescriptor.replaceVariables(keytabDescriptor.getConfiguration(), configurations),
+                            variableReplacementHelper.replaceVariables(keytabDescriptor.getOwnerName(), configurations),
+                            variableReplacementHelper.replaceVariables(keytabDescriptor.getOwnerAccess(), configurations),
+                            variableReplacementHelper.replaceVariables(keytabDescriptor.getGroupName(), configurations),
+                            variableReplacementHelper.replaceVariables(keytabDescriptor.getGroupAccess(), configurations),
+                            variableReplacementHelper.replaceVariables(keytabDescriptor.getConfiguration(), configurations),
                             keytabDescriptor.isCachable());
                   }
 

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

@@ -18,20 +18,8 @@
 
 package org.apache.ambari.server.state.kerberos;
 
-import com.google.gson.Gson;
-import com.google.gson.JsonSyntaxException;
-import com.google.gson.reflect.TypeToken;
-import org.apache.ambari.server.AmbariException;
-
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.FileReader;
-import java.io.IOException;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 /**
  * AbstractKerberosDescriptor is the base class for all Kerberos*Descriptor and associated classes.
@@ -41,11 +29,6 @@ import java.util.regex.Pattern;
  */
 public abstract class AbstractKerberosDescriptor {
 
-  /**
-   * a regular expression Pattern used to find "variable" placeholders in strings
-   */
-  private static final Pattern PATTERN_VARIABLE = Pattern.compile("\\$\\{(?:([\\w\\-\\.]+)/)?([\\w\\-\\.]+)\\}");
-
   /**
    * An AbstractKerberosDescriptor serving as the parent (or container) for this
    * AbstractKerberosDescriptor.
@@ -61,76 +44,6 @@ public abstract class AbstractKerberosDescriptor {
    */
   private String name = null;
 
-  /**
-   * Performs variable replacement on the supplied String value using values from the replacementsMap.
-   * <p/>
-   * The value is a String containing one or more "variables" in the form of ${variable_name}, such
-   * that "variable_name" may indicate a group identifier; else "" is used as the group.
-   * For example:
-   * <p/>
-   * variable_name:  group: ""; property: "variable_name"
-   * group1/variable_name:  group: "group1"; property: "variable_name"
-   * root/group1/variable_name:  Not Supported
-   * <p/>
-   * The replacementsMap is a Map of Maps creating a (small) hierarchy of data to traverse in order
-   * to resolve the variable.
-   * <p/>
-   * If a variable resolves to one or more variables, that new variable(s) will be processed and replaced.
-   * If variable exists after a set number of iterations it is assumed that a cycle has been created
-   * and the process will abort returning a String in a possibly unexpected state.
-   *
-   * @param value           a String containing zero or more variables to be replaced
-   * @param replacementsMap a Map of data used to perform the variable replacements
-   * @return a new String
-   */
-  public static String replaceVariables(String value, Map<String, Map<String, String>> replacementsMap) throws AmbariException {
-    if ((value != null) && (replacementsMap != null) && !replacementsMap.isEmpty()) {
-      int count = 0; // Used to help prevent an infinite loop...
-      boolean replacementPerformed;
-
-      do {
-        if (++count > 1000) {
-          throw new AmbariException(String.format("Circular reference found while replacing variables in %s", value));
-        }
-
-        Matcher matcher = PATTERN_VARIABLE.matcher(value);
-        StringBuffer sb = new StringBuffer();
-
-        replacementPerformed = false;
-
-        while (matcher.find()) {
-          String type = matcher.group(1);
-          String name = matcher.group(2);
-          Map<String, String> replacements;
-
-          if ((name != null) && !name.isEmpty()) {
-            if (type == null) {
-              replacements = replacementsMap.get("");
-            } else {
-              replacements = replacementsMap.get(type);
-            }
-
-            if (replacements != null) {
-              String replacement = replacements.get(name);
-
-              if (replacement != null) {
-                // Escape '$' and '\' so they don't cause any issues.
-                matcher.appendReplacement(sb, replacement.replace("\\", "\\\\").replace("$", "\\$"));
-                replacementPerformed = true;
-              }
-            }
-          }
-        }
-
-        matcher.appendTail(sb);
-        value = sb.toString();
-      }
-      while (replacementPerformed); // Process the string again to make sure new variables were not introduced
-    }
-
-    return value;
-  }
-
   /**
    * Generates a Map of data that represents this AbstractKerberosDescriptor implementation.
    * <p/>

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

@@ -220,12 +220,12 @@ public class KerberosKeytabDescriptor extends AbstractKerberosDescriptor {
    * <ul>
    * <li>
    * ${variable} placeholders are replaced on the server - see
-   * {@link org.apache.ambari.server.state.kerberos.KerberosDescriptor#replaceVariables(String, java.util.Map)}
+   * {@link VariableReplacementHelper#replaceVariables(String, Map)}
    * </li>
    * </ul>
    *
    * @return a String declaring the keytab file's absolute path
-   * @see org.apache.ambari.server.state.kerberos.KerberosDescriptor#replaceVariables(String, java.util.Map)
+   * @see VariableReplacementHelper#replaceVariables(String, Map)
    */
   public String getFile() {
     return getName();

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

@@ -138,13 +138,13 @@ public class KerberosPrincipalDescriptor extends AbstractKerberosDescriptor {
    * <ul>
    * <li>
    * ${variable} placeholders are replaced on the server - see
-   * {@link org.apache.ambari.server.state.kerberos.KerberosDescriptor#replaceVariables(String, java.util.Map)}
+   * {@link VariableReplacementHelper#replaceVariables(String, Map)}
    * </li>
    * <li>the _HOST placeholder is replaced on the hosts to dynamically populate the relevant hostname</li>
    * </ul>
    *
    * @return a String declaring this principal's value
-   * @see org.apache.ambari.server.state.kerberos.KerberosDescriptor#replaceVariables(String, java.util.Map)
+   * @see VariableReplacementHelper#replaceVariables(String, Map)
    */
   public String getValue() {
     return getName();

ambari-server/src/main/resources/common-services/HIVE/0.12.0.2.0/kerberos.json

@@ -21,7 +21,7 @@
         {
           "webhcat-site": {
             "templeton.kerberos.secret": "secret",
-            "templeton.hive.properties": "hive.metastore.local=false,hive.metastore.uris=thrift://${clusterHostInfo/hive_metastore_host}:9083,hive.metastore.sasl.enabled=true,hive.metastore.execute.setugi=true,hive.metastore.warehouse.dir=/apps/hive/warehouse,hive.exec.mode.local.auto=false,hive.metastore.kerberos.principal=hive/_HOST@${realm}"
+            "templeton.hive.properties": "hive.metastore.local=false,hive.metastore.uris=${clusterHostInfo/hive_metastore_host|each(thrift://%s:9083, \\\\,, \\s*\\,\\s*)},hive.metastore.sasl.enabled=true,hive.metastore.execute.setugi=true,hive.metastore.warehouse.dir=/apps/hive/warehouse,hive.exec.mode.local.auto=false,hive.metastore.kerberos.principal=hive/_HOST@${realm}"
           }
         },
         {

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

@@ -318,128 +318,7 @@ public class KerberosDescriptorTest {
     validateUpdatedData(descriptor);
   }
 
-  @Test
-  public void testReplaceVariables() throws AmbariException {
-    Map<String, Map<String, String>> configurations = new HashMap<String, Map<String, String>>() {
-      {
-        put("", new HashMap<String, String>() {{
-          put("global_variable", "Hello World");
-          put("variable-name", "dash");
-          put("variable_name", "underscore");
-          put("variable.name", "dot");
-        }});
-
-        put("config_type", new HashMap<String, String>() {{
-          put("variable-name", "config_type_dash");
-          put("variable_name", "config_type_underscore");
-          put("variable.name", "config_type_dot");
-        }});
-
-        put("config.type", new HashMap<String, String>() {{
-          put("variable-name", "config.type_dash");
-          put("variable_name", "config.type_underscore");
-          put("variable.name", "config.type_dot");
-        }});
-
-        put("config-type", new HashMap<String, String>() {{
-          put("variable.name", "Replacement1");
-          put("variable.name1", "${config-type2/variable.name}");
-          put("variable.name2", "");
-        }});
-
-        put("config-type2", new HashMap<String, String>() {{
-          put("variable.name", "Replacement2");
-          put("self_reference", "${config-type2/self_reference}");  // This essentially references itself.
-          put("${config-type/variable.name}_reference", "Replacement in the key");
-        }});
-      }
-    };
-
-    Assert.assertEquals("concrete",
-        KerberosDescriptor.replaceVariables("concrete", configurations));
-
-    Assert.assertEquals("Hello World",
-        KerberosDescriptor.replaceVariables("${global_variable}", configurations));
-
-    Assert.assertEquals("Replacement1",
-        KerberosDescriptor.replaceVariables("${config-type/variable.name}", configurations));
-
-    Assert.assertEquals("Replacement1|Replacement2",
-        KerberosDescriptor.replaceVariables("${config-type/variable.name}|${config-type2/variable.name}", configurations));
-
-    Assert.assertEquals("Replacement1|Replacement2|${config-type3/variable.name}",
-        KerberosDescriptor.replaceVariables("${config-type/variable.name}|${config-type2/variable.name}|${config-type3/variable.name}", configurations));
-
-    Assert.assertEquals("Replacement2|Replacement2",
-        KerberosDescriptor.replaceVariables("${config-type/variable.name1}|${config-type2/variable.name}", configurations));
-
-    Assert.assertEquals("Replacement1_reference",
-        KerberosDescriptor.replaceVariables("${config-type/variable.name}_reference", configurations));
-
-    Assert.assertEquals("dash",
-        KerberosDescriptor.replaceVariables("${variable-name}", configurations));
-
-    Assert.assertEquals("underscore",
-        KerberosDescriptor.replaceVariables("${variable_name}", configurations));
-
-    Assert.assertEquals("config_type_dot",
-        KerberosDescriptor.replaceVariables("${config_type/variable.name}", configurations));
-
-    Assert.assertEquals("config_type_dash",
-        KerberosDescriptor.replaceVariables("${config_type/variable-name}", configurations));
-
-    Assert.assertEquals("config_type_underscore",
-        KerberosDescriptor.replaceVariables("${config_type/variable_name}", configurations));
-
-    Assert.assertEquals("config.type_dot",
-        KerberosDescriptor.replaceVariables("${config.type/variable.name}", configurations));
-
-    Assert.assertEquals("config.type_dash",
-        KerberosDescriptor.replaceVariables("${config.type/variable-name}", configurations));
-
-    Assert.assertEquals("config.type_underscore",
-        KerberosDescriptor.replaceVariables("${config.type/variable_name}", configurations));
-
-    Assert.assertEquals("dot",
-        KerberosDescriptor.replaceVariables("${variable.name}", configurations));
-
-    // Replacement yields an empty string
-    Assert.assertEquals("",
-        KerberosDescriptor.replaceVariables("${config-type/variable.name2}", configurations));
-
-
-    // This might cause an infinite loop... we assume protection is in place...
-    try {
-      Assert.assertEquals("${config-type2/self_reference}",
-          KerberosDescriptor.replaceVariables("${config-type2/self_reference}", configurations));
-      Assert.fail(String.format("%s expected to be thrown", AmbariException.class.getName()));
-    } catch (AmbariException e) {
-      // This is expected...
-    }
-  }
-
-  @Test
-  public void testReplaceComplicatedVariables() throws AmbariException {
-    Map<String, Map<String, String>> configurations = new HashMap<String, Map<String, String>>() {
-      {
-        put("", new HashMap<String, String>() {{
-          put("host", "c6401.ambari.apache.org");
-          put("realm", "EXAMPLE.COM");
-        }});
-      }
-    };
-
-    Assert.assertEquals("hive.metastore.local=false,hive.metastore.uris=thrift://c6401.ambari.apache.org:9083,hive.metastore.sasl.enabled=true,hive.metastore.execute.setugi=true,hive.metastore.warehouse.dir=/apps/hive/warehouse,hive.exec.mode.local.auto=false,hive.metastore.kerberos.principal=hive/_HOST@EXAMPLE.COM",
-        KerberosDescriptor.replaceVariables("hive.metastore.local=false,hive.metastore.uris=thrift://${host}:9083,hive.metastore.sasl.enabled=true,hive.metastore.execute.setugi=true,hive.metastore.warehouse.dir=/apps/hive/warehouse,hive.exec.mode.local.auto=false,hive.metastore.kerberos.principal=hive/_HOST@${realm}", configurations));
-
-    Assert.assertEquals("Hello my realm is {EXAMPLE.COM}",
-        KerberosDescriptor.replaceVariables("Hello my realm is {${realm}}", configurations));
-
-    Assert.assertEquals("$c6401.ambari.apache.org",
-        KerberosDescriptor.replaceVariables("$${host}", configurations));
-  }
-
-  @Test
+    @Test
   public void testGetReferencedIdentityDescriptor() throws IOException {
     URL systemResourceURL = ClassLoader.getSystemResource("kerberos/test_get_referenced_identity_descriptor.json");
     Assert.assertNotNull(systemResourceURL);

ambari-web/app/controllers/main/admin/kerberos/step4_controller.js

@@ -196,24 +196,10 @@ App.KerberosWizardStep4Controller = App.WizardStep7Controller.extend(App.AddSecu
   },
 
   /**
-   * Function to override kerberos descriptor's property values
+   * Function to override kerberos descriptor property values
    */
   tweakConfigProperty: function(config) {
-    if (config.name === 'templeton.hive.properties') {
-      var defaultHiveMsPort = "9083";
-      var hiveMSHosts = App.HostComponent.find().filterProperty('componentName', 'HIVE_METASTORE');
-      if (hiveMSHosts.length > 1) {
-        var hiveMSHostNames = hiveMSHosts.mapProperty('hostName');
-        var port = config.value.match(/:[0-9]{2,4}/);
-        port = port ? port[0].slice(1) : defaultHiveMsPort;
-        for (var i = 0; i < hiveMSHostNames.length; i++) {
-          hiveMSHostNames[i] = "thrift://" + hiveMSHostNames[i] + ":" + port;
-        }
-        var configValue =  config.value.replace(/thrift.+[0-9]{2,},/i, hiveMSHostNames.join('\\,') + ",");
-        config.set('value', configValue);
-        config.set('recommendedValue', configValue);
-      }
-    }
+    // This is a placeholder in the event kerberos descriptor property values need to be changed
   },
 
   /**