Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloudflare DNS backend #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jcgruenhage
Copy link

@jcgruenhage jcgruenhage commented Jan 15, 2023

Partially resolves #20. Does not implement the https backend, just the DNS one, but that's probably still enough for @simbleau.

This has been tested and is working as expected:

    let mut resolutions = public_ip::resolve(public_ip::dns::CLOUDFLARE, public_ip::Version::Any);
    while let Some(result) = resolutions.next().await {
        println!("{:?}", result);
    }

This snippet prints four results, the public v4 two times and then the public v6 two times.

@simbleau
Copy link

Glorious. Can't wait to use this.

@jcgruenhage
Copy link
Author

Well, if you really can't wait, this might be helpful:

[patch.crates-io]
cloudflare = { git = "https://github.com/jcgruenhage/cloudflare-rs.git", branch = "make-owner-fields-optional" }
public-ip = { git = "https://github.com/jcgruenhage/rust-public-ip.git", branch = "cloudflare-provider" }

Source: https://git.jcg.re/jcgruenhage/cloudflare-ddns-service/src/branch/main/Cargo.toml#L25-L27, cargo reference: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html#the-patch-section

Speaking of things that might be useful to you, even though this is slightly offtopic here: Any reason you're not using the cloudflare crate in cddns? I'm using it in my cloudflare ddns tool (see link above) and am quite happy with it, aside of the patch I need for it to work atm

@simbleau
Copy link

simbleau commented Jan 16, 2023

cloudflare-rs is a "work in progress" and unmaintained. I wouldn't rely on it for the same reasons I've gotten into a bind relying on @avitex to merge this: Authors leave their work frequently.

Seeing as I only needed 2-3 endpoint from Cloudflare and needed async support, I didn't think looking for a crate would be a good idea. I also am also considering to parse public IPs myself using the Cloudflare backend, seeing as the author of this library probably won't merge this soon and I've noticed the resolver is quite slow.

I want to take cddns to a higher standard than anything out there for free. I hope you can take a look and try it out. Maybe we can combine our efforts into cddns if you think it's a good tool.

@jcgruenhage
Copy link
Author

cloudflare-rs is a "work in progress" and unmaintained. I wouldn't rely on it for the same reasons I've gotten into a bind relying on @avitex to merge this: Authors leave their work frequently.

Well, cloudflare-rs is a WIP, but it's definitely not unmaintained. Sure, you don't have to rely on it, but doing so decreases the amount of code duplication. If you've got a problem with the crate, you can always fix the problem and contribute the fix upstream, so I wouldn't say it's more of a problem than maintaining your own API wrapper.

Seeing as I only needed 2-3 endpoint from Cloudflare and needed async support, I didn't think looking for a crate would be a good idea. I also am also considering to parse public IPs myself using the Cloudflare backend, seeing as the author of this library probably won't merge this soon and I've noticed the resolver is quite slow.

For a project that has had releases for over 3 years, 4 months since the last commit is fairly good in my books, so I wouldn't expect this to be ignored for long.

Other than that: Sure, if you want to query your public IP from another source, you're absolutely free to do so, but having a well maintained library that gives you that access is a better bet IMO. Just doing that bit I'm contributing in your code would be less reliable than what this crate is doing: It's doing fallback to other providers, meaning if one goes offline, you'll get them from another source. When it comes to performance: #22 for example is an obvious case highlighting what could be improved here, but I'm sure we'll get there eventually.

There's another library out there, https://lib.rs/crates/gip, which does basically the same thing as this one, just with a slightly different set of supported providers, but aside of that it's very similar. I chose to contribute to this crate here though because it's more widely used, so the impact here is a bit higher.

I want to take cddns to a higher standard than anything out there for free. I hope you can take a look and try it out. Maybe we can combine our efforts into cddns if you think it's a good tool.

I have a very low opinion of cloudflare and will migrate off of their infrastructure in the mid-term, so no, not interested in combining our efforts. What I'd be migrating to is a more modular approach, where the ddns daemon will call a hook script each time the IP changes, which you could use with https://github.com/danielpigott/cloudflare-cli for example to replicate a cloudflare based setup, but also with https://lib.rs/crates/trust-dns-util for example to use standard conform RFC2136 DNS Update requests instead of using a proprietary API.

Sorry for the wall-of-text btw, and sorry to @avitex for derailing the conversation here so much :/

@nrdxp
Copy link

nrdxp commented Sep 5, 2023

I can't seem to ever get my ip to resolve using this patch, I always get a None back

@jcgruenhage
Copy link
Author

@nrdxp thanks for catching this, should be fixed now.

@jcgruenhage jcgruenhage mentioned this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Cloudflare as backend DNS
3 participants