AMBARI-14325. Storm nimbus.seeds property not handled correctly in Blueprint processor resulting Storm not being able to start. (Sebastian Toader via rnettleton)

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java

@@ -1525,7 +1525,7 @@ public class BlueprintConfigurationProcessor {
    * Topology based updater which replaces original host names (possibly more than one) contained in a property
    * value with the host names which runs the associated component in the new cluster.
    */
-  private static class MultipleHostTopologyUpdater implements PropertyUpdater {
+  protected static class MultipleHostTopologyUpdater implements PropertyUpdater {
 
     private static final Character DEFAULT_SEPARATOR = ',';
 
@@ -1626,15 +1626,28 @@ public class BlueprintConfigurationProcessor {
         }
       }
 
+      return sb.append(resolveHostGroupPlaceholder(origValue, prefix, hostStrings)).toString();
+    }
+
+    /**
+     * Resolves the host group place holders in the passed in original value.
+     * @param originalValue The original value containing the place holders to be resolved.
+     * @param prefix The prefix to be added to the returned value.
+     * @param hostStrings The collection of host names that are mapped to the host groups to be resolved
+     * @return The new value with place holders resolved.
+     */
+    protected String resolveHostGroupPlaceholder(String originalValue, String prefix, Collection<String> hostStrings) {
       String suffix = null;
+      StringBuilder sb = new StringBuilder();
+
       // parse out prefix if one exists
-      Matcher matcher = HOSTGROUP_PORT_REGEX.matcher(origValue);
+      Matcher matcher = HOSTGROUP_PORT_REGEX.matcher(originalValue);
       if (matcher.find()) {
         int indexOfStart = matcher.start();
         // handle the case of a YAML config property
-        if ((indexOfStart > 0) && (!origValue.substring(0, indexOfStart).equals("['"))) {
+        if ((indexOfStart > 0) && (!originalValue.substring(0, indexOfStart).equals("['")) && (!originalValue.substring(0, indexOfStart).equals("[")) ) {
           // append prefix before adding host names
-          prefix = origValue.substring(0, indexOfStart);
+          prefix = originalValue.substring(0, indexOfStart);
           sb.append(prefix);
         }
 
@@ -1644,8 +1657,8 @@ public class BlueprintConfigurationProcessor {
           indexOfEnd = matcher.end();
         } while (matcher.find());
 
-        if (indexOfEnd < (origValue.length() - 1)) {
-          suffix = origValue.substring(indexOfEnd);
+        if (indexOfEnd < (originalValue.length())) {
+          suffix = originalValue.substring(indexOfEnd);
         }
       }
 
@@ -1664,7 +1677,7 @@ public class BlueprintConfigurationProcessor {
         sb.append(host);
       }
 
-      if ((suffix != null) && (!suffix.equals("']"))) {
+      if ((suffix != null) && (!suffix.equals("']")) && (!suffix.equals("]")) ) {
         sb.append(suffix);
       }
       return sb.toString();

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java

@@ -18,6 +18,7 @@
 
 package org.apache.ambari.server.controller.internal;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
@@ -6185,6 +6186,92 @@ public class BlueprintConfigurationProcessorTest {
     assertEquals(expectedValue, newValue);
   }
 
+  @Test
+  public void testMultipleHostTopologyUpdaterWithYamlPropertySingleHostValue() throws Exception {
+    // Given
+    String component = "test_component";
+    BlueprintConfigurationProcessor.MultipleHostTopologyUpdater mhtu = new BlueprintConfigurationProcessor.MultipleHostTopologyUpdater(component);
+
+    String propertyOriginalValue1 = "['%HOSTGROUP::group_1%']";
+    String propertyOriginalValue2 = "[%HOSTGROUP::group_1%]";
+
+    // When
+    String updatedValue1 = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue1, null, ImmutableList.<String>of("host1:100"));
+    String updatedValue2 = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue2, null, ImmutableList.<String>of("host1:100"));
+
+    // Then
+    assertEquals("host1:100", updatedValue1);
+
+    assertEquals("host1:100", updatedValue2);
+  }
+
+
+
+  @Test
+  public void testMultipleHostTopologyUpdaterWithYamlPropertyMultiHostValue() throws Exception {
+    // Given
+    String component = "test_component";
+    BlueprintConfigurationProcessor.MultipleHostTopologyUpdater mhtu = new BlueprintConfigurationProcessor.MultipleHostTopologyUpdater(component);
+
+    String propertyOriginalValue1 = "['%HOSTGROUP::group_1%', '%HOSTGROUP::group_2%']";
+    String propertyOriginalValue2 = "[%HOSTGROUP::group_1%, %HOSTGROUP::group_2%]";
+
+    // When
+    String updatedValue1 = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue1, null, ImmutableList.<String>of("host1:100", "host2:200"));
+    String updatedValue2 = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue2, null, ImmutableList.<String>of("host1:100", "host2:200"));
+
+    // Then
+    assertEquals("host1:100,host2:200", updatedValue1);
+
+    assertEquals("host1:100,host2:200", updatedValue2);
+  }
+
+
+  @Test
+  public void testMultipleHostTopologyUpdaterWithSingleHostWithSuffixValue() throws Exception {
+    // Given
+    String component = "test_component";
+    BlueprintConfigurationProcessor.MultipleHostTopologyUpdater mhtu = new BlueprintConfigurationProcessor.MultipleHostTopologyUpdater(component);
+
+    String propertyOriginalValue = "http://%HOSTGROUP::group_1%#";
+
+    // When
+    String updatedValue = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue, null, ImmutableList.<String>of("host1:100"));
+
+    // Then
+    assertEquals("http://host1:100#", updatedValue);
+  }
+
+  @Test
+  public void testMultipleHostTopologyUpdaterWithMultiHostWithSuffixValue() throws Exception {
+    // Given
+    String component = "test_component";
+    BlueprintConfigurationProcessor.MultipleHostTopologyUpdater mhtu = new BlueprintConfigurationProcessor.MultipleHostTopologyUpdater(component);
+
+    String propertyOriginalValue = "http://%HOSTGROUP::group_1,HOSTGROUP::group_2%/resource";
+
+    // When
+    String updatedValue = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue, null, ImmutableList.<String>of("host1:100", "host2:200"));
+
+    // Then
+    assertEquals("http://host1:100,host2:200/resource", updatedValue);
+  }
+
+  @Test
+  public void testMultipleHostTopologyUpdaterWithMultiHostValue() throws Exception {
+    // Given
+    String component = "test_component";
+    BlueprintConfigurationProcessor.MultipleHostTopologyUpdater mhtu = new BlueprintConfigurationProcessor.MultipleHostTopologyUpdater(component);
+
+    String propertyOriginalValue = "%HOSTGROUP::group_1%:11,%HOSTGROUP::group_2%:11";
+
+    // When
+    String updatedValue = mhtu.resolveHostGroupPlaceholder(propertyOriginalValue, null, ImmutableList.<String>of("host1:100", "host2:200"));
+
+    // Then
+    assertEquals("host1:100,host2:200", updatedValue);
+  }
+
 
   private Map<String, AdvisedConfiguration> createAdvisedConfigMap() {
     Map<String, AdvisedConfiguration> advMap = new HashMap<String, AdvisedConfiguration>();