[fix] Updated auto-install script prompt for SSL certificate #556#564
[fix] Updated auto-install script prompt for SSL certificate #556#564nemesifier merged 1 commit intomasterfrom
Conversation
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
pandafy
moved this from To do (general)
to Needs review
in OpenWISP Contributor's Board
|
No actionable comments were generated in the recent review. 🎉 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2026-02-17T12:50:25.569ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
WalkthroughThis pull request removes the CERT_ADMIN_EMAIL variable from the OpenWISP installation and configuration process, addressing the obsolescence of Let's Encrypt renewal emails. The auto-install script replaces the email prompt with a yes/no question for Let's Encrypt usage. The corresponding environment variable is removed from configuration files, certbot commands are updated to exclude the email parameter, and related documentation is deleted. Self-signed certificate flow remains unchanged when users decline Let's Encrypt. Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant InstallScript as "auto-install.sh"
participant Certbot
participant SystemFiles as "nginx/certs/files"
User->>InstallScript: Run installer / answer prompt "Use Let's Encrypt SSL? (y/N)"
InstallScript->>InstallScript: Interpret response -> set SSL_CERT_MODE (Yes or SelfSigned)
alt User answered yes
InstallScript->>Certbot: Invoke certbot certonly --standalone --noninteractive --agree-tos --rsa-key-size ... --domain example.com
Certbot-->>InstallScript: Return cert files (privkey.pem/fullchain.pem)
InstallScript->>SystemFiles: Write certs to expected paths
else User answered no
InstallScript->>SystemFiles: Generate self-signed certs (openssl or existing logic)
end
InstallScript->>SystemFiles: Update config / ENV (remove CERT_ADMIN_EMAIL usage)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
deploy/auto-install.sh (1)
120-121: Validate yes/no input to avoid accidental SelfSigned selection.Any non-
yes/yvalue currently flips to SelfSigned, which can happen on typos or empty input. Consider an explicit yes/no loop and then setSSL_CERT_MODEfrom that result.♻️ Suggested change
- # SSL Configuration - echo -ne ${GRN}"(5/5) Use Let's Encrypt to generate SSL certificate (yes/no): "${NON} - read use_letsencrypt + # SSL Configuration + while :; do + echo -ne ${GRN}"(5/5) Use Let's Encrypt to generate SSL certificate (yes/no): "${NON} + read use_letsencrypt + case "${use_letsencrypt,,}" in + yes|y) ssl_cert_mode="Yes"; break ;; + no|n) ssl_cert_mode="SelfSigned"; break ;; + *) echo -e ${YLW}"Please answer yes or no."${NON} ;; + esac + done @@ - if [[ "${use_letsencrypt,,}" == "yes" || "${use_letsencrypt,,}" == "y" ]]; then - set_env "SSL_CERT_MODE" "Yes" - else - set_env "SSL_CERT_MODE" "SelfSigned" - fi + set_env "SSL_CERT_MODE" "$ssl_cert_mode"Also applies to: 164-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deploy/auto-install.sh` around lines 120 - 121, The prompt currently reads into use_letsencrypt and treats any non-yes as SelfSigned; change this to an explicit validation loop that repeatedly prompts (using the same echo prompt text) until the user enters a clear yes or no (accept case-insensitive "y"/"yes" and "n"/"no"), then set SSL_CERT_MODE to "Let's Encrypt" (or "letsencrypt") on yes and "SelfSigned" on no; apply the same validated-loop logic to the other prompt block that also reads user input (the second read block around the other SSL prompt). Use the existing variable names (use_letsencrypt and SSL_CERT_MODE) so the rest of the script continues to work.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.envdeploy/auto-install.shdocs/user/settings.rstimages/common/utils.shimages/openwisp_nginx/Dockerfile
💤 Files with no reviewable changes (3)
- .env
- images/openwisp_nginx/Dockerfile
- docs/user/settings.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI Build
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@images/common/utils.sh`:
- Around line 33-40: The certbot invocations in the create_prod_certs function
can fail on a fresh LetsEncrypt volume because Certbot requires either --email
or --register-unsafely-without-email when using --noninteractive --agree-tos;
update both certbot certonly calls (the ones using ${DASHBOARD_DOMAIN} and
${API_DOMAIN}) to include --register-unsafely-without-email so account
registration succeeds without an email during initial deployment.
---
Nitpick comments:
In `@deploy/auto-install.sh`:
- Around line 120-121: The prompt currently reads into use_letsencrypt and
treats any non-yes as SelfSigned; change this to an explicit validation loop
that repeatedly prompts (using the same echo prompt text) until the user enters
a clear yes or no (accept case-insensitive "y"/"yes" and "n"/"no"), then set
SSL_CERT_MODE to "Let's Encrypt" (or "letsencrypt") on yes and "SelfSigned" on
no; apply the same validated-loop logic to the other prompt block that also
reads user input (the second read block around the other SSL prompt). Use the
existing variable names (use_letsencrypt and SSL_CERT_MODE) so the rest of the
script continues to work.
| certbot certonly --standalone --noninteractive --agree-tos \ | ||
| --rsa-key-size 4096 \ | ||
| --domain ${DASHBOARD_DOMAIN} \ | ||
| --email ${CERT_ADMIN_EMAIL} | ||
| --domain ${DASHBOARD_DOMAIN} | ||
| fi | ||
| if [ ! -f /etc/letsencrypt/live/${API_DOMAIN}/privkey.pem ]; then | ||
| certbot certonly --standalone --noninteractive --agree-tos \ | ||
| --rsa-key-size 4096 \ | ||
| --domain ${API_DOMAIN} \ | ||
| --email ${CERT_ADMIN_EMAIL} | ||
| --domain ${API_DOMAIN} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find Dockerfile or image configuration files
find . -name "Dockerfile*" -o -name "docker-compose*" | head -20Repository: openwisp/docker-openwisp
Length of output: 415
🏁 Script executed:
# Look for any image build configuration in the images directory
find images -type f -name "Dockerfile*" -o -name "*.sh" | grep -E "(Dockerfile|^Dockerfile)" | head -20Repository: openwisp/docker-openwisp
Length of output: 376
🏁 Script executed:
# Search for how certbot is installed in the codebase
rg "certbot" --type dockerfile --type shell -B 2 -A 2 | head -100Repository: openwisp/docker-openwisp
Length of output: 102
🏁 Script executed:
# Find the images/common directory and utils.sh
find images -name "utils.sh" -o -name "common" -type dRepository: openwisp/docker-openwisp
Length of output: 104
🏁 Script executed:
# Search for certbot in shell files
rg "certbot" --type shell -B 2 -A 2 | head -150Repository: openwisp/docker-openwisp
Length of output: 97
🏁 Script executed:
# Check the base Dockerfile which likely installs certbot
cat images/openwisp_base/Dockerfile | head -100Repository: openwisp/docker-openwisp
Length of output: 4177
🏁 Script executed:
# Read the utils.sh file to see full context
cat -n images/common/utils.sh | head -80Repository: openwisp/docker-openwisp
Length of output: 3136
🏁 Script executed:
# Search for certbot installation in all Dockerfiles
rg "certbot" . --no-heading -B 3 -A 3Repository: openwisp/docker-openwisp
Length of output: 2270
🌐 Web query:
Alpine Linux certbot version requirements --noninteractive --email
💡 Result:
On Alpine Linux, there isn’t a special Alpine-specific minimum version for these flags—--noninteractive/--non-interactive and --email are long-standing Certbot CLI options and are present in current Alpine packages (e.g., Alpine v3.22 ships certbot 4.0.0 in community). [1]
What matters is Certbot’s non-interactive requirements:
- If you run with
--noninteractive(aka--non-interactive), you must also provide the inputs Certbot would otherwise prompt for—most commonly:--agree-tos- either
--email you@example.comor--register-unsafely-without-email(not recommended) [2][3]
Example (typical webroot issuance):
certbot certonly --noninteractive --agree-tos --email you@example.com \
--webroot -w /var/www/html -d example.comTo confirm what your Alpine image actually has installed:
certbot --version
apk info -v certbotSources: [1] [2] [3]
🌐 Web query:
certbot documentation --noninteractive flag email requirement
💡 Result:
With Certbot, -n/--noninteractive means Certbot will not prompt you, so any info it would normally ask for must already be available.
- If no ACME account is registered yet on that machine, Certbot will refuse to proceed non-interactively unless you provide:
--agree-tosand- either
-m/--email you@example.comor--register-unsafely-without-email(discouraged) [1][2].
- If an ACME account already exists locally, Certbot will reuse it and typically won’t require
--emailon subsequent runs (because the account already has contact info) [3].
You can also pre-create/update the account explicitly (so later --noninteractive runs don’t need to prompt) using the registration options (-m/--email, --update-registration, etc.) [1][2].
Sources:
[1] Certbot manpage / CLI options (--email, --register-unsafely-without-email) https://eff-certbot.readthedocs.io/en/zoraconpatch-readthedocs-reqtxt/man/certbot.html
[2] Certbot User Guide (same registration options) https://dokk.org/documentation/certbot/v0.22.1/using/
[3] Let’s Encrypt Community Support explanation of account reuse in noninteractive mode https://community.letsencrypt.org/t/further-automating-certification-creation-command/52873
🏁 Script executed:
# Check if there's any account pre-setup or email configuration
rg "register-unsafely|--email|certbot register|ACME_EMAIL|CERTBOT_EMAIL" images/ docs/ --no-heading -B 2 -A 2Repository: openwisp/docker-openwisp
Length of output: 50
🏁 Script executed:
# Check docker-compose.yml for environment variables related to certbot/email
cat docker-compose.ymlRepository: openwisp/docker-openwisp
Length of output: 5705
Add --register-unsafely-without-email to certbot commands in create_prod_certs function.
Certbot with --noninteractive --agree-tos requires either --email or --register-unsafely-without-email when registering a new ACME account. On initial deployment with a fresh /etc/letsencrypt volume, the current commands will fail and block TLS certificate generation. Add the flag to both certbot certonly invocations:
Suggested fix
- certbot certonly --standalone --noninteractive --agree-tos \
+ certbot certonly --standalone --noninteractive --agree-tos \
+ --register-unsafely-without-email \
--rsa-key-size 4096 \
--domain ${DASHBOARD_DOMAIN}
@@
- certbot certonly --standalone --noninteractive --agree-tos \
+ certbot certonly --standalone --noninteractive --agree-tos \
+ --register-unsafely-without-email \
--rsa-key-size 4096 \
--domain ${API_DOMAIN}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| certbot certonly --standalone --noninteractive --agree-tos \ | |
| --rsa-key-size 4096 \ | |
| --domain ${DASHBOARD_DOMAIN} \ | |
| --email ${CERT_ADMIN_EMAIL} | |
| --domain ${DASHBOARD_DOMAIN} | |
| fi | |
| if [ ! -f /etc/letsencrypt/live/${API_DOMAIN}/privkey.pem ]; then | |
| certbot certonly --standalone --noninteractive --agree-tos \ | |
| --rsa-key-size 4096 \ | |
| --domain ${API_DOMAIN} \ | |
| --email ${CERT_ADMIN_EMAIL} | |
| --domain ${API_DOMAIN} | |
| certbot certonly --standalone --noninteractive --agree-tos \ | |
| --register-unsafely-without-email \ | |
| --rsa-key-size 4096 \ | |
| --domain ${DASHBOARD_DOMAIN} | |
| fi | |
| if [ ! -f /etc/letsencrypt/live/${API_DOMAIN}/privkey.pem ]; then | |
| certbot certonly --standalone --noninteractive --agree-tos \ | |
| --register-unsafely-without-email \ | |
| --rsa-key-size 4096 \ | |
| --domain ${API_DOMAIN} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@images/common/utils.sh` around lines 33 - 40, The certbot invocations in the
create_prod_certs function can fail on a fresh LetsEncrypt volume because
Certbot requires either --email or --register-unsafely-without-email when using
--noninteractive --agree-tos; update both certbot certonly calls (the ones using
${DASHBOARD_DOMAIN} and ${API_DOMAIN}) to include
--register-unsafely-without-email so account registration succeeds without an
email during initial deployment.
There was a problem hiding this comment.
Since certbot 3.3, the register-unsafely-without-email is no longer required.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which version of certbot is installed in the Docker images
rg "certbot" images/ --type dockerfile -B 2 -A 2
# Check for certbot installation in Dockerfiles
cat images/openwisp_nginx/Dockerfile | grep -i certbot -A 5 -B 5
Length of output: 302
🌐 Web query:
certbot 3.3 changelog email requirement removal
💡 Result:
Certbot 3.3.0 (released March 11, 2025) changed account registration so an email address is no longer effectively required:
- Non-interactive mode: you no longer need
--register-unsafely-without-emailto register without an email. [1] - Interactive mode: at the email prompt, pressing Enter will register without an email. [1]
Context: Let’s Encrypt also announced it would end certificate expiration notification emails on June 4, 2025, reducing the value of supplying an email for reminders. [2]
Sources: [1] Certbot 3.3.0 release notes/changelog, Let’s Encrypt Community Support; [2] Let’s Encrypt blog announcement.
@pandafy Thank you for the correction! You're absolutely right. The web search confirms that certbot 3.3.0 (released March 11, 2025) removed the requirement for --register-unsafely-without-email in non-interactive mode, and pressing Enter at the email prompt now registers without an email.
Since the Alpine image installs certbot via apk add certbot without version pinning, and we're currently in February 2026, the installed version would be 3.3+. Your implementation is correct as-is—the certbot commands will work fine without the email parameter or the unsafe registration flag.
I apologize for the incorrect review comment!
✏️ Learnings added
Learnt from: pandafy
Repo: openwisp/docker-openwisp PR: 564
File: images/common/utils.sh:33-40
Timestamp: 2026-02-17T12:50:25.569Z
Learning: In certbot 3.3.0+ (released March 2025), the `--register-unsafely-without-email` flag is no longer required when using `--noninteractive --agree-tos` without providing an email address. Account registration will proceed without an email by default.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
github-project-automation
bot
moved this from Needs review
to In progress
in OpenWISP Contributor's Board
Removed CERT_ADMIN_EMAIL setting as Let's Encrypt no long sends expiration email to the user. Fixes #556
pandafy
force-pushed
the
issues/556-ssl-certificate
branch
from
59d689c to
89df106
Compare
github-project-automation
bot
moved this from In progress
to Done
in OpenWISP Contributor's Board
2 participants
Checklist
Reference to Existing Issue
Fixes #556
Description of Changes
Removed CERT_ADMIN_EMAIL setting as Let's Encrypt no long sends expiration email to the user.