diff --git a/sdk/scheduler/build.gradle b/sdk/scheduler/build.gradle index 3cb78438532..bc6d09b201c 100644 --- a/sdk/scheduler/build.gradle +++ b/sdk/scheduler/build.gradle @@ -111,7 +111,7 @@ configurations { ext { // Only include version numbers here if it'd be redundant to repeat them below: - jacksonVer = "2.6.7" + jacksonVer = "2.9.9" curatorVer = "4.0.1" httpClientVer = "4.5.2" jerseyVer = "2.23" @@ -125,7 +125,7 @@ dependencies { compile "com.fasterxml.jackson.datatype:jackson-datatype-json-org:${jacksonVer}" compile "com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:${jacksonVer}" compile "com.fasterxml.jackson.core:jackson-databind:${jacksonVer}" - compile 'com.hubspot.jackson:jackson-datatype-protobuf:0.9.9-preJackson2.7-proto3' + compile 'com.hubspot.jackson:jackson-datatype-protobuf:0.9.11-jackson2.9' compile 'com.googlecode.protobuf-java-format:protobuf-java-format:1.4' compile 'com.github.spullara.mustache.java:compiler:0.9.2' compile 'commons-codec:commons-codec:1.11' diff --git a/sdk/scheduler/src/main/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParser.java b/sdk/scheduler/src/main/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParser.java index b79acbcd10a..90d9a7f2439 100644 --- a/sdk/scheduler/src/main/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParser.java +++ b/sdk/scheduler/src/main/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParser.java @@ -3,6 +3,7 @@ import com.mesosphere.sdk.offer.LoggingUtils; import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.annotations.VisibleForTesting; import jersey.repackaged.com.google.common.collect.Lists; @@ -80,7 +81,7 @@ public static PlacementRule parse(String podName, String marathonConstraints) { return new AndRule(rowRules); } catch (IOException e) { - LOGGER.error("Failed to parse marathon constraints", podName, marathonConstraints); + LOGGER.error("Failed to parse marathon constraints [{}] for {}", marathonConstraints, podName); return new InvalidPlacementRule(marathonConstraints, e.getMessage()); } } @@ -126,6 +127,8 @@ private static PlacementRule parseRow( @VisibleForTesting static List> splitConstraints(String marathonConstraints) { ObjectMapper mapper = new ObjectMapper(); + // Always parse the string in full. Leftover trailing tokens result in incomplete (and may be invalid) rules. + mapper.enable(DeserializationFeature.FAIL_ON_TRAILING_TOKENS); // The marathon doc uses a format like: '[["a", "b", "c"], ["d", "e"]]' // Meanwhile the marathon web interface uses a format like: 'a:b:c,d:e' try { diff --git a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/DefaultPodSpec.java b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/DefaultPodSpec.java index 88cc9a99a4c..7c96bea1aab 100644 --- a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/DefaultPodSpec.java +++ b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/DefaultPodSpec.java @@ -575,13 +575,10 @@ public Builder seccompProfileName(String seccompProfileName) { } /** - * Returns a {@code DefaultPodSpec} built from the parameters previously set. - * * @return a {@code DefaultPodSpec} built with parameters of this {@code DefaultPodSpec.Builder} */ public DefaultPodSpec build() { - DefaultPodSpec defaultPodSpec = new DefaultPodSpec(this); - return defaultPodSpec; + return new DefaultPodSpec(this); } } } diff --git a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawNetwork.java b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawNetwork.java index cb8b38a7180..1567aeadc42 100644 --- a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawNetwork.java +++ b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawNetwork.java @@ -31,7 +31,7 @@ private RawNetwork( * Included so that we support empty network specifications (e.g. a network of {@code networks: dcos:}). */ @JsonCreator - private RawNetwork(String ignored) { + private RawNetwork() { this(Collections.emptyList(), Collections.emptyList(), ""); } diff --git a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawPod.java b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawPod.java index 4f7e376cf1c..745ee5dc45a 100644 --- a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawPod.java +++ b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawPod.java @@ -4,6 +4,8 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.Nulls; import org.apache.commons.collections.CollectionUtils; import java.util.Collection; @@ -22,7 +24,8 @@ public final class RawPod { private final String image; - private final WriteOnceLinkedHashMap networks; + @JsonSetter(contentNulls = Nulls.AS_EMPTY) + private WriteOnceLinkedHashMap networks; private final WriteOnceLinkedHashMap rlimits; @@ -56,7 +59,6 @@ private RawPod( @JsonProperty("placement") String placement, @JsonProperty("count") Integer count, @JsonProperty("image") String image, - @JsonProperty("networks") WriteOnceLinkedHashMap networks, @JsonProperty("rlimits") WriteOnceLinkedHashMap rlimits, @JsonProperty("uris") Collection uris, @JsonProperty("tasks") WriteOnceLinkedHashMap tasks, @@ -73,7 +75,6 @@ private RawPod( this.placement = placement; this.count = count; this.image = image; - this.networks = networks == null ? new WriteOnceLinkedHashMap<>() : networks; this.rlimits = rlimits == null ? new WriteOnceLinkedHashMap<>() : rlimits; this.uris = uris; this.tasks = tasks; diff --git a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawServiceSpec.java b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawServiceSpec.java index 59550e19791..28b65fd1ad0 100644 --- a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawServiceSpec.java +++ b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/RawServiceSpec.java @@ -29,6 +29,7 @@ public final class RawServiceSpec { static { // If the user provides duplicate fields (e.g. 'count' twice), throw an error instead of silently dropping data: YAML_MAPPER.enable(JsonParser.Feature.STRICT_DUPLICATE_DETECTION); + YAML_MAPPER.enable(JsonParser.Feature.ALLOW_YAML_COMMENTS); } private final String name; diff --git a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/YAMLToInternalMappers.java b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/YAMLToInternalMappers.java index 2f023a1fca2..3a811ea454b 100644 --- a/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/YAMLToInternalMappers.java +++ b/sdk/scheduler/src/main/java/com/mesosphere/sdk/specification/yaml/YAMLToInternalMappers.java @@ -300,9 +300,8 @@ private static PodSpec convertPod( WriteOnceLinkedHashMap rawNetworks = rawPod.getNetworks(); final Collection networks = new ArrayList<>(); if (MapUtils.isNotEmpty(rawNetworks)) { - networks.addAll(rawNetworks.entrySet().stream() - .map(rawNetworkEntry -> { - String networkName = rawNetworkEntry.getKey(); + networks.addAll(rawNetworks.keySet().stream() + .map(networkName -> { DcosConstants.warnIfUnsupportedNetwork(networkName); networkNames.add(networkName); RawNetwork rawNetwork = rawNetworks.get(networkName); diff --git a/sdk/scheduler/src/test/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParserTest.java b/sdk/scheduler/src/test/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParserTest.java index 3fae8bf41e3..2141751ffa6 100644 --- a/sdk/scheduler/src/test/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParserTest.java +++ b/sdk/scheduler/src/test/java/com/mesosphere/sdk/offer/evaluate/placement/MarathonConstraintParserTest.java @@ -2,7 +2,6 @@ import org.junit.Test; -import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -16,7 +15,7 @@ public class MarathonConstraintParserTest { static final String POD_NAME = "hello"; @Test - public void testSplitConstraints() throws IOException { + public void testSplitConstraints() { assertEquals(Arrays.asList(Arrays.asList("")), MarathonConstraintParser.splitConstraints("")); assertEquals(Arrays.asList(Arrays.asList("a")), @@ -41,7 +40,7 @@ public void testSplitConstraints() throws IOException { } @Test - public void testUniqueOperator() throws IOException { + public void testUniqueOperator() { // example from marathon documentation: String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['hostname', 'UNIQUE']]")).toString(); assertEquals("MaxPerHostnameRule{max=1, task-filter=RegexMatcher{pattern='hello-.*'}}", constraintStr); @@ -56,7 +55,7 @@ public void testUniqueOperator() throws IOException { } @Test - public void testClusterOperator() throws IOException { + public void testClusterOperator() { // example from marathon documentation: String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['rack-id', 'CLUSTER', 'rack-1']]")).toString(); assertEquals("AttributeRule{matcher=ExactMatcher{str='rack-id:rack-1'}}", constraintStr); @@ -71,7 +70,7 @@ public void testClusterOperator() throws IOException { } @Test - public void testGroupByOperator() throws IOException { + public void testGroupByOperator() { // example from marathon documentation: String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['rack-id', 'GROUP_BY']]")).toString(); assertEquals("RoundRobinByAttributeRule{attribute=rack-id, attribute-count=Optional.empty, task-filter=RegexMatcher{pattern='hello-.*'}}", constraintStr); @@ -101,7 +100,7 @@ public void testGroupByOperator() throws IOException { } @Test - public void testLikeOperator() throws IOException { + public void testLikeOperator() { // example from marathon documentation: String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['rack-id', 'LIKE', 'rack-[1-3]']]")).toString(); assertEquals("AttributeRule{matcher=RegexMatcher{pattern='rack-id:rack-[1-3]'}}", constraintStr); @@ -115,7 +114,7 @@ public void testLikeOperator() throws IOException { } @Test - public void testIsOperator() throws IOException { + public void testIsOperator() { String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['foo', 'IS', 'bar']]")).toString(); assertEquals("AttributeRule{matcher=ExactMatcher{str='foo:bar'}}", constraintStr); assertEquals(constraintStr, MarathonConstraintParser.parse(POD_NAME, unescape("['foo', 'IS', 'bar']")).toString()); @@ -138,7 +137,7 @@ public void testIsOperator() throws IOException { } @Test - public void testUnlikeOperator() throws IOException { + public void testUnlikeOperator() { // example from marathon documentation: String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['rack-id', 'UNLIKE', 'rack-[7-9]']]")).toString(); assertEquals("NotRule{rule=AttributeRule{matcher=RegexMatcher{pattern='rack-id:rack-[7-9]'}}}", constraintStr); @@ -152,7 +151,7 @@ public void testUnlikeOperator() throws IOException { } @Test - public void testMaxPerOperator() throws IOException { + public void testMaxPerOperator() { // example from marathon documentation: String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape("[['rack-id', 'MAX_PER', '2']]")).toString(); assertEquals("AndRule{rules=[AttributeRule{matcher=RegexMatcher{pattern='rack-id:.*'}}, " + @@ -167,7 +166,7 @@ public void testMaxPerOperator() throws IOException { } @Test - public void testManyOperators() throws IOException { + public void testManyOperators() { String constraintStr = MarathonConstraintParser.parse(POD_NAME, unescape( "[['hostname', 'UNIQUE'], " + "['rack-id', 'CLUSTER', 'rack-1'], " @@ -194,79 +193,90 @@ public void testManyOperators() throws IOException { } @Test - public void testEscapedCommaRegex() throws IOException { + public void testEscapedCommaRegex() { assertEquals("AttributeRule{matcher=RegexMatcher{pattern='rack-id:rack-{1,3}'}}", MarathonConstraintParser.parse(POD_NAME, "rack-id:LIKE:rack-{1\\,3}").toString()); } @Test - public void testEscapedColonRegex() throws IOException { + public void testEscapedColonRegex() { assertEquals("AttributeRule{matcher=RegexMatcher{pattern='rack-id:foo:bar:baz'}}", MarathonConstraintParser.parse(POD_NAME, "rack-id:LIKE:foo\\:bar\\:baz").toString()); } @Test - public void testEmptyConstraint() throws IOException { + public void testEmptyConstraint() { assertEquals("PassthroughRule{}", MarathonConstraintParser.parse(POD_NAME, "").toString()); } @Test - public void testEmptyArrayConstraint() throws IOException { + public void testEmptyArrayConstraint() { assertEquals("PassthroughRule{}", MarathonConstraintParser.parse(POD_NAME, "[]").toString()); } @Test - public void testOverEscapedConstraintIsInvalid() throws IOException { + public void testOverEscapedConstraintIsInvalid() { assertTrue("too many \\'s", isInvalidConstraints("[[\\\"hostname\\\",\\\"MAX_PER\\\",\\\"1\\\"]]")); } @Test - public void testBadListConstraint() throws IOException { + public void testBadListConstraint() { assertTrue("missing ']]'", isInvalidConstraints(unescape("[['rack-id', 'MAX_PER', '2'"))); } @Test - public void testBadFlatConstraint() throws IOException { + public void testBadFlatConstraint() { assertTrue("Missing last element", isInvalidConstraints("rack-id:MAX_PER:,")); } @Test - public void testBadParamGroupBy() throws IOException { + public void testBadParamGroupBy() { assertTrue("Expected integer", isInvalidConstraints("rack-id:GROUP_BY:foo")); } @Test - public void testBadParamMaxPer() throws IOException { + public void testBadParamMaxPer() { assertTrue("Expected integer", isInvalidConstraints("rack-id:MAX_PER:foo")); } @Test - public void testMissingParamCluster() throws IOException { + public void testExtraTokens() { + for (String constraint: Arrays.asList( + unescape("['hostname','UNIQUE'],[['hostname','LIKE','10.0.3.6']"), + unescape("['hostname','UNIQUE'],["), + unescape("[['hostname','UNIQUE'],['hostname','LIKE','10.0.3.6']"), + unescape("[['hostname','UNIQUE']],"))) { + assertTrue("Trailing tokens", isInvalidConstraints(constraint)); + } + } + + @Test + public void testMissingParamCluster() { assertTrue("Expected parameter", isInvalidConstraints("rack-id:CLUSTER")); } @Test - public void testMissingParamLike() throws IOException { + public void testMissingParamLike() { assertTrue("Expected parameter", isInvalidConstraints("rack-id:LIKE")); } @Test - public void testMissingParamUnlike() throws IOException { + public void testMissingParamUnlike() { assertTrue("Expected parameter", isInvalidConstraints("rack-id:UNLIKE")); } @Test - public void testMissingParamMaxPer() throws IOException { + public void testMissingParamMaxPer() { assertTrue("Expected parameter", isInvalidConstraints("rack-id:MAX_PER")); } @Test - public void testUnknownCommand() throws IOException { + public void testUnknownCommand() { assertTrue(isInvalidConstraints("rack-id:FOO:foo")); } @Test - public void testTooManyElemenents() throws IOException { + public void testTooManyElemenents() { assertTrue(isInvalidConstraints("rack-id:LIKE:foo:bar")); } @@ -310,7 +320,7 @@ private static String unescape(String s) { return s.replace('\'', '"'); } - private boolean isInvalidConstraints(String constraints) throws IOException { + private boolean isInvalidConstraints(String constraints) { return MarathonConstraintParser.parse(POD_NAME, constraints) instanceof InvalidPlacementRule; } } diff --git a/sdk/scheduler/src/test/resources/invalid-image-null.yml b/sdk/scheduler/src/test/resources/invalid-image-null.yml index 38d050f51f6..a2fc57c20c7 100644 --- a/sdk/scheduler/src/test/resources/invalid-image-null.yml +++ b/sdk/scheduler/src/test/resources/invalid-image-null.yml @@ -2,7 +2,7 @@ name: "invalid-image-null-test" pods: server: count: 1 - image: + image: "" tasks: server: goal: RUNNING