Revert "AMBARI-20488 Config types are validated before the cluster resources are persisted"

This reverts commit 7cbad939f6cf3ebdde59c3829672683f334dd32b.

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

@@ -18,6 +18,7 @@
 package org.apache.ambari.server.controller.internal;
 
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -35,17 +36,15 @@ import org.apache.ambari.server.topology.Credential;
 import org.apache.ambari.server.topology.HostGroupInfo;
 import org.apache.ambari.server.topology.InvalidTopologyTemplateException;
 import org.apache.ambari.server.topology.NoSuchBlueprintException;
+import org.apache.ambari.server.topology.RequiredPasswordValidator;
 import org.apache.ambari.server.topology.SecurityConfiguration;
 import org.apache.ambari.server.topology.TopologyValidator;
-import org.apache.ambari.server.topology.validators.ClusterConfigTypeValidator;
-import org.apache.ambari.server.topology.validators.RequiredPasswordValidator;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.common.base.Enums;
 import com.google.common.base.Optional;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 
 /**
  * Request for provisioning a cluster.
@@ -119,6 +118,7 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
   public static final String QUICKLINKS_PROFILE_SERVICES_PROPERTY = "quicklinks_profile/services";
 
 
+
   /**
    * configuration factory
    */
@@ -145,8 +145,6 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
 
   private final String quickLinksProfileJson;
 
-  private final List<TopologyValidator> topologyValidators;
-
   private final static Logger LOG = LoggerFactory.getLogger(ProvisionClusterRequest.class);
 
   /**
@@ -179,7 +177,7 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
     this.securityConfiguration = securityConfiguration;
 
     Configuration configuration = configurationFactory.getConfiguration(
-      (Collection<Map<String, String>>) properties.get(CONFIGURATIONS_PROPERTY));
+        (Collection<Map<String, String>>) properties.get(CONFIGURATIONS_PROPERTY));
     configuration.setParentConfiguration(blueprint.getConfiguration());
     setConfiguration(configuration);
 
@@ -193,11 +191,10 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
 
     try {
       this.quickLinksProfileJson = processQuickLinksProfile(properties);
-    } catch (QuickLinksProfileEvaluationException ex) {
+    }
+    catch (QuickLinksProfileEvaluationException ex) {
       throw new InvalidTopologyTemplateException("Invalid quick links profile", ex);
     }
-
-    topologyValidators = ImmutableList.of(new RequiredPasswordValidator(defaultPassword), new ClusterConfigTypeValidator());
   }
 
   private String processQuickLinksProfile(Map<String, Object> properties) throws QuickLinksProfileEvaluationException {
@@ -272,7 +269,7 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
 
   @Override
   public List<TopologyValidator> getTopologyValidators() {
-    return topologyValidators;
+    return Collections.<TopologyValidator>singletonList(new RequiredPasswordValidator(defaultPassword));
   }
 
   @Override
@@ -307,7 +304,7 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
    */
   private void parseHostGroupInfo(Map<String, Object> properties) throws InvalidTopologyTemplateException {
     Collection<Map<String, Object>> hostGroups =
-      (Collection<Map<String, Object>>) properties.get(HOSTGROUPS_PROPERTY);
+        (Collection<Map<String, Object>>) properties.get(HOSTGROUPS_PROPERTY);
 
     if (hostGroups == null || hostGroups.isEmpty()) {
       throw new InvalidTopologyTemplateException("'host_groups' element must be included in cluster create body");
@@ -337,11 +334,11 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
 
     processHostCountAndPredicate(hostGroupProperties, hostGroupInfo);
     processGroupHosts(name, (Collection<Map<String, String>>)
-      hostGroupProperties.get(HOSTGROUP_HOSTS_PROPERTY), hostGroupInfo);
+        hostGroupProperties.get(HOSTGROUP_HOSTS_PROPERTY), hostGroupInfo);
 
     // don't set the parent configuration
     hostGroupInfo.setConfiguration(configurationFactory.getConfiguration(
-      (Collection<Map<String, String>>) hostGroupProperties.get(CONFIGURATIONS_PROPERTY)));
+        (Collection<Map<String, String>>) hostGroupProperties.get(CONFIGURATIONS_PROPERTY)));
   }
 
   /**
@@ -353,20 +350,20 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
    * @throws InvalidTopologyTemplateException  specified host group properties fail validation
    */
   private void processHostCountAndPredicate(Map<String, Object> hostGroupProperties, HostGroupInfo hostGroupInfo)
-    throws InvalidTopologyTemplateException {
+      throws InvalidTopologyTemplateException {
 
     if (hostGroupProperties.containsKey(HOSTGROUP_HOST_COUNT_PROPERTY)) {
       hostGroupInfo.setRequestedCount(Integer.valueOf(String.valueOf(
-        hostGroupProperties.get(HOSTGROUP_HOST_COUNT_PROPERTY))));
+          hostGroupProperties.get(HOSTGROUP_HOST_COUNT_PROPERTY))));
       LOG.info("Stored expected hosts count {} for group {}",
-        hostGroupInfo.getRequestedHostCount(), hostGroupInfo.getHostGroupName());
+               hostGroupInfo.getRequestedHostCount(), hostGroupInfo.getHostGroupName());
     }
 
     if (hostGroupProperties.containsKey(HOSTGROUP_HOST_PREDICATE_PROPERTY)) {
       if (hostGroupInfo.getRequestedHostCount() == 0) {
         throw new InvalidTopologyTemplateException(String.format(
-          "Host group '%s' must not specify 'host_predicate' without 'host_count'",
-          hostGroupInfo.getHostGroupName()));
+            "Host group '%s' must not specify 'host_predicate' without 'host_count'",
+            hostGroupInfo.getHostGroupName()));
       }
 
       String hostPredicate = String.valueOf(hostGroupProperties.get(HOSTGROUP_HOST_PREDICATE_PROPERTY));
@@ -376,7 +373,7 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
         LOG.info("Compiled host predicate {} for group {}", hostPredicate, hostGroupInfo.getHostGroupName());
       } catch (InvalidQueryException e) {
         throw new InvalidTopologyTemplateException(
-          String.format("Unable to compile host predicate '%s': %s", hostPredicate, e), e);
+            String.format("Unable to compile host predicate '%s': %s", hostPredicate, e), e);
       }
     }
   }
@@ -391,17 +388,17 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
    * @throws InvalidTopologyTemplateException specified host group properties fail validation
    */
   private void processGroupHosts(String name, Collection<Map<String, String>> hosts, HostGroupInfo hostGroupInfo)
-    throws InvalidTopologyTemplateException {
+      throws InvalidTopologyTemplateException {
 
     if (hosts != null) {
       if (hostGroupInfo.getRequestedHostCount() != 0) {
         throw new InvalidTopologyTemplateException(String.format(
-          "Host group '%s' must not contain both a 'hosts' element and a 'host_count' value", name));
+            "Host group '%s' must not contain both a 'hosts' element and a 'host_count' value", name));
       }
 
       if (hostGroupInfo.getPredicate() != null) {
         throw new InvalidTopologyTemplateException(String.format(
-          "Host group '%s' must not contain both a 'hosts' element and a 'host_predicate' value", name));
+            "Host group '%s' must not contain both a 'hosts' element and a 'host_predicate' value", name));
       }
 
       for (Map<String, String> hostProperties : hosts) {
@@ -411,15 +408,15 @@ public class ProvisionClusterRequest extends BaseClusterRequest {
 
         if (hostProperties.containsKey(HOSTGROUP_HOST_RACK_INFO_PROPERTY)) {
           hostGroupInfo.addHostRackInfo(
-            hostProperties.get(HOSTGROUP_HOST_FQDN_PROPERTY),
-            hostProperties.get(HOSTGROUP_HOST_RACK_INFO_PROPERTY));
+              hostProperties.get(HOSTGROUP_HOST_FQDN_PROPERTY),
+              hostProperties.get(HOSTGROUP_HOST_RACK_INFO_PROPERTY));
         }
       }
     }
 
     if (hostGroupInfo.getRequestedHostCount() == 0) {
       throw new InvalidTopologyTemplateException(String.format(
-        "Host group '%s' must contain at least one 'hosts/fqdn' or a 'host_count' value", name));
+          "Host group '%s' must contain at least one 'hosts/fqdn' or a 'host_count' value", name));
     }
   }
 

ambari-server/src/main/java/org/apache/ambari/server/topology/validators/RequiredPasswordValidator.java → ambari-server/src/main/java/org/apache/ambari/server/topology/RequiredPasswordValidator.java

@@ -10,28 +10,23 @@
  *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
+ * distributed under the License is distribut
+ * ed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
-package org.apache.ambari.server.topology.validators;
+package org.apache.ambari.server.topology;
+
+import org.apache.ambari.server.controller.internal.Stack;
+import org.apache.ambari.server.state.PropertyInfo;
 
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 
-import org.apache.ambari.server.controller.internal.Stack;
-import org.apache.ambari.server.state.PropertyInfo;
-import org.apache.ambari.server.topology.Blueprint;
-import org.apache.ambari.server.topology.ClusterTopology;
-import org.apache.ambari.server.topology.HostGroup;
-import org.apache.ambari.server.topology.HostGroupInfo;
-import org.apache.ambari.server.topology.InvalidTopologyException;
-import org.apache.ambari.server.topology.TopologyValidator;
-
 /**
  * Validates that all required passwords are provided.
  */

ambari-server/src/main/java/org/apache/ambari/server/topology/validators/ClusterConfigTypeValidator.java

@@ -1,76 +0,0 @@
-/*
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.ambari.server.topology.validators;
-
-import java.util.HashSet;
-import java.util.Set;
-
-import org.apache.ambari.server.topology.ClusterTopology;
-import org.apache.ambari.server.topology.InvalidTopologyException;
-import org.apache.ambari.server.topology.TopologyValidator;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-/**
- * Validates configuration types related to services specified in the blueprint.
- * If the cluster creation template contains configuration types that are not related to services in the blueprint the
- * validator fails interrupting the cluster provisioning.
- */
-public class ClusterConfigTypeValidator implements TopologyValidator {
-  private static final Logger LOGGER = LoggerFactory.getLogger(ClusterConfigTypeValidator.class);
-
-  @Override
-  public void validate(ClusterTopology topology) throws InvalidTopologyException {
-
-    if (topology.getConfiguration() == null) {
-      LOGGER.debug("No configuration is set into the topology");
-      return;
-    }
-
-    if (topology.getConfiguration().getProperties() == null) {
-      LOGGER.debug("No properties is set into the topology configuration");
-      return;
-    }
-
-    // config types in from the request
-    Set<String> clusterConfigTypes = topology.getConfiguration().getProperties().keySet();
-    LOGGER.debug("Cluster config types: {}", clusterConfigTypes);
-
-    if (clusterConfigTypes == null || clusterConfigTypes.isEmpty()) {
-      LOGGER.debug("No config types to be checked.");
-      return;
-    }
-
-    // collecting all config types for services in the blueprint (from the related stack)
-    Set<String> serviceConfigTypes = new HashSet<>();
-    for (String serviceName : topology.getBlueprint().getServices()) {
-      serviceConfigTypes.addAll(topology.getBlueprint().getStack().getConfigurationTypes(serviceName));
-    }
-
-    // identifying invalid config types
-    Set<String> configTypeIntersection = new HashSet<String>(serviceConfigTypes);
-
-    // if the intersection is changed, there's been some wrong config type provided in the cluster configuration
-    if (configTypeIntersection.retainAll(clusterConfigTypes)) {
-      LOGGER.debug("Valid config types: {}", configTypeIntersection);
-
-      // get the wrong  config types
-      Set<String> invalidConfigTypes = new HashSet<>(clusterConfigTypes);
-      invalidConfigTypes.removeAll(configTypeIntersection);
-
-      LOGGER.error("The following config typess are wrong: {}", invalidConfigTypes);
-      throw new InvalidTopologyException("The following configuration types are invalid: " + invalidConfigTypes.toString());
-    }
-  }
-}

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

@@ -49,9 +49,9 @@ import org.apache.ambari.server.topology.BlueprintFactory;
 import org.apache.ambari.server.topology.Configuration;
 import org.apache.ambari.server.topology.HostGroupInfo;
 import org.apache.ambari.server.topology.InvalidTopologyTemplateException;
+import org.apache.ambari.server.topology.RequiredPasswordValidator;
 import org.apache.ambari.server.topology.TopologyRequest;
 import org.apache.ambari.server.topology.TopologyValidator;
-import org.apache.ambari.server.topology.validators.RequiredPasswordValidator;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -112,7 +112,7 @@ public class ProvisionClusterRequestTest {
     assertSame(blueprint, provisionClusterRequest.getBlueprint());
     Map<String, HostGroupInfo> hostGroupInfo = provisionClusterRequest.getHostGroupInfo();
     assertEquals(1, hostGroupInfo.size());
-    assertEquals(2, provisionClusterRequest.getTopologyValidators().size());
+    assertEquals(1, provisionClusterRequest.getTopologyValidators().size());
 
     // group1
     // host info
@@ -164,7 +164,7 @@ public class ProvisionClusterRequestTest {
     assertSame(blueprint, provisionClusterRequest.getBlueprint());
     Map<String, HostGroupInfo> hostGroupInfo = provisionClusterRequest.getHostGroupInfo();
     assertEquals(1, hostGroupInfo.size());
-    assertEquals(2, provisionClusterRequest.getTopologyValidators().size());
+    assertEquals(1, provisionClusterRequest.getTopologyValidators().size());
 
     // group2
     HostGroupInfo group2Info = hostGroupInfo.get("group2");
@@ -216,7 +216,7 @@ public class ProvisionClusterRequestTest {
     assertSame(blueprint, provisionClusterRequest.getBlueprint());
     Map<String, HostGroupInfo> hostGroupInfo = provisionClusterRequest.getHostGroupInfo();
     assertEquals(2, hostGroupInfo.size());
-    assertEquals(2, provisionClusterRequest.getTopologyValidators().size());
+    assertEquals(1, provisionClusterRequest.getTopologyValidators().size());
 
     // group1
     // host info
@@ -374,7 +374,7 @@ public class ProvisionClusterRequestTest {
     TopologyRequest request = new ProvisionClusterRequest(properties, null);
     List<TopologyValidator> validators = request.getTopologyValidators();
 
-    assertEquals(2, validators.size());
+    assertEquals(1, validators.size());
     TopologyValidator pwdValidator = validators.get(0);
 
     TopologyValidator noDefaultPwdValidator = new RequiredPasswordValidator(null);
@@ -388,7 +388,7 @@ public class ProvisionClusterRequestTest {
     TopologyRequest request = new ProvisionClusterRequest(properties, null);
     List<TopologyValidator> validators = request.getTopologyValidators();
 
-    assertEquals(2, validators.size());
+    assertEquals(1, validators.size());
     TopologyValidator pwdValidator = validators.get(0);
 
     TopologyValidator defaultPwdValidator = new RequiredPasswordValidator("pwd");

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

@@ -18,12 +18,11 @@
 
 package org.apache.ambari.server.topology;
 
-import static junit.framework.Assert.assertEquals;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.reset;
-import static org.powermock.api.easymock.PowerMock.createNiceMock;
-import static org.powermock.api.easymock.PowerMock.verify;
+import org.apache.ambari.server.controller.internal.Stack;
+import org.apache.ambari.server.state.PropertyInfo;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.Collection;
@@ -32,12 +31,12 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 
-import org.apache.ambari.server.controller.internal.Stack;
-import org.apache.ambari.server.state.PropertyInfo;
-import org.apache.ambari.server.topology.validators.RequiredPasswordValidator;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
+import static junit.framework.Assert.assertEquals;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reset;
+import static org.powermock.api.easymock.PowerMock.createNiceMock;
+import static org.powermock.api.easymock.PowerMock.verify;
 
 /**
  * Unit tests for RequiredPasswordValidator.

ambari-server/src/test/java/org/apache/ambari/server/topology/validators/ClusterConfigTypeValidatorTest.java

@@ -1,110 +0,0 @@
-package org.apache.ambari.server.topology.validators;
-
-/*
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Map;
-
-import org.apache.ambari.server.controller.internal.Stack;
-import org.apache.ambari.server.topology.Blueprint;
-import org.apache.ambari.server.topology.ClusterTopology;
-import org.apache.ambari.server.topology.Configuration;
-import org.apache.ambari.server.topology.InvalidTopologyException;
-import org.easymock.EasyMock;
-import org.easymock.EasyMockRule;
-import org.easymock.EasyMockSupport;
-import org.easymock.Mock;
-import org.easymock.TestSubject;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-
-import nl.jqno.equalsverifier.EqualsVerifier;
-
-public class ClusterConfigTypeValidatorTest extends EasyMockSupport {
-
-  @Rule
-  public EasyMockRule mocks = new EasyMockRule(this);
-
-
-  @Mock
-  private Configuration clusterConfigurationMock;
-
-  @Mock
-  private Map<String, Map<String, String>> clusterConfigurationMapMock;
-
-  @Mock
-  private Blueprint blueprintMock;
-
-  @Mock
-  private Stack stackMock;
-
-  @Mock
-  private ClusterTopology clusterTopologyMock;
-
-  @TestSubject
-  private ClusterConfigTypeValidator clusterConfigTypeValidator = new ClusterConfigTypeValidator();
-
-  @Before
-  public void before() {
-    EasyMock.expect(clusterTopologyMock.getConfiguration()).andReturn(clusterConfigurationMock).anyTimes();
-    EasyMock.expect(clusterConfigurationMock.getProperties()).andReturn(clusterConfigurationMapMock).anyTimes();
-
-    EasyMock.expect(clusterTopologyMock.getBlueprint()).andReturn(blueprintMock).anyTimes();
-    EasyMock.expect(blueprintMock.getStack()).andReturn(stackMock).anyTimes();
-  }
-
-  @After
-  public void after() {
-    resetAll();
-  }
-
-  @Test(expected = InvalidTopologyException.class)
-  public void testShouldValidationFailWhenInvalidConfigGroupSpecifiedInCCTemplate() throws Exception {
-    // given
-    EasyMock.expect(clusterConfigurationMapMock.keySet()).andReturn(new HashSet<String>(Arrays.asList("oozie-site")));
-    EasyMock.expect(blueprintMock.getServices()).andReturn(new HashSet<String>(Arrays.asList("YARN", "HDFS")));
-
-    EasyMock.expect(stackMock.getConfigurationTypes("HDFS")).andReturn(Arrays.asList("core-site"));
-    EasyMock.expect(stackMock.getConfigurationTypes("YARN")).andReturn(Arrays.asList("yarn-site"));
-
-    replayAll();
-
-    //when
-    clusterConfigTypeValidator.validate(clusterTopologyMock);
-
-    // then
-    // Exception is thrown
-  }
-
-  @Test
-  public void testShouldValidationPassIfNoConfigTypesSpecifiedInCCTemplate() throws Exception {
-    //GIVEN
-    EasyMock.expect(clusterConfigurationMapMock.keySet()).andReturn(Collections.<String>emptySet());
-    replayAll();
-
-    //WHEN
-    clusterConfigTypeValidator.validate(clusterTopologyMock);
-
-  }
-
-  @Test
-  public void testEquals() throws Exception {
-    EqualsVerifier.forClass(ClusterConfigTypeValidator.class).usingGetClass().verify();
-  }
-}