From 6f02b17968d02c2e27b523e40a25ef4c4815d20a Mon Sep 17 00:00:00 2001 From: Felipe Lalanne <1822826+pipex@users.noreply.github.com> Date: Tue, 23 Apr 2024 19:21:52 -0400 Subject: [PATCH] Refactor MDNS resolver into a module Also add integration tests for the resolver functionality to prevent regressions. Change-type: patch --- docker-compose.test.yml | 8 +- src/app.ts | 141 +--------------------------------- src/mdns.ts | 139 +++++++++++++++++++++++++++++++++ test/integration/mdns.spec.ts | 24 ++++++ 4 files changed, 172 insertions(+), 140 deletions(-) create mode 100644 src/mdns.ts create mode 100644 test/integration/mdns.spec.ts diff --git a/docker-compose.test.yml b/docker-compose.test.yml index c435b8d4..7d9b4e92 100644 --- a/docker-compose.test.yml +++ b/docker-compose.test.yml @@ -40,13 +40,18 @@ services: MOCK_SYSTEMD_UNITS: openvpn.service avahi.socket docker: - image: docker:24-dind + image: docker:dind stop_grace_period: 3s privileged: true environment: DOCKER_TLS_CERTDIR: '' command: --tls=false # --debug + mdns: + image: ydkn/avahi:latest + # This hostname should be resolved + hostname: my-device + sut: # Build the supervisor code for development and testing build: @@ -69,6 +74,7 @@ services: - balena-supervisor-sut - docker - mock-systemd + - mdns stop_grace_period: 3s volumes: - dbus:/shared/dbus diff --git a/src/app.ts b/src/app.ts index e8f68b02..11d3c563 100644 --- a/src/app.ts +++ b/src/app.ts @@ -6,145 +6,8 @@ import { setDefaultAutoSelectFamilyAttemptTimeout } from 'net'; // Increase the timeout for the happy eyeballs algorithm to 5000ms to avoid issues on slower networks setDefaultAutoSelectFamilyAttemptTimeout(5000); -import { NOTFOUND } from 'dns'; -import * as mdnsResolver from 'mdns-resolver'; - -class DnsLookupError extends Error { - public constructor(public code: string = NOTFOUND) { - super(); - } -} - -interface DnsLookupCallback { - (err: any): void; - (err: undefined | null, address: string, family: number): void; - ( - err: undefined | null, - addresses: Array<{ address: string; family: number }>, - ): void; -} - -interface DnsLookupOpts { - verbatim: boolean; - all: boolean; - family: number; -} - -/* - * ===================================================== - * NOTE: This is required since Alpine/musl-based images - * do not support mDNS name resolution in the musl libs. - * - * In order to support .local mDNS names in development - * and on openBalena setups, this extra code will perform - * the lookup instead of passing it to the built-in - * function. - * ================================================== - */ -async function mdnsLookup( - name: string, - opts: Partial | number | null | undefined, - cb: DnsLookupCallback, -) { - // determine which resolvers to use... - const getResolvers = () => { - // logic to determine the families based on Node docs. - // See: https://nodejs.org/api/dns.html#dns_dns_lookup_hostname_options_callback - const families = (() => { - // strictly defined... - if (opts === 4 || opts === 6) { - return [opts as number]; - } - - // opts is passed, not a number and the `family` parameter is passed... - if (opts && typeof opts !== 'number' && opts.family) { - return [opts.family]; - } - - // default to resolve both v4 and v6... - return [4, 6]; - })(); - - // map them and return... - return families.map(async (family) => { - let address = ''; - try { - address = await mdnsResolver.resolve(name, family === 6 ? 'AAAA' : 'A'); - } catch { - /* noop */ - } - - return { address, family }; - }); - }; - - // resolve the addresses... - const results = await Promise.all(getResolvers()); - // remove any that didn't resolve... - let allAddresses = results.filter((result) => result.address !== ''); - - // unless the results should be returned verbatim, sort them so v4 comes first... - if (opts && typeof opts !== 'number' && !opts.verbatim) { - allAddresses = allAddresses.sort((a, b) => a.family - b.family); - } - - // see if we resolved anything... - if (allAddresses.length === 0) { - // nothing! return a suitable error... - return cb(new DnsLookupError()); - } - - // all the addresses were requested... - if (opts && typeof opts !== 'number' && opts.all) { - return cb(null, allAddresses); - } - - // only a single address was requested... - const [{ address: addr, family: fmly }] = allAddresses; - return cb(null, addr, fmly); -} - -// This was originally wrapped in a do block in -// coffeescript, and it's not clear now why that was the -// case, so I'm going to maintain that behaviour -(() => { - // Make NodeJS RFC 3484 compliant for properly handling IPv6 - // See: https://github.com/nodejs/node/pull/14731 - // https://github.com/nodejs/node/pull/17793 - // We disable linting for the next line. The require call - // is necesary for monkey-patching the dns module - const dns = require('dns'); // eslint-disable-line - const { lookup } = dns; - - dns.lookup = ( - name: string, - opts: Partial | number | null | undefined, - cb: DnsLookupCallback, - ) => { - if (typeof cb !== 'function') { - return lookup(name, { verbatim: true }, opts); - } - - // Try a regular dns lookup first - return lookup( - name, - Object.assign({ verbatim: true }, opts), - (error: any, address: string, family: number) => { - if (error == null) { - return cb(null, address, family); - } - - // If the regular lookup fails, we perform a mdns lookup if the - // name ends with .local - if (name && name.endsWith('.local')) { - return mdnsLookup(name, opts, cb); - } - - return cb(error); - }, - ); - }; -})(); +// Setup MDNS resolution +import './mdns'; import Supervisor from './supervisor'; import process from 'process'; diff --git a/src/mdns.ts b/src/mdns.ts new file mode 100644 index 00000000..b710a6c9 --- /dev/null +++ b/src/mdns.ts @@ -0,0 +1,139 @@ +import { NOTFOUND } from 'dns'; +import * as mdnsResolver from 'mdns-resolver'; + +class DnsLookupError extends Error { + public constructor(public code: string = NOTFOUND) { + super(); + } +} + +interface DnsLookupCallback { + (err: any): void; + (err: undefined | null, address: string, family: number): void; + ( + err: undefined | null, + addresses: Array<{ address: string; family: number }>, + ): void; +} + +interface DnsLookupOpts { + verbatim: boolean; + all: boolean; + family: number; +} + +/* + * ===================================================== + * NOTE: This is required since Alpine/musl-based images + * do not support mDNS name resolution in the musl libs. + * + * In order to support .local mDNS names in development + * and on openBalena setups, this extra code will perform + * the lookup instead of passing it to the built-in + * function. + * ================================================== + */ +async function mdnsLookup( + name: string, + opts: Partial | number | null | undefined, + cb: DnsLookupCallback, +) { + // determine which resolvers to use... + const getResolvers = () => { + // logic to determine the families based on Node docs. + // See: https://nodejs.org/api/dns.html#dns_dns_lookup_hostname_options_callback + const families = (() => { + // strictly defined... + if (opts === 4 || opts === 6) { + return [opts as number]; + } + + // opts is passed, not a number and the `family` parameter is passed... + if (opts && typeof opts !== 'number' && opts.family) { + return [opts.family]; + } + + // default to resolve both v4 and v6... + return [4, 6]; + })(); + + // map them and return... + return families.map(async (family) => { + let address = ''; + try { + address = await mdnsResolver.resolve(name, family === 6 ? 'AAAA' : 'A'); + } catch { + /* noop */ + } + + return { address, family }; + }); + }; + + // resolve the addresses... + const results = await Promise.all(getResolvers()); + // remove any that didn't resolve... + let allAddresses = results.filter((result) => result.address !== ''); + + // unless the results should be returned verbatim, sort them so v4 comes first... + if (opts && typeof opts !== 'number' && !opts.verbatim) { + allAddresses = allAddresses.sort((a, b) => a.family - b.family); + } + + // see if we resolved anything... + if (allAddresses.length === 0) { + // nothing! return a suitable error... + return cb(new DnsLookupError()); + } + + // all the addresses were requested... + if (opts && typeof opts !== 'number' && opts.all) { + return cb(null, allAddresses); + } + + // only a single address was requested... + const [{ address: addr, family: fmly }] = allAddresses; + return cb(null, addr, fmly); +} + +// This was originally wrapped in a do block in +// coffeescript, and it's not clear now why that was the +// case, so I'm going to maintain that behaviour +(() => { + // Make NodeJS RFC 3484 compliant for properly handling IPv6 + // See: https://github.com/nodejs/node/pull/14731 + // https://github.com/nodejs/node/pull/17793 + // We disable linting for the next line. The require call + // is necesary for monkey-patching the dns module + const dns = require('dns'); // eslint-disable-line + const { lookup } = dns; + + dns.lookup = ( + name: string, + opts: Partial | number | null | undefined, + cb: DnsLookupCallback, + ) => { + if (typeof cb !== 'function') { + return lookup(name, { verbatim: true }, opts); + } + + // Try a regular dns lookup first + return lookup( + name, + Object.assign({ verbatim: true }, opts), + (error: any, address: string, family: number) => { + if (error == null) { + return cb(null, address, family); + } + + // If the regular lookup fails, we perform a mdns lookup if the + // name ends with .local + if (name && name.endsWith('.local')) { + return mdnsLookup(name, opts, cb); + } + + return cb(error); + }, + ); + }; +})(); diff --git a/test/integration/mdns.spec.ts b/test/integration/mdns.spec.ts new file mode 100644 index 00000000..8097ac5b --- /dev/null +++ b/test/integration/mdns.spec.ts @@ -0,0 +1,24 @@ +import { expect } from 'chai'; +import { promisify } from 'util'; + +import '~/src/mdns'; + +describe('MDNS', () => { + it('resolves global domain names', async () => { + const dns = require('dns'); // eslint-disable-line + const lookup = promisify(dns.lookup); + const res = await lookup('www.google.com', { all: true }); + expect(res).to.not.be.null; + expect(res.length).to.be.greaterThan(0); + expect(Object.keys(res[0])).to.deep.equal(['address', 'family']); + }); + + it('resolves .local domain names', async () => { + const dns = require('dns'); // eslint-disable-line + const lookup = promisify(dns.lookup); + const res = await lookup('my-device.local', { all: true }); + expect(res).to.not.be.null; + expect(res.length).to.be.greaterThan(0); + expect(Object.keys(res[0])).to.deep.equal(['address', 'family']); + }); +});