pdns: reconstruct zone URLs to enable non-root folder API endpoints #2141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jotasi
force-pushed
the
feature/enable_pdns_rooted_at_path
branch
from
b54178a to
df7dfff
Compare
|
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. |
|
You don't need to rebase unless there are conflicts. |
ldez
force-pushed
the
feature/enable_pdns_rooted_at_path
branch
from
df7dfff to
1805ea5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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