diff --git a/src/main/scala/mesosphere/marathon/api/v2/validation/PodsValidation.scala b/src/main/scala/mesosphere/marathon/api/v2/validation/PodsValidation.scala index 70acb61ed14..a13405522d9 100644 --- a/src/main/scala/mesosphere/marathon/api/v2/validation/PodsValidation.scala +++ b/src/main/scala/mesosphere/marathon/api/v2/validation/PodsValidation.scala @@ -294,7 +294,7 @@ trait PodsValidation { } def podDefValidator(enabledFeatures: Set[String], mesosMasterVersion: SemanticVersion): Validator[Pod] = validator[Pod] { pod => - PathId(pod.id) as "id" is valid and valid(PathId.absolutePathValidator) + PathId(pod.id) as "id" is valid and PathId.absolutePathValidator and PathId.nonEmptyPath pod.user is optional(notEmpty) pod.environment is envValidator(pod, enabledFeatures) pod.volumes is every(volumeValidator(pod.containers)) and isTrue("volume names are unique") { volumes: Seq[Volume] => diff --git a/src/main/scala/mesosphere/marathon/state/AppDefinition.scala b/src/main/scala/mesosphere/marathon/state/AppDefinition.scala index d84471ca7fa..e96b903f5fc 100644 --- a/src/main/scala/mesosphere/marathon/state/AppDefinition.scala +++ b/src/main/scala/mesosphere/marathon/state/AppDefinition.scala @@ -456,8 +456,7 @@ object AppDefinition extends GeneralPurposeCombinators { def validAppDefinition( enabledFeatures: Set[String])(implicit pluginManager: PluginManager): Validator[AppDefinition] = validator[AppDefinition] { app => - app.id is valid - app.id is PathId.absolutePathValidator + app.id is valid and PathId.absolutePathValidator and PathId.nonEmptyPath app.dependencies is every(PathId.validPathWithBase(app.id.parent)) } and validBasicAppDefinition(enabledFeatures) and pluginValidators diff --git a/src/main/scala/mesosphere/marathon/state/PathId.scala b/src/main/scala/mesosphere/marathon/state/PathId.scala index 5b23b05a8f9..4ada6796716 100644 --- a/src/main/scala/mesosphere/marathon/state/PathId.scala +++ b/src/main/scala/mesosphere/marathon/state/PathId.scala @@ -137,17 +137,23 @@ object PathId { path is validPathChars } + /** + * Make sure that the given path is a child of the defined parent path. + * Every root and every relative path can be ignored. + */ private def childOf(parent: PathId): Validator[PathId] = { isTrue[PathId](s"Identifier is not child of $parent. Hint: use relative paths.") { child => - parent == PathId.empty || !parent.absolute || - (parent.absolute && child.canonicalPath(parent).parent == parent) + parent == PathId.empty || !parent.absolute || (parent.absolute && child.canonicalPath(parent).parent == parent) } } + /** + * Makes sure, the path is not only the root path and is not empty. + */ + val nonEmptyPath = isTrue[PathId]("Path must contain at least one path element") { _.path.nonEmpty } + /** * Needed for AppDefinitionValidatorTest.testSchemaLessStrictForId. */ - val absolutePathValidator = isTrue[PathId]("Path needs to be absolute") { path => - path.absolute - } + val absolutePathValidator = isTrue[PathId]("Path needs to be absolute") { _.absolute } } diff --git a/src/main/scala/mesosphere/marathon/util/SemanticVersion.scala b/src/main/scala/mesosphere/marathon/util/SemanticVersion.scala index 67e49f968d2..f937092833a 100644 --- a/src/main/scala/mesosphere/marathon/util/SemanticVersion.scala +++ b/src/main/scala/mesosphere/marathon/util/SemanticVersion.scala @@ -11,6 +11,8 @@ case class SemanticVersion(major: Int, minor: Int, patch: Int) { object SemanticVersion { val Pattern: Regex = """(\d+)\.(\d+)\.(\d+).*""".r + val zero = SemanticVersion(0, 0, 0) + // If we can not match the version pattern we prefer to throw an // error rather than silently construct a dummy version. def apply(version: String): Option[SemanticVersion] = version match { diff --git a/src/test/scala/mesosphere/marathon/api/v2/validation/PodsValidationTest.scala b/src/test/scala/mesosphere/marathon/api/v2/validation/PodsValidationTest.scala new file mode 100644 index 00000000000..73178bbb738 --- /dev/null +++ b/src/test/scala/mesosphere/marathon/api/v2/validation/PodsValidationTest.scala @@ -0,0 +1,68 @@ +package mesosphere.marathon +package v2.validation + +import mesosphere.UnitTest +import com.wix.accord.scalatest.ResultMatchers +import mesosphere.marathon.api.v2.validation.PodsValidation +import mesosphere.marathon.raml.{ Endpoint, Network, NetworkMode, Pod, PodContainer, Resources, Volume, VolumeMount } +import mesosphere.marathon.util.SemanticVersion + +class PodsValidationTest extends UnitTest with ResultMatchers with PodsValidation { + + "A pod definition" should { + + "be rejected if the id is empty" in new Fixture { + private val invalid = validPod.copy(id = "/") + validator(invalid) should failWith("id" -> "Path must contain at least one path element") + } + + "be rejected if the id is not absolute" in new Fixture { + private val invalid = validPod.copy(id = "some/foo") + validator(invalid) should failWith("id" -> "Path needs to be absolute") + } + + "be rejected if a defined user is empty" in new Fixture { + private val invalid = validPod.copy(user = Some("")) + validator(invalid) should failWith("user" -> "must not be empty") + } + + "be rejected if no container is defined" in new Fixture { + private val invalid = validPod.copy(containers = Seq.empty) + validator(invalid) should failWith("containers" -> "must not be empty") + } + + "be rejected if container names are not unique" in new Fixture { + private val invalid = validPod.copy(containers = Seq(validContainer, validContainer)) + validator(invalid) should failWith("containers" -> "container names are unique") + } + + "be rejected if endpoint names are not unique" in new Fixture { + val endpoint = Endpoint("endpoint", hostPort = Some(123)) + private val invalid = validPod.copy(containers = Seq(validContainer.copy(endpoints = Seq(endpoint, endpoint)))) + validator(invalid) should failWith("value" -> "Endpoint names are unique") + } + + "be rejected if volume names are not unique" in new Fixture { + val volume = Volume("volume", host = Some("/foo")) + val volumeMount = VolumeMount(volume.name, "/bla") + private val invalid = validPod.copy( + volumes = Seq(volume, volume), + containers = Seq(validContainer.copy(volumeMounts = Seq(volumeMount))) + ) + validator(invalid) should failWith("volumes" -> "volume names are unique") + } + } + + class Fixture { + val validContainer = PodContainer( + name = "ct1", + resources = Resources() + ) + val validPod = Pod( + id = "/some/pod", + containers = Seq(validContainer), + networks = Seq(Network(mode = NetworkMode.Host)) + ) + val validator = podDefValidator(Set.empty, SemanticVersion.zero) + } +}