Skip to content

Commit 2511933

Browse files
committed
fix(dstack-ingress): make ACME_STAGING work in both modes, not just tls-alpn-01
ACME_STAGING was read only on the tls-alpn-01 path. Under dns-01 nothing looked at it -- certman.py and functions.sh both read CERTBOT_STAGING -- so setting it there silently issued *production* certificates. The README did not mark it mode-specific, so there was nothing to warn you. Same shape as the RENEW_DAYS_BEFORE bug, and worse in effect: instead of a knob that does nothing, this one does the opposite of what it says on a path where the mistake costs a real certificate and real rate limit. Normalise both names once in entrypoint.sh and export them. This does belong there, unlike the wildcard rule: an ACME account and a staging toggle are genuinely shared by both modes, which is exactly what that file validates. The per-mode copies in dns01.sh and tlsalpn.sh are gone. Name them after ACME rather than certbot throughout. What you configure is an ACME account; which client implements a mode is an implementation detail, and this image already changed it once. CERTBOT_EMAIL and CERTBOT_STAGING keep working -- they are the historical names, not the preferred ones, and the README no longer implies the preference depends on the mode. Also fixes the webhook example, which showed the payload's keys in an order the code does not emit (it sorts them), and drops a stale duplicate comment on ensure_placeholder_certs that credited certbot with forwarding the tls-alpn-01 challenge.
1 parent 098741c commit 2511933

6 files changed

Lines changed: 37 additions & 22 deletions

File tree

custom-domain/dstack-ingress/README.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ environment:
159159
| `DOMAIN` | Your domain (single-domain mode). Supports wildcards (`*.example.com`) |
160160
| `TARGET_ENDPOINT` | Backend address, e.g. `app:80` or `http://app:80` |
161161
| `GATEWAY_DOMAIN` | dstack gateway domain (e.g. `_.dstack-prod5.phala.network`) |
162-
| `CERTBOT_EMAIL` | *(optional)* ACME contact address. `ACME_EMAIL` is accepted too, and is the preferred name in tls-alpn-01 mode |
162+
| `ACME_EMAIL` | *(optional)* ACME contact address, in either mode. `CERTBOT_EMAIL` is the historical name and still works. See below — it is optional, and published |
163163
| `DNS_PROVIDER` | DNS provider (`cloudflare`, `linode`, `namecheap`) |
164164

165165
### Optional
@@ -171,8 +171,7 @@ environment:
171171
| `ROUTING_MAP` | | Multi-domain routing: `domain=host:port` per line |
172172
| `SET_CAA` | `false` | Enable CAA DNS record (dns-01 only; tls-alpn-01 cannot write DNS) |
173173
| `TXT_PREFIX` | `_dstack-app-address` | DNS TXT record prefix |
174-
| `CERTBOT_STAGING` | `false` | Use Let's Encrypt staging server. `ACME_STAGING` is the preferred name in tls-alpn-01 mode |
175-
| `ACME_EMAIL` | | ACME contact address, in either mode. Falls back to `CERTBOT_EMAIL`. Optional — see below |
174+
| `ACME_STAGING` | `false` | Use Let's Encrypt staging, in either mode. `CERTBOT_STAGING` is the historical name and still works |
176175
| `CHALLENGE_TYPE` | `dns-01` | `dns-01` (certbot + DNS credentials) or `tls-alpn-01` (lego, no DNS credentials) |
177176
| `DNS_SETUP_MODE` | `wait` | tls-alpn-01 only: `wait`, `print` or `webhook` — see below |
178177
| `DNS_SETUP_TIMEOUT` | `1800` | tls-alpn-01 only: seconds to wait for the records to appear |
@@ -345,7 +344,7 @@ Encrypt dropping expiry mail, and the container passes it for you.
345344

346345
```json
347346
{
348-
"payload": "{\"version\":1,\"domain\":\"app.example.com\",\"records\":[...]}",
347+
"payload": "{\"app_id\":\"\",\"challenge\":\"tls-alpn-01\",\"domain\":\"app.example.com\",\"instance_id\":\"\",\"records\":[…],\"timestamp\":1234567890,\"version\":1}",
349348
"hmac_sha256": "…",
350349
"attestation": { "quote": "…", "report_data": "…" }
351350
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@
1313
sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
1414

1515

16+
def staging_enabled() -> bool:
17+
"""Whether to use Let's Encrypt staging.
18+
19+
entrypoint.sh normalises the two names, but certman.py is also runnable on
20+
its own, so accept both here as well. ACME_STAGING wins: the setting is
21+
about the ACME server, not about certbot.
22+
"""
23+
value = os.environ.get("ACME_STAGING") or os.environ.get("CERTBOT_STAGING", "false")
24+
return value == "true"
25+
26+
1627
class CertManager:
1728
"""Certificate management using DNS provider infrastructure."""
1829

@@ -299,7 +310,7 @@ def _build_certbot_command(self, action: str, domain: str, email: str) -> List[s
299310
else:
300311
base_cmd.append("--register-unsafely-without-email")
301312
base_cmd.extend(["-d", domain])
302-
if os.environ.get("CERTBOT_STAGING", "false") == "true":
313+
if staging_enabled():
303314
base_cmd.extend(["--staging"])
304315

305316
if getattr(self.provider, 'CERTBOT_PROPAGATION_SECONDS'):
@@ -513,7 +524,7 @@ def acme_account_exists(self) -> bool:
513524
"""
514525
import glob
515526
api_endpoint = "acme-v02.api.letsencrypt.org"
516-
if os.environ.get("CERTBOT_STAGING", "false") == "true":
527+
if staging_enabled():
517528
api_endpoint = "acme-staging-v02.api.letsencrypt.org"
518529
pattern = f"/etc/letsencrypt/accounts/{api_endpoint}/directory/*/regr.json"
519530
return len(glob.glob(pattern)) > 0

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,9 @@ source /scripts/functions.sh
1111
source /scripts/haproxy-lib.sh
1212
source /scripts/evidence-lib.sh
1313

14-
# ACME contact address. CERTBOT_EMAIL is the documented name on this path --
15-
# certbot really is the client here -- and ACME_EMAIL is accepted too so one
16-
# variable works in either mode. Optional either way.
17-
ACME_EMAIL=${ACME_EMAIL:-${CERTBOT_EMAIL:-}}
18-
export ACME_EMAIL
14+
# ACME_EMAIL / ACME_STAGING are normalised by entrypoint.sh and exported. The
15+
# contact address is optional; certman.py asks for a contactless account when
16+
# none is given.
1917

2018
# certbot stores certificates under /etc/letsencrypt/live/<domain>/.
2119
cert_fullchain_path() { echo "/etc/letsencrypt/live/$(cert_dir_name "$1")/fullchain.pem"; }

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@ EVIDENCE_PORT=${EVIDENCE_PORT:-80}
2222
ALPN=${ALPN:-}
2323
CHALLENGE_TYPE=${CHALLENGE_TYPE:-dns-01}
2424

25+
# ACME account settings, normalised once for both modes. What you are
26+
# configuring is an ACME account, not a particular client -- which client
27+
# implements a mode is an implementation detail, and this image already changed
28+
# it once (certbot for dns-01, lego for tls-alpn-01). So ACME_EMAIL and
29+
# ACME_STAGING are the names; CERTBOT_EMAIL and CERTBOT_STAGING are the
30+
# historical ones and keep working.
31+
#
32+
# Normalising here rather than per mode is what fixes the real bug: ACME_STAGING
33+
# used to be read only on the tls-alpn-01 path, so setting it under dns-01
34+
# silently issued *production* certificates.
35+
ACME_EMAIL=${ACME_EMAIL:-${CERTBOT_EMAIL:-}}
36+
ACME_STAGING=${ACME_STAGING:-${CERTBOT_STAGING:-false}}
37+
2538
case "$CHALLENGE_TYPE" in
2639
dns-01|tls-alpn-01) ;;
2740
*)
@@ -72,6 +85,7 @@ done
7285
# the helpers it invokes see the sanitized values.
7386
export PORT DOMAIN DOMAINS TARGET_ENDPOINT ROUTING_MAP TXT_PREFIX MAXCONN
7487
export TIMEOUT_CONNECT TIMEOUT_CLIENT TIMEOUT_SERVER EVIDENCE_SERVER EVIDENCE_PORT ALPN
88+
export ACME_EMAIL ACME_STAGING
7589

7690
case "$CHALLENGE_TYPE" in
7791
dns-01)

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,8 @@ get_letsencrypt_account_path() {
166166
local base_path="/etc/letsencrypt/accounts"
167167
local api_endpoint="acme-v02.api.letsencrypt.org"
168168

169-
if [[ "$CERTBOT_STAGING" == "true" ]]; then
169+
# entrypoint.sh normalises CERTBOT_STAGING into ACME_STAGING.
170+
if [[ "$ACME_STAGING" == "true" ]]; then
170171
api_endpoint="acme-staging-v02.api.letsencrypt.org"
171172
fi
172173

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,13 @@ source /scripts/functions.sh
1212
source /scripts/haproxy-lib.sh
1313
source /scripts/evidence-lib.sh
1414

15-
# ACME account settings. CERTBOT_EMAIL / CERTBOT_STAGING are the historical
16-
# names, kept working for anyone migrating a compose file from dns-01, but there
17-
# is no certbot on this path -- the ACME client here is lego.
15+
# ACME_EMAIL / ACME_STAGING are normalised by entrypoint.sh and exported, so
16+
# they are already correct here whichever name the operator used.
1817
#
1918
# The contact address is optional. RFC 8555 makes it optional, Let's Encrypt
2019
# stopped sending expiry notifications in 2025, and it does not stay private:
2120
# the account document is published at /evidences/acme-account.json, so an
2221
# address set here is readable by anyone who fetches the evidence.
23-
ACME_EMAIL=${ACME_EMAIL:-${CERTBOT_EMAIL:-}}
24-
ACME_STAGING=${ACME_STAGING:-${CERTBOT_STAGING:-false}}
25-
export ACME_EMAIL ACME_STAGING
2622

2723
LEGO_BIN=${LEGO_BIN:-/usr/local/bin/lego}
2824
LEGO_PATH=${LEGO_PATH:-/etc/letsencrypt/lego}
@@ -148,10 +144,6 @@ EOF
148144
# haproxy refuses to start when the crt directory has no PEM in it, and here it
149145
# has to be listening before the first certificate can exist -- it is what
150146
# forwards the challenge to lego. Seed a placeholder the real cert overwrites.
151-
# haproxy refuses to start when the crt directory has no PEM in it. In
152-
# tls-alpn-01 mode the proxy has to be listening *before* the first certificate
153-
# exists, because it is what forwards the ACME challenge to certbot, so seed a
154-
# self-signed placeholder that the real certificate overwrites later.
155147
ensure_placeholder_certs() {
156148
local all_domains domain pem
157149
all_domains=$(get-all-domains.sh)

0 commit comments

Comments
 (0)