From 95fb568aaeb0d5a0fa8b4b88f650b51d3b96786d Mon Sep 17 00:00:00 2001 From: Felipe Lalanne Date: Mon, 26 Apr 2021 12:06:04 -0400 Subject: [PATCH] Bump dockerode types to 2.5.34 This commit updates dockerode types to the latest 2.x version, removing the need for custom composer types for network. This commit also modifies network tests to use the new types Change-type: minor --- package-lock.json | 6 +- package.json | 2 +- src/compose/network.ts | 112 +++++++--------- src/compose/types/network.ts | 60 --------- test/17-compose-network.spec.ts | 125 ------------------ test/compose/network.spec.ts | 226 +++++++++++++++++++++++++++++++- 6 files changed, 275 insertions(+), 256 deletions(-) delete mode 100644 test/17-compose-network.spec.ts diff --git a/package-lock.json b/package-lock.json index 4add29a4..f3ca0692 100644 --- a/package-lock.json +++ b/package-lock.json @@ -471,9 +471,9 @@ "dev": true }, "@types/dockerode": { - "version": "2.5.28", - "resolved": "https://registry.npmjs.org/@types/dockerode/-/dockerode-2.5.28.tgz", - "integrity": "sha512-YHs025G2P+h+CDlmub33SY/CvGWCHpHATbgq73wykyvnZZjL0Iq+w+Vv214w3cac5McimFhsVecDJBNgPuMo4Q==", + "version": "2.5.34", + "resolved": "https://registry.npmjs.org/@types/dockerode/-/dockerode-2.5.34.tgz", + "integrity": "sha512-LcbLGcvcBwBAvjH9UrUI+4qotY+A5WCer5r43DR5XHv2ZIEByNXFdPLo1XxR+v/BjkGjlggW8qUiXuVEhqfkpA==", "dev": true, "requires": { "@types/node": "*" diff --git a/package.json b/package.json index 49fcd298..5d50de93 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "@types/common-tags": "^1.8.0", "@types/copy-webpack-plugin": "^6.0.0", "@types/dbus": "^1.0.0", - "@types/dockerode": "^2.5.28", + "@types/dockerode": "^2.5.34", "@types/event-stream": "^3.3.34", "@types/express": "^4.17.3", "@types/lockfile": "^1.0.1", diff --git a/src/compose/network.ts b/src/compose/network.ts index cac81d0e..06475673 100644 --- a/src/compose/network.ts +++ b/src/compose/network.ts @@ -1,20 +1,13 @@ import * as Bluebird from 'bluebird'; import * as _ from 'lodash'; +import * as dockerode from 'dockerode'; import { docker } from '../lib/docker-utils'; -import { InvalidAppIdError } from '../lib/errors'; import logTypes = require('../lib/log-types'); -import { checkInt } from '../lib/validation'; import * as logger from '../logger'; import * as ComposeUtils from './utils'; -import { - ComposeNetworkConfig, - DockerIPAMConfig, - DockerNetworkConfig, - NetworkConfig, - NetworkInspect, -} from './types/network'; +import { ComposeNetworkConfig, NetworkConfig } from './types/network'; import { InvalidNetworkConfigurationError, @@ -28,50 +21,42 @@ export class Network { private constructor() {} - public static fromDockerNetwork(network: NetworkInspect): Network { + public static fromDockerNetwork( + network: dockerode.NetworkInspectInfo, + ): Network { const ret = new Network(); const match = network.Name.match(/^([0-9]+)_(.+)$/); if (match == null) { throw new InvalidNetworkNameError(network.Name); } - const appId = checkInt(match[1]) || null; - if (!appId) { - throw new InvalidAppIdError(match[1]); - } + + // If the regex match succeeds `match[1]` should be a number + const appId = parseInt(match[1], 10); ret.appId = appId; ret.name = match[2]; + + const config = network.IPAM?.Config || []; + ret.config = { driver: network.Driver, ipam: { - driver: network.IPAM.Driver, - config: _.map(network.IPAM.Config, (conf) => { - const newConf: NetworkConfig['ipam']['config'][0] = {}; - - if (conf.Subnet != null) { - newConf.subnet = conf.Subnet; - } - if (conf.Gateway != null) { - newConf.gateway = conf.Gateway; - } - if (conf.IPRange != null) { - newConf.ipRange = conf.IPRange; - } - if (conf.AuxAddress != null) { - newConf.auxAddress = conf.AuxAddress; - } - - return newConf; - }), - options: network.IPAM.Options == null ? {} : network.IPAM.Options, + driver: network.IPAM?.Driver ?? 'default', + config: config.map((conf) => ({ + ...(conf.Subnet && { subnet: conf.Subnet }), + ...(conf.Gateway && { gateway: conf.Gateway }), + ...(conf.IPRange && { ipRange: conf.IPRange }), + ...(conf.AuxAddress && { auxAddress: conf.AuxAddress }), + })), + options: network.IPAM?.Options ?? {}, }, enableIPv6: network.EnableIPv6, internal: network.Internal, - labels: _.omit(ComposeUtils.normalizeLabels(network.Labels), [ + labels: _.omit(ComposeUtils.normalizeLabels(network.Labels ?? {}), [ 'io.balena.supervised', ]), - options: network.Options, + options: network.Options ?? {}, }; return ret; @@ -90,19 +75,26 @@ export class Network { Network.validateComposeConfig(network); - const ipam: Partial = network.ipam || {}; - if (ipam.driver == null) { - ipam.driver = 'default'; - } - if (ipam.config == null) { - ipam.config = []; - } - if (ipam.options == null) { - ipam.options = {}; - } + const ipam = network.ipam ?? {}; + const driver = ipam.driver ?? 'default'; + const config = ipam.config ?? []; + const options = ipam.options ?? {}; + net.config = { driver: network.driver || 'bridge', - ipam: ipam as ComposeNetworkConfig['ipam'], + ipam: { + driver, + config: config.map((conf) => ({ + ...(conf.subnet && { subnet: conf.subnet }), + ...(conf.gateway && { gateway: conf.gateway }), + ...(conf.ip_range && { ipRange: conf.ip_range }), + // TODO: compose defines aux_addresses as a dict but dockerode and the + // engine accepts a single AuxAddress. What happens when multiple addresses + // are given? + ...(conf.aux_addresses && { auxAddress: conf.aux_addresses }), + })) as ComposeNetworkConfig['ipam']['config'], + options, + }, enableIPv6: network.enable_ipv6 || false, internal: network.internal || false, labels: network.labels || {}, @@ -130,31 +122,23 @@ export class Network { network: { name: this.name }, }); - return await docker.createNetwork(this.toDockerConfig()); + await docker.createNetwork(this.toDockerConfig()); } - public toDockerConfig(): DockerNetworkConfig { + public toDockerConfig(): dockerode.NetworkCreateOptions { return { Name: Network.generateDockerName(this.appId, this.name), Driver: this.config.driver, CheckDuplicate: true, IPAM: { Driver: this.config.ipam.driver, - Config: _.map(this.config.ipam.config, (conf) => { - const ipamConf: DockerIPAMConfig = {}; - if (conf.subnet != null) { - ipamConf.Subnet = conf.subnet; - } - if (conf.gateway != null) { - ipamConf.Gateway = conf.gateway; - } - if (conf.auxAddress != null) { - ipamConf.AuxAddress = conf.auxAddress; - } - if (conf.ipRange != null) { - ipamConf.IPRange = conf.ipRange; - } - return ipamConf; + Config: this.config.ipam.config.map((conf) => { + return { + ...(conf.subnet && { Subnet: conf.subnet }), + ...(conf.gateway && { Gateway: conf.gateway }), + ...(conf.auxAddress && { AuxAddress: conf.auxAddress }), + ...(conf.ipRange && { IPRange: conf.ipRange }), + }; }), Options: this.config.ipam.options, }, diff --git a/src/compose/types/network.ts b/src/compose/types/network.ts index e3cf3f29..5ffb0155 100644 --- a/src/compose/types/network.ts +++ b/src/compose/types/network.ts @@ -1,39 +1,3 @@ -// It appears the dockerode typings are incomplete, -// extend here for now. -// TODO: Upstream these to definitelytyped -export interface NetworkInspect { - Name: string; - Id: string; - Created: string; - Scope: string; - Driver: string; - EnableIPv6: boolean; - IPAM: { - Driver: string; - Options: null | { [optName: string]: string }; - Config: Array<{ - Subnet: string; - Gateway: string; - IPRange?: string; - AuxAddress?: string; - }>; - }; - Internal: boolean; - Attachable: boolean; - Ingress: boolean; - Containers: { - [containerId: string]: { - Name: string; - EndpointID: string; - MacAddress: string; - IPv4Address: string; - IPv6Address: string; - }; - }; - Options: { [optName: string]: string }; - Labels: { [labelName: string]: string }; -} - export interface ComposeNetworkConfig { driver: string; driver_opts: Dictionary; @@ -70,27 +34,3 @@ export interface NetworkConfig { labels: { [labelName: string]: string }; options: { [optName: string]: string }; } - -export interface DockerIPAMConfig { - Subnet?: string; - IPRange?: string; - Gateway?: string; - AuxAddress?: string; -} - -export interface DockerNetworkConfig { - Name: string; - Driver?: string; - CheckDuplicate: boolean; - IPAM?: { - Driver?: string; - Config?: DockerIPAMConfig[]; - Options?: Dictionary; - }; - Internal?: boolean; - Attachable?: boolean; - Ingress?: boolean; - Options?: Dictionary; - Labels?: Dictionary; - EnableIPv6?: boolean; -} diff --git a/test/17-compose-network.spec.ts b/test/17-compose-network.spec.ts deleted file mode 100644 index 23c912d9..00000000 --- a/test/17-compose-network.spec.ts +++ /dev/null @@ -1,125 +0,0 @@ -import { expect } from './lib/chai-config'; -import { Network } from '../src/compose/network'; - -describe('compose/network', function () { - describe('compose config -> internal config', function () { - it('should convert a compose configuration to an internal representation', function () { - const network = Network.fromComposeObject( - 'test', - 123, - { - driver: 'bridge', - ipam: { - driver: 'default', - config: [ - { - subnet: '172.25.0.0/25', - gateway: '172.25.0.1', - }, - ], - }, - }, - // @ts-ignore ignore passing nulls instead of actual objects - { logger: null, docker: null }, - ); - - expect(network.config).to.deep.equal({ - driver: 'bridge', - ipam: { - driver: 'default', - config: [ - { - subnet: '172.25.0.0/25', - gateway: '172.25.0.1', - }, - ], - options: {}, - }, - enableIPv6: false, - internal: false, - labels: {}, - options: {}, - }); - }); - - it('should handle an incomplete ipam configuration', function () { - const network = Network.fromComposeObject( - 'test', - 123, - { - ipam: { - config: [ - { - subnet: '172.25.0.0/25', - gateway: '172.25.0.1', - }, - ], - }, - }, - // @ts-ignore ignore passing nulls instead of actual objects - { logger: null, docker: null }, - ); - - expect(network.config).to.deep.equal({ - driver: 'bridge', - enableIPv6: false, - internal: false, - labels: {}, - options: {}, - ipam: { - driver: 'default', - options: {}, - config: [ - { - subnet: '172.25.0.0/25', - gateway: '172.25.0.1', - }, - ], - }, - }); - }); - }); - - describe('internal config -> docker config', () => - it('should convert an internal representation to a docker representation', function () { - const network = Network.fromComposeObject( - 'test', - 123, - { - driver: 'bridge', - ipam: { - driver: 'default', - config: [ - { - subnet: '172.25.0.0/25', - gateway: '172.25.0.1', - }, - ], - }, - }, - // @ts-ignore ignore passing nulls instead of actual objects - { logger: null, docker: null }, - ); - - expect(network.toDockerConfig()).to.deep.equal({ - Name: '123_test', - Driver: 'bridge', - CheckDuplicate: true, - IPAM: { - Driver: 'default', - Config: [ - { - Subnet: '172.25.0.0/25', - Gateway: '172.25.0.1', - }, - ], - Options: {}, - }, - EnableIPv6: false, - Internal: false, - Labels: { - 'io.balena.supervised': 'true', - }, - }); - })); -}); diff --git a/test/compose/network.spec.ts b/test/compose/network.spec.ts index 89b97c5a..462f8b58 100644 --- a/test/compose/network.spec.ts +++ b/test/compose/network.spec.ts @@ -2,9 +2,10 @@ import ChaiConfig = require('../lib/chai-config'); const { expect } = ChaiConfig; import { Network } from '../../src/compose/network'; +import { NetworkInspectInfo } from 'dockerode'; -describe('Network', () => { - describe('fromComposeObject', () => { +describe('compose/network', () => { + describe('creating a network from a compose object', () => { it('creates a default network configuration if no config is given', () => { const network = Network.fromComposeObject('default', 12345, {}); @@ -54,6 +55,7 @@ describe('Network', () => { { subnet: '172.20.0.0/16', ip_range: '172.20.10.0/24', + aux_addresses: { host0: '172.20.10.15', host1: '172.20.10.16' }, gateway: '172.20.0.1', }, ], @@ -67,8 +69,9 @@ describe('Network', () => { config: [ { subnet: '172.20.0.0/16', - ip_range: '172.20.10.0/24', + ipRange: '172.20.10.0/24', gateway: '172.20.0.1', + auxAddress: { host0: '172.20.10.15', host1: '172.20.10.16' }, }, ], options: {}, @@ -109,4 +112,221 @@ describe('Network', () => { ); }); }); + + describe('creating a network from docker engine state', () => { + it('rejects networks without the proper name format', () => { + expect(() => + Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: 'abcd', + } as NetworkInspectInfo), + ).to.throw(); + + expect(() => + Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: 'abcd_1234', + } as NetworkInspectInfo), + ).to.throw(); + + expect(() => + Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: 'abcd_abcd', + } as NetworkInspectInfo), + ).to.throw(); + + expect(() => + Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: '1234', + } as NetworkInspectInfo), + ).to.throw(); + }); + + it('creates a network object from a docker network configuration', () => { + const network = Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: '1234_default', + Driver: 'bridge', + EnableIPv6: true, + IPAM: { + Driver: 'default', + Options: {}, + Config: [ + { + Subnet: '172.18.0.0/16', + Gateway: '172.18.0.1', + }, + ], + } as NetworkInspectInfo['IPAM'], + Internal: true, + Containers: {}, + Options: { + 'com.docker.some-option': 'abcd', + } as NetworkInspectInfo['Options'], + Labels: { + 'io.balena.features.something': '123', + } as NetworkInspectInfo['Labels'], + } as NetworkInspectInfo); + + expect(network.appId).to.equal(1234); + expect(network.name).to.equal('default'); + expect(network.config.enableIPv6).to.equal(true); + expect(network.config.ipam.driver).to.equal('default'); + expect(network.config.ipam.options).to.deep.equal({}); + expect(network.config.ipam.config).to.deep.equal([ + { + subnet: '172.18.0.0/16', + gateway: '172.18.0.1', + }, + ]); + expect(network.config.internal).to.equal(true); + expect(network.config.options).to.deep.equal({ + 'com.docker.some-option': 'abcd', + }); + expect(network.config.labels).to.deep.equal({ + 'io.balena.features.something': '123', + }); + }); + + it('normalizes legacy label names and excludes supervised label', () => { + const network = Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: '1234_default', + IPAM: { + Driver: 'default', + Options: {}, + Config: [], + } as NetworkInspectInfo['IPAM'], + Labels: { + 'io.resin.features.something': '123', + 'io.balena.features.dummy': 'abc', + 'io.balena.supervised': 'true', + } as NetworkInspectInfo['Labels'], + } as NetworkInspectInfo); + + expect(network.config.labels).to.deep.equal({ + 'io.balena.features.something': '123', + 'io.balena.features.dummy': 'abc', + }); + }); + }); + + describe('creating a network compose configuration from a network instance', () => { + it('creates a docker compose network object from the internal network config', () => { + const network = Network.fromDockerNetwork({ + Id: 'deadbeef', + Name: '1234_default', + Driver: 'bridge', + EnableIPv6: true, + IPAM: { + Driver: 'default', + Options: {}, + Config: [ + { + Subnet: '172.18.0.0/16', + Gateway: '172.18.0.1', + }, + ], + } as NetworkInspectInfo['IPAM'], + Internal: true, + Containers: {}, + Options: { + 'com.docker.some-option': 'abcd', + } as NetworkInspectInfo['Options'], + Labels: { + 'io.balena.features.something': '123', + } as NetworkInspectInfo['Labels'], + } as NetworkInspectInfo); + + // Convert to compose object + const compose = network.toComposeObject(); + expect(compose.driver).to.equal('bridge'); + expect(compose.driver_opts).to.deep.equal({ + 'com.docker.some-option': 'abcd', + }); + expect(compose.enable_ipv6).to.equal(true); + expect(compose.internal).to.equal(true); + expect(compose.ipam).to.deep.equal({ + driver: 'default', + options: {}, + config: [ + { + subnet: '172.18.0.0/16', + gateway: '172.18.0.1', + }, + ], + }); + expect(compose.labels).to.deep.equal({ + 'io.balena.features.something': '123', + }); + }); + }); + + describe('generateDockerName', () => { + it('creates a proper network name from the user given name and the app id', () => { + expect(Network.generateDockerName(12345, 'default')).to.equal( + '12345_default', + ); + expect(Network.generateDockerName(12345, 'bleh')).to.equal('12345_bleh'); + expect(Network.generateDockerName(1, 'default')).to.equal('1_default'); + }); + }); + + describe('comparing network configurations', () => { + it('ignores IPAM configuration', () => { + const network = Network.fromComposeObject('default', 12345, { + ipam: { + driver: 'default', + config: [ + { + subnet: '172.20.0.0/16', + ip_range: '172.20.10.0/24', + gateway: '172.20.0.1', + }, + ], + options: {}, + }, + }); + expect( + network.isEqualConfig(Network.fromComposeObject('default', 12345, {})), + ).to.be.true; + + // Only ignores ipam.config, not other ipam elements + expect( + network.isEqualConfig( + Network.fromComposeObject('default', 12345, { + ipam: { driver: 'aaa' }, + }), + ), + ).to.be.false; + }); + + it('compares configurations recursively', () => { + expect( + Network.fromComposeObject('default', 12345, {}).isEqualConfig( + Network.fromComposeObject('default', 12345, {}), + ), + ).to.be.true; + expect( + Network.fromComposeObject('default', 12345, { + driver: 'default', + }).isEqualConfig(Network.fromComposeObject('default', 12345, {})), + ).to.be.false; + expect( + Network.fromComposeObject('default', 12345, { + enable_ipv6: true, + }).isEqualConfig(Network.fromComposeObject('default', 12345, {})), + ).to.be.false; + expect( + Network.fromComposeObject('default', 12345, { + enable_ipv6: false, + internal: false, + }).isEqualConfig( + Network.fromComposeObject('default', 12345, { internal: true }), + ), + ).to.be.false; + }); + }); });