Skip to content

Conversation

@davidandreoletti
Copy link
Contributor

@davidandreoletti davidandreoletti commented Sep 30, 2025

📦 Package Details

Maintainer: @feckert

Description:
Fixes #27526.

OVH changed its API to update DNS records. It now requires HTTP Basic Authorization header. As such the default ddns-script method to update the DNS record is failing. The fix is to move DNS record updates into its own script/package.

🧪 Run Testing Details

  • **OpenWrt Version: SNAPSHOTS **
  • OpenWrt Target/Subtarget: rockchip/armv8
  • **OpenWrt Device: NanoPi R4s 4G **

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

If your PR contains a patch:

No patch.

@davidandreoletti davidandreoletti force-pushed the ddns-scripts-fix-ovh-dns-record-update branch from 0e9aac2 to ed94a2c Compare September 30, 2025 05:01
@davidandreoletti davidandreoletti marked this pull request as draft September 30, 2025 05:01
@davidandreoletti davidandreoletti force-pushed the ddns-scripts-fix-ovh-dns-record-update branch from ed94a2c to 19ef850 Compare September 30, 2025 05:22
@BKPepe
Copy link
Member

BKPepe commented Sep 30, 2025

@kolacinskikarol In the past, you were doing some changes for OVH ddns-script. Would you mind to take a look at it, please?

@davidandreoletti davidandreoletti force-pushed the ddns-scripts-fix-ovh-dns-record-update branch 2 times, most recently from 836874e to 289d433 Compare October 1, 2025 08:30
@davidandreoletti davidandreoletti marked this pull request as ready for review October 3, 2025 02:05
@davidandreoletti
Copy link
Contributor Author

@kolacinskikarol @BKPepe This is ready for review.

@BKPepe BKPepe requested a review from Copilot October 3, 2025 05:16
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the OVH DNS record update functionality by adapting to OVH's API changes that now require HTTP Basic Authorization. The solution involves moving the DNS update logic from a simple URL-based approach to a dedicated script that properly handles authentication.

  • Updated ovh.com.json configuration to use a custom update script instead of direct URL calls
  • Created update_ovh_com.sh script that implements proper HTTP Basic Authorization for OVH's API
  • Added new ddns-scripts-ovh package to handle OVH-specific DNS updates

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
net/ddns-scripts/files/usr/share/ddns/default/ovh.com.json Updated to reference custom update script instead of direct API URLs
net/ddns-scripts/files/usr/lib/ddns/update_ovh_com.sh New script implementing OVH DynHost API with HTTP Basic Authorization
net/ddns-scripts/Makefile Added new ddns-scripts-ovh package definition and build configuration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@davidandreoletti davidandreoletti force-pushed the ddns-scripts-fix-ovh-dns-record-update branch from a08158f to fcfdd67 Compare October 3, 2025 12:58
@feckert
Copy link
Member

feckert commented Oct 6, 2025

Please bump PKG_RELEASE in the make file. The other things LGTM.
Thanks

@davidandreoletti davidandreoletti force-pushed the ddns-scripts-fix-ovh-dns-record-update branch from fcfdd67 to 4bc9d9c Compare October 6, 2025 08:20
@davidandreoletti
Copy link
Contributor Author

Please bump PKG_RELEASE in the make file. The other things LGTM. Thanks

done.

OVH changed its API to update DNS records. It now requires HTTP Basic
Authorization header. As such the default ddns-script method to update
the DNS record is failing. The fix is to move DNS record updates into
its own script/package.

Signed-off-by: David Andreoletti <[email protected]>
@davidandreoletti davidandreoletti force-pushed the ddns-scripts-fix-ovh-dns-record-update branch from 4bc9d9c to 73266f8 Compare October 7, 2025 01:06
@feckert feckert merged commit 104aabd into openwrt:master Oct 7, 2025
12 checks passed
@feckert
Copy link
Member

feckert commented Oct 7, 2025

Thanks merged!

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.

dns-scripts: https://USER:[email protected]/nic/update?system=dyndns&hostname=$HOSTNAME&myip=$IP fails with curl

3 participants