Skip to content

Commit 4b7feb3

Browse files
durrantadjik1PavelSafronovCopilot
authored
refactor(NODE-7313): only use dns.resolve for all types (#4846)
Co-authored-by: Sergey Zelenov <sergey.zelenov@mongodb.com> Co-authored-by: Pavel Safronov <pavel.safronov@mongodb.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 275afa5 commit 4b7feb3

File tree

10 files changed

+152
-163
lines changed

10 files changed

+152
-163
lines changed

‎src/cmap/auth/gssapi.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ export async function performGSSAPICanonicalizeHostName(
169169

170170
try {
171171
// Perform a reverse ptr lookup on the ip address.
172-
const results = await dns.promises.resolvePtr(address);
172+
const results = await dns.promises.resolve(address, 'PTR');
173173
// If the ptr did not error but had no results, return the host.
174174
return results.length > 0 ? results[0] : host;
175175
} catch {
@@ -188,7 +188,7 @@ export async function performGSSAPICanonicalizeHostName(
188188
export async function resolveCname(host: string): Promise<string> {
189189
// Attempt to resolve the host name
190190
try {
191-
const results = await dns.promises.resolveCname(host);
191+
const results = await dns.promises.resolve(host, 'CNAME');
192192
// Get the first resolved host id
193193
return results.length > 0 ? results[0] : host;
194194
} catch {

‎src/connection_string.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,26 +42,30 @@ const LB_REPLICA_SET_ERROR = 'loadBalanced option not supported with a replicaSe
4242
const LB_DIRECT_CONNECTION_ERROR =
4343
'loadBalanced option not supported when directConnection is provided';
4444

45-
function retryDNSTimeoutFor(api: 'resolveSrv'): (a: string) => Promise<dns.SrvRecord[]>;
46-
function retryDNSTimeoutFor(api: 'resolveTxt'): (a: string) => Promise<string[][]>;
45+
function retryDNSTimeoutFor(rrtype: 'SRV'): (lookupAddress: string) => Promise<dns.SrvRecord[]>;
46+
function retryDNSTimeoutFor(rrtype: 'TXT'): (lookupAddress: string) => Promise<string[][]>;
4747
function retryDNSTimeoutFor(
48-
api: 'resolveSrv' | 'resolveTxt'
49-
): (a: string) => Promise<dns.SrvRecord[] | string[][]> {
48+
rrtype: 'SRV' | 'TXT'
49+
): (lookupAddress: string) => Promise<dns.SrvRecord[] | string[][]> {
50+
const resolve =
51+
rrtype === 'SRV'
52+
? (address: string) => dns.promises.resolve(address, 'SRV')
53+
: (address: string) => dns.promises.resolve(address, 'TXT');
5054
return async function dnsReqRetryTimeout(lookupAddress: string) {
5155
try {
52-
return await dns.promises[api](lookupAddress);
56+
return await resolve(lookupAddress);
5357
} catch (firstDNSError) {
5458
if (firstDNSError.code === dns.TIMEOUT) {
55-
return await dns.promises[api](lookupAddress);
59+
return await resolve(lookupAddress);
5660
} else {
5761
throw firstDNSError;
5862
}
5963
}
6064
};
6165
}
6266

63-
const resolveSrv = retryDNSTimeoutFor('resolveSrv');
64-
const resolveTxt = retryDNSTimeoutFor('resolveTxt');
67+
const resolveSrv = retryDNSTimeoutFor('SRV');
68+
const resolveTxt = retryDNSTimeoutFor('TXT');
6569

6670
/**
6771
* Lookup a `mongodb+srv` connection string, combine the parts and reparse it as a normal

‎src/sdam/srv_polling.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export class SrvPoller extends TypedEventEmitter<SrvPollerEvents> {
116116
let srvRecords;
117117

118118
try {
119-
srvRecords = await dns.promises.resolveSrv(this.srvAddress);
119+
srvRecords = await dns.promises.resolve(this.srvAddress, 'SRV');
120120
} catch {
121121
this.failure();
122122
return;

‎test/integration/initial-dns-seedlist-discovery/dns_seedlist.test.ts‎

Lines changed: 27 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ describe('DNS timeout errors', () => {
2323

2424
beforeEach(async function () {
2525
client = new MongoClient(CONNECTION_STRING, { serverSelectionTimeoutMS: 2000, tls: false });
26+
stub = sinon.stub(dns.promises, 'resolve').callThrough();
2627
});
2728

2829
afterEach(async function () {
@@ -31,134 +32,108 @@ describe('DNS timeout errors', () => {
3132
await client.close();
3233
});
3334

34-
const restoreDNS =
35-
api =>
36-
async (...args) => {
37-
sinon.restore();
38-
return await dns.promises[api](...args);
39-
};
35+
const restoreDNS = rrtype => async hostname => {
36+
stub.restore();
37+
return await dns.promises.resolve(hostname, rrtype);
38+
};
4039

4140
describe('when SRV record look up times out', () => {
4241
beforeEach(() => {
43-
stub = sinon
44-
.stub(dns.promises, 'resolveSrv')
42+
stub
43+
.withArgs(sinon.match.string, 'SRV')
4544
.onFirstCall()
4645
.rejects(new DNSTimeoutError())
4746
.onSecondCall()
48-
.callsFake(restoreDNS('resolveSrv'));
49-
});
50-
51-
afterEach(async function () {
52-
sinon.restore();
47+
.callsFake(restoreDNS('SRV'));
5348
});
5449

5550
it('retries timeout error', metadata, async () => {
5651
await client.connect();
57-
expect(stub).to.have.been.calledTwice;
52+
expect(stub.withArgs(sinon.match.string, 'SRV')).to.have.been.calledTwice;
5853
});
5954
});
6055

6156
describe('when TXT record look up times out', () => {
6257
beforeEach(() => {
63-
stub = sinon
64-
.stub(dns.promises, 'resolveTxt')
58+
stub
59+
.withArgs(sinon.match.string, 'TXT')
6560
.onFirstCall()
6661
.rejects(new DNSTimeoutError())
6762
.onSecondCall()
68-
.callsFake(restoreDNS('resolveTxt'));
69-
});
70-
71-
afterEach(async function () {
72-
sinon.restore();
63+
.callsFake(restoreDNS('TXT'));
7364
});
7465

7566
it('retries timeout error', metadata, async () => {
7667
await client.connect();
77-
expect(stub).to.have.been.calledTwice;
68+
expect(stub.withArgs(sinon.match.string, 'TXT')).to.have.been.calledTwice;
7869
});
7970
});
8071

8172
describe('when SRV record look up times out twice', () => {
8273
beforeEach(() => {
83-
stub = sinon
84-
.stub(dns.promises, 'resolveSrv')
74+
stub
75+
.withArgs(sinon.match.string, 'SRV')
8576
.onFirstCall()
8677
.rejects(new DNSTimeoutError())
8778
.onSecondCall()
8879
.rejects(new DNSTimeoutError());
8980
});
9081

91-
afterEach(async function () {
92-
sinon.restore();
93-
});
94-
9582
it('throws timeout error', metadata, async () => {
9683
const error = await client.connect().catch(error => error);
9784
expect(error).to.be.instanceOf(DNSTimeoutError);
98-
expect(stub).to.have.been.calledTwice;
85+
expect(stub.withArgs(sinon.match.string, 'SRV')).to.have.been.calledTwice;
9986
});
10087
});
10188

10289
describe('when TXT record look up times out twice', () => {
10390
beforeEach(() => {
104-
stub = sinon
105-
.stub(dns.promises, 'resolveTxt')
91+
stub
92+
.withArgs(sinon.match.string, 'TXT')
10693
.onFirstCall()
10794
.rejects(new DNSTimeoutError())
10895
.onSecondCall()
10996
.rejects(new DNSTimeoutError());
11097
});
11198

112-
afterEach(async function () {
113-
sinon.restore();
114-
});
115-
11699
it('throws timeout error', metadata, async () => {
117100
const error = await client.connect().catch(error => error);
118101
expect(error).to.be.instanceOf(DNSTimeoutError);
119-
expect(stub).to.have.been.calledTwice;
102+
expect(stub.withArgs(sinon.match.string, 'TXT')).to.have.been.calledTwice;
120103
});
121104
});
122105

123106
describe('when SRV record look up throws a non-timeout error', () => {
124107
beforeEach(() => {
125-
stub = sinon
126-
.stub(dns.promises, 'resolveSrv')
108+
stub
109+
.withArgs(sinon.match.string, 'SRV')
127110
.onFirstCall()
128111
.rejects(new DNSSomethingError())
129112
.onSecondCall()
130-
.callsFake(restoreDNS('resolveSrv'));
131-
});
132-
133-
afterEach(async function () {
134-
sinon.restore();
113+
.callsFake(restoreDNS('SRV'));
135114
});
136115

137116
it('throws that error', metadata, async () => {
138117
const error = await client.connect().catch(error => error);
139118
expect(error).to.be.instanceOf(DNSSomethingError);
140-
expect(stub).to.have.been.calledOnce;
119+
expect(stub.withArgs(sinon.match.string, 'SRV')).to.have.been.calledOnce;
141120
});
142121
});
143122

144123
describe('when TXT record look up throws a non-timeout error', () => {
145124
beforeEach(() => {
146-
stub = sinon
147-
.stub(dns.promises, 'resolveTxt')
125+
stub
126+
.withArgs(sinon.match.string, 'TXT')
148127
.onFirstCall()
149128
.rejects(new DNSSomethingError())
150129
.onSecondCall()
151-
.callsFake(restoreDNS('resolveTxt'));
152-
});
153-
154-
afterEach(async function () {
155-
sinon.restore();
130+
.callsFake(restoreDNS('TXT'));
156131
});
157132

158133
it('throws that error', metadata, async () => {
159134
const error = await client.connect().catch(error => error);
160135
expect(error).to.be.instanceOf(DNSSomethingError);
161-
expect(stub).to.have.been.calledOnce;
136+
expect(stub.withArgs(sinon.match.string, 'TXT')).to.have.been.calledOnce;
162137
});
163138
});
164139
});

‎test/integration/initial-dns-seedlist-discovery/initial_dns_seedlist_discovery.prose.test.ts‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
1919
beforeEach(async function () {
2020
// this fn stubs DNS resolution to always pass - so we are only checking pre-DNS validation
2121

22-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
22+
const stub = sinon.stub(dns.promises, 'resolve');
23+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
2324
return [
2425
{
2526
name: 'resolved.mongo.localhost',
@@ -30,7 +31,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
3031
];
3132
});
3233

33-
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
34+
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
3435
throw { code: 'ENODATA' };
3536
});
3637

@@ -80,9 +81,11 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
8081
* - the SRV mongodb+srv://blogs.mongodb.com resolving to blogs.evil.com
8182
* Remember, the domain of an SRV with one or two . separated parts is the SRVs entire hostname.
8283
*/
84+
let stub;
8385

8486
beforeEach(async function () {
85-
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
87+
stub = sinon.stub(dns.promises, 'resolve');
88+
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
8689
throw { code: 'ENODATA' };
8790
});
8891
});
@@ -92,7 +95,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
9295
});
9396

9497
it('an SRV with one domain level causes a runtime error', async function () {
95-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
98+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
9699
return [
97100
{
98101
name: 'localhost.mongodb',
@@ -111,7 +114,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
111114
});
112115

113116
it('an SRV with two domain levels causes a runtime error', async function () {
114-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
117+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
115118
return [
116119
{
117120
name: 'test_1.evil.local', // this string only ends with part of the domain, not all of it!
@@ -130,7 +133,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
130133
});
131134

132135
it('an SRV with three or more domain levels causes a runtime error', async function () {
133-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
136+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
134137
return [
135138
{
136139
name: 'blogs.evil.com',
@@ -162,8 +165,11 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
162165
context(
163166
'when given a host from DNS resolution that is identical to the original SRVs hostname',
164167
function () {
168+
let stub;
169+
165170
beforeEach(async function () {
166-
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
171+
stub = sinon.stub(dns.promises, 'resolve');
172+
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
167173
throw { code: 'ENODATA' };
168174
});
169175
});
@@ -173,7 +179,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
173179
});
174180

175181
it('an SRV with one domain level causes a runtime error', async function () {
176-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
182+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
177183
return [
178184
{
179185
name: 'localhost',
@@ -194,7 +200,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
194200
});
195201

196202
it('an SRV with two domain levels causes a runtime error', async function () {
197-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
203+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
198204
return [
199205
{
200206
name: 'mongo.local',
@@ -229,8 +235,11 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
229235
context(
230236
'when given a returned address that does NOT share the domain name of the SRV record because its missing a `.`',
231237
function () {
238+
let stub;
239+
232240
beforeEach(async function () {
233-
sinon.stub(dns.promises, 'resolveTxt').callsFake(async () => {
241+
stub = sinon.stub(dns.promises, 'resolve');
242+
stub.withArgs(sinon.match.string, 'TXT').callsFake(async () => {
234243
throw { code: 'ENODATA' };
235244
});
236245
});
@@ -240,7 +249,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
240249
});
241250

242251
it('an SRV with one domain level causes a runtime error', async function () {
243-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
252+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
244253
return [
245254
{
246255
name: 'test_1.cluster_1localhost',
@@ -259,7 +268,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
259268
});
260269

261270
it('an SRV with two domain levels causes a runtime error', async function () {
262-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
271+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
263272
return [
264273
{
265274
name: 'test_1.my_hostmongo.local',
@@ -278,7 +287,7 @@ describe('Initial DNS Seedlist Discovery (Prose Tests)', () => {
278287
});
279288

280289
it('an SRV with three domain levels causes a runtime error', async function () {
281-
sinon.stub(dns.promises, 'resolveSrv').callsFake(async () => {
290+
stub.withArgs(sinon.match.string, 'SRV').callsFake(async () => {
282291
return [
283292
{
284293
name: 'cluster.testmongodb.com',

0 commit comments

Comments
 (0)