Skip to content
This repository has been archived by the owner on Mar 14, 2023. It is now read-only.

modernize porkbun-ddns #6

Closed
wants to merge 3 commits into from
Closed

Conversation

con-f-use
Copy link

Modernizes the Python DDNS client:

  • Use argparse for CLI argument handling (better help text, validation and error messages)
  • Accept config file from stdin
  • Use modern Python naming conventions
  • Use ipaddress for validation
  • Default value for the base url of the API
  • Hashbang and magic __main__ for use as executable script
  • Closes JSON Decode Error #2
  • Closes DDNS for IPv6 #3

Modernizes the Python DDNS client:

* Use argparse for CLI argument handling (better help text, validation and error messages)
* Accept config file from stdin
* Use modern Python naming conventions
* Use ipaddress for validation
* Default value for the base url of the API
* Closes porkbundomains#2 
* Closes porkbundomains#3
@con-f-use
Copy link
Author

con-f-use commented Dec 22, 2021

@rossporkbun @eddie-porkbun please review. I tried to keep the CLI as much as possible - at some point, in a later PR, I'd like to add braking changes if that is okay with you:

Namely, a default config file location in ~/.config/porkbun/ddns.conf that can be overwritten via environment variable and -c, --config option as well as the possibility to save domain and subdomain there and overwrite everything with their own command line options. Also: logging, verbosity option and maybe a setup.py and flake.nix for packaging and easy distribution. But only if you're interested.

@benneti
Copy link

benneti commented Feb 12, 2022

If I try to use this with -i IPV6 adress, it set's the A record, I guess it should properly format the args when the ip is given.

@con-f-use
Copy link
Author

con-f-use commented Feb 12, 2022

@benneti
Copy link

benneti commented Feb 12, 2022 via email

@benneti
Copy link

benneti commented Feb 12, 2022

ok sorry it works as intended now, I guess I mixed up the lines.
The actual problem is that if I set the AAAA record it also deletes the A record, but I would like to have both set at the same time.
Basically I think

def delete_record(args):
    type_ = "A" if args.public_ip.version == 4 else "AAAA"
    for i in get_records(args)["records"]:
        if i["name"] == args.fqdn and i["type"] in [type_, "ALIAS", "CNAME"]:
            print("Deleting existing {}-Record: {}".format(i["type"], i))
            api(args, "/dns/delete/" + args.domain + "/" + i["id"])

would be better suited

porkbun-ddns.py Outdated Show resolved Hide resolved
@con-f-use
Copy link
Author

con-f-use commented Feb 13, 2022

@benneti
Did you know, you can make code suggestions in comments:

image

You just need to be in "Files Changed" and then click on a staring line green plus icon and drag to the last line your change is about.

@benneti
Copy link

benneti commented Feb 13, 2022

I did not, but I think I could also have done a review to achieve the same.
Thanks for putting it in the PR anyway!

@rossporkbun
Copy link
Contributor

@con-f-use sorry for the delay in this response. Thank you so much for doing all this work, and I bow to your Python knowledge, it bests mine.

I'm guessing this can't be made Python 2 compliant, which is a necessary requirement for the program. If you can, I'd be glad to consider the pull request. Otherwise, if you want to release the client on your own, I will happily link to your repo in our official documentation. That way, you can maintain it yourself rather than relying on me to approve your pull requests, as I am admittedly slow to take action.

@con-f-use
Copy link
Author

con-f-use commented Jun 18, 2022

I think it can be lightly tweaked to work with Python 2 AND 3. I'll see what I can do.

That being said, a word of advice: Python 2 has been end of life for close to two three years non and does not get any updates, security or otherwise. You should really switch any remaining infrastructure, because soon you'll face increasing problems with even installing it, since a lot of tools (pip, setuptools and the python package index) will or have already phased out support. I saw those problems daily in the company I work for.

@con-f-use
Copy link
Author

con-f-use commented Oct 7, 2022

It does work with python2.7 as long as py2-ipaddress and requests are installed in the right versions.

@mietzen
Copy link

mietzen commented Mar 11, 2023

@rossporkbun any Update on this? This would be highly beneficial for all users with IPv6 dual-stack lite connection which getting more common.

@dramborleg
Copy link

@rossporkbun any Update on this? This would be highly beneficial for all users with IPv6 dual-stack lite connection which getting more common.

Given the low activity I'd be surprised to see a response in the near future, but just in case it happens to help you I figured I would mention that ddclient recently added porkbun support in ddclient/ddclient#437. Not sure if it's in an official ddclient release yet.

@eddie-porkbun
Copy link
Contributor

@rossporkbun any Update on this? This would be highly beneficial for all users with IPv6 dual-stack lite connection which getting more common.

Ross actually has left Porkbun some time ago for new adventures. Ultimately this was meant to be an example of how to use our API for DDNS, provided as is, rather than an officially supported client. Using ddclient is probably a better way to go and I'll be sure to test it out myself soon.

@mietzen
Copy link

mietzen commented Mar 14, 2023

@eddie-porkbun
I think it would be a good Idea to update the ReadMe, reference ddclient and archive this repo.
Since this repo is still one of the first hits when you Google ddns and porkbun.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DDNS for IPv6 JSON Decode Error
6 participants