Skip to content

fix(domain): add --domain validation and fix spinner leaks in domain create #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
stantheman0128 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(domain): add --domain validation and fix spinner leaks in domain create #222

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/cmd/domain/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func runCreateDomainInteractive(f *cmdutil.Factory, opts *Options) error {
s.Start()
available, _, err := f.ApiClient.CheckDomainAvailable(context.Background(), opts.domainName, opts.IsGenerated, project.Region.ID)
if err != nil {
s.Stop()
return err
}
s.Stop()
Expand All @@ -125,6 +126,7 @@ func runCreateDomainInteractive(f *cmdutil.Factory, opts *Options) error {
s.Start()
existedDomains, err := f.ApiClient.ListDomains(context.Background(), opts.id, opts.environmentID)
if err != nil {
s.Stop()
return err
}
s.Stop()
Expand Down Expand Up @@ -162,6 +164,10 @@ func runCreateDomainNonInteractive(f *cmdutil.Factory, opts *Options) error {
return fmt.Errorf("--id or --name is required")
}

if opts.domainName == "" {
return fmt.Errorf("--domain is required in non-interactive mode")
}
Comment on lines +167 to +169
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Harden --domain validation against whitespace-only input.

Line 167 currently rejects only "". Values like " " still pass and can fail later with a confusing API error.

🔧 Suggested patch 
 import (
 	"context"
 	"fmt"
+	"strings"
@@
-	if opts.domainName == "" {
+	if strings.TrimSpace(opts.domainName) == "" {
 		return fmt.Errorf("--domain is required in non-interactive mode")
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if opts.domainName == "" {
return fmt.Errorf("--domain is required in non-interactive mode")
}
import (
"context"
"fmt"
"strings"
)
// ... existing code ...
if strings.TrimSpace(opts.domainName) == "" {
return fmt.Errorf("--domain is required in non-interactive mode")
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/cmd/domain/create/create.go` around lines 167 - 169, The current
non-interactive validation only checks opts.domainName == "" and allows
whitespace-only names; change the check to trim whitespace (e.g., use
strings.TrimSpace on opts.domainName) and treat a trimmed empty string the same
as missing, returning the existing error fmt.Errorf("--domain is required in
non-interactive mode"); update the validation in the same function where
opts.domainName is currently checked so whitespace-only inputs are rejected
early.


if opts.environmentID == "" {
envID, err := util.ResolveEnvironmentIDByServiceID(f.ApiClient, opts.id)
if err != nil {
Expand All @@ -184,6 +190,7 @@ func runCreateDomainNonInteractive(f *cmdutil.Factory, opts *Options) error {
}

if err != nil {
s.Stop()
return fmt.Errorf("add domain failed: %w", err)
}
s.Stop()
Expand Down