-
Notifications
You must be signed in to change notification settings - Fork 285
Introduce PowerDnsWriter for zone synchronization to PowerDNS #2764
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
base: master
Are you sure you want to change the base?
Conversation
Create a PowerDNS writer
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.
Reviewed 4 of 31 files at r1.
Reviewable status: 4 of 31 files reviewed, 6 unresolved discussions
core/src/main/java/google/registry/dns/writer/powerdns/resources/openAPI.yaml
line 1398 at r1 (raw file):
result: type: string description: 'A message about the result like "Flushed cache"'
Missing a trailing newline.
Please run gradlew spotlessApply
to update this and other files.
core/src/main/java/google/registry/dns/writer/powerdns/client/PowerDNSClient.java
line 47 at r1 (raw file):
* <p>The API key is retrieved from the environment variable {@code POWERDNS_API_KEY}. */ public class PowerDNSClient {
Can visibility be reduced to package-protected?
Code quote:
public
core/src/main/java/google/registry/dns/writer/powerdns/client/PowerDNSClient.java
line 98 at r1 (raw file):
if (requestBody != null) requestBody.writeTo(buffer); else return ""; return buffer.readUtf8();
Our style guide requires braces. Please run gradlew :core:checkStyleMain
.
Code quote:
if (requestBody != null) requestBody.writeTo(buffer);
else return "";
return buffer.readUtf8();
core/src/main/java/google/registry/module/RegistryComponent.java
line 66 at r1 (raw file):
CloudTasksUtilsModule.class, ConfigModule.class, PowerDnsConfigModule.class,
Please pull from upstream and update the new DnsWritersModule
introduced in pr 2769.
core/src/main/java/google/registry/config/RegistryConfig.java
line 95 at r1 (raw file):
* and those values merged into the POJO. */ static RegistryConfigSettings getConfigSettings() {
Please keep getConfigSettings
as a shorthand to the new method using default files.
Code quote:
getConfigSettings() {
core/src/main/java/google/registry/dns/writer/powerdns/PowerDnsWriter.java
line 52 at r1 (raw file):
* This request is then converted into a PowerDNS Zone object and sent to the PowerDNS API. */ public class PowerDnsWriter extends DnsUpdateWriter {
Is package-protected access sufficient?
Code quote:
public
Update configuration mgmt from review comments
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.
Reviewable status: 3 of 31 files reviewed, 6 unresolved discussions (waiting on @weiminyu)
core/src/main/java/google/registry/config/RegistryConfig.java
line 95 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Please keep
getConfigSettings
as a shorthand to the new method using default files.
Done 👍
core/src/main/java/google/registry/dns/writer/powerdns/PowerDnsWriter.java
line 52 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Is package-protected access sufficient?
I followed the same pattern with public
scope as the other DNS writer implementations. Since they're all public, I made this one public as well.
core/src/main/java/google/registry/dns/writer/powerdns/client/PowerDNSClient.java
line 47 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Can visibility be reduced to package-protected?
No, compilation fails if public
is changed to protected
core/src/main/java/google/registry/dns/writer/powerdns/client/PowerDNSClient.java
line 98 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Our style guide requires braces. Please run
gradlew :core:checkStyleMain
.
Done 👍
core/src/main/java/google/registry/dns/writer/powerdns/resources/openAPI.yaml
line 1398 at r1 (raw file):
Previously, weiminyu (Weimin Yu) wrote…
Missing a trailing newline.
Please run
gradlew spotlessApply
to update this and other files.
Done 👍
@weiminyu general question, how do I handle CLA for Co-Pilot? I accepted a suggestion in my branch and now I'm failing the CLA check. I tried rebasing to remove the offending commit, but it still seems to persist in the commit history and fails the CLA check 🙈 ![]() |
Sync from upstream google/nomulus parent
Previously, weiminyu (Weimin Yu) wrote…
Thanks for refactoring those module imports. Pulled from upstream and adjusted the PowerDNS modules. |
We plan on using this for EPP password resets and registry lock password resets for now.
This avoids potential replication lag issues when requesting info on domains that were just created.
* Remove registrar id from invoice grouping key * Fix formatting issues * Update BillingEventTests
This is just so that we can add an additional layer of security on verification
If contacts are optional, they should be optional in the command too.
The Cloud DNS rest api is now case-sensitive about enum names (must be lower case, counterintuitively).
We can probably improve on this in the future if we want, but there's a lot of boilerplate that we don't need to repeat over and over
This picks up a few changes including aligning the placement of quotes in text blocks with the Google style guide.
Add a new DnsWritersModule for use by the component classes. To override the set of writers installed, we can easily overwrite this file with a private version.
This implements two type of changes: 1. changing the link type for things like the terms of service 2. adding the request URL to each and every link with the "value" field. This is a bit tricky to implement because the links are generated in various places, but we can implement it by adding it to the results after generation. See b/418782147 for more information
It was making the undo nomulus command look like this: )nomulus ...
A future PR will add the actions that save and use this object. That future PR will also require loading RegistrarPoc objects given the registrar ID, hence the change in that class.
Pubapi actions should always use cache, regardless of the config settings on caching. In EppResource.java, the original `loadCached(Iterable<VKey>)` method is renamed to `loadByCacheIfEnabled`. The original `loadCached(Vkey)` method is renamed to `loadByCache` and always uses cache. In EppResourceUtils.java, the original `loadByForeignKeyCached` method is renamed to `loadByForeignKeyByCacheIfEnabled`. A new `loadByForeignKeyByCache` method, which always uses cache. In ForeighKeyUtils.java, the original `loadCached` method is renamed to `loadByCacheIfEnabled`, and a new `loadCached` method is added which always uses cache. Also added a `getContactsFromReplica` method in Registrar, for use by RDAP actions.
Scripts needed by cron jobs wrongly removed by PR 2661. TESTED: in crash.
In RDAP, domain queries are the most common by a factor of like 40,000 so we should optimize these as much as possible. We already have an EPP resource / foreign key cache which does improve performance somewhat but looking at some sample logs, it only cuts the RDAP request times by like 40% (looking at requests for the same domain a few seconds apart). History entries don't change often, so we should cache them to make subsequent queries faster as well. In addition, we're only caching two fields per repo ID (modification time, registrar ID) so we can cache more entries than we can for the EPP resource cache (which stores large objects).
…mmit Remove commit from co-pilot due to conflict with google repo requirements
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.
I've never encountered this situation. Did you do merge
with the main branch? Now your commits are intermingled with submitted commits in main, and the offending commit e0d63e
is placed earlier than some main commits. I can't figure out anyway to modify it. You
We typically create a new branch for a PR, and rebase
with the main branch after pulling the latest to main.
This way all new commits are placed after submitted commits, and in most cases we can squash commits.
Reviewable status: 3 of 35 files reviewed, 1 unresolved discussion
core/src/main/java/google/registry/config/RegistryConfig.java
line 106 at r3 (raw file):
* be changed to public scope if needed by other packages. */ private static RegistryConfigSettings getConfigSettings() {
Please make this method as public, so that tools don't flag it.
We have some code on the side that calls this method.
Code quote:
private
Introduce a new DNS writer package that can forward zone changes to a PowerDNS backend. Leverages the PowerDNS API to manage records, with full support for DNSSEC and TSIG key management.
An older version of this PR has been closed by the author.
Background
At Unstoppable Domains we use PowerDNS as the backend for DNS zones and DNSSEC management. We've created a Nomulus plugin to facilitate synchronization with PowerDNS, that we believe would be useful for the Nomulus community at large.
Usage
In order to use the new PowerDnsWriter, a TLD must be created with a
PowerDnsWriter
specified in thednsWriters
stanza of the TLD configuration yaml.The PowerDNS config can be modified in the main config package within the
files/power-dns
directory. Environmental overrides are handled modularly for PowerDNS similar to the main config. For example, the default config shows the available options for the PowerDNS settings, which can be customized per environment using one of thefiles/power-dns/env-*
files. The config structure is show below for reference:DNSSEC
If enabled, the PowerDnsWriter will ensure a TLD is properly configured with a KSK and ZSK and use them to sign zone records. The ZSK will be automatically rotated on a monthly basis. The Nomulus registry operator must still take the manual step to publish the KSK
DS
record to the parent root server(s) to establish a chain of trust. TheDS
value(s) to publish are available in the Nomulus GAE logs, or can be retrieved from the PowerDNS command line.TSIG
If enabled, the PowerDnsWriter will establish a TSIG configuration for each managed TLD to facilitate secure zone replication. If zone replication is desired, the TSIG key needs to be manually retrieved from the PowerDNS command line and shared with the secondary DNS server.
PowerDNS configuration
Consult the PowerDNS documentation to setup your own instance. The key requirement is to ensure the PowerDNS API server is enabled and accessible from your Nomulus environment.
SSL
SSL connections are not directly supported from PowerDNS. As a security best practice, we suggest using Nginx to terminate SSL and proxy requests to the PowerDNS backend.
Private VPC
Due to the sensitive nature of the PowerDNS API, we suggest using a private VPC that is shared between the PowerDNS host and the Nomulus instance. This network pattern allows Nomulus to communicate with the PowerDNS API while shielding it from the public Internet traffic.
This change is