From 57c18382d8769a1c9eee4d0d22006828e32d6899 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jan 2020 09:42:13 -0800 Subject: [PATCH 1/5] grpc-js: Use an object to represent subchannel addresses --- packages/grpc-js/src/channel.ts | 3 +- .../grpc-js/src/load-balancer-pick-first.ts | 6 +- .../grpc-js/src/load-balancer-round-robin.ts | 4 +- packages/grpc-js/src/load-balancer.ts | 6 +- packages/grpc-js/src/resolver-dns.ts | 27 ++++--- packages/grpc-js/src/resolver-uds.ts | 5 +- packages/grpc-js/src/resolver.ts | 3 +- .../grpc-js/src/resolving-load-balancer.ts | 9 ++- packages/grpc-js/src/subchannel-pool.ts | 73 +++++++++---------- packages/grpc-js/src/subchannel.ts | 61 +++++++++++++--- packages/grpc-js/test/test-resolver.ts | 37 +++++----- 11 files changed, 136 insertions(+), 98 deletions(-) diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index 6cf987cb..a19cc217 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -38,6 +38,7 @@ import { getDefaultAuthority } from './resolver'; import { LoadBalancingConfig } from './load-balancing-config'; import { ServiceConfig, validateServiceConfig } from './service-config'; import { trace } from './logging'; +import { SubchannelAddress } from './subchannel'; export enum ConnectivityState { CONNECTING, @@ -145,7 +146,7 @@ export class ChannelImplementation implements Channel { this.subchannelPool = getSubchannelPool((options['grpc.use_local_subchannel_pool'] ?? 0) === 0); const channelControlHelper: ChannelControlHelper = { createSubchannel: ( - subchannelAddress: string, + subchannelAddress: SubchannelAddress, subchannelArgs: ChannelOptions ) => { return this.subchannelPool.getOrCreateSubchannel( diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts index a5fbf0fa..fffb2a2a 100644 --- a/packages/grpc-js/src/load-balancer-pick-first.ts +++ b/packages/grpc-js/src/load-balancer-pick-first.ts @@ -30,7 +30,7 @@ import { UnavailablePicker, } from './picker'; import { LoadBalancingConfig } from './load-balancing-config'; -import { Subchannel, ConnectivityStateListener } from './subchannel'; +import { Subchannel, ConnectivityStateListener, SubchannelAddress } from './subchannel'; import * as logging from './logging'; import { LogVerbosity } from './constants'; @@ -76,7 +76,7 @@ export class PickFirstLoadBalancer implements LoadBalancer { /** * The list of backend addresses most recently passed to `updateAddressList`. */ - private latestAddressList: string[] = []; + private latestAddressList: SubchannelAddress[] = []; /** * The list of subchannels this load balancer is currently attempting to * connect to. @@ -369,7 +369,7 @@ export class PickFirstLoadBalancer implements LoadBalancer { } updateAddressList( - addressList: string[], + addressList: SubchannelAddress[], lbConfig: LoadBalancingConfig | null ): void { // lbConfig has no useful information for pick first load balancing diff --git a/packages/grpc-js/src/load-balancer-round-robin.ts b/packages/grpc-js/src/load-balancer-round-robin.ts index e3eedfe5..2aeda430 100644 --- a/packages/grpc-js/src/load-balancer-round-robin.ts +++ b/packages/grpc-js/src/load-balancer-round-robin.ts @@ -30,7 +30,7 @@ import { UnavailablePicker, } from './picker'; import { LoadBalancingConfig } from './load-balancing-config'; -import { Subchannel, ConnectivityStateListener } from './subchannel'; +import { Subchannel, ConnectivityStateListener, SubchannelAddress } from './subchannel'; const TYPE_NAME = 'round_robin'; @@ -168,7 +168,7 @@ export class RoundRobinLoadBalancer implements LoadBalancer { } updateAddressList( - addressList: string[], + addressList: SubchannelAddress[], lbConfig: LoadBalancingConfig | null ): void { this.resetSubchannelList(); diff --git a/packages/grpc-js/src/load-balancer.ts b/packages/grpc-js/src/load-balancer.ts index 69114521..5fa4bdc5 100644 --- a/packages/grpc-js/src/load-balancer.ts +++ b/packages/grpc-js/src/load-balancer.ts @@ -16,7 +16,7 @@ */ import { ChannelOptions } from './channel-options'; -import { Subchannel } from './subchannel'; +import { Subchannel, SubchannelAddress } from './subchannel'; import { ConnectivityState } from './channel'; import { Picker } from './picker'; import { LoadBalancingConfig } from './load-balancing-config'; @@ -34,7 +34,7 @@ export interface ChannelControlHelper { * @param subchannelArgs Extra channel arguments specified by the load balancer */ createSubchannel( - subchannelAddress: string, + subchannelAddress: SubchannelAddress, subchannelArgs: ChannelOptions ): Subchannel; /** @@ -66,7 +66,7 @@ export interface LoadBalancer { * if one was provided */ updateAddressList( - addressList: string[], + addressList: SubchannelAddress[], lbConfig: LoadBalancingConfig | null ): void; /** diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts index 9f91d70a..9d357996 100644 --- a/packages/grpc-js/src/resolver-dns.ts +++ b/packages/grpc-js/src/resolver-dns.ts @@ -30,6 +30,7 @@ import { StatusObject } from './call-stream'; import { Metadata } from './metadata'; import * as logging from './logging'; import { LogVerbosity } from './constants'; +import { SubchannelAddress } from './subchannel'; const TRACER_NAME = 'dns_resolver'; @@ -112,7 +113,7 @@ const dnsLookupPromise = util.promisify(dns.lookup); * @param target * @return An "IP:port" string in an array if parsing was successful, `null` otherwise */ -function parseIP(target: string): string[] | null { +function parseIP(target: string): SubchannelAddress[] | null { /* These three regular expressions are all mutually exclusive, so we just * want the first one that matches the target string, if any do. */ const ipv4Match = IPV4_REGEX.exec(target); @@ -123,14 +124,14 @@ function parseIP(target: string): string[] | null { } // ipv6 addresses should be bracketed - const addr = ipv4Match ? match[1] : `[${match[1]}]`; + const addr = match[1]; let port: string; if (match[2]) { port = match[2]; } else { port = DEFAULT_PORT; } - return [`${addr}:${port}`]; + return [{host: addr, port: +port}]; } /** @@ -161,7 +162,7 @@ function mergeArrays(...arrays: T[][]): T[] { * Resolver implementation that handles DNS names and IP addresses. */ class DnsResolver implements Resolver { - private readonly ipResult: string[] | null; + private readonly ipResult: SubchannelAddress[] | null; private readonly dnsHostname: string | null; private readonly port: string | null; /* The promise results here contain, in order, the A record, the AAAA record, @@ -222,23 +223,21 @@ class DnsResolver implements Resolver { this.pendingResultPromise.then( ([addressList, txtRecord]) => { this.pendingResultPromise = null; - const ip4Addresses: string[] = addressList - .filter(addr => addr.family === 4) - .map(addr => `${addr.address}:${this.port}`); - let ip6Addresses: string[]; + const ip4Addresses: dns.LookupAddress[] = addressList + .filter(addr => addr.family === 4); + let ip6Addresses: dns.LookupAddress[]; if (semver.satisfies(process.version, IPV6_SUPPORT_RANGE)) { - ip6Addresses = addressList - .filter(addr => addr.family === 6) - .map(addr => `[${addr.address}]:${this.port}`); + ip6Addresses = addressList.filter(addr => addr.family === 6); } else { ip6Addresses = []; } - const allAddresses: string[] = mergeArrays( + const allAddresses: SubchannelAddress[] = mergeArrays( ip4Addresses, ip6Addresses - ); + ).map(addr => {return {host: addr.address, port: +this.port!};}); + const allAddressesString: string = '[' + allAddresses.map(addr => addr.host + ':' + addr.port).join(',') + ']'; trace( - 'Resolved addresses for target ' + this.target + ': ' + allAddresses + 'Resolved addresses for target ' + this.target + ': ' + allAddressesString ); if (allAddresses.length === 0) { this.listener.onError(this.defaultResolutionError); diff --git a/packages/grpc-js/src/resolver-uds.ts b/packages/grpc-js/src/resolver-uds.ts index 536a018b..64c628f3 100644 --- a/packages/grpc-js/src/resolver-uds.ts +++ b/packages/grpc-js/src/resolver-uds.ts @@ -20,6 +20,7 @@ import { registerResolver, registerDefaultResolver, } from './resolver'; +import { SubchannelAddress } from './subchannel'; function getUdsName(target: string): string { /* Due to how this resolver is registered, it should only be constructed @@ -34,9 +35,9 @@ function getUdsName(target: string): string { } class UdsResolver implements Resolver { - private addresses: string[] = []; + private addresses: SubchannelAddress[] = []; constructor(target: string, private listener: ResolverListener) { - this.addresses = [getUdsName(target)]; + this.addresses = [{path: getUdsName(target)}]; } updateResolution(): void { process.nextTick( diff --git a/packages/grpc-js/src/resolver.ts b/packages/grpc-js/src/resolver.ts index 3af5da9c..b2be310f 100644 --- a/packages/grpc-js/src/resolver.ts +++ b/packages/grpc-js/src/resolver.ts @@ -20,6 +20,7 @@ import { ServiceConfig } from './service-config'; import * as resolver_dns from './resolver-dns'; import * as resolver_uds from './resolver-uds'; import { StatusObject } from './call-stream'; +import { SubchannelAddress } from './subchannel'; /** * A listener object passed to the resolver's constructor that provides name @@ -36,7 +37,7 @@ export interface ResolverListener { * service configuration was invalid */ onSuccessfulResolution( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ): void; diff --git a/packages/grpc-js/src/resolving-load-balancer.ts b/packages/grpc-js/src/resolving-load-balancer.ts index 496e1345..a9ba0594 100644 --- a/packages/grpc-js/src/resolving-load-balancer.ts +++ b/packages/grpc-js/src/resolving-load-balancer.ts @@ -34,6 +34,7 @@ import { StatusObject } from './call-stream'; import { Metadata } from './metadata'; import * as logging from './logging'; import { LogVerbosity } from './constants'; +import { SubchannelAddress } from './subchannel'; const TRACER_NAME = 'resolving_load_balancer'; @@ -132,7 +133,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { this.updateState(ConnectivityState.IDLE, new QueuePicker(this)); this.innerResolver = createResolver(target, { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: ServiceError | null ) => { @@ -243,7 +244,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { this.innerChannelControlHelper = { createSubchannel: ( - subchannelAddress: string, + subchannelAddress: SubchannelAddress, subchannelArgs: ChannelOptions ) => { return this.channelControlHelper.createSubchannel( @@ -289,7 +290,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { this.replacementChannelControlHelper = { createSubchannel: ( - subchannelAddress: string, + subchannelAddress: SubchannelAddress, subchannelArgs: ChannelOptions ) => { return this.channelControlHelper.createSubchannel( @@ -409,7 +410,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { } updateAddressList( - addressList: string[], + addressList: SubchannelAddress[], lbConfig: LoadBalancingConfig | null ) { throw new Error('updateAddressList not supported on ResolvingLoadBalancer'); diff --git a/packages/grpc-js/src/subchannel-pool.ts b/packages/grpc-js/src/subchannel-pool.ts index 009141e3..3a49295b 100644 --- a/packages/grpc-js/src/subchannel-pool.ts +++ b/packages/grpc-js/src/subchannel-pool.ts @@ -16,7 +16,7 @@ */ import { ChannelOptions, channelOptionsEqual } from './channel-options'; -import { Subchannel } from './subchannel'; +import { Subchannel, SubchannelAddress, subchannelAddressEqual } from './subchannel'; import { ChannelCredentials } from './channel-credentials'; // 10 seconds in milliseconds. This value is arbitrary. @@ -28,13 +28,12 @@ const REF_CHECK_INTERVAL = 10_000; export class SubchannelPool { private pool: { - [channelTarget: string]: { - [subchannelTarget: string]: Array<{ - channelArguments: ChannelOptions; - channelCredentials: ChannelCredentials; - subchannel: Subchannel; - }>; - }; + [channelTarget: string]: Array<{ + subchannelAddress: SubchannelAddress; + channelArguments: ChannelOptions; + channelCredentials: ChannelCredentials; + subchannel: Subchannel; + }>; } = Object.create(null); /** @@ -62,23 +61,20 @@ export class SubchannelPool { * do not need to be filtered */ // tslint:disable-next-line:forin for (const channelTarget in this.pool) { - // tslint:disable-next-line:forin - for (const subchannelTarget in this.pool[channelTarget]) { - const subchannelObjArray = this.pool[channelTarget][subchannelTarget]; + const subchannelObjArray = this.pool[channelTarget]; - const refedSubchannels = subchannelObjArray.filter( - value => !value.subchannel.unrefIfOneRef() - ); + const refedSubchannels = subchannelObjArray.filter( + value => !value.subchannel.unrefIfOneRef() + ); - if (refedSubchannels.length > 0) { - allSubchannelsUnrefed = false; - } - - /* For each subchannel in the pool, try to unref it if it has - * exactly one ref (which is the ref from the pool itself). If that - * does happen, remove the subchannel from the pool */ - this.pool[channelTarget][subchannelTarget] = refedSubchannels; + if (refedSubchannels.length > 0) { + allSubchannelsUnrefed = false; } + + /* For each subchannel in the pool, try to unref it if it has + * exactly one ref (which is the ref from the pool itself). If that + * does happen, remove the subchannel from the pool */ + this.pool[channelTarget] = refedSubchannels; } /* Currently we do not delete keys with empty values. If that results * in significant memory usage we should change it. */ @@ -114,25 +110,24 @@ export class SubchannelPool { */ getOrCreateSubchannel( channelTarget: string, - subchannelTarget: string, + subchannelTarget: SubchannelAddress, channelArguments: ChannelOptions, channelCredentials: ChannelCredentials ): Subchannel { this.ensureCleanupTask(); if (channelTarget in this.pool) { - if (subchannelTarget in this.pool[channelTarget]) { - const subchannelObjArray = this.pool[channelTarget][subchannelTarget]; - for (const subchannelObj of subchannelObjArray) { - if ( - channelOptionsEqual( - channelArguments, - subchannelObj.channelArguments - ) && - channelCredentials._equals(subchannelObj.channelCredentials) - ) { - return subchannelObj.subchannel; - } + const subchannelObjArray = this.pool[channelTarget]; + for (const subchannelObj of subchannelObjArray) { + if ( + subchannelAddressEqual(subchannelTarget, subchannelObj.subchannelAddress) && + channelOptionsEqual( + channelArguments, + subchannelObj.channelArguments + ) && + channelCredentials._equals(subchannelObj.channelCredentials) + ) { + return subchannelObj.subchannel; } } } @@ -144,12 +139,10 @@ export class SubchannelPool { channelCredentials ); if (!(channelTarget in this.pool)) { - this.pool[channelTarget] = Object.create(null); + this.pool[channelTarget] = []; } - if (!(subchannelTarget in this.pool[channelTarget])) { - this.pool[channelTarget][subchannelTarget] = []; - } - this.pool[channelTarget][subchannelTarget].push({ + this.pool[channelTarget].push({ + subchannelAddress: subchannelTarget, channelArguments, channelCredentials, subchannel, diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index b6b282d5..2a123f04 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -20,12 +20,13 @@ import { ChannelCredentials } from './channel-credentials'; import { Metadata } from './metadata'; import { Http2CallStream } from './call-stream'; import { ChannelOptions } from './channel-options'; -import { PeerCertificate, checkServerIdentity } from 'tls'; +import { PeerCertificate, checkServerIdentity, TLSSocket } from 'tls'; import { ConnectivityState } from './channel'; import { BackoffTimeout, BackoffOptions } from './backoff-timeout'; import { getDefaultAuthority } from './resolver'; import * as logging from './logging'; import { LogVerbosity } from './constants'; +import { SocketConnectOpts } from 'net'; const { version: clientVersion } = require('../../package.json'); @@ -73,6 +74,22 @@ function uniformRandom(min: number, max: number) { const tooManyPingsData: Buffer = Buffer.from('too_many_pings', 'ascii'); +/** + * This represents a single backend address to connect to. This interface is a + * subset of net.SocketConnectOpts, i.e. the options described at + * https://nodejs.org/api/net.html#net_socket_connect_options_connectlistener. + * Those are in turn a subset of the options that can be passed to http2.connect. + */ +export interface SubchannelAddress { + port?: number; + host?: string; + path?: string; +} + +export function subchannelAddressEqual(address1: SubchannelAddress, address2: SubchannelAddress) : boolean { + return address1.port === address2.port && address1.host === address2.host && address1.path === address2.path; +} + export class Subchannel { /** * The subchannel's current connectivity state. Invariant: `session` === `null` @@ -135,6 +152,11 @@ export class Subchannel { */ private refcount = 0; + /** + * A string representation of the subchannel address, for logging/tracing + */ + private subchannelAddressString: string; + /** * A class representing a connection to a single backend. * @param channelTarget The target string for the channel as a whole @@ -147,7 +169,7 @@ export class Subchannel { */ constructor( private channelTarget: string, - private subchannelAddress: string, + private subchannelAddress: SubchannelAddress, private options: ChannelOptions, private credentials: ChannelCredentials ) { @@ -187,6 +209,11 @@ export class Subchannel { ); } }, backoffOptions); + if (subchannelAddress.host || subchannelAddress.port) { + this.subchannelAddressString = `${subchannelAddress.host}:${subchannelAddress.port}`; + } else { + this.subchannelAddressString = `${subchannelAddress.path}`; + } } /** @@ -225,10 +252,11 @@ export class Subchannel { } private startConnectingInternal() { - const connectionOptions: http2.SecureClientSessionOptions = + let connectionOptions: http2.SecureClientSessionOptions = this.credentials._getConnectionOptions() || {}; let addressScheme = 'http://'; if ('secureContext' in connectionOptions) { + connectionOptions.protocol = 'https:'; addressScheme = 'https://'; // If provided, the value of grpc.ssl_target_name_override should be used // to override the target hostname when checking server identity. @@ -248,8 +276,21 @@ export class Subchannel { connectionOptions.servername = getDefaultAuthority(this.channelTarget); } } + connectionOptions = Object.assign(connectionOptions, this.subchannelAddress); + /* http2.connect uses the options here: + * https://github.com/nodejs/node/blob/70c32a6d190e2b5d7b9ff9d5b6a459d14e8b7d59/lib/internal/http2/core.js#L3028-L3036 + * The spread operator overides earlier values with later ones, so any port + * or host values in the options will be used rather than any values extracted + * from the first argument. In addition, the path overrides the host and port, + * as documented for plaintext connections here: + * https://nodejs.org/api/net.html#net_socket_connect_options_connectlistener + * and for TLS connections here: + * https://nodejs.org/api/tls.html#tls_tls_connect_options_callback. + * The first argument just needs to be parseable as a URL and the scheme + * determines whether the connection will be established over TLS or not. + */ const session = http2.connect( - addressScheme + this.subchannelAddress, + addressScheme + getDefaultAuthority(this.channelTarget), connectionOptions ); this.session = session; @@ -328,7 +369,7 @@ export class Subchannel { return false; } trace( - this.subchannelAddress + + this.subchannelAddressString + ' ' + ConnectivityState[this.connectivityState] + ' -> ' + @@ -400,7 +441,7 @@ export class Subchannel { callRef() { trace( - this.subchannelAddress + + this.subchannelAddressString + ' callRefcount ' + this.callRefcount + ' -> ' + @@ -417,7 +458,7 @@ export class Subchannel { callUnref() { trace( - this.subchannelAddress + + this.subchannelAddressString + ' callRefcount ' + this.callRefcount + ' -> ' + @@ -435,7 +476,7 @@ export class Subchannel { ref() { trace( - this.subchannelAddress + + this.subchannelAddressString + ' callRefcount ' + this.refcount + ' -> ' + @@ -446,7 +487,7 @@ export class Subchannel { unref() { trace( - this.subchannelAddress + + this.subchannelAddressString + ' callRefcount ' + this.refcount + ' -> ' + @@ -557,6 +598,6 @@ export class Subchannel { } getAddress(): string { - return this.subchannelAddress; + return this.subchannelAddressString; } } diff --git a/packages/grpc-js/test/test-resolver.ts b/packages/grpc-js/test/test-resolver.ts index 951291a6..fc158510 100644 --- a/packages/grpc-js/test/test-resolver.ts +++ b/packages/grpc-js/test/test-resolver.ts @@ -21,6 +21,7 @@ import * as assert from 'assert'; import * as resolverManager from '../src/resolver'; import { ServiceConfig } from '../src/service-config'; import { StatusObject } from '../src/call-stream'; +import { SubchannelAddress } from '../src/subchannel'; describe('Name Resolver', () => { describe('DNS Names', function() { @@ -33,11 +34,11 @@ describe('Name Resolver', () => { const target = 'localhost:50051'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('127.0.0.1:50051')); + assert(addressList.some(addr => addr.host === '127.0.0.1' && addr.port === 50051)); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -52,11 +53,11 @@ describe('Name Resolver', () => { const target = 'localhost'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('127.0.0.1:443')); + assert(addressList.some(addr => addr.host === '127.0.0.1' && addr.port === 443)); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -71,11 +72,11 @@ describe('Name Resolver', () => { const target = '1.2.3.4'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('1.2.3.4:443')); + assert(addressList.some(addr => addr.host === '1.2.3.4' && addr.port === 443)); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -90,11 +91,11 @@ describe('Name Resolver', () => { const target = '::1'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('[::1]:443')); + assert(addressList.some(addr => addr.host === '::1' && addr.port === 443)); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -109,11 +110,11 @@ describe('Name Resolver', () => { const target = '[::1]:50051'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('[::1]:50051')); + assert(addressList.some(addr => addr.host === '::1' && addr.port === 50051)); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -128,7 +129,7 @@ describe('Name Resolver', () => { const target = 'example.com'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { @@ -146,7 +147,7 @@ describe('Name Resolver', () => { const target = 'loopback4.unittest.grpc.io'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { @@ -166,7 +167,7 @@ describe('Name Resolver', () => { const target = 'network-tools.com'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { @@ -196,7 +197,7 @@ describe('Name Resolver', () => { const target2 = 'grpc-test4.sandbox.googleapis.com'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { @@ -218,11 +219,11 @@ describe('Name Resolver', () => { const target = 'unix:socket'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('socket')); + assert(addressList.some(addr => addr.path = 'socket')); done(); }, onError: (error: StatusObject) => { @@ -236,11 +237,11 @@ describe('Name Resolver', () => { const target = 'unix:///tmp/socket'; const listener: resolverManager.ResolverListener = { onSuccessfulResolution: ( - addressList: string[], + addressList: SubchannelAddress[], serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.includes('/tmp/socket')); + assert(addressList.some(addr => addr.path = '/tmp/socket')); done(); }, onError: (error: StatusObject) => { From c5428c57330d6a5f52e3edd9600f021e5f158123 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jan 2020 09:56:49 -0800 Subject: [PATCH 2/5] lint and formatting fixes --- packages/grpc-js/src/channel-credentials.ts | 4 +-- packages/grpc-js/src/channel.ts | 4 ++- .../grpc-js/src/load-balancer-pick-first.ts | 6 +++- .../grpc-js/src/load-balancer-round-robin.ts | 6 +++- packages/grpc-js/src/resolver-dns.ts | 19 ++++++++---- packages/grpc-js/src/resolver-uds.ts | 2 +- packages/grpc-js/src/server-credentials.ts | 4 +-- packages/grpc-js/src/server.ts | 9 ++++-- packages/grpc-js/src/subchannel-pool.ts | 15 +++++++--- packages/grpc-js/src/subchannel.ts | 18 ++++++++--- packages/grpc-js/src/tls-helpers.ts | 5 ++-- packages/grpc-js/test/test-resolver.ts | 30 ++++++++++++++----- 12 files changed, 89 insertions(+), 33 deletions(-) diff --git a/packages/grpc-js/src/channel-credentials.ts b/packages/grpc-js/src/channel-credentials.ts index f42c7e93..5d5b87d5 100644 --- a/packages/grpc-js/src/channel-credentials.ts +++ b/packages/grpc-js/src/channel-credentials.ts @@ -18,7 +18,7 @@ import { ConnectionOptions, createSecureContext, PeerCertificate } from 'tls'; import { CallCredentials } from './call-credentials'; -import {CIPHER_SUITES, getDefaultRootsData} from './tls-helpers'; +import { CIPHER_SUITES, getDefaultRootsData } from './tls-helpers'; // tslint:disable-next-line:no-any function verifyIsBufferOrNull(obj: any, friendlyName: string): void { @@ -190,7 +190,7 @@ class SecureChannelCredentialsImpl extends ChannelCredentials { ca: rootCerts || undefined, key: privateKey || undefined, cert: certChain || undefined, - ciphers: CIPHER_SUITES + ciphers: CIPHER_SUITES, }); this.connectionOptions = { secureContext }; if (verifyOptions && verifyOptions.checkServerIdentity) { diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index a19cc217..d43da0e5 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -143,7 +143,9 @@ export class ChannelImplementation implements Channel { ) { /* The global boolean parameter to getSubchannelPool has the inverse meaning to what * the grpc.use_local_subchannel_pool channel option means. */ - this.subchannelPool = getSubchannelPool((options['grpc.use_local_subchannel_pool'] ?? 0) === 0); + this.subchannelPool = getSubchannelPool( + (options['grpc.use_local_subchannel_pool'] ?? 0) === 0 + ); const channelControlHelper: ChannelControlHelper = { createSubchannel: ( subchannelAddress: SubchannelAddress, diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts index fffb2a2a..52ce8457 100644 --- a/packages/grpc-js/src/load-balancer-pick-first.ts +++ b/packages/grpc-js/src/load-balancer-pick-first.ts @@ -30,7 +30,11 @@ import { UnavailablePicker, } from './picker'; import { LoadBalancingConfig } from './load-balancing-config'; -import { Subchannel, ConnectivityStateListener, SubchannelAddress } from './subchannel'; +import { + Subchannel, + ConnectivityStateListener, + SubchannelAddress, +} from './subchannel'; import * as logging from './logging'; import { LogVerbosity } from './constants'; diff --git a/packages/grpc-js/src/load-balancer-round-robin.ts b/packages/grpc-js/src/load-balancer-round-robin.ts index 2aeda430..4956e11e 100644 --- a/packages/grpc-js/src/load-balancer-round-robin.ts +++ b/packages/grpc-js/src/load-balancer-round-robin.ts @@ -30,7 +30,11 @@ import { UnavailablePicker, } from './picker'; import { LoadBalancingConfig } from './load-balancing-config'; -import { Subchannel, ConnectivityStateListener, SubchannelAddress } from './subchannel'; +import { + Subchannel, + ConnectivityStateListener, + SubchannelAddress, +} from './subchannel'; const TYPE_NAME = 'round_robin'; diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts index 9d357996..a5cd028a 100644 --- a/packages/grpc-js/src/resolver-dns.ts +++ b/packages/grpc-js/src/resolver-dns.ts @@ -131,7 +131,7 @@ function parseIP(target: string): SubchannelAddress[] | null { } else { port = DEFAULT_PORT; } - return [{host: addr, port: +port}]; + return [{ host: addr, port: +port }]; } /** @@ -223,8 +223,9 @@ class DnsResolver implements Resolver { this.pendingResultPromise.then( ([addressList, txtRecord]) => { this.pendingResultPromise = null; - const ip4Addresses: dns.LookupAddress[] = addressList - .filter(addr => addr.family === 4); + const ip4Addresses: dns.LookupAddress[] = addressList.filter( + addr => addr.family === 4 + ); let ip6Addresses: dns.LookupAddress[]; if (semver.satisfies(process.version, IPV6_SUPPORT_RANGE)) { ip6Addresses = addressList.filter(addr => addr.family === 6); @@ -234,10 +235,16 @@ class DnsResolver implements Resolver { const allAddresses: SubchannelAddress[] = mergeArrays( ip4Addresses, ip6Addresses - ).map(addr => {return {host: addr.address, port: +this.port!};}); - const allAddressesString: string = '[' + allAddresses.map(addr => addr.host + ':' + addr.port).join(',') + ']'; + ).map(addr => ({ host: addr.address, port: +this.port! })); + const allAddressesString: string = + '[' + + allAddresses.map(addr => addr.host + ':' + addr.port).join(',') + + ']'; trace( - 'Resolved addresses for target ' + this.target + ': ' + allAddressesString + 'Resolved addresses for target ' + + this.target + + ': ' + + allAddressesString ); if (allAddresses.length === 0) { this.listener.onError(this.defaultResolutionError); diff --git a/packages/grpc-js/src/resolver-uds.ts b/packages/grpc-js/src/resolver-uds.ts index 64c628f3..91128d2c 100644 --- a/packages/grpc-js/src/resolver-uds.ts +++ b/packages/grpc-js/src/resolver-uds.ts @@ -37,7 +37,7 @@ function getUdsName(target: string): string { class UdsResolver implements Resolver { private addresses: SubchannelAddress[] = []; constructor(target: string, private listener: ResolverListener) { - this.addresses = [{path: getUdsName(target)}]; + this.addresses = [{ path: getUdsName(target) }]; } updateResolution(): void { process.nextTick( diff --git a/packages/grpc-js/src/server-credentials.ts b/packages/grpc-js/src/server-credentials.ts index b56cb68a..17ab2980 100644 --- a/packages/grpc-js/src/server-credentials.ts +++ b/packages/grpc-js/src/server-credentials.ts @@ -16,7 +16,7 @@ */ import { SecureServerOptions } from 'http2'; -import {CIPHER_SUITES, getDefaultRootsData} from './tls-helpers'; +import { CIPHER_SUITES, getDefaultRootsData } from './tls-helpers'; export interface KeyCertPair { private_key: Buffer; @@ -75,7 +75,7 @@ export abstract class ServerCredentials { cert, key, requestCert: checkClientCertificate, - ciphers: CIPHER_SUITES + ciphers: CIPHER_SUITES, }); } } diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index 03fcfaf9..a2db30be 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -203,11 +203,16 @@ export class Server { const options: ListenOptions = { host: url.hostname, port: +url.port }; const serverOptions: http2.ServerOptions = {}; if ('grpc.max_concurrent_streams' in this.options) { - serverOptions.settings = {maxConcurrentStreams: this.options['grpc.max_concurrent_streams']}; + serverOptions.settings = { + maxConcurrentStreams: this.options['grpc.max_concurrent_streams'], + }; } if (creds._isSecure()) { - const secureServerOptions = Object.assign(serverOptions, creds._getSettings()!); + const secureServerOptions = Object.assign( + serverOptions, + creds._getSettings()! + ); this.http2Server = http2.createSecureServer(secureServerOptions); } else { this.http2Server = http2.createServer(serverOptions); diff --git a/packages/grpc-js/src/subchannel-pool.ts b/packages/grpc-js/src/subchannel-pool.ts index 3a49295b..61a4e13c 100644 --- a/packages/grpc-js/src/subchannel-pool.ts +++ b/packages/grpc-js/src/subchannel-pool.ts @@ -16,7 +16,11 @@ */ import { ChannelOptions, channelOptionsEqual } from './channel-options'; -import { Subchannel, SubchannelAddress, subchannelAddressEqual } from './subchannel'; +import { + Subchannel, + SubchannelAddress, + subchannelAddressEqual, +} from './subchannel'; import { ChannelCredentials } from './channel-credentials'; // 10 seconds in milliseconds. This value is arbitrary. @@ -72,8 +76,8 @@ export class SubchannelPool { } /* For each subchannel in the pool, try to unref it if it has - * exactly one ref (which is the ref from the pool itself). If that - * does happen, remove the subchannel from the pool */ + * exactly one ref (which is the ref from the pool itself). If that + * does happen, remove the subchannel from the pool */ this.pool[channelTarget] = refedSubchannels; } /* Currently we do not delete keys with empty values. If that results @@ -120,7 +124,10 @@ export class SubchannelPool { const subchannelObjArray = this.pool[channelTarget]; for (const subchannelObj of subchannelObjArray) { if ( - subchannelAddressEqual(subchannelTarget, subchannelObj.subchannelAddress) && + subchannelAddressEqual( + subchannelTarget, + subchannelObj.subchannelAddress + ) && channelOptionsEqual( channelArguments, subchannelObj.channelArguments diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 2a123f04..ae6f206b 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -86,8 +86,15 @@ export interface SubchannelAddress { path?: string; } -export function subchannelAddressEqual(address1: SubchannelAddress, address2: SubchannelAddress) : boolean { - return address1.port === address2.port && address1.host === address2.host && address1.path === address2.path; +export function subchannelAddressEqual( + address1: SubchannelAddress, + address2: SubchannelAddress +): boolean { + return ( + address1.port === address2.port && + address1.host === address2.host && + address1.path === address2.path + ); } export class Subchannel { @@ -194,7 +201,7 @@ export class Subchannel { clearTimeout(this.keepaliveTimeoutId); const backoffOptions: BackoffOptions = { initialDelay: options['grpc.initial_reconnect_backoff_ms'], - maxDelay: options['grpc.max_reconnect_backoff_ms'] + maxDelay: options['grpc.max_reconnect_backoff_ms'], }; this.backoffTimeout = new BackoffTimeout(() => { if (this.continueConnecting) { @@ -276,7 +283,10 @@ export class Subchannel { connectionOptions.servername = getDefaultAuthority(this.channelTarget); } } - connectionOptions = Object.assign(connectionOptions, this.subchannelAddress); + connectionOptions = Object.assign( + connectionOptions, + this.subchannelAddress + ); /* http2.connect uses the options here: * https://github.com/nodejs/node/blob/70c32a6d190e2b5d7b9ff9d5b6a459d14e8b7d59/lib/internal/http2/core.js#L3028-L3036 * The spread operator overides earlier values with later ones, so any port diff --git a/packages/grpc-js/src/tls-helpers.ts b/packages/grpc-js/src/tls-helpers.ts index 161666ed..3f7a62e7 100644 --- a/packages/grpc-js/src/tls-helpers.ts +++ b/packages/grpc-js/src/tls-helpers.ts @@ -17,7 +17,8 @@ import * as fs from 'fs'; -export const CIPHER_SUITES: string | undefined = process.env.GRPC_SSL_CIPHER_SUITES; +export const CIPHER_SUITES: string | undefined = + process.env.GRPC_SSL_CIPHER_SUITES; const DEFAULT_ROOTS_FILE_PATH = process.env.GRPC_DEFAULT_SSL_ROOTS_FILE_PATH; @@ -31,4 +32,4 @@ export function getDefaultRootsData(): Buffer | null { return defaultRootsData; } return null; -} \ No newline at end of file +} diff --git a/packages/grpc-js/test/test-resolver.ts b/packages/grpc-js/test/test-resolver.ts index fc158510..38ed5069 100644 --- a/packages/grpc-js/test/test-resolver.ts +++ b/packages/grpc-js/test/test-resolver.ts @@ -38,7 +38,11 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.host === '127.0.0.1' && addr.port === 50051)); + assert( + addressList.some( + addr => addr.host === '127.0.0.1' && addr.port === 50051 + ) + ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -57,7 +61,11 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.host === '127.0.0.1' && addr.port === 443)); + assert( + addressList.some( + addr => addr.host === '127.0.0.1' && addr.port === 443 + ) + ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -76,7 +84,11 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.host === '1.2.3.4' && addr.port === 443)); + assert( + addressList.some( + addr => addr.host === '1.2.3.4' && addr.port === 443 + ) + ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -95,7 +107,9 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.host === '::1' && addr.port === 443)); + assert( + addressList.some(addr => addr.host === '::1' && addr.port === 443) + ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -114,7 +128,9 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.host === '::1' && addr.port === 50051)); + assert( + addressList.some(addr => addr.host === '::1' && addr.port === 50051) + ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); }, @@ -223,7 +239,7 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.path = 'socket')); + assert(addressList.some(addr => (addr.path = 'socket'))); done(); }, onError: (error: StatusObject) => { @@ -241,7 +257,7 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => addr.path = '/tmp/socket')); + assert(addressList.some(addr => (addr.path = '/tmp/socket'))); done(); }, onError: (error: StatusObject) => { From 4f55a83b670c10507d73fb3d81efd65fc1028310 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jan 2020 10:21:47 -0800 Subject: [PATCH 3/5] Remove extraneous line of code --- packages/grpc-js/src/subchannel.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index ae6f206b..b03205c7 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -263,7 +263,6 @@ export class Subchannel { this.credentials._getConnectionOptions() || {}; let addressScheme = 'http://'; if ('secureContext' in connectionOptions) { - connectionOptions.protocol = 'https:'; addressScheme = 'https://'; // If provided, the value of grpc.ssl_target_name_override should be used // to override the target hostname when checking server identity. From 1fe6432d92d21459cc5699c4c305aff227f59b1f Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jan 2020 16:50:29 -0800 Subject: [PATCH 4/5] Differentiate more strongly between TCP and IPC addresses --- packages/grpc-js/src/resolver-dns.ts | 4 +-- packages/grpc-js/src/subchannel.ts | 44 +++++++++++++++++++------- packages/grpc-js/test/test-resolver.ts | 44 +++++++++++++++++++++----- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts index a5cd028a..6885f351 100644 --- a/packages/grpc-js/src/resolver-dns.ts +++ b/packages/grpc-js/src/resolver-dns.ts @@ -30,7 +30,7 @@ import { StatusObject } from './call-stream'; import { Metadata } from './metadata'; import * as logging from './logging'; import { LogVerbosity } from './constants'; -import { SubchannelAddress } from './subchannel'; +import { SubchannelAddress, TcpSubchannelAddress } from './subchannel'; const TRACER_NAME = 'dns_resolver'; @@ -232,7 +232,7 @@ class DnsResolver implements Resolver { } else { ip6Addresses = []; } - const allAddresses: SubchannelAddress[] = mergeArrays( + const allAddresses: TcpSubchannelAddress[] = mergeArrays( ip4Addresses, ip6Addresses ).map(addr => ({ host: addr.address, port: +this.port! })); diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index b03205c7..06f26892 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -26,7 +26,7 @@ import { BackoffTimeout, BackoffOptions } from './backoff-timeout'; import { getDefaultAuthority } from './resolver'; import * as logging from './logging'; import { LogVerbosity } from './constants'; -import { SocketConnectOpts } from 'net'; +import * as net from 'net'; const { version: clientVersion } = require('../../package.json'); @@ -74,27 +74,42 @@ function uniformRandom(min: number, max: number) { const tooManyPingsData: Buffer = Buffer.from('too_many_pings', 'ascii'); +export interface TcpSubchannelAddress { + port: number; + host: string; +} + +export interface IpcSubchannelAddress { + path: string; +} + /** * This represents a single backend address to connect to. This interface is a * subset of net.SocketConnectOpts, i.e. the options described at * https://nodejs.org/api/net.html#net_socket_connect_options_connectlistener. * Those are in turn a subset of the options that can be passed to http2.connect. */ -export interface SubchannelAddress { - port?: number; - host?: string; - path?: string; +export type SubchannelAddress = TcpSubchannelAddress | IpcSubchannelAddress; + +export function isTcpSubchannelAddress( + address: SubchannelAddress +): address is TcpSubchannelAddress { + return 'port' in address; } export function subchannelAddressEqual( address1: SubchannelAddress, address2: SubchannelAddress ): boolean { - return ( - address1.port === address2.port && - address1.host === address2.host && - address1.path === address2.path - ); + if (isTcpSubchannelAddress(address1)) { + return ( + isTcpSubchannelAddress(address2) && + address1.host === address2.host && + address1.port === address2.port + ); + } else { + return !isTcpSubchannelAddress(address2) && address1.path === address2.path; + } } export class Subchannel { @@ -216,7 +231,7 @@ export class Subchannel { ); } }, backoffOptions); - if (subchannelAddress.host || subchannelAddress.port) { + if (isTcpSubchannelAddress(subchannelAddress)) { this.subchannelAddressString = `${subchannelAddress.host}:${subchannelAddress.port}`; } else { this.subchannelAddressString = `${subchannelAddress.path}`; @@ -281,6 +296,13 @@ export class Subchannel { } else { connectionOptions.servername = getDefaultAuthority(this.channelTarget); } + } else { + connectionOptions.createConnection = (authority, option) => { + /* net.NetConnectOpts is declared in a way that is more restrictive + * than what net.connect will actually accept, so we use the type + * assertion to work around that. */ + return net.connect(this.subchannelAddress as net.NetConnectOpts); + }; } connectionOptions = Object.assign( connectionOptions, diff --git a/packages/grpc-js/test/test-resolver.ts b/packages/grpc-js/test/test-resolver.ts index 38ed5069..ebb4d567 100644 --- a/packages/grpc-js/test/test-resolver.ts +++ b/packages/grpc-js/test/test-resolver.ts @@ -21,7 +21,7 @@ import * as assert from 'assert'; import * as resolverManager from '../src/resolver'; import { ServiceConfig } from '../src/service-config'; import { StatusObject } from '../src/call-stream'; -import { SubchannelAddress } from '../src/subchannel'; +import { SubchannelAddress, isTcpSubchannelAddress } from '../src/subchannel'; describe('Name Resolver', () => { describe('DNS Names', function() { @@ -40,7 +40,10 @@ describe('Name Resolver', () => { ) => { assert( addressList.some( - addr => addr.host === '127.0.0.1' && addr.port === 50051 + addr => + isTcpSubchannelAddress(addr) && + addr.host === '127.0.0.1' && + addr.port === 50051 ) ); // We would check for the IPv6 address but it needs to be omitted on some Node versions @@ -63,7 +66,10 @@ describe('Name Resolver', () => { ) => { assert( addressList.some( - addr => addr.host === '127.0.0.1' && addr.port === 443 + addr => + isTcpSubchannelAddress(addr) && + addr.host === '127.0.0.1' && + addr.port === 443 ) ); // We would check for the IPv6 address but it needs to be omitted on some Node versions @@ -86,7 +92,10 @@ describe('Name Resolver', () => { ) => { assert( addressList.some( - addr => addr.host === '1.2.3.4' && addr.port === 443 + addr => + isTcpSubchannelAddress(addr) && + addr.host === '1.2.3.4' && + addr.port === 443 ) ); // We would check for the IPv6 address but it needs to be omitted on some Node versions @@ -108,7 +117,12 @@ describe('Name Resolver', () => { serviceConfigError: StatusObject | null ) => { assert( - addressList.some(addr => addr.host === '::1' && addr.port === 443) + addressList.some( + addr => + isTcpSubchannelAddress(addr) && + addr.host === '::1' && + addr.port === 443 + ) ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); @@ -129,7 +143,12 @@ describe('Name Resolver', () => { serviceConfigError: StatusObject | null ) => { assert( - addressList.some(addr => addr.host === '::1' && addr.port === 50051) + addressList.some( + addr => + isTcpSubchannelAddress(addr) && + addr.host === '::1' && + addr.port === 50051 + ) ); // We would check for the IPv6 address but it needs to be omitted on some Node versions done(); @@ -239,7 +258,11 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => (addr.path = 'socket'))); + assert( + addressList.some( + addr => !isTcpSubchannelAddress(addr) && addr.path === 'socket' + ) + ); done(); }, onError: (error: StatusObject) => { @@ -257,7 +280,12 @@ describe('Name Resolver', () => { serviceConfig: ServiceConfig | null, serviceConfigError: StatusObject | null ) => { - assert(addressList.some(addr => (addr.path = '/tmp/socket'))); + assert( + addressList.some( + addr => + !isTcpSubchannelAddress(addr) && addr.path === '/tmp/socket' + ) + ); done(); }, onError: (error: StatusObject) => { From 0995c9b0e6ad4c9feb459d4e8b76241333a1e919 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 29 Jan 2020 16:56:05 -0800 Subject: [PATCH 5/5] Update comment with new information --- packages/grpc-js/src/subchannel.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 06f26892..bb64bd93 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -297,6 +297,9 @@ export class Subchannel { connectionOptions.servername = getDefaultAuthority(this.channelTarget); } } else { + /* In all but the most recent versions of Node, http2.connect does not use + * the options when establishing plaintext connections, so we need to + * establish that connection explicitly. */ connectionOptions.createConnection = (authority, option) => { /* net.NetConnectOpts is declared in a way that is more restrictive * than what net.connect will actually accept, so we use the type @@ -316,7 +319,12 @@ export class Subchannel { * as documented for plaintext connections here: * https://nodejs.org/api/net.html#net_socket_connect_options_connectlistener * and for TLS connections here: - * https://nodejs.org/api/tls.html#tls_tls_connect_options_callback. + * https://nodejs.org/api/tls.html#tls_tls_connect_options_callback. In + * earlier versions of Node, http2.connect passes these options to + * tls.connect but not net.connect, so in the insecure case we still need + * to set the createConnection option above to create the connection + * explicitly. We cannot do that in the TLS case because http2.connect + * passes necessary additional options to tls.connect. * The first argument just needs to be parseable as a URL and the scheme * determines whether the connection will be established over TLS or not. */