Skip to content

Fix dns_arvan: Correct API usage for domain lookup and improve error handling#6789

Open
abooraja wants to merge 11 commits intoacmesh-official:devfrom
abooraja:fix/arvan-dns-api-domain-lookup
Open

Fix dns_arvan: Correct API usage for domain lookup and improve error handling#6789
abooraja wants to merge 11 commits intoacmesh-official:devfrom
abooraja:fix/arvan-dns-api-domain-lookup

Conversation

@abooraja
Copy link

Problem

The dns_arvan DNS API was failing with "invalid domain" error when trying to obtain SSL certificates for domains. The issue was caused by incorrect API usage:

  1. _get_root() function: Was attempting to query /domains/{domain} directly, which is not supported by ArvanCloud API v4.0. The API requires listing all domains first, then searching through the list.

  2. _arvan_rest() function: Was incorrectly appending data to GET request URLs and had improper error handling.

  3. Shellcheck compliance: The code had SC2181 style issues with exit code checking.

Solution

Changes Made:

  1. Refactored _get_root() function:

    • Now fetches the complete list of domains from /domains endpoint first
    • Searches through the response to find the matching domain
    • Extracts domain_id using multiple robust regex patterns to handle different JSON response formats
    • Properly handles subdomain detection
  2. Fixed _arvan_rest() function:

    • Correctly handles GET requests: queries ARVAN_API_URL/$ep when endpoint is provided, or ARVAN_API_URL for listing domains
    • Added direct exit code checking (fixed shellcheck SC2181)
    • Improved error handling for all request types (GET, POST, DELETE)
  3. Enhanced dns_arvan_add() function:

    • Better duplicate record detection with multiple patterns
    • Improved error logging
  4. Enhanced dns_arvan_rm() function:

    • Multiple regex patterns to reliably extract record_id for deletion
    • Better success/failure detection
    • Improved error logging
  5. Code quality:

    • Converted all Persian comments to English
    • Fixed all shellcheck errors
    • Code follows acme.sh DNS API standards

Testing

  • ✅ Successfully obtained wildcard SSL certificate for mizekar.site and *.mizekar.site
  • ✅ Tested with Let's Encrypt staging environment
  • ✅ Tested with Let's Encrypt production environment
  • ✅ All shellcheck checks pass
  • ✅ DNS records are properly added and removed

API Documentation

Based on ArvanCloud API v4.0 documentation:

Related

Fixes #<ISSUE_NUMBER>

Checklist

  • Code follows the DNS API development guide
  • All shellcheck errors fixed
  • Tested with real domain and certificate issuance
  • Comments are in English
  • Code is compatible with existing acme.sh structure

   - Fix _get_root() to fetch domains list first, then search within response
   - Fix _arvan_rest() GET request handling
   - Improve dns_arvan_rm() with multiple pattern matching
   - Enhance error handling and debug output
   - Fixes acmesh-official#6788
@github-actions
Copy link

Welcome
READ ME !!!!!
Read me !!!!!!
First thing: don't send PR to the master branch, please send to the dev branch instead.
Please read the DNS API Dev Guide.
You MUST pass the DNS-API-Test.
Then reply on this message, otherwise, your code will not be reviewed or merged.
Please also make sure to add/update the usage here: https://github.com/acmesh-official/acme.sh/wiki/dnsapi2
注意: 必须通过了 DNS-API-Test 才会被 review. 无论是修改, 还是新加的 dns api, 都必须确保通过这个测试.

@abooraja
Copy link
Author

abooraja commented Feb 14, 2026

✅ I have read the DNS API Dev Guide: https://github.com/acmesh-official/acme.sh/wiki/DNS-API-Dev-Guide

✅ I have read the DNS-API-Test guide: https://github.com/acmesh-official/acme.sh/wiki/DNS-API-Test

✅ I understand that the DNS-API-Test must pass before the code can be reviewed.

✅ I will update the wiki documentation at: https://github.com/acmesh-official/acme.sh/wiki/dnsapi2

Status:

  • Code follows DNS API development guide
  • All shellcheck errors fixed
  • Comments are in English
  • Tested locally with real domain
  • DNS-API-Test will be run by maintainers (requires secrets)
  • Wiki documentation will be updated

Testing:

  • Successfully obtained wildcard SSL certificate for mizekar.site and *.mizekar.site
  • Tested with both Let's Encrypt staging and production environments
  • All shellcheck checks pass

Copilot AI review requested due to automatic review settings February 22, 2026 10:30
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 critical issues with the ArvanCloud DNS API integration that was causing certificate issuance failures. The main problem was incorrect API usage where the code was trying to query individual domains directly via /domains/{domain}, which isn't supported by ArvanCloud API v4.0. The fix implements the correct approach of listing all domains first and then searching through the list.

Changes:

  • Refactored _get_root() to fetch complete domain list first, then search through it with robust JSON parsing
  • Fixed _arvan_rest() to properly handle GET requests without appending data to URLs, and added direct exit code checking
  • Enhanced error handling throughout with better error messages and multiple fallback patterns for JSON extraction

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

if ! printf "%s" "$response" | grep \"current_page\":1 >/dev/null; then
if ! printf "%s" "$response" | grep -q "\"current_page\":1"; then
_err "Error on Arvan Api"
_err "Please create a github issue with debbug log"
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

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

This error message contains a typo: "debbug" should be "debug".

Suggested change
_err "Please create a github issue with debbug log"
_err "Please create a github issue with debug log"

Copilot uses AI. Check for mistakes.
@neilpang
Copy link
Member

you need to run the dns api test on your repo, not here.

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.

3 participants