From 909821107dffa5d5521d9c0ea6405ec128d1f60c Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 12:34:22 -0700 Subject: [PATCH 1/9] refactor(dns): split dns module into per-command files; fix CNAME conflict errors Refresh the Domains v3 OpenAPI spec and use it to fix `dns set`/`dns add` surfacing an opaque error on a CNAME name-exclusivity conflict, plus a related fix in `domain purchase`. Also break up the ~1760-line dns/mod.rs into per-command files (list/add/set/delete) with shared helpers split into records.rs and conflicts.rs, mirroring the domain/ module's layout for easier navigation. No behavior change from the reorganization itself. Co-Authored-By: Claude Sonnet 5 --- rust/domains-client/openapi/domains.oas3.json | 129 +- .../openapi/swagger_domains.v3.yaml | 151 ++- rust/domains-client/src/lib.rs | 37 +- rust/src/dns/add.rs | 214 ++++ rust/src/dns/conflicts.rs | 296 +++++ rust/src/dns/delete.rs | 183 +++ rust/src/dns/list.rs | 75 ++ rust/src/dns/mod.rs | 1112 +---------------- rust/src/dns/records.rs | 412 ++++++ rust/src/dns/set.rs | 522 ++++++++ rust/src/domain/common.rs | 59 +- rust/src/domain/guides/domain-purchase.md | 3 - rust/src/domain/mod.rs | 2 +- rust/src/domain/purchase.rs | 86 +- 14 files changed, 2039 insertions(+), 1242 deletions(-) create mode 100644 rust/src/dns/add.rs create mode 100644 rust/src/dns/conflicts.rs create mode 100644 rust/src/dns/delete.rs create mode 100644 rust/src/dns/list.rs create mode 100644 rust/src/dns/records.rs create mode 100644 rust/src/dns/set.rs diff --git a/rust/domains-client/openapi/domains.oas3.json b/rust/domains-client/openapi/domains.oas3.json index 162e4a8d..0745f3aa 100644 --- a/rust/domains-client/openapi/domains.oas3.json +++ b/rust/domains-client/openapi/domains.oas3.json @@ -2,7 +2,7 @@ "openapi": "3.0.3", "info": { "title": "Domain Lifecycle Management API", - "version": "3.0.0", + "version": "3.1.0", "x-visibility": "public", "description": "The GoDaddy Domain Lifecycle Management API provides comprehensive capabilities\nfor discovering, registering, managing, renewing, transferring, and reselling\ndomain names. This is major version 3, designed for agent-first interactions\nwhile remaining fully usable by direct API clients and resellers.\n\n## Namespace\n\nAll paths are under `/v3/domains/`. The namespace is `domains` (the business capability);\nthe core entity collection is `/domain-names`.\n\n## Key Conventions\n\n**Quote/execute for commercial operations.** Every commercial mutation (register,\nrenew, transfer) requires a `quoteToken` minted by the corresponding quote\ncollection endpoint. Execution without a prior quote is structurally impossible.\nThe token locks the price, resolved settings, and required legal agreements for a\n10-minute TTL. Quote calls are free, read-only, and safe to call speculatively.\n\n**Async commercial operations.** `POST /registrations`, `POST /renewals`, and\n`POST /transfers` each return `202 Accepted` with the concrete entity body\n(`Registration`, `Renewal`, or `Transfer`) and a `Location` header. Poll\n`links[rel=self]` on the returned entity until status is `COMPLETED` or `FAILED`.\nEach concrete resource is also reachable via `GET /operations/{operationId}`;\nthe `operationId` is included in the entity for clients that prefer the abstract\nview. Non-commercial mutations return a `DomainOperation` body.\n\n**Entity-oriented resource model.** Domains are the core entity of this API\n(exposed as `/domain-names` in the path to distinguish the resource collection\nfrom the `domains` namespace prefix). Register, renew, and transfer are\ncommercial actions executed by `POST` to their corresponding top-level\nresource collections (`/registrations`, `/renewals`, `/transfers`); each\naccepts a prior quote and returns an async entity to poll until completion.\nFor commercial execute calls, the target domain is\nexpressed in the request body, not the path. Sub-resources (`contacts`,\n`nameservers`, `privacy`, `auto-renew`, `transfer-lock`, `records`) only exist\nin the context of a specific domain-name instance. The `/check-availability`\ncontroller accepts GET (single domain) or POST (1\u201350 domains) and carries no\npersistent identity.\n\n**Flat, two-level maximum.** No resource path goes deeper than\n`/{collection}/{id}/{sub-resource}` or `/{collection}/{id}/{sub-collection}/{id}`.\n\n**Reseller on-behalf-of.** Resellers pass `X-Shopper-Id`; all operations are\nthen scoped to that shopper. Absent the header, the authenticated entity's own\naccount is used.\n\n## Launch Scope (v3.0)\nStandard TLDs only. TLDs with eligibility requirements (.us, .ca, .eu) return\n`UNSUPPORTED_TLD` until Phase 2.\n", "contact": { @@ -590,14 +590,9 @@ "quoteToken": "7f3a2b1c-9d8e-4012-a5b6-c1d2e3f4a5b6", "consent": { "agreementTypes": [ - "DNRA" + "API_DPA" ], - "agreedAt": "2026-06-12T10:02:00Z", - "agreedBy": { - "type": "DIRECT", - "principal": "shopper_123", - "ip": "203.0.113.7" - } + "agreedAt": "2026-06-12T10:02:00Z" } } } @@ -630,6 +625,41 @@ "application/json": { "schema": { "$ref": "#/components/schemas/Registration" + }, + "examples": { + "accepted": { + "summary": "Registration accepted with server-derived consent", + "value": { + "registrationId": "3a8f1c2d-7e6b-4f9a-b1c0-d2e3f4a5b6c7", + "domain": "example.com", + "period": 1, + "price": { + "currencyCode": "USD", + "value": 1199 + }, + "consent": { + "agreementTypes": [ + "API_DPA" + ], + "agreedAt": "2026-06-12T10:02:00Z", + "agreedBy": { + "type": "DIRECT", + "principal": "550e8400-e29b-41d4-a716-446655440000", + "ip": "203.0.113.7" + } + }, + "status": "CONFIRMED", + "operationId": "9f1c2e7a-4b3d-4e8f-a1c2-3d4e5f6a7b8c", + "createdAt": "2026-06-12T10:02:05Z", + "updatedAt": "2026-06-12T10:02:05Z", + "links": [ + { + "rel": "self", + "href": "/v3/domains/registrations/3a8f1c2d-7e6b-4f9a-b1c0-d2e3f4a5b6c7" + } + ] + } + } } } } @@ -1841,7 +1871,7 @@ } }, "409": { - "description": "Conflict \u2014 the request cannot be completed in the current state. Used for quote lifecycle errors (quote_expired, quote_mismatch, quote_consumed, consent_principal_mismatch), domain state conflicts such as domain_already_exists, and immutable DNS records (dns_record_not_mutable) such as GoDaddy-managed SOA and NS records.\n", + "description": "Conflict \u2014 the request cannot be completed in the current state. Used for quote lifecycle errors (quote_expired, quote_mismatch, quote_consumed), domain state conflicts such as domain_already_exists, and immutable DNS records (dns_record_not_mutable) such as GoDaddy-managed SOA and NS records.\n", "headers": { "X-Request-Id": { "$ref": "#/components/headers/xRequestId" @@ -1912,7 +1942,7 @@ "properties": { "agreementType": { "description": "The type of legal agreement. Identifies which agreement text the customer must accept.\n", - "example": "DNRA", + "example": "API_DPA", "allOf": [ { "$ref": "#/components/schemas/AgreementType" @@ -2022,12 +2052,11 @@ }, "Consent": { "title": "Consent", - "description": "Customer consent record for a domain operation, capturing which legal agreements were accepted, when, and by whom. This object is self-reported by the caller and treated as supplementary attestation. The server verifies agreedBy.principal against the authenticated identity (auth token + X-Shopper-Id resolution) and rejects with consent_principal_mismatch on disagreement. The persisted consent record is the union of this claimed block and the verified auth context.\n", + "description": "Customer consent record for a domain operation, capturing which legal agreements were accepted, when, and by whom. On execute, the caller supplies agreementTypes and agreedAt. The server derives agreedBy from the authenticated request context (OAuth identity, X-Shopper-Id, client IP, and transmission channel).\n", "type": "object", "required": [ "agreementTypes", - "agreedAt", - "agreedBy" + "agreedAt" ], "properties": { "agreementTypes": { @@ -2038,7 +2067,7 @@ }, "description": "The agreement types the customer accepted. Must match the agreementType values returned in the corresponding quote's requiredAgreements array.\n", "example": [ - "DNRA" + "API_DPA" ] }, "agreedAt": { @@ -2050,13 +2079,19 @@ ] }, "agreedBy": { - "$ref": "#/components/schemas/ConsentActor" + "description": "Who gave consent and how it was transmitted. Server-derived from the execute request's auth context.\n", + "readOnly": true, + "allOf": [ + { + "$ref": "#/components/schemas/ConsentActor" + } + ] } } }, "ConsentActor": { "title": "Consent Actor", - "description": "Identifies who gave consent and who transmitted it. One uniform schema for all actor types. Self-reported by the caller and treated as supplementary attestation; the server verifies principal against the resolved auth identity (OAuth token + X-Shopper-Id) and rejects with consent_principal_mismatch on disagreement. The persisted consent record is the union of this block and the verified auth context.\nprincipal identifies the account holder whose consent is being recorded. actor identifies the automated or intermediary party that transmitted the consent when different from the principal. For DIRECT, actor is omitted.\n", + "description": "Identifies who gave consent and how it was transmitted. One uniform schema for all actor types. Populated by the server from the execute request's authenticated context \u2014 not supplied by the caller.\n", "type": "object", "x-sensitivity": "confidential", "required": [ @@ -2065,7 +2100,7 @@ ], "properties": { "type": { - "description": "Who transmitted consent relative to the principal.\n", + "description": "How consent was transmitted relative to the principal. Server-assigned from the request's auth and channel context.\n", "example": "DIRECT", "allOf": [ { @@ -2075,17 +2110,17 @@ }, "principal": { "type": "string", - "description": "The shopper or account ID whose consent is being recorded. Must match the shopper resolved from the OAuth token and X-Shopper-Id header; mismatch returns consent_principal_mismatch.\n", - "example": "shopper_123" + "description": "The resolved customer identifier whose consent is recorded, derived from the auth context on the execute request.\n", + "example": "550e8400-e29b-41d4-a716-446655440000" }, "actor": { "type": "string", - "description": "The automated agent or system that transmitted the consent. Omitted for DIRECT. For AGENT, identifies the specific agent instance. For RESELLER, may identify the reseller when distinct from the OAuth token subject.\n", + "description": "The party that transmitted the consent when distinct from the principal. Omitted for DIRECT.\n", "example": "agent:claude/atlas-1" }, "ip": { "type": "string", - "description": "The IP address of the principal at the time consent was expressed, if known. Optional \u2014 an absent IP with a verified principal is preferred over a fabricated one.\n", + "description": "Client IP address captured from the execute request at the time consent was recorded. Omitted when unavailable.\n", "example": "203.0.113.7" } } @@ -2463,6 +2498,11 @@ "description": "The domain name to register, in punycode A-label form for IDNs. Must match the domain in the quoteToken.\n", "example": "example.com" }, + "orderId": { + "type": "string", + "description": "The orderId of the order that created this registration. Present only when the registration is completed. Not present for failed or pending registrations.\n", + "readOnly": true + }, "period": { "type": "integer", "minimum": 1, @@ -2471,6 +2511,15 @@ "description": "Registration period in years. Must match the period in the quote.", "example": 1 }, + "price": { + "description": "The locked registration price for the quoted period. Held for the duration of the quoteToken's TTL. Represents the total amount that will be charged on execute.\n", + "readOnly": true, + "allOf": [ + { + "$ref": "#/components/schemas/simple-money" + } + ] + }, "profileId": { "description": "ID of a saved registration profile to use for contacts and preference defaults. Omit to fall back to the account-default profile for the domain's TLD, then to account identity.\n", "allOf": [ @@ -2498,7 +2547,7 @@ ] }, "consent": { - "description": "The customer's consent record for the legal agreements returned in the quote. Must reference the same agreementTypes as the quote.\n", + "description": "The customer's consent for the legal agreements returned in the quote. On execute, supply agreementTypes and agreedAt; agreedBy is server-derived and returned on the persisted record.\n", "allOf": [ { "$ref": "#/components/schemas/Consent" @@ -3148,22 +3197,41 @@ }, "AgreementType": { "title": "Agreement Type", - "description": "The type of legal agreement that must be accepted prior to executing a domain operation. Additional agreement types may be returned for specific TLDs or product combinations. DNRA \u2014 Domain Name Registration Agreement. DNTA \u2014 Domain Name Transfer Agreement. DNPA \u2014 Domain Name Privacy Agreement. HTTPS_NOTICE \u2014 HTTPS notice acknowledgment for eligible TLDs. AURA \u2014 AU Domain Agreement for .au TLD registrations and transfers. CIRA \u2014 Canadian Internet Registration Authority Agreement for .ca TLD registrations and transfers.\n", - "type": "string" + "description": "The type of legal agreement that must be accepted prior to executing a domain operation. Additional agreement types may be returned for specific TLDs or product combinations. API_DPA - API Domain Purchase Agreement. DNTA \u2014 Domain Name Transfer Agreement. DNPA \u2014 Domain Name Privacy Agreement. HTTPS_NOTICE \u2014 HTTPS notice acknowledgment for eligible TLDs. AURA \u2014 AU Domain Agreement for .au TLD registrations and transfers. CIRA \u2014 Canadian Internet Registration Authority Agreement for .ca TLD registrations and transfers.\n", + "type": "string", + "enum": [ + "API_DPA", + "AURA", + "CIRA", + "DNPA", + "DNTA", + "HTTPS_NOTICE" + ] }, "ConsentActorType": { "title": "Consent Actor Type", - "description": "Who transmitted consent on behalf of the principal. DIRECT \u2014 the principal acted directly; actor is omitted. AGENT \u2014 an AI or automation acted on behalf of the principal. RESELLER \u2014 a reseller acted on behalf of a shopper.\n", - "type": "string" + "description": "How consent was transmitted relative to the principal. DIRECT \u2014 the principal transmitted consent directly; actor is omitted. AGENT \u2014 consent was transmitted by an AI or automation on the principal's behalf. RESELLER \u2014 consent was transmitted by a reseller on the principal's behalf.\n", + "type": "string", + "enum": [ + "AGENT", + "DIRECT", + "RESELLER" + ] }, "DomainOperationStatus": { "title": "Domain Operation Status", "description": "The execution state of an asynchronous domain operation. CONFIRMED \u2014 operation has been accepted and is queued for execution. EXECUTING \u2014 operation is actively being processed by the registry or downstream systems. COMPLETED \u2014 operation finished successfully; result data is available. FAILED \u2014 operation terminated with an unrecoverable error; error detail is attached.\n", - "type": "string" + "type": "string", + "enum": [ + "COMPLETED", + "CONFIRMED", + "EXECUTING", + "FAILED" + ] }, "DomainStatus": { "title": "Domain Status", - "description": "The lifecycle status of a registered domain. Reflects the domain's current operational state within the registry and GoDaddy's management layer. ACTIVE \u2014 domain is registered and is active. CANCELLED \u2014 domain has been cancelled by the user or system, and is not reclaimable. DELETED_REDEEMABLE \u2014 domain is in ICANN redemption grace period; recovery fees apply. EXPIRED \u2014 registration period has ended; domain is pending deletion or redemption. FAILED - domain registration or transfer error. HELD_REGISTRAR - domain is held at the registrar and cannot be transferred or modified - this is usually the result of a dispute. LOCKED_REGISTRAR \u2014 domain is locked at the registrar - this is usually the result of spam, abuse, etc. OWNERSHIP_CHANGED - domain has been moved to another account. PARKED - domain has been parked. PENDING_REGISTRATION - domain is pending setup at the registry. PENDING_TRANSFER \u2014 an outbound transfer to another registrar is in progress. REPOSSESSED - domain has been confiscated - this is usually the result of a chargeback, fraud, abuse, etc. SUSPENDED \u2014 domain has been administratively suspended by the registry or registrar. TRANSFERRED - domain has been transferred to another registrar.\n", + "description": "The lifecycle status of a registered domain. Reflects the domain's current operational state within the registry and GoDaddy's management layer. ACTIVE \u2014 domain is registered and is active. CANCELLED \u2014 domain has been cancelled by the user or system, and is not be reclaimable. DELETED_REDEEMABLE \u2014 domain is in ICANN redemption grace period; recovery fees apply. EXPIRED \u2014 registration period has ended; domain is pending deletion or redemption. FAILED - domain registration or transfer error. HELD_REGISTRAR - domain is held at the registrar and cannot be transferred or modified - this is usually the result of a dispute. LOCKED_REGISTRAR \u2014 domain is locked at the registrar - this is usually the result of a spam, abuse, etc. OWNERSHIP_CHANGED - domain has been moved to another account. PARKED - domain has been parked. PENDING_REGISTRATION - domain is pending setup at the registry. PENDING_TRANSFER \u2014 an outbound transfer to another registrar is in progress. REPOSSESSED - domain has been confiscated - this is usually the result of a chargeback, fraud, abuse, etc. SUSPENDED \u2014 domain has been administratively suspended by the registry or registrar. TRANSFERRED - domain has been transferred to another registrar.\n", "type": "string", "enum": [ "ACTIVE", @@ -3185,7 +3253,10 @@ "DomainOperationType": { "title": "Domain Operation Type", "description": "The type of asynchronous domain operation. Used to distinguish which workflow is being polled on the /operations/{operationId} endpoint. REGISTER \u2014 new domain registration.\n", - "type": "string" + "type": "string", + "enum": [ + "REGISTER" + ] }, "DNSRecordType": { "title": "DNS Record Type", diff --git a/rust/domains-client/openapi/swagger_domains.v3.yaml b/rust/domains-client/openapi/swagger_domains.v3.yaml index 825d3fc0..e8a216c0 100644 --- a/rust/domains-client/openapi/swagger_domains.v3.yaml +++ b/rust/domains-client/openapi/swagger_domains.v3.yaml @@ -1,7 +1,7 @@ openapi: 3.0.3 info: title: Domain Lifecycle Management API - version: 3.0.0 + version: 3.1.0 x-visibility: public description: 'The GoDaddy Domain Lifecycle Management API provides comprehensive capabilities @@ -670,12 +670,8 @@ paths: quoteToken: 7f3a2b1c-9d8e-4012-a5b6-c1d2e3f4a5b6 consent: agreementTypes: - - DNRA + - API_DPA agreedAt: '2026-06-12T10:02:00Z' - agreedBy: - type: DIRECT - principal: shopper_123 - ip: 203.0.113.7 security: - oauth2: - domains.domain:create @@ -695,6 +691,31 @@ paths: application/json: schema: $ref: '#/components/schemas/Registration' + examples: + accepted: + summary: Registration accepted with server-derived consent + value: + registrationId: 3a8f1c2d-7e6b-4f9a-b1c0-d2e3f4a5b6c7 + domain: example.com + period: 1 + price: + currencyCode: USD + value: 1199 + consent: + agreementTypes: + - API_DPA + agreedAt: '2026-06-12T10:02:00Z' + agreedBy: + type: DIRECT + principal: 550e8400-e29b-41d4-a716-446655440000 + ip: 203.0.113.7 + status: CONFIRMED + operationId: 9f1c2e7a-4b3d-4e8f-a1c2-3d4e5f6a7b8c + createdAt: '2026-06-12T10:02:05Z' + updatedAt: '2026-06-12T10:02:05Z' + links: + - rel: self + href: /v3/domains/registrations/3a8f1c2d-7e6b-4f9a-b1c0-d2e3f4a5b6c7 '400': $ref: '#/components/responses/400' '401': @@ -1415,8 +1436,8 @@ components: $ref: '#/components/schemas/error' '409': description: 'Conflict — the request cannot be completed in the current state. Used for quote lifecycle errors (quote_expired, - quote_mismatch, quote_consumed, consent_principal_mismatch), domain state conflicts such as domain_already_exists, - and immutable DNS records (dns_record_not_mutable) such as GoDaddy-managed SOA and NS records. + quote_mismatch, quote_consumed), domain state conflicts such as domain_already_exists, and immutable DNS records (dns_record_not_mutable) + such as GoDaddy-managed SOA and NS records. ' headers: @@ -1474,7 +1495,7 @@ components: description: 'The type of legal agreement. Identifies which agreement text the customer must accept. ' - example: DNRA + example: API_DPA allOf: - $ref: '#/components/schemas/AgreementType' title: @@ -1594,16 +1615,14 @@ components: Consent: title: Consent description: 'Customer consent record for a domain operation, capturing which legal agreements were accepted, when, - and by whom. This object is self-reported by the caller and treated as supplementary attestation. The server verifies - agreedBy.principal against the authenticated identity (auth token + X-Shopper-Id resolution) and rejects with consent_principal_mismatch - on disagreement. The persisted consent record is the union of this claimed block and the verified auth context. + and by whom. On execute, the caller supplies agreementTypes and agreedAt. The server derives agreedBy from the authenticated + request context (OAuth identity, X-Shopper-Id, client IP, and transmission channel). ' type: object required: - agreementTypes - agreedAt - - agreedBy properties: agreementTypes: type: array @@ -1615,7 +1634,7 @@ components: ' example: - - DNRA + - API_DPA agreedAt: description: 'The timestamp at which the principal expressed consent. Should reflect when the customer clicked accept or confirmed the operation, not when the API call was made. @@ -1624,16 +1643,16 @@ components: allOf: - $ref: '#/components/schemas/date-time' agreedBy: - $ref: '#/components/schemas/ConsentActor' + description: 'Who gave consent and how it was transmitted. Server-derived from the execute request''s auth context. + + ' + readOnly: true + allOf: + - $ref: '#/components/schemas/ConsentActor' ConsentActor: title: Consent Actor - description: 'Identifies who gave consent and who transmitted it. One uniform schema for all actor types. Self-reported - by the caller and treated as supplementary attestation; the server verifies principal against the resolved auth identity - (OAuth token + X-Shopper-Id) and rejects with consent_principal_mismatch on disagreement. The persisted consent record - is the union of this block and the verified auth context. - - principal identifies the account holder whose consent is being recorded. actor identifies the automated or intermediary - party that transmitted the consent when different from the principal. For DIRECT, actor is omitted. + description: 'Identifies who gave consent and how it was transmitted. One uniform schema for all actor types. Populated + by the server from the execute request''s authenticated context — not supplied by the caller. ' type: object @@ -1643,7 +1662,8 @@ components: - principal properties: type: - description: 'Who transmitted consent relative to the principal. + description: 'How consent was transmitted relative to the principal. Server-assigned from the request''s auth and + channel context. ' example: DIRECT @@ -1651,22 +1671,21 @@ components: - $ref: '#/components/schemas/ConsentActorType' principal: type: string - description: 'The shopper or account ID whose consent is being recorded. Must match the shopper resolved from the - OAuth token and X-Shopper-Id header; mismatch returns consent_principal_mismatch. + description: 'The resolved customer identifier whose consent is recorded, derived from the auth context on the execute + request. ' - example: shopper_123 + example: 550e8400-e29b-41d4-a716-446655440000 actor: type: string - description: 'The automated agent or system that transmitted the consent. Omitted for DIRECT. For AGENT, identifies - the specific agent instance. For RESELLER, may identify the reseller when distinct from the OAuth token subject. + description: 'The party that transmitted the consent when distinct from the principal. Omitted for DIRECT. ' example: agent:claude/atlas-1 ip: type: string - description: 'The IP address of the principal at the time consent was expressed, if known. Optional — an absent - IP with a verified principal is preferred over a fabricated one. + description: 'Client IP address captured from the execute request at the time consent was recorded. Omitted when + unavailable. ' example: 203.0.113.7 @@ -2097,6 +2116,13 @@ components: ' example: example.com + orderId: + type: string + description: 'The orderId of the order that created this registration. Present only when the registration is completed. + Not present for failed or pending registrations. + + ' + readOnly: true period: type: integer minimum: 1 @@ -2104,6 +2130,14 @@ components: default: 1 description: Registration period in years. Must match the period in the quote. example: 1 + price: + description: 'The locked registration price for the quoted period. Held for the duration of the quoteToken''s TTL. + Represents the total amount that will be charged on execute. + + ' + readOnly: true + allOf: + - $ref: '#/components/schemas/simple-money' profileId: description: 'ID of a saved registration profile to use for contacts and preference defaults. Omit to fall back to the account-default profile for the domain''s TLD, then to account identity. @@ -2129,8 +2163,8 @@ components: allOf: - $ref: '#/components/schemas/uuid' consent: - description: 'The customer''s consent record for the legal agreements returned in the quote. Must reference the - same agreementTypes as the quote. + description: 'The customer''s consent for the legal agreements returned in the quote. On execute, supply agreementTypes + and agreedAt; agreedBy is server-derived and returned on the persisted record. ' allOf: @@ -2803,20 +2837,32 @@ components: AgreementType: title: Agreement Type description: 'The type of legal agreement that must be accepted prior to executing a domain operation. Additional agreement - types may be returned for specific TLDs or product combinations. DNRA — Domain Name Registration Agreement. DNTA — - Domain Name Transfer Agreement. DNPA — Domain Name Privacy Agreement. HTTPS_NOTICE — HTTPS notice acknowledgment for - eligible TLDs. AURA — AU Domain Agreement for .au TLD registrations and transfers. CIRA — Canadian Internet Registration - Authority Agreement for .ca TLD registrations and transfers. + types may be returned for specific TLDs or product combinations. API_DPA - API Domain Purchase Agreement. DNTA — Domain + Name Transfer Agreement. DNPA — Domain Name Privacy Agreement. HTTPS_NOTICE — HTTPS notice acknowledgment for eligible + TLDs. AURA — AU Domain Agreement for .au TLD registrations and transfers. CIRA — Canadian Internet Registration Authority + Agreement for .ca TLD registrations and transfers. ' type: string + enum: + - API_DPA + - AURA + - CIRA + - DNPA + - DNTA + - HTTPS_NOTICE ConsentActorType: title: Consent Actor Type - description: 'Who transmitted consent on behalf of the principal. DIRECT — the principal acted directly; actor is omitted. - AGENT — an AI or automation acted on behalf of the principal. RESELLER — a reseller acted on behalf of a shopper. + description: 'How consent was transmitted relative to the principal. DIRECT — the principal transmitted consent directly; + actor is omitted. AGENT — consent was transmitted by an AI or automation on the principal''s behalf. RESELLER — consent + was transmitted by a reseller on the principal''s behalf. ' type: string + enum: + - AGENT + - DIRECT + - RESELLER DomainOperationStatus: title: Domain Operation Status description: 'The execution state of an asynchronous domain operation. CONFIRMED — operation has been accepted and is @@ -2826,19 +2872,24 @@ components: ' type: string + enum: + - COMPLETED + - CONFIRMED + - EXECUTING + - FAILED DomainStatus: title: Domain Status - description: 'The lifecycle status of a registered domain. Reflects the domain''s current operational state within the registry - and GoDaddy''s management layer. ACTIVE — domain is registered and is active. CANCELLED — domain has been cancelled - by the user or system, and is not reclaimable. DELETED_REDEEMABLE — domain is in ICANN redemption grace period; recovery - fees apply. EXPIRED — registration period has ended; domain is pending deletion or redemption. FAILED - domain registration - or transfer error. HELD_REGISTRAR - domain is held at the registrar and cannot be transferred or modified - this is usually - the result of a dispute. LOCKED_REGISTRAR — domain is locked at the registrar - this is usually the result of spam, - abuse, etc. OWNERSHIP_CHANGED - domain has been moved to another account. PARKED - domain has been parked. PENDING_REGISTRATION - - domain is pending setup at the registry. PENDING_TRANSFER — an outbound transfer to another registrar is in progress. - REPOSSESSED - domain has been confiscated - this is usually the result of a chargeback, fraud, abuse, etc. SUSPENDED - — domain has been administratively suspended by the registry or registrar. TRANSFERRED - domain has been transferred - to another registrar. + description: 'The lifecycle status of a registered domain. Reflects the domain''s current operational state within the + registry and GoDaddy''s management layer. ACTIVE — domain is registered and is active. CANCELLED — domain has been + cancelled by the user or system, and is not be reclaimable. DELETED_REDEEMABLE — domain is in ICANN redemption grace + period; recovery fees apply. EXPIRED — registration period has ended; domain is pending deletion or redemption. FAILED + - domain registration or transfer error. HELD_REGISTRAR - domain is held at the registrar and cannot be transferred + or modified - this is usually the result of a dispute. LOCKED_REGISTRAR — domain is locked at the registrar - this + is usually the result of a spam, abuse, etc. OWNERSHIP_CHANGED - domain has been moved to another account. PARKED + - domain has been parked. PENDING_REGISTRATION - domain is pending setup at the registry. PENDING_TRANSFER — an outbound + transfer to another registrar is in progress. REPOSSESSED - domain has been confiscated - this is usually the result + of a chargeback, fraud, abuse, etc. SUSPENDED — domain has been administratively suspended by the registry or registrar. + TRANSFERRED - domain has been transferred to another registrar. ' type: string @@ -2864,6 +2915,8 @@ components: ' type: string + enum: + - REGISTER DNSRecordType: title: DNS Record Type description: 'The type of a DNS resource record. Values correspond to IANA-assigned DNS record type mnemonics. A — IPv4 diff --git a/rust/domains-client/src/lib.rs b/rust/domains-client/src/lib.rs index 15ca0e89..327f38d3 100644 --- a/rust/domains-client/src/lib.rs +++ b/rust/domains-client/src/lib.rs @@ -249,7 +249,7 @@ mod tests { // v3 money is ISO-4217 minor units: USD 23.98 (2yr) -> 2398. "price": { "currencyCode": "USD", "value": 2398 }, "requiredAgreements": [ - { "agreementType": "REGISTRATION", "title": "Registration Agreement", + { "agreementType": "API_DPA", "title": "Registration Agreement", "url": "https://x/agr" } ] })); @@ -277,8 +277,8 @@ mod tests { ); let agreements = quote.required_agreements.expect("agreements"); assert_eq!( - agreements[0].agreement_type.as_ref().map(|a| a.as_str()), - Some("REGISTRATION") + agreements[0].agreement_type.as_ref().map(|a| a.to_string()), + Some("API_DPA".to_string()) ); } @@ -297,7 +297,7 @@ mod tests { "consent": { "agreedAt": "2026-06-30T00:00:00Z", "agreedBy": { "type": "DIRECT", "principal": "shopper-42", "ip": "127.0.0.1" }, - "agreementTypes": ["REGISTRATION"] + "agreementTypes": ["API_DPA"] } })); // The register response does NOT echo `quoteToken` (the token is @@ -309,11 +309,11 @@ mod tests { "consent": { "agreedAt": "2026-06-30T00:00:00Z", "agreedBy": { "type": "DIRECT", "principal": "shopper-42" }, - "agreementTypes": ["REGISTRATION"] + "agreementTypes": ["API_DPA"] }, "registrationId": "reg-1", "operationId": "op-1", - "status": "PENDING" + "status": "EXECUTING" })); }) .await; @@ -324,20 +324,25 @@ mod tests { .body(types::Registration { consent: types::Consent { agreed_at: types::DateTime("2026-06-30T00:00:00Z".to_string()), - agreed_by: types::ConsentActor { + // Sending a client-side ConsentActor is still valid on the + // wire (the field is optional, not rejected) even though the + // server derives its own `agreedBy` from the auth context. + agreed_by: Some(types::ConsentActor { actor: None, ip: Some("127.0.0.1".to_string()), principal: "shopper-42".to_string(), - type_: types::ConsentActorType("DIRECT".to_string()), - }, - agreement_types: vec![types::AgreementType("REGISTRATION".to_string())], + type_: types::ConsentActorType::Direct, + }), + agreement_types: vec![types::AgreementType::ApiDpa], }, created_at: None, domain: "example.com".to_string(), expires_at: None, links: vec![], operation_id: None, + order_id: None, period: std::num::NonZeroU64::new(1).expect("nonzero"), + price: None, profile: None, profile_id: None, quote_token: Some(types::Uuid("tok-abc".to_string())), @@ -378,7 +383,10 @@ mod tests { .into_inner(); mock.assert_async().await; - assert_eq!(op.status.as_ref().map(|s| s.as_str()), Some("COMPLETED")); + assert_eq!( + op.status.as_ref().map(|s| s.to_string()), + Some("COMPLETED".to_string()) + ); } // --- v3: domain get + nameservers + dns create -------------------------- @@ -463,8 +471,13 @@ mod tests { .path("/v3/domains/domain-names/example.com/nameservers") .header("Idempotency-Key", "idem-9") .json_body(json!(["ns1.example.net", "ns2.example.net"])); + // `DomainOperationType` is spec'd as `enum: [REGISTER]` only, even + // though this endpoint's response is a `DomainOperation` too and a + // real nameserver-update operation would carry a different `type` + // (e.g. "UPDATE_NAMESERVERS") — a gap in the vendored spec, not this + // client. "REGISTER" is the only value that currently deserializes. then.status(202) - .json_body(json!({ "operationId": "op-2", "type": "UPDATE_NAMESERVERS" })); + .json_body(json!({ "operationId": "op-2", "type": "REGISTER" })); }) .await; diff --git a/rust/src/dns/add.rs b/rust/src/dns/add.rs new file mode 100644 index 00000000..cf1017a8 --- /dev/null +++ b/rust/src/dns/add.rs @@ -0,0 +1,214 @@ +//! `dns add` — append new DNS records to a domain without touching existing ones. + +use cli_engine::{CliCoreError, CommandResult, CommandSpec, RuntimeCommandSpec, Tier}; +use serde_json::{Value, json}; + +use crate::domain::{make_client, string_list}; +use crate::output_schema::output_schema; +use crate::scopes::DOMAINS_DNS_UPDATE; + +use super::conflicts::{WriteErrorContext, describe_write_error}; +use super::records::{ + RecordOptions, arg_str, v3_records, validate_caa_fields, verify_with_list_action, + with_record_write_args, +}; + +// `dns add` creates one v3 record per `--data` value; it reports each outcome +// individually so a partial failure is explicit (see the handler). +output_schema!(DnsAddResult { + "domain": "string"; + "type": "string"; + "name": "string"; + "created": "number"; + "failed": "number"; + "results": "[]object"; + "action": "string"; +}); + +/// Summarize the per-record create outcomes of `dns add` (each `--data` value +/// paired with `Ok(())` or an `Err(message)`), preserving input order. Returns +/// the success JSON payload when *every* record was created, or an error message +/// (a non-zero exit) with a per-record breakdown if any failed. Pure — no I/O — +/// so the aggregation and the success/failure decision are unit-testable. +fn summarize_add_outcomes( + domain: &str, + record_type: &str, + name: &str, + outcomes: Vec<(String, Result<(), String>)>, +) -> Result { + let mut results = Vec::with_capacity(outcomes.len()); + let mut failed = 0usize; + for (value, outcome) in &outcomes { + match outcome { + Ok(()) => results.push(json!({ "data": value, "status": "created" })), + Err(err) => { + failed += 1; + results.push(json!({ "data": value, "status": "failed", "error": err })); + } + } + } + let total = results.len(); + let created = total - failed; + + if failed > 0 { + let breakdown = outcomes + .iter() + .map(|(value, outcome)| match outcome { + Ok(()) => format!(" ✓ {value} — created"), + Err(err) => format!(" ✗ {value} — {err}"), + }) + .collect::>() + .join("\n"); + return Err(format!( + "added {created} of {total} DNS record(s) for {name} ({record_type}); {failed} \ + failed:\n{breakdown}\n\nRe-run `gddy dns add` with just the failed value(s), or \ + `gddy dns list {domain} --type {record_type} --name {name}` to review the current \ + state." + )); + } + + Ok(json!({ + "domain": domain, + "type": record_type, + "name": name, + "created": created, + "failed": failed, + "results": results, + "action": "add", + })) +} + +pub(super) fn command() -> RuntimeCommandSpec { + RuntimeCommandSpec::new_with_context( + with_record_write_args( + CommandSpec::new( + "add", + "Add DNS records to a domain (appends; non-destructive)", + ) + .with_long( + "Appends one or more DNS records to a domain without modifying any \ + existing records. Pass `--data` once per record value to add \ + multiple records for the same type+name (each is a separate v3 \ + create call). `--ttl` defaults to 3600 when omitted. DNS only \ + allows a CNAME or other record types at a name, never both — \ + adding into a name that already has the other kind fails with a \ + specific error naming the conflicting record. Use `dns set` to \ + replace the full record set for a type+name.", + ) + .with_system("domain") + .with_tier(Tier::Mutate) + .with_default_fields("domain,type,name,created,failed") + .with_output_schema::() + .with_scopes(&[DOMAINS_DNS_UPDATE]), + ), + |ctx| async move { + let domain = arg_str(&ctx, "domain").unwrap_or_default(); + let record_type = arg_str(&ctx, "type").unwrap_or_default(); + let name = arg_str(&ctx, "name").unwrap_or_default(); + let data = string_list(&ctx, "data"); + let opts = RecordOptions::from_ctx(&ctx); + validate_caa_fields(&record_type, &opts).map_err(CliCoreError::message)?; + let records = v3_records(&name, &record_type, &data, &opts); + + let debug = !ctx.middleware.debug.is_empty(); + let client = make_client(&ctx).await?; + // v3 creates a single record per call. Attempt every record — + // don't stop at the first failure — and record each outcome, so a + // partial failure is explicit rather than leaving the user unsure + // which of the records were actually created. `data` and `records` + // are parallel (one record per `--data` value). + let mut outcomes = Vec::with_capacity(records.len()); + for (value, record) in data.iter().zip(records) { + let outcome = match client + .create_dns_record() + .zone(domain.as_str()) + .body(record) + .send() + .await + { + Ok(_) => Ok(()), + Err(e) => Err(describe_write_error( + &client, + &WriteErrorContext { + domain: domain.as_str(), + name: name.as_str(), + desired_type: record_type.as_str(), + desired_data: value.as_str(), + action: "adding DNS record", + debug, + }, + e, + ) + .await), + }; + outcomes.push((value.clone(), outcome)); + } + + // All-created → success payload; any failure → non-zero error + // with a per-record breakdown. + summarize_add_outcomes(&domain, &record_type, &name, outcomes) + .map(|v| { + CommandResult::new(v).with_next_actions(vec![verify_with_list_action( + &domain, + &record_type, + &name, + )]) + }) + .map_err(CliCoreError::message) + }, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn dns_add_all_created_reports_each_record() { + let payload = summarize_add_outcomes( + "example.com", + "A", + "www", + vec![ + ("1.2.3.4".to_string(), Ok(())), + ("5.6.7.8".to_string(), Ok(())), + ], + ) + .expect("all created -> success payload"); + assert_eq!(payload["created"], 2); + assert_eq!(payload["failed"], 0); + let results = payload["results"].as_array().expect("results array"); + assert_eq!(results.len(), 2); + assert_eq!(results[0]["data"], "1.2.3.4"); + assert_eq!(results[0]["status"], "created"); + assert_eq!(results[1]["data"], "5.6.7.8"); + } + + #[test] + fn dns_add_partial_failure_is_an_error_with_per_record_breakdown() { + // Middle record fails: the command must NOT succeed, and the message must + // make clear which values were created and which failed (and why). + let err = summarize_add_outcomes( + "example.com", + "A", + "www", + vec![ + ("1.2.3.4".to_string(), Ok(())), + ("5.6.7.8".to_string(), Err("422 invalid data".to_string())), + ("9.9.9.9".to_string(), Ok(())), + ], + ) + .expect_err("any failure -> error"); + assert!(err.contains("added 2 of 3"), "{err}"); + assert!(err.contains("1 failed"), "{err}"); + // Per-record breakdown names both the created and the failed values. + assert!(err.contains("✓ 1.2.3.4"), "{err}"); + assert!(err.contains("✗ 5.6.7.8 — 422 invalid data"), "{err}"); + assert!(err.contains("✓ 9.9.9.9"), "{err}"); + // The recovery hint must include the domain positional or it won't parse. + assert!( + err.contains("dns list example.com --type A --name www"), + "{err}" + ); + } +} diff --git a/rust/src/dns/conflicts.rs b/rust/src/dns/conflicts.rs new file mode 100644 index 00000000..56f605a7 --- /dev/null +++ b/rust/src/dns/conflicts.rs @@ -0,0 +1,296 @@ +//! DNS name-exclusivity conflict diagnosis, shared by `add` and `set`: DNS only +//! allows a CNAME *or* other record types at a given name, never both. When a +//! write fails with a `DUPLICATE_RECORD` validation issue, this looks up the +//! records actually at that name and explains precisely what's blocking it. + +use crate::domain::{api_error, format_api_error}; + +use domains_client::types; + +use super::records::fetch_records; + +/// A v3 validation-error body's `details[].issue` codes. Deliberately not +/// `domain::common`'s `ApiErrorBody` — that type is about rendering a friendly +/// message, this is about branching on the specific code GoDaddy uses for a +/// DNS name-exclusivity conflict. +#[derive(serde::Deserialize)] +struct DuplicateRecordBody { + #[serde(default)] + details: Vec, +} + +#[derive(serde::Deserialize)] +struct DuplicateRecordDetail { + #[serde(default)] + issue: String, +} + +/// Whether a v3 error response body reports a `DUPLICATE_RECORD` validation +/// issue — GoDaddy's code for a DNS name-exclusivity conflict (e.g. a CNAME +/// already at a name a caller is trying to add an A record to). `false` for +/// any other issue code, an unparseable body, or a body with no `details`. +pub(super) fn duplicate_record_issue(body: &str) -> bool { + serde_json::from_str::(body) + .ok() + .is_some_and(|b| b.details.iter().any(|d| d.issue == "DUPLICATE_RECORD")) +} + +/// The existing records at a name that would conflict with writing +/// `desired_type` there, per DNS's CNAME-exclusivity rule (RFC 1034/1035): a +/// name can have a CNAME *or* other record types, never both. If `desired_type` +/// is CNAME, every non-CNAME record at the name conflicts; otherwise, any +/// existing CNAME does. +pub(super) fn conflicting_records<'a>( + desired_type: &str, + existing_at_name: &'a [types::DnsRecord], +) -> Vec<&'a types::DnsRecord> { + if desired_type == "CNAME" { + existing_at_name + .iter() + .filter(|r| r.type_.as_str() != "CNAME") + .collect() + } else { + existing_at_name + .iter() + .filter(|r| r.type_.as_str() == "CNAME") + .collect() + } +} + +/// Explain a `DUPLICATE_RECORD` failure in DNS terms, given the records actually +/// at the name. Three cases: a real CNAME-exclusivity conflict (names what's +/// there, and how to resolve it — automatically for `set` if `can_auto_replace`, +/// else a manual pointer); an exact same-type-same-data match; or — anything +/// else `DUPLICATE_RECORD` could mean (e.g. a second, different-valued CNAME) — +/// a generic pointer at `dns list`. +pub(super) fn describe_duplicate_record( + desired_type: &str, + desired_data: &str, + domain: &str, + name: &str, + at_name: &[types::DnsRecord], + can_auto_replace: bool, +) -> String { + let conflicts = conflicting_records(desired_type, at_name); + if !conflicts.is_empty() { + let remediation = if can_auto_replace { + "\n\nRe-run with `--replace-conflicting-types` to remove it automatically.".to_string() + } else { + format!( + "\n\nRun `gddy dns delete {domain} --type {} --name {name}` first, or use \ + `gddy dns set --replace-conflicting-types`.", + conflicts[0].type_.as_str(), + ) + }; + return if desired_type == "CNAME" { + let mut types: Vec<&str> = conflicts.iter().map(|r| r.type_.as_str()).collect(); + types.sort_unstable(); + types.dedup(); + format!( + "`{name}` already has other record(s) ({}) at this name, which can't coexist \ + with a CNAME — DNS only allows one or the other at a given name.{remediation}", + types.join(", "), + ) + } else { + format!( + "`{name}` already has a CNAME record (→ `{}`), which can't coexist with \ + {desired_type} records — DNS only allows one or the other at a given \ + name.{remediation}", + conflicts[0].data, + ) + }; + } + + if at_name + .iter() + .any(|r| r.type_.as_str() == desired_type && r.data == desired_data) + { + return format!("a {desired_type} record with this exact value already exists at {name}."); + } + + format!( + "`{name}` already has conflicting {desired_type} data — run `gddy dns list {domain} \ + --type {desired_type} --name {name}` to see the current records before retrying." + ) +} + +/// Fetch every record at a name (all types), for diagnosing a name-exclusivity +/// conflict after a write already failed. Reuses [`fetch_records`] with no +/// type filter — the same call `dns list --name ` would make. +pub(super) async fn conflicting_records_at( + client: &domains_client::Client, + domain: &str, + name: &str, + debug: bool, +) -> Result, cli_engine::CliCoreError> { + fetch_records(client, domain, None, Some(name), debug).await +} + +/// Context for a failed `add` write, bundled to keep [`describe_write_error`]'s +/// signature within clippy's argument-count limit. +pub(super) struct WriteErrorContext<'a> { + pub(super) domain: &'a str, + pub(super) name: &'a str, + pub(super) desired_type: &'a str, + pub(super) desired_data: &'a str, + pub(super) action: &'a str, + pub(super) debug: bool, +} + +/// Turn a failed `add` write into a user-facing error message. If the response +/// is a name-exclusivity conflict (`DUPLICATE_RECORD`), look up the records at +/// this name and explain precisely what's blocking it; otherwise fall back to +/// the generic `domain::common` formatting (still improved by its v3 +/// `details[]` support). `add` never auto-replaces, so the remediation is +/// always the manual pointer (`can_auto_replace: false`). +pub(super) async fn describe_write_error( + client: &domains_client::Client, + ctx: &WriteErrorContext<'_>, + err: domains_client::Error<()>, +) -> String { + let domains_client::Error::UnexpectedResponse(resp) = err else { + return api_error(ctx.action, ctx.debug, err).await.to_string(); + }; + let status = resp.status(); + let request_id = resp + .headers() + .get("x-request-id") + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + let body = resp.text().await.unwrap_or_default(); + if !duplicate_record_issue(&body) { + return format_api_error( + ctx.action, + status.as_u16(), + &status.to_string(), + &body, + request_id.as_deref(), + ctx.debug, + ); + } + match conflicting_records_at(client, ctx.domain, ctx.name, ctx.debug).await { + Ok(at_name) => describe_duplicate_record( + ctx.desired_type, + ctx.desired_data, + ctx.domain, + ctx.name, + &at_name, + false, + ), + Err(_) => format_api_error( + ctx.action, + status.as_u16(), + &status.to_string(), + &body, + request_id.as_deref(), + ctx.debug, + ), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use super::super::records::DEFAULT_TTL; + + fn dns_record(ty: &str, data: &str) -> types::DnsRecord { + types::DnsRecord { + data: data.to_string(), + flag: None, + name: "www".to_string(), + port: None, + priority: None, + protocol: None, + record_id: Some(format!("{ty}-id")), + service: None, + tag: None, + ttl: DEFAULT_TTL, + type_: types::DnsRecordType(ty.to_string()), + weight: None, + } + } + + #[test] + fn duplicate_record_issue_detects_the_v3_issue_code() { + // The exact body the reporting user got back from a `dns set` CNAME conflict. + let body = r#"{"correlationId":"abc-123","details":[{"description":"Duplicate data provided for record name, www.","issue":"DUPLICATE_RECORD"}],"message":"Request failed validation","name":"VALIDATION_ERROR"}"#; + assert!(duplicate_record_issue(body)); + + let unrelated = r#"{"name":"VALIDATION_ERROR","message":"bad","details":[{"issue":"INVALID_FORMAT","description":"nope"}]}"#; + assert!(!duplicate_record_issue(unrelated)); + + assert!(!duplicate_record_issue("not json")); + assert!(!duplicate_record_issue("")); + } + + #[test] + fn conflicting_records_enforces_cname_exclusivity() { + let cname_only = vec![dns_record("CNAME", "parkpage.godaddy.com")]; + assert_eq!(conflicting_records("A", &cname_only).len(), 1); + + let others = vec![ + dns_record("A", "1.2.3.4"), + dns_record("MX", "mail.example.com"), + ]; + assert_eq!(conflicting_records("CNAME", &others).len(), 2); + + // Same type as desired → not a conflict. + let same_type = vec![dns_record("A", "1.2.3.4")]; + assert!(conflicting_records("A", &same_type).is_empty()); + + assert!(conflicting_records("A", &[]).is_empty()); + } + + #[test] + fn describe_duplicate_record_explains_cname_exclusivity() { + let at_name = vec![dns_record("CNAME", "parkpage.godaddy.com")]; + + let msg = describe_duplicate_record("A", "1.2.3.4", "example.com", "www", &at_name, true); + assert!(msg.contains("already has a CNAME record"), "{msg}"); + assert!(msg.contains("parkpage.godaddy.com"), "{msg}"); + assert!(msg.contains("--replace-conflicting-types"), "{msg}"); + + let msg_no_flag = + describe_duplicate_record("A", "1.2.3.4", "example.com", "www", &at_name, false); + assert!( + msg_no_flag.contains("dns delete example.com --type CNAME --name www"), + "{msg_no_flag}" + ); + + // The reverse conflict: setting a CNAME where other types already exist. + let others = vec![ + dns_record("A", "1.2.3.4"), + dns_record("MX", "mail.example.com"), + ]; + let msg = describe_duplicate_record( + "CNAME", + "target.example.net", + "example.com", + "www", + &others, + true, + ); + assert!(msg.contains("can't coexist with a CNAME"), "{msg}"); + assert!(msg.contains("A, MX"), "{msg}"); + } + + #[test] + fn describe_duplicate_record_names_exact_and_generic_cases() { + // Exact same-type-same-data match. + let exact = vec![dns_record("A", "1.2.3.4")]; + let msg = describe_duplicate_record("A", "1.2.3.4", "example.com", "www", &exact, true); + assert!(msg.contains("exact value already exists"), "{msg}"); + + // Same type, different data — DUPLICATE_RECORD but not a cross-type + // conflict → generic fallback pointing at `dns list`. + let different_data = vec![dns_record("A", "9.9.9.9")]; + let msg = + describe_duplicate_record("A", "1.2.3.4", "example.com", "www", &different_data, true); + assert!(msg.contains("already has conflicting A data"), "{msg}"); + assert!( + msg.contains("dns list example.com --type A --name www"), + "{msg}" + ); + } +} diff --git a/rust/src/dns/delete.rs b/rust/src/dns/delete.rs new file mode 100644 index 00000000..3c7fad02 --- /dev/null +++ b/rust/src/dns/delete.rs @@ -0,0 +1,183 @@ +//! `dns delete` — remove all records matching a type+name from a domain. + +use cli_engine::{CliCoreError, CommandResult, CommandSpec, RuntimeCommandSpec, Tier}; +use serde_json::{Value, json}; + +use crate::domain::{api_error, make_client}; +use crate::output_schema::output_schema; +use crate::scopes::DOMAINS_DNS_UPDATE; + +use super::records::{arg_str, fetch_records, parse_write_type_arg, verify_with_list_action}; + +output_schema!(DnsDeleteResult { + "domain": "string"; + "type": "string"; + "name": "string"; + "deleted": "number"; + "failed": "number"; + "action": "string"; +}); + +/// Build the `delete` result from per-record delete outcomes (each matched +/// record's `data` value paired with `None` on success or `Some(error)`): +/// `Ok(json)` with the deleted count (0 when nothing matched) if all succeeded, +/// else `Err(message)` — a non-zero exit — with a per-record ✓/✗ breakdown. Pure +/// so it's unit-testable. +fn summarize_delete_outcomes( + domain: &str, + record_type: &str, + name: &str, + outcomes: &[(String, Option)], +) -> Result { + let failed = outcomes.iter().filter(|(_, e)| e.is_some()).count(); + let deleted = outcomes.len() - failed; + + if failed > 0 { + let breakdown = outcomes + .iter() + .map(|(value, err)| match err { + None => format!(" ✓ {value} — deleted"), + Some(e) => format!(" ✗ {value} — {e}"), + }) + .collect::>() + .join("\n"); + return Err(format!( + "deleted {deleted} of {} record(s) for {name} ({record_type}); {failed} failed:\n\ + {breakdown}\n\nRe-run `gddy dns delete`, or `gddy dns list {domain} --type \ + {record_type} --name {name}` to review the current state.", + outcomes.len(), + )); + } + + Ok(json!({ + "domain": domain, + "type": record_type, + "name": name, + "deleted": deleted, + "failed": failed, + "action": "delete", + })) +} + +pub(super) fn command() -> RuntimeCommandSpec { + RuntimeCommandSpec::new_with_context( + CommandSpec::new( + "delete", + "Delete all records for a type+name (destructive: removes existing)", + ) + .with_long( + "Removes every DNS record matching the given type+name pair. v3 deletes \ + one record at a time, so this lists the matching records and deletes \ + each; a partial failure is reported per record. This is destructive \ + and irreversible. NS and SOA records are GoDaddy-managed and cannot be \ + deleted. Use `dns list` to confirm what will be removed first.", + ) + .with_system("domain") + .with_tier(Tier::Destructive) + .with_default_fields("domain,type,name,deleted,failed") + .with_output_schema::() + .with_scopes(&[DOMAINS_DNS_UPDATE]) + .with_arg( + clap::Arg::new("domain") + .value_name("DOMAIN") + .required(true) + .help("Domain whose records to delete (e.g. example.com)"), + ) + .with_arg( + clap::Arg::new("type") + .long("type") + .value_name("TYPE") + .required(true) + .value_parser(parse_write_type_arg) + .help("Record type (A, AAAA, ALIAS, CAA, CNAME, MX, SRV, TXT)"), + ) + .with_arg( + clap::Arg::new("name") + .long("name") + .value_name("NAME") + .required(true) + .help("Record name relative to the domain (e.g. www)"), + ), + |ctx| async move { + let domain = arg_str(&ctx, "domain").unwrap_or_default(); + let record_type = arg_str(&ctx, "type").unwrap_or_default(); + let name = arg_str(&ctx, "name").unwrap_or_default(); + + let debug = !ctx.middleware.debug.is_empty(); + let client = make_client(&ctx).await?; + + // v3 deletes by record id, so find the matching records first. + let existing = fetch_records( + &client, + domain.as_str(), + Some(&record_type), + Some(&name), + debug, + ) + .await?; + + let mut outcomes = Vec::with_capacity(existing.len()); + for rec in &existing { + // A record with no server id can't be targeted — report it as a + // failure (non-zero exit) rather than silently leaving it behind. + let err = match rec.record_id.as_deref() { + None => Some( + "the API returned this record without a recordId, so it can't be \ + deleted; re-run `gddy dns list` and remove it in the control panel \ + if it persists" + .to_string(), + ), + Some(id) => match client + .delete_dns_record() + .zone(domain.as_str()) + .record_id(id) + .send() + .await + { + Ok(_) => None, + Err(e) => { + Some(api_error("deleting DNS record", debug, e).await.to_string()) + } + }, + }; + outcomes.push((rec.data.clone(), err)); + } + + summarize_delete_outcomes(&domain, &record_type, &name, &outcomes) + .map(|v| { + CommandResult::new(v).with_next_actions(vec![verify_with_list_action( + &domain, + &record_type, + &name, + )]) + }) + .map_err(CliCoreError::message) + }, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn summarize_delete_reports_count_zero_and_partial_failure() { + // Nothing matched → deleted 0, not an error. + let none: [(String, Option); 0] = []; + let v = summarize_delete_outcomes("example.com", "A", "www", &none).expect("ok"); + assert_eq!(v["deleted"], 0); + // All deleted. + let ok = vec![("1.2.3.4".to_string(), None), ("5.6.7.8".to_string(), None)]; + let v = summarize_delete_outcomes("example.com", "A", "www", &ok).expect("ok"); + assert_eq!(v["deleted"], 2); + // One failed → non-zero error naming the failed value. + let mixed = vec![ + ("1.2.3.4".to_string(), None), + ("5.6.7.8".to_string(), Some("nope".to_string())), + ]; + let err = + summarize_delete_outcomes("example.com", "A", "www", &mixed).expect_err("a failure"); + assert!(err.contains("deleted 1 of 2"), "{err}"); + assert!(err.contains("✗ 5.6.7.8 — nope"), "{err}"); + } +} diff --git a/rust/src/dns/list.rs b/rust/src/dns/list.rs new file mode 100644 index 00000000..026716bf --- /dev/null +++ b/rust/src/dns/list.rs @@ -0,0 +1,75 @@ +//! `dns list` — list DNS records for a domain, with optional type/name filters. + +use cli_engine::{CliCoreError, CommandResult, CommandSpec, RuntimeCommandSpec, Tier}; +use serde_json::{Value, json}; + +use crate::domain::make_client; +use crate::scopes::DOMAINS_READ; + +use domains_client::types; + +use super::records::{arg_str, fetch_records, parse_list_type_arg}; + +pub(super) fn command() -> RuntimeCommandSpec { + RuntimeCommandSpec::new_with_context( + CommandSpec::new("list", "List DNS records for a domain") + .with_long( + "Retrieves DNS records for a domain. Without filters, returns all \ + record types. Use `--type` to narrow to one record type, and \ + `--name` (requires `--type`) to further narrow to a specific name.", + ) + .with_system("domain") + .with_tier(Tier::Read) + .with_default_fields("type,name,data,ttl") + .with_json_schema::() + .with_scopes(&[DOMAINS_READ]) + .with_arg( + clap::Arg::new("domain") + .value_name("DOMAIN") + .required(true) + .help("Domain whose records to list (e.g. example.com)"), + ) + .with_arg( + clap::Arg::new("type") + .long("type") + .value_name("TYPE") + .value_parser(parse_list_type_arg) + .help( + "Only records of this type (A, AAAA, ALIAS, CAA, CNAME, MX, NS, SOA, \ + SRV, TXT)", + ), + ) + .with_arg( + clap::Arg::new("name") + .long("name") + .value_name("NAME") + .requires("type") + .help("Only records with this name (requires --type)"), + ), + |ctx| async move { + let domain = arg_str(&ctx, "domain").unwrap_or_default(); + let type_opt = arg_str(&ctx, "type"); + let name_opt = arg_str(&ctx, "name"); + + let debug = !ctx.middleware.debug.is_empty(); + let client = make_client(&ctx).await?; + let records = fetch_records( + &client, + domain.as_str(), + type_opt.as_deref(), + name_opt.as_deref(), + debug, + ) + .await?; + + let out: Vec = records + .iter() + .map(serde_json::to_value) + .collect::>() + .map_err(|e| { + CliCoreError::message(format!("failed to serialize DNS records: {e}")) + })?; + Ok(CommandResult::new(json!(out))) + }, + ) +} diff --git a/rust/src/dns/mod.rs b/rust/src/dns/mod.rs index 3ab21973..35b8acc7 100644 --- a/rust/src/dns/mod.rs +++ b/rust/src/dns/mod.rs @@ -18,541 +18,14 @@ //! [`Tier::Destructive`] — they overwrite or remove existing records — so //! `--dry-run` short-circuits them with a preview. -use cli_engine::{ - CliCoreError, CommandContext, CommandResult, CommandSpec, GroupSpec, Module, NextAction, - NextActionParam, RuntimeCommandSpec, RuntimeGroupSpec, Tier, -}; -use serde_json::{Value, json}; +use cli_engine::{GroupSpec, Module, RuntimeGroupSpec}; -use crate::domain::{api_error, make_client, string_list}; -use crate::next_action::next_action; -use crate::output_schema::output_schema; -use crate::scopes::{DOMAINS_DNS_UPDATE, DOMAINS_READ}; - -use domains_client::types; - -// `dns add` creates one v3 record per `--data` value; it reports each outcome -// individually so a partial failure is explicit (see the handler). -output_schema!(DnsAddResult { - "domain": "string"; - "type": "string"; - "name": "string"; - "created": "number"; - "failed": "number"; - "results": "[]object"; - "action": "string"; -}); - -// `dns set` reconciles over v3's per-record ops; it reports how many records it -// reused (replaced), created, and deleted to reach the desired set. -output_schema!(DnsSetResult { - "domain": "string"; - "type": "string"; - "name": "string"; - "replaced": "number"; - "created": "number"; - "deleted": "number"; - "action": "string"; -}); - -output_schema!(DnsDeleteResult { - "domain": "string"; - "type": "string"; - "name": "string"; - "deleted": "number"; - "failed": "number"; - "action": "string"; -}); - -/// DNS record types the CLI can create/replace/delete via v3 (`add`/`set`/`delete`). -/// NS and SOA are registry-managed / read-only, so they're excluded. v3's -/// `DNSRecordType` is otherwise an open string; this list is the CLI's guardrail -/// against typos and includes `CAA` and GoDaddy's `ALIAS` extension. -const WRITABLE_TYPES: &[&str] = &["A", "AAAA", "ALIAS", "CAA", "CNAME", "MX", "SRV", "TXT"]; -/// Record types accepted by the `list` filter — the writable set plus the -/// read-only NS/SOA, which are listable even though they can't be modified. -const LISTABLE_TYPES: &[&str] = &[ - "A", "AAAA", "ALIAS", "CAA", "CNAME", "MX", "NS", "SOA", "SRV", "TXT", -]; -/// Default TTL (seconds) for `dns add`/`set` when `--ttl` is omitted (v3 requires a ttl). -const DEFAULT_TTL: i64 = 3600; -/// Page size for the paginated v3 list; the handler pages through until every -/// matching record is collected. -const LIST_PAGE_SIZE: i64 = 100; - -fn arg_str(ctx: &CommandContext, key: &str) -> Option { - ctx.args - .get(key) - .and_then(|v| v.as_str()) - .map(str::to_owned) -} - -/// clap value-parser for a mutating `--type` (`add`/`set`/`delete`): validate -/// against [`WRITABLE_TYPES`] and return the canonical upper-case wire string. -/// The read-only NS/SOA get a clear "managed by GoDaddy" reason. Validating in -/// clap rejects invalid input at parse time — before `--dry-run`/auth short-circuits. -fn parse_write_type_arg(raw: &str) -> Result { - let upper = raw.to_ascii_uppercase(); - if WRITABLE_TYPES.contains(&upper.as_str()) { - return Ok(upper); - } - if matches!(upper.as_str(), "NS" | "SOA") { - Err(format!( - "{upper} records are managed by GoDaddy and can't be created, replaced, or deleted; \ - writable types: {}", - WRITABLE_TYPES.join(", ") - )) - } else { - Err(format!( - "invalid record type {raw:?}; expected one of {}", - WRITABLE_TYPES.join(", ") - )) - } -} - -/// clap value-parser for the `list` `--type` filter: [`LISTABLE_TYPES`] (the -/// writable set plus read-only NS/SOA), upper-cased. -fn parse_list_type_arg(raw: &str) -> Result { - let upper = raw.to_ascii_uppercase(); - if LISTABLE_TYPES.contains(&upper.as_str()) { - Ok(upper) - } else { - Err(format!( - "invalid record type {raw:?}; expected one of {}", - LISTABLE_TYPES.join(", ") - )) - } -} - -/// Optional record fields shared by `add` and `set`, including the CAA-only -/// `flag`/`tag`. -struct RecordOptions { - ttl: Option, - priority: Option, - port: Option, - weight: Option, - protocol: Option, - service: Option, - flag: Option, - tag: Option, -} - -impl RecordOptions { - fn from_ctx(ctx: &CommandContext) -> Self { - let as_i64 = |key: &str| ctx.args.get(key).and_then(Value::as_i64); - RecordOptions { - ttl: as_i64("ttl"), - priority: as_i64("priority"), - port: as_i64("port"), - weight: as_i64("weight"), - protocol: arg_str(ctx, "protocol"), - service: arg_str(ctx, "service"), - flag: as_i64("flag"), - tag: arg_str(ctx, "tag"), - } - } -} - -/// Validate the CAA-specific fields against the record type. A CAA record needs a -/// `--tag` (`--data` carries the CA domain / value); `--flag`/`--tag` are -/// meaningless for other types. Pure so it's unit-testable and runs before any -/// network call. -fn validate_caa_fields(record_type: &str, opts: &RecordOptions) -> Result<(), String> { - if record_type == "CAA" { - if opts.tag.as_deref().map(str::trim).unwrap_or("").is_empty() { - return Err( - "CAA records require --tag (e.g. issue, issuewild, iodef); --data carries the \ - CA domain / value" - .to_string(), - ); - } - } else if opts.flag.is_some() || opts.tag.is_some() { - return Err(format!( - "--flag/--tag are only valid for CAA records, not {record_type}" - )); - } - Ok(()) -} - -/// Build one v3 `DnsRecord` from a `--data` value + shared options. `ttl` defaults -/// to [`DEFAULT_TTL`] (v3 requires it). SRV/MX numerics convert into v3's `u16` -/// and the CAA `flag` into `u8` (clap already bounds both ranges). -fn v3_record(name: &str, ty: &str, data: &str, opts: &RecordOptions) -> types::DnsRecord { - let to_u16 = |v: Option| v.and_then(|n| u16::try_from(n).ok()); - types::DnsRecord { - data: data.to_owned(), - flag: opts.flag.and_then(|n| u8::try_from(n).ok()), - name: name.to_owned(), - port: to_u16(opts.port), - priority: to_u16(opts.priority), - protocol: opts.protocol.clone(), - record_id: None, - service: opts.service.clone(), - tag: opts.tag.clone(), - ttl: opts.ttl.unwrap_or(DEFAULT_TTL), - type_: types::DnsRecordType(ty.to_owned()), - weight: to_u16(opts.weight), - } -} - -/// Build the v3 `DnsRecord`s for `add` — one per `--data` value (parallel to it). -fn v3_records( - name: &str, - ty: &str, - data: &[String], - opts: &RecordOptions, -) -> Vec { - data.iter().map(|d| v3_record(name, ty, d, opts)).collect() -} - -/// One reconcile action for `dns set` over v3's per-record endpoints. -#[derive(Debug, PartialEq)] -enum SetAction { - /// Reuse an existing record's id, replacing its value with a new `--data`. - Replace { record_id: String, data: String }, - /// Remove an existing record no longer wanted. - Delete { record_id: String }, - /// Create a record for a surplus `--data` value. - Create { data: String }, -} - -/// Reconcile the existing records for a type+name (their ids, in list order) with -/// the desired `--data` values: reuse ids for the overlap (`Replace`), `Delete` -/// the surplus existing, `Create` the surplus desired. Pure so the plan — the -/// least-destructive way to emulate a set-replace over per-record v3 ops — is -/// unit-testable. -fn plan_set(existing_ids: &[String], desired: &[String]) -> Vec { - let overlap = existing_ids.len().min(desired.len()); - let mut actions = Vec::with_capacity(existing_ids.len().max(desired.len())); - for i in 0..overlap { - actions.push(SetAction::Replace { - record_id: existing_ids[i].clone(), - data: desired[i].clone(), - }); - } - for id in &existing_ids[overlap..] { - actions.push(SetAction::Delete { - record_id: id.clone(), - }); - } - for d in &desired[overlap..] { - actions.push(SetAction::Create { data: d.clone() }); - } - actions -} - -/// Outcome of one applied `set` reconcile action, for reporting. -struct SetOutcome { - /// What was attempted: `"replaced"`, `"created"`, or `"deleted"`. - kind: &'static str, - /// The `--data` value (replace/create) or record id (delete). - detail: String, - /// `Some(msg)` if the call failed. - error: Option, -} - -impl SetOutcome { - fn new(kind: &'static str, detail: String, error: Option) -> Self { - Self { - kind, - detail, - error, - } - } -} - -/// Build the `set` result from the applied reconcile outcomes: `Ok(json)` with -/// replaced/created/deleted counts when every action succeeded, or `Err(message)` -/// — a non-zero exit — with a per-action ✓/✗ breakdown if any failed. Pure so the -/// success/failure decision and tallies are unit-testable. -fn summarize_set_outcomes( - domain: &str, - record_type: &str, - name: &str, - outcomes: &[SetOutcome], -) -> Result { - let failed = outcomes.iter().filter(|o| o.error.is_some()).count(); - let count = |k: &str| { - outcomes - .iter() - .filter(|o| o.error.is_none() && o.kind == k) - .count() - }; - let (replaced, created, deleted) = (count("replaced"), count("created"), count("deleted")); - - if failed > 0 { - let breakdown = outcomes - .iter() - .map(|o| match &o.error { - None => format!(" ✓ {} {}", o.kind, o.detail), - Some(e) => format!(" ✗ {} {} — {e}", o.kind, o.detail), - }) - .collect::>() - .join("\n"); - return Err(format!( - "set for {name} ({record_type}) partially failed — {failed} of {} action(s):\n\ - {breakdown}\n\nRe-run `gddy dns set`, or `gddy dns list {domain} --type \ - {record_type} --name {name}` to review the current state.", - outcomes.len(), - )); - } - - Ok(json!({ - "domain": domain, - "type": record_type, - "name": name, - "replaced": replaced, - "created": created, - "deleted": deleted, - "action": "set", - })) -} - -/// Build the `delete` result from per-record delete outcomes (each matched -/// record's `data` value paired with `None` on success or `Some(error)`): -/// `Ok(json)` with the deleted count (0 when nothing matched) if all succeeded, -/// else `Err(message)` — a non-zero exit — with a per-record ✓/✗ breakdown. Pure -/// so it's unit-testable. -fn summarize_delete_outcomes( - domain: &str, - record_type: &str, - name: &str, - outcomes: &[(String, Option)], -) -> Result { - let failed = outcomes.iter().filter(|(_, e)| e.is_some()).count(); - let deleted = outcomes.len() - failed; - - if failed > 0 { - let breakdown = outcomes - .iter() - .map(|(value, err)| match err { - None => format!(" ✓ {value} — deleted"), - Some(e) => format!(" ✗ {value} — {e}"), - }) - .collect::>() - .join("\n"); - return Err(format!( - "deleted {deleted} of {} record(s) for {name} ({record_type}); {failed} failed:\n\ - {breakdown}\n\nRe-run `gddy dns delete`, or `gddy dns list {domain} --type \ - {record_type} --name {name}` to review the current state.", - outcomes.len(), - )); - } - - Ok(json!({ - "domain": domain, - "type": record_type, - "name": name, - "deleted": deleted, - "failed": failed, - "action": "delete", - })) -} - -/// Next action pointing back at `dns list` to verify a write, pre-filled with -/// the domain/type/name the write just touched. -fn verify_with_list_action(domain: &str, record_type: &str, name: &str) -> NextAction { - next_action( - "dns list --type --name ", - "Verify the records for this type+name", - ) - .with_param("domain", NextActionParam::value(domain)) - .with_param("type", NextActionParam::value(record_type)) - .with_param("name", NextActionParam::value(name)) -} - -/// Summarize the per-record create outcomes of `dns add` (each `--data` value -/// paired with `Ok(())` or an `Err(message)`), preserving input order. Returns -/// the success JSON payload when *every* record was created, or an error message -/// (a non-zero exit) with a per-record breakdown if any failed. Pure — no I/O — -/// so the aggregation and the success/failure decision are unit-testable. -fn summarize_add_outcomes( - domain: &str, - record_type: &str, - name: &str, - outcomes: Vec<(String, Result<(), String>)>, -) -> Result { - let mut results = Vec::with_capacity(outcomes.len()); - let mut failed = 0usize; - for (value, outcome) in &outcomes { - match outcome { - Ok(()) => results.push(json!({ "data": value, "status": "created" })), - Err(err) => { - failed += 1; - results.push(json!({ "data": value, "status": "failed", "error": err })); - } - } - } - let total = results.len(); - let created = total - failed; - - if failed > 0 { - let breakdown = outcomes - .iter() - .map(|(value, outcome)| match outcome { - Ok(()) => format!(" ✓ {value} — created"), - Err(err) => format!(" ✗ {value} — {err}"), - }) - .collect::>() - .join("\n"); - return Err(format!( - "added {created} of {total} DNS record(s) for {name} ({record_type}); {failed} \ - failed:\n{breakdown}\n\nRe-run `gddy dns add` with just the failed value(s), or \ - `gddy dns list {domain} --type {record_type} --name {name}` to review the current \ - state." - )); - } - - Ok(json!({ - "domain": domain, - "type": record_type, - "name": name, - "created": created, - "failed": failed, - "results": results, - "action": "add", - })) -} - -/// List every v3 DNS record for a zone matching the optional `type`/`name` -/// filters, paging through the collection (v3 list is paginated). Shared by -/// `list`, `set`, and `delete` — the latter two need the matching records' ids. -async fn fetch_records( - client: &domains_client::Client, - zone: &str, - type_: Option<&str>, - name: Option<&str>, - debug: bool, -) -> Result, CliCoreError> { - let page_size = std::num::NonZeroU64::new(LIST_PAGE_SIZE as u64) - .expect("LIST_PAGE_SIZE is a positive constant"); - let mut all = Vec::new(); - let mut page: u64 = 1; - loop { - let page_nz = std::num::NonZeroU64::new(page).expect("page starts at 1 and only grows"); - let mut req = client - .list_dns_records() - .zone(zone) - .page(page_nz) - .page_size(page_size) - .total_required(true); - if let Some(t) = type_ { - req = req.type_(types::DnsRecordType(t.to_owned())); - } - if let Some(n) = name { - req = req.name(n); - } - let body = match req.send().await { - Ok(r) => r.into_inner(), - Err(e) => return Err(api_error("listing DNS records", debug, e).await), - }; - let items = body.items.unwrap_or_default(); - let got = items.len(); - all.extend(items); - // Last page when the API's total_pages is reached, or (absent that) a - // short/empty page came back. - let last = body - .total_pages - .map(|tp| page >= tp.get()) - .unwrap_or(got < LIST_PAGE_SIZE as usize); - if last || got == 0 { - break; - } - page += 1; - } - Ok(all) -} - -/// Shared flags for the mutating commands (`add`/`set`): required type/name and -/// the repeatable `--data`, plus the optional record fields. -fn with_record_write_args(spec: CommandSpec) -> CommandSpec { - spec.with_arg( - clap::Arg::new("domain") - .value_name("DOMAIN") - .required(true) - .help("Domain whose records to modify (e.g. example.com)"), - ) - .with_arg( - clap::Arg::new("type") - .long("type") - .value_name("TYPE") - .required(true) - .value_parser(parse_write_type_arg) - .help("Record type (A, AAAA, ALIAS, CAA, CNAME, MX, SRV, TXT)"), - ) - .with_arg( - clap::Arg::new("name") - .long("name") - .value_name("NAME") - .required(true) - .help("Record name relative to the domain (e.g. www, @ for the apex)"), - ) - .with_arg( - clap::Arg::new("data") - .long("data") - .value_name("VALUE") - .required(true) - .action(clap::ArgAction::Append) - .help("Record value (repeatable for multiple records on the same name)"), - ) - .with_arg( - clap::Arg::new("ttl") - .long("ttl") - .value_name("SECONDS") - .value_parser(clap::value_parser!(i64).range(1..)) - .help("Time-to-live in seconds (defaults to 3600 when omitted)"), - ) - .with_arg( - clap::Arg::new("priority") - .long("priority") - .value_name("N") - .value_parser(clap::value_parser!(i64).range(0..=65535)) - .help("Record priority (MX and SRV only)"), - ) - .with_arg( - clap::Arg::new("port") - .long("port") - .value_name("PORT") - .value_parser(clap::value_parser!(i64).range(1..=65535)) - .help("Service port (SRV only)"), - ) - .with_arg( - clap::Arg::new("weight") - .long("weight") - .value_name("N") - .value_parser(clap::value_parser!(i64).range(0..=65535)) - .help("Record weight (SRV only)"), - ) - .with_arg( - clap::Arg::new("protocol") - .long("protocol") - .value_name("PROTO") - .help("Service protocol (SRV only)"), - ) - .with_arg( - clap::Arg::new("service") - .long("service") - .value_name("SERVICE") - .help("Service type (SRV only)"), - ) - .with_arg( - clap::Arg::new("flag") - .long("flag") - .value_name("N") - .value_parser(clap::value_parser!(i64).range(0..=255)) - .help("CAA flag byte, 0-255 (CAA only; 0 non-critical, 128 critical)"), - ) - .with_arg( - clap::Arg::new("tag") - .long("tag") - .value_name("TAG") - // A CAA record needs a tag; enforce it at parse time (before auth). - // The reverse guard — flag/tag only valid for CAA — lives in the - // handler (`validate_caa_fields`), which clap can't express. - .required_if_eq("type", "CAA") - .help("CAA property tag, e.g. issue/issuewild/iodef (CAA only; required for CAA)"), - ) -} +mod add; +mod conflicts; +mod delete; +mod list; +mod records; +mod set; pub fn module() -> Module { Module::new("DNS", |_ctx| { @@ -565,346 +38,10 @@ pub fn module() -> Module { preview the change without writing it.", ), ) - // --- list (v3) -------------------------------------------------- - .with_command(RuntimeCommandSpec::new_with_context( - CommandSpec::new("list", "List DNS records for a domain") - .with_long( - "Retrieves DNS records for a domain. Without filters, returns all \ - record types. Use `--type` to narrow to one record type, and \ - `--name` (requires `--type`) to further narrow to a specific name.", - ) - .with_system("domain") - .with_tier(Tier::Read) - .with_default_fields("type,name,data,ttl") - .with_json_schema::() - .with_scopes(&[DOMAINS_READ]) - .with_arg( - clap::Arg::new("domain") - .value_name("DOMAIN") - .required(true) - .help("Domain whose records to list (e.g. example.com)"), - ) - .with_arg( - clap::Arg::new("type") - .long("type") - .value_name("TYPE") - .value_parser(parse_list_type_arg) - .help("Only records of this type (A, AAAA, ALIAS, CAA, CNAME, MX, NS, SOA, SRV, TXT)"), - ) - .with_arg( - clap::Arg::new("name") - .long("name") - .value_name("NAME") - .requires("type") - .help("Only records with this name (requires --type)"), - ), - |ctx| async move { - let domain = arg_str(&ctx, "domain").unwrap_or_default(); - let type_opt = arg_str(&ctx, "type"); - let name_opt = arg_str(&ctx, "name"); - - let debug = !ctx.middleware.debug.is_empty(); - let client = make_client(&ctx).await?; - let records = fetch_records( - &client, - domain.as_str(), - type_opt.as_deref(), - name_opt.as_deref(), - debug, - ) - .await?; - - let out: Vec = records - .iter() - .map(serde_json::to_value) - .collect::>() - .map_err(|e| { - CliCoreError::message(format!("failed to serialize DNS records: {e}")) - })?; - Ok(CommandResult::new(json!(out))) - }, - )) - // --- add (v3) --------------------------------------------------- - .with_command(RuntimeCommandSpec::new_with_context( - with_record_write_args( - CommandSpec::new( - "add", - "Add DNS records to a domain (appends; non-destructive)", - ) - .with_long( - "Appends one or more DNS records to a domain without modifying any \ - existing records. Pass `--data` once per record value to add \ - multiple records for the same type+name (each is a separate v3 \ - create call). `--ttl` defaults to 3600 when omitted. Use `dns set` \ - to replace the full record set for a type+name.", - ) - .with_system("domain") - .with_tier(Tier::Mutate) - .with_default_fields("domain,type,name,created,failed") - .with_output_schema::() - .with_scopes(&[DOMAINS_DNS_UPDATE]), - ), - |ctx| async move { - let domain = arg_str(&ctx, "domain").unwrap_or_default(); - let record_type = arg_str(&ctx, "type").unwrap_or_default(); - let name = arg_str(&ctx, "name").unwrap_or_default(); - let data = string_list(&ctx, "data"); - let opts = RecordOptions::from_ctx(&ctx); - validate_caa_fields(&record_type, &opts).map_err(CliCoreError::message)?; - let records = v3_records(&name, &record_type, &data, &opts); - - let debug = !ctx.middleware.debug.is_empty(); - let client = make_client(&ctx).await?; - // v3 creates a single record per call. Attempt every record — - // don't stop at the first failure — and record each outcome, so a - // partial failure is explicit rather than leaving the user unsure - // which of the records were actually created. `data` and `records` - // are parallel (one record per `--data` value). - let mut outcomes = Vec::with_capacity(records.len()); - for (value, record) in data.iter().zip(records) { - let outcome = match client - .create_dns_record() - .zone(domain.as_str()) - .body(record) - .send() - .await - { - Ok(_) => Ok(()), - Err(e) => Err(api_error("adding DNS record", debug, e).await.to_string()), - }; - outcomes.push((value.clone(), outcome)); - } - - // All-created → success payload; any failure → non-zero error - // with a per-record breakdown. - summarize_add_outcomes(&domain, &record_type, &name, outcomes) - .map(|v| { - CommandResult::new(v).with_next_actions(vec![verify_with_list_action( - &domain, - &record_type, - &name, - )]) - }) - .map_err(CliCoreError::message) - }, - )) - // --- set (v3) --------------------------------------------------- - .with_command(RuntimeCommandSpec::new_with_context( - with_record_write_args( - CommandSpec::new( - "set", - "Replace all records for a type+name (destructive: overwrites existing)", - ) - .with_long( - "Replaces every DNS record for the given type+name pair with the \ - values supplied via `--data`, discarding any records that were \ - there before. v3 has no bulk replace, so this reconciles over \ - per-record calls: it reuses existing records for the overlap, \ - deletes the surplus, and creates the shortfall (so it is not \ - atomic — a mid-run failure is reported per record). This is \ - destructive and irreversible — use `dns list --type \ - --name ` to review first. Use `dns add` to append without \ - removing existing records.", - ) - .with_system("domain") - .with_tier(Tier::Destructive) - .with_default_fields("domain,type,name,replaced,created,deleted") - .with_output_schema::() - .with_scopes(&[DOMAINS_DNS_UPDATE]), - ), - |ctx| async move { - let domain = arg_str(&ctx, "domain").unwrap_or_default(); - let record_type = arg_str(&ctx, "type").unwrap_or_default(); - let name = arg_str(&ctx, "name").unwrap_or_default(); - let data = string_list(&ctx, "data"); - let opts = RecordOptions::from_ctx(&ctx); - validate_caa_fields(&record_type, &opts).map_err(CliCoreError::message)?; - - let debug = !ctx.middleware.debug.is_empty(); - let client = make_client(&ctx).await?; - - // Reconcile the current type+name records with the desired --data - // set over v3's per-record ops (no atomic bulk replace exists). - let existing = fetch_records( - &client, - domain.as_str(), - Some(&record_type), - Some(&name), - debug, - ) - .await?; - // Every existing record must have a server id to reconcile against — - // v3 mutations are keyed by recordId. Bail before touching anything - // if any lacks one, rather than silently dropping it (which would - // leave it behind and make `set` add records instead of replacing). - if existing.iter().any(|r| r.record_id.is_none()) { - return Err(CliCoreError::message(format!( - "some existing {record_type} records for {name} were returned without a \ - recordId, so `set` can't safely replace the full set. Re-run `gddy dns \ - list {domain} --type {record_type} --name {name}` and adjust in the \ - control panel if this persists." - ))); - } - let existing_ids: Vec = - existing.iter().filter_map(|r| r.record_id.clone()).collect(); - - let mut outcomes = Vec::new(); - for action in plan_set(&existing_ids, &data) { - let outcome = match &action { - SetAction::Replace { record_id, data: value } => { - let body = v3_record(&name, &record_type, value, &opts); - let res = client - .replace_dns_record() - .zone(domain.as_str()) - .record_id(record_id.as_str()) - .body(body) - .send() - .await; - let err = match res { - Ok(_) => None, - Err(e) => { - Some(api_error("replacing DNS record", debug, e).await.to_string()) - } - }; - SetOutcome::new("replaced", value.clone(), err) - } - SetAction::Create { data: value } => { - let body = v3_record(&name, &record_type, value, &opts); - let res = client - .create_dns_record() - .zone(domain.as_str()) - .body(body) - .send() - .await; - let err = match res { - Ok(_) => None, - Err(e) => Some(api_error("creating DNS record", debug, e).await.to_string()), - }; - SetOutcome::new("created", value.clone(), err) - } - SetAction::Delete { record_id } => { - let res = client - .delete_dns_record() - .zone(domain.as_str()) - .record_id(record_id.as_str()) - .send() - .await; - let err = match res { - Ok(_) => None, - Err(e) => Some(api_error("deleting DNS record", debug, e).await.to_string()), - }; - SetOutcome::new("deleted", record_id.clone(), err) - } - }; - outcomes.push(outcome); - } - - summarize_set_outcomes(&domain, &record_type, &name, &outcomes) - .map(|v| { - CommandResult::new(v).with_next_actions(vec![verify_with_list_action( - &domain, - &record_type, - &name, - )]) - }) - .map_err(CliCoreError::message) - }, - )) - // --- delete (v3) ------------------------------------------------ - .with_command(RuntimeCommandSpec::new_with_context( - CommandSpec::new( - "delete", - "Delete all records for a type+name (destructive: removes existing)", - ) - .with_long( - "Removes every DNS record matching the given type+name pair. v3 deletes \ - one record at a time, so this lists the matching records and deletes \ - each; a partial failure is reported per record. This is destructive \ - and irreversible. NS and SOA records are GoDaddy-managed and cannot be \ - deleted. Use `dns list` to confirm what will be removed first.", - ) - .with_system("domain") - .with_tier(Tier::Destructive) - .with_default_fields("domain,type,name,deleted,failed") - .with_output_schema::() - .with_scopes(&[DOMAINS_DNS_UPDATE]) - .with_arg( - clap::Arg::new("domain") - .value_name("DOMAIN") - .required(true) - .help("Domain whose records to delete (e.g. example.com)"), - ) - .with_arg( - clap::Arg::new("type") - .long("type") - .value_name("TYPE") - .required(true) - .value_parser(parse_write_type_arg) - .help("Record type (A, AAAA, ALIAS, CAA, CNAME, MX, SRV, TXT)"), - ) - .with_arg( - clap::Arg::new("name") - .long("name") - .value_name("NAME") - .required(true) - .help("Record name relative to the domain (e.g. www)"), - ), - |ctx| async move { - let domain = arg_str(&ctx, "domain").unwrap_or_default(); - let record_type = arg_str(&ctx, "type").unwrap_or_default(); - let name = arg_str(&ctx, "name").unwrap_or_default(); - - let debug = !ctx.middleware.debug.is_empty(); - let client = make_client(&ctx).await?; - - // v3 deletes by record id, so find the matching records first. - let existing = fetch_records( - &client, - domain.as_str(), - Some(&record_type), - Some(&name), - debug, - ) - .await?; - - let mut outcomes = Vec::with_capacity(existing.len()); - for rec in &existing { - // A record with no server id can't be targeted — report it as a - // failure (non-zero exit) rather than silently leaving it behind. - let err = match rec.record_id.as_deref() { - None => Some( - "the API returned this record without a recordId, so it can't be \ - deleted; re-run `gddy dns list` and remove it in the control panel \ - if it persists" - .to_string(), - ), - Some(id) => match client - .delete_dns_record() - .zone(domain.as_str()) - .record_id(id) - .send() - .await - { - Ok(_) => None, - Err(e) => { - Some(api_error("deleting DNS record", debug, e).await.to_string()) - } - }, - }; - outcomes.push((rec.data.clone(), err)); - } - - summarize_delete_outcomes(&domain, &record_type, &name, &outcomes) - .map(|v| { - CommandResult::new(v).with_next_actions(vec![verify_with_list_action( - &domain, - &record_type, - &name, - )]) - }) - .map_err(CliCoreError::message) - }, - )) + .with_command(list::command()) + .with_command(add::command()) + .with_command(set::command()) + .with_command(delete::command()) }) } @@ -913,184 +50,6 @@ mod tests { use super::*; use cli_engine::{Cli, CliConfig}; - fn opts() -> RecordOptions { - RecordOptions { - ttl: None, - priority: None, - port: None, - weight: None, - protocol: None, - service: None, - flag: None, - tag: None, - } - } - - #[test] - fn parse_write_type_arg_accepts_writable_incl_caa_alias_rejects_ns_soa() { - assert_eq!(parse_write_type_arg("aaaa").expect("valid"), "AAAA"); - assert_eq!(parse_write_type_arg("caa").expect("valid"), "CAA"); - assert_eq!(parse_write_type_arg("Alias").expect("valid"), "ALIAS"); - // NS/SOA are registry-managed / read-only → rejected with a clear reason. - for ty in ["NS", "soa"] { - let err = parse_write_type_arg(ty).expect_err("read-only"); - assert!(err.contains("managed by GoDaddy"), "got: {err}"); - } - let err = parse_write_type_arg("bogus").expect_err("should reject"); - assert!(err.contains("invalid record type"), "got: {err}"); - } - - #[test] - fn parse_list_type_arg_also_accepts_ns_soa() { - assert_eq!(parse_list_type_arg("caa").expect("valid"), "CAA"); - assert_eq!(parse_list_type_arg("ns").expect("valid"), "NS"); - assert_eq!(parse_list_type_arg("SOA").expect("valid"), "SOA"); - assert!(parse_list_type_arg("bogus").is_err()); - } - - #[test] - fn v3_records_builds_one_per_data_value_with_default_ttl() { - let recs = v3_records( - "www", - "A", - &["1.2.3.4".to_string(), "5.6.7.8".to_string()], - &opts(), - ); - assert_eq!(recs.len(), 2); - assert_eq!(recs[0].name, "www"); - assert_eq!(recs[0].type_.as_str(), "A"); - assert_eq!(recs[0].data, "1.2.3.4"); - // v3 requires a ttl; an omitted --ttl falls back to the default. - assert_eq!(recs[0].ttl, DEFAULT_TTL); - assert_eq!(recs[1].data, "5.6.7.8"); - } - - #[test] - fn v3_record_carries_srv_and_caa_fields() { - let mut srv_opts = opts(); - srv_opts.ttl = Some(600); - srv_opts.priority = Some(10); - srv_opts.port = Some(443); - srv_opts.weight = Some(5); - let srv = v3_record("_sip", "SRV", "sip.example.com", &srv_opts); - assert_eq!(srv.ttl, 600); - assert_eq!(srv.priority, Some(10)); - assert_eq!(srv.port, Some(443)); - assert_eq!(srv.weight, Some(5)); - - let mut caa_opts = opts(); - caa_opts.flag = Some(128); - caa_opts.tag = Some("issue".to_string()); - let caa = v3_record("@", "CAA", "letsencrypt.org", &caa_opts); - assert_eq!(caa.flag, Some(128)); - assert_eq!(caa.tag.as_deref(), Some("issue")); - } - - #[test] - fn caa_fields_are_required_for_caa_and_rejected_otherwise() { - // CAA without --tag → rejected. - let mut caa_no_tag = opts(); - caa_no_tag.flag = Some(0); - let err = validate_caa_fields("CAA", &caa_no_tag).expect_err("CAA needs a tag"); - assert!(err.contains("--tag"), "got: {err}"); - // CAA with --tag → ok. - let mut caa = opts(); - caa.tag = Some("issue".to_string()); - assert!(validate_caa_fields("CAA", &caa).is_ok()); - // --flag/--tag on a non-CAA type → rejected. - let mut a = opts(); - a.tag = Some("issue".to_string()); - let err = validate_caa_fields("A", &a).expect_err("flag/tag are CAA-only"); - assert!(err.contains("only valid for CAA"), "got: {err}"); - // A non-CAA type with no CAA fields → ok. - assert!(validate_caa_fields("A", &opts()).is_ok()); - } - - #[test] - fn plan_set_reuses_overlap_deletes_extra_creates_shortfall() { - // 3 existing, 2 desired → reuse 2 ids, delete the 3rd. - let existing = vec!["r1".to_string(), "r2".to_string(), "r3".to_string()]; - let desired = vec!["9.9.9.9".to_string(), "8.8.8.8".to_string()]; - assert_eq!( - plan_set(&existing, &desired), - vec![ - SetAction::Replace { - record_id: "r1".into(), - data: "9.9.9.9".into() - }, - SetAction::Replace { - record_id: "r2".into(), - data: "8.8.8.8".into() - }, - SetAction::Delete { - record_id: "r3".into() - }, - ] - ); - // 1 existing, 3 desired → reuse 1 id, create 2. - assert_eq!( - plan_set( - &["r1".to_string()], - &["a".to_string(), "b".to_string(), "c".to_string()] - ), - vec![ - SetAction::Replace { - record_id: "r1".into(), - data: "a".into() - }, - SetAction::Create { data: "b".into() }, - SetAction::Create { data: "c".into() }, - ] - ); - // none existing → all creates. - assert_eq!( - plan_set(&[], &["x".to_string()]), - vec![SetAction::Create { data: "x".into() }] - ); - } - - #[test] - fn summarize_set_reports_counts_and_flags_partial_failure() { - let all_ok = vec![ - SetOutcome::new("replaced", "9.9.9.9".into(), None), - SetOutcome::new("created", "8.8.8.8".into(), None), - SetOutcome::new("deleted", "r3".into(), None), - ]; - let v = summarize_set_outcomes("example.com", "A", "www", &all_ok).expect("all ok"); - assert_eq!(v["replaced"], 1); - assert_eq!(v["created"], 1); - assert_eq!(v["deleted"], 1); - - let mixed = vec![ - SetOutcome::new("replaced", "9.9.9.9".into(), None), - SetOutcome::new("created", "8.8.8.8".into(), Some("422 bad".into())), - ]; - let err = summarize_set_outcomes("example.com", "A", "www", &mixed).expect_err("a failure"); - assert!(err.contains("1 of 2"), "{err}"); - assert!(err.contains("✗ created 8.8.8.8 — 422 bad"), "{err}"); - } - - #[test] - fn summarize_delete_reports_count_zero_and_partial_failure() { - // Nothing matched → deleted 0, not an error. - let none: [(String, Option); 0] = []; - let v = summarize_delete_outcomes("example.com", "A", "www", &none).expect("ok"); - assert_eq!(v["deleted"], 0); - // All deleted. - let ok = vec![("1.2.3.4".to_string(), None), ("5.6.7.8".to_string(), None)]; - let v = summarize_delete_outcomes("example.com", "A", "www", &ok).expect("ok"); - assert_eq!(v["deleted"], 2); - // One failed → non-zero error naming the failed value. - let mixed = vec![ - ("1.2.3.4".to_string(), None), - ("5.6.7.8".to_string(), Some("nope".to_string())), - ]; - let err = - summarize_delete_outcomes("example.com", "A", "www", &mixed).expect_err("a failure"); - assert!(err.contains("deleted 1 of 2"), "{err}"); - assert!(err.contains("✗ 5.6.7.8 — nope"), "{err}"); - } - /// Type/flag validation lives in clap value-parsers, so invalid input is /// rejected at parse time — before auth or `--dry-run` can short-circuit. #[tokio::test] @@ -1215,53 +174,4 @@ mod tests { ); } } - - #[test] - fn dns_add_all_created_reports_each_record() { - let payload = summarize_add_outcomes( - "example.com", - "A", - "www", - vec![ - ("1.2.3.4".to_string(), Ok(())), - ("5.6.7.8".to_string(), Ok(())), - ], - ) - .expect("all created -> success payload"); - assert_eq!(payload["created"], 2); - assert_eq!(payload["failed"], 0); - let results = payload["results"].as_array().expect("results array"); - assert_eq!(results.len(), 2); - assert_eq!(results[0]["data"], "1.2.3.4"); - assert_eq!(results[0]["status"], "created"); - assert_eq!(results[1]["data"], "5.6.7.8"); - } - - #[test] - fn dns_add_partial_failure_is_an_error_with_per_record_breakdown() { - // Middle record fails: the command must NOT succeed, and the message must - // make clear which values were created and which failed (and why). - let err = summarize_add_outcomes( - "example.com", - "A", - "www", - vec![ - ("1.2.3.4".to_string(), Ok(())), - ("5.6.7.8".to_string(), Err("422 invalid data".to_string())), - ("9.9.9.9".to_string(), Ok(())), - ], - ) - .expect_err("any failure -> error"); - assert!(err.contains("added 2 of 3"), "{err}"); - assert!(err.contains("1 failed"), "{err}"); - // Per-record breakdown names both the created and the failed values. - assert!(err.contains("✓ 1.2.3.4"), "{err}"); - assert!(err.contains("✗ 5.6.7.8 — 422 invalid data"), "{err}"); - assert!(err.contains("✓ 9.9.9.9"), "{err}"); - // The recovery hint must include the domain positional or it won't parse. - assert!( - err.contains("dns list example.com --type A --name www"), - "{err}" - ); - } } diff --git a/rust/src/dns/records.rs b/rust/src/dns/records.rs new file mode 100644 index 00000000..6971847b --- /dev/null +++ b/rust/src/dns/records.rs @@ -0,0 +1,412 @@ +//! Record building, fetching, and argument helpers shared by every `dns` +//! subcommand: type/name validation, the `RecordOptions` bundle, converting +//! CLI input into v3 `DnsRecord`s, and the paginated `list_dns_records` fetch. + +use cli_engine::{CliCoreError, CommandContext, CommandSpec, NextAction, NextActionParam}; +use serde_json::Value; + +use crate::domain::api_error; +use crate::next_action::next_action; + +use domains_client::types; + +/// DNS record types the CLI can create/replace/delete via v3 (`add`/`set`/`delete`). +/// NS and SOA are registry-managed / read-only, so they're excluded. v3's +/// `DNSRecordType` is otherwise an open string; this list is the CLI's guardrail +/// against typos and includes `CAA` and GoDaddy's `ALIAS` extension. +pub(super) const WRITABLE_TYPES: &[&str] = + &["A", "AAAA", "ALIAS", "CAA", "CNAME", "MX", "SRV", "TXT"]; +/// Record types accepted by the `list` filter — the writable set plus the +/// read-only NS/SOA, which are listable even though they can't be modified. +pub(super) const LISTABLE_TYPES: &[&str] = &[ + "A", "AAAA", "ALIAS", "CAA", "CNAME", "MX", "NS", "SOA", "SRV", "TXT", +]; +/// Default TTL (seconds) for `dns add`/`set` when `--ttl` is omitted (v3 requires a ttl). +pub(super) const DEFAULT_TTL: i64 = 3600; +/// Page size for the paginated v3 list; the handler pages through until every +/// matching record is collected. +const LIST_PAGE_SIZE: i64 = 100; + +pub(super) fn arg_str(ctx: &CommandContext, key: &str) -> Option { + ctx.args + .get(key) + .and_then(|v| v.as_str()) + .map(str::to_owned) +} + +pub(super) fn arg_bool(ctx: &CommandContext, key: &str) -> bool { + ctx.args.get(key).and_then(Value::as_bool).unwrap_or(false) +} + +/// clap value-parser for a mutating `--type` (`add`/`set`/`delete`): validate +/// against [`WRITABLE_TYPES`] and return the canonical upper-case wire string. +/// The read-only NS/SOA get a clear "managed by GoDaddy" reason. Validating in +/// clap rejects invalid input at parse time — before `--dry-run`/auth short-circuits. +pub(super) fn parse_write_type_arg(raw: &str) -> Result { + let upper = raw.to_ascii_uppercase(); + if WRITABLE_TYPES.contains(&upper.as_str()) { + return Ok(upper); + } + if matches!(upper.as_str(), "NS" | "SOA") { + Err(format!( + "{upper} records are managed by GoDaddy and can't be created, replaced, or deleted; \ + writable types: {}", + WRITABLE_TYPES.join(", ") + )) + } else { + Err(format!( + "invalid record type {raw:?}; expected one of {}", + WRITABLE_TYPES.join(", ") + )) + } +} + +/// clap value-parser for the `list` `--type` filter: [`LISTABLE_TYPES`] (the +/// writable set plus read-only NS/SOA), upper-cased. +pub(super) fn parse_list_type_arg(raw: &str) -> Result { + let upper = raw.to_ascii_uppercase(); + if LISTABLE_TYPES.contains(&upper.as_str()) { + Ok(upper) + } else { + Err(format!( + "invalid record type {raw:?}; expected one of {}", + LISTABLE_TYPES.join(", ") + )) + } +} + +/// Optional record fields shared by `add` and `set`, including the CAA-only +/// `flag`/`tag`. +pub(super) struct RecordOptions { + pub(super) ttl: Option, + pub(super) priority: Option, + pub(super) port: Option, + pub(super) weight: Option, + pub(super) protocol: Option, + pub(super) service: Option, + pub(super) flag: Option, + pub(super) tag: Option, +} + +impl RecordOptions { + pub(super) fn from_ctx(ctx: &CommandContext) -> Self { + let as_i64 = |key: &str| ctx.args.get(key).and_then(Value::as_i64); + RecordOptions { + ttl: as_i64("ttl"), + priority: as_i64("priority"), + port: as_i64("port"), + weight: as_i64("weight"), + protocol: arg_str(ctx, "protocol"), + service: arg_str(ctx, "service"), + flag: as_i64("flag"), + tag: arg_str(ctx, "tag"), + } + } +} + +/// Validate the CAA-specific fields against the record type. A CAA record needs a +/// `--tag` (`--data` carries the CA domain / value); `--flag`/`--tag` are +/// meaningless for other types. Pure so it's unit-testable and runs before any +/// network call. +pub(super) fn validate_caa_fields(record_type: &str, opts: &RecordOptions) -> Result<(), String> { + if record_type == "CAA" { + if opts.tag.as_deref().map(str::trim).unwrap_or("").is_empty() { + return Err( + "CAA records require --tag (e.g. issue, issuewild, iodef); --data carries the \ + CA domain / value" + .to_string(), + ); + } + } else if opts.flag.is_some() || opts.tag.is_some() { + return Err(format!( + "--flag/--tag are only valid for CAA records, not {record_type}" + )); + } + Ok(()) +} + +/// Build one v3 `DnsRecord` from a `--data` value + shared options. `ttl` defaults +/// to [`DEFAULT_TTL`] (v3 requires it). SRV/MX numerics convert into v3's `u16` +/// and the CAA `flag` into `u8` (clap already bounds both ranges). +pub(super) fn v3_record( + name: &str, + ty: &str, + data: &str, + opts: &RecordOptions, +) -> types::DnsRecord { + let to_u16 = |v: Option| v.and_then(|n| u16::try_from(n).ok()); + types::DnsRecord { + data: data.to_owned(), + flag: opts.flag.and_then(|n| u8::try_from(n).ok()), + name: name.to_owned(), + port: to_u16(opts.port), + priority: to_u16(opts.priority), + protocol: opts.protocol.clone(), + record_id: None, + service: opts.service.clone(), + tag: opts.tag.clone(), + ttl: opts.ttl.unwrap_or(DEFAULT_TTL), + type_: types::DnsRecordType(ty.to_owned()), + weight: to_u16(opts.weight), + } +} + +/// Build the v3 `DnsRecord`s for `add` — one per `--data` value (parallel to it). +pub(super) fn v3_records( + name: &str, + ty: &str, + data: &[String], + opts: &RecordOptions, +) -> Vec { + data.iter().map(|d| v3_record(name, ty, d, opts)).collect() +} + +/// List every v3 DNS record for a zone matching the optional `type`/`name` +/// filters, paging through the collection (v3 list is paginated). Shared by +/// `list`, `set`, and `delete` — the latter two need the matching records' ids. +pub(super) async fn fetch_records( + client: &domains_client::Client, + zone: &str, + type_: Option<&str>, + name: Option<&str>, + debug: bool, +) -> Result, CliCoreError> { + let page_size = std::num::NonZeroU64::new(LIST_PAGE_SIZE as u64) + .expect("LIST_PAGE_SIZE is a positive constant"); + let mut all = Vec::new(); + let mut page: u64 = 1; + loop { + let page_nz = std::num::NonZeroU64::new(page).expect("page starts at 1 and only grows"); + let mut req = client + .list_dns_records() + .zone(zone) + .page(page_nz) + .page_size(page_size) + .total_required(true); + if let Some(t) = type_ { + req = req.type_(types::DnsRecordType(t.to_owned())); + } + if let Some(n) = name { + req = req.name(n); + } + let body = match req.send().await { + Ok(r) => r.into_inner(), + Err(e) => return Err(api_error("listing DNS records", debug, e).await), + }; + let items = body.items.unwrap_or_default(); + let got = items.len(); + all.extend(items); + // Last page when the API's total_pages is reached, or (absent that) a + // short/empty page came back. + let last = body + .total_pages + .map(|tp| page >= tp.get()) + .unwrap_or(got < LIST_PAGE_SIZE as usize); + if last || got == 0 { + break; + } + page += 1; + } + Ok(all) +} + +/// Shared flags for the mutating commands (`add`/`set`): required type/name and +/// the repeatable `--data`, plus the optional record fields. +pub(super) fn with_record_write_args(spec: CommandSpec) -> CommandSpec { + spec.with_arg( + clap::Arg::new("domain") + .value_name("DOMAIN") + .required(true) + .help("Domain whose records to modify (e.g. example.com)"), + ) + .with_arg( + clap::Arg::new("type") + .long("type") + .value_name("TYPE") + .required(true) + .value_parser(parse_write_type_arg) + .help("Record type (A, AAAA, ALIAS, CAA, CNAME, MX, SRV, TXT)"), + ) + .with_arg( + clap::Arg::new("name") + .long("name") + .value_name("NAME") + .required(true) + .help("Record name relative to the domain (e.g. www, @ for the apex)"), + ) + .with_arg( + clap::Arg::new("data") + .long("data") + .value_name("VALUE") + .required(true) + .action(clap::ArgAction::Append) + .help("Record value (repeatable for multiple records on the same name)"), + ) + .with_arg( + clap::Arg::new("ttl") + .long("ttl") + .value_name("SECONDS") + .value_parser(clap::value_parser!(i64).range(1..)) + .help("Time-to-live in seconds (defaults to 3600 when omitted)"), + ) + .with_arg( + clap::Arg::new("priority") + .long("priority") + .value_name("N") + .value_parser(clap::value_parser!(i64).range(0..=65535)) + .help("Record priority (MX and SRV only)"), + ) + .with_arg( + clap::Arg::new("port") + .long("port") + .value_name("PORT") + .value_parser(clap::value_parser!(i64).range(1..=65535)) + .help("Service port (SRV only)"), + ) + .with_arg( + clap::Arg::new("weight") + .long("weight") + .value_name("N") + .value_parser(clap::value_parser!(i64).range(0..=65535)) + .help("Record weight (SRV only)"), + ) + .with_arg( + clap::Arg::new("protocol") + .long("protocol") + .value_name("PROTO") + .help("Service protocol (SRV only)"), + ) + .with_arg( + clap::Arg::new("service") + .long("service") + .value_name("SERVICE") + .help("Service type (SRV only)"), + ) + .with_arg( + clap::Arg::new("flag") + .long("flag") + .value_name("N") + .value_parser(clap::value_parser!(i64).range(0..=255)) + .help("CAA flag byte, 0-255 (CAA only; 0 non-critical, 128 critical)"), + ) + .with_arg( + clap::Arg::new("tag") + .long("tag") + .value_name("TAG") + // A CAA record needs a tag; enforce it at parse time (before auth). + // The reverse guard — flag/tag only valid for CAA — lives in the + // handler (`validate_caa_fields`), which clap can't express. + .required_if_eq("type", "CAA") + .help("CAA property tag, e.g. issue/issuewild/iodef (CAA only; required for CAA)"), + ) +} + +/// Next action pointing back at `dns list` to verify a write, pre-filled with +/// the domain/type/name the write just touched. +pub(super) fn verify_with_list_action(domain: &str, record_type: &str, name: &str) -> NextAction { + next_action( + "dns list --type --name ", + "Verify the records for this type+name", + ) + .with_param("domain", NextActionParam::value(domain)) + .with_param("type", NextActionParam::value(record_type)) + .with_param("name", NextActionParam::value(name)) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn opts() -> RecordOptions { + RecordOptions { + ttl: None, + priority: None, + port: None, + weight: None, + protocol: None, + service: None, + flag: None, + tag: None, + } + } + + #[test] + fn parse_write_type_arg_accepts_writable_incl_caa_alias_rejects_ns_soa() { + assert_eq!(parse_write_type_arg("aaaa").expect("valid"), "AAAA"); + assert_eq!(parse_write_type_arg("caa").expect("valid"), "CAA"); + assert_eq!(parse_write_type_arg("Alias").expect("valid"), "ALIAS"); + // NS/SOA are registry-managed / read-only → rejected with a clear reason. + for ty in ["NS", "soa"] { + let err = parse_write_type_arg(ty).expect_err("read-only"); + assert!(err.contains("managed by GoDaddy"), "got: {err}"); + } + let err = parse_write_type_arg("bogus").expect_err("should reject"); + assert!(err.contains("invalid record type"), "got: {err}"); + } + + #[test] + fn parse_list_type_arg_also_accepts_ns_soa() { + assert_eq!(parse_list_type_arg("caa").expect("valid"), "CAA"); + assert_eq!(parse_list_type_arg("ns").expect("valid"), "NS"); + assert_eq!(parse_list_type_arg("SOA").expect("valid"), "SOA"); + assert!(parse_list_type_arg("bogus").is_err()); + } + + #[test] + fn v3_records_builds_one_per_data_value_with_default_ttl() { + let recs = v3_records( + "www", + "A", + &["1.2.3.4".to_string(), "5.6.7.8".to_string()], + &opts(), + ); + assert_eq!(recs.len(), 2); + assert_eq!(recs[0].name, "www"); + assert_eq!(recs[0].type_.as_str(), "A"); + assert_eq!(recs[0].data, "1.2.3.4"); + // v3 requires a ttl; an omitted --ttl falls back to the default. + assert_eq!(recs[0].ttl, DEFAULT_TTL); + assert_eq!(recs[1].data, "5.6.7.8"); + } + + #[test] + fn v3_record_carries_srv_and_caa_fields() { + let mut srv_opts = opts(); + srv_opts.ttl = Some(600); + srv_opts.priority = Some(10); + srv_opts.port = Some(443); + srv_opts.weight = Some(5); + let srv = v3_record("_sip", "SRV", "sip.example.com", &srv_opts); + assert_eq!(srv.ttl, 600); + assert_eq!(srv.priority, Some(10)); + assert_eq!(srv.port, Some(443)); + assert_eq!(srv.weight, Some(5)); + + let mut caa_opts = opts(); + caa_opts.flag = Some(128); + caa_opts.tag = Some("issue".to_string()); + let caa = v3_record("@", "CAA", "letsencrypt.org", &caa_opts); + assert_eq!(caa.flag, Some(128)); + assert_eq!(caa.tag.as_deref(), Some("issue")); + } + + #[test] + fn caa_fields_are_required_for_caa_and_rejected_otherwise() { + // CAA without --tag → rejected. + let mut caa_no_tag = opts(); + caa_no_tag.flag = Some(0); + let err = validate_caa_fields("CAA", &caa_no_tag).expect_err("CAA needs a tag"); + assert!(err.contains("--tag"), "got: {err}"); + // CAA with --tag → ok. + let mut caa = opts(); + caa.tag = Some("issue".to_string()); + assert!(validate_caa_fields("CAA", &caa).is_ok()); + // --flag/--tag on a non-CAA type → rejected. + let mut a = opts(); + a.tag = Some("issue".to_string()); + let err = validate_caa_fields("A", &a).expect_err("flag/tag are CAA-only"); + assert!(err.contains("only valid for CAA"), "got: {err}"); + // A non-CAA type with no CAA fields → ok. + assert!(validate_caa_fields("A", &opts()).is_ok()); + } +} diff --git a/rust/src/dns/set.rs b/rust/src/dns/set.rs new file mode 100644 index 00000000..f575d872 --- /dev/null +++ b/rust/src/dns/set.rs @@ -0,0 +1,522 @@ +//! `dns set` — replace every record for a type+name pair (destructive). + +use cli_engine::{CliCoreError, CommandResult, CommandSpec, RuntimeCommandSpec, Tier}; +use serde_json::{Value, json}; + +use crate::domain::{api_error, format_api_error, make_client, string_list}; +use crate::output_schema::output_schema; +use crate::scopes::DOMAINS_DNS_UPDATE; + +use domains_client::types; + +use super::conflicts::{ + conflicting_records, conflicting_records_at, describe_duplicate_record, duplicate_record_issue, +}; +use super::records::{ + RecordOptions, arg_bool, arg_str, fetch_records, v3_record, validate_caa_fields, + verify_with_list_action, with_record_write_args, +}; + +// `dns set` reconciles over v3's per-record ops; it reports how many records it +// reused (replaced), created, and deleted to reach the desired set. +output_schema!(DnsSetResult { + "domain": "string"; + "type": "string"; + "name": "string"; + "replaced": "number"; + "created": "number"; + "deleted": "number"; + "action": "string"; +}); + +/// One reconcile action for `dns set` over v3's per-record endpoints. +#[derive(Debug, PartialEq)] +enum SetAction { + /// Reuse an existing record's id, replacing its value with a new `--data`. + Replace { record_id: String, data: String }, + /// Remove an existing record no longer wanted. + Delete { record_id: String }, + /// Create a record for a surplus `--data` value. + Create { data: String }, +} + +/// Reconcile the existing records for a type+name (their ids, in list order) with +/// the desired `--data` values: reuse ids for the overlap (`Replace`), `Delete` +/// the surplus existing, `Create` the surplus desired. Pure so the plan — the +/// least-destructive way to emulate a set-replace over per-record v3 ops — is +/// unit-testable. +fn plan_set(existing_ids: &[String], desired: &[String]) -> Vec { + let overlap = existing_ids.len().min(desired.len()); + let mut actions = Vec::with_capacity(existing_ids.len().max(desired.len())); + for i in 0..overlap { + actions.push(SetAction::Replace { + record_id: existing_ids[i].clone(), + data: desired[i].clone(), + }); + } + for id in &existing_ids[overlap..] { + actions.push(SetAction::Delete { + record_id: id.clone(), + }); + } + for d in &desired[overlap..] { + actions.push(SetAction::Create { data: d.clone() }); + } + actions +} + +/// Outcome of one applied `set` reconcile action, for reporting. +struct SetOutcome { + /// What was attempted: `"replaced"`, `"created"`, or `"deleted"`. + kind: &'static str, + /// The `--data` value (replace/create) or record id (delete). + detail: String, + /// `Some(msg)` if the call failed. + error: Option, +} + +impl SetOutcome { + fn new(kind: &'static str, detail: String, error: Option) -> Self { + Self { + kind, + detail, + error, + } + } +} + +/// Build the `set` result from the applied reconcile outcomes: `Ok(json)` with +/// replaced/created/deleted counts when every action succeeded, or `Err(message)` +/// — a non-zero exit — with a per-action ✓/✗ breakdown if any failed. Pure so the +/// success/failure decision and tallies are unit-testable. +fn summarize_set_outcomes( + domain: &str, + record_type: &str, + name: &str, + outcomes: &[SetOutcome], +) -> Result { + let failed = outcomes.iter().filter(|o| o.error.is_some()).count(); + let count = |k: &str| { + outcomes + .iter() + .filter(|o| o.error.is_none() && o.kind == k) + .count() + }; + let (replaced, created, deleted) = (count("replaced"), count("created"), count("deleted")); + + if failed > 0 { + let breakdown = outcomes + .iter() + .map(|o| match &o.error { + None => format!(" ✓ {} {}", o.kind, o.detail), + Some(e) => format!(" ✗ {} {} — {e}", o.kind, o.detail), + }) + .collect::>() + .join("\n"); + return Err(format!( + "set for {name} ({record_type}) partially failed — {failed} of {} action(s):\n\ + {breakdown}\n\nRe-run `gddy dns set`, or `gddy dns list {domain} --type \ + {record_type} --name {name}` to review the current state.", + outcomes.len(), + )); + } + + Ok(json!({ + "domain": domain, + "type": record_type, + "name": name, + "replaced": replaced, + "created": created, + "deleted": deleted, + "action": "set", + })) +} + +/// Delete each conflicting record — used only by `set --replace-conflicting-types` +/// to auto-resolve a name-exclusivity conflict before retrying the write. +/// Returns one `SetOutcome` per record (rather than bailing on the first +/// failure) so a partial deletion is reported explicitly, consistent with +/// `dns delete`'s per-record reporting. +async fn delete_conflicts( + client: &domains_client::Client, + domain: &str, + records: &[types::DnsRecord], + debug: bool, +) -> Vec { + let mut outcomes = Vec::with_capacity(records.len()); + for rec in records { + let detail = format!("{} {}", rec.type_.as_str(), rec.data); + let Some(record_id) = rec.record_id.as_deref() else { + outcomes.push(SetOutcome::new( + "deleted", + detail, + Some( + "the API returned this record without a recordId, so it can't be removed \ + automatically; remove it in the control panel" + .to_string(), + ), + )); + continue; + }; + let err = match client + .delete_dns_record() + .zone(domain) + .record_id(record_id) + .send() + .await + { + Ok(_) => None, + Err(e) => Some( + api_error("deleting conflicting DNS record", debug, e) + .await + .to_string(), + ), + }; + outcomes.push(SetOutcome::new("deleted", detail, err)); + } + outcomes +} + +/// One `set` create/replace call, dispatched by [`send_write`]. +enum WriteCall<'a> { + Create, + Replace { record_id: &'a str }, +} + +async fn send_write( + client: &domains_client::Client, + domain: &str, + call: &WriteCall<'_>, + body: types::DnsRecord, +) -> Result<(), domains_client::Error<()>> { + match call { + WriteCall::Create => client + .create_dns_record() + .zone(domain) + .body(body) + .send() + .await + .map(|_| ()), + WriteCall::Replace { record_id } => client + .replace_dns_record() + .zone(domain) + .record_id(*record_id) + .body(body) + .send() + .await + .map(|_| ()), + } +} + +/// Context for one `set` create/replace action, bundled to keep +/// [`write_with_conflict_handling`]'s signature within clippy's +/// argument-count limit. +struct WriteRequest<'a> { + domain: &'a str, + name: &'a str, + record_type: &'a str, + value: &'a str, + opts: &'a RecordOptions, + replace_conflicting: bool, + debug: bool, +} + +/// Apply one `set` create/replace action, with the same name-exclusivity +/// conflict handling `add` gets (see [`super::conflicts::describe_write_error`]) +/// plus an opt-in auto-fix: with `--replace-conflicting-types`, a genuine +/// `DUPLICATE_RECORD` failure deletes the conflicting record(s) and retries the +/// write once. Returns the delete outcomes (if any) followed by the outcome for +/// the create/replace itself, so `outcomes.extend(...)` folds both into the same +/// reconcile summary. +async fn write_with_conflict_handling( + client: &domains_client::Client, + req: &WriteRequest<'_>, + call: WriteCall<'_>, +) -> Vec { + let (kind, action) = match &call { + WriteCall::Create => ("created", "creating DNS record"), + WriteCall::Replace { .. } => ("replaced", "replacing DNS record"), + }; + let outcome = |err: Option| SetOutcome::new(kind, req.value.to_string(), err); + + let body = v3_record(req.name, req.record_type, req.value, req.opts); + let err = match send_write(client, req.domain, &call, body).await { + Ok(()) => return vec![outcome(None)], + Err(e) => e, + }; + let domains_client::Error::UnexpectedResponse(resp) = err else { + return vec![outcome(Some( + api_error(action, req.debug, err).await.to_string(), + ))]; + }; + let status = resp.status(); + let request_id = resp + .headers() + .get("x-request-id") + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + let resp_body = resp.text().await.unwrap_or_default(); + + if !duplicate_record_issue(&resp_body) { + let msg = format_api_error( + action, + status.as_u16(), + &status.to_string(), + &resp_body, + request_id.as_deref(), + req.debug, + ); + return vec![outcome(Some(msg))]; + } + + let at_name = match conflicting_records_at(client, req.domain, req.name, req.debug).await { + Ok(records) => records, + Err(e) => return vec![outcome(Some(e.to_string()))], + }; + let conflicts: Vec = conflicting_records(req.record_type, &at_name) + .into_iter() + .cloned() + .collect(); + + if !req.replace_conflicting || conflicts.is_empty() { + // Either the caller didn't opt in, or this DUPLICATE_RECORD wasn't a + // cross-type conflict (e.g. an exact duplicate) — nothing to auto-fix. + let msg = describe_duplicate_record( + req.record_type, + req.value, + req.domain, + req.name, + &at_name, + true, + ); + return vec![outcome(Some(msg))]; + } + + let mut outcomes = delete_conflicts(client, req.domain, &conflicts, req.debug).await; + let retry_body = v3_record(req.name, req.record_type, req.value, req.opts); + let retry_err = match send_write(client, req.domain, &call, retry_body).await { + Ok(()) => None, + Err(e) => Some(api_error(action, req.debug, e).await.to_string()), + }; + outcomes.push(outcome(retry_err)); + outcomes +} + +pub(super) fn command() -> RuntimeCommandSpec { + RuntimeCommandSpec::new_with_context( + with_record_write_args( + CommandSpec::new( + "set", + "Replace all records for a type+name (destructive: overwrites existing)", + ) + .with_long( + "Replaces every DNS record for the given type+name pair with the \ + values supplied via `--data`, discarding any records that were \ + there before. v3 has no bulk replace, so this reconciles over \ + per-record calls: it reuses existing records for the overlap, \ + deletes the surplus, and creates the shortfall (so it is not \ + atomic — a mid-run failure is reported per record). This is \ + destructive and irreversible — use `dns list --type \ + --name ` to review first. Use `dns add` to append without \ + removing existing records.\n\ + \n\ + DNS only allows a CNAME or other record types at a name, never \ + both. Setting into a name that already has the other kind fails \ + with a specific error naming the conflict; pass \ + `--replace-conflicting-types` to remove the conflicting record(s) \ + automatically and retry.", + ) + .with_system("domain") + .with_tier(Tier::Destructive) + .with_default_fields("domain,type,name,replaced,created,deleted") + .with_output_schema::() + .with_scopes(&[DOMAINS_DNS_UPDATE]), + ) + .with_arg( + clap::Arg::new("replace-conflicting-types") + .long("replace-conflicting-types") + .action(clap::ArgAction::SetTrue) + .help( + "Remove any existing CNAME (or, if setting a CNAME, any other \ + type) at this name first", + ), + ), + |ctx| async move { + let domain = arg_str(&ctx, "domain").unwrap_or_default(); + let record_type = arg_str(&ctx, "type").unwrap_or_default(); + let name = arg_str(&ctx, "name").unwrap_or_default(); + let data = string_list(&ctx, "data"); + let opts = RecordOptions::from_ctx(&ctx); + let replace_conflicting = arg_bool(&ctx, "replace-conflicting-types"); + validate_caa_fields(&record_type, &opts).map_err(CliCoreError::message)?; + + let debug = !ctx.middleware.debug.is_empty(); + let client = make_client(&ctx).await?; + + // Reconcile the current type+name records with the desired --data + // set over v3's per-record ops (no atomic bulk replace exists). + let existing = fetch_records( + &client, + domain.as_str(), + Some(&record_type), + Some(&name), + debug, + ) + .await?; + // Every existing record must have a server id to reconcile against — + // v3 mutations are keyed by recordId. Bail before touching anything + // if any lacks one, rather than silently dropping it (which would + // leave it behind and make `set` add records instead of replacing). + if existing.iter().any(|r| r.record_id.is_none()) { + return Err(CliCoreError::message(format!( + "some existing {record_type} records for {name} were returned without a \ + recordId, so `set` can't safely replace the full set. Re-run `gddy dns \ + list {domain} --type {record_type} --name {name}` and adjust in the \ + control panel if this persists." + ))); + } + let existing_ids: Vec = existing + .iter() + .filter_map(|r| r.record_id.clone()) + .collect(); + + let mut outcomes = Vec::new(); + for action in plan_set(&existing_ids, &data) { + match action { + SetAction::Replace { + record_id, + data: value, + } => { + let req = WriteRequest { + domain: domain.as_str(), + name: name.as_str(), + record_type: record_type.as_str(), + value: &value, + opts: &opts, + replace_conflicting, + debug, + }; + outcomes.extend( + write_with_conflict_handling( + &client, + &req, + WriteCall::Replace { + record_id: record_id.as_str(), + }, + ) + .await, + ); + } + SetAction::Create { data: value } => { + let req = WriteRequest { + domain: domain.as_str(), + name: name.as_str(), + record_type: record_type.as_str(), + value: &value, + opts: &opts, + replace_conflicting, + debug, + }; + outcomes.extend( + write_with_conflict_handling(&client, &req, WriteCall::Create).await, + ); + } + SetAction::Delete { record_id } => { + let res = client + .delete_dns_record() + .zone(domain.as_str()) + .record_id(record_id.as_str()) + .send() + .await; + let err = match res { + Ok(_) => None, + Err(e) => { + Some(api_error("deleting DNS record", debug, e).await.to_string()) + } + }; + outcomes.push(SetOutcome::new("deleted", record_id.clone(), err)); + } + } + } + + summarize_set_outcomes(&domain, &record_type, &name, &outcomes) + .map(|v| { + CommandResult::new(v).with_next_actions(vec![verify_with_list_action( + &domain, + &record_type, + &name, + )]) + }) + .map_err(CliCoreError::message) + }, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn plan_set_reuses_overlap_deletes_extra_creates_shortfall() { + // 3 existing, 2 desired → reuse 2 ids, delete the 3rd. + let existing = vec!["r1".to_string(), "r2".to_string(), "r3".to_string()]; + let desired = vec!["9.9.9.9".to_string(), "8.8.8.8".to_string()]; + assert_eq!( + plan_set(&existing, &desired), + vec![ + SetAction::Replace { + record_id: "r1".into(), + data: "9.9.9.9".into() + }, + SetAction::Replace { + record_id: "r2".into(), + data: "8.8.8.8".into() + }, + SetAction::Delete { + record_id: "r3".into() + }, + ] + ); + // 1 existing, 3 desired → reuse 1 id, create 2. + assert_eq!( + plan_set( + &["r1".to_string()], + &["a".to_string(), "b".to_string(), "c".to_string()] + ), + vec![ + SetAction::Replace { + record_id: "r1".into(), + data: "a".into() + }, + SetAction::Create { data: "b".into() }, + SetAction::Create { data: "c".into() }, + ] + ); + // none existing → all creates. + assert_eq!( + plan_set(&[], &["x".to_string()]), + vec![SetAction::Create { data: "x".into() }] + ); + } + + #[test] + fn summarize_set_reports_counts_and_flags_partial_failure() { + let all_ok = vec![ + SetOutcome::new("replaced", "9.9.9.9".into(), None), + SetOutcome::new("created", "8.8.8.8".into(), None), + SetOutcome::new("deleted", "r3".into(), None), + ]; + let v = summarize_set_outcomes("example.com", "A", "www", &all_ok).expect("all ok"); + assert_eq!(v["replaced"], 1); + assert_eq!(v["created"], 1); + assert_eq!(v["deleted"], 1); + + let mixed = vec![ + SetOutcome::new("replaced", "9.9.9.9".into(), None), + SetOutcome::new("created", "8.8.8.8".into(), Some("422 bad".into())), + ]; + let err = summarize_set_outcomes("example.com", "A", "www", &mixed).expect_err("a failure"); + assert!(err.contains("1 of 2"), "{err}"); + assert!(err.contains("✗ created 8.8.8.8 — 422 bad"), "{err}"); + } +} diff --git a/rust/src/domain/common.rs b/rust/src/domain/common.rs index 892f2dcc..1412d565 100644 --- a/rust/src/domain/common.rs +++ b/rust/src/domain/common.rs @@ -254,7 +254,7 @@ pub(crate) async fn api_error( /// Build the user-facing message for an unexpected API response. Pure so the /// HTTP 402 → payment-method guidance and the `--debug` request-id line are /// unit-testable. -fn format_api_error( +pub(crate) fn format_api_error( action: &str, status: u16, status_display: &str, @@ -287,14 +287,17 @@ fn format_api_error( msg } -/// A Domains API validation error body. Tolerates both the v1 shape -/// (`{"fields":[...]}`) and the v3 shape (`{"error":{"fields":[...]}}`). +/// A Domains API validation error body. Tolerates the v1 field-level shape +/// (`{"fields":[...]}` or `{"error":{"fields":[...]}}`) and the v3 top-level +/// shape (`{"name","message","correlationId","details":[{"issue","description"}]}`). #[derive(serde::Deserialize)] struct ApiErrorBody { #[serde(default)] fields: Vec, #[serde(default)] error: Option, + #[serde(default)] + details: Vec, } #[derive(serde::Deserialize)] @@ -311,6 +314,12 @@ struct ApiFieldError { path: String, } +#[derive(serde::Deserialize)] +struct ApiDetailError { + #[serde(default)] + description: String, +} + /// Render a structured validation error as a plain-English bullet list, or `None` /// when `body` isn't a field-level validation error. fn friendly_field_errors(body: &str) -> Option { @@ -320,14 +329,25 @@ fn friendly_field_errors(body: &str) -> Option { } else { parsed.error.map(|e| e.fields).unwrap_or_default() }; - if fields.is_empty() { - return None; + if !fields.is_empty() { + let lines: Vec = fields + .iter() + .map(|f| format!(" • {}", describe_field_error(f))) + .collect(); + return Some(format!("some fields are invalid:\n{}", lines.join("\n"))); } - let lines: Vec = fields - .iter() - .map(|f| format!(" • {}", describe_field_error(f))) - .collect(); - Some(format!("some fields are invalid:\n{}", lines.join("\n"))) + if !parsed.details.is_empty() { + let lines: Vec = parsed + .details + .iter() + .filter(|d| !d.description.is_empty()) + .map(|d| format!(" • {}", d.description)) + .collect(); + if !lines.is_empty() { + return Some(format!("some fields are invalid:\n{}", lines.join("\n"))); + } + } + None } fn describe_field_error(f: &ApiFieldError) -> String { @@ -629,6 +649,25 @@ mod tests { ); } + #[test] + fn v3_top_level_details_render_as_plain_english() { + // The real body a DNS write returns on a name-exclusivity conflict: no + // `fields`/`error` envelope at all, just a top-level `details[]`. + let body = r#"{"correlationId":"abc-123","details":[{"description":"Duplicate data provided for record name, www.","issue":"DUPLICATE_RECORD"}],"message":"Request failed validation","name":"VALIDATION_ERROR"}"#; + let msg = format_api_error( + "dns set", + 422, + "422 Unprocessable Entity", + body, + None, + false, + ); + assert!( + msg.contains("Duplicate data provided for record name, www."), + "{msg}" + ); + } + #[test] fn terminal_status_detection() { assert!(is_terminal_status("COMPLETED")); diff --git a/rust/src/domain/guides/domain-purchase.md b/rust/src/domain/guides/domain-purchase.md index 7262d647..41bd466d 100644 --- a/rust/src/domain/guides/domain-purchase.md +++ b/rust/src/domain/guides/domain-purchase.md @@ -64,9 +64,6 @@ token has expired or isn't found, just re-run `gddy domain quote`. without it and it lists the agreements you must accept. - `--confirm` acknowledges that the purchase **charges your account**. Without it, the command stops before buying. -- `--agreed-by ` — the originating IP recorded with your consent. Defaults - to `127.0.0.1`; pass your real public IP if you need accurate consent - attribution. ## Contacts diff --git a/rust/src/domain/mod.rs b/rust/src/domain/mod.rs index ca0a0b16..bd25a80d 100644 --- a/rust/src/domain/mod.rs +++ b/rust/src/domain/mod.rs @@ -32,7 +32,7 @@ mod suggest; // Shared with the `dns` module, which builds the same Domains API client and // reuses the repeatable-argument helper. -pub(crate) use common::{api_error, make_client, string_list}; +pub(crate) use common::{api_error, format_api_error, make_client, string_list}; pub fn module() -> Module { Module::new("Domains", |_ctx| { diff --git a/rust/src/domain/purchase.rs b/rust/src/domain/purchase.rs index b8d9fa40..fa3a925e 100644 --- a/rust/src/domain/purchase.rs +++ b/rust/src/domain/purchase.rs @@ -29,10 +29,12 @@ fn iso_datetime(now: chrono::DateTime) -> String { now.to_rfc3339_opts(chrono::SecondsFormat::Secs, true) } -/// The GoDaddy account/customer id used as the consent `principal`, taken from the -/// OAuth token's typed `sub` claim (`customer:`). The v3 register endpoint -/// verifies this principal against the authenticated identity, so a non-customer -/// subject is rejected with a clear error before the paid call. +/// Validate the OAuth token is a customer identity (`customer:`), so a +/// non-customer subject is rejected with a clear local error *before* the paid +/// call and before the cached quote is consumed. `agreedBy` is server-derived +/// from the request's auth context (the API no longer accepts a caller-supplied +/// consent principal), so the returned id isn't sent anywhere — this is purely a +/// fail-fast check. fn consent_principal(cred: &Credential) -> Result { let id = cred .sub @@ -45,9 +47,9 @@ fn consent_principal(cred: &Credential) -> Result { cred.sub )) })?; - // The principal is sent to the paid register endpoint; validate it's a - // customer UUID up front so a malformed `customer:` subject fails - // fast with a clear message rather than as an opaque server-side rejection. + // Validate it's a customer UUID up front so a malformed + // `customer:` subject fails fast with a clear message rather + // than as an opaque server-side rejection. if uuid::Uuid::parse_str(id).is_err() { return Err(CliCoreError::message(format!( "the OAuth token's customer subject ({:?}) is not a valid UUID; `domain purchase` \ @@ -124,10 +126,18 @@ fn purchase_consent_types( ))); } - Ok(agreement_types + agreement_types .iter() - .map(|t| types::AgreementType(t.clone())) - .collect()) + .map(|t| { + t.parse::().map_err(|_| { + CliCoreError::message(format!( + "the cached quote for {domain} recorded an agreement type ({t:?}) this CLI \ + version doesn't recognize (the cache is stale or from a different CLI \ + version); re-run `gddy domain quote {domain}` for a fresh quote." + )) + }) + }) + .collect() } pub(super) fn command() -> RuntimeCommandSpec { @@ -176,12 +186,6 @@ pub(super) fn command() -> RuntimeCommandSpec { them; review with `gddy guide domain-purchase`)", ), ) - .with_arg( - clap::Arg::new("agreed-by") - .long("agreed-by") - .value_name("IP") - .help("Originating IP recorded with your consent (defaults to 127.0.0.1)"), - ) .with_arg( clap::Arg::new("confirm") .long("confirm") @@ -205,19 +209,13 @@ pub(super) fn command() -> RuntimeCommandSpec { .get("confirm") .and_then(|v| v.as_bool()) .unwrap_or(false); - let agreed_by = ctx - .args - .get("agreed-by") - .and_then(|v| v.as_str()) - .unwrap_or("127.0.0.1") - .to_owned(); let debug = !ctx.middleware.debug.is_empty(); - // The v3 register endpoint needs the customer identity from the OAuth - // token (for the consent principal). Resolve auth *before* consuming - // the cached quote so a failed login never touches the cache. + // Resolve auth *before* consuming the cached quote, so a token that + // isn't customer-scoped fails locally rather than after the cache + // entry (and the ~10-minute quote window) is spent. let cred = ctx.credential().await?; - let principal = consent_principal(&cred)?; + consent_principal(&cred)?; // Load the quote the user reviewed. Read-only: the entry is only // removed once the registration succeeds, so an un-`--agree`d run or @@ -281,12 +279,9 @@ pub(super) fn command() -> RuntimeCommandSpec { let consent = types::Consent { agreed_at: types::DateTime(iso_datetime(chrono::Utc::now())), - agreed_by: types::ConsentActor { - actor: None, - ip: Some(agreed_by), - principal, - type_: types::ConsentActorType("DIRECT".to_string()), - }, + // Server-derived from the execute request's auth context; the + // caller no longer supplies this. + agreed_by: None, agreement_types, }; let registration = types::Registration { @@ -296,7 +291,9 @@ pub(super) fn command() -> RuntimeCommandSpec { expires_at: None, links: vec![], operation_id: None, + order_id: None, period: period_nz, + price: None, profile, profile_id: None, quote_token: Some(types::Uuid(quote_token.clone())), @@ -436,7 +433,7 @@ mod tests { fn purchase_consent_requires_agree_then_confirm() { // Titles + types as they'd come from a cached quote. let titles = vec!["Registration Agreement (https://x)".to_string()]; - let ty = vec!["DNRA".to_string()]; + let ty = vec!["API_DPA".to_string()]; let err = purchase_consent_types("example.com", 1, false, true, &titles, &ty) .expect_err("must require --agree"); @@ -453,7 +450,21 @@ mod tests { let types = purchase_consent_types("example.com", 1, true, true, &titles, &ty) .expect("both gates satisfied"); assert_eq!(types.len(), 1); - assert_eq!(types[0].as_str(), "DNRA"); + assert_eq!(types[0].to_string(), "API_DPA"); + } + + #[test] + fn purchase_consent_types_rejects_a_cached_type_this_cli_no_longer_recognizes() { + // A quote cached before an agreement-type rename (e.g. the API's + // DNRA -> API_DPA rename) must fail with a clear re-quote message + // rather than panicking or silently dropping the type. + let titles = vec!["Registration Agreement (https://x)".to_string()]; + let ty = vec!["DNRA".to_string()]; + let err = purchase_consent_types("example.com", 1, true, true, &titles, &ty) + .expect_err("unrecognized agreement type must error"); + let msg = err.to_string(); + assert!(msg.contains("doesn't recognize"), "{msg}"); + assert!(msg.contains("gddy domain quote example.com"), "{msg}"); } #[test] @@ -484,11 +495,12 @@ mod tests { // An older cached quote may carry agreement types but no human titles // (titles are serde-default). The --agree prompt must still list // something actionable — fall back to the types. - let err = purchase_consent_types("example.com", 1, false, true, &[], &["DNRA".to_string()]) - .expect_err("must require --agree"); + let err = + purchase_consent_types("example.com", 1, false, true, &[], &["API_DPA".to_string()]) + .expect_err("must require --agree"); let msg = err.to_string(); assert!(msg.contains("requires agreeing"), "{msg}"); - assert!(msg.contains("- DNRA"), "{msg}"); + assert!(msg.contains("- API_DPA"), "{msg}"); } #[test] From 399c79b9759c429501e421c02417fec01973ff3d Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 12:45:03 -0700 Subject: [PATCH 2/9] fix(dns): make set/add conflict remediation hints runnable, fix delete detail format Copilot review feedback on PR #102: - dns add's DUPLICATE_RECORD remediation suggested `dns set --replace-conflicting-types` without its required args; point back at the reconstructable `dns add` command instead. - dns set's partial-failure remediation suggested `gddy dns set` bare, which can't be reconstructed (missing --data); rephrase to "the original command". - dns set's plain-delete outcomes reported the raw record id while --replace-conflicting-types deletes reported " ", mixing identifiers in the same breakdown; both now use the type+data format. Co-Authored-By: Claude Sonnet 5 --- rust/src/dns/conflicts.rs | 5 +++-- rust/src/dns/set.rs | 14 +++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/rust/src/dns/conflicts.rs b/rust/src/dns/conflicts.rs index 56f605a7..a2e8cd1e 100644 --- a/rust/src/dns/conflicts.rs +++ b/rust/src/dns/conflicts.rs @@ -77,8 +77,9 @@ pub(super) fn describe_duplicate_record( "\n\nRe-run with `--replace-conflicting-types` to remove it automatically.".to_string() } else { format!( - "\n\nRun `gddy dns delete {domain} --type {} --name {name}` first, or use \ - `gddy dns set --replace-conflicting-types`.", + "\n\nRun `gddy dns delete {domain} --type {} --name {name}` first, then \ + re-run `gddy dns add {domain} --type {desired_type} --name {name} --data \ + {desired_data}`.", conflicts[0].type_.as_str(), ) }; diff --git a/rust/src/dns/set.rs b/rust/src/dns/set.rs index f575d872..17d37679 100644 --- a/rust/src/dns/set.rs +++ b/rust/src/dns/set.rs @@ -115,8 +115,8 @@ fn summarize_set_outcomes( .join("\n"); return Err(format!( "set for {name} ({record_type}) partially failed — {failed} of {} action(s):\n\ - {breakdown}\n\nRe-run `gddy dns set`, or `gddy dns list {domain} --type \ - {record_type} --name {name}` to review the current state.", + {breakdown}\n\nRe-run the original `gddy dns set` command, or `gddy dns list \ + {domain} --type {record_type} --name {name}` to review the current state.", outcomes.len(), )); } @@ -434,7 +434,15 @@ pub(super) fn command() -> RuntimeCommandSpec { Some(api_error("deleting DNS record", debug, e).await.to_string()) } }; - outcomes.push(SetOutcome::new("deleted", record_id.clone(), err)); + // Match `delete_conflicts`'s " " detail format so a + // partial-failure breakdown doesn't mix raw record ids with + // human-readable values. + let detail = existing + .iter() + .find(|r| r.record_id.as_deref() == Some(record_id.as_str())) + .map(|r| format!("{record_type} {}", r.data)) + .unwrap_or_else(|| record_id.clone()); + outcomes.push(SetOutcome::new("deleted", detail, err)); } } } From d2760693e9347ef31b5c78531c3ecf4886e58d3e Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 12:53:47 -0700 Subject: [PATCH 3/9] fix(dns): name every conflicting type in dns add's manual remediation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DUPLICATE_RECORD remediation hint for a CNAME-vs-other-types conflict only named the first conflicting type's delete command, so a name with e.g. both A and MX records would leave the MX record behind after following the hint, and the re-run `dns add` would still fail. Also correct a doc comment claiming `conflicting_records_at`'s no-type-filter fetch matches what `dns list --name` does — `dns list` actually requires `--type` whenever `--name` is given, so this helper does something the CLI doesn't expose. --- rust/src/dns/conflicts.rs | 46 +++++++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/rust/src/dns/conflicts.rs b/rust/src/dns/conflicts.rs index a2e8cd1e..cedada1f 100644 --- a/rust/src/dns/conflicts.rs +++ b/rust/src/dns/conflicts.rs @@ -73,24 +73,31 @@ pub(super) fn describe_duplicate_record( ) -> String { let conflicts = conflicting_records(desired_type, at_name); if !conflicts.is_empty() { + let mut conflict_types: Vec<&str> = conflicts.iter().map(|r| r.type_.as_str()).collect(); + conflict_types.sort_unstable(); + conflict_types.dedup(); + let remediation = if can_auto_replace { "\n\nRe-run with `--replace-conflicting-types` to remove it automatically.".to_string() } else { + // A CNAME conflict can span multiple non-CNAME types (e.g. A + MX) at + // the same name — every one of them has to be deleted before the add + // can succeed, not just the first. + let deletes = conflict_types + .iter() + .map(|t| format!("gddy dns delete {domain} --type {t} --name {name}")) + .collect::>() + .join("; "); format!( - "\n\nRun `gddy dns delete {domain} --type {} --name {name}` first, then \ - re-run `gddy dns add {domain} --type {desired_type} --name {name} --data \ - {desired_data}`.", - conflicts[0].type_.as_str(), + "\n\nRun `{deletes}` first, then re-run `gddy dns add {domain} --type \ + {desired_type} --name {name} --data {desired_data}`.", ) }; return if desired_type == "CNAME" { - let mut types: Vec<&str> = conflicts.iter().map(|r| r.type_.as_str()).collect(); - types.sort_unstable(); - types.dedup(); format!( "`{name}` already has other record(s) ({}) at this name, which can't coexist \ with a CNAME — DNS only allows one or the other at a given name.{remediation}", - types.join(", "), + conflict_types.join(", "), ) } else { format!( @@ -117,7 +124,9 @@ pub(super) fn describe_duplicate_record( /// Fetch every record at a name (all types), for diagnosing a name-exclusivity /// conflict after a write already failed. Reuses [`fetch_records`] with no -/// type filter — the same call `dns list --name ` would make. +/// type filter — unlike `dns list`, which requires `--type` whenever `--name` +/// is given, this deliberately fetches every type at the name, since the +/// conflict could be with any of them. pub(super) async fn conflicting_records_at( client: &domains_client::Client, domain: &str, @@ -274,6 +283,25 @@ mod tests { ); assert!(msg.contains("can't coexist with a CNAME"), "{msg}"); assert!(msg.contains("A, MX"), "{msg}"); + + // Manual remediation for a multi-type conflict must name every + // conflicting type's delete command, not just the first. + let msg_no_flag = describe_duplicate_record( + "CNAME", + "target.example.net", + "example.com", + "www", + &others, + false, + ); + assert!( + msg_no_flag.contains("dns delete example.com --type A --name www"), + "{msg_no_flag}" + ); + assert!( + msg_no_flag.contains("dns delete example.com --type MX --name www"), + "{msg_no_flag}" + ); } #[test] From 740191b0b40c081c33dedfe0829a0e6cfaefdc64 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 13:02:21 -0700 Subject: [PATCH 4/9] fix(domain): fall back to issue code when v3 error detail has no description The v3 error-details schema documents description as unstable ("MAY change... MUST NOT depend on this value") while issue is the stable fine-grained code. friendly_field_errors dropped any detail with an empty description, producing an opaque fallback message even when a usable issue code was present. Co-Authored-By: Claude Sonnet 5 --- rust/src/domain/common.rs | 51 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/rust/src/domain/common.rs b/rust/src/domain/common.rs index 1412d565..dc3b2f10 100644 --- a/rust/src/domain/common.rs +++ b/rust/src/domain/common.rs @@ -318,6 +318,24 @@ struct ApiFieldError { struct ApiDetailError { #[serde(default)] description: String, + #[serde(default)] + issue: String, +} + +impl ApiDetailError { + /// The human-readable `description` when present; the v3 spec documents it + /// as unstable ("MAY change... MUST NOT depend on this value") and often + /// absent, so fall back to the stable `issue` code rather than dropping the + /// detail entirely. + fn render(&self) -> Option<&str> { + if !self.description.is_empty() { + Some(self.description.as_str()) + } else if !self.issue.is_empty() { + Some(self.issue.as_str()) + } else { + None + } + } } /// Render a structured validation error as a plain-English bullet list, or `None` @@ -340,8 +358,8 @@ fn friendly_field_errors(body: &str) -> Option { let lines: Vec = parsed .details .iter() - .filter(|d| !d.description.is_empty()) - .map(|d| format!(" • {}", d.description)) + .filter_map(ApiDetailError::render) + .map(|text| format!(" • {text}")) .collect(); if !lines.is_empty() { return Some(format!("some fields are invalid:\n{}", lines.join("\n"))); @@ -668,6 +686,35 @@ mod tests { ); } + #[test] + fn v3_details_with_no_description_fall_back_to_the_issue_code() { + // `description` is documented as unstable and often omitted; `issue` is + // the stable code. A detail with only `issue` must still render + // something useful, not fall through to the generic opaque message. + let body = r#"{"correlationId":"abc-123","details":[{"issue":"INVALID_NAMESERVER"}],"message":"Request failed validation","name":"VALIDATION_ERROR"}"#; + let msg = format_api_error( + "dns set", + 422, + "422 Unprocessable Entity", + body, + None, + false, + ); + assert!(msg.contains("INVALID_NAMESERVER"), "{msg}"); + + // Both empty → no friendly rendering, falls through to the generic message. + let body = r#"{"correlationId":"abc-123","details":[{}],"message":"Request failed validation","name":"VALIDATION_ERROR"}"#; + let msg = format_api_error( + "dns set", + 422, + "422 Unprocessable Entity", + body, + None, + false, + ); + assert!(!msg.contains("some fields are invalid"), "{msg}"); + } + #[test] fn terminal_status_detection() { assert!(is_terminal_status("COMPLETED")); From 210c555867066b38f97fcd95b9d736d489ff05c7 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 13:19:06 -0700 Subject: [PATCH 5/9] fix(dns): make add/delete recovery hints runnable commands The partial-failure hints told users to "re-run gddy dns add"/"gddy dns delete" without the required domain positional or --type/--name flags, so they couldn't be copy-pasted as-is. Co-Authored-By: Claude Sonnet 5 --- rust/src/dns/add.rs | 10 +++++++--- rust/src/dns/delete.rs | 14 ++++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/rust/src/dns/add.rs b/rust/src/dns/add.rs index cf1017a8..eb97d4bb 100644 --- a/rust/src/dns/add.rs +++ b/rust/src/dns/add.rs @@ -61,9 +61,9 @@ fn summarize_add_outcomes( .join("\n"); return Err(format!( "added {created} of {total} DNS record(s) for {name} ({record_type}); {failed} \ - failed:\n{breakdown}\n\nRe-run `gddy dns add` with just the failed value(s), or \ - `gddy dns list {domain} --type {record_type} --name {name}` to review the current \ - state." + failed:\n{breakdown}\n\nRe-run `gddy dns add {domain} --type {record_type} --name \ + {name} --data ` with just the failed value(s), or `gddy dns list {domain} \ + --type {record_type} --name {name}` to review the current state." )); } @@ -206,6 +206,10 @@ mod tests { assert!(err.contains("✗ 5.6.7.8 — 422 invalid data"), "{err}"); assert!(err.contains("✓ 9.9.9.9"), "{err}"); // The recovery hint must include the domain positional or it won't parse. + assert!( + err.contains("dns add example.com --type A --name www --data "), + "{err}" + ); assert!( err.contains("dns list example.com --type A --name www"), "{err}" diff --git a/rust/src/dns/delete.rs b/rust/src/dns/delete.rs index 3c7fad02..78b02f73 100644 --- a/rust/src/dns/delete.rs +++ b/rust/src/dns/delete.rs @@ -43,8 +43,9 @@ fn summarize_delete_outcomes( .join("\n"); return Err(format!( "deleted {deleted} of {} record(s) for {name} ({record_type}); {failed} failed:\n\ - {breakdown}\n\nRe-run `gddy dns delete`, or `gddy dns list {domain} --type \ - {record_type} --name {name}` to review the current state.", + {breakdown}\n\nRe-run `gddy dns delete {domain} --type {record_type} --name \ + {name}`, or `gddy dns list {domain} --type {record_type} --name {name}` to \ + review the current state.", outcomes.len(), )); } @@ -179,5 +180,14 @@ mod tests { summarize_delete_outcomes("example.com", "A", "www", &mixed).expect_err("a failure"); assert!(err.contains("deleted 1 of 2"), "{err}"); assert!(err.contains("✗ 5.6.7.8 — nope"), "{err}"); + // The recovery hint must include the domain positional and flags or it won't parse. + assert!( + err.contains("dns delete example.com --type A --name www"), + "{err}" + ); + assert!( + err.contains("dns list example.com --type A --name www"), + "{err}" + ); } } From b941a6f88ba205d154fc71d52addac97e4f80473 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 13:31:50 -0700 Subject: [PATCH 6/9] fix(dns): make the missing-recordId delete hint runnable The "re-run gddy dns list" hint for a record returned without a recordId omitted the domain positional and --type/--name flags, same class of bug as the add/delete recovery hints fixed earlier. Co-Authored-By: Claude Sonnet 5 --- rust/src/dns/delete.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/rust/src/dns/delete.rs b/rust/src/dns/delete.rs index 78b02f73..180472d3 100644 --- a/rust/src/dns/delete.rs +++ b/rust/src/dns/delete.rs @@ -122,12 +122,11 @@ pub(super) fn command() -> RuntimeCommandSpec { // A record with no server id can't be targeted — report it as a // failure (non-zero exit) rather than silently leaving it behind. let err = match rec.record_id.as_deref() { - None => Some( + None => Some(format!( "the API returned this record without a recordId, so it can't be \ - deleted; re-run `gddy dns list` and remove it in the control panel \ - if it persists" - .to_string(), - ), + deleted; re-run `gddy dns list {domain} --type {record_type} --name \ + {name}` and remove it in the control panel if it persists" + )), Some(id) => match client .delete_dns_record() .zone(domain.as_str()) From e477300dfe47de4bd0128d039b4a30acf8826d4b Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 13:51:28 -0700 Subject: [PATCH 7/9] fix: clarify discarded consent id, add write_with_conflict_handling tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - purchase.rs: name the discarded consent_principal() result and explain why only its validation (not the customer id) is needed here. - dns/set.rs: cover write_with_conflict_handling's DUPLICATE_RECORD branching — direct success, conflict without --replace-conflicting-types, and conflict-with-the-flag (delete_conflicts outcome then retry outcome, in order) — against a mocked domains-client server. Co-Authored-By: Claude Sonnet 5 --- rust/src/dns/set.rs | 177 ++++++++++++++++++++++++++++++++++++ rust/src/domain/purchase.rs | 7 +- 2 files changed, 182 insertions(+), 2 deletions(-) diff --git a/rust/src/dns/set.rs b/rust/src/dns/set.rs index 17d37679..9a040acf 100644 --- a/rust/src/dns/set.rs +++ b/rust/src/dns/set.rs @@ -527,4 +527,181 @@ mod tests { assert!(err.contains("1 of 2"), "{err}"); assert!(err.contains("✗ created 8.8.8.8 — 422 bad"), "{err}"); } + + // --- write_with_conflict_handling --------------------------------------- + // + // These exercise the DUPLICATE_RECORD branching against a mock server: the + // no-conflict path, the conflict-without-`--replace-conflicting-types` path + // (message only, no delete attempted), and the conflict-with-the-flag path + // (delete_conflicts outcome then the retry outcome, in that order). + + use httpmock::prelude::*; + + fn client_for(server: &MockServer) -> domains_client::Client { + domains_client::client_with_auth( + &server.base_url(), + "Bearer tok", + "godaddy-cli/test", + "req-1", + ) + .expect("build client") + } + + fn write_opts() -> RecordOptions { + RecordOptions { + ttl: None, + priority: None, + port: None, + weight: None, + protocol: None, + service: None, + flag: None, + tag: None, + } + } + + const DUPLICATE_RECORD_BODY: &str = r#"{"correlationId":"c-1","details":[{"issue":"DUPLICATE_RECORD","description":"Duplicate data provided for record name, www."}],"message":"Request failed validation","name":"VALIDATION_ERROR"}"#; + + #[tokio::test] + async fn write_with_conflict_handling_creates_without_conflict() { + let server = MockServer::start_async().await; + let create = server + .mock_async(|when, then| { + when.method(POST) + .path("/v3/domains/zones/example.com/dns-records"); + then.status(201).json_body( + json!({ "type": "A", "name": "www", "data": "1.2.3.4", "ttl": 3600 }), + ); + }) + .await; + + let opts = write_opts(); + let req = WriteRequest { + domain: "example.com", + name: "www", + record_type: "A", + value: "1.2.3.4", + opts: &opts, + replace_conflicting: false, + debug: false, + }; + let outcomes = + write_with_conflict_handling(&client_for(&server), &req, WriteCall::Create).await; + + create.assert_async().await; + assert_eq!(outcomes.len(), 1); + assert_eq!(outcomes[0].kind, "created"); + assert!(outcomes[0].error.is_none(), "{:?}", outcomes[0].error); + } + + #[tokio::test] + async fn write_with_conflict_handling_conflict_without_flag_reports_message_and_skips_delete() { + let server = MockServer::start_async().await; + server + .mock_async(|when, then| { + when.method(POST) + .path("/v3/domains/zones/example.com/dns-records"); + then.status(422).json_body( + serde_json::from_str::(DUPLICATE_RECORD_BODY).expect("valid json"), + ); + }) + .await; + server + .mock_async(|when, then| { + when.method(GET) + .path("/v3/domains/zones/example.com/dns-records") + .query_param("name", "www"); + then.status(200).json_body(json!({ + "items": [ + { "type": "CNAME", "name": "www", "data": "parkpage.godaddy.com", "ttl": 3600, "recordId": "cn-1" } + ], + "totalItems": 1, + "totalPages": 1, + "links": [] + })); + }) + .await; + + let opts = write_opts(); + let req = WriteRequest { + domain: "example.com", + name: "www", + record_type: "A", + value: "1.2.3.4", + opts: &opts, + replace_conflicting: false, + debug: false, + }; + let outcomes = + write_with_conflict_handling(&client_for(&server), &req, WriteCall::Create).await; + + // Only the message outcome — no delete attempted since the flag wasn't set. + assert_eq!(outcomes.len(), 1); + let err = outcomes[0].error.as_deref().expect("conflict reported"); + assert!(err.contains("already has a CNAME record"), "{err}"); + assert!(err.contains("--replace-conflicting-types"), "{err}"); + } + + #[tokio::test] + async fn write_with_conflict_handling_replace_flag_deletes_then_retries_in_order() { + let server = MockServer::start_async().await; + server + .mock_async(|when, then| { + when.method(POST) + .path("/v3/domains/zones/example.com/dns-records"); + then.status(422).json_body( + serde_json::from_str::(DUPLICATE_RECORD_BODY).expect("valid json"), + ); + }) + .await; + server + .mock_async(|when, then| { + when.method(GET) + .path("/v3/domains/zones/example.com/dns-records") + .query_param("name", "www"); + then.status(200).json_body(json!({ + "items": [ + { "type": "CNAME", "name": "www", "data": "parkpage.godaddy.com", "ttl": 3600, "recordId": "cn-1" } + ], + "totalItems": 1, + "totalPages": 1, + "links": [] + })); + }) + .await; + let delete = server + .mock_async(|when, then| { + when.method(DELETE) + .path("/v3/domains/zones/example.com/dns-records/cn-1"); + then.status(204); + }) + .await; + + let opts = write_opts(); + let req = WriteRequest { + domain: "example.com", + name: "www", + record_type: "A", + value: "1.2.3.4", + opts: &opts, + replace_conflicting: true, + debug: false, + }; + let outcomes = + write_with_conflict_handling(&client_for(&server), &req, WriteCall::Create).await; + + delete.assert_async().await; + // delete_conflicts' outcome must come first, then the retry outcome — + // the order `summarize_set_outcomes`'s breakdown reports to the user. + assert_eq!(outcomes.len(), 2); + assert_eq!(outcomes[0].kind, "deleted"); + assert_eq!(outcomes[0].detail, "CNAME parkpage.godaddy.com"); + assert!(outcomes[0].error.is_none(), "{:?}", outcomes[0].error); + assert_eq!(outcomes[1].kind, "created"); + assert_eq!(outcomes[1].detail, "1.2.3.4"); + // The retry hits the same mocked conflict response; unlike the first + // attempt, a failure here is reported generically (no re-diagnosis). + let err = outcomes[1].error.as_deref().expect("retry still failed"); + assert!(err.contains("Duplicate data provided"), "{err}"); + } } diff --git a/rust/src/domain/purchase.rs b/rust/src/domain/purchase.rs index fa3a925e..dfe5011c 100644 --- a/rust/src/domain/purchase.rs +++ b/rust/src/domain/purchase.rs @@ -213,9 +213,12 @@ pub(super) fn command() -> RuntimeCommandSpec { // Resolve auth *before* consuming the cached quote, so a token that // isn't customer-scoped fails locally rather than after the cache - // entry (and the ~10-minute quote window) is spent. + // entry (and the ~10-minute quote window) is spent. The register + // endpoint derives `agreedBy` server-side, so only the validation + // (the early return on error) is needed here — the customer id + // itself is intentionally discarded. let cred = ctx.credential().await?; - consent_principal(&cred)?; + let _customer_id = consent_principal(&cred)?; // Load the quote the user reviewed. Read-only: the entry is only // removed once the registration succeeds, so an un-`--agree`d run or From 1855a3fa0aeb39000e27d803b1bcd58f8bd71dc4 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 14:01:19 -0700 Subject: [PATCH 8/9] fix(domain purchase): hint at upgrading the CLI for unrecognized agreement types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A re-quote won't help when the API has introduced a genuinely new agreement type rather than the cache just being stale — that requires a newer CLI build with the updated generated AgreementType enum. --- rust/src/domain/purchase.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rust/src/domain/purchase.rs b/rust/src/domain/purchase.rs index dfe5011c..2d90ac25 100644 --- a/rust/src/domain/purchase.rs +++ b/rust/src/domain/purchase.rs @@ -132,8 +132,10 @@ fn purchase_consent_types( t.parse::().map_err(|_| { CliCoreError::message(format!( "the cached quote for {domain} recorded an agreement type ({t:?}) this CLI \ - version doesn't recognize (the cache is stale or from a different CLI \ - version); re-run `gddy domain quote {domain}` for a fresh quote." + version doesn't recognize; re-run `gddy domain quote {domain}` for a fresh \ + quote in case the cache was just stale. If the fresh quote still fails, the \ + API has introduced an agreement type this CLI build doesn't know about yet — \ + update to the latest CLI version and try again." )) }) }) @@ -468,6 +470,9 @@ mod tests { let msg = err.to_string(); assert!(msg.contains("doesn't recognize"), "{msg}"); assert!(msg.contains("gddy domain quote example.com"), "{msg}"); + // A fresh quote won't help if the API itself added a new agreement type + // this CLI build predates — the message must also point at upgrading. + assert!(msg.contains("update to the latest CLI version"), "{msg}"); } #[test] From ccde55f1b47fd7ec7aef7064b91a40712864e23b Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 14 Jul 2026 14:07:10 -0700 Subject: [PATCH 9/9] fix(dns): pluralize the auto-replace remediation hint A name-exclusivity conflict can span multiple existing records (e.g. A + MX both blocking a CNAME write); "remove it automatically" implied just one, unlike the matching CLI help text which already says "conflicting record(s)". --- rust/src/dns/conflicts.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust/src/dns/conflicts.rs b/rust/src/dns/conflicts.rs index cedada1f..45a614a0 100644 --- a/rust/src/dns/conflicts.rs +++ b/rust/src/dns/conflicts.rs @@ -78,7 +78,9 @@ pub(super) fn describe_duplicate_record( conflict_types.dedup(); let remediation = if can_auto_replace { - "\n\nRe-run with `--replace-conflicting-types` to remove it automatically.".to_string() + "\n\nRe-run with `--replace-conflicting-types` to remove the conflicting record(s) \ + automatically." + .to_string() } else { // A CNAME conflict can span multiple non-CNAME types (e.g. A + MX) at // the same name — every one of them has to be deleted before the add