brianreumere / gandi-automatic-dns

Dynamic DNS shell script for Gandi
Other
100 stars 22 forks source link

Systematic try+fail updating since last commit for API change #17

Closed JMD-tech closed 7 years ago

JMD-tech commented 7 years ago

First, thanks for this script i've been using since a long time.

I was using an old 0.2 version, and tried to upgrade to the newer version after receiving Gandi's API update notification mail.

Tried new version (214012f), but this one was both systematically trying to update even if IP was matching, and not actually doing the update (but still reporting methodResponse = 1)

Kept old version on my production server, and used a test VM to repeat the problem with different commit versions. Made numerous tests, taken long notes which i can copy here if you'd like more informations, but to sum up i pinned it down to one working configuration:

taking the previous commit before API upgrade (9efbbf3), and changing only at line 137: sed -n 's/.*<int>\([0-9]*\).*/\1/p' to sed -n 's/.*<string>\([0-9]*\).*/\1/p'

It appears that Gandi really implemented the int => string change in the return values, but not the parameters: domain.zone.record.list systematically fails with zone_id as string returning: [...cut XML before...]

Error on object : OBJECT_ZONE (CAUSE_BADPARAMETER) [invalid method parameter(s)]

[...cut XML after...] and working correctly with zone_id as int

That would be compliant with their documentation on this page: http://doc.rpc.gandi.net/domain/reference.html?zone.record.list#domain.zone.record.list but not this (conflicting) other one: http://doc.rpc.gandi.net/domain/reference.html#RecordListOptions

Hope it's clear enough, i still have the test VM if you need some more tests to reproduce, but in my setup it's the only way i made it work, by keeping the zone_id input parameters as int, and only changing the sed filter for domain.zone.record.list output parsing in update() (It seems the one call to record.list in check() was already parsing for <string> in 9efbbf3)

reimannf commented 7 years ago

I think @JMD-tech is completely right. Basically gandi changed the return struct, not the rpc function signature (see additionally http://doc.rpc.gandi.net/domain/reference.html?zone.record.list#ZoneRecordReturn). As far as I can see you don't use that field id from record_list or record_update at all. So reverting https://github.com/brianpcurran/gandi-automatic-dns/commit/214012f8dc27510fad0fd12cd4eb2f59c0dd6ef0 worked for me.

brianreumere commented 7 years ago

Thanks for catching this. I made the last change a little too quickly :). Hopefully fixed in cf04ba2.

brianreumere commented 7 years ago

@JMD-tech Can you test and let me know?

JMD-tech commented 7 years ago

Ok, nearly good, the check() was now OK but not the update(), there is still a rpc "domain.zone.record.update" "string" "$zone_id"... at line 141 that causes an [invalid method parameter(s)] error, but by just changing this one to int now it performs the update.

Also checked that with it fixed, it also detects correctly if the IP is already the same and rightly skips the update.

fabrice-baray commented 7 years ago

Hi, Thanks for the fixes. Did the same change line 141 suggested by JMD-tech and it works for me. Cheers. Fabrice

ghost commented 7 years ago

First of all, I've been using this script for a very long time now too. I've been experiencing issues since Gandi updated their API. The latest in date is discribed in this ticket. I would like to thank you JMD-tech and fabrice-baray for spotting the typos, the script now works great again.

brianreumere commented 7 years ago

Thanks again for catching even more bugs :). I've made that change on line 141 so hopefully things are good now.