AMBARI-13075. Make ambari-server robust/debuggable if user accidentally adds a config type to a config groups when it does not exist as base type (aonishuk)

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

@@ -373,6 +373,17 @@ public class ConfigGroupResourceProvider extends
     return responses;
   }
 
+  private void verifyConfigs(Map<String, Config> configs, String clusterName) throws AmbariException {
+    Clusters clusters = getManagementController().getClusters();
+    for (String key : configs.keySet()) {
+      if(!clusters.getCluster(clusterName).isConfigTypeExists(key)){
+        throw new AmbariException("Trying to add not existent config type to config group:"+
+        " configType="+key+
+        " cluster="+clusterName);
+      }
+    }
+  }
+
   private void verifyHostList(Cluster cluster, Map<Long, Host> hosts,
                               ConfigGroupRequest request) throws AmbariException {
 
@@ -595,6 +606,7 @@ public class ConfigGroupResourceProvider extends
       configGroup.setHosts(hosts);
 
       // Update Configs
+      verifyConfigs(request.getConfigs(), request.getClusterName());
       configGroup.setConfigurations(request.getConfigs());
 
       // Save

ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java

@@ -380,6 +380,12 @@ public interface Cluster {
    */
   Config getDesiredConfigByType(String configType);
 
+  /**
+   * Check if config type exists in cluster.
+   * @param configType the type of configuration
+   * @return <code>true</code> if config type exists, else - <code>false</code>
+   */
+  boolean isConfigTypeExists(String configType);
   /**
    * Gets the desired configurations for the cluster.
    * @return a map of type-to-configuration information.

ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java

@@ -2010,6 +2010,11 @@ public class ClusterImpl implements Cluster {
           c.setServiceName(null);
           c.setTag(e.getTag());
           c.setUser(e.getUser());
+          if(!allConfigs.containsKey(e.getType())) {
+            LOG.error("Config inconsistency exists:" +
+                " unknown configType=" + e.getType());
+            continue;
+          }
           c.setVersion(allConfigs.get(e.getType()).get(e.getTag()).getVersion());
 
           map.put(e.getType(), c);
@@ -2454,6 +2459,22 @@ public class ClusterImpl implements Cluster {
     }
   }
 
+  @Override
+  public boolean isConfigTypeExists(String configType) {
+    clusterGlobalLock.readLock().lock();
+    try {
+      for (ClusterConfigMappingEntity e : clusterEntity.getConfigMappingEntities()) {
+        if (e.getType().equals(configType)) {
+          return true;
+        }
+      }
+
+      return false;
+    } finally {
+      clusterGlobalLock.readLock().unlock();
+    }
+  }
+
   @Override
   public Map<Long, Map<String, DesiredConfig>> getHostsDesiredConfigs(Collection<Long> hostIds) {
 

ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java

@@ -1355,7 +1355,12 @@ public class HostImpl implements Host {
             hostConfig = new HostConfig();
             hostConfigMap.put(configType, hostConfig);
             if (cluster != null) {
-              hostConfig.setDefaultVersionTag(cluster.getDesiredConfigByType(configType).getTag());
+              Config conf = cluster.getDesiredConfigByType(configType);
+              if(conf == null)
+                LOG.error("Config inconsistency exists:"+
+                    " unknown configType="+configType);
+              else
+                hostConfig.setDefaultVersionTag(conf.getTag());
             }
           }
           Config config = configEntry.getValue();

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

@@ -32,6 +32,7 @@ import org.apache.ambari.server.controller.spi.Request;
 import org.apache.ambari.server.controller.spi.Resource;
 import org.apache.ambari.server.controller.spi.ResourceAlreadyExistsException;
 import org.apache.ambari.server.controller.spi.ResourceProvider;
+import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.utilities.PredicateBuilder;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.apache.ambari.server.orm.InMemoryDefaultTestModule;
@@ -253,7 +254,111 @@ public class ConfigGroupResourceProviderTest {
     assertNotNull(exception);
     assertTrue(exception instanceof ResourceAlreadyExistsException);
   }
+  @Test
+  public void testUpdateConfigGroupWithWrongConfigType() throws Exception {
+    AmbariManagementController managementController = createMock(AmbariManagementController.class);
+    RequestStatusResponse response = createNiceMock(RequestStatusResponse.class);
+    ConfigHelper configHelper = createNiceMock(ConfigHelper.class);
+    Clusters clusters = createNiceMock(Clusters.class);
+    Cluster cluster = createNiceMock(Cluster.class);
+    Host h1 = createNiceMock(Host.class);
+    Host h2 = createNiceMock(Host.class);
+    HostEntity hostEntity1 = createMock(HostEntity.class);
+    HostEntity hostEntity2 = createMock(HostEntity.class);
+
+    final ConfigGroup configGroup = createNiceMock(ConfigGroup.class);
+    ConfigGroupResponse configGroupResponse = createNiceMock
+        (ConfigGroupResponse.class);
 
+    expect(cluster.isConfigTypeExists("core-site")).andReturn(false).anyTimes();
+    expect(managementController.getClusters()).andReturn(clusters).anyTimes();
+    expect(managementController.getAuthName()).andReturn("admin").anyTimes();
+    expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();
+    expect(clusters.getHost("h1")).andReturn(h1);
+    expect(clusters.getHost("h2")).andReturn(h2);
+    expect(hostDAO.findByName("h1")).andReturn(hostEntity1).anyTimes();
+    expect(hostDAO.findById(1L)).andReturn(hostEntity1).anyTimes();
+    expect(hostDAO.findByName("h2")).andReturn(hostEntity2).anyTimes();
+    expect(hostDAO.findById(2L)).andReturn(hostEntity2).anyTimes();
+    expect(hostEntity1.getHostId()).andReturn(1L).atLeastOnce();
+    expect(hostEntity2.getHostId()).andReturn(2L).atLeastOnce();
+    expect(h1.getHostId()).andReturn(1L).anyTimes();
+    expect(h2.getHostId()).andReturn(2L).anyTimes();
+
+    expect(configGroup.getName()).andReturn("test-1").anyTimes();
+    expect(configGroup.getId()).andReturn(25L).anyTimes();
+    expect(configGroup.getTag()).andReturn("tag-1").anyTimes();
+
+    expect(configGroup.convertToResponse()).andReturn(configGroupResponse).anyTimes();
+    expect(configGroupResponse.getClusterName()).andReturn("Cluster100").anyTimes();
+    expect(configGroupResponse.getId()).andReturn(25L).anyTimes();
+
+    expect(cluster.getConfigGroups()).andStubAnswer(new IAnswer<Map<Long, ConfigGroup>>() {
+      @Override
+      public Map<Long, ConfigGroup> answer() throws Throwable {
+        Map<Long, ConfigGroup> configGroupMap = new HashMap<Long, ConfigGroup>();
+        configGroupMap.put(configGroup.getId(), configGroup);
+        return configGroupMap;
+      }
+    });
+
+    replay(managementController, clusters, cluster,
+        configGroup, response, configGroupResponse, configHelper, hostDAO, hostEntity1, hostEntity2, h1, h2);
+
+    ResourceProvider provider = getConfigGroupResourceProvider
+        (managementController);
+
+    Map<String, Object> properties = new LinkedHashMap<String, Object>();
+
+    Set<Map<String, Object>> hostSet = new HashSet<Map<String, Object>>();
+    Map<String, Object> host1 = new HashMap<String, Object>();
+    host1.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, "h1");
+    hostSet.add(host1);
+    Map<String, Object> host2 = new HashMap<String, Object>();
+    host2.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTNAME_PROPERTY_ID, "h2");
+    hostSet.add(host2);
+
+    Set<Map<String, Object>> configSet = new HashSet<Map<String, Object>>();
+    Map<String, String> configMap = new HashMap<String, String>();
+    Map<String, Object> configs = new HashMap<String, Object>();
+    configs.put("type", "core-site");
+    configs.put("tag", "version100");
+    configMap.put("key1", "value1");
+    configs.put("properties", configMap);
+    configSet.add(configs);
+
+    properties.put(ConfigGroupResourceProvider
+        .CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID, "Cluster100");
+    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_NAME_PROPERTY_ID,
+        "test-1");
+    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_TAG_PROPERTY_ID,
+        "tag-1");
+    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_HOSTS_PROPERTY_ID,
+        hostSet );
+    properties.put(ConfigGroupResourceProvider.CONFIGGROUP_CONFIGS_PROPERTY_ID,
+        configSet);
+
+    Map<String, String> mapRequestProps = new HashMap<String, String>();
+    mapRequestProps.put("context", "Called from a test");
+
+    Request request = PropertyHelper.getUpdateRequest(properties, mapRequestProps);
+
+    Predicate predicate = new PredicateBuilder().property
+        (ConfigGroupResourceProvider.CONFIGGROUP_CLUSTER_NAME_PROPERTY_ID).equals
+        ("Cluster100").and().
+        property(ConfigGroupResourceProvider.CONFIGGROUP_ID_PROPERTY_ID).equals
+        (25L).toPredicate();
+    SystemException systemException = null;
+    try {
+      provider.updateResources(request, predicate);
+    }
+    catch (SystemException e){
+      systemException = e;
+    }
+    assertNotNull(systemException);
+    verify(managementController, clusters, cluster,
+        configGroup, response, configGroupResponse, configHelper, hostDAO, hostEntity1, hostEntity2, h1, h2);
+  }
   @Test
   public void testUpdateConfigGroup() throws Exception {
     AmbariManagementController managementController = createMock(AmbariManagementController.class);
@@ -270,6 +375,7 @@ public class ConfigGroupResourceProviderTest {
     ConfigGroupResponse configGroupResponse = createNiceMock
       (ConfigGroupResponse.class);
 
+    expect(cluster.isConfigTypeExists("core-site")).andReturn(true).anyTimes();
     expect(managementController.getClusters()).andReturn(clusters).anyTimes();
     expect(managementController.getAuthName()).andReturn("admin").anyTimes();
     expect(clusters.getCluster("Cluster100")).andReturn(cluster).anyTimes();