You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by be...@apache.org on 2018/11/15 21:15:16 UTC
[ambari] branch trunk updated: [Ambari-24906] JSON validation for
AddServiceRequest (#2617)
This is an automated email from the ASF dual-hosted git repository.
benyoka pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new 6f7b0a9 [Ambari-24906] JSON validation for AddServiceRequest (#2617)
6f7b0a9 is described below
commit 6f7b0a9542a3f4c013faa90a4303a31238cc5c78
Author: benyoka <be...@users.noreply.github.com>
AuthorDate: Thu Nov 15 22:15:12 2018 +0100
[Ambari-24906] JSON validation for AddServiceRequest (#2617)
* AMBARI-24906 validate configs in JSON files (benyoka)
* AMBARI-2490 fix test and licence issue (benyoka)
* AMBARI-2490 fix missing files (benyoka)
* AMBARI-24906 review comments (benyoka)
---
.../server/controller/AddServiceRequest.java | 5 +-
.../ambari/server/topology/Configurable.java | 93 +---------
.../{Configurable.java => ConfigurableHelper.java} | 105 +++++++----
.../server/topology/ConfigurationFactory.java | 36 +++-
.../server/controller/AddServiceRequestTest.java | 8 +-
.../ambari/server/topology/ConfigurableTest.java | 66 +++++--
.../add_service_api/invalid_configurables.txt | 193 +++++++++++++++++++++
.../add_service_api/request_invalid_3.json | 42 +++++
8 files changed, 398 insertions(+), 150 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java
index 22a32ce..ccf2ecf 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AddServiceRequest.java
@@ -37,6 +37,7 @@ import org.apache.ambari.annotations.ApiIgnore;
import org.apache.ambari.server.controller.internal.ProvisionAction;
import org.apache.ambari.server.topology.ConfigRecommendationStrategy;
import org.apache.ambari.server.topology.Configurable;
+import org.apache.ambari.server.topology.ConfigurableHelper;
import org.apache.ambari.server.topology.Configuration;
import com.fasterxml.jackson.annotation.JsonCreator;
@@ -85,7 +86,7 @@ public final class AddServiceRequest {
@JsonProperty(COMPONENTS)Set<Component> components,
@JsonProperty(CONFIGURATIONS) Collection<? extends Map<String, ?>> configs) {
this(operationType, recommendationStrategy, provisionAction, stackName, stackVersion, services, components,
- Configurable.parseConfigs(configs));
+ ConfigurableHelper.parseConfigs(configs));
}
@@ -169,7 +170,7 @@ public final class AddServiceRequest {
@JsonProperty(CONFIGURATIONS)
@ApiModelProperty(name = CONFIGURATIONS)
public Collection<Map<String, Map<String, ?>>> getConfigurationContents() {
- return Configurable.convertConfigToMap(configuration);
+ return ConfigurableHelper.convertConfigToMap(configuration);
}
// ------- inner classes -------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java
index e8f780d..eb356a1 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java
@@ -18,23 +18,13 @@
package org.apache.ambari.server.topology;
-import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.PROPERTIES_ATTRIBUTES_PROPERTY_ID;
-import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.PROPERTIES_PROPERTY_ID;
-
-import java.util.ArrayList;
import java.util.Collection;
-import java.util.HashMap;
import java.util.Map;
-import java.util.Set;
-
-import javax.annotation.Nullable;
import org.apache.ambari.annotations.ApiIgnore;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Sets;
import io.swagger.annotations.ApiModelProperty;
@@ -45,7 +35,7 @@ import io.swagger.annotations.ApiModelProperty;
*/
public interface Configurable {
- public static final String CONFIGURATIONS = "configurations";
+ String CONFIGURATIONS = "configurations";
@JsonIgnore
@ApiIgnore
@@ -58,90 +48,13 @@ public interface Configurable {
@JsonProperty(CONFIGURATIONS)
@ApiModelProperty(name = CONFIGURATIONS)
default void setConfigs(Collection<? extends Map<String, ?>> configs) {
- setConfiguration(parseConfigs(configs));
+ setConfiguration(ConfigurableHelper.parseConfigs(configs));
}
@JsonProperty(CONFIGURATIONS)
@ApiModelProperty(name = CONFIGURATIONS)
default Collection<Map<String, Map<String, ?>>> getConfigs() {
- return convertConfigToMap(getConfiguration());
- }
-
- /**
- * Parses configuration maps The configs can be in fully structured JSON, e.g.
- * <code>
- * [{"hdfs-site":
- * "properties": {
- * ""dfs.replication": "3",
- * ...
- * },
- * properties_attributes: {}
- * }]
- * </code>
- * or flattened like
- * <code>
- * [{
- * "hdfs-site/properties/dfs.replication": "3",
- * ...
- * }]
- * </code>
- * In the latter case it calls {@link ConfigurationFactory#getConfiguration(Collection)}
- * @param configs
- * @return
- */
- static Configuration parseConfigs(@Nullable Collection<? extends Map<String, ?>> configs) {
- Configuration configuration;
-
- if (null == configs) {
- configuration = new Configuration(new HashMap<>(), new HashMap<>());
- }
- else if (!configs.isEmpty() && configs.iterator().next().keySet().iterator().next().contains("/")) {
- // Configuration has keys with slashes like "zk.cfg/properties/dataDir" means it is coming through
- // the resource framework and must be parsed with configuration factories
- configuration = new ConfigurationFactory().getConfiguration((Collection<Map<String, String>>)configs);
- }
- else {
- // If the configuration does not have keys with slashes it means it is coming from plain JSON and needs to be
- // parsed accordingly.
- Map<String, Map<String, String>> allProperties = new HashMap<>();
- Map<String, Map<String, Map<String, String>>> allAttributes = new HashMap<>();
- configs.forEach( item -> {
- String configName = item.keySet().iterator().next();
- Map<String, Object> configData = (Map<String, Object>) item.get(configName);
- if (configData.containsKey(PROPERTIES_PROPERTY_ID)) {
- Map<String, String> properties = (Map<String, String>)configData.get(PROPERTIES_PROPERTY_ID);
- allProperties.put(configName, properties);
- }
- if (configData.containsKey(PROPERTIES_ATTRIBUTES_PROPERTY_ID)) {
- Map<String, Map<String, String>> attributes =
- (Map<String, Map<String, String>>)configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID);
- allAttributes.put(configName, attributes);
- }
- });
- configuration = new Configuration(allProperties, allAttributes);
- }
- return configuration;
- }
-
- /**
- * Converts {@link Configuration} objects to a collection easily serializable to Json
- * @param configuration the configuration to convert
- * @return the resulting collection
- */
- static Collection<Map<String, Map<String, ?>>> convertConfigToMap(Configuration configuration) {
- Collection<Map<String, Map<String, ?>>> configurations = new ArrayList<>();
- Set<String> allConfigTypes = Sets.union(configuration.getProperties().keySet(), configuration.getAttributes().keySet());
- for (String configType: allConfigTypes) {
- Map<String, Map<String, ? extends Object>> configData = new HashMap<>();
- if (configuration.getProperties().containsKey(configType)) {
- configData.put(PROPERTIES_PROPERTY_ID, configuration.getProperties().get(configType));
- }
- if (configuration.getAttributes().containsKey(configType)) {
- configData.put(PROPERTIES_ATTRIBUTES_PROPERTY_ID, configuration.getAttributes().get(configType));
- }
- configurations.add(ImmutableMap.of(configType, configData));
- }
- return configurations;
+ return ConfigurableHelper.convertConfigToMap(getConfiguration());
}
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java
similarity index 54%
copy from ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java
copy to ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java
index e8f780d..53cef2f 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/Configurable.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurableHelper.java
@@ -18,54 +18,33 @@
package org.apache.ambari.server.topology;
+import static com.google.common.base.Preconditions.checkArgument;
import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.PROPERTIES_ATTRIBUTES_PROPERTY_ID;
import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.PROPERTIES_PROPERTY_ID;
+import static org.apache.ambari.server.topology.ConfigurationFactory.isKeyInLegacyFormat;
+import static org.apache.ambari.server.topology.ConfigurationFactory.isKeyInNewFormat;
+import static org.apache.ambari.server.topology.ConfigurationFactory.splitConfigurationKey;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import javax.annotation.Nullable;
-import org.apache.ambari.annotations.ApiIgnore;
-
-import com.fasterxml.jackson.annotation.JsonIgnore;
-import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
-import io.swagger.annotations.ApiModelProperty;
-
/**
- * Provides support for JSON serializaion of {@link Configuration} objects. Can handle both plain JSON and Ambari style
- * flattened JSON such as {@code "hdfs-site/properties/dfs.replication": "3"}. Objects may implement this interface or
- * call its static utility methods.
+ * Helper class for {@link Configurable}
*/
-public interface Configurable {
-
- public static final String CONFIGURATIONS = "configurations";
-
- @JsonIgnore
- @ApiIgnore
- void setConfiguration(Configuration configuration);
-
- @JsonIgnore
- @ApiIgnore
- Configuration getConfiguration();
-
- @JsonProperty(CONFIGURATIONS)
- @ApiModelProperty(name = CONFIGURATIONS)
- default void setConfigs(Collection<? extends Map<String, ?>> configs) {
- setConfiguration(parseConfigs(configs));
- }
+public class ConfigurableHelper {
- @JsonProperty(CONFIGURATIONS)
- @ApiModelProperty(name = CONFIGURATIONS)
- default Collection<Map<String, Map<String, ?>>> getConfigs() {
- return convertConfigToMap(getConfiguration());
- }
+ static final ImmutableSet<String> PERMITTED_CONFIG_FIELDS = ImmutableSet.of(PROPERTIES_PROPERTY_ID, PROPERTIES_ATTRIBUTES_PROPERTY_ID);
/**
* Parses configuration maps The configs can be in fully structured JSON, e.g.
@@ -89,15 +68,16 @@ public interface Configurable {
* @param configs
* @return
*/
- static Configuration parseConfigs(@Nullable Collection<? extends Map<String, ?>> configs) {
+ public static Configuration parseConfigs(@Nullable Collection<? extends Map<String, ?>> configs) {
Configuration configuration;
- if (null == configs) {
- configuration = new Configuration(new HashMap<>(), new HashMap<>());
+ if (null == configs || configs.isEmpty()) {
+ configuration = Configuration.newEmpty();
}
else if (!configs.isEmpty() && configs.iterator().next().keySet().iterator().next().contains("/")) {
// Configuration has keys with slashes like "zk.cfg/properties/dataDir" means it is coming through
// the resource framework and must be parsed with configuration factories
+ checkFlattenedConfig(configs);
configuration = new ConfigurationFactory().getConfiguration((Collection<Map<String, String>>)configs);
}
else {
@@ -106,18 +86,37 @@ public interface Configurable {
Map<String, Map<String, String>> allProperties = new HashMap<>();
Map<String, Map<String, Map<String, String>>> allAttributes = new HashMap<>();
configs.forEach( item -> {
+
+ checkArgument(item.size() == 1, "Each config object must have a single property which is the name of the config," +
+ " e.g. \"cluster-env\" : {...}");
+
+ Map.Entry<String, ?> configEntry = item.entrySet().iterator().next();
String configName = item.keySet().iterator().next();
- Map<String, Object> configData = (Map<String, Object>) item.get(configName);
+
+ checkArgument(configEntry.getValue() instanceof Map,
+ "The value for %s must be a JSON object (found: %s)", configName, getClassName(configEntry.getValue()));
+
+ Map<String, Object> configData = (Map<String, Object>) configEntry.getValue();
+
+ Set<String> extraKeys = Sets.difference(configData.keySet(), PERMITTED_CONFIG_FIELDS);
+ checkArgument(extraKeys.isEmpty(), "Invalid fields in %s configuration: %s", configName, extraKeys);
+
if (configData.containsKey(PROPERTIES_PROPERTY_ID)) {
+ checkMap(PROPERTIES_PROPERTY_ID, configData.get(PROPERTIES_PROPERTY_ID), String.class);
+
Map<String, String> properties = (Map<String, String>)configData.get(PROPERTIES_PROPERTY_ID);
allProperties.put(configName, properties);
}
if (configData.containsKey(PROPERTIES_ATTRIBUTES_PROPERTY_ID)) {
+ checkMap(PROPERTIES_ATTRIBUTES_PROPERTY_ID, configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID), Map.class);
Map<String, Map<String, String>> attributes =
(Map<String, Map<String, String>>)configData.get(PROPERTIES_ATTRIBUTES_PROPERTY_ID);
+ attributes.entrySet().forEach( entry -> checkMap(entry.getKey(), entry.getValue(), String.class));
+
allAttributes.put(configName, attributes);
}
});
+
configuration = new Configuration(allProperties, allAttributes);
}
return configuration;
@@ -128,7 +127,7 @@ public interface Configurable {
* @param configuration the configuration to convert
* @return the resulting collection
*/
- static Collection<Map<String, Map<String, ?>>> convertConfigToMap(Configuration configuration) {
+ public static Collection<Map<String, Map<String, ?>>> convertConfigToMap(Configuration configuration) {
Collection<Map<String, Map<String, ?>>> configurations = new ArrayList<>();
Set<String> allConfigTypes = Sets.union(configuration.getProperties().keySet(), configuration.getAttributes().keySet());
for (String configType: allConfigTypes) {
@@ -144,4 +143,38 @@ public interface Configurable {
return configurations;
}
+ private static void checkFlattenedConfig(Collection<? extends Map<String, ?>> configs) {
+ configs.forEach( config -> {
+ if ( !config.isEmpty() ) {
+ List<String> firstKey = splitConfigurationKey(config.keySet().iterator().next());
+ String configType = firstKey.get(0);
+ boolean legacyConfig = isKeyInLegacyFormat(firstKey);
+
+ config.keySet().forEach( key -> {
+ List<String> keyParts = splitConfigurationKey(key);
+ checkArgument(Objects.equals(configType, keyParts.get(0)),
+ "Invalid config type: %s. Should be: %s", keyParts.get(0), configType);
+ checkArgument(legacyConfig && isKeyInLegacyFormat(keyParts) || !legacyConfig && isKeyInNewFormat(keyParts),
+ "Expected key in %s format, found [%s]",
+ legacyConfig ? "[config_type/property_name]" :
+ "[config_type/properties/config_name] or [config_type/properties_attributes/attribute_name/property_name]",
+ key);
+ });
+ }
+ });
+ }
+
+ private static void checkMap(String fieldName, Object mapObj, Class<?> valueType) {
+ checkArgument(mapObj instanceof Map, "'%s' needs to be a JSON object. Found: %s", fieldName, getClassName(mapObj));
+ Map<?, ?> map = (Map<?, ?>)mapObj;
+ map.forEach( (__, value) -> {
+ checkArgument(valueType.isInstance(value), "Expected %s as value type, found %s, type: %s",
+ valueType.getName(), value, getClassName(value));
+ });
+ }
+
+ private static String getClassName(Object object) {
+ return null != object ? object.getClass().getName() : null;
+ }
+
}
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurationFactory.java b/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurationFactory.java
index 3a58592..a66f1b7 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurationFactory.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigurationFactory.java
@@ -18,8 +18,13 @@
package org.apache.ambari.server.topology;
+import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.PROPERTIES_ATTRIBUTES_PROPERTY_ID;
+import static org.apache.ambari.server.controller.internal.BlueprintResourceProvider.PROPERTIES_PROPERTY_ID;
+
+import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
/**
@@ -53,22 +58,37 @@ public class ConfigurationFactory {
private ConfigurationStrategy decidePopulationStrategy(Map<String, String> configuration) {
if (configuration != null && !configuration.isEmpty()) {
String keyEntry = configuration.keySet().iterator().next();
- String[] keyNameTokens = keyEntry.split("/");
- int levels = keyNameTokens.length;
- String propertiesType = keyNameTokens[1];
- if (levels == 2) {
+ List<String> keyNameTokens = splitConfigurationKey(keyEntry);
+
+ if (isKeyInLegacyFormat(keyNameTokens)) {
return new ConfigurationStrategyV1();
- } else if ((levels == 3 && BlueprintFactory.PROPERTIES_PROPERTY_ID.equals(propertiesType))
- || (levels == 4 && BlueprintFactory.PROPERTIES_ATTRIBUTES_PROPERTY_ID.equals(propertiesType))) {
+ }
+ else if (isKeyInNewFormat(keyNameTokens)) {
return new ConfigurationStrategyV2();
- } else {
+ }
+ else {
throw new IllegalArgumentException(SCHEMA_IS_NOT_SUPPORTED_MESSAGE);
}
- } else {
+ }
+ else {
return new ConfigurationStrategyV2();
}
}
+ static List<String> splitConfigurationKey(String configurationKey) {
+ return Arrays.asList(configurationKey.split("/"));
+ }
+
+ static boolean isKeyInLegacyFormat(List<String> configurationKey) {
+ return configurationKey.size() == 2;
+ }
+
+ static boolean isKeyInNewFormat(List<String> configurationKey) {
+ String propertiesType = configurationKey.get(1);
+ return configurationKey.size() == 3 && PROPERTIES_PROPERTY_ID.equals(propertiesType)
+ || configurationKey.size() == 4 && PROPERTIES_ATTRIBUTES_PROPERTY_ID.equals(propertiesType);
+ }
+
/**
* The structure of blueprints is evolving where multiple resource
* structures are to be supported. This class abstracts the population
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java
index cb8d03b..ed1a865 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AddServiceRequestTest.java
@@ -69,7 +69,7 @@ public class AddServiceRequestTest {
private static String REQUEST_MINIMAL_COMPONENTS_ONLY;
private static String REQUEST_INVALID_NO_SERVICES_AND_COMPONENTS;
private static String REQUEST_INVALID_INVALID_FIELD;
-
+ private static String REQUEST_INVALID_INVALID_CONFIG;
private ObjectMapper mapper = new ObjectMapper();
@@ -81,6 +81,7 @@ public class AddServiceRequestTest {
REQUEST_MINIMAL_COMPONENTS_ONLY = read("add_service_api/request4.json");
REQUEST_INVALID_NO_SERVICES_AND_COMPONENTS = read("add_service_api/request_invalid_1.json");
REQUEST_INVALID_INVALID_FIELD = read("add_service_api/request_invalid_2.json");
+ REQUEST_INVALID_INVALID_CONFIG = read("add_service_api/request_invalid_3.json");
}
@Test
@@ -192,6 +193,11 @@ public class AddServiceRequestTest {
mapper.readValue(REQUEST_INVALID_INVALID_FIELD, AddServiceRequest.class);
}
+ @Test(expected = JsonProcessingException.class)
+ public void testDeserialize_invalid_invalidConfiguration() throws Exception {
+ mapper.readValue(REQUEST_INVALID_INVALID_CONFIG, AddServiceRequest.class);
+ }
+
@Test
public void testSerialize_basic() throws Exception {
AddServiceRequest request = mapper.readValue(REQUEST_ALL_FIELDS_SET, AddServiceRequest.class);
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java b/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java
index 69aec1f..4bbc0c3 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/topology/ConfigurableTest.java
@@ -19,28 +19,41 @@
package org.apache.ambari.server.topology;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.net.URL;
+import java.nio.charset.StandardCharsets;
+import java.util.List;
+import java.util.regex.Pattern;
+import org.apache.commons.lang3.exception.ExceptionUtils;
import org.junit.Before;
import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableMap;
import com.google.common.io.Resources;
public class ConfigurableTest {
public static final String JSON_LOCATION = "add_service_api/configurable.json";
public static final String JSON_LOCATION2 = "add_service_api/configurable2.json";
+ public static final String INVALID_CONFIGS_LOCATION = "add_service_api/invalid_configurables.txt";
private TestConfigurable configurable;
private ObjectMapper mapper;
+ Logger LOG = LoggerFactory.getLogger(ConfigurableTest.class);
+
@Before
public void setUp() throws Exception {
mapper = new ObjectMapper();
URL url = Resources.getResource(JSON_LOCATION);
- configurable = new ObjectMapper().readValue(url, TestConfigurable.class);
+ configurable = mapper.readValue(url, TestConfigurable.class);
}
/**
@@ -57,6 +70,34 @@ public class ConfigurableTest {
configurable.getConfiguration().getAttributes());
}
+ @Test
+ public void parseInvalidConfigurables() throws Exception {
+ String invalidConfigsTxt = Resources.toString(Resources.getResource(INVALID_CONFIGS_LOCATION), StandardCharsets.UTF_8);
+ List<String> invalidConfigs = Splitter
+ // filter block comment (Apache license) and use line comments as separators between json snippets
+ .on(Pattern.compile("'''(.|[\\r\\n])*?'''|#.*$", Pattern.MULTILINE))
+ .omitEmptyStrings()
+ .trimResults()
+ .splitToList(invalidConfigsTxt);
+
+ for (String config: invalidConfigs) {
+ LOG.info("Invalid config to parse:\n{}", config);
+ try {
+ mapper.readValue(config, TestConfigurable.class);
+ fail("Expected " + JsonProcessingException.class.getSimpleName() + " while processing config:\n" + config);
+ }
+ catch (JsonProcessingException ex) {
+ Throwable rootCause = ExceptionUtils.getRootCause(ex);
+ LOG.info("Error message: {}", rootCause.getMessage());
+ assertTrue(
+ "Expected " + IllegalArgumentException.class.getSimpleName() + " during parsing JSON:\n" + config +
+ "\n found: " + rootCause,
+ rootCause instanceof IllegalArgumentException);
+ }
+ }
+
+ }
+
/**
* Deserialize normal JSON configuration
*/
@@ -86,19 +127,18 @@ public class ConfigurableTest {
configurable.getConfiguration().getAttributes());
}
-}
+ static class TestConfigurable implements Configurable {
+ Configuration configuration;
-class TestConfigurable implements Configurable {
- Configuration configuration;
+ @Override
+ public Configuration getConfiguration() {
+ return configuration;
+ }
- @Override
- public Configuration getConfiguration() {
- return configuration;
- }
+ @Override
+ public void setConfiguration(Configuration configuration) {
+ this.configuration = configuration;
+ }
- @Override
- public void setConfiguration(Configuration configuration) {
- this.configuration = configuration;
}
-
-}
\ No newline at end of file
+}
diff --git a/ambari-server/src/test/resources/add_service_api/invalid_configurables.txt b/ambari-server/src/test/resources/add_service_api/invalid_configurables.txt
new file mode 100644
index 0000000..9c25b3e
--- /dev/null
+++ b/ambari-server/src/test/resources/add_service_api/invalid_configurables.txt
@@ -0,0 +1,193 @@
+'''
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you 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.
+'''
+
+# Configs in normal JSON format
+
+# Bad structure
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : {
+ "dataDir" : "/zookeeper1"
+ }
+ },
+ "zookeeper-env" : {
+ "properties" : {
+ "zk_user" : "zookeeper"
+ }
+ }
+ }
+ ]
+}
+
+# Non-string property in properties_attributes
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : {
+ "dataDir" : "/zookeeper1"
+ },
+ "properties_attributes": {
+ "final": {
+ "someProp": "true",
+ "non-string-prop": 5
+ }
+ }
+ }
+ }
+ ]
+}
+
+# Non-string property in properties
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : {
+ "dataDir" : "/zookeeper1",
+ "non-string-prop": 5
+ },
+ "properties_attributes": {
+ "final": {
+ "someProp": "true"
+ }
+ }
+ }
+ }
+ ]
+}
+
+# Ivalid field: features
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : {
+ "dataDir" : "/zookeeper1"
+ },
+ "properties_attributes": {
+ "final": {
+ "someProp": "true"
+ }
+ },
+ "features": {}
+ }
+ }
+ ]
+}
+
+# Invalid properties (string instead of JSON object)
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : "should be a json object",
+ "properties_attributes": {
+ "final": {
+ "someProp": "true"
+ }
+ }
+ }
+ }
+ ]
+}
+
+# Invalid attributes 1 (string instead of JSON object)
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : {
+ "dataDir" : "/zookeeper1"
+ },
+ "properties_attributes": "should be a json object"
+ }
+ }
+ ]
+}
+
+# Invalid attributes 2 (string instead of JSON object)
+{
+ "configurations" : [
+ {
+ "zoo.cfg" : {
+ "properties" : {
+ "dataDir" : "/zookeeper1"
+ },
+ "properties_attributes": {
+ "final": "should be a json object"
+ }
+ }
+ }
+ ]
+}
+
+# Flattened JSON configs (gone through ResourceManager)
+
+# Invalid field (features): zoo.cfg/features/dataDir
+{
+ "configurations" : [
+ {
+ "zoo.cfg/properties/dataDir" : "/zookeeper1",
+ "zoo.cfg/properties_attributes/final/someProp": "true",
+ "zoo.cfg/features/dataDir" : "/zookeeper1"
+ }
+ ]
+}
+
+# Invalid field (path too long): zoo.cfg/properties/dataDir/value
+{
+ "configurations" : [
+ {
+ "zoo.cfg/properties/dataDir/value" : "/zookeeper1",
+ "zoo.cfg/properties_attributes/final/someProp": "true"
+ }
+ ]
+}
+
+# Invalid field (path too short): zoo.cfg/properties
+{
+ "configurations" : [
+ {
+ "zoo.cfg/properties" : "/zookeeper1",
+ "zoo.cfg/properties_attributes/final/someProp": "true"
+ }
+ ]
+}
+
+# Invalid field (path too long): zoo.cfg/properties_attributes/final/someProp/value
+{
+ "configurations" : [
+ {
+ "zoo.cfg/properties/dataDir" : "/zookeeper1",
+ "zoo.cfg/properties_attributes/final/someProp/value": "true"
+ }
+ ]
+}
+
+# Invalid field (path too short): zoo.cfg/properties_attributes/final
+{
+ "configurations" : [
+ {
+ "zoo.cfg/properties/dataDir" : "/zookeeper1",
+ "zoo.cfg/properties_attributes/final": "true"
+ }
+ ]
+}
\ No newline at end of file
diff --git a/ambari-server/src/test/resources/add_service_api/request_invalid_3.json b/ambari-server/src/test/resources/add_service_api/request_invalid_3.json
new file mode 100644
index 0000000..6199f5e
--- /dev/null
+++ b/ambari-server/src/test/resources/add_service_api/request_invalid_3.json
@@ -0,0 +1,42 @@
+{
+ "operation_type" : "ADD_SERVICE",
+ "config_recommendation_strategy" : "ALWAYS_APPLY",
+ "provision_action" : "INSTALL_ONLY",
+ "stack_name" : "HDP",
+ "stack_version" : "3.0",
+
+ "services": [
+ { "name" : "STORM" },
+ { "name" : "BEACON" }
+ ],
+
+ "components" : [
+ {
+ "component_name" : "NIMBUS",
+ "fqdn" : "c7401.ambari.apache.org"
+ },
+ {
+ "component_name" : "BEACON_SERVER",
+ "fqdn" : "c7402.ambari.apache.org"
+ }
+ ],
+
+ "configurations" : [
+ {
+ "storm-site" : {
+ "properties_attributes" : {
+ "final" : {
+ "fs.defaultFS" : "true"
+ }
+ },
+ "properties" : {
+ "ipc.client.connect.max.retries" : "50"
+ }
+ }
+ },
+ {
+ "foo-env" : "invalid"
+ }
+ ]
+
+}
\ No newline at end of file