-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(NODE-2939): add new hostname canonicalization opts #3131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
342e819
to
77a4f37
Compare
4fea7eb
to
48025aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaving some comments here because we pulled in more specs than just this ticket, so we should tag the corresponding tickets on this PR and then close them when this merges
ff3fc90
to
3c6d586
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make sure we rebase after #3138 goes in, because the credential option parsing will be affected
0979fbb
to
20c8d55
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@durran Thanks for updating the tests! Would it be worthwhile/possible to add the expectations on the fallback behavior at each point in each case? Just to make sure we don't accidentally break that in the future? Edit: if possible, we'd also want to confirm that the final response correctly uses the reply of the respective functions (like, we'd want to stub the reply from the reverse lookup to return something different than the forward, and then make sure we use the reverse response)
We actually didn't want to stub the later case, which is why I opened https://jira.mongodb.org/browse/BUILD-14646 to get an integration test for it afterwards. But I can write a test that stubs out all the return values in the meantime. |
1fe21b2
to
140c651
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - great job with the new tests!
Description
NODE-2939
Also syncs NODE-2443 and NODE-2563
Updates
CANONICALIZE_HOST_NAME
options to accepttrue
,false
, "none", "forward", "forwardAndReverse".What is changing?
When canonicalizing the host name when using GSSAPI, the driver will now behave as follows for these values:
true
or "forwardAndReverse": Performs a forward DNS lookup of the host and a reverse lookup of the IP address to obtain the hostname. If the reverse lookup fails the driver falls back to a cname lookup.false
or "none": Does no hostname canonicalization.When syncing the spec tests some other auth spec tests we hadn't synced yet were there, so I decided to leave them and skip the single failure.
Is there new documentation needed for these changes?
None
What is the motivation for this change?
DRIVERS-1803 / NODE-2939
Double check the following
npm run check:lint
script<type>(NODE-xxxx)<!>: <description>