Skip to content

Commit 90db698

Browse files
committed
refactor(dstack-ingress): let the provider choose the gateway record, as it already did
DELEGATION_GATEWAY_RECORD reimplemented an extension point that was already there. set_alias_record means "point this name at that target, in whatever form this provider allows beside a CAA" -- Linode overrides it to publish an address record, with a docstring saying exactly why: it refuses a CAA next to a CNAME, which is the conflict delegation runs into. So there were two mechanisms deciding the same thing: the per-provider override, and a `case cloudflare) cname ;; *) a ;;` of my own a layer up. They agreed on Linode by coincidence and would have disagreed on any provider that added an override later, since the shell branch never consulted it. Worse, the previous commit made the `cname` branch call set_cname specifically to bypass Linode's override -- treating the correct provider-specific behaviour as the bug. Delegation now asks for an alias and takes what it gets. Gone with the switch: the provider-detection action, the set_a action, dnsguide's --resolve, and the README section explaining which providers get which. A provider that needs a form other than CNAME overrides set_alias_record, which is one place to look instead of two. The cross-type clearing that review found stays, moved to where it belongs: set_alias_record removes address records before writing a CNAME, Linode's override removes a CNAME before writing an address record. Every caller benefits, not just delegation, and the rule sits next to the knowledge of which form this provider uses. Route53 is worth a look separately -- it takes the base CNAME path, and if it also refuses a CAA beside a CNAME it needs the same override Linode has. Untested either way.
1 parent 0a91c91 commit 90db698

6 files changed

Lines changed: 43 additions & 134 deletions

File tree

custom-domain/dstack-ingress/README.md

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ environment:
193193
| `ALPN` | | TLS ALPN protocols (e.g. `h2,http/1.1`). Only set if backends support h2c |
194194
| `DELEGATION_ZONE` | | Zone this container writes into, so the DNS token needs no access to the served domain's own zone (see below) |
195195
| `DELEGATION_PROPAGATION_SECONDS` | `120` | Wait after writing the delegated challenge TXT before validation. Must outlast the record TTL (60s), or a resolver still serving the previous attempt's value fails validation. Keep well under ~250s — certbot is killed after a 300s per-run timeout |
196-
| `DELEGATION_GATEWAY_RECORD` | per provider | How the delegated name points at the gateway: `cname` (Cloudflare default; follows a gateway that moves) or `a` (default elsewhere, for providers that refuse a CAA beside a CNAME) |
197196

198197
For DNS provider credentials, see [DNS_PROVIDERS.md](DNS_PROVIDERS.md).
199198

@@ -232,29 +231,12 @@ _dstack-app-address.svc.example.com.deleg.example.net TXT "<app-id>:<port>"
232231
_acme-challenge.svc.example.com.deleg.example.net TXT <challenge> (transient)
233232
```
234233

235-
The gateway pointer and the CAA share a name, which RFC 1034 does not allow —
236-
a CNAME excludes every other type at its name. **Cloudflare allows the pair
237-
anyway**, and Let's Encrypt honours the CAA it finds there: with a CAA that
238-
forbids it, the staging CA refuses with `While processing CAA for
239-
svc.example.com: CAA record for svc.example.com prevents issuance`.
240-
241-
Providers that enforce the standard will reject the CAA beside the CNAME, so
242-
they get an address record instead — `A` and `CAA` coexist legally. The default
243-
follows the provider:
244-
245-
| Provider | Gateway pointer | Gateway that moves |
246-
|---|---|---|
247-
| `cloudflare` | `CNAME` to `GATEWAY_DOMAIN` | followed by DNS, immediately |
248-
| everything else | `A`, re-resolved each pass | picked up within `RENEW_INTERVAL` |
249-
250-
`DELEGATION_GATEWAY_RECORD=cname\|a` overrides it. Only Cloudflare has been
251-
tested; if another provider accepts a CAA beside a CNAME, setting `cname` there
252-
gets the better behaviour.
253-
254-
A wildcard and its base name share a delegated target: both `*.example.com` and
255-
`example.com` map to `example.com.<delegation-zone>`. One deployment serving both
256-
is fine, since it publishes the same values twice. Two deployments pointing at
257-
different gateways are not — give them separate delegation zones.
234+
The gateway pointer and the CAA share a name, which DNS does not allow for a
235+
CNAME. Which form works is a property of the provider — Cloudflare tolerates the
236+
pair, Linode publishes an address record instead — and `set_alias_record` in the
237+
provider layer is where that choice is made. Delegation asks for an alias and
238+
takes what it gets. A provider that needs a form other than CNAME overrides that
239+
method, as Linode does.
258240

259241
**A wildcard needs a fourth CNAME.** RFC 8659 evaluates `*.app.example.com` at
260242
`app.example.com`, so the base gets its own alias and the container publishes an

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

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -116,29 +116,10 @@ set_txt_record() {
116116
# never needs touching again -- not when the app id changes with the compose,
117117
# not when the ACME account is recreated, not when the gateway moves.
118118
#
119-
# How the delegated name points at the gateway.
120-
#
121-
# A CNAME is better -- a gateway that moves is followed automatically -- but the
122-
# same name also has to carry the CAA, and RFC 1034 says a CNAME excludes every
123-
# other type at its name. Cloudflare allows the pair anyway, and Let's Encrypt
124-
# honours the CAA it finds there (verified against the staging CA). Providers
125-
# that enforce the standard reject it, so they get an address record instead:
126-
# A and CAA coexist legally, at the cost of re-resolving GATEWAY_DOMAIN once a
127-
# pass rather than letting DNS follow it.
128-
#
129-
# Default per provider, since only Cloudflare is known to allow the pair.
130-
# DELEGATION_GATEWAY_RECORD overrides it either way.
131-
delegation_gateway_record() {
132-
if [ -n "${DELEGATION_GATEWAY_RECORD:-}" ]; then
133-
echo "$DELEGATION_GATEWAY_RECORD"
134-
return
135-
fi
136-
case "$(dnsman.py provider 2>/dev/null)" in
137-
cloudflare) echo cname ;;
138-
*) echo a ;;
139-
esac
140-
}
141-
119+
# The gateway pointer and the CAA end up on one name, which DNS does not allow
120+
# for a CNAME. Which form works is a provider property -- Cloudflare tolerates
121+
# the pair, Linode publishes an address record instead -- and set_alias_record
122+
# is where that knowledge lives, so ask for an alias and let the provider pick.
142123
delegated_name() {
143124
local domain="${1#\*.}"
144125
echo "${domain}.${DELEGATION_ZONE}"
@@ -148,37 +129,7 @@ delegation_publish() {
148129
local domain="$1" target txt_name
149130
target=$(delegated_name "$domain")
150131

151-
# The set_* helpers only look at the type they are about to write, so a
152-
# leftover record of the other type stays and blocks the new one -- DNS
153-
# forbids a CNAME beside anything else. Clear it first, so switching
154-
# DELEGATION_GATEWAY_RECORD either way actually takes effect.
155-
case "$(delegation_gateway_record)" in
156-
cname)
157-
dnsman.py unset --domain "$target" --type A >/dev/null 2>&1 || true
158-
# set_cname, not set_alias: some providers redefine "alias" to mean
159-
# an address record (Linode does, precisely to dodge the CAA/CNAME
160-
# conflict), and this branch has to mean what it says.
161-
dnsman.py set_cname --domain "$target" --content "$GATEWAY_DOMAIN" || return 1
162-
;;
163-
a)
164-
# For providers that refuse a CAA beside a CNAME. Costs the
165-
# automatic following of a gateway that moves: the address is
166-
# re-resolved once per pass instead.
167-
local addrs addr
168-
addrs=$(dnsguide.py --resolve "$GATEWAY_DOMAIN" 2>/dev/null)
169-
if [ -z "$addrs" ]; then
170-
echo "Error: could not resolve $GATEWAY_DOMAIN to any address" >&2
171-
return 1
172-
fi
173-
addr=$(echo "$addrs" | head -1)
174-
dnsman.py unset --domain "$target" --type CNAME >/dev/null 2>&1 || true
175-
dnsman.py set_a --domain "$target" --content "$addr" || return 1
176-
;;
177-
*)
178-
echo "Error: invalid DELEGATION_GATEWAY_RECORD (expected cname or a)" >&2
179-
return 1
180-
;;
181-
esac
132+
dnsman.py set_alias --domain "$target" --content "$GATEWAY_DOMAIN" || return 1
182133

183134
txt_name="$(txt_record_name "$domain").${DELEGATION_ZONE}"
184135
dnsman.py set_txt --domain "$txt_name" --content "$(txt_record_value)" || return 1

custom-domain/dstack-ingress/scripts/dns_providers/base.py

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -160,27 +160,40 @@ def set_a_record(
160160
)
161161
return self.create_dns_record(new_record)
162162

163+
def clear_conflicting_alias(self, name: str, keep: "RecordType") -> None:
164+
"""Remove address/alias records at `name` other than `keep`.
165+
166+
DNS forbids a CNAME beside anything else, and the set_* helpers only
167+
look at the type they are about to write, so a name that already holds
168+
the other form blocks the write. That happens whenever the form changes
169+
-- a provider gaining an override, or a deployment moving between
170+
providers -- and the failure looks like a permissions problem rather
171+
than a leftover record.
172+
"""
173+
for record_type in (RecordType.CNAME, RecordType.A, RecordType.AAAA):
174+
if record_type == keep:
175+
continue
176+
for record in self.get_dns_records(name, record_type):
177+
if record.id:
178+
self.delete_dns_record(record.id, name)
179+
163180
def set_alias_record(
164181
self,
165182
name: str,
166183
content: str,
167184
ttl: int = 60,
168185
proxied: bool = False,
169186
) -> bool:
170-
"""Set an alias record (delete existing and create new).
171-
172-
Creates a CNAME record by default. Some providers may override this
173-
to use A records instead (e.g., Linode to avoid CAA conflicts).
187+
"""Point `name` at `content`, in whatever form this provider allows.
174188
175-
Args:
176-
name: The record name
177-
content: The alias target (domain name)
178-
ttl: Time to live
179-
proxied: Whether to proxy through provider (if supported)
189+
A CNAME by default. Providers override this where a CNAME will not do
190+
-- Linode publishes an address record instead, because it refuses a CAA
191+
beside a CNAME, and delegation needs both on one name.
180192
181-
Returns:
182-
True if successful, False otherwise
193+
Callers should not second-guess the form: which one works is a property
194+
of the provider, and this is where that knowledge lives.
183195
"""
196+
self.clear_conflicting_alias(name, RecordType.CNAME)
184197
return self.set_cname_record(name, content, ttl, proxied)
185198

186199
def set_cname_record(

custom-domain/dstack-ingress/scripts/dns_providers/linode.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ def set_alias_record(
292292
"""
293293
# Resolve domain to IP
294294
domain = content
295+
self.clear_conflicting_alias(name, RecordType.A)
295296
print(f"Trying to resolve: {domain}")
296297
ip_address = socket.gethostbyname(domain)
297298
print(f"✅ Resolved {domain} to IP: {ip_address}")

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -491,12 +491,6 @@ def main() -> int:
491491
default=os.environ.get("DOH_RESOLVERS", os.environ.get("DOH_RESOLVER", DEFAULT_DOH)),
492492
help="comma-separated DoH endpoints",
493493
)
494-
parser.add_argument(
495-
"--resolve",
496-
default="",
497-
help="print the A records for a name and exit (used to publish the "
498-
"gateway address into a delegated zone)",
499-
)
500494
parser.add_argument("--json", action="store_true", help="also emit the records as JSON")
501495
parser.add_argument(
502496
"--include",
@@ -506,20 +500,12 @@ def main() -> int:
506500
)
507501
args = parser.parse_args()
508502

509-
if not args.resolve:
510-
missing = [n for n, v in (("--domain", args.domain),
511-
("--alias-target", args.alias_target),
512-
("--txt-name", args.txt_name),
513-
("--txt-value", args.txt_value)) if not v]
514-
if missing:
515-
parser.error(f"{', '.join(missing)} required unless --resolve is given")
516-
517-
if args.resolve:
518-
# Address lookup for callers that need to publish an A record rather
519-
# than a CNAME. Same resolvers, same union semantics as everything else.
520-
for addr in query_union(args.resolve, RR_A, args.resolver):
521-
print(addr)
522-
return 0
503+
missing = [n for n, v in (("--domain", args.domain),
504+
("--alias-target", args.alias_target),
505+
("--txt-name", args.txt_name),
506+
("--txt-value", args.txt_value)) if not v]
507+
if missing:
508+
parser.error(f"{', '.join(missing)} are required")
523509

524510
if not args.caa_name:
525511
args.caa_name = args.domain.lstrip("*.")

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

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#!/usr/bin/env python3
22

33
from dns_providers import DNSProviderFactory
4-
from dns_providers.base import RecordType
54
import argparse
65
import os
76
import sys
@@ -15,11 +14,10 @@ def main():
1514
)
1615
parser.add_argument(
1716
"action",
18-
choices=["provider", "set_cname", "set_alias", "set_a", "set_txt", "unset_txt", "unset", "set_caa"],
17+
choices=["set_cname", "set_alias", "set_txt", "unset_txt", "set_caa"],
1918
help="Action to perform",
2019
)
2120
parser.add_argument("--domain", default="", help="Domain name")
22-
parser.add_argument("--type", default="", help="Record type, for unset")
2321
parser.add_argument("--provider", help="DNS provider (cloudflare, linode)")
2422
# Zone ID is now handled internally by each provider
2523
parser.add_argument(
@@ -32,11 +30,6 @@ def main():
3230

3331
args = parser.parse_args()
3432

35-
if args.action == "provider":
36-
# Which provider is in use, so callers can adapt to what it allows.
37-
print(DNSProviderFactory._detect_provider_type())
38-
return
39-
4033
if not args.domain:
4134
print("Error: --domain is required", file=sys.stderr)
4235
sys.exit(1)
@@ -78,23 +71,6 @@ def main():
7871
sys.exit(1)
7972
print(f"Successfully set TXT record for {args.domain}")
8073

81-
elif args.action == "set_a":
82-
success = provider.set_a_record(args.domain, args.content)
83-
if not success:
84-
print(f"Failed to set A record for {args.domain}", file=sys.stderr)
85-
sys.exit(1)
86-
print(f"Successfully set A record for {args.domain}")
87-
88-
elif args.action == "unset":
89-
if not args.type:
90-
print("Error: --type is required for unset", file=sys.stderr)
91-
sys.exit(1)
92-
success = provider.unset_records(args.domain, RecordType[args.type.upper()])
93-
if not success:
94-
print(f"Failed to unset {args.type} records for {args.domain}", file=sys.stderr)
95-
sys.exit(1)
96-
print(f"Successfully unset {args.type} records for {args.domain}")
97-
9874
elif args.action == "unset_txt":
9975
success = provider.unset_txt_record(args.domain)
10076
if not success:

0 commit comments

Comments
 (0)