AMBARI-14901. NPE when configuring Kerberos at provisioning secure cluster with Blueprint. (Sandor Magyari via rnettleton)

ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java

@@ -161,9 +161,8 @@ public class ClusterConfigurationRequest {
         Map<String, String> clusterConfigProperties = existingConfigurations.get(configType);
         Map<String, String> stackDefaultConfigProperties = stackDefaultProps.get(configType);
         for (String property : propertyMap.keySet()) {
-          if (clusterConfigProperties == null || !clusterConfigProperties.containsKey(property)
-                 || (clusterConfigProperties.get(property) == null && stackDefaultConfigProperties.get(property) == null)
-                 || (clusterConfigProperties.get(property) != null && clusterConfigProperties.get(property).equals(stackDefaultConfigProperties.get(property)))) {
+          // update value only if property value configured in Blueprint /ClusterTemplate is not a custom one
+          if (!propertyHasCustomValue(clusterConfigProperties, stackDefaultConfigProperties, property)) {
             LOG.debug("Update Kerberos related config property: {} {} {}", configType, property, propertyMap.get
               (property));
             clusterConfiguration.setProperty(configType, property, propertyMap.get(property));
@@ -179,6 +178,36 @@ public class ClusterConfigurationRequest {
     return updatedConfigTypes;
   }
 
+  /**
+   * Returns true if the property exists in clusterConfigProperties and has a custom user defined value. Property has
+   * custom value in case we there's no stack default value for it or it's not equal to stack default value.
+   * @param clusterConfigProperties
+   * @param stackDefaultConfigProperties
+   * @param property
+   * @return
+   */
+  private boolean propertyHasCustomValue(Map<String, String> clusterConfigProperties, Map<String, String>
+    stackDefaultConfigProperties, String property) {
+
+    boolean propertyHasCustomValue = false;
+    if (clusterConfigProperties != null) {
+      String propertyValue = clusterConfigProperties.get(property);
+      if (propertyValue != null) {
+        if (stackDefaultConfigProperties != null) {
+          String stackDefaultValue = stackDefaultConfigProperties.get(property);
+          if (stackDefaultValue != null) {
+            propertyHasCustomValue = !propertyValue.equals(stackDefaultValue);
+          } else {
+            propertyHasCustomValue = true;
+          }
+        } else {
+          propertyHasCustomValue = true;
+        }
+      }
+    }
+    return propertyHasCustomValue;
+  }
+
   private Map<String, String> createComponentHostMap(Blueprint blueprint) {
     Map<String, String> componentHostsMap = new HashMap<String, String>();
     for (String service : blueprint.getServices()) {

ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java

@@ -18,13 +18,18 @@
 
 package org.apache.ambari.server.topology;
 
+import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorBlueprintProcessor;
 import org.apache.ambari.server.controller.AmbariManagementController;
 import org.apache.ambari.server.controller.ConfigurationRequest;
 import org.apache.ambari.server.controller.KerberosHelper;
+import org.apache.ambari.server.controller.internal.ConfigurationTopologyException;
 import org.apache.ambari.server.controller.internal.Stack;
+import org.apache.ambari.server.serveraction.kerberos.KerberosInvalidConfigurationException;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
+import org.easymock.Capture;
+import org.easymock.CaptureType;
 import org.easymock.EasyMockRule;
 import org.easymock.Mock;
 import org.easymock.MockType;
@@ -49,7 +54,10 @@ import static org.easymock.EasyMock.anyString;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.newCapture;
 import static org.easymock.EasyMock.verify;
+import static org.easymock.EasyMock.capture;
+import static org.junit.Assert.assertEquals;
 
 /**
  * ClusterConfigurationRequest unit tests
@@ -89,12 +97,82 @@ public class ClusterConfigurationRequestTest {
   @Mock(type = MockType.NICE)
   private KerberosHelper kerberosHelper;
 
+  /**
+   * testConfigType config type should be in updatedConfigTypes, as no custom property in Blueprint
+   * ==> Kerberos config property should be updated
+   * @throws Exception
+   */
   @Test
-  public void testProcessClusterConfigRequestIncludeKererosConfigs() throws Exception {
+  public void testProcessWithKerberos_UpdateKererosConfigProperty_WithNoCustomValue() throws Exception {
+
+    Capture<? extends Set<String>> captureUpdatedConfigTypes = testProcessWithKerberos(null, "defaultTestValue");
+
+    Set<String> updatedConfigTypes = captureUpdatedConfigTypes.getValue();
+    assertEquals(2, updatedConfigTypes.size());
+  }
+
+  /**
+   * testConfigType config type should be in updatedConfigTypes, as testProperty in Blueprint is equal to stack
+   * default ==> Kerberos config property should be updated
+   * @throws Exception
+   */
+  @Test
+  public void testProcessWithKerberos_UpdateKererosConfigProperty_WithCustomValueEqualToStackDefault() throws
+    Exception {
+
+    Capture<? extends Set<String>> captureUpdatedConfigTypes = testProcessWithKerberos("defaultTestValue", "defaultTestValue");
+
+    Set<String> updatedConfigTypes = captureUpdatedConfigTypes.getValue();
+    assertEquals(2, updatedConfigTypes.size());
+
+  }
+
+  /**
+   * testConfigType config type shouldn't be in updatedConfigTypes, as testProperty in Blueprint is different that
+   * stack default (custom value) ==> Kerberos config property shouldn't be updated
+   * @throws Exception
+   */
+  @Test
+  public void testProcessWithKerberos_DontUpdateKererosConfigProperty_WithCustomValueDifferentThanStackDefault() throws
+    Exception {
+
+    Capture<? extends Set<String>> captureUpdatedConfigTypes = testProcessWithKerberos("testPropertyValue", "defaultTestValue");
+
+    Set<String> updatedConfigTypes = captureUpdatedConfigTypes.getValue();
+    assertEquals(1, updatedConfigTypes.size());
+  }
+
+  /**
+   * testConfigType config type shouldn't be in updatedConfigTypes, as testProperty in Blueprint is a custom value
+   * (no default value in stack for testProperty)
+   * ==> Kerberos config property shouldn't be updated
+   * @throws Exception
+   */
+  @Test
+  public void testProcessWithKerberos_DontUpdateKererosConfigProperty_WithCustomValueNoStackDefault() throws Exception {
+
+    Capture<? extends Set<String>> captureUpdatedConfigTypes = testProcessWithKerberos("testPropertyValue", null);
+
+    Set<String> updatedConfigTypes = captureUpdatedConfigTypes.getValue();
+    assertEquals(1, updatedConfigTypes.size());
+  }
+
+  private Capture<? extends Set<String>> testProcessWithKerberos(String blueprintPropertyValue, String
+    stackPropertyValue) throws AmbariException, KerberosInvalidConfigurationException, ConfigurationTopologyException {
+
 
     Map<String, Map<String, String>> existingConfig = new HashMap<String, Map<String, String>>();
-    Configuration stackConfig = new Configuration(existingConfig,
+    Configuration stackDefaultConfig = new Configuration(existingConfig,
       new HashMap<String, Map<String, Map<String, String>>>());
+    if (stackPropertyValue != null) {
+      stackDefaultConfig.setProperty("testConfigType", "testProperty", stackPropertyValue);
+    }
+
+    Configuration blueprintConfig = new Configuration(stackDefaultConfig.getFullProperties(),
+      new HashMap<String, Map<String, Map<String, String>>>());
+    if (blueprintPropertyValue != null) {
+      blueprintConfig.setProperty("testConfigType", "testProperty", blueprintPropertyValue);
+    }
 
     PowerMock.mockStatic(AmbariContext.class);
     AmbariContext.getController();
@@ -117,7 +195,7 @@ public class ClusterConfigurationRequestTest {
     services.add("KERBEROS");
     services.add("ZOOKEPER");
     expect(blueprint.getServices()).andReturn(services).anyTimes();
-    expect(stack.getConfiguration(services)).andReturn(stackConfig).once();
+    expect(stack.getConfiguration(services)).andReturn(stackDefaultConfig).once();
 
     List<String> hdfsComponents = new ArrayList<>();
     hdfsComponents.add("NAMENODE");
@@ -132,7 +210,7 @@ public class ClusterConfigurationRequestTest {
 
     expect(topology.getConfigRecommendationStrategy()).andReturn(ConfigRecommendationStrategy.NEVER_APPLY).anyTimes();
     expect(topology.getBlueprint()).andReturn(blueprint).anyTimes();
-    expect(topology.getConfiguration()).andReturn(stackConfig).anyTimes();
+    expect(topology.getConfiguration()).andReturn(blueprintConfig).anyTimes();
     expect(topology.getHostGroupInfo()).andReturn(Collections.<String, HostGroupInfo>emptyMap());
     expect(topology.getClusterId()).andReturn(Long.valueOf(1)).anyTimes();
     expect(ambariContext.getClusterName(Long.valueOf(1))).andReturn("testCluster").anyTimes();
@@ -141,17 +219,22 @@ public class ClusterConfigurationRequestTest {
 
     Map<String, Map<String, String>> kerberosConfig = new HashMap<String, Map<String, String>>();
     Map<String, String> properties = new HashMap<>();
-    properties.put("testPorperty", "testValue");
+    properties.put("testProperty", "KERBEROStestValue");
     kerberosConfig.put("testConfigType", properties);
     expect(kerberosHelper.ensureHeadlessIdentities(anyObject(Cluster.class), anyObject(Map.class), anyObject
       (Set.class))).andReturn(true).once();
     expect(kerberosHelper.getServiceConfigurationUpdates(anyObject(Cluster.class), anyObject(Map.class), anyObject
       (Set.class), anyBoolean(), anyBoolean(), eq(false))).andReturn(kerberosConfig).once();
 
+    Capture<? extends String> captureClusterName = newCapture(CaptureType.ALL);
+    Capture<? extends Set<String>> captureUpdatedConfigTypes = newCapture(CaptureType.ALL);
+    ambariContext.waitForConfigurationResolution(capture(captureClusterName), capture
+      (captureUpdatedConfigTypes));
+    expectLastCall();
 
     PowerMock.replay(stack, blueprint, topology, controller, clusters, kerberosHelper, ambariContext,
       AmbariContext
-      .class);
+        .class);
 
     ClusterConfigurationRequest clusterConfigurationRequest = new ClusterConfigurationRequest(
       ambariContext, topology, false, stackAdvisorBlueprintProcessor, true);
@@ -159,6 +242,10 @@ public class ClusterConfigurationRequestTest {
 
     verify(blueprint, topology, ambariContext, controller, kerberosHelper);
 
+
+    String clusterName = captureClusterName.getValue();
+    assertEquals("testCluster", clusterName);
+    return captureUpdatedConfigTypes;
   }
 
   @Test
@@ -221,6 +308,4 @@ public class ClusterConfigurationRequestTest {
 
   }
 
-
-
 }