fix: resolve DNS staging QA failures

CHANGELOG.md

@@ -13,7 +13,9 @@ Format based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), using
 
 ### Fixed
 
-- `dns record-create` prints one row for the value it created. `dns record-delete` prints a pre-delete snapshot of the selected record set in table, JSON, and YAML output, marked by a `SOURCE` column in tables and `source: pre_delete_snapshot` in structured output. The snapshot may differ from the values deleted if the record set changes concurrently, and the CLI warns when it prints one. Both commands support `--show-zone` to print the resulting zone. CAA matching accepts equivalent numeric flags, tag case, and quoted values. MX output includes its preference, including `0`. `dns show` remains the full-zone read command. If a successful delete cannot identify the previous values, the CLI prints an empty result and a warning.
+- `dns record-delete` now deletes each value in the finite pre-delete snapshot with the raw content required by the DNS API, fixing multi-value record-set deletion. Values added after the read are left alone. A failure after partial deletion returns an error and identifies the partial progress instead of reporting a successful snapshot. An already absent DNS record is handled idempotently when the DNS API returns its verified `DNS record not found.` response.
+- `dns record-create` now rejects unquoted CAA values before sending a request; use `--content '0 issue "letsencrypt.org"'`.
+- `dns record-create` prints one row for the value it created. `dns record-delete` prints a pre-delete snapshot of the selected record set in table, JSON, and YAML output, marked by a `SOURCE` column in tables and `source: pre_delete_snapshot` in structured output. Both commands support `--show-zone` to print the resulting zone. MX output includes its preference, including `0`. `dns show` remains the full-zone read command.
 
 ## [v1.0.1] - 2026-10-11
 

docs/commands.md

@@ -442,9 +442,11 @@ zcp dns record-create --domain <domain-slug> --name @ --type MX --content mail.e
 # Record creation shows the affected value. By default, deletion prints a pre-delete
 # snapshot, marked as `Pre-delete snapshot` in table output and
 # `source: pre_delete_snapshot` in JSON/YAML, and warns on stderr that concurrent
-# changes may make it differ from values actually removed. With --show-zone, deletion
+# changes may make it differ from values actually removed. When the selected record set
+# has several values, deletion sends one request for each value in that initial snapshot;
+# values added concurrently are left alone. With --show-zone, deletion
 # prints the resulting zone without the snapshot label or warning.
-# CAA create matching accepts equivalent numeric flags, tag case, and quoted values.
+# CAA values must be quoted, for example: --content '0 issue "letsencrypt.org"'.
 # If a successful delete cannot identify previous values, it prints an empty result and a warning.
 zcp dns record-create --domain <domain-slug> --name www --type A --content 192.0.2.1 --show-zone
 

internal/commands/dns.go

@@ -3,6 +3,7 @@ package commands
 import (
 	"bufio"
 	"context"
+	"errors"
 	"fmt"
 	"net/netip"
 	"os"
@@ -315,6 +316,11 @@ func newDNSRecordCreateCmd() *cobra.Command {
 			if content == "" {
 				return fmt.Errorf("--content is required")
 			}
+			if strings.EqualFold(recordType, "CAA") {
+				if err := validateCAARecordContent(content); err != nil {
+					return err
+				}
+			}
 			if ttl <= 0 {
 				ttl = 14400
 			}
@@ -484,14 +490,36 @@ func runDNSRecordDeleteByName(cmd *cobra.Command, domainSlug, name, recordType s
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(getTimeout(cmd))*time.Second)
 	defer cancel()
 
-	if err := svc.DeleteRecordByName(ctx, domainSlug, fqdn, recordType); err != nil {
-		if apierrors.IsResourceNotFound(err) {
-			fmt.Fprintf(os.Stderr, "DNS record %s %q not found (already deleted).\n", recordType, fqdn)
-			if !showZone {
-				return printDNSRecordRows(printer, nil)
+	if showErr == nil && len(snapshot) == 0 {
+		if showZone {
+			return printDNSRecordRows(printer, domain.Records)
+		}
+		return printDNSRecordRows(printer, nil)
+	}
+
+	if len(snapshot) == 0 {
+		if err := svc.DeleteRecordByName(ctx, domainSlug, fqdn, recordType); err != nil {
+			if isDNSRecordNotFound(err) {
+				fmt.Fprintf(os.Stderr, "DNS record %s %q not found (already deleted).\n", recordType, fqdn)
+				if !showZone {
+					return printDNSRecordRows(printer, nil)
+				}
+			} else {
+				return fmt.Errorf("dns record-delete: %w", err)
 			}
-		} else {
-			return fmt.Errorf("dns record-delete: %w", err)
+		}
+	} else {
+		values := dnsRecordValues(snapshot)
+		deleted := 0
+		for _, value := range values {
+			if err := svc.DeleteRecordValueByName(ctx, domainSlug, fqdn, recordType, value); err != nil {
+				if isDNSRecordNotFound(err) {
+					continue
+				}
+				fmt.Fprintf(os.Stderr, "DNS record-delete stopped after deleting %d of %d values from the pre-delete snapshot; remaining snapshot values may still exist.\n", deleted, len(values))
+				return fmt.Errorf("dns record-delete: %w", err)
+			}
+			deleted++
 		}
 	}
 
@@ -510,6 +538,18 @@ func runDNSRecordDeleteByName(cmd *cobra.Command, domainSlug, name, recordType s
 	return printDNSRecordSnapshotRows(printer, snapshot)
 }
 
+// isDNSRecordNotFound handles the DNS endpoint's verified idempotency
+// response. Unlike the normal API not-found status, it reports an absent DNS
+// record as HTTP 500 with this exact message. Do not broaden this exception:
+// other 500 responses remain failures.
+func isDNSRecordNotFound(err error) bool {
+	if apierrors.IsResourceNotFound(err) {
+		return true
+	}
+	var apiErr *apierrors.APIError
+	return errors.As(err, &apiErr) && apiErr.StatusCode == 500 && strings.TrimSpace(apiErr.Message) == "DNS record not found."
+}
+
 // suppliedDNSRecordFQDN returns a normalized FQDN only when the caller made
 // its absolute form explicit with a trailing dot. It never derives a name from
 // a domain slug, because slugs are not DNS zone names.
@@ -659,6 +699,14 @@ func recordContents(record *dns.Record) []string {
 	return []string{record.Content}
 }
 
+func dnsRecordValues(records []dns.Record) []string {
+	values := make([]string, 0)
+	for i := range records {
+		values = append(values, recordContents(&records[i])...)
+	}
+	return values
+}
+
 func dnsRecordMatchesRequest(record dns.Record, content string, req dns.CreateRecordRequest) bool {
 	if strings.EqualFold(record.Type, "CAA") {
 		return caaRecordMatches(content, req.Content)
@@ -719,6 +767,27 @@ func parseCAAContent(content string) (uint8, string, string, bool) {
 	return uint8(flags), tag, value, true
 }
 
+func validateCAARecordContent(content string) error {
+	original := strings.TrimSpace(content)
+	flagsEnd := strings.IndexAny(original, " \t\n\r")
+	if flagsEnd <= 0 {
+		return fmt.Errorf(`CAA --content must use a quoted value (e.g. 0 issue "letsencrypt.org")`)
+	}
+	remainder := strings.TrimLeft(original[flagsEnd:], " \t\n\r")
+	tagEnd := strings.IndexAny(remainder, " \t\n\r")
+	if tagEnd <= 0 {
+		return fmt.Errorf(`CAA --content must use a quoted value (e.g. 0 issue "letsencrypt.org")`)
+	}
+	value := strings.TrimSpace(remainder[tagEnd:])
+	if _, ok := decodeQuotedTXTContent(value); !ok {
+		return fmt.Errorf(`CAA --content value must be quoted (e.g. 0 issue "letsencrypt.org")`)
+	}
+	if _, _, _, ok := parseCAAContent(original); !ok {
+		return fmt.Errorf(`CAA --content must use the form <flags> <tag> "value" (e.g. 0 issue "letsencrypt.org")`)
+	}
+	return nil
+}
+
 func isCAATag(tag string) bool {
 	for i := range tag {
 		if (tag[i] < 'a' || tag[i] > 'z') && (tag[i] < 'A' || tag[i] > 'Z') && (tag[i] < '0' || tag[i] > '9') {

internal/commands/dns_record_output_test.go

@@ -182,7 +182,7 @@ func TestDNSRecordCreateNormalizesCAAOutput(t *testing.T) {
 			defer srv.Close()
 			t.Setenv("ZCP_BEARER_TOKEN", "test-token")
 
-			out, _, err := execCmd(t, NewDNSCmd(), "record-create", "--domain", "example-com-1", "--name", "caa", "--type", "CAA", "--content", "000 issue letsencrypt.org", "--api-url", srv.URL, "--output", format)
+			out, _, err := execCmd(t, NewDNSCmd(), "record-create", "--domain", "example-com-1", "--name", "caa", "--type", "CAA", "--content", `000 issue "letsencrypt.org"`, "--api-url", srv.URL, "--output", format)
 			if err != nil {
 				t.Fatalf("record-create error = %v", err)
 			}
@@ -360,7 +360,7 @@ func TestDNSRecordDeleteReportsSnapshotAndShowZone(t *testing.T) {
 	if err := json.Unmarshal([]byte(out), &snapshot); err != nil {
 		t.Fatalf("JSON output = %q: %v", out, err)
 	}
-	if deleteCalls != 1 || getCalls != 1 || len(snapshot) != 2 || snapshot[1]["content"] != "192.0.2.2" || snapshot[0]["source"] != "pre_delete_snapshot" || snapshot[1]["source"] != "pre_delete_snapshot" || !strings.Contains(stderr, "snapshot read before deletion") {
+	if deleteCalls != 2 || getCalls != 1 || len(snapshot) != 2 || snapshot[1]["content"] != "192.0.2.2" || snapshot[0]["source"] != "pre_delete_snapshot" || snapshot[1]["source"] != "pre_delete_snapshot" || !strings.Contains(stderr, "snapshot read before deletion") {
 		t.Errorf("get=%d delete=%d snapshot=%#v, want marked pre-delete values", getCalls, deleteCalls, snapshot)
 	}
 
@@ -397,8 +397,8 @@ func TestDNSRecordDeleteMissingAndFailureDoNotFabricateRows(t *testing.T) {
 		t.Setenv("ZCP_BEARER_TOKEN", "test-token")
 
 		out, stderr, err := execCaptureStdio(t, NewDNSCmd(), "record-delete", "--domain", "example-com-1", "--name", "www", "--type", "A", "--yes", "--api-url", srv.URL, "--output", "json")
-		if err != nil || strings.TrimSpace(out) != "[]" || deleteCalls != 1 || !strings.Contains(stderr, "previous values are unavailable") || strings.Contains(stderr, "snapshot read before deletion") {
-			t.Errorf("error=%v output=%q stderr=%q deleteCalls=%d, want empty result and warning after delete", err, out, stderr, deleteCalls)
+		if err != nil || strings.TrimSpace(out) != "[]" || deleteCalls != 0 || stderr != "" {
+			t.Errorf("error=%v output=%q stderr=%q deleteCalls=%d, want empty result without a delete request", err, out, stderr, deleteCalls)
 		}
 	})
 
@@ -415,8 +415,8 @@ func TestDNSRecordDeleteMissingAndFailureDoNotFabricateRows(t *testing.T) {
 		t.Setenv("ZCP_BEARER_TOKEN", "test-token")
 
 		out, stderr, err := execCaptureStdio(t, NewDNSCmd(), "record-delete", "--domain", "example-com-1", "--name", "www", "--type", "A", "--yes", "--api-url", srv.URL, "--output", "json")
-		if err != nil || strings.TrimSpace(out) != "[]" || strings.Contains(stderr, "previous values are unavailable") || strings.Contains(stderr, "snapshot read before deletion") {
-			t.Errorf("error=%v output=%q stderr=%q, want empty result without missing-details warning for an already deleted record", err, out, stderr)
+		if err != nil || !strings.Contains(out, "192.0.2.1") || !strings.Contains(stderr, "snapshot read before deletion") {
+			t.Errorf("error=%v output=%q stderr=%q, want the pre-delete snapshot when the value disappears after the read", err, out, stderr)
 		}
 	})
 
@@ -599,10 +599,12 @@ func TestDNSRecordDeleteAlreadyDeletedShowZone(t *testing.T) {
 		args          []string
 		showZone      bool
 		postShowError bool
+		wantSnapshot  bool
 	}{
 		{
-			name: "name and type without show zone",
-			args: []string{"--name", "www", "--type", "A"},
+			name:         "name and type without show zone",
+			args:         []string{"--name", "www", "--type", "A"},
+			wantSnapshot: true,
 		},
 		{
 			name:     "name and type with show zone",
@@ -680,6 +682,13 @@ func TestDNSRecordDeleteAlreadyDeletedShowZone(t *testing.T) {
 				}
 				return
 			}
+			if tt.wantSnapshot {
+				var rows []map[string]string
+				if json.Unmarshal([]byte(out), &rows) != nil || showCalls != 1 || len(rows) != 1 || rows[0]["content"] != "192.0.2.1" || rows[0]["source"] != "pre_delete_snapshot" || strings.Contains(stderr, "previous values are unavailable") || !strings.Contains(stderr, "snapshot read before deletion") {
+					t.Errorf("show=%d output=%q stderr=%q, want pre-delete snapshot and warning when the value was already deleted", showCalls, out, stderr)
+				}
+				return
+			}
 			if showCalls != 1 || strings.TrimSpace(out) != "[]" || strings.Contains(stderr, "previous values are unavailable") || strings.Contains(stderr, "snapshot read before deletion") {
 				t.Errorf("show=%d output=%q stderr=%q, want empty output without missing-details warning", showCalls, out, stderr)
 			}
@@ -805,3 +814,175 @@ func TestDNSRecordDeleteLegacyIDGetsFreshTimeoutAfterPreRead(t *testing.T) {
 		t.Errorf("error=%v delete=%d output=%q stderr=%q, want successful delete with empty rows and warning", err, deleteCalls, out, stderr)
 	}
 }
+
+func TestDNSRecordDeleteUsesRawInitialSnapshotValues(t *testing.T) {
+	tests := []struct {
+		name       string
+		recordType string
+		contents   []string
+	}{
+		{"A", "A", []string{"192.0.2.10", "192.0.2.11"}},
+		{"MX", "MX", []string{"0 mail.example.com.", "10 backup.example.com."}},
+		{"TXT", "TXT", []string{`"one"`, `"two"`}},
+		{"CAA", "CAA", []string{`0 issue "letsencrypt.org"`, `0 iodef "mailto:security@example.com"`}},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var contents []string
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				switch r.Method {
+				case http.MethodGet:
+					payload, err := json.Marshal(tt.contents)
+					if err != nil {
+						t.Fatal(err)
+					}
+					fmt.Fprintf(w, `{"status":"Success","data":{"name":"example.com","records":[{"name":"value.example.com.","type":%q,"ttl":60,"contents":%s}]}}`, tt.recordType, payload)
+				case http.MethodDelete:
+					content := r.URL.Query().Get("content")
+					if content == "" {
+						w.WriteHeader(http.StatusInternalServerError)
+						fmt.Fprint(w, `{"status":"Error","message":"Content is required to delete a record from a multi-value record set."}`)
+						return
+					}
+					contents = append(contents, content)
+					fmt.Fprint(w, `{"status":"Success"}`)
+				default:
+					http.NotFound(w, r)
+				}
+			}))
+			defer srv.Close()
+			t.Setenv("ZCP_BEARER_TOKEN", "test-token")
+
+			out, _, err := execCaptureStdio(t, NewDNSCmd(), "record-delete", "--domain", "example-com-1", "--name", "value", "--type", tt.recordType, "--yes", "--api-url", srv.URL, "--output", "json")
+			if err != nil {
+				t.Fatalf("record-delete error = %v", err)
+			}
+			if got, want := strings.Join(contents, "|"), strings.Join(tt.contents, "|"); got != want {
+				t.Errorf("delete content = %q, want %q", got, want)
+			}
+			if !strings.Contains(out, "pre_delete_snapshot") {
+				t.Errorf("output = %q, want snapshot rows", out)
+			}
+		})
+	}
+}
+
+func TestDNSRecordDeleteStopsOnPartialFailureWithoutSnapshotOutput(t *testing.T) {
+	var contents []string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.Method {
+		case http.MethodGet:
+			fmt.Fprint(w, `{"status":"Success","data":{"name":"example.com","records":[{"name":"www.example.com.","type":"A","ttl":60,"contents":["192.0.2.10","192.0.2.11"]}]}}`)
+		case http.MethodDelete:
+			contents = append(contents, r.URL.Query().Get("content"))
+			if len(contents) == 2 {
+				http.Error(w, "backend unavailable", http.StatusServiceUnavailable)
+				return
+			}
+			fmt.Fprint(w, `{"status":"Success"}`)
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	defer srv.Close()
+	t.Setenv("ZCP_BEARER_TOKEN", "test-token")
+
+	out, stderr, err := execCaptureStdio(t, NewDNSCmd(), "record-delete", "--domain", "example-com-1", "--name", "www", "--type", "A", "--yes", "--api-url", srv.URL, "--output", "json")
+	if err == nil || !strings.Contains(stderr, "stopped after deleting 1 of 2 values") || strings.Contains(out, "pre_delete_snapshot") {
+		t.Errorf("error=%v output=%q stderr=%q, want partial failure without success output", err, out, stderr)
+	}
+	if got := strings.Join(contents, "|"); got != "192.0.2.10|192.0.2.11" {
+		t.Errorf("delete content = %q, want both initial values in order", got)
+	}
+}
+
+func TestDNSRecordDeleteDoesNotDrainConcurrentValues(t *testing.T) {
+	var deleteCalls, showCalls int
+	remaining := []string{"192.0.2.10"}
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		switch r.Method {
+		case http.MethodGet:
+			showCalls++
+			payload, err := json.Marshal(remaining)
+			if err != nil {
+				t.Fatal(err)
+			}
+			fmt.Fprintf(w, `{"status":"Success","data":{"name":"example.com","records":[{"name":"www.example.com.","type":"A","ttl":60,"contents":%s}]}}`, payload)
+		case http.MethodDelete:
+			deleteCalls++
+			if r.URL.Query().Get("content") != "192.0.2.10" {
+				http.Error(w, "unexpected value", http.StatusBadRequest)
+				return
+			}
+			remaining = []string{"192.0.2.99"}
+			fmt.Fprint(w, `{"status":"Success"}`)
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	defer srv.Close()
+	t.Setenv("ZCP_BEARER_TOKEN", "test-token")
+
+	out, _, err := execCaptureStdio(t, NewDNSCmd(), "record-delete", "--domain", "example-com-1", "--name", "www", "--type", "A", "--yes", "--show-zone", "--api-url", srv.URL, "--output", "json")
+	if err != nil || deleteCalls != 1 || showCalls != 2 || !strings.Contains(out, "192.0.2.99") {
+		t.Errorf("error=%v deleteCalls=%d showCalls=%d output=%q, want only the initial value deleted and the concurrent value preserved", err, deleteCalls, showCalls, out)
+	}
+}
+
+func TestDNSRecordDeleteTreatsOnlyExactDNS500AsAbsent(t *testing.T) {
+	for _, tt := range []struct {
+		name      string
+		message   string
+		wantError bool
+	}{
+		{"verified absent response", "DNS record not found.", false},
+		{"unrelated server failure", "database unavailable", true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				switch r.Method {
+				case http.MethodGet:
+					http.Error(w, "pre-read unavailable", http.StatusServiceUnavailable)
+				case http.MethodDelete:
+					w.Header().Set("Content-Type", "application/json")
+					w.WriteHeader(http.StatusInternalServerError)
+					fmt.Fprintf(w, `{"status":"Error","message":%q}`, tt.message)
+				default:
+					http.NotFound(w, r)
+				}
+			}))
+			defer srv.Close()
+			t.Setenv("ZCP_BEARER_TOKEN", "test-token")
+
+			out, stderr, err := execCaptureStdio(t, NewDNSCmd(), "record-delete", "--domain", "example-com-1", "--name", "www.example.com.", "--type", "A", "--yes", "--api-url", srv.URL, "--output", "json")
+			if tt.wantError {
+				if err == nil || strings.Contains(out, "pre_delete_snapshot") {
+					t.Errorf("error=%v output=%q, want unrelated 500 to fail", err, out)
+				}
+				return
+			}
+			if err != nil || strings.TrimSpace(out) != "[]" || !strings.Contains(stderr, "not found (already deleted)") {
+				t.Errorf("error=%v output=%q stderr=%q, want idempotent absent result", err, out, stderr)
+			}
+		})
+	}
+}
+
+func TestDNSRecordCreateRejectsUnquotedCAABeforeHTTP(t *testing.T) {
+	var calls int
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		calls++
+		http.Error(w, "must not be called", http.StatusInternalServerError)
+	}))
+	defer srv.Close()
+	t.Setenv("ZCP_BEARER_TOKEN", "test-token")
+
+	_, _, err := execCaptureStdio(t, NewDNSCmd(), "record-create", "--domain", "example-com-1", "--name", "caa", "--type", "CAA", "--content", "0 issue letsencrypt.org", "--api-url", srv.URL)
+	if err == nil || !strings.Contains(err.Error(), `0 issue "letsencrypt.org"`) || calls != 0 {
+		t.Errorf("error=%v calls=%d, want local quoted-CAA validation without HTTP", err, calls)
+	}
+}