leamas / ddupdate

Update DNS Data for Dynamic IP Addresses
MIT License
40 stars 28 forks source link

Support multiple credentials with the same provider #56

Closed g2p closed 2 years ago

g2p commented 2 years ago

Some dynamic dns providers (like Google domains) support updating multiple hosts, each with their own API credentials.

To specify credentials that are specific to a target domain at a provider, put the following in ~/.netrc:

machine mydomain.tld.domains.google.com.ddupdate login AAA password BBB
machine mydomain2.tld.domains.google.com.ddupdate login CCC password DDD

With machine of the form:

targetdomain.providerdomain.ddupdate

Closes #52.

leamas commented 2 years ago

No. As I see it , this can and should be done without affecting other plugins. The need for separate accounts is very specific and should not require changes in other plugins.

I'm very conservative since it's a mess to test them...

g2p commented 2 years ago

It's a compatible change, in as much as the *.ddupdate pattern is unlikely to have been used before. But I'll revert the changes to the other plugins.

leamas commented 2 years ago

Sorry, it's weekend, life takes some toll. Back to business.

This looks mostly fine. However, I wonder about the change to get_netrc_auth(machine): why have you added the targethost parameter here?

As I see it, get_netrc_auth could be exactly as it was, besides handling the .ddupdate suffix. That is, if one requests data for host.domain.tld and that does not exists, get_netrc_auth tries host.domain.tld.ddupdate.

EDIT: Actuallly, the other way around: it first tries host.domain.tld.ddupdate, falling back to host.domain.tld.

I don't see that the providerhost semantics makes any sense in this function. If a plugin like google domains has the need, it could just try different hosts to get a match. The basic semantics if netrc is a mapping from host -> authentication data, can we let it be that way?

leamas commented 2 years ago

Ping?

g2p commented 2 years ago

The logic in this PR uses both providerhost and targethost, and looks for targethost.providerhost.ddupdate (falling back to using providerhost as before). And it does it in a common function, which will be used if another plugin needs to disambiguate as well. As I see it, if we introduce a different domain pattern, taking both components into account ensures that there is no ambiguity and that the change will only need to be done once.

leamas commented 2 years ago

Yes, I can follow your line of reasoning.

However, with fresh eyes I just don't like that the somewhat special needs from the google domain plugin adds complexity to the common library. Walking this path, other plugins will have their needs and the central parts evolves to a mess.

The simple solution is perhaps that you just copy _get_netrcauth() and _http_basic_authsetup() to the plugin and modify them as you want there. Such changes will be local, you don't need to generalize and I can sleep well.

Thoughts?

g2p commented 2 years ago

Sure, I can move the functions. Will update the PR.

leamas commented 2 years ago

Great. We are approaching a new major release, would be nice to get this onboard.

leamas commented 2 years ago

Thanks for PR and a good dialog! Sorry for long delays on my side.

g2p commented 2 years ago

Thank you for your patience as well.