Skip to content

Add Abion.com DNS API#6885

Open
stelar7 wants to merge 2 commits into
acmesh-official:devfrom
stelar7:abion-dns
Open

Add Abion.com DNS API#6885
stelar7 wants to merge 2 commits into
acmesh-official:devfrom
stelar7:abion-dns

Conversation

@stelar7

@stelar7 stelar7 commented Mar 30, 2026

Copy link
Copy Markdown

No description provided.

@github-actions

Copy link
Copy Markdown

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, 都必须确保通过这个测试.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new acme.sh DNS API hook for managing DNS-01 TXT records via Abion.com’s API.

Changes:

  • Introduces dnsapi/dns_abion.sh with an Abion API endpoint constant and request helper.
  • Implements dns_abion_add plus root-zone discovery logic (_get_root) and REST wrapper (_abion_rest).

Comment thread dnsapi/dns_abion.sh
Comment on lines +19 to +33
_debug "First detect the root zone"
if ! _get_root "$fulldomain"; then
_err "invalid domain"
return 1
fi

_debug _sub_domain "$_sub_domain"
_debug _domain "$_domain"

ABION_API_KEY="${ABION_API_KEY:-$(_readaccountconf_mutable ABION_API_KEY)}"

if [ -z "$ABION_API_KEY" ]; then
ABION_API_KEY=""
_err "You need to spesify ABION_API_KEY."
return 1

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

dns_abion_add calls _get_root before loading ABION_API_KEY, but _get_root immediately performs an authenticated API call (_abion_rest GET zones). With an unset key, root detection will fail (likely 401/parse failure) even when the domain is valid. Load/read the API key (and validate it) before any API request/root detection.

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh
Comment on lines +15 to +16
dns_abion_add() {
fulldomain=$1

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

This provider file only defines dns_abion_add, but acme.sh cleanup requires a matching ${provider}_rm function (see acme.sh _clearupdns calling ${_currentRoot}_rm). Without dns_abion_rm, renew/cleanup will error and TXT records will be left behind.

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh Outdated
return 1
fi

zones=$(echo "$response" | jq -r ".data | map(.id) | .[]")

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

_get_root parses the zones list using jq, which introduces a non-standard runtime dependency; most acme.sh environments (and other dnsapi hooks) avoid requiring jq. Please replace this with shell parsing using existing helpers (e.g., _normalizeJson, _egrep_o, sed) or ensure parsing works without external dependencies.

Suggested change
zones=$(echo "$response" | jq -r ".data | map(.id) | .[]")
zones=$(echo "$response" | _normalizeJson | sed 's/},{/}\
{/g' | _egrep_o '"id":"[^"]*"' | sed 's/^"id":"//; s/"$//')

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh Outdated
Comment on lines +79 to +83
if [ "$method" == "GET" ]; then
response="$(_get "$ABION_Api_Endpoint/$endpoint")"
else
response="$(_post "$data" "$ABION_Api_Endpoint/$endpoint" "" "$method")"
fi

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

[ "$method" == "GET" ] is a bashism; POSIX sh only guarantees = for string comparison. Using == can cause syntax/logic failures on some platforms, breaking all requests. Use = here (and keep the script strictly sh-compatible).

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh Outdated

if [ -z "$ABION_API_KEY" ]; then
ABION_API_KEY=""
_err "You need to spesify ABION_API_KEY."

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

Typo in error message: "spesify" should be "specify".

Suggested change
_err "You need to spesify ABION_API_KEY."
_err "You need to specify ABION_API_KEY."

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh Outdated
Comment on lines +38 to +39
_abion_rest PATCH "zones/$_domain" "{\"data\":{\"type\":\"zone\",\"id\":\"$_domain\",\"attributes\":{\"records\":{\"$_sub_domain\":{\"TXT\":[{\"rdata\":\"$txtvalue\",\"ttl\":300}]}}}}}"
}

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

dns_abion_add doesn't verify that the PATCH request succeeded at the application level (e.g., checking the JSON response for expected fields/success) and _abion_rest only checks the transport exit code. Since _post can return 0 even on HTTP 4xx (e.g., wget handling), this can report success while the API rejected the change. Please add response validation and surface the response body (or an extracted error message) on failure.

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh Outdated

zones=$(echo "$response" | jq -r ".data | map(.id) | .[]")

num_labels=$(echo "$domain" | tr '.' '\n' | wc -l)

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

num_labels=$(... | wc -l) may include leading spaces (per POSIX wc), and those can break numeric comparisons in [ "$i" -le "$num_labels" ] on some shells. Strip whitespace (e.g., tr -d ' ') or extract digits before using it in test arithmetic comparisons.

Suggested change
num_labels=$(echo "$domain" | tr '.' '\n' | wc -l)
num_labels=$(echo "$domain" | tr '.' '\n' | wc -l | tr -d '[:space:]')

Copilot uses AI. Check for mistakes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.

Comment thread dnsapi/dns_abion.sh Outdated
Comment on lines +49 to +52

zones=$(echo "$response" | jq -r ".data | map(.id) | .[]")

num_labels=$(echo "$domain" | tr '.' '\n' | wc -l)

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

This code depends on jq to parse the API response. jq is not a guaranteed dependency in acme.sh environments and is discouraged in dnsapi hooks; parse using shell tools/helpers instead so the hook remains portable.

Copilot generated this review using guidance from repository custom instructions.
Comment thread dnsapi/dns_abion.sh
Comment on lines +78 to +83

if [ "$method" == "GET" ]; then
response="$(_get "$ABION_Api_Endpoint/$endpoint")"
else
response="$(_post "$data" "$ABION_Api_Endpoint/$endpoint" "" "$method")"
fi

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

[ "$method" == "GET" ] is a bashism; POSIX sh only supports = for string comparison. Using == can fail on some shells (e.g., dash).

Copilot uses AI. Check for mistakes.
Comment thread dnsapi/dns_abion.sh Outdated
Comment on lines +52 to +55
num_labels=$(echo "$domain" | tr '.' '\n' | wc -l)
i=2
while [ "$i" -le "$num_labels" ]; do
candidate=$(echo "$domain" | cut -d'.' -f${i}-)

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

In _get_root(), the label loop counter starts at 2. Per acme.sh DNS API conventions this should start from 1 to support DNS alias mode; otherwise some domains can’t be resolved to the correct root zone.

Copilot generated this review using guidance from repository custom instructions.
Comment thread dnsapi/dns_abion.sh Outdated
Comment on lines +59 to +65
_sub_domain=$(echo "$domain" | cut -d'.' -f1-$((i - 1)))
_domain="$candidate"
return 0
fi
done

i=$((i + 1))

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

Avoid $(( ... )) arithmetic in dnsapi hooks; the project provides _math for portability. Replace the arithmetic expansions used to compute the subdomain slice and loop increment with _math (and adjust surrounding logic accordingly).

Copilot generated this review using guidance from repository custom instructions.
Comment thread dnsapi/dns_abion.sh Outdated
Comment on lines +5 to +6
Options:
ABION_API_KEY key

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

dns_abion_info block is missing a Docs/wiki link and the option description is vague ("ABION_API_KEY key"). Consider aligning with other dnsapi hooks by adding a Docs line and clarifying what the key is / where to obtain it.

Suggested change
Options:
ABION_API_KEY key
Docs: github.com/acmesh-official/acme.sh/wiki/dnsapi#dns_abion
Options:
ABION_API_KEY API key for abion.com; obtain it from your Abion account/API settings

Copilot uses AI. Check for mistakes.
@neilpang

neilpang commented Jul 1, 2026

Copy link
Copy Markdown
Member

1. add overwrites existing TXT records at the same name. The PATCH in dns_abion_add sets records[$_sub_domain][TXT] to a single-element array, and Abion replaces the whole TXT list for that name. So wildcard certs (base + * place two challenge values on the same _acme-challenge name) break — the second add wipes the first. add needs to read the existing TXT records, append the new value, and PATCH the union, the same read-modify-write rm already does.

2. Run the DNS-API-Test on your fork — it has to pass before this can be merged. There are no Actions runs on your fork at all right now.

3. shfmt is failing — run shfmt -i 2 -w dnsapi/dns_abion.sh.

Minor: rm rebuilds the remaining records with a hardcoded "ttl":60, which resets the TTL of any other TXT records at that name; and the Options: block is missing a Docs: line.

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