search

go-acme / lego

Let's Encrypt/ACME client and library written in Go
https://go-acme.github.io/lego/
MIT License
7.44k stars 987 forks source link

pdns: reconstruct zone URLs to enable non-root folder API endpoints #2141

Closed jotasi closed 1 month ago

jotasi commented 3 months ago

As I was trying to both better understand the behavior and create a patch for myself that makes lego work for my DNS provider's API, I have implemented a fix for #2128 even as the discussion in the issue has not yet progressed any further. If you require further discussion before this PR should be considered, please let me know.

This corresponds to the first of the two solutions, that I've proposed in #2128. It replaces the usage of the (at least for my provider) absolute URL zone.URL, which resulted in incorrect URLs being used when the API endpoint is not located at the server's root, by relative URLs. AFAICS, these should now work in all cases (both if the API endpoint is located at the server's root and if it isn't).

The two added test cases for non-root API endpoints also demonstrate the issue (if you add them without the changes to the code, the tests fail).

The constructed URLs reflect the URLs as they are documented in the PDNS API documentation for the update for a specific zone and for notifying for a specific zone. The description in the API is not 100% unambiguous as {zone_id} is not explicitly defined as to be the same as the Zone's ID returned by the API for that zone (as "id"). But both conceptually (as the zone's ID's main feature is URL-embedability) and from looking at the PDNS code (at least to the level that I could dig through it without spending a lot of time on it), this should be the case. If this is true, I don't see a downside of constructing the URLs in this way for existing use cases and it enables mine.

Sadly, I don't have a PDNS installation to play with and test this further (and also no time to set one up for this). Therefore, I cannot verify myself that it properly works in all cases except for the automated tests and my considerations listed above.

Fixes #2128

jotasi commented 3 months ago

I rebased onto the current master to keep the branch up to date. I'll do so every once in a while. If this is not helpful for your review, please let me know.

ldez commented 3 months ago

You don't need to rebase unless there are conflicts.