feat: add domain registration & DNS management commands#189
Merged
Conversation
… commands Add complete domain lifecycle management to the CLI: - `domain search` — check domain registration availability - `domain purchase` — purchase a domain with 90s timeout for backend saga - `domain list-registered` / `domain get-registered` — view owned domains - `domain renew` / `domain auto-renew` — renewal management - `domain dns list/create/update/delete` — DNS record CRUD - `domain registrant list/create/update/delete` — registrant profile management UX improvements: - DNS commands accept `--domain` (name) instead of requiring `--domain-id` - `dns update` and `dns delete` identify records by `--type` + `--name` instead of requiring users to look up record IDs - All commands support interactive mode with guided prompts - All commands support `--json` output Aligns CLI with backend GraphQL API naming: - `checkDomainRegistrationAvailability` (not `searchDomains`) - `registeredDomainDNSRecords` / `createRegisteredDomainDNSRecord` etc. - `RegisteredDomainDNSRecordType` enum Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds a full registered-domain feature set to the CLI: domain search/purchase/renew/auto-renew, DNS record CRUD, registrant profile CRUD, corresponding API client methods, and new data models with CLI output helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI
participant Prompter
participant ApiClient
participant Printer
User->>CLI: run "purchase [domain]" (flags)
CLI->>Prompter: prompt for missing inputs (domain, registrant)
Prompter-->>CLI: user selections
CLI->>ApiClient: CheckDomainRegistrationAvailability(domain)
ApiClient-->>CLI: availability + price
CLI->>Prompter: confirm purchase (if interactive)
Prompter-->>CLI: confirmation
CLI->>ApiClient: PurchaseDomain(domain, registrantID)
ApiClient-->>CLI: PurchaseDomainResult
CLI->>Printer: print JSON or table/log
Printer-->>User: output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
internal/cmd/domain/domain.go (1)
25-25: Redundant alias: the alias matches the command name.
Aliases: []string{"domain"}is the same as theUsefield value. This alias has no effect since users would already invoke the command aszeabur domain.🔧 Proposed fix
- Aliases: []string{"domain"},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/domain/domain.go` at line 25, Remove the redundant alias declaration that duplicates the command name: delete the Aliases: []string{"domain"} entry from the Cobra command definition (the command block containing the Use: "domain" field / the command variable where Aliases is defined), or set Aliases to nil/empty if explicit presence is required; this ensures no-op alias is removed while leaving the rest of the command (Use, Short, Run, etc.) unchanged.internal/cmd/domain/dns/dnsutil/resolve.go (1)
25-37: Clarify behavior when both filters are empty.When both
recordTypeandnameare empty strings,FindRecordreturns all records. This appears intentional based on the filter logic, but consider adding a doc comment to clarify expected behavior.📝 Suggested documentation
+// FindRecord filters DNS records by type and/or name (case-insensitive). +// Pass empty strings to skip filtering on that field. +// Returns all records if both filters are empty. func FindRecord(records model.DNSRecords, recordType, name string) ([]model.DNSRecord, error) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/domain/dns/dnsutil/resolve.go` around lines 25 - 37, Add a doc comment for the FindRecord function that explicitly documents its filter semantics: that passing empty string for recordType and/or name acts as a wildcard (matches any type/name) and that when both are empty the function returns all records; reference the function name FindRecord and its parameters recordType and name (and the types model.DNSRecords/model.DNSRecord) so callers understand the intended behavior and returned error when no matches are found.internal/cmd/domain/registrant/create/create.go (1)
59-130: Interactive mode allows empty required fields.In
runCreateInteractive, if a user presses Enter without typing anything, the field remains empty (e.g.,firstNamestays""). The function then callsrunCreateNonInteractivewhich will catch this and return an error, but the UX could be improved by re-prompting for required fields.💡 Example: loop until non-empty for required fields
for opts.firstName == "" { opts.firstName, err = f.Prompter.Input("First name (required): ", "") if err != nil { return err } if opts.firstName == "" { f.Log.Warn("First name is required") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/domain/registrant/create/create.go` around lines 59 - 130, runCreateInteractive allows required fields to remain empty if the user just hits Enter, delegating validation to runCreateNonInteractive; update runCreateInteractive so each required field (firstName, lastName, email, phone, address1, city, state, country, postalCode) uses a loop that re-prompts via f.Prompter.Input until a non-empty value is provided (handle and return any input error), and optionally call f.Log.Warn or similar between attempts to tell the user the field is required; keep optional fields (address2, organization) single-prompt. Ensure you only change runCreateInteractive and keep runCreateNonInteractive validation as-is.internal/cmd/domain/registrant/update/update.go (1)
79-112: Consider validating that at least one field is provided for update.Currently, a user can call
update --id <id>without specifying any fields to change. This would result in an API call with an empty input, which may be a no-op or could cause unexpected behavior depending on the backend.💡 Proposed validation
+ // Check if at least one field is being updated + if opts.firstName == "" && opts.lastName == "" && opts.email == "" && + opts.phone == "" && opts.address1 == "" && opts.address2 == "" && + opts.city == "" && opts.state == "" && opts.country == "" && + opts.postalCode == "" && opts.organization == "" { + return fmt.Errorf("at least one field must be specified to update") + } + input := model.UpdateRegistrantProfileInput{}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/domain/registrant/update/update.go` around lines 79 - 112, The update handler currently builds a model.UpdateRegistrantProfileInput from opts but doesn't ensure any field was set; after populating input (the variable named input of type model.UpdateRegistrantProfileInput built from opts), add a check that at least one field pointer in input is non-nil (or equivalently that any opts.* string was non-empty) and if none are set return or exit with a clear error (e.g., "no fields to update") to prevent sending an empty update; update the calling function that handles the command to surface this error to the user (referencing opts, input, and model.UpdateRegistrantProfileInput to locate the code).internal/cmd/domain/dns/list/list.go (1)
63-98: Consider extracting domain resolution to a shared helper.The
resolveDomainIDfunction here duplicates the same logic found indns/delete/delete.goanddns/update/update.go. Whilednsutil.ResolveDomainIDhandles the name→ID lookup, the interactive selection fallback is repeated in each command.♻️ Suggested approach
Extract the full resolution logic (including interactive fallback) to
dnsutil:// In dnsutil/resolve.go func ResolveDomainIDInteractive(ctx context.Context, f *cmdutil.Factory, domainID, domain string) (string, error) { if domainID != "" { return domainID, nil } if domain != "" { return ResolveDomainID(ctx, f.ApiClient, domain) } if !f.Interactive { return "", fmt.Errorf("--domain is required") } // ... interactive selection logic }This would reduce duplication across list/create/update/delete commands.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cmd/domain/dns/list/list.go` around lines 63 - 98, Extract the repeated domain-resolution + interactive fallback into a new dnsutil helper (e.g. func ResolveDomainIDInteractive(ctx context.Context, f *cmdutil.Factory, domainID, domain string) (string, error)) that implements the current logic in resolveDomainID: return domainID if set, call dnsutil.ResolveDomainID(ctx, f.ApiClient, domain) when domain is provided, and if neither is set and f.Interactive is true present the interactive selection using f.ApiClient.ListRegisteredDomains and f.Prompter.Select (return an error when non‑interactive). Replace the local resolveDomainID function in list.go and the duplicated implementations in dns/delete/delete.go and dns/update/update.go to call dnsutil.ResolveDomainIDInteractive and assign the returned ID to opts.domainID, preserving existing error messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cmd/domain/auto-renew/auto_renew.go`:
- Around line 65-82: The mutual exclusivity check for opts.enable and
opts.disable must run before any interactive prompts; move the existing check
(the fmt.Errorf("cannot use both --enable and --disable") guard that references
opts.enable and opts.disable) to immediately after flag parsing and before the
block that checks !opts.enable && !opts.disable and the Prompter.Select call
(which uses f.Interactive and f.Prompter.Select). This ensures you validate the
flags first and return early if both are set, avoiding prompting for domain
selection when the flags are invalid.
In `@internal/cmd/domain/dns/update/update.go`:
- Around line 69-81: The update logic treats zero-values as "not set",
preventing explicit disabling/zeroing; change flag handling to detect whether
each flag was explicitly provided (use cmd.Flags().Changed for the proxied, ttl
and priority flags) and only set input.Proxied, input.TTL, and input.Priority
when the corresponding "was set" boolean is true; add boolean variables (e.g.
proxiedSet, ttlSet, prioritySet) when parsing flags and use them in runUpdate to
decide whether to assign opts.proxied, opts.ttl, opts.priority into the
model.UpdateDNSRecordInput fields.
In `@internal/cmd/domain/get-registered/get.go`:
- Around line 18-19: The cobra command is incorrectly named "get" — update the
cobra.Command literal assigned to cmd so its Use field is "get-registered"
(matching the package name get-registered and the PR objective); locate the cmd
:= &cobra.Command{ ... } construct and change Use: "get" to Use:
"get-registered" so the CLI becomes `zeabur domain get-registered`.
In `@pkg/model/registered_domain.go`:
- Around line 62-75: The Rows method on DomainSearchResults is causing the
prealloc lint failure because rows is built with repeated appends; preallocate
the slice capacity to avoid reallocations and satisfy the linter: initialize
rows with make([][]string, 0, len(ds)) (or allocate a len(ds) slice and assign
by index) and then populate it using the existing loop that builds each []string
for d.Domain, avail, price; update the function Rows accordingly.
- Around line 41-46: The Rows method on RegisteredDomains currently appends into
rows without preallocating, triggering the linter prealloc failure; change
Rows() to preallocate the outer slice using make with length or capacity based
on len(ds) (referencing RegisteredDomains.Rows and the local variable rows) and
then populate it (e.g., iterate with index i and assign rows[i] = d.Rows()[0] or
use append with preallocated capacity) so the slice is allocated up-front.
---
Nitpick comments:
In `@internal/cmd/domain/dns/dnsutil/resolve.go`:
- Around line 25-37: Add a doc comment for the FindRecord function that
explicitly documents its filter semantics: that passing empty string for
recordType and/or name acts as a wildcard (matches any type/name) and that when
both are empty the function returns all records; reference the function name
FindRecord and its parameters recordType and name (and the types
model.DNSRecords/model.DNSRecord) so callers understand the intended behavior
and returned error when no matches are found.
In `@internal/cmd/domain/dns/list/list.go`:
- Around line 63-98: Extract the repeated domain-resolution + interactive
fallback into a new dnsutil helper (e.g. func ResolveDomainIDInteractive(ctx
context.Context, f *cmdutil.Factory, domainID, domain string) (string, error))
that implements the current logic in resolveDomainID: return domainID if set,
call dnsutil.ResolveDomainID(ctx, f.ApiClient, domain) when domain is provided,
and if neither is set and f.Interactive is true present the interactive
selection using f.ApiClient.ListRegisteredDomains and f.Prompter.Select (return
an error when non‑interactive). Replace the local resolveDomainID function in
list.go and the duplicated implementations in dns/delete/delete.go and
dns/update/update.go to call dnsutil.ResolveDomainIDInteractive and assign the
returned ID to opts.domainID, preserving existing error messages.
In `@internal/cmd/domain/domain.go`:
- Line 25: Remove the redundant alias declaration that duplicates the command
name: delete the Aliases: []string{"domain"} entry from the Cobra command
definition (the command block containing the Use: "domain" field / the command
variable where Aliases is defined), or set Aliases to nil/empty if explicit
presence is required; this ensures no-op alias is removed while leaving the rest
of the command (Use, Short, Run, etc.) unchanged.
In `@internal/cmd/domain/registrant/create/create.go`:
- Around line 59-130: runCreateInteractive allows required fields to remain
empty if the user just hits Enter, delegating validation to
runCreateNonInteractive; update runCreateInteractive so each required field
(firstName, lastName, email, phone, address1, city, state, country, postalCode)
uses a loop that re-prompts via f.Prompter.Input until a non-empty value is
provided (handle and return any input error), and optionally call f.Log.Warn or
similar between attempts to tell the user the field is required; keep optional
fields (address2, organization) single-prompt. Ensure you only change
runCreateInteractive and keep runCreateNonInteractive validation as-is.
In `@internal/cmd/domain/registrant/update/update.go`:
- Around line 79-112: The update handler currently builds a
model.UpdateRegistrantProfileInput from opts but doesn't ensure any field was
set; after populating input (the variable named input of type
model.UpdateRegistrantProfileInput built from opts), add a check that at least
one field pointer in input is non-nil (or equivalently that any opts.* string
was non-empty) and if none are set return or exit with a clear error (e.g., "no
fields to update") to prevent sending an empty update; update the calling
function that handles the command to surface this error to the user (referencing
opts, input, and model.UpdateRegistrantProfileInput to locate the code).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6e743f3-e594-471a-a3ef-12dc48ad7b39
📒 Files selected for processing (21)
internal/cmd/domain/auto-renew/auto_renew.gointernal/cmd/domain/dns/create/create.gointernal/cmd/domain/dns/delete/delete.gointernal/cmd/domain/dns/dns.gointernal/cmd/domain/dns/dnsutil/resolve.gointernal/cmd/domain/dns/list/list.gointernal/cmd/domain/dns/update/update.gointernal/cmd/domain/domain.gointernal/cmd/domain/get-registered/get.gointernal/cmd/domain/list-registered/list.gointernal/cmd/domain/purchase/purchase.gointernal/cmd/domain/registrant/create/create.gointernal/cmd/domain/registrant/delete/delete.gointernal/cmd/domain/registrant/list/list.gointernal/cmd/domain/registrant/registrant.gointernal/cmd/domain/registrant/update/update.gointernal/cmd/domain/renew/renew.gointernal/cmd/domain/search/search.gopkg/api/interface.gopkg/api/registered_domain.gopkg/model/registered_domain.go
- Move --enable/--disable mutual exclusivity check before interactive prompts in auto-renew - Use cmd.Flags().Changed() for ttl/priority/proxied in dns update to allow explicit zero values - Fix get-registered command Use name from "get" to "get-registered" - Preallocate slices in RegisteredDomains.Rows() and DomainSearchResults.Rows() to fix prealloc lint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
checkDomainRegistrationAvailability,registeredDomainDNSRecords,RegisteredDomainDNSRecordType, etc.)UX Highlights
--domain(e.g.--domain zbr-test-final.org) instead of requiring--domain-iddns updateanddns deleteidentify records by--type+--name(e.g.--type A --name test-cli) — no need to look up record IDs--jsonoutputNew Commands
domain search <domain>domain purchase <domain>domain list-registereddomain get-registereddomain renewdomain auto-renewdomain dns listdomain dns createdomain dns updatedomain dns deletedomain registrant listdomain registrant createdomain registrant updatedomain registrant deleteTest plan
go build ./...compiles successfullygo vet ./...no warningszeabur domain search example.comreturns availability + pricezeabur domain list-registeredlists owned domainszeabur domain dns list --domain zbr-test-final.orglists DNS recordszeabur domain dns create --domain zbr-test-final.org --type A --name test-cli --content 1.2.3.4creates recordzeabur domain dns delete --domain zbr-test-final.org --type A --name test-clideletes record🤖 Generated with Claude Code
Summary by CodeRabbit