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

NS1: Generate resource record ids locally #148

Merged
merged 8 commits into from Dec 3, 2019
Merged

NS1: Generate resource record ids locally #148

merged 8 commits into from Dec 3, 2019

Conversation

cttttt
Copy link
Contributor

@cttttt cttttt commented Nov 22, 2019

Problem

Our current approach for fetching the resource records in a zone requires:

  • A GET on the target zone. The response contains a summary of the resource record sets contained in that zone.
    • These summaries missing ids for individual resource records.
  • A GET on each resource recordset in the target zone to reveal resource records with id's.
  • All of these fetches are rate limited at 3rps.
  • Problem Every record present in NS1 managed zones will slow down subsequent syncs by 0.3 seconds.

How does this help?

  • We require resource records to be individually id'ed so we can compute zone updates in a provider agnostic way, then perform the actual updates across all providers using this id.
  • As a workaround, this change will have record_store compute these id s as a digest of the details of each resource record. Currently, the resource record set id, the resource record type and rrdata are used in the digest. Since these id's are only used within each record_store session, we can improve the digest in the future if there are any problems.

- Makes it a little easier to focus in on certain tests.
- Encapsulate enough details on all forms of an answer form the NS1 API,
  so that we can get IDs in a uniform way from both full answers (from a
  `/zone/domain/type` call) and from short answers (from a
  `/zone/domain` call).
lib/record_store/provider/ns1.rb Outdated Show resolved Hide resolved
lib/record_store/provider/ns1.rb Outdated Show resolved Hide resolved
lib/record_store/provider/ns1.rb Outdated Show resolved Hide resolved
lib/record_store/provider/ns1.rb Outdated Show resolved Hide resolved
lib/record_store/provider/ns1.rb Show resolved Hide resolved
lib/record_store/provider/ns1.rb Outdated Show resolved Hide resolved
lib/record_store/provider/ns1.rb Show resolved Hide resolved
Where possible, encapsulate answers from the NS1 API under our new
abstraction, and fetch fields (like `id`) through this abstraction.
Answer ids just need to be comparable, so marshalling them just wastes
time.
Tempted to make this a linter rule, but I'd be here all day.
Copy link
Contributor

@XaF XaF left a comment

Choose a reason for hiding this comment

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

Just a small comment, but otherwise LGTM

lib/record_store/provider/ns1.rb Outdated Show resolved Hide resolved
@cttttt
Copy link
Contributor Author

cttttt commented Nov 26, 2019

Bumped version, and rotated my API key (whoops). Will ship at my next opportunity.

@cttttt cttttt merged commit 041d362 into master Dec 3, 2019
@cttttt cttttt deleted the made-up-ids branch December 3, 2019 21:21
@cttttt cttttt temporarily deployed to production December 4, 2019 14:07 Inactive
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.

None yet

2 participants