fix: Correctly handle multiple hosts ports pointing to a container port

When assigning multiple host ports to a single container port before
this change, the supervisor would incorrectly take only the first host
port into consideration. This change makes it so that every host port
per container port is considered.

Closes: #986
Change-type: patch
Signed-off-by: Cameron Diver <cameron@balena.io>

src/compose/ports.ts

@@ -90,24 +90,25 @@ export class PortMap {
 	public static fromDockerOpts(portBindings: PortBindings): PortMap[] {
 		// Create a list of portBindings, rather than the map (which we can't
 		// order)
-		const portMaps = _.map(portBindings, (hostObj, internalStr) => {
+		const portMaps = _.flatMap(portBindings, (hostObj, internalStr) => {
 			const match = internalStr.match(DOCKER_OPTS_PORTS_REGEX);
 			if (match == null) {
 				throw new Error(`Could not parse docker port output: ${internalStr}`);
 			}
 			const internal = parseInt(match[1], 10);
-			const external = parseInt(hostObj[0].HostPort, 10);
-
-			const host = hostObj[0].HostIp;
 			const proto = match[2] || 'tcp';
 
-			return new PortMap({
-				internalStart: internal,
-				internalEnd: internal,
-				externalStart: external,
-				externalEnd: external,
-				protocol: proto,
-				host,
+			return _.map(hostObj, ({ HostIp, HostPort }) => {
+				const external = parseInt(HostPort, 10);
+				const host = HostIp;
+				return new PortMap({
+					internalStart: internal,
+					internalEnd: internal,
+					externalStart: external,
+					externalEnd: external,
+					protocol: proto,
+					host,
+				});
 			});
 		});
 

src/compose/service.ts

@@ -775,7 +775,12 @@ export class Service {
 		_.each(this.config.portMaps, pmap => {
 			const { exposedPorts, portBindings } = pmap.toDockerOpts();
 			_.merge(exposed, exposedPorts);
-			_.merge(ports, portBindings);
+			_.mergeWith(ports, portBindings, (destVal, srcVal) => {
+				if (destVal == null) {
+					return srcVal;
+				}
+				return destVal.concat(srcVal);
+			});
 		});
 
 		// We also want to merge the compose and image exposedPorts

test/16-ports.spec.coffee

@@ -243,6 +243,28 @@ describe 'Ports', ->
 				})
 			])
 
+		it 'should correctly detect multiple hosts ports on an internal port', ->
+			expect(PortMap.fromDockerOpts({
+				'100/tcp': [{ HostIp: '123', HostPort: 200 }, { HostIp: '123', HostPort: 201 }]
+			})).to.deep.equal([
+				new PortMap({
+					internalStart: 100,
+					internalEnd: 100,
+					externalStart: 200,
+					externalEnd: 200,
+					protocol: 'tcp',
+					host: '123'
+				})
+				new PortMap({
+					internalStart: 100,
+					internalEnd: 100,
+					externalStart: 201,
+					externalEnd: 201,
+					protocol: 'tcp',
+					host: '123'
+				})
+			])
+
 	describe 'Running container comparison', ->
 		it 'should not consider order when comparing current and target state', ->
 			portBindings = require('./data/ports/not-ascending/port-bindings.json')