PowerDNS dns provider.

[kylejohnson]

Thank you for contributing to sewer.
Every contribution to sewer is important to us.
You may not know it, but you have just contributed to making the world a more safer and secure place.

What(What have you changed?)

Added a dns provider for the PowerDNS API.

Why(Why did you change it?)

So that DNS authentication can be performed against a PowerDNS server.

[codecov]

Codecov Report

Merging #147 into master will increase coverage by 0.30%.
The diff coverage is 93.33%.

@@            Coverage Diff             @@
##           master     #147      +/-   ##
==========================================
+ Coverage   86.53%   86.84%   +0.30%     
==========================================
  Files          14       15       +1     
  Lines         943      988      +45     
==========================================
+ Hits          816      858      +42     
- Misses        127      130       +3     

Impacted Files	Coverage Δ
sewer/dns_providers/powerdns.py	93.33% <93.33%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbee725...7fb1603. Read the comment docs.

[mmaney]

@kylejohnson I've had a need to know what the domain.tld of the fqdn that's passed to the provider class is in a still-private hack for updating an acme-validation subzone (challenges are in host.acme-validation.domain.tld linked by static cnames in the apex zone). One reason that provider is still private code is that it currently has the domain.tld hardwired. :-(

Would having that apex zone name passed to the dns code help with PowerDNS, or is that just one part of the search you have to do? (or @anyone, would it be useful for other dns_providers?)

[kylejohnson]

@mmaney

Would having that apex zone name passed to the dns code help with PowerDNS

Yes, but I don't know that we ever really know the apex domain; you can request a certificate for any number of subdomains without also requesting for the apex domain, and you can also request certificates for multiple, separate apex domains (I think?).

Either way, PowerDNS makes it very easy to check if a given zone_id exists, and the validate_powerdns_zone function takes care of the rest. I imagine the code would not be applicable with different providers.

[kylejohnson]

So obviously that doesn't look good, but it looks like the testing for sewer isn't so great to begin with?

[mmaney]

@kylejohnson Ah, I see, you're using a part of the PowerDNS system to find the apex for each request. I don't know if there was anything like that that I could use, but in any case I was coming from the perspective that I had a bunch of fqdns to validate (and they did share an apex, whether that was one of the fqdns or not), and of course I knew - the literal me setting up and running the requests - what the apex was. If I'd had something like that virtual URI tree (?) that this driver queries... nah, at this point it would still be hard-wired, but I might be thinking of finding it in a like manner as a way forward.

Thanks for the food for thought!

[komuw]

@kylejohnson do you mind splitting this into two different PR's.

• one where you add support for powerDNS
• another for the refactoring that you have done

[komuw]

@kylejohnson tests are failing

[komuw]

note to self; related: #144

[kylejohnson]

@komuw Sorry, didn't see the other refactoring changes included. This PR now only has the powerdns changes.

[kylejohnson]

@komuw Tests for my changes look OK, I think.
Locally, they seem to run OK:

kjohnson@kyle-workstation01:~/work/sewer$ python -m unittest sewer.dns_providers.tests.test_powerdns.TestPowerDNS
...
----------------------------------------------------------------------
Ran 3 tests in 0.001s

OK

[komuw]

@kylejohnson

You need to add powerdns to:

1. setup.py; somewhere here:

   sewer/setup.py

   Line 29 in ee0f259

   "duckdns": [""],

   
   and also here:

   sewer/setup.py

   Line 111 in ee0f259

   "duckdns": dns_provider_deps_map["duckdns"],

2. https://github.com/komuw/sewer/blob/master/sewer/cli.py somewhere here:

   sewer/sewer/cli.py

   Line 74 in ee0f259

   "cloudns",

   
   and somehere here:

   sewer/sewer/cli.py

   Lines 300 to 311 in ee0f259

   	elif dns_provider == "cloudns":
   	from . import ClouDNSDns
   	try:
   	dns_class = ClouDNSDns()
   	logger.info("chosen_dns_provider. Using {0} as dns provider.".format(dns_provider))
   	except KeyError as e:
   	logger.error("ERROR:: Please supply {0} as an environment variable.".format(str(e)))
   	raise
   	else:
   	raise ValueError("The dns provider {0} is not recognised.".format(dns_provider))

3. README.md somewhere here: https://github.com/komuw/sewer/blame/master/README.md#L26
   and somewhere here: https://github.com/komuw/sewer/blame/master/README.md#L70-L71

[komuw]

@kylejohnson the CI(circleCI) is failing because this PR is going to reduce the test code coverage to less than 85%.

As you can see, this is because the file sewer/dns_providers/powerdns.py only has a test coverage of 29%.
Compare this to the file, sewer/dns_providers/route53.py which has a test coverage of 89%.

The main lines in sewer/dns_providers/powerdns.py that are missing test coverage are line 55 to line 91:

sewer/sewer/dns_providers/powerdns.py

Lines 55 to 91 in 7fbcbcc

	if changetype not in ("REPLACE", "DELETE"):
	raise ValueError("changetype is not valid.")
	payload = {
	"rrsets": [
	{
	"name": "_acme-challenge" + "." + domain_name + ".",
	"type": "TXT",
	"ttl": 60,
	"changetype": changetype,
	"records": [{"content": f'"{domain_dns_value}"', "disabled": False}],
	}
	]
	}
	self.logger.debug(f"PowerDNS domain name: {domain_name}")
	self.logger.debug(f"PowerDNS payload: {payload}")
	apex_domain = self.validate_powerdns_zone(domain_name)
	url = self.powerdns_api_url + "/" + apex_domain
	self.logger.debug(f"apex_domain: {apex_domain}")
	self.logger.debug(f"url: {url}")
	try:
	response = requests.patch(
	url, data=json.dumps(payload), headers={"X-API-Key": self.powerdns_api_key}
	)
	self.logger.debug(f"PowerDNS response: {response.status_code}, {response.text}")
	except Exception as e:
	self.logger.error(f"Unable to communicate with PowerDNS API: {e}")
	raise e
	# Per https://doc.powerdns.com/authoritative/http-api/zone.html:
	# PATCH /servers/{server_id}/zones/{zone_id}
	# Creates/modifies/deletes RRsets present in the payload and their comments.
	# Returns 204 No Content on success.
	if response.status_code != 204:
	raise ValueError(f"Error creating or deleting PowerDNS record: {response.text}")

So it seems like adding a test(unit test or otherwise) for the method _common_dns_record would help to restore the test code coverage

[komuw]

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- sewer/dns_providers/powerdns.py  4
- sewer/dns_providers/tests/test_powerdns.py  2
         

See the complete overview on Codacy

[kylejohnson]

@komuw Thank you for walking me through this.

[mmaney]

@kylejohnson I'm trying to sort out how best to solve #162, and powerdns is one of two providers that do not strip the "*." from wildcard domains. Is that required for powerdns to work, or is it an oversight that will cause it to fail, like route53 does in that bug?

If powerdns doesn't need it, then the obvious fix is to not add it in sewer, and then the other providers can be cleaned up at our leisure. If powerdns does need the star, then... Well, it's easy enough to patch route53 to strip it, as most of the providers probably do. But I really hope we can clean this up instead.

Thanks!

[kylejohnson]

@mmaney Hmm, I didn't explicitly run across this in my tests, integrations or usage. In fact, I haven't used sewer with a wildcard cert yet.

That said, it looks like powerdns is not happy:
"message": "Error generating certificate - Error creating or deleting PowerDNS record: {\"error\": \"Name '_acme-challenge.*.domain.tld.' contains unsupported characters\"}"

It looks like validate_powerdns_zone works correctly as it is able to walk backwards from *.domain.tld to find domain.tld as the apex zone.

I'll need to dig a little bit more and see what the exact query that powerdns.py is sending to the powerdns API, because for example I do have existing CNAME records for widlcard domains.

(Note that to produce the above error, I tried using a wildcard alt domain, e.g. domain_name=domain, domain_alt_names=[f"*.{domain}"])

[mmaney]

Yes, that's the case which seems to trip up a couple of service providers - when a single cert asks to cover the domain.tld and all subdomains *.domain.tld ... which is probably fairly common since many sites want www.d.t and naked d.t to refer to the same web site.

[josemaia]

Hello guys, when can I expect there to be a new release of sewer containing the powerDNS provider? I implemented this provider manually in my project using sewer but I'd rather have a better-tested version!

[mmaney]

Have you been following the master branch here @josemaia? I've been working on reducing my "future" branch to provide the changes that still need to go in for the next 0.8 release, and I hope to have a PR (perhaps lacking documentation updates that also need to go in) later today. That will have a couple of the recent DNS providers as well as the protocol updates, and should be worthy of your attention.

And after that I expect 0.8 will get bugfixes, maybe some new provider modules, but not the less compatible changes I've been working on in my local tree. That stuff will go into a 0.9 branch that will be more experimental for a while until it's ready for release.

Oops, almost lost my question for you: you're hoping for a better-tested what, exactly? Since the protocol changes went in the ACME code in client has been able to be exercised against LE's staging and has been tested that way. And getting that working again since they dropped support for the earlier draft protocol in staging, hence being ready for production to drop it later this year, has been one of my goals this year. As for the powerdns driver, or most any of the providers, the only real-world testing they can get is from people like you, who have access to the service provider's API. Rejoice, for powerdns has at least two such users now (that I know of)! :-)

[josemaia]

By better-tested I simply meant using Kyle's code instead of my own homebrewed version, haha. Thank you for the update, looking forward to it.