Skip to content

fix(api-catalog): resync spec drift and support TLSA DNS records - #244

Open
jpage-godaddy wants to merge 2 commits into
mainfrom
spec-drift
Open

fix(api-catalog): resync spec drift and support TLSA DNS records#244
jpage-godaddy wants to merge 2 commits into
mainfrom
spec-drift

Conversation

@jpage-godaddy

Copy link
Copy Markdown
Collaborator

Summary

  • Resyncs the embedded API catalog, the domains-client codegen spec, and the hosting-nodejs schema against upstream, fixing the failing drift check (https://github.com/godaddy/cli/actions/runs/33697956320/job/100470838384).
  • The domains v3 spec bumped to v3.3.0 in the process, adding TLSA record support (RFC 6698/DANE): DnsRecord.data became optional, with certificateData/matchingType/selector/usage required for TLSA specifically.
  • Updates gddy dns command code (records.rs, delete.rs, conflicts.rs, set/*) and domains-client's own tests to treat data as Option<String> throughout. TLSA isn't among the CLI's writable/listable record types (tracked separately in DEVEX-1105), so every record the CLI builds still populates data and leaves the new TLSA-only fields None.
  • hosting-nodejs spec changes are description-only (OAuth scope wording clarifications); no scope keys added or removed.

Test plan

  • cargo check --workspace
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace (752 + 15 + 15 passing)
  • cargo fmt --check
  • ./rust/scripts/check-module-size.sh

Regenerates the embedded API catalog, the domains-client codegen spec,
and the hosting-nodejs schema against upstream. The domains v3 spec
bumped to v3.3.0, adding TLSA record support (RFC 6698/DANE): DnsRecord's
data field became optional, with new certificateData/matchingType/
selector/usage fields required for TLSA specifically. Updates the DNS
command code to treat data as optional throughout, since TLSA is not
among the CLI's writable/listable record types.
Copilot AI lite review requested due to automatic review settings September 3, 2026 20:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

dns delete currently formats missing record data as an empty string in failure breakdown output, which can produce confusing user-facing messages and should be aligned with the explicit “(no data)” placeholder used elsewhere.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Resyncs the embedded API catalog/codegen inputs with upstream, updating the Domains v3 schema to v3.3.0 and adapting the Rust CLI + domains-client to the DnsRecord.data: Option<String> change introduced for TLSA support.

Changes:

  • Update DNS CLI record handling (dns set/dns delete/conflict detection) to treat DnsRecord.data as optional and avoid panics/invalid formatting.
  • Regenerate/refresh API schema artifacts (domains v3.3.0; hosting-nodejs description-only scope wording changes; manifest timestamp update).
  • Update tests and domains-client usage to construct/validate DnsRecord with data: Some(...) and new TLSA-only fields set to None.
File summaries
File Description
rust/src/dns/set/write.rs Adjusts conflict-deletion reporting to handle optional data.
rust/src/dns/set/outcome.rs Updates record labels to tolerate data: None in outcome reporting.
rust/src/dns/set/mod.rs Fixes optional-data plumbing when applying record replacements.
rust/src/dns/records.rs Builds v3 DnsRecord with TLSA-only fields absent and data: Some(...); updates tests for Option.
rust/src/dns/delete.rs Adapts delete outcome collection for optional data and updates tests/fixtures.
rust/src/dns/conflicts.rs Updates duplicate/conflict detection and messaging for optional data; updates tests/fixtures.
rust/schemas/openapi/hosting-nodejs-public-v1.yaml Syncs upstream wording changes in OAuth scope descriptions.
rust/schemas/api/manifest.json Updates generated timestamp for the synced schema bundle.
rust/schemas/api/domains.json Syncs Domains schema (v3.3.0) adding TLSA semantics and conditional required fields.
rust/domains-client/src/lib.rs Updates domains-client tests to construct DnsRecord with optional data + TLSA-only fields.
rust/domains-client/openapi/domains.oas3.json Syncs OAS schema to v3.3.0 including TLSA record model updates.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rust/src/dns/delete.rs
…lete

Matches the placeholder already used in conflicts.rs/set/write.rs/
set/outcome.rs for a record whose data is absent (e.g. a TLSA record),
instead of silently rendering it as a blank string in the per-record
delete breakdown.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The Option-data migration is applied consistently across the touched CLI paths and tests, and the remaining changes are upstream spec resyncs/description-only updates.

Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants