Fixes #5116 by validating all endpoints of a pod for overlapping host…

… and container ports.

Summary: See #5116

Test Plan: sbt test

Reviewers: unterstein, jenkins, jdef

Reviewed By: unterstein, jenkins, jdef

Subscribers: marathon-team

Differential Revision: https://phabricator.mesosphere.com/D496

src/main/scala/mesosphere/marathon/api/v2/validation/PodsValidation.scala

@@ -302,6 +302,21 @@ trait PodsValidation {
     }
   }
 
+  val endpointNamesUnique: Validator[Pod] = isTrue("Endpoint names are unique") { pod: Pod =>
+    val names = pod.containers.flatMap(_.endpoints.map(_.name))
+    names.distinct.size == names.size
+  }
+
+  val endpointContainerPortsUnique: Validator[Pod] = isTrue("Container ports are unique") { pod: Pod =>
+    val containerPorts = pod.containers.flatMap(_.endpoints.flatMap(_.containerPort))
+    containerPorts.distinct.size == containerPorts.size
+  }
+
+  val endpointHostPortsUnique: Validator[Pod] = isTrue("Host ports are unique") { pod: Pod =>
+    val hostPorts = pod.containers.flatMap(_.endpoints.flatMap(_.hostPort)).filter(_ != 0)
+    hostPorts.distinct.size == hostPorts.size
+  }
+
   def podDefValidator(enabledFeatures: Set[String], mesosMasterVersion: SemanticVersion): Validator[Pod] = validator[Pod] { pod =>
     PathId(pod.id) as "id" is valid and PathId.absolutePathValidator and PathId.nonEmptyPath
     pod.user is optional(notEmpty)
@@ -320,10 +335,7 @@ trait PodsValidation {
     pod.networks is every(networkValidator)
     pod.scheduling is optional(schedulingValidator)
     pod.scaling is optional(scalingValidator)
-    pod is isTrue("Endpoint names are unique") { pod: Pod =>
-      val names = pod.containers.flatMap(_.endpoints.map(_.name))
-      names.distinct.size == names.size
-    }
+    pod is endpointNamesUnique and endpointContainerPortsUnique and endpointHostPortsUnique
   }
 }
 

src/test/scala/mesosphere/marathon/api/v2/validation/PodsValidationTest.scala

@@ -37,11 +37,29 @@ class PodsValidationTest extends UnitTest with ResultMatchers with PodsValidatio
     }
 
     "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))))
+      val endpoint1 = Endpoint("endpoint", hostPort = Some(123))
+      val endpoint2 = Endpoint("endpoint", hostPort = Some(124))
+      private val invalid = validPod.copy(containers = Seq(validContainer.copy(endpoints = Seq(endpoint1, endpoint2))))
       validator(invalid) should failWith("value" -> "Endpoint names are unique")
     }
 
+    "be rejected if endpoint host ports are not unique" in new Fixture {
+      val endpoint1 = Endpoint("endpoint1", hostPort = Some(123))
+      val endpoint2 = Endpoint("endpoint2", hostPort = Some(123))
+      private val invalid = validPod.copy(containers = Seq(validContainer.copy(endpoints = Seq(endpoint1, endpoint2))))
+      validator(invalid) should failWith("value" -> "Host ports are unique")
+    }
+
+    "be rejected if endpoint container ports are not unique" in new Fixture {
+      val endpoint1 = Endpoint("endpoint1", containerPort = Some(123))
+      val endpoint2 = Endpoint("endpoint2", containerPort = Some(123))
+      private val invalid = validPod.copy(
+        networks = Seq(Network(mode = NetworkMode.Container)),
+        containers = Seq(validContainer.copy(endpoints = Seq(endpoint1, endpoint2)))
+      )
+      validator(invalid) should failWith("value" -> "Container ports 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")