search

qdm12 / ddns-updater

Container to update DNS records periodically with WebUI for many DNS providers
https://hub.docker.com/r/qmcgaw/ddns-updater/
MIT License
1.46k stars 146 forks source link

Implementated global validation of domain strings #638

Closed CyberAustin closed 3 months ago

CyberAustin commented 4 months ago

Should close #626 and provide a basic sanity check for all providers, agnostic of provider specific requirements.

CyberAustin commented 4 months ago

Ugh. Can't run linters or devcontainer in Replit. Have to fix this later at home.

JK. Forgot GitHub codespaces was a thing.

CyberAustin commented 4 months ago

I don't know why the linter is failing in GitHub Actions because it passes in the dev container. Anyway, ready for review.

CyberAustin commented 4 months ago

Now, we need to add a check the domain isn't "" for the providers actually using the domain parameter. Most of them do, but there are 2-3 exceptions (like duckdns, goip). So that would be edits in each of the provider subpackage within internal/provider/providers

I'm not sure what you're asking here. Could you explain it a different way.

qdm12 commented 4 months ago

We need to add a check in most providers that the domain is not empty "". Because in the current "above layer" check, we check the domain is empty (🆗) or valid (🆗), but some providers require a non-empty domain (most except a few).

For example in https://github.com/qdm12/ddns-updater/blob/6a6b1a8ebbaf228f3454d6a1f4a2a61addea6c0d/internal/provider/providers/cloudflare/provider.go#L97

add a case to check the domain is not empty:

case p.domain == "":
    return fmt.Errorf("%w", errors.ErrDomainNotSet)
CyberAustin commented 4 months ago

Should be good now. Please take a look @qdm12