Update dns_beget.sh #6509
Update dns_beget.sh #6509
Conversation
|
Welcome |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Beget DNS API integration to work with changes in the Beget API. The update simplifies the implementation by removing complex subdomain handling and DNS record parsing logic, replacing it with a more direct approach using API calls and fallback dig polling.
Key Changes:
- Simplified API call implementation with direct URL construction and curl usage
- Replaced complex DNS record parsing with direct TXT record management
- Added fallback mechanism using dig when API calls fail
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| [ -n "$input_data" ] && url="${url}&input_data=$(echo -n "$input_data" | jq -s -R -r @uri)" | ||
|
|
||
| return 0 | ||
| echo "[DEBUG] _api_call url=$url" |
There was a problem hiding this comment.
Debug output exposes sensitive credentials (username and password) in the URL. This could leak credentials to logs. Consider masking credentials in debug output or using a different logging approach.
| echo "[DEBUG] _api_call url=$url" | |
| # Mask credentials in debug output | |
| masked_url="$api_url?login=***&passwd=***&input_format=json&output_format=json" | |
| [ -n "$input_data" ] && masked_url="${masked_url}&input_data=$(echo -n "$input_data" | jq -s -R -r @uri)" | |
| echo "[DEBUG] _api_call url=$masked_url" |
| Beget_Username="${Beget_Username:?Please set Beget_Username}" | ||
| Beget_Password="${Beget_Password:?Please set Beget_Password}" |
There was a problem hiding this comment.
The credential handling pattern has changed from the original implementation which used _readaccountconf_mutable and _saveaccountconf_mutable functions. This breaks the existing credential management pattern used throughout the acme.sh project.
| Beget_Username="${Beget_Username:?Please set Beget_Username}" | |
| Beget_Password="${Beget_Password:?Please set Beget_Password}" | |
| # Credential management: read from account config if not set | |
| if [ -z "$Beget_Username" ]; then | |
| Beget_Username="$(_readaccountconf_mutable BEGET_User)" | |
| fi | |
| if [ -z "$Beget_Password" ]; then | |
| Beget_Password="$(_readaccountconf_mutable BEGET_Password)" | |
| fi | |
| if [ -z "$Beget_Username" ]; then | |
| _err "Please set BEGET_User." | |
| return 1 | |
| fi | |
| if [ -z "$Beget_Password" ]; then | |
| _err "Please set BEGET_Password." | |
| return 1 | |
| fi | |
| # Save credentials to account config for future use | |
| _saveaccountconf_mutable BEGET_User "$Beget_Username" | |
| _saveaccountconf_mutable BEGET_Password "$Beget_Password" |
|
|
||
| old_txts="[]" | ||
| i=1 | ||
| while [ $i -le 6 ]; do # 6 раз по 20 секунд = максимум 120 |
There was a problem hiding this comment.
Comment contains text in Russian (Cyrillic characters). Comments should be in English for consistency with the codebase.
| while [ $i -le 6 ]; do # 6 раз по 20 секунд = максимум 120 | |
| while [ $i -le 6 ]; do # 6 times 20 seconds = maximum 120 seconds |
| res=$(_api_call "$Beget_Api/dns/getData" "{\"fqdn\":\"$fulldomain\"}") || { | ||
| echo "[ERROR] API getData did not return a response" | ||
| return 1 | ||
| fi | ||
| _debug _domain_id "$_domain_id" | ||
| _debug _sub_domain "$_sub_domain" | ||
| _debug _domain "$_domain" | ||
|
|
||
| if [ -z "$_sub_domain" ]; then | ||
| _debug "$fulldomain is a root domain." | ||
| return 0 | ||
| fi | ||
| } |
There was a problem hiding this comment.
The || operator will trigger on any non-zero exit code from _api_call, but _api_call uses curl -s which returns the response regardless of HTTP status. This condition may incorrectly trigger when the API returns a valid response with an error status.
| _api_call "$Beget_Api/dns/changeRecords" "$data" || { | ||
| echo "[ERROR] Error calling changeRecords" | ||
| return 1 | ||
| fi | ||
|
|
||
| _debug "Cleanup subdomen records" | ||
| data="{\"fqdn\":\"$fulldomain\",\"records\":{}}" | ||
| res=$(_api_call "$Beget_Api/dns/changeRecords" "$data") | ||
| if ! _is_api_reply_ok "$res"; then | ||
| _debug "Can't cleanup $fulldomain records." | ||
| fi | ||
|
|
||
| data="{\"fqdn\":\"www.$fulldomain\",\"records\":{}}" | ||
| res=$(_api_call "$Beget_Api/dns/changeRecords" "$data") | ||
| if ! _is_api_reply_ok "$res"; then | ||
| _debug "Can't cleanup www.$fulldomain records." | ||
| fi | ||
| } |
There was a problem hiding this comment.
Same issue as above - the || operator will trigger on any non-zero exit code from _api_call, but the function doesn't validate HTTP status codes or API response status, potentially masking actual API errors.
|
Hey @temi4, your pull request is sending to master branch. Please consider closing this PR and create a new PR to merge your commits to dev instead. Thanks! |
2 participants
Beget change api, I create fix.
All work now.