fix(dstack-ingress): count either issuance attempt as a change

Author: Kevin Wang

custom-domain/dstack-ingress/README.md

@@ -159,7 +159,7 @@ environment:
 | `DOMAIN` | Your domain (single-domain mode). Supports wildcards (`*.example.com`) |
 | `TARGET_ENDPOINT` | Backend address, e.g. `app:80` or `http://app:80` |
 | `GATEWAY_DOMAIN` | dstack gateway domain (e.g. `_.dstack-prod5.phala.network`) |
-| `CERTBOT_EMAIL` | Email for Let's Encrypt registration. Not required in tls-alpn-01 mode, where the preferred name is `ACME_EMAIL` |
+| `CERTBOT_EMAIL` | *(optional)* ACME contact address. `ACME_EMAIL` is accepted too, and is the preferred name in tls-alpn-01 mode |
 | `DNS_PROVIDER` | DNS provider (`cloudflare`, `linode`, `namecheap`) |
 
 ### Optional
@@ -172,7 +172,7 @@ environment:
 | `SET_CAA` | `false` | Enable CAA DNS record (dns-01 only; tls-alpn-01 cannot write DNS) |
 | `TXT_PREFIX` | `_dstack-app-address` | DNS TXT record prefix |
 | `CERTBOT_STAGING` | `false` | Use Let's Encrypt staging server. `ACME_STAGING` is the preferred name in tls-alpn-01 mode |
-| `ACME_EMAIL` | | Optional ACME contact address in tls-alpn-01 mode. Falls back to `CERTBOT_EMAIL`; there is no certbot on that path |
+| `ACME_EMAIL` | | ACME contact address, in either mode. Falls back to `CERTBOT_EMAIL`. Optional — see below |
 | `CHALLENGE_TYPE` | `dns-01` | `dns-01` (certbot + DNS credentials) or `tls-alpn-01` (lego, no DNS credentials) |
 | `DNS_SETUP_MODE` | `wait` | tls-alpn-01 only: `wait`, `print` or `webhook` — see below |
 | `DNS_SETUP_TIMEOUT` | `1800` | tls-alpn-01 only: seconds to wait for the records to appear |
@@ -307,16 +307,20 @@ Two consequences:
   means updating DNS. `DNS_SETUP_MODE=webhook` exists so this can be automated;
   doing it by hand means downtime on every redeploy.
 
-### The ACME contact address is optional, and public
+## The ACME contact address is optional, and public
 
-`ACME_EMAIL` may be left unset. RFC 8555 makes the ACME `contact` field
-optional, and Let's Encrypt stopped sending expiry notification mail in 2025, so
-setting one buys little.
+`ACME_EMAIL` (or `CERTBOT_EMAIL`) may be left unset in **either** mode. RFC 8555
+makes the ACME `contact` field optional, and Let's Encrypt stopped sending expiry
+notification mail in 2025, so setting one buys little.
 
 It also does not stay private. The ACME account document is published as
-attestation evidence at `/evidences/acme-account.json`, so an address set here
-is readable by anyone who fetches the evidence endpoint. Leave it unset unless
-you specifically want a contact on the account.
+attestation evidence at `/evidences/acme-account.json`, so an address set here is
+readable by anyone who fetches the evidence endpoint. Leave it unset unless you
+specifically want a contact on the account.
+
+Under the hood the two clients differ: lego simply omits the flag, while certbot
+needs `--register-unsafely-without-email` — its "unsafely" naming predates Let's
+Encrypt dropping expiry mail, and the container passes it for you.
 
 ### Limitations
 

custom-domain/dstack-ingress/scripts/certman.py

@@ -286,8 +286,18 @@ def _build_certbot_command(self, action: str, domain: str, email: str) -> List[s
                     f"Credentials file does not exist: {credentials_file}")
 
         if action == "certonly":
-            base_cmd.extend(["--agree-tos", "--no-eff-email",
-                            "--email", email, "-d", domain])
+            base_cmd.extend(["--agree-tos", "--no-eff-email"])
+            # The ACME contact address is optional (RFC 8555 section 7.3), and
+            # it is published: the account document is served as attestation
+            # evidence, so an address set here is readable by anyone who
+            # fetches it. certbot will not simply omit the flag, so ask for a
+            # contactless account explicitly. Its "unsafely" naming predates
+            # Let's Encrypt dropping expiry notification mail in 2025.
+            if email:
+                base_cmd.extend(["--email", email])
+            else:
+                base_cmd.append("--register-unsafely-without-email")
+            base_cmd.extend(["-d", domain])
         if os.environ.get("CERTBOT_STAGING", "false") == "true":
             base_cmd.extend(["--staging"])
 
@@ -501,15 +511,11 @@ def main():
             )
             sys.exit(1)
 
-        # Email is required for obtain and auto actions
-        if args.action in ["obtain", "auto"] and not args.email:
-            if not os.environ.get("CERTBOT_EMAIL"):
-                print(
-                    "Error: --email is required or set CERTBOT_EMAIL environment variable",
-                    file=sys.stderr,
-                )
-                sys.exit(1)
-            args.email = os.environ["CERTBOT_EMAIL"]
+        # The contact address is optional; see _build_certbot_command.
+        if not args.email:
+            args.email = os.environ.get("ACME_EMAIL") or os.environ.get(
+                "CERTBOT_EMAIL", ""
+            )
 
         success, needs_evidence = manager.run_action(
             args.domain, args.email, args.action

custom-domain/dstack-ingress/scripts/dns01.sh

@@ -11,6 +11,12 @@ source /scripts/functions.sh
 source /scripts/haproxy-lib.sh
 source /scripts/evidence-lib.sh
 
+# ACME contact address. CERTBOT_EMAIL is the documented name on this path --
+# certbot really is the client here -- and ACME_EMAIL is accepted too so one
+# variable works in either mode. Optional either way.
+ACME_EMAIL=${ACME_EMAIL:-${CERTBOT_EMAIL:-}}
+export ACME_EMAIL
+
 # certbot stores certificates under /etc/letsencrypt/live/<domain>/.
 cert_fullchain_path() { echo "/etc/letsencrypt/live/$(cert_dir_name "$1")/fullchain.pem"; }
 cert_privkey_path()   { echo "/etc/letsencrypt/live/$(cert_dir_name "$1")/privkey.pem"; }
@@ -136,20 +142,38 @@ set_caa_record() {
 }
 
 process_domain() {
-    local domain="$1"
+    local domain="$1" first status
 
     set_alias_record "$domain"
     set_txt_record "$domain"
+
     # The CAA record names our ACME account, so the account has to exist first:
-    # try once (expected to fail on a fresh deployment), write CAA, try again.
-    issue_certificate "$domain" || echo "First certificate attempt failed for $domain, retrying after the CAA record is set"
+    # try once, write CAA, try again. The first attempt is expected to fail when
+    # a CAA record from an earlier account is still in place.
+    if issue_certificate "$domain"; then first=0; else first=$?; fi
+    if [ "$first" -eq 1 ]; then
+        echo "First certificate attempt failed for $domain, retrying after the CAA record is set"
+    fi
+
     set_caa_record "$domain"
-    issue_certificate "$domain"
+
+    if issue_certificate "$domain"; then status=0; else status=$?; fi
+
+    # Either attempt producing a certificate counts as a change. Reporting only
+    # the second one hid the common case: on a clean first deployment attempt
+    # one obtains the certificate and attempt two reports "nothing to renew",
+    # so the pass looked quiet and skipped evidence generation entirely.
+    if [ "$first" -eq 0 ] || [ "$status" -eq 0 ]; then
+        return 0
+    fi
+    return "$status"
 }
 
 issue_certificate() {
     source /opt/app-venv/bin/activate
-    certman.py auto --domain "$1" --email "$CERTBOT_EMAIL"
+    # The contact address is optional; certman.py asks for a contactless
+    # account when none is given.
+    certman.py auto --domain "$1" ${ACME_EMAIL:+--email "$ACME_EMAIL"}
 }
 
 build_combined_pems() {