Skip to content

NEW PROVIDER: Oracle Cloud#1021

Merged
tlimoncelli merged 7 commits intoDNSControl:masterfrom
kallsyms:master
Jan 24, 2021
Merged

NEW PROVIDER: Oracle Cloud#1021
tlimoncelli merged 7 commits intoDNSControl:masterfrom
kallsyms:master

Conversation

@kallsyms
Copy link
Copy Markdown
Contributor

This PR introduces a provider for Oracle Cloud DNS.

OCI DNS API Docs: https://docs.oracle.com/en-us/iaas/api/#/en/dns/20180115/

Few miscellaneous notes from development:

  • This currently marks DS records as not supported, despite Oracle saying they are. I'm getting 500s from the API when running the integration tests. I can investigate this further if wanted.
  • The API provides only full updates (i.e. replace the entire given rrset/fqdn/zone) or patch (specifying a batch of add/remove operations on the given rrset/rqdn/zone). This PR implements the provider with batched patches per zone.
  • The patch API doesn't provide a way to update a single existing record, so this implementation batches a remove then an add operation (mimicking how the console does it).

Addresses #419.

Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

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

Looks great so far! I will have a closer look in a few days.

don't worry about adding DS in a separate PR. It's common to do that. Get the base working well then augment it.

Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

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

Great work! There is very close to being ready to merge. I just had a few small changes.

By the way... in intergration_test.go, Oracle isn't running the pager601 tests. Since the API is slow, and since page size is 100, this test isn't needed. That said, it might be useful to run this test just once to verify functionality (but don't include it in the PR).

Comment thread providers/oracle/oracleProvider.go Outdated
Comment thread providers/oracle/oracleProvider.go Outdated
@kallsyms
Copy link
Copy Markdown
Contributor Author

Changes addressed! Just tried the pager601 test and it creates more than 100 records (so the batching works), but the test hits the 60s deadline before it finishes adding all of them.

Copy link
Copy Markdown
Contributor

@tlimoncelli tlimoncelli left a comment

Choose a reason for hiding this comment

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

Looks great! Super minor change and we're all done!

(thanks for running the pager601 test. Yeah, timeouts are expected. At least it worked up until the timeout!)

Comment thread providers/oracle/oracleProvider.go Outdated
Comment thread providers/oracle/oracleProvider.go Outdated
Comment thread providers/oracle/oracleProvider.go Outdated
@kallsyms
Copy link
Copy Markdown
Contributor Author

Done!

@tlimoncelli tlimoncelli merged commit 945ffb7 into DNSControl:master Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants