fix(dstack-ingress): make DELEGATION_GATEWAY_RECORD actually switch, and mean

what it says

Two problems with the gateway pointer, both from review.

Switching the setting did not switch the record. set_a_record only
inspects A records and set_cname_record only CNAMEs, so each leaves the
other type in place -- and DNS forbids a CNAME beside anything else, so
the new record then fails to create. Publishing now clears the other type
first, and switching either way was checked against Cloudflare: CNAME to
A removes the CNAME and leaves only the address, and back again.

`cname` did not guarantee a CNAME. The branch called set_alias, and
"alias" is provider-defined: Linode overrides it to create an A record
deliberately, since Linode refuses a CAA beside a CNAME. So on Linode the
setting said cname and produced an address record. The branch now calls
set_cname explicitly, which no provider redefines.

unset_txt_record generalises to unset_records(name, type) for the
clearing step; the TXT wrapper stays for the challenge hook.

Also spell out the rename in the README rather than leaving it to the
changelog. ACME_CHALLENGE_ALIAS is not accepted, and a deployment that
keeps it does not fail loudly -- it silently leaves delegation mode and
starts writing to the served domain's zone, which is the one thing
delegation exists to prevent.

Author: kvinwang

custom-domain/dstack-ingress/README.md

@@ -205,6 +205,12 @@ served name lives under a shared production zone (e.g. `svc.example.com` under
 `example.com`), that token can edit every record in the zone, which may be more
 privilege than you want.
 
+> **Renamed.** This was `ACME_CHALLENGE_ALIAS`, from when the challenge was the
+> only thing delegated. The old name is **not** accepted — a deployment still
+> setting it silently leaves delegation mode and starts writing to the served
+> domain's zone, which is what delegation exists to avoid. Rename it when
+> upgrading.
+
 Set `DELEGATION_ZONE=<delegation-zone>` to move every name this deployment
 needs into a zone your token controls. You create three CNAMEs in the served
 domain's zone **once, before deploying**, and then never touch DNS again — not

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

@@ -148,9 +148,17 @@ delegation_publish() {
     local domain="$1" target txt_name
     target=$(delegated_name "$domain")
 
+    # The set_* helpers only look at the type they are about to write, so a
+    # leftover record of the other type stays and blocks the new one -- DNS
+    # forbids a CNAME beside anything else. Clear it first, so switching
+    # DELEGATION_GATEWAY_RECORD either way actually takes effect.
     case "$(delegation_gateway_record)" in
         cname)
-            dnsman.py set_alias --domain "$target" --content "$GATEWAY_DOMAIN" || return 1
+            dnsman.py unset --domain "$target" --type A >/dev/null 2>&1 || true
+            # set_cname, not set_alias: some providers redefine "alias" to mean
+            # an address record (Linode does, precisely to dodge the CAA/CNAME
+            # conflict), and this branch has to mean what it says.
+            dnsman.py set_cname --domain "$target" --content "$GATEWAY_DOMAIN" || return 1
             ;;
         a)
             # For providers that refuse a CAA beside a CNAME. Costs the
@@ -163,6 +171,7 @@ delegation_publish() {
                 return 1
             fi
             addr=$(echo "$addrs" | head -1)
+            dnsman.py unset --domain "$target" --type CNAME >/dev/null 2>&1 || true
             dnsman.py set_a --domain "$target" --content "$addr" || return 1
             ;;
         *)

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

@@ -257,40 +257,48 @@ def zone_is_resolvable(self, name: str) -> bool:
         """
         return True
 
-    def unset_txt_record(self, name: str) -> bool:
-        """Delete all TXT records for a name.
+    def unset_records(self, name: str, record_type: RecordType) -> bool:
+        """Delete every record of one type at a name.
 
-        Used to clean up ACME DNS-01 challenge records (e.g. from a certbot
-        --manual cleanup hook). Missing records are treated as success.
+        Used to clean up an ACME challenge, and to clear a record type before
+        publishing a different one at the same name -- the set_* helpers only
+        look at the type they are about to write, so a leftover CNAME would
+        otherwise block an A record and vice versa.
 
         Args:
             name: The record name
+            record_type: Which type to remove
 
         Returns:
             True if all matching records were removed (or none existed).
         """
-        # get_dns_records returns [] both for "this name has no TXT records" and
-        # for "the zone could not be resolved", so an empty list on its own would
-        # report a failed cleanup as a success. Ask whether the zone resolves
-        # before believing the emptiness.
-        records = self.get_dns_records(name, RecordType.TXT)
+        # get_dns_records returns [] both for "this name has no such records"
+        # and for "the zone could not be resolved", so an empty list on its own
+        # would report a failed cleanup as a success. Ask whether the zone
+        # resolves before believing the emptiness.
+        records = self.get_dns_records(name, record_type)
         if not records and not self.zone_is_resolvable(name):
             print(
-                f"Error: cannot delete TXT records for {name}: zone lookup failed",
+                f"Error: cannot delete {record_type.value} records for {name}: "
+                f"zone lookup failed",
                 file=sys.stderr,
             )
             return False
 
         ok = True
         for record in records:
             if not record.id:
-                print(f"Warning: TXT record for {name} has no id; cannot delete")
+                print(f"Warning: {record_type.value} record for {name} has no id; cannot delete")
                 ok = False
                 continue
             if not self.delete_dns_record(record.id, name):
                 ok = False
         return ok
 
+    def unset_txt_record(self, name: str) -> bool:
+        """Delete all TXT records for a name."""
+        return self.unset_records(name, RecordType.TXT)
+
     def set_caa_record(
         self,
         name: str,

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

@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 from dns_providers import DNSProviderFactory
+from dns_providers.base import RecordType
 import argparse
 import os
 import sys
@@ -14,10 +15,11 @@ def main():
     )
     parser.add_argument(
         "action",
-        choices=["provider", "set_cname", "set_alias", "set_a", "set_txt", "unset_txt", "set_caa"],
+        choices=["provider", "set_cname", "set_alias", "set_a", "set_txt", "unset_txt", "unset", "set_caa"],
         help="Action to perform",
     )
     parser.add_argument("--domain", default="", help="Domain name")
+    parser.add_argument("--type", default="", help="Record type, for unset")
     parser.add_argument("--provider", help="DNS provider (cloudflare, linode)")
     # Zone ID is now handled internally by each provider
     parser.add_argument(
@@ -83,6 +85,16 @@ def main():
                 sys.exit(1)
             print(f"Successfully set A record for {args.domain}")
 
+        elif args.action == "unset":
+            if not args.type:
+                print("Error: --type is required for unset", file=sys.stderr)
+                sys.exit(1)
+            success = provider.unset_records(args.domain, RecordType[args.type.upper()])
+            if not success:
+                print(f"Failed to unset {args.type} records for {args.domain}", file=sys.stderr)
+                sys.exit(1)
+            print(f"Successfully unset {args.type} records for {args.domain}")
+
         elif args.action == "unset_txt":
             success = provider.unset_txt_record(args.domain)
             if not success: