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.

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

Update Autopull from ICANN JSON to address contract renewal noise #1806

Closed
dnsguru opened this issue Jul 20, 2023 · 20 comments
Closed

Update Autopull from ICANN JSON to address contract renewal noise #1806

dnsguru opened this issue Jul 20, 2023 · 20 comments
Assignees
Labels
✅ autopull Automation pull from Authoritative ICANN json source 🚩ICANN (IANA/ICP-3) Section PR changes in the ICANN/IANA section typically reserved for TLDs.

Comments

@dnsguru
Copy link
Member

dnsguru commented Jul 20, 2023

I have a (hopefully) fast and easy update to the autopull @cpu

Please See PR #1805 and look at the code change being automated.

The very first of 10 year contract renewals between registry operators and ICANN are being reflected in the json as a reset contract date, and this is causing autopull noise. Can the autopull have some logic that ignores a contract date change if there is no change in the sponsoring entity name?

We are now at one decade from some of the first contracts being signed between ICANN and the registry operators that applied for and successfully received delegations to operate registries that they applied for in the 2012 round of new gTLDs. So we'll have a lot of noise come through as all of these will have date changes as they renew contracts.

The 99.8% majority, but not all, of the new TLDs from the ICANN 2012 new gTLD round (give or take .web and/or some others that are taking longer to sort out stuff) have been delegated.

We'd want to keep the autopull going as it has been helpful for quickly catching changes in sponsoring organization per TLD or where there are applicants sunsetting their aspirations, but it is believed that at whatever time in the future that the next next round opens and delivers TLDs, that the current automation process would be able to handle it.

As a background, we used the contracting date to front-load the addition of TLDs before they were being added to the root by the IANA. The contracting date would precede whatever pre-delegation testing period that would occur before the TLD would be added to the root zone.

To address the time-lag for browsers' (and others') incorporation of the PSL as a static resource within code creating a situation where a gTLD would get added to the root but work on some browsers and not others based upon the timing of the last snapshot taken, there was a choice to use the date which the registry operator and ICANN had entered into a contract as a 'trajectory signal' - that the TLD would be being added to the root within a reasonably short timeframe.

We would still want that to be happening, but just want to make this little tweak on the date changes to mute a flood of autopulls that would have no practical functional value.

@dnsguru dnsguru added ✅ autopull Automation pull from Authoritative ICANN json source 🚩ICANN (IANA/ICP-3) Section PR changes in the ICANN/IANA section typically reserved for TLDs. labels Jul 20, 2023
@cpu
Copy link
Contributor

cpu commented Jul 20, 2023

Just wanting to note that I saw the tag here and will take a look when time permits. Thanks!

@dnsguru
Copy link
Member Author

dnsguru commented Jul 20, 2023

Thanks Daniel - hopefully this is trivial to ignore the DateOfContractSignature string if it is the only change in list/tools/newgtlds.go per entry.

I will leave the #1805 in an open state so that testing is possible, but I suspect we'll get daily nudges

@dnsguru
Copy link
Member Author

dnsguru commented Jul 24, 2023

ALTERNATIVE:
If the if adding in logic on the commented org changing or not TLD is complex, there is a brute force option here - I wonder if we just pull out the contract date altogether on these. Will make for one huge pull request because all nTLD will need an update to remove the dates across all the nTLDs, but subsequently we won't have week to week noise as the 2012-round nTLDs re-contract.

We'd included them in order for folks to track when they were introduced to the contracting while awaiting the full roster of 2012-round TLDs to hit the root zone due to the Root Zone Operators wanting a cadence not to exceed 20/week... we're well past all of the considerations and future proofing stuff that were cautiously added, and likely not to see more TLDs added until the next round might open up.

@dnsguru
Copy link
Member Author

dnsguru commented Jul 24, 2023

ADDITIONALLY:
I have contacted the CTO's office at ICANN to request that the contract renewal date be a separate field in the json file.

IF they make that change, we need do nothing but close the open pull request(s) that were date-only and the automation can remain as-is.

@cpu
Copy link
Contributor

cpu commented Jul 25, 2023

hopefully this is trivial to ignore the DateOfContractSignature string if it is the only change in list/tools/newgtlds.go per entry.

This is not impossible, but also not trivial for reason I previously called out in this comment: The existing tooling blindly replaces the whole section each time it runs. To be able to say "is the only thing that changed the DateOfContractSignature" it would need to actually parse the old content first.

Further, using github.com/weppos/publicsuffix-go to parse the PSL DAT as I suggested in #1325 won't help with this particular request because its representation (sensibly) does not include comments.

I wonder if we just pull out the contract date altogether on these.

Assuming I understand you correctly this would probably only require a small tweak to pslEntry.Comment to not render the contract date.

As a meta-comment: is there consensus from the other maintainers (is that just @weppos these days?) about the nature of this problem and the proposed solutions?

Perhaps I'm underestimating the scale of the "noise" but it seems relatively trivial to review the pull requests that are only updating the date of contract signature. That requires no code changes, no review of the new code, and would keep the contract date information available and accessible to consumers.

@dnsguru
Copy link
Member Author

dnsguru commented Jul 25, 2023 via email

@dnsguru
Copy link
Member Author

dnsguru commented Jul 25, 2023

What about just setting the contract date to the string "in contract " on line 121?

Or a blank string ""

@dnsguru
Copy link
Member Author

dnsguru commented Jul 26, 2023

@cpu I made a 'caveman' pull request - which would result in just putting the word "Contracted" in place of the contractdate field.

Temporary solution while I have a request in to ICANN.org technical services to see about them using a different field for renewals.

@weppos
Copy link
Member

weppos commented Aug 1, 2023

Alternative implementation #1812

@weppos
Copy link
Member

weppos commented Aug 1, 2023

I reviewed both PRs and I'm inclined to prefer #1812 approach to skip the value altogether. I can go ahead, merge it and monitor the processing.

Let me know if you are good with it @dnsguru .

@cpu thanks for finding the time to write the PR. I know you're not actively involved with LE anymore and you probably have better things to spend your time with. I value your time, and I really appreciate you invested some of it to help here this time. 🙏

@dnsguru
Copy link
Member Author

dnsguru commented Aug 2, 2023

In scrolling through and looking at the effect of this change and how the new format of entries appear in the context of everything else in the PSL, I wonder if removing the date was the right choice vs replacing it with the URL at IANA per TLD?

// aaa : 2015-02-26 American Automobile Association, Inc.
aaa

// aaa : American Automobile Association, Inc.
// https://www.iana.org/domains/root/db/aaa.html
aaa

Using existing variables, we could generate it into another comment line, and the result would be more consistent with the overall format, and be in accordance with the guidelines.

@dnsguru
Copy link
Member Author

dnsguru commented Aug 2, 2023

Also, I would like us to consider and implement the change in automation that adds the labelling that I'd proposed in PR #1815

@weppos
Copy link
Member

weppos commented Aug 3, 2023

@dnsguru to be clear, what you are suggesting regarding the template is:

  1. Remove the date (as already done)
  2. Add a second line to the template with the IANA url

Is it correct? if my understanding is correct, #1816 could be merged in the meanwhile as it does implement (1). I can then provide a follow up PR for (2).

Also, I would like us to consider and implement the change in automation that adds the labelling that I'd proposed in PR #1815

It must be me, but I've read it twice and I'm not sure I understood the second part of the request:

Did not see a way to also automate the project as well in the automation handler, but this will save volunteers steps in labelling when reviewing.

Is that part of the PR? Or would it be a "feature request" for a change. I guess I'm confused as that PR bring some changes, and then also brings requests for more changes.

@cpu
Copy link
Contributor

cpu commented Aug 3, 2023

  1. Add a second line to the template with the IANA url

#1817

@cpu
Copy link
Contributor

cpu commented Aug 3, 2023

I think in the future it'd be better to revert commits like d751aa5 instead of commenting out the code in follow-ups: 46fd989

It would also be prudent to run the unit tests for your changes (especially if avoiding pull requests). They would have caught this error before it was merged.

@dnsguru
Copy link
Member Author

dnsguru commented Aug 3, 2023 via email

@cpu
Copy link
Contributor

cpu commented Aug 3, 2023

Turns out a PR might not have caught the test failures anyway 😓 The workflow was only being triggered by push events, fix in #1818.

@dnsguru
Copy link
Member Author

dnsguru commented Aug 3, 2023 via email

@dnsguru
Copy link
Member Author

dnsguru commented Aug 4, 2023

@mozfreddyb This is a change to the comment lines for the nTLDs and the formatting that happens from the automation. It is anticipated that the impact for most all integrators of the PSL is going to be nil

@dnsguru
Copy link
Member Author

dnsguru commented Sep 14, 2023

closing this as I believe we have sorted out the concern about the impacts on the automation.

@dnsguru dnsguru closed this as completed Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✅ autopull Automation pull from Authoritative ICANN json source 🚩ICANN (IANA/ICP-3) Section PR changes in the ICANN/IANA section typically reserved for TLDs.
Projects
Status: Done or Won't
Development

No branches or pull requests

3 participants