duckDns: Revert renew each domain first#4122
Conversation
- Dehydrated needs some fixes to support this first
📝 WalkthroughWalkthroughVersion bumped to 1.26.0, changelog updated. Renewal script altered to disable per-domain dehydrated calls inside the loop and rely on a single bulk dehydrated invocation with aggregated domain arguments. A prior informational log line was commented out. No other behavioral or structural changes. Changes
Sequence Diagram(s)sequenceDiagram
participant S6 as s6 service
participant Script as duckdns/run
participant Dehyd as dehydrated
participant LE as Let's Encrypt
S6->>Script: Start renewal
Script->>Script: Collect domains/aliases into domain_args
Note over Script: Per-domain dehydrated calls disabled
Script->>Dehyd: Run once with aggregated domain_args
Dehyd->>LE: ACME challenges/renewals
LE-->>Dehyd: Certificates/response
Dehyd-->>Script: Result
Script-->>S6: Exit
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
duckdns/CHANGELOG.md (1)
5-6: Tighten phrasing and style for consistencyMinor editorial nits: prefer “rate limit” (no hyphen) and US spelling “behavior” to match most existing entries.
-- Workaround for potential rate-limit for some people -- Fallback to similar renewal behaviour to 1.19 while still supporting wildcards and other fixes +- Workaround for potential rate limit for some people +- Fallback to renewal behavior similar to 1.19 while still supporting wildcards and other fixesduckdns/rootfs/etc/s6-overlay/s6-rc.d/duckdns/run (3)
54-57: Good rationale documented; add a tracking referenceThe explanatory comments clarify the revert. Consider adding a TODO with an upstream issue/PR reference to dehydrated to aid future cleanup.
- # Note, the below is commented out for now as dehydrated force-renews each domain when domain input is different from the previous run - # This is causing rate-limiting from Let's Encrypt - # Need to resolve issue with dehydrated first to properly support single TXT records + # Note, the below is commented out for now as dehydrated force-renews each domain when domain input is different from the previous run + # This is causing rate-limiting from Let's Encrypt + # TODO: Track upstream dehydrated fix to properly support single TXT records (add issue/PR reference)
59-60: Gate per-domain renew behind a feature flag instead of commenting outKeeps the revert as default while enabling quick toggling for testing/triage without editing code.
- #dehydrated --cron --algo "${ALGO}" --hook /root/hooks.sh --challenge dns-01 --domain "${domain}" --out "${CERT_DIR}" --config "${WORK_DIR}/config" || true + if [[ "${PER_DOMAIN_RENEW:-0}" == "1" ]]; then + # Caution: can trigger Let's Encrypt rate limits; enable only for testing or when dehydrated fix is available. + dehydrated --cron --algo "${ALGO}" --hook /root/hooks.sh --challenge dns-01 --domain "${domain}" --out "${CERT_DIR}" --config "${WORK_DIR}/config" || true + fi
63-63: Prefer debug-level logging over removing the messageThis log helps diagnose renewal flow without spamming info logs.
- #bashio::log.info "Final pass to combine individual certificates" + bashio::log.debug "Final pass to combine individual certificates"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
duckdns/CHANGELOG.md(1 hunks)duckdns/config.yaml(1 hunks)duckdns/rootfs/etc/s6-overlay/s6-rc.d/duckdns/run(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
*/**(html|markdown|md)
⚙️ CodeRabbit Configuration File
*/**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
*/**(html|markdown|md): - Use bold to mark UI strings.
If "" are used to mark UI strings, replace them by bold.
Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Use sentence-style capitalization also in headings.
do not comment on HTML used for icons
Avoid flagging inline HTML for embedding videos in future reviews for this repository.
Files:
duckdns/CHANGELOG.mdduckdns/config.yaml
🔇 Additional comments (3)
duckdns/config.yaml (1)
2-2: Version bump aligns with CHANGELOG and revert scopeManifest version update to 1.26.0 is consistent with the described behavior change.
duckdns/CHANGELOG.md (1)
3-3: Changelog entry added for 1.26.0Entry accurately reflects the revert and rate-limit context.
duckdns/rootfs/etc/s6-overlay/s6-rc.d/duckdns/run (1)
54-60: Hooks implement the single-TXT record strategy via alias mapping
Thedeploy_challengeandclean_challengefunctions induckdns/rootfs/root/hooks.shread the.aliasessection from the Dehydrated config to consolidate all validations onto a single TXT record (using theALIASvariable) and clear it appropriately. No further changes are required.
|
I'm reverting the one change I made earlier that renews each domain first due to the DuckDNS limitation of single TXT record. |
|
Closing. Replaced with a proper fix: #4126 |
2 participants
Summary by CodeRabbit
Bug Fixes
Documentation
Chores