Skip to content

Fix list-response JSON tags + guard getRawValue against empty arrays - #154

Merged
abh1sar merged 2 commits into
apache:mainfrom
jmsperu:fix/list-response-json-tags
Aug 5, 2026
Merged

Fix list-response JSON tags + guard getRawValue against empty arrays#154
abh1sar merged 2 commits into
apache:mainfrom
jmsperu:fix/list-response-json-tags

Conversation

@jmsperu

@jmsperu jmsperu commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Fixes several list-response robustness bugs in the same family as #135/#136 (PR #137).

1. List wrapper JSON-tag mismatches (silent data loss)

The generator derives the list wrapper tag from parseSingular(), which does not always match the key CloudStack returns. When it does not, Count unmarshals but the slice stays nil — the caller silently gets an empty list, and the Get<X>ByID helper then panics on slice[0]. I verified each against the CloudStack source object names (setObjectName(...)):

API current tag correct key (source)
listHypervisorCapabilities hypervisorcapability hypervisorCapabilities
listGuestNetworkIpv6Prefixes guestnetworkipv6prefixe guestnetworkipv6prefix
listLBHealthCheckPolicies lbhealthcheckpolicy healthcheckpolicies
listLBStickinessPolicies lbstickinesspolicy stickinesspolicies

Fixed via explicit cases in the generator override switch (right where the metrics cases from #135/#136 already live) plus the regenerated tags. The inner healthcheckpolicy / stickinesspolicy tags are correct and left untouched.

(I also checked the other -ies/-es siblings — autoscalepolicy, backuprepository, oscategory, snapshotpolicy, webhookdelivery — and they match the source, so they are not touched here.)

2. getRawValue() panic on empty count-wrapped array

In the count-wrapped branch, getRawValue did return resp[0], nil with no length check, so a {"count":0,"<entity>":[]} body panics with index-out-of-range. Now returns a descriptive error. Latent/defensive (only a malformed/edge-case body reaches it), not a live crash.

Tests

  • test/ListResponseJSONTagsRegression_test.go — unmarshals each of the 4 responses under the correct key; fails on the old tags (nil slice), passes with the fix.
  • cloudstack/GetRawValueGuard_test.gopanics without the guard (reproduces the index-out-of-range), passes with it.

Verification

  • go build ./... clean; both regression tests pass with the fix and fail/panic when reverted.

Note (follow-up, not in this PR)

The root cause is that wrapper keys are fabricated from parseSingular() rather than sourced from the API’s declared object name. A systemic fix would source the key from listApis.json / a curated override map. Happy to open a separate issue for that.

Several List<X>Response structs use a json tag that does not match the key
CloudStack actually returns (the generator derives it from parseSingular()),
so Count parses but the slice stays nil — silent data loss. Verified against
the CloudStack source object names (setObjectName):

- listHypervisorCapabilities: hypervisorcapability -> hypervisorCapabilities
- listGuestNetworkIpv6Prefixes: guestnetworkipv6prefixe -> guestnetworkipv6prefix
- listLBHealthCheckPolicies: lbhealthcheckpolicy -> healthcheckpolicies
- listLBStickinessPolicies: lbstickinesspolicy -> stickinesspolicies

Fixed via explicit cases in the generator's override switch (alongside the
existing metrics cases) and the regenerated tags.

Also guards getRawValue() against a count-wrapped response carrying an empty
data array, which previously panicked on resp[0] (index out of range); it now
returns a descriptive error.

Adds regression tests for both.

Signed-off-by: James Peru <james@xcobean.com>

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

LGTM

@abh1sar
abh1sar merged commit 340f0b1 into apache:main Aug 5, 2026
3 checks passed
abh1sar added a commit that referenced this pull request Aug 11, 2026
CloudStack keys a list response's items by the response object name, not by the
API name the generator derived the tag from, so 16 list APIs decoded as a
permanently empty slice while count parsed from the same body and no error was
returned. The keys are now data in a documented listResponseKeys map rather than
24 hand-written switch arms, each one observed on a real server and pinned by a
test, leaving only genuine shape differences in the switch; listCaCertificate and
listUsageServerMetrics also become pointers, since they return a single object
with no count. Also moves the getRawValue empty-array guard from #154 into the
generator, where it had been edited into the generated file only and so was
reverted by every `make code`.
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