Skip to content

Fix panic when serializing UUID-typed request parameters - #160

Merged
abh1sar merged 1 commit into
mainfrom
fix-uuid-request-param-serialization
Aug 5, 2026
Merged

Fix panic when serializing UUID-typed request parameters#160
abh1sar merged 1 commit into
mainfrom
fix-uuid-request-param-serialization

Conversation

@abh1sar

@abh1sar abh1sar commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

generateConvertCode in the generator treats the generated UUID Go type the same as string when building toURLValues(), emitting v.(string) for a value that is actually stored as cloudstack.UUID. Since Go type assertions require an exact dynamic type match, this panics at runtime for any parameter whose Go type is UUID.

Currently managementserverid is the only parameter routed through the UUID type (via the longToStringConvertedParams map), and it's used on listAsyncJobs, listHosts, listHostsMetrics, triggerShutdown, cancelShutdown, prepareForShutdown, listWebhookDeliveries, and deleteWebhookDelivery. This has been dormant so far because managementserverid is optional on all of these commands, so the setter is never exercised by the standard generated tests, but it panics on every real call that actually sets it.

Fix: assert the value as UUID and convert to string, instead of asserting it as string:

case "string":
    pn("u.Set(\"%s\", v.(string))", name)
case "UUID":
    pn("u.Set(\"%s\", string(v.(UUID)))", name)

Regenerated the affected service files (AsyncjobService.go, HostService.go, ManagementService.go, WebhookService.go); no other diff.

Testing

Tested against a real CloudStack installation, calling listHosts, listHostsMetrics, and listAsyncJobs with managementserverid set (the three read-only commands among the affected list; skipped triggerShutdown/cancelShutdown/prepareForShutdown since those actually change the management server's shutdown state).

Before:

=== listHosts (with managementserverid filter) - EXPECTING PANIC ===
panic: interface conversion: interface {} is cloudstack.UUID, not string

github.com/apache/cloudstack-go/v2/cloudstack.(*ListHostsParams).toURLValues(...)
	cloudstack/HostService.go:2845
github.com/apache/cloudstack-go/v2/cloudstack.(*HostService).ListHosts(...)
	cloudstack/HostService.go:3402

After:

=== listHosts (with managementserverid filter) ===
  ok, count=4
=== listHostsMetrics (with managementserverid filter) ===
  ok, count=2
=== listAsyncJobs (with managementserverid filter) ===
  ok, count=0
All calls succeeded, no panic.

generateConvertCode treated the generated "UUID" Go type the same as
"string" when building toURLValues(), emitting v.(string) for a value
that is actually stored as cloudstack.UUID. That type assertion panics
at runtime since Go requires an exact dynamic type match.

Affects managementserverid, currently the only parameter routed
through the UUID type, on listAsyncJobs, listHosts, listHostsMetrics,
triggerShutdown, cancelShutdown, prepareForShutdown,
listWebhookDeliveries, and deleteWebhookDelivery. It was previously
dormant since the parameter is optional on all of them and the setter
was never exercised by the standard generated tests.

Assert the value as UUID and convert to string, instead of asserting
it as string.
@abh1sar
abh1sar requested review from Pearl1594 and sureshanaparti and a lite review from Copilot August 4, 2026 16:03

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.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes a runtime panic when serializing UUID-typed request parameters by generating correct toURLValues() conversion code for UUID values.

Changes:

  • Update generator to treat UUID distinctly from string when emitting URL value conversion code.
  • Regenerate affected service files so managementserverid is serialized via string(v.(UUID)).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
generate/generate.go Fixes generator logic to avoid invalid string type assertion for UUID.
cloudstack/WebhookService.go Regenerated: serializes managementserverid as UUID -> string.
cloudstack/ManagementService.go Regenerated: serializes managementserverid as UUID -> string.
cloudstack/HostService.go Regenerated: serializes managementserverid as UUID -> string.
cloudstack/AsyncjobService.go Regenerated: serializes managementserverid as UUID -> string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cloudstack/ManagementService.go
Comment thread generate/generate.go

@sureshanaparti sureshanaparti 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.

clgtm

@abh1sar
abh1sar merged commit e2d3072 into main Aug 5, 2026
4 checks passed
@abh1sar
abh1sar deleted the fix-uuid-request-param-serialization branch August 5, 2026 15:25
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.

3 participants