-
-
Notifications
You must be signed in to change notification settings - Fork 612
iana: Embed & parse reserved IP registries from primary source #8249
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
Move `policy.IsReservedIP` to `iana.IsReservedAddr`. Move `policy.IsReservedPrefix` to `iana.IsReservedPrefix`. Embed & parse IANA's special-purpose address registries for IPv4 and IPv6 in their original CSV format. Fixes #8080
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.
nit: this probably shouldn't close 8080, since it doesn't auto-update our copy of the registry yet. Let's either do that here, or leave 8080 open and leave a TODO in the action yml file.
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.
The structure of the update job looks good, but workflows are hard to review. Can you push this branch to your personal fork of the boulder repo, make the checked-in files out-of-date, and then run the workflow and link to the resulting run and PR here?
Sure thing! I had to modify the one line that sets the PR reviewer. Here are the results:
I did one test run before remembering to make the checked-in files out of date and, delightfully, caught a real correction that IANA must have made within the past few days. |
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.
A really neat change that I had a lot of fun reading through. Just some small comments, but otherwise I think it looks great!
iana/ip.go
Outdated
whitespacesRE := regexp.MustCompile(`\s{2,}`) | ||
// Footnotes at the end, like `[2]`. | ||
footnotesRE := regexp.MustCompile(`\[\d+\]$`) |
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.
We typically declare these outside of a function so that they fail at compilation time.
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.
Note that this doesn't make a difference in this case.
Putting them outside the function doesn't cause them to fail at compilation time, rather it causes them to fail at package loading time, i.e. startup time. But this function is called from init()
, so a failure here would also be caught at startup time.
Move
policy.IsReservedIP
toiana.IsReservedAddr
.Move
policy.IsReservedPrefix
toiana.IsReservedPrefix
.Embed & parse IANA's special-purpose address registries for IPv4 and IPv6 in their original CSV format.
Fixes #8080