Provider Porkbun: Fix wildcard domain modifications

[bentemple]

Description

Porkbun does not properly handle wildcard domains with an HTML encoded *. By default the URL is encoded, and * becomes %2A which then causes Porkbun to not return the correct DNS entry for *.domain.com resulting in duplicate *.domain.com DNS entries and other unwanted behavior. This updates the logic to append the domain owner (subdomain) to the url after HTML encoding it, preventing the '*' or any other subdomain characters from being HTML encoded.

Test-Plan

Setup several domains with *.domain.tld and domain.tld dns A entries.
Set the values for some dns A entries to 127.0.0.1
Verified that DDNS-Updater was able to correctly update all entries with the invalid IP address set.

[bentemple]

Alternatively, we could call delete/create here and that would force an update while avoiding the 400 error.

[qdm12]

1. Did you face this problem in practice?
2. AFAIK this can only happen if the dns server is caching the old records due to a large TTL, although the update has been sent. In that case, we should address this at "upper" layers in the code so it applies to all providers. For example, if an update was sent, even if the resolved IP address mismatches your public IP address, do not attempt to send another update for the TTL value (optional, defaults to 300 seconds, configurable in the JSON of each record). I would like to suggest to revert the changes related to this here (i.e. getRecords back to getRecordIDs etc. and address this in another PR.

[bentemple]

1. So, I did face it. I would remove the updates.json, and then get into a bad state where it would attempt to update the entry, but then get a 400 error.
   I've done some more testing and I can't repro it, in the way I was expecting and I think much of it must have been all related to the wildcard issue. With the wild card issue, I would end up with multiple DNS entries, one for 127.0.0.1, one for my actual IP. I believe Porkbun under the hood stores the *.domain.tld differently from the %2A.domain.tld hence update would attempt to update the %2A.domain.tld, but it would be correct already and then fail.

The primary issue that this does solve matches (2) and is isolated to the issue of, when you're first setting up ddns-updater, if the DNS entry hasn't propogated fully and you remove your updates.json, for example, because something in it gets messed up, then subsequent runs of ddns-updater will attempt to update the dns values, get 400, and then fail out from then on because it keeps trying to update the DNS entry. So basically, once you get into the bad 400 state, it will never fix itself, because it will indefinitely try to update the DNS entry, but the DNS entry is actually correct.

Additionally in v2.7.0, my first entry had just my single domain.tld, and if I didn't put in "host":"@" it wouldn't work. that doesn't really seem to be the case on the current master though.

So the confluence of the 3 bugs:

1. 400 error when attempting to edit a domain that hasn't fully propogated to the DNS records
2. *.domain.tld being broken causing it to get into a state it could never resolve (constantly trying to update the same correct entry, 400 error, etc...)
3. the @ host not being documented but being required for 2.7.0 (or it seemed like it was specifically for porkbun), causing me to get into a state where I needed to remove my updates.json, but that would then cause more 400 errors, because the entries were correct but erroring when it tried to update them

All this resulted in me almost giving up on using DDNS-Updater, and I have seen other comments to that affect. I believe they ran into the same problems I did. That being said, I love golang, and this project def has exactly the features that I want in a DDNS updater, so I figured I fix it, and maybe upstream the fixes as well :). You've done a really great job with this project!

I'll split everything out into separate PRs, and we can discuss / merge or trash whatever. They're already all split up, I'll just divide and PR individually.

[bentemple]

sorry, didn't read what you put fully for 2.
so yes, this is primarily a fix for (2) as you stated, and yes, fixing at a higher level is probably a good idea.

I do feel like the context of why each of these fixes came about as without them, I got into a quite bad state very early in my setup. heh

[bentemple]

I split into #774 which we can probably just close as won't merge and update as you say at a higher level in the logic.

[qdm12]

Thanks for the PR! 💯
The wildcard handling was definitely completely messed up, thanks for fixing it! 👍
On the other hand, I think we should address the do-not-retry-an-update-that-soon aspect in another PR so it applies to all providers.

[qdm12]

1. Did you face this problem in practice?
2. AFAIK this can only happen if the dns server is caching the old records due to a large TTL, although the update has been sent. In that case, we should address this at "upper" layers in the code so it applies to all providers. For example, if an update was sent, even if the resolved IP address mismatches your public IP address, do not attempt to send another update for the TTL value (optional, defaults to 300 seconds, configurable in the JSON of each record). I would like to suggest to revert the changes related to this here (i.e. getRecords back to getRecordIDs etc. and address this in another PR.

[bentemple]

Thanks for the PR! 💯 The wildcard handling was definitely completely messed up, thanks for fixing it! 👍 On the other hand, I think we should address the do-not-retry-an-update-that-soon aspect in another PR so it applies to all providers.

You're welcome! Thanks for maintaining a great project! I love so many of the features this has. I looked at alternatives when I struggled to get it working with porkbun, but none of the others matched this in terms of simplicity, configurability, WebUI, and generally just being quite intelligent about how it utilizes various providers for the NSLookup by cycling through them. So happy to spend some time to try to make other people's lives easier and better and help them be able to use this as well. You've already put tons of work into it, so just trying to contribute a little as well :)

[qdm12]

Thank you for the kind words (and kind PRs!!) 💯
This PR definitely will help Porkbun users, this is awesome.
I'll start working on #485 (in a "retro-compatible" way) since that would address the 'ttl' issue I believe. If you feel I missed something, feel free to comment on that issue. The only problem would be if you change the record manually and then run ddns-updater within the ttl, but we can't be resistant to every user manipulations, especially across all dns registrars! 🤷

[qdm12]

Also my apologies for the long delay, I've been busy working on https://github.com/gluetun and dealing with some life thingies recently 😉 Merging this 💯 👍 🎖️

[bentemple]

Also my apologies for the long delay, I've been busy working on https://github.com/gluetun and dealing with some life thingies recently 😉 Merging this 💯 👍 🎖️

heh, no issues. I actually think, a lot of times slower is better when it comes to open source. Good to let things sit for a while, there's no real rush, and if someone really needs something, they can always pull the commits and build it themselves. Plus I've been running with this code the whole time, so isn't like I need it either and that just puts more testing into it as well.