fix: prevent data source filter panics on unknown or non-string fields - #344
Open
nagaboinaramgopal wants to merge 2 commits into
Open
fix: prevent data source filter panics on unknown or non-string fields#344nagaboinaramgopal wants to merge 2 commits into
nagaboinaramgopal wants to merge 2 commits into
Conversation
The data source filter helpers indexed the resource JSON map and asserted the
result to string: <json>[updatedName].(string). This panics when a filter name
matches no field (the lookup returns nil) or when the field is not a string
(numbers unmarshal to float64, booleans to bool). Format the value with
fmt.Sprintf("%v", ...) instead, matching applyIpAddressFilters. Covers the
instance, network_offering, physical_network, service_offering, ssh_keypair,
template, user, volume, vpc, vpn_connection and zone data sources. Adds a unit
test.
Signed-off-by: Ramgopal Nagaboina <ramgopal.nagaboina.dev@gmail.com>
fmt.Sprintf("%v", nil) yields the literal "<nil>", which a permissive regex such as ".*" would match, so a filter on a field that does not exist could match. Look the field up explicitly and return no match when it is absent or nil, in every data source filter (including ipaddress and vpc_offering, which already used fmt.Sprintf). Extend the unit test to cover a permissive regex on a missing field and a positive match.
nagaboinaramgopal
force-pushed
the
fix/datasource-filter-panic
branch
from
September 7, 2026 20:12
2a7d799 to
d5f1178
Compare
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.
Description
Several data sources match their filters against the looked-up JSON field, and there were two problems with the JSON-backed implementations.
They asserted the value as a string, for example
volumeJSON[name].(string). A filter on a field that does not exist getsnil, and one on a numeric field gets afloat64, so the assertion panics the provider withinterface conversion: interface {} is nil, not string.Stringifying with
fmt.Sprintf("%v", ...)alone is still wrong for a missing field, sincefmt.Sprintf("%v", nil)is the literal<nil>, which a permissive regex such as.*would match, so a filter on a nonexistent field could match.Fixed both across the affected JSON-backed data source filters: look the field up explicitly, return no match when it is absent or nil, and otherwise stringify with
fmt.Sprintf("%v", ...)so numeric and bool fields match safely. The reflection based (pod, cluster) and custom (role, project) filter implementations work differently and are not touched here.Testing
Added a unit test on the volume data source covering a filter on an unknown field, a permissive regex on a missing field, a numeric field matched against its string form, and a positive match. It needs no live CloudStack:
The unknown and numeric cases panic against the original code, and the missing-field case matches
<nil>; all pass with the fix.