Expose ports from port mappings on services

PR #2217 removed the expose configuration but also caused a regresion
where ports set via the `ports` configuration would no longer get
exposed to the host, despite portmappings being set. This fixes that
issue by exposing only those ports comming from port mappings.

Change-type: patch

src/compose/ports.ts

@@ -15,6 +15,7 @@ export interface PortBindings {
 }
 
 export interface DockerPortOptions {
+	exposedPorts: Dictionary<{}>;
 	portBindings: PortBindings;
 }
 
@@ -48,15 +49,19 @@ export class PortMap {
 			this.ports.externalEnd,
 		);
 
+		const exposedPorts: { [key: string]: {} } = {};
 		const portBindings: PortBindings = {};
 
 		_.zipWith(internalRange, externalRange, (internal, external) => {
+			exposedPorts[`${internal}/${this.ports.protocol}`] = {};
+
 			portBindings[`${internal}/${this.ports.protocol}`] = [
 				{ HostIp: this.ports.host, HostPort: external.toString() },
 			];
 		});
 
 		return {
+			exposedPorts,
 			portBindings,
 		};
 	}

src/compose/service.ts

@@ -651,7 +651,7 @@ export class Service {
 		containerIds: Dictionary<string>;
 	}): Dockerode.ContainerCreateOptions {
 		const { binds, mounts, volumes } = this.getBindsMountsAndVolumes();
-		const { portBindings } = this.generatePortBindings();
+		const { exposedPorts, portBindings } = this.generateExposeAndPorts();
 
 		const tmpFs: Dictionary<''> = this.config.tmpfs.reduce(
 			(dict, tmp) => ({ ...dict, [tmp]: '' }),
@@ -689,6 +689,7 @@ export class Service {
 					this.config.environment,
 				),
 			),
+			ExposedPorts: exposedPorts,
 			Image: this.config.image,
 			Labels: this.config.labels,
 			NetworkingConfig:
@@ -927,11 +928,13 @@ export class Service {
 		return { binds, mounts, volumes };
 	}
 
-	private generatePortBindings(): DockerPortOptions {
+	private generateExposeAndPorts(): DockerPortOptions {
+		const exposed: DockerPortOptions['exposedPorts'] = {};
 		const ports: DockerPortOptions['portBindings'] = {};
 
 		_.each(this.config.portMaps, (pmap) => {
-			const { portBindings } = pmap.toDockerOpts();
+			const { exposedPorts, portBindings } = pmap.toDockerOpts();
+			_.merge(exposed, exposedPorts);
 			_.mergeWith(ports, portBindings, (destVal, srcVal) => {
 				if (destVal == null) {
 					return srcVal;
@@ -940,7 +943,7 @@ export class Service {
 			});
 		});
 
-		return { portBindings: ports };
+		return { exposedPorts: exposed, portBindings: ports };
 	}
 
 	private static extendEnvVars(

test/integration/state-engine.spec.ts

@@ -3,6 +3,7 @@ import * as Docker from 'dockerode';
 import { TargetStateV2 } from '~/lib/legacy';
 import * as request from 'supertest';
 import { setTimeout as delay } from 'timers/promises';
+import { exec } from '~/lib/fs-utils';
 
 const BALENA_SUPERVISOR_ADDRESS =
 	process.env.BALENA_SUPERVISOR_ADDRESS || 'http://balena-supervisor:48484';
@@ -97,9 +98,11 @@ describe('state engine', () => {
 							serviceName: 'one',
 							restart: 'unless-stopped',
 							running: true,
-							command: 'sleep infinity',
+							command:
+								'sh -c "while true; do echo -n \'Hello World!!\' | nc -lv -p 8080; done"',
 							stop_signal: 'SIGKILL',
 							networks: ['default'],
+							ports: ['8080'],
 							labels: {},
 							environment: {},
 						},
@@ -134,6 +137,10 @@ describe('state engine', () => {
 			{ Name: '/one_11_1_deadbeef', State: 'running' },
 			{ Name: '/two_12_1_deadbeef', State: 'running' },
 		]);
+
+		// Test that the service is running and accesssible via port 8080
+		// this will throw if the server does not respond
+		await exec('nc -v docker 8080 -z');
 	});
 
 	// This test recovery from issue #1576, where a device running a service from the target release

test/unit/compose/ports.spec.ts

@@ -98,6 +98,9 @@ describe('compose/ports', function () {
 	describe('toDockerOpts', function () {
 		it('should correctly generate docker options', () =>
 			expect(new PortMapPublic('80').toDockerOpts()).to.deep.equal({
+				exposedPorts: {
+					'80/tcp': {},
+				},
 				portBindings: {
 					'80/tcp': [{ HostIp: '', HostPort: '80' }],
 				},
@@ -105,6 +108,14 @@ describe('compose/ports', function () {
 
 		it('should correctly generate docker options for a port range', () =>
 			expect(new PortMapPublic('80-85').toDockerOpts()).to.deep.equal({
+				exposedPorts: {
+					'80/tcp': {},
+					'81/tcp': {},
+					'82/tcp': {},
+					'83/tcp': {},
+					'84/tcp': {},
+					'85/tcp': {},
+				},
 				portBindings: {
 					'80/tcp': [{ HostIp: '', HostPort: '80' }],
 					'81/tcp': [{ HostIp: '', HostPort: '81' }],

test/unit/compose/service.spec.ts

@@ -112,6 +112,7 @@ describe('compose/service: unit tests', () => {
 					serviceId: 3,
 					imageId: 4,
 					composition: {
+						// This will be ignored
 						expose: [1000, '243/udp'],
 						ports: ['2344', '2345:2354', '2346:2367/udp'],
 					},
@@ -120,6 +121,7 @@ describe('compose/service: unit tests', () => {
 					imageInfo: {
 						Config: {
 							ExposedPorts: {
+								// This will be ignored by generateExposeAndPorts
 								'53/tcp': {},
 								'53/udp': {},
 								'2354/tcp': {},
@@ -129,7 +131,7 @@ describe('compose/service: unit tests', () => {
 				} as any,
 			);
 
-			const ports = (s as any).generatePortBindings();
+			const ports = (s as any).generateExposeAndPorts();
 			expect(ports.portBindings).to.deep.equal({
 				'2344/tcp': [
 					{
@@ -150,6 +152,13 @@ describe('compose/service: unit tests', () => {
 					},
 				],
 			});
+			// Only exposes ports coming from the `ports`
+			// property
+			expect(ports.exposedPorts).to.deep.equal({
+				'2344/tcp': {},
+				'2354/tcp': {},
+				'2367/udp': {},
+			});
 		});
 
 		it('correctly handles port ranges', async () => {
@@ -168,7 +177,7 @@ describe('compose/service: unit tests', () => {
 				{ appName: 'test' } as any,
 			);
 
-			const ports = (s as any).generatePortBindings();
+			const ports = (s as any).generateExposeAndPorts();
 			expect(ports.portBindings).to.deep.equal({
 				'2000/tcp': [
 					{
@@ -195,6 +204,13 @@ describe('compose/service: unit tests', () => {
 					},
 				],
 			});
+
+			expect(ports.exposedPorts).to.deep.equal({
+				'2000/tcp': {},
+				'2001/tcp': {},
+				'2002/tcp': {},
+				'2003/tcp': {},
+			});
 		});
 
 		it('should correctly handle large port ranges', async function () {
@@ -213,23 +229,7 @@ describe('compose/service: unit tests', () => {
 				{ appName: 'test' } as any,
 			);
 
-			expect((s as any).generatePortBindings).to.not.throw;
-		});
-
-		it('should not report implied exposed ports from portMappings', async () => {
-			const service = await Service.fromComposeObject(
-				{
-					appId: 123456,
-					serviceId: 123456,
-					serviceName: 'test',
-					composition: {
-						ports: ['80:80', '100:100'],
-					},
-				},
-				{ appName: 'test' } as any,
-			);
-
-			expect(service.config).to.not.have.property('expose');
+			expect((s as any).generateExposeAndPorts()).to.not.throw;
 		});
 
 		it('should correctly handle spaces in volume definitions', async () => {