Skip to content

Add an "about" link to registrars in RDAP #2771

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

Merged
merged 1 commit into from
Jul 8, 2025

Conversation

gbrodman
Copy link
Collaborator

@gbrodman gbrodman commented Jun 12, 2025

From the response profile:
2.4.6. Registrar URL - The entity with the registrar role in the RDAP response MUST contain a links member [RFC9083]. The links object MUST contain the elements: value, identical to the the RDAP Base URL for the Registrar as provided in the IANA “Registrar IDs” registry (i.e., https://www.iana.org/assignments/registrar-ids); rel:about, and href containing the Registrar URL. Note: in cases where the Registry Operator acts as sponsoring Registrar (e.g., IANA Registrar ID 9999), the href shall contain a URL from the Registry.


This change is Reviewable

@gbrodman gbrodman force-pushed the rdapRegistrar branch 3 times, most recently from 5d11f3e to 7b5399c Compare June 13, 2025 15:40
@gbrodman gbrodman requested a review from CydeWeys June 13, 2025 15:42
@gbrodman
Copy link
Collaborator Author

@CydeWeys friendly ping

gbrodman added a commit to gbrodman/nomulus that referenced this pull request Jun 25, 2025
This corresponds to the Feb 2024 response profile section 1.2 and
implementation guide 1.3 respectively, now that we comply (or are, at
least closer to complying), with the Feb 2024 versions.

This should probably depend on google#2771

because that includes a small change included in the Feb 2024 version
gbrodman added a commit to gbrodman/nomulus that referenced this pull request Jun 25, 2025
This corresponds to the Feb 2024 response profile section 1.2 and
implementation guide 1.3 respectively, now that we comply (or are, at
least closer to complying), with the Feb 2024 versions.

This should probably depend on google#2771

because that includes a small change included in the Feb 2024 version
@gbrodman gbrodman requested a review from weiminyu June 26, 2025 21:00
gbrodman added a commit to gbrodman/nomulus that referenced this pull request Jun 26, 2025
This corresponds to the Feb 2024 response profile section 1.2 and
implementation guide 1.3 respectively, now that we comply (or are, at
least closer to complying), with the Feb 2024 versions.

This should probably depend on google#2771
because that includes a small change included in the Feb 2024 version

This also updates the documentation to reference the proper areas of the
specifications.
Copy link
Collaborator

@weiminyu weiminyu left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 37 files reviewed, 2 unresolved discussions (waiting on @CydeWeys)


core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java line 83 at r1 (raw file):

  private static void processAllRegistrars(ImmutableMap<String, String> ianaIdsToUrls) {
    int nonUpdatedRegistrars = 0;
    // Split into multiple transactions to avoid load-save-reload conflicts. Re-building a registrar

Not clear what this means. I don't see reloads in the code flow.

Code quote:

load-save-reload

core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java line 85 at r1 (raw file):

    // Split into multiple transactions to avoid load-save-reload conflicts. Re-building a registrar
    // requires a full (cached) load of all registrars to avoid IANA ID conflicts.
    Iterable<Registrar> registrars = tm().transact(Registrar::loadAll);

Use transact(REPEATABLE_READ, callable) can reduce race.

Code quote:

tm().transact(

From the response profile:
2.4.6. Registrar URL - The entity with the registrar role in the RDAP response
MUST contain a links member [RFC9083]. The links object MUST contain
the elements: value, identical to the the RDAP Base URL for the
Registrar as provided in the IANA “Registrar IDs” registry (i.e.,
https://www.iana.org/assignments/registrar-ids); rel:about, and href
containing the Registrar URL. Note: in cases where the Registry Operator
acts as sponsoring Registrar (e.g., IANA Registrar ID 9999), the href shall
contain a URL from the Registry.
Copy link
Collaborator Author

@gbrodman gbrodman left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 37 files reviewed, 2 unresolved discussions (waiting on @CydeWeys and @weiminyu)


core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java line 83 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

Not clear what this means. I don't see reloads in the code flow.

not directly, but as the comment notes, the Registrar.Builder::build method requires reloading all registrars to avoid IANA ID conflicts.

So, in the previous all-in-one-transaction code path, it throws an error if more than one registrar is modified -- because the firstly-changed registrar is reloaded in the "build" method of the secondly-changed registrar.

We never saw this error in practice because we never encountered the case where more than one registrar URL changed at a time.


core/src/main/java/google/registry/rdap/UpdateRegistrarRdapBaseUrlsAction.java line 85 at r1 (raw file):

Previously, weiminyu (Weimin Yu) wrote…

Use transact(REPEATABLE_READ, callable) can reduce race.

sure

Copy link
Collaborator

@weiminyu weiminyu left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 37 files reviewed, 1 unresolved discussion (waiting on @CydeWeys)

@gbrodman gbrodman enabled auto-merge July 8, 2025 14:16
@weiminyu weiminyu self-requested a review July 8, 2025 14:52
Copy link
Collaborator

@weiminyu weiminyu left a comment

Choose a reason for hiding this comment

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

Reviewed 35 of 37 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @CydeWeys)

@gbrodman gbrodman added this pull request to the merge queue Jul 8, 2025
Merged via the queue into google:master with commit f34aec8 Jul 8, 2025
9 checks passed
@gbrodman gbrodman deleted the rdapRegistrar branch July 8, 2025 15:45
gbrodman added a commit to gbrodman/nomulus that referenced this pull request Jul 8, 2025
This corresponds to the Feb 2024 response profile section 1.2 and
implementation guide 1.3 respectively, now that we comply (or are, at
least closer to complying), with the Feb 2024 versions.

This should probably depend on google#2771
because that includes a small change included in the Feb 2024 version

This also updates the documentation to reference the proper areas of the
specifications.
github-merge-queue bot pushed a commit that referenced this pull request Jul 9, 2025
This corresponds to the Feb 2024 response profile section 1.2 and
implementation guide 1.3 respectively, now that we comply (or are, at
least closer to complying), with the Feb 2024 versions.

This should probably depend on #2771
because that includes a small change included in the Feb 2024 version

This also updates the documentation to reference the proper areas of the
specifications.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants