Skip to content
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.

Sign up for GitHub

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

Jump to bottom

DRIVERS-2922: Allow valid SRV hostnames with fewer than 3 parts #1628

Merged
merged 28 commits into from
Sep 24, 2024
Merged

DRIVERS-2922: Allow valid SRV hostnames with fewer than 3 parts #1628

merged 28 commits into from
Sep 24, 2024

Conversation

aditi-khare-mongoDB
Copy link
Contributor

@aditi-khare-mongoDB aditi-khare-mongoDB commented Aug 8, 2024 •
edited
Loading

Description

SRVs are no longer required to have a given number of parts, URI terminology is more clearly defined, and stricter parent match requirements are introduced.

What is changing?

  1. Update SRV URI Validation Requirement such that needing 3 . separated parts in the hostname is no longer required.
  2. Clarify that 'hostname' refers to the entire {hostname} and that the {subdomain} is only part of the hostname. Previously, in the document, {hostname} referred to the {subdomain} and 'hostname' referred to the {subdomain}.{domainname} which was confusing
  3. Update the parent domain match requirement for the returned address such that SRVS with <3 parts MUST end with all parts of the SRV and have an additional domain level
  4. Update the change log to reflect all of the above changes.

Proof of Concept

See here for Node's PoC/implementation of downstream changes.

Please complete the following before merging:

  • Update changelog.
  • Make sure there are generated JSON files from the YAML test files.
  • Test changes in at least one language driver.
  • Test these changes against all server versions and topologies (including standalone, replica set, sharded
    clusters, and serverless).

@aditi-khare-mongoDB aditi-khare-mongoDB force-pushed the drivers-2922/uri-validation branch from d110745 to a663ea3 Compare August 8, 2024 21:04
@aditi-khare-mongoDB aditi-khare-mongoDB force-pushed the drivers-2922/uri-validation branch from 67e06f9 to f27364c Compare August 8, 2024 21:10
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title feat(DRIVERS-2922): loosen options parser restrictions DRIVERS-2922: Loosen options parser URI restrictions Aug 8, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Loosen options parser URI restrictions DRIVERS-2922: Loosen URI Validation Requirements Aug 8, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Loosen URI Validation Requirements DRIVERS-2922: Correct URI Validation Requirements Aug 9, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Correct URI Validation Requirements DRIVERS-2922: Correct SRV URI Validation Requirements Aug 13, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB force-pushed the drivers-2922/uri-validation branch from 608f76e to f27364c Compare August 14, 2024 15:07
@aditi-khare-mongoDB aditi-khare-mongoDB marked this pull request as ready for review August 14, 2024 15:29
@aditi-khare-mongoDB aditi-khare-mongoDB requested a review from a team as a code owner August 14, 2024 15:29
@aditi-khare-mongoDB aditi-khare-mongoDB requested review from dariakp and nbbeeken and removed request for a team August 14, 2024 15:29
@nbbeeken nbbeeken mentioned this pull request Aug 15, 2024
Merged
5 tasks
nbbeeken
nbbeeken requested changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start, there's a markdown format linter error and see comments:

source/initial-dns-seedlist-discovery/initial-dns-seedlist-discovery.md Outdated Show resolved Hide resolved
source/initial-dns-seedlist-discovery/initial-dns-seedlist-discovery.md Outdated Show resolved Hide resolved
source/initial-dns-seedlist-discovery/initial-dns-seedlist-discovery.md Outdated Show resolved Hide resolved
nbbeeken
nbbeeken requested changes Aug 19, 2024
View reviewed changes
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A driver MUST verify that the host names returned through SRV records have the same parent {domainname}. Drivers MUST
raise an error and MUST NOT initiate a connection to any returned host name which does not share the same
{domainname}.

This section needs adjusting to include what validation is done when there is only one level in the domain.

@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Correct SRV URI Validation Requirements DRIVERS-2922: Allow valid SRV hostnames with less than 3 parts Aug 20, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Allow valid SRV hostnames with less than 3 parts DRIVERS-2922: Allow Valid SRV Hostnames With Less Than 3 Parts Aug 20, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Allow Valid SRV Hostnames With Less Than 3 Parts DRIVERS-2922: Allow valid SRV hostnames with less than 3 parts Aug 21, 2024
ShaneHarvey
ShaneHarvey reviewed Sep 18, 2024
View reviewed changes
Copy link
Member

@ShaneHarvey ShaneHarvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pre-commit is failing: https://github.com/mongodb/specifications/actions/runs/10885366203/job/30202807636

source/initial-dns-seedlist-discovery/initial-dns-seedlist-discovery.md Show resolved Hide resolved
@aditi-khare-mongoDB aditi-khare-mongoDB changed the title DRIVERS-2922: Allow valid SRV hostnames with less than 3 parts DRIVERS-2922: Allow valid SRV hostnames with fewer than 3 parts Sep 24, 2024
@aditi-khare-mongoDB aditi-khare-mongoDB merged commit 8e9d768 into master Sep 24, 2024
4 checks passed
@aditi-khare-mongoDB aditi-khare-mongoDB deleted the drivers-2922/uri-validation branch September 24, 2024 20:51
isabelatkinson pushed a commit to isabelatkinson/specifications that referenced this pull request Sep 25, 2024
@aditi-khare-mongoDB @isabelatkinson
DRIVERS-2922: Allow valid SRV hostnames with fewer than 3 parts (mong…
10146d2 
…odb#1628)

* feat(DRIVERS-2922): loosen options parser restrictions

lint

clarify

fix capitalization

* add test criteria

* grammar fix

* wording fix

* temp commit - changing terminology

* change terminology

* update changelog

* add in prose test ref

* add parent matching requirements

* update changelod

* added in new prose test requirements + fixed formatting

* requested changes

* requested changes 2

* uniform formatting + fix typo

* team review requested changes

* team review requested changes

* team review requested changes 2

* update deprecation comment

* typo

* clarify subdomain

* add in Shanes test

* update changelog date

* add in specific cases

* fix test cases

* fix tests

* fix tests

* grammar fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dariakp dariakp dariakp approved these changes

@nbbeeken nbbeeken nbbeeken approved these changes

@ShaneHarvey ShaneHarvey ShaneHarvey approved these changes

@spencerjackson spencerjackson spencerjackson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aditi-khare-mongoDB @ShaneHarvey @spencerjackson @nbbeeken @dariakp