Skip to content

Commit

Permalink
Merge pull request #2776 from gkampitakis/gkampitakis/patch_dns_lookup
Browse files Browse the repository at this point in the history
fix: support authority overrides in the DNS resolver
  • Loading branch information
murgatroid99 committed Jul 10, 2024
2 parents da54e75 + 201595c commit f867643
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 24 deletions.
5 changes: 5 additions & 0 deletions doc/environment_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,8 @@ can be set.
- INFO - log INFO and ERROR message
- ERROR - log only errors (default)
- NONE - won't log any

* GRPC_NODE_USE_ALTERNATIVE_RESOLVER
Allows changing dns resolve behavior and parse DNS server authority as described in https://github.com/grpc/grpc/blob/master/doc/naming.md
- true - use alternative resolver
- false - use default resolver (default)
2 changes: 1 addition & 1 deletion gulpfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const nativeTestOnly = gulp.parallel(healthCheck.test);

const nativeTest = gulp.series(build, nativeTestOnly);

const testOnly = gulp.parallel(jsCore.test, nativeTestOnly, protobuf.test, jsXds.test, reflection.test);
const testOnly = gulp.series(jsCore.test, nativeTestOnly, protobuf.test, jsXds.test, reflection.test);

const test = gulp.series(build, testOnly, internalTest.test);

Expand Down
23 changes: 22 additions & 1 deletion packages/grpc-js/gulpfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,27 @@ const runTests = checkTask(() => {
);
});

const test = gulp.series(install, copyTestFixtures, runTests);
const testWithAlternativeResolver = checkTask(() => {
process.env.GRPC_NODE_USE_ALTERNATIVE_RESOLVER = 'true';
return gulp.src(`${outDir}/test/test-resolver.js`).pipe(
mocha({
reporter: 'mocha-jenkins-reporter',
require: ['ts-node/register'],
})
);
});

const resetEnv = () => {
process.env.GRPC_NODE_USE_ALTERNATIVE_RESOLVER = 'false';
return Promise.resolve();
};

const test = gulp.series(
install,
copyTestFixtures,
runTests,
testWithAlternativeResolver,
resetEnv
);

export { install, lint, clean, cleanAll, compile, test };
19 changes: 19 additions & 0 deletions packages/grpc-js/src/environment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright 2024 gRPC authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

export const GRPC_NODE_USE_ALTERNATIVE_RESOLVER =
(process.env.GRPC_NODE_USE_ALTERNATIVE_RESOLVER ?? 'false') === 'true';
73 changes: 54 additions & 19 deletions packages/grpc-js/src/resolver-dns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ import {
registerResolver,
registerDefaultScheme,
} from './resolver';
import * as dns from 'dns';
import * as util from 'util';
import { promises as dns } from 'node:dns';
import { extractAndSelectServiceConfig, ServiceConfig } from './service-config';
import { Status } from './constants';
import { StatusObject } from './call-interface';
Expand All @@ -33,6 +32,7 @@ import { GrpcUri, uriToString, splitHostPort } from './uri-parser';
import { isIPv6, isIPv4 } from 'net';
import { ChannelOptions } from './channel-options';
import { BackoffOptions, BackoffTimeout } from './backoff-timeout';
import { GRPC_NODE_USE_ALTERNATIVE_RESOLVER } from './environment';

const TRACER_NAME = 'dns_resolver';

Expand All @@ -47,9 +47,6 @@ export const DEFAULT_PORT = 443;

const DEFAULT_MIN_TIME_BETWEEN_RESOLUTIONS_MS = 30_000;

const resolveTxtPromise = util.promisify(dns.resolveTxt);
const dnsLookupPromise = util.promisify(dns.lookup);

/**
* Resolver implementation that handles DNS names and IP addresses.
*/
Expand All @@ -63,7 +60,7 @@ class DnsResolver implements Resolver {
* Failures are handled by the backoff timer.
*/
private readonly minTimeBetweenResolutionsMs: number;
private pendingLookupPromise: Promise<dns.LookupAddress[]> | null = null;
private pendingLookupPromise: Promise<TcpSubchannelAddress[]> | null = null;
private pendingTxtPromise: Promise<string[][]> | null = null;
private latestLookupResult: Endpoint[] | null = null;
private latestServiceConfig: ServiceConfig | null = null;
Expand All @@ -76,12 +73,17 @@ class DnsResolver implements Resolver {
private isNextResolutionTimerRunning = false;
private isServiceConfigEnabled = true;
private returnedIpResult = false;
private alternativeResolver = new dns.Resolver();

constructor(
private target: GrpcUri,
private listener: ResolverListener,
channelOptions: ChannelOptions
) {
trace('Resolver constructed for target ' + uriToString(target));
if (target.authority) {
this.alternativeResolver.setServers([target.authority]);
}
const hostPort = splitHostPort(target.path);
if (hostPort === null) {
this.ipResult = null;
Expand Down Expand Up @@ -185,11 +187,7 @@ class DnsResolver implements Resolver {
* revert to an effectively blank one. */
this.latestLookupResult = null;
const hostname: string = this.dnsHostname;
/* We lookup both address families here and then split them up later
* because when looking up a single family, dns.lookup outputs an error
* if the name exists but there are no records for that family, and that
* error is indistinguishable from other kinds of errors */
this.pendingLookupPromise = dnsLookupPromise(hostname, { all: true });
this.pendingLookupPromise = this.lookup(hostname);
this.pendingLookupPromise.then(
addressList => {
if (this.pendingLookupPromise === null) {
Expand All @@ -198,17 +196,12 @@ class DnsResolver implements Resolver {
this.pendingLookupPromise = null;
this.backoff.reset();
this.backoff.stop();
const subchannelAddresses: TcpSubchannelAddress[] = addressList.map(
addr => ({ host: addr.address, port: +this.port! })
);
this.latestLookupResult = subchannelAddresses.map(address => ({
this.latestLookupResult = addressList.map(address => ({
addresses: [address],
}));
const allAddressesString: string =
'[' +
subchannelAddresses
.map(addr => addr.host + ':' + addr.port)
.join(',') +
addressList.map(addr => addr.host + ':' + addr.port).join(',') +
']';
trace(
'Resolved addresses for target ' +
Expand Down Expand Up @@ -253,7 +246,7 @@ class DnsResolver implements Resolver {
/* We handle the TXT query promise differently than the others because
* the name resolution attempt as a whole is a success even if the TXT
* lookup fails */
this.pendingTxtPromise = resolveTxtPromise(hostname);
this.pendingTxtPromise = this.resolveTxt(hostname);
this.pendingTxtPromise.then(
txtRecord => {
if (this.pendingTxtPromise === null) {
Expand Down Expand Up @@ -302,6 +295,48 @@ class DnsResolver implements Resolver {
}
}

private async lookup(hostname: string): Promise<TcpSubchannelAddress[]> {
if (GRPC_NODE_USE_ALTERNATIVE_RESOLVER) {
trace('Using alternative DNS resolver.');

const records = await Promise.allSettled([
this.alternativeResolver.resolve4(hostname),
this.alternativeResolver.resolve6(hostname),
]);

if (records.every(result => result.status === 'rejected')) {
throw new Error((records[0] as PromiseRejectedResult).reason);
}

return records
.reduce<string[]>((acc, result) => {
return result.status === 'fulfilled'
? [...acc, ...result.value]
: acc;
}, [])
.map(addr => ({
host: addr,
port: +this.port!,
}));
}

/* We lookup both address families here and then split them up later
* because when looking up a single family, dns.lookup outputs an error
* if the name exists but there are no records for that family, and that
* error is indistinguishable from other kinds of errors */
const addressList = await dns.lookup(hostname, { all: true });
return addressList.map(addr => ({ host: addr.address, port: +this.port! }));
}

private async resolveTxt(hostname: string): Promise<string[][]> {
if (GRPC_NODE_USE_ALTERNATIVE_RESOLVER) {
trace('Using alternative DNS resolver.');
return this.alternativeResolver.resolveTxt(hostname);
}

return dns.resolveTxt(hostname);
}

private startNextResolutionTimer() {
clearTimeout(this.nextResolutionTimer);
this.nextResolutionTimer = setTimeout(() => {
Expand Down
16 changes: 13 additions & 3 deletions packages/grpc-js/test/test-resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
subchannelAddressEqual,
} from '../src/subchannel-address';
import { parseUri, GrpcUri } from '../src/uri-parser';
import { GRPC_NODE_USE_ALTERNATIVE_RESOLVER } from '../src/environment';

function hasMatchingAddress(
endpointList: Endpoint[],
Expand All @@ -55,7 +56,10 @@ describe('Name Resolver', () => {
describe('DNS Names', function () {
// For some reason DNS queries sometimes take a long time on Windows
this.timeout(4000);
it('Should resolve localhost properly', done => {
it('Should resolve localhost properly', function (done) {
if (GRPC_NODE_USE_ALTERNATIVE_RESOLVER) {
this.skip();
}
const target = resolverManager.mapUriDefaultScheme(
parseUri('localhost:50051')!
)!;
Expand All @@ -82,7 +86,10 @@ describe('Name Resolver', () => {
const resolver = resolverManager.createResolver(target, listener, {});
resolver.updateResolution();
});
it('Should default to port 443', done => {
it('Should default to port 443', function (done) {
if (GRPC_NODE_USE_ALTERNATIVE_RESOLVER) {
this.skip();
}
const target = resolverManager.mapUriDefaultScheme(
parseUri('localhost')!
)!;
Expand Down Expand Up @@ -402,7 +409,10 @@ describe('Name Resolver', () => {
const resolver2 = resolverManager.createResolver(target2, listener, {});
resolver2.updateResolution();
});
it('should not keep repeating successful resolutions', done => {
it('should not keep repeating successful resolutions', function (done) {
if (GRPC_NODE_USE_ALTERNATIVE_RESOLVER) {
this.skip();
}
const target = resolverManager.mapUriDefaultScheme(
parseUri('localhost')!
)!;
Expand Down

0 comments on commit f867643

Please sign in to comment.