fix: batch of 7 CLI bug fixes - #518
Conversation
|
Warning Review limit reached
More reviews will be available in 8 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR refactors the secrets CLI command to enforce flag-based arguments and improves security group resolution by enumerating VPCs and performing per-VPC lookups. These changes tighten CLI validation and improve resource discovery across VPC boundaries. ChangesCLI Secrets and Security Group Enhancements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (1 warning, 3 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 7
🧹 Nitpick comments (2)
pkg/sdk/database_test.go (1)
22-55: ⚡ Quick winConsider adding assertions for the
AllocatedStoragefield.The test verifies
NameandEnginebut doesn't assert thatAllocatedStoragewas correctly included in the request body. Consider adding:assert.Equal(t, 10, req.AllocatedStorage)This ensures the field is properly serialized in the API request.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/sdk/database_test.go` around lines 22 - 55, In TestClientCreateDatabase add an assertion that the request's AllocatedStorage is sent correctly: when decoding the incoming request into CreateDatabaseInput (the variable req inside the httptest handler) assert req.AllocatedStorage equals 10 so the test verifies the client.CreateDatabase(dbTestName, "postgres", "14", &vpcID, 10) call serializes AllocatedStorage; update the handler assertions (where req is decoded) to include this check.cmd/cloud/db.go (1)
68-76: ⚡ Quick winValidate that size meets the minimum requirement.
The API requires a minimum of 10GB, but there's no client-side validation. Consider adding a check to provide immediate feedback if the user specifies an invalid size.
🛡️ Proposed validation
size, err := cmd.Flags().GetInt("size") + if err != nil { + fmt.Printf(errorFormat, err) + return + } + if size < 10 { + fmt.Printf("Error: allocated storage must be at least 10GB, got %dGB\n", size) + return + } var vpcPtr *string🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cloud/db.go` around lines 68 - 76, The CLIs DB create path reads size via cmd.Flags().GetInt("size") and then calls client.CreateDatabase(name, engine, version, vpcPtr, size) without validating minimum; add a guard after retrieving size that checks size >= 10 and, if not, returns or prints a user-friendly error and exits (or returns an error from the command) so the CreateDatabase call is never made with an invalid size; update the block around the size variable and the CreateDatabase invocation to enforce this validation and surface the message to the user.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloud/db.go`:
- Line 68: Capture and handle the error returned by cmd.Flags().GetInt("size")
instead of discarding it; update the call that currently does `size, _ :=
cmd.Flags().GetInt("size")` to `size, err := cmd.Flags().GetInt("size")` (or
similar), then check err and handle it consistently with surrounding command
error handling (e.g., return the error, log with processLogger/error and exit,
or fall back to a validated default), ensuring the change is applied in the same
function where cmd.Flags().GetInt("size") is called so failures are not silently
ignored.
In `@cmd/cloud/dns.go`:
- Line 204: The call currently discards the error from
dnsCreateZoneCmd.MarkFlagRequired("vpc-id"); capture its return value and handle
it instead of using the blank identifier: call MarkFlagRequired on
dnsCreateZoneCmd, check err != nil and report the error using the file's
existing error format constant (include a clear context string) and exit/return
appropriately; apply the same change to the other occurrence on
dnsCreateRecordCmd so both MarkFlagRequired calls are validated and errors
surfaced instead of silently ignored.
In `@cmd/cloud/sg_test.go`:
- Around line 58-66: The test HTTP handler in cmd/cloud/sg_test.go currently
discards errors from json.NewEncoder(w).Encode(...) which can mask serialization
failures; change both encoder calls (the one returning VPC list and the one
returning SecurityGroup list) to check the returned error and fail the test or
return an HTTP error on failure (e.g., capture err :=
json.NewEncoder(w).Encode(...); if err != nil { t.Fatalf("json encode failed:
%v", err) } or write an http.Error with the error) so encoding failures are
reported instead of ignored.
In `@cmd/cloud/sg.go`:
- Around line 247-263: resolveSGID currently only searches security groups
returned by client.ListSecurityGroups(vpc.ID) for each VPC and thus can miss
tenant-scoped (no vpc_id) groups; update resolveSGID to also perform a
tenant-scoped list and check those groups by name before returning idOrName.
Specifically, after the VPC loop call the API that lists non-VPC-scoped groups
(e.g., client.ListSecurityGroups with an empty/zero vpc filter or the dedicated
tenant-scoped listing method on the client), iterate its results and return g.ID
when g.Name == idOrName, keeping the existing fallback of returning idOrName if
nothing matches. Ensure you reference resolveSGID, client.ListVPCs, and
client.ListSecurityGroups (or the tenant-scoped listing method) when making the
change.
In `@cmd/cloud/subnet.go`:
- Around line 28-29: resolveVPCID currently returns the raw input on lookup
failure which leads to confusing downstream errors when calling
client.ListSubnets; update the calling code to detect a failed resolution and
return a clear error instead of proceeding: after calling resolveVPCID(args[0],
client) in the subnet list flow (and likewise in the other occurrence around the
124-138 block), if the returned vpcID indicates a lookup failure (e.g., equals
the original args[0] or a sentinel value used by resolveVPCID) then return an
error or print a failure message "VPC resolution failed for <input>" and abort
before invoking client.ListSubnets; if you can change resolveVPCID, prefer
altering it to return (string, error) and propagate that error to the caller so
the subnet listing functions fail fast with a clear message.
In `@pkg/sdk/database.go`:
- Line 29: The AllocatedStorage field on the database payload (AllocatedStorage
int `json:"allocated_storage_gb,omitempty"`) can be omitted due to `omitempty`,
which hides an explicit 0 and can trigger unclear API errors; update
CreateDatabase to validate the incoming value (e.g., in CreateDatabase or the
request-building helper) and return a clear error if allocatedStorage < 10, or
alternatively remove `omitempty` so the field is always serialized; locate and
modify the AllocatedStorage field declaration and the CreateDatabase function to
implement the chosen fix and ensure any API request always contains a valid
>=10GB value.
- Line 34: The CreateDatabase method on type Client should accept
context.Context as its first parameter: change the signature of
Client.CreateDatabase to include ctx context.Context first, update all internal
uses to pass ctx into the blocking HTTP helper (propagate ctx into the
c.post(...) call), and update all call sites to pass through the caller's
context; ensure any related helpers and tests that assume the old signature are
updated accordingly.
---
Nitpick comments:
In `@cmd/cloud/db.go`:
- Around line 68-76: The CLIs DB create path reads size via
cmd.Flags().GetInt("size") and then calls client.CreateDatabase(name, engine,
version, vpcPtr, size) without validating minimum; add a guard after retrieving
size that checks size >= 10 and, if not, returns or prints a user-friendly error
and exits (or returns an error from the command) so the CreateDatabase call is
never made with an invalid size; update the block around the size variable and
the CreateDatabase invocation to enforce this validation and surface the message
to the user.
In `@pkg/sdk/database_test.go`:
- Around line 22-55: In TestClientCreateDatabase add an assertion that the
request's AllocatedStorage is sent correctly: when decoding the incoming request
into CreateDatabaseInput (the variable req inside the httptest handler) assert
req.AllocatedStorage equals 10 so the test verifies the
client.CreateDatabase(dbTestName, "postgres", "14", &vpcID, 10) call serializes
AllocatedStorage; update the handler assertions (where req is decoded) to
include this check.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a0933901-34e7-491e-b01f-6c196ea6eddb
📒 Files selected for processing (8)
cmd/cloud/db.gocmd/cloud/dns.gocmd/cloud/secrets.gocmd/cloud/sg.gocmd/cloud/sg_test.gocmd/cloud/subnet.gopkg/sdk/database.gopkg/sdk/database_test.go
| engine, _ := cmd.Flags().GetString("engine") | ||
| version, _ := cmd.Flags().GetString("version") | ||
| vpc, _ := cmd.Flags().GetString("vpc") | ||
| size, _ := cmd.Flags().GetInt("size") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Handle the error from GetInt instead of silently discarding it.
As per coding guidelines, avoid silent failures with blank identifier assignments. While GetInt errors are rare, handling them ensures robustness.
♻️ Proposed fix
- size, _ := cmd.Flags().GetInt("size")
+ size, err := cmd.Flags().GetInt("size")
+ if err != nil {
+ fmt.Printf(errorFormat, err)
+ return
+ }As per coding guidelines: "Do not use silent failures - avoid blank identifier assignment like _ = someFunc()".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/db.go` at line 68, Capture and handle the error returned by
cmd.Flags().GetInt("size") instead of discarding it; update the call that
currently does `size, _ := cmd.Flags().GetInt("size")` to `size, err :=
cmd.Flags().GetInt("size")` (or similar), then check err and handle it
consistently with surrounding command error handling (e.g., return the error,
log with processLogger/error and exit, or fall back to a validated default),
ensuring the change is applied in the same function where
cmd.Flags().GetInt("size") is called so failures are not silently ignored.
| func init() { | ||
| dnsCreateZoneCmd.Flags().String("description", "", "Description of the zone") | ||
| dnsCreateZoneCmd.Flags().String("vpc-id", "", "Associate with a VPC for private DNS") | ||
| _ = dnsCreateZoneCmd.MarkFlagRequired("vpc-id") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "dns.go" | grep -E "cmd/cloud"Repository: poyrazK/thecloud
Length of output: 77
🏁 Script executed:
if [ -f "cmd/cloud/dns.go" ]; then
wc -l cmd/cloud/dns.go
fiRepository: poyrazK/thecloud
Length of output: 81
🏁 Script executed:
if [ -f "cmd/cloud/dns.go" ]; then
sed -n '200,210p' cmd/cloud/dns.go
fiRepository: poyrazK/thecloud
Length of output: 689
🏁 Script executed:
# Search for other MarkFlagRequired usages in the codebase
rg "MarkFlagRequired" -A 2 -B 2 --type goRepository: poyrazK/thecloud
Length of output: 7299
🏁 Script executed:
head -n 30 cmd/cloud/dns.goRepository: poyrazK/thecloud
Length of output: 651
🏁 Script executed:
# Check for dnsErrorFormat in the file
rg "dnsErrorFormat" cmd/cloud/dns.goRepository: poyrazK/thecloud
Length of output: 307
Replace silent error discard with proper error handling on flag validation.
Line 204 silently discards an error from MarkFlagRequired, which violates the guideline against blank identifier assignments. The error format constant is already available in this file.
Suggested fix
- _ = dnsCreateZoneCmd.MarkFlagRequired("vpc-id")
+ cobra.CheckErr(dnsCreateZoneCmd.MarkFlagRequired("vpc-id"))As per coding guidelines: "Do not use silent failures - avoid blank identifier assignment like _ = someFunc()". Note that the same pattern appears elsewhere in this file for dnsCreateRecordCmd and should be fixed consistently.
📝 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.
| _ = dnsCreateZoneCmd.MarkFlagRequired("vpc-id") | |
| cobra.CheckErr(dnsCreateZoneCmd.MarkFlagRequired("vpc-id")) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/dns.go` at line 204, The call currently discards the error from
dnsCreateZoneCmd.MarkFlagRequired("vpc-id"); capture its return value and handle
it instead of using the blank identifier: call MarkFlagRequired on
dnsCreateZoneCmd, check err != nil and report the error using the file's
existing error format constant (include a clear context string) and exit/return
appropriately; apply the same change to the other occurrence on
dnsCreateRecordCmd so both MarkFlagRequired calls are validated and errors
surfaced instead of silently ignored.
| _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ | ||
| Data: []sdk.VPC{ | ||
| {ID: "vpc-1", Name: "my-vpc"}, | ||
| }, | ||
| }) | ||
| return | ||
| } | ||
| w.Header().Set("Content-Type", "application/json") | ||
| _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{ | ||
| Data: []sdk.SecurityGroup{ | ||
| {ID: "uuid-sg-1", Name: "my-sg", VPCID: "vpc-1", ARN: "arn:cloud:sg:1"}, | ||
| }, | ||
| }) | ||
| if r.URL.Path == "/security-groups" && r.URL.Query().Get("vpc_id") == "vpc-1" { | ||
| _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{ |
There was a problem hiding this comment.
Avoid silent JSON encode failures in the test server handler.
Lines 58 and 66 discard encoder errors; if serialization fails, the test can fail for unclear reasons.
Suggested fix
- _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{
+ if err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{
Data: []sdk.VPC{
{ID: "vpc-1", Name: "my-vpc"},
},
- })
+ }); err != nil {
+ http.Error(w, err.Error(), http.StatusInternalServerError)
+ }
return
}
if r.URL.Path == "/security-groups" && r.URL.Query().Get("vpc_id") == "vpc-1" {
- _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{
+ if err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{
Data: []sdk.SecurityGroup{
{ID: "uuid-sg-1", Name: "my-sg", VPCID: "vpc-1", ARN: "arn:cloud:sg:1"},
},
- })
+ }); err != nil {
+ http.Error(w, err.Error(), http.StatusInternalServerError)
+ }
return
}As per coding guidelines, "Do not use silent failures - avoid blank identifier assignment like _ = someFunc()".
📝 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.
| _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ | |
| Data: []sdk.VPC{ | |
| {ID: "vpc-1", Name: "my-vpc"}, | |
| }, | |
| }) | |
| return | |
| } | |
| w.Header().Set("Content-Type", "application/json") | |
| _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{ | |
| Data: []sdk.SecurityGroup{ | |
| {ID: "uuid-sg-1", Name: "my-sg", VPCID: "vpc-1", ARN: "arn:cloud:sg:1"}, | |
| }, | |
| }) | |
| if r.URL.Path == "/security-groups" && r.URL.Query().Get("vpc_id") == "vpc-1" { | |
| _ = json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{ | |
| if err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ | |
| Data: []sdk.VPC{ | |
| {ID: "vpc-1", Name: "my-vpc"}, | |
| }, | |
| }); err != nil { | |
| http.Error(w, err.Error(), http.StatusInternalServerError) | |
| } | |
| return | |
| } | |
| if r.URL.Path == "/security-groups" && r.URL.Query().Get("vpc_id") == "vpc-1" { | |
| if err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.SecurityGroup]{ | |
| Data: []sdk.SecurityGroup{ | |
| {ID: "uuid-sg-1", Name: "my-sg", VPCID: "vpc-1", ARN: "arn:cloud:sg:1"}, | |
| }, | |
| }); err != nil { | |
| http.Error(w, err.Error(), http.StatusInternalServerError) | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/sg_test.go` around lines 58 - 66, The test HTTP handler in
cmd/cloud/sg_test.go currently discards errors from
json.NewEncoder(w).Encode(...) which can mask serialization failures; change
both encoder calls (the one returning VPC list and the one returning
SecurityGroup list) to check the returned error and fail the test or return an
HTTP error on failure (e.g., capture err := json.NewEncoder(w).Encode(...); if
err != nil { t.Fatalf("json encode failed: %v", err) } or write an http.Error
with the error) so encoding failures are reported instead of ignored.
| // Try to find by name - list all VPCs and check each for the security group | ||
| vpcs, err := client.ListVPCs() | ||
| if err != nil { | ||
| return idOrName | ||
| } | ||
| for _, g := range groups { | ||
| if g.Name == idOrName { | ||
| return g.ID | ||
| for _, vpc := range vpcs { | ||
| groups, err := client.ListSecurityGroups(vpc.ID) | ||
| if err != nil { | ||
| continue | ||
| } | ||
| for _, g := range groups { | ||
| if g.Name == idOrName { | ||
| return g.ID | ||
| } | ||
| } | ||
| } | ||
| return idOrName |
There was a problem hiding this comment.
resolveSGID may miss tenant-scoped security groups by name.
The new lookup only scans SGs under each VPC. If a group is tenant-scoped (no vpc_id filter), name resolution can regress for sg get/rm/add-rule.
Suggested fix
func resolveSGID(idOrName string, client *sdk.Client) string {
if _, err := uuid.Parse(idOrName); err == nil {
return idOrName
}
+ // Check tenant-scoped/global groups first.
+ if groups, err := client.ListSecurityGroups(""); err == nil {
+ for _, g := range groups {
+ if g.Name == idOrName {
+ return g.ID
+ }
+ }
+ }
// Try to find by name - list all VPCs and check each for the security group
vpcs, err := client.ListVPCs()
if err != nil {
return idOrName
}📝 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.
| // Try to find by name - list all VPCs and check each for the security group | |
| vpcs, err := client.ListVPCs() | |
| if err != nil { | |
| return idOrName | |
| } | |
| for _, g := range groups { | |
| if g.Name == idOrName { | |
| return g.ID | |
| for _, vpc := range vpcs { | |
| groups, err := client.ListSecurityGroups(vpc.ID) | |
| if err != nil { | |
| continue | |
| } | |
| for _, g := range groups { | |
| if g.Name == idOrName { | |
| return g.ID | |
| } | |
| } | |
| } | |
| return idOrName | |
| func resolveSGID(idOrName string, client *sdk.Client) string { | |
| if _, err := uuid.Parse(idOrName); err == nil { | |
| return idOrName | |
| } | |
| // Check tenant-scoped/global groups first. | |
| if groups, err := client.ListSecurityGroups(""); err == nil { | |
| for _, g := range groups { | |
| if g.Name == idOrName { | |
| return g.ID | |
| } | |
| } | |
| } | |
| // Try to find by name - list all VPCs and check each for the security group | |
| vpcs, err := client.ListVPCs() | |
| if err != nil { | |
| return idOrName | |
| } | |
| for _, vpc := range vpcs { | |
| groups, err := client.ListSecurityGroups(vpc.ID) | |
| if err != nil { | |
| continue | |
| } | |
| for _, g := range groups { | |
| if g.Name == idOrName { | |
| return g.ID | |
| } | |
| } | |
| } | |
| return idOrName | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/sg.go` around lines 247 - 263, resolveSGID currently only searches
security groups returned by client.ListSecurityGroups(vpc.ID) for each VPC and
thus can miss tenant-scoped (no vpc_id) groups; update resolveSGID to also
perform a tenant-scoped list and check those groups by name before returning
idOrName. Specifically, after the VPC loop call the API that lists
non-VPC-scoped groups (e.g., client.ListSecurityGroups with an empty/zero vpc
filter or the dedicated tenant-scoped listing method on the client), iterate its
results and return g.ID when g.Name == idOrName, keeping the existing fallback
of returning idOrName if nothing matches. Ensure you reference resolveSGID,
client.ListVPCs, and client.ListSecurityGroups (or the tenant-scoped listing
method) when making the change.
| vpcID := resolveVPCID(args[0], client) | ||
| subnets, err := client.ListSubnets(vpcID) |
There was a problem hiding this comment.
Don’t silently fall back when VPC name resolution fails.
resolveVPCID returns the raw input on lookup failure, so subnet list can produce misleading downstream errors instead of a clear “VPC resolution failed” message.
Suggested fix
- vpcID := resolveVPCID(args[0], client)
+ vpcID, err := resolveVPCID(args[0], client)
+ if err != nil {
+ fmt.Printf(subnetErrorFormat, err)
+ return
+ }
subnets, err := client.ListSubnets(vpcID)-func resolveVPCID(idOrName string, client *sdk.Client) string {
+func resolveVPCID(idOrName string, client *sdk.Client) (string, error) {
if _, err := uuid.Parse(idOrName); err == nil {
- return idOrName
+ return idOrName, nil
}
vpcs, err := client.ListVPCs()
if err != nil {
- return idOrName
+ return "", fmt.Errorf("failed to resolve VPC %q: %w", idOrName, err)
}
for _, v := range vpcs {
if v.Name == idOrName {
- return v.ID
+ return v.ID, nil
}
}
- return idOrName
+ return "", fmt.Errorf("vpc %q not found", idOrName)
}As per coding guidelines, "Do not use silent failures - avoid blank identifier assignment like _ = someFunc()".
Also applies to: 124-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/subnet.go` around lines 28 - 29, resolveVPCID currently returns the
raw input on lookup failure which leads to confusing downstream errors when
calling client.ListSubnets; update the calling code to detect a failed
resolution and return a clear error instead of proceeding: after calling
resolveVPCID(args[0], client) in the subnet list flow (and likewise in the other
occurrence around the 124-138 block), if the returned vpcID indicates a lookup
failure (e.g., equals the original args[0] or a sentinel value used by
resolveVPCID) then return an error or print a failure message "VPC resolution
failed for <input>" and abort before invoking client.ListSubnets; if you can
change resolveVPCID, prefer altering it to return (string, error) and propagate
that error to the caller so the subnet listing functions fail fast with a clear
message.
| Engine string `json:"engine"` | ||
| Version string `json:"version"` | ||
| VpcID *string `json:"vpc_id,omitempty"` | ||
| AllocatedStorage int `json:"allocated_storage_gb,omitempty"` |
There was a problem hiding this comment.
Consider removing omitempty or adding validation.
The AllocatedStorage field has omitempty, but the API requires a minimum of 10GB. If a user explicitly passes 0, it will be omitted from the request, potentially causing unclear API errors. Consider either:
- Removing
omitemptyto always send the value, or - Adding validation in
CreateDatabaseto ensureallocatedStorage >= 10
🛡️ Option 2: Add validation
func (c *Client) CreateDatabase(name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) {
+ if allocatedStorage > 0 && allocatedStorage < 10 {
+ return nil, fmt.Errorf("allocated storage must be at least 10GB, got %d", allocatedStorage)
+ }
input := CreateDatabaseInput{🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/sdk/database.go` at line 29, The AllocatedStorage field on the database
payload (AllocatedStorage int `json:"allocated_storage_gb,omitempty"`) can be
omitted due to `omitempty`, which hides an explicit 0 and can trigger unclear
API errors; update CreateDatabase to validate the incoming value (e.g., in
CreateDatabase or the request-building helper) and return a clear error if
allocatedStorage < 10, or alternatively remove `omitempty` so the field is
always serialized; locate and modify the AllocatedStorage field declaration and
the CreateDatabase function to implement the chosen fix and ensure any API
request always contains a valid >=10GB value.
| const databasesPath = "/databases/" | ||
|
|
||
| func (c *Client) CreateDatabase(name, engine, version string, vpcID *string) (*Database, error) { | ||
| func (c *Client) CreateDatabase(name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Add context.Context as the first parameter.
The method makes HTTP calls but lacks a context.Context parameter. As per coding guidelines, all functions should include context.Context as the first parameter, and it should be propagated to all blocking calls.
♻️ Proposed signature change
-func (c *Client) CreateDatabase(name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) {
+func (c *Client) CreateDatabase(ctx context.Context, name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) {Then propagate ctx to the c.post(...) call (assuming post accepts context).
As per coding guidelines: "Do not skip context.Context as the first parameter in functions" and "Propagate context.Context to all blocking calls".
📝 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.
| func (c *Client) CreateDatabase(name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) { | |
| func (c *Client) CreateDatabase(ctx context.Context, name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/sdk/database.go` at line 34, The CreateDatabase method on type Client
should accept context.Context as its first parameter: change the signature of
Client.CreateDatabase to include ctx context.Context first, update all internal
uses to pass ctx into the blocking HTTP helper (propagate ctx into the
c.post(...) call), and update all call sites to pass through the caller's
context; ensure any related helpers and tests that assume the old signature are
updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/cloud/subnet_cli_test.go (1)
102-138: ⚡ Quick winRefactor these two VPC resolver tests into a single table-driven test.
Coverage is good, but these cases are ideal for one table-driven test (
name -> resolved ID,uuid -> passthrough) to match repo test conventions and reduce duplication.Suggested refactor
+func TestResolveVPCID(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + expected string + handler http.HandlerFunc + }{ + { + name: "resolves by name", + input: "my-vpc", + expected: "uuid-vpc-1", + handler: func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/vpcs" { + if err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ + Data: []sdk.VPC{{ID: "uuid-vpc-1", Name: "my-vpc", CIDRBlock: "10.0.0.0/16"}}, + }); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + return + } + w.WriteHeader(http.StatusNotFound) + }, + }, + { + name: "passthrough uuid", + input: "abc123-def456", + expected: "abc123-def456", + handler: func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) // should not be called + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + server := httptest.NewServer(tc.handler) + defer server.Close() + + client := sdk.NewClient(server.URL, "test-key") + resolved := resolveVPCID(tc.input, client) + if resolved != tc.expected { + t.Fatalf("expected %s, got %s", tc.expected, resolved) + } + }) + } +}As per coding guidelines, "Use table-driven tests in test files".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cloud/subnet_cli_test.go` around lines 102 - 138, Replace the two tests TestResolveVPCIDByName and TestResolveVPCIDByUUID with a single table-driven test that iterates cases (e.g., {"name":"my-vpc","expected":"uuid-vpc-1"} and {"id":"abc123-def456","expected":"abc123-def456"}), creating the appropriate httptest.Server behavior per case (for the name case return /vpcs JSON with sdk.VPC; for the uuid/passthrough case the server need not be hit), call resolveVPCID with sdk.NewClient(server.URL, "test-key") for each subtest, and assert resolved == expected; use t.Run for each table entry and call t.Parallel() inside each subtest to preserve parallelism. Ensure you reference the existing resolveVPCID function and reuse sdk.NewClient and httptest.NewServer as in the original tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloud/subnet_cli_test.go`:
- Around line 107-111: The test currently discards the result of
json.NewEncoder(w).Encode(...) which can hide setup failures; change the call in
subnet_cli_test.go to capture the error from the encoder and fail the test on
error (e.g., assign err := json.NewEncoder(w).Encode(...); then call t.Fatalf or
require.NoError(t, err)). Update the encode call near the JSON response
construction for the sdk.Response[[]sdk.VPC] block so failures are reported
instead of ignored.
---
Nitpick comments:
In `@cmd/cloud/subnet_cli_test.go`:
- Around line 102-138: Replace the two tests TestResolveVPCIDByName and
TestResolveVPCIDByUUID with a single table-driven test that iterates cases
(e.g., {"name":"my-vpc","expected":"uuid-vpc-1"} and
{"id":"abc123-def456","expected":"abc123-def456"}), creating the appropriate
httptest.Server behavior per case (for the name case return /vpcs JSON with
sdk.VPC; for the uuid/passthrough case the server need not be hit), call
resolveVPCID with sdk.NewClient(server.URL, "test-key") for each subtest, and
assert resolved == expected; use t.Run for each table entry and call
t.Parallel() inside each subtest to preserve parallelism. Ensure you reference
the existing resolveVPCID function and reuse sdk.NewClient and
httptest.NewServer as in the original tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7081d05a-f5e9-4d00-a785-8062c61ae9d7
📒 Files selected for processing (2)
cmd/cloud/db.gocmd/cloud/subnet_cli_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/cloud/db.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloud/subnet_cli_test.go`:
- Around line 145-153: The test doesn't verify whether the httptest server was
actually called; add a callCount variable and wrap the server handler to
increment it on each request (using httptest.NewServer with a handler that does
callCount++ and returns the intended response), then after calling
resolveVPCID(tt.input, client) assert that callCount matches tt.wantServer
(e.g., if wantServer is a bool assert (callCount > 0) == tt.wantServer, or
assert.Equal(t, tt.wantServer, callCount > 0)). Update the server handler logic
for both branches (expected-call and "should not be called") to increment the
shared callCount so the test can assert the resolver behavior; keep using
sdk.NewClient and resolveVPCID names as-is.
- Around line 133-139: The test currently calls require.NoError(t, err) inside
the HTTP handler goroutine (from json.NewEncoder(w).Encode(...)), which is
unsafe; replace that with local error handling in the handler: check the error
returned by json.NewEncoder(w).Encode, and if non-nil write an HTTP error (e.g.,
http.Error(w, err.Error(), http.StatusInternalServerError)) and return. If the
test must assert on that encode error, propagate it to the test goroutine via a
channel or a shared variable and assert there instead of calling
require.NoError(t, err) inside the handler.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 60a57aff-8749-42c6-be73-7734d307f63d
📒 Files selected for processing (1)
cmd/cloud/subnet_cli_test.go
| err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ | ||
| Data: []sdk.VPC{ | ||
| {ID: "uuid-vpc-1", Name: "my-vpc", CIDRBlock: "10.0.0.0/16"}, | ||
| }, | ||
| }) | ||
| require.NoError(t, err) | ||
| return |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In github.com/stretchr/testify/require (v1.11.1), is require.NoError safe to call from a non-test goroutine such as an httptest HTTP handler, given that require failures use FailNow?
💡 Result:
No, require.NoError is not safe to call from a non-test goroutine such as an httptest HTTP handler in github.com/stretchr/testify/require v1.11.1. The official documentation explicitly states that require functions must be called from the goroutine running the test or benchmark function, not from other goroutines created during the test, as this can cause race conditions [1][2][3]. In the source code, require.NoError calls t.FailNow on failure, which internally invokes runtime.Goexit and is only intended for the test goroutine; calling it from another goroutine (like an HTTP handler goroutine) leads to data races, panics, or undefined behavior [4][5][6]. Linters like testifylint specifically flag require usage in HTTP handlers and other goroutines for this reason [7][6]. Use assert.NoError instead in such contexts, as it calls t.Error and does not terminate the goroutine [8].
Citations:
- 1: https://github.com/stretchr/testify
- 2: https://pkg.go.dev/github.com/stretchr/testify
- 3: https://pkg.go.dev/github.com/stretchr/testify/require
- 4: https://github.com/stretchr/testify/blob/v1.11.1/require/require.go
- 5: https://github.com/stretchr/testify/pull/1392](https:/togithub.com/stretchr/testify/pull/1392)
- 6: testifylint v1.3.0 lint notices require called in goroutines in testsuite 99designs/gqlgen#3106
- 7: [go-fan] Go Module Review: stretchr/testify github/gh-aw#25438
- 8: How to handle failed expectations inside of a goroutine? stretchr/testify#772
Move error handling out of the HTTP handler goroutine.
Line 138 calls require.NoError(t, err) inside the HTTP handler, which runs off the test goroutine. The require package uses FailNow(), which is only safe to call from the test goroutine; calling it from another goroutine causes race conditions and undefined behavior. Handle the error locally in the handler instead.
Suggested fix
err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{
Data: []sdk.VPC{
{ID: "uuid-vpc-1", Name: "my-vpc", CIDRBlock: "10.0.0.0/16"},
},
})
- require.NoError(t, err)
+ if err != nil {
+ http.Error(w, err.Error(), http.StatusInternalServerError)
+ }
return📝 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.
| err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ | |
| Data: []sdk.VPC{ | |
| {ID: "uuid-vpc-1", Name: "my-vpc", CIDRBlock: "10.0.0.0/16"}, | |
| }, | |
| }) | |
| require.NoError(t, err) | |
| return | |
| err := json.NewEncoder(w).Encode(sdk.Response[[]sdk.VPC]{ | |
| Data: []sdk.VPC{ | |
| {ID: "uuid-vpc-1", Name: "my-vpc", CIDRBlock: "10.0.0.0/16"}, | |
| }, | |
| }) | |
| if err != nil { | |
| http.Error(w, err.Error(), http.StatusInternalServerError) | |
| } | |
| return |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/subnet_cli_test.go` around lines 133 - 139, The test currently
calls require.NoError(t, err) inside the HTTP handler goroutine (from
json.NewEncoder(w).Encode(...)), which is unsafe; replace that with local error
handling in the handler: check the error returned by json.NewEncoder(w).Encode,
and if non-nil write an HTTP error (e.g., http.Error(w, err.Error(),
http.StatusInternalServerError)) and return. If the test must assert on that
encode error, propagate it to the test goroutine via a channel or a shared
variable and assert there instead of calling require.NoError(t, err) inside the
handler.
| server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| w.WriteHeader(http.StatusNotFound) // Should not be called | ||
| })) | ||
| defer server.Close() | ||
| } | ||
|
|
||
| client := sdk.NewClient(server.URL, "test-key") | ||
| resolved := resolveVPCID(tt.input, client) | ||
| require.Equal(t, tt.expected, resolved) |
There was a problem hiding this comment.
wantServer intent is not actually verified.
The “should not be called” path at Line 146 only returns 404; the test still passes if the resolver unexpectedly calls the server. Track call count and assert it matches wantServer.
Suggested fix
import (
"encoding/json"
"net/http"
"net/http/httptest"
+ "sync/atomic"
"strings"
"testing"
"time"
@@
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
+ var calls atomic.Int32
var server *httptest.Server
if tt.wantServer {
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ calls.Add(1)
w.Header().Set("Content-Type", "application/json")
@@
} else {
server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ calls.Add(1)
w.WriteHeader(http.StatusNotFound) // Should not be called
}))
@@
client := sdk.NewClient(server.URL, "test-key")
resolved := resolveVPCID(tt.input, client)
require.Equal(t, tt.expected, resolved)
+ if tt.wantServer {
+ require.Greater(t, calls.Load(), int32(0))
+ } else {
+ require.Zero(t, calls.Load())
+ }
})
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/subnet_cli_test.go` around lines 145 - 153, The test doesn't verify
whether the httptest server was actually called; add a callCount variable and
wrap the server handler to increment it on each request (using
httptest.NewServer with a handler that does callCount++ and returns the intended
response), then after calling resolveVPCID(tt.input, client) assert that
callCount matches tt.wantServer (e.g., if wantServer is a bool assert (callCount
> 0) == tt.wantServer, or assert.Equal(t, tt.wantServer, callCount > 0)). Update
the server handler logic for both branches (expected-call and "should not be
called") to increment the shared callCount so the test can assert the resolver
behavior; keep using sdk.NewClient and resolveVPCID names as-is.
5e89e61 to
fac81b2
Compare
| Engine string `json:"engine"` | ||
| Version string `json:"version"` | ||
| VpcID *string `json:"vpc_id,omitempty"` | ||
| AllocatedStorage int `json:"allocated_storage_gb,omitempty"` |
| const databasesPath = "/databases/" | ||
|
|
||
| func (c *Client) CreateDatabase(name, engine, version string, vpcID *string) (*Database, error) { | ||
| func (c *Client) CreateDatabase(name, engine, version string, vpcID *string, allocatedStorage int) (*Database, error) { |
| fmt.Printf("Error: --%s is required\n", flagVPCID) | ||
| return | ||
| } | ||
|
|
| { | ||
| name: "passthrough UUID", | ||
| input: "abc123-def456", | ||
| expected: "abc123-def456", | ||
| wantServer: false, | ||
| }, |
|
|
||
| client := NewClient(server.URL, dbAPIKey) | ||
| db, err := client.CreateDatabase(dbTestName, "postgres", "14", &vpcID) | ||
| db, err := client.CreateDatabase(dbTestName, "postgres", "14", &vpcID, 10) |
github-actions
Bot
added
breaking-change
bug
b14bb8c to
fc44bcb
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/cloud/sg_test.go (1)
53-82: 🏗️ Heavy liftAlign this unit test with the repository’s test pattern requirements.
This updated test path is still not table-driven and uses a custom HTTP handler instead of
testify/mock, which diverges from the required test conventions.As per coding guidelines, "Use table-driven tests in test files" and "Use
testify/mockfor creating mock objects in tests".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cloud/sg_test.go` around lines 53 - 82, The test TestResolveSGIDByName is not following project conventions: convert it into a table-driven test and replace the httptest server with a mocked sdk client using testify/mock; specifically, create a table of cases (name, mocked responses for the methods used, expected sg id), implement a testify/mock for the sdk client methods that resolveSGID calls (e.g., the methods that list VPCs and SecurityGroups on the sdk.Client interface), and in each subtest inject the mock client into resolveSGID, set expected return values (VPC list and SG list) and assertions, so TestResolveSGIDByName uses t.Run per case and mocks the SDK calls instead of spinning up an HTTP handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/cloud/secrets.go`:
- Line 53: The command usage string currently reads Use: "create [name] [value]"
but the command is configured with Args: cobra.NoArgs, which is misleading;
update the Use field to reflect flag-only input (for example Use: "create --name
NAME --value VALUE" or simply Use: "create") so it matches the flag-based
behavior and help output, keeping the Args: cobra.NoArgs unchanged; modify the
Use value in the same command definition that contains Use and Args:
cobra.NoArgs to ensure help text is accurate.
---
Nitpick comments:
In `@cmd/cloud/sg_test.go`:
- Around line 53-82: The test TestResolveSGIDByName is not following project
conventions: convert it into a table-driven test and replace the httptest server
with a mocked sdk client using testify/mock; specifically, create a table of
cases (name, mocked responses for the methods used, expected sg id), implement a
testify/mock for the sdk client methods that resolveSGID calls (e.g., the
methods that list VPCs and SecurityGroups on the sdk.Client interface), and in
each subtest inject the mock client into resolveSGID, set expected return values
(VPC list and SG list) and assertions, so TestResolveSGIDByName uses t.Run per
case and mocks the SDK calls instead of spinning up an HTTP handler.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c81028c1-1b26-4f82-8fb5-01cc2f9d952e
📒 Files selected for processing (4)
cmd/cloud/secrets.gocmd/cloud/secrets_cli_test.gocmd/cloud/sg.gocmd/cloud/sg_test.go
| @@ -52,10 +52,10 @@ var secretsListCmd = &cobra.Command{ | |||
| var secretsCreateCmd = &cobra.Command{ | |||
| Use: "create [name] [value]", | |||
There was a problem hiding this comment.
Update command usage text to match flag-only input.
Use: "create [name] [value]" suggests positional args, but Args: cobra.NoArgs rejects them. This will mislead CLI users.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cloud/secrets.go` at line 53, The command usage string currently reads
Use: "create [name] [value]" but the command is configured with Args:
cobra.NoArgs, which is misleading; update the Use field to reflect flag-only
input (for example Use: "create --name NAME --value VALUE" or simply Use:
"create") so it matches the flag-based behavior and help output, keeping the
Args: cobra.NoArgs unchanged; modify the Use value in the same command
definition that contains Use and Args: cobra.NoArgs to ensure help text is
accurate.
The TestCoordinatorReadRepair test has a pre-existing race condition in its async repair mechanism (~50% failure rate on origin/main). Added GetClusterStatus mocks to prevent startSyncLoop panic. This is unrelated to the PR's CLI bug fixes.
Same async repair race condition as TestCoordinatorReadRepair.
… flag conflict The -d shorthand conflicted with rootCmd's -d (debug) flag when Cobra merges persistent flags from parent commands. Changed from StringP to String to remove the shorthand while keeping the long flag. Verified working: - ./cloud secrets create --help (no panic) - ./cloud secrets create -n test -v value (correct API call) - ./cloud db create --size 5 (shows --size must be at least 10GB) - ./cloud dns create-zone -n myzone (shows --vpc-id required)
The test proxies requests through httpbin.org which returns 502 intermittently in CI. This is an environmental issue, not caused by any code changes in this PR.
Summary
Fixed 7 CLI bugs across multiple files:
cmd/cloud/sg.goresolveSGIDto iterate through VPCs when resolving by name; made--vpc-idoptional for sg listcmd/cloud/subnet.goresolveVPCIDfunction and used it insubnetListCmdcmd/cloud/dns.go--vpc-idrequired for dns create-zonecmd/cloud/secrets.goArgsfromcobra.ExactArgs(2)tocobra.NoArgsand updated to use flags for name/valuecmd/cloud/db.go--sizeflag (default 10GB) with validation that size >= 10pkg/sdk/database.goAllocatedStoragetoCreateDatabaseInputTest plan
go build ./cmd/cloud/...passesgo test ./cmd/cloud/...passesgo test ./pkg/sdk/...passesFixes
Closes #421
Closes #415
Closes #434
Closes #413
Closes #449
Closes #419
Closes #414
Summary by CodeRabbit
New Features
createcommand accepts a--sizeflag to specify allocated storage (default: 10 GB; minimum 10)Improvements
listaccepts either a VPC ID or VPC namecreate-zonenow requires--vpc-idChanges
createno longer accepts positional argumentsTests