fix(promlib): add lenient type coercion to ParsePromOptions for API/Terraform stored fields [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] - #303
Conversation
…erraform stored fields
Signed commits report1 of 1 commit between
This repository requires all commits to be signed. See GitHub docs on commit signature verification. |
|
|
| // for a string field). The UI always writes normalized types, and the | ||
| // previous schemaless reader accepted both. | ||
| raw := make(map[string]any) | ||
| if err2 := json.Unmarshal(data, &raw); err2 != nil { |
There was a problem hiding this comment.
| if err2 := json.Unmarshal(data, &raw); err2 != nil { | |
| if err = json.Unmarshal(data, &raw); err != nil { |
| if v, ok := raw[key]; ok { | ||
| switch val := v.(type) { | ||
| case string: | ||
| raw[key] = val == "true" || val == "1" || val == "yes" |
There was a problem hiding this comment.
I don't think we should accept yes, no, 0, or 1, as boolean values. Modern YAML is strict and only accepts true or false.
| "incrementalQuerying", "disableRecordingRules", | ||
| "oauthPassThru", "seriesEndpoint", "disableGrafanaCache", | ||
| } { | ||
| if v, ok := raw[key]; ok { |
There was a problem hiding this comment.
If a value cannot be coerced we should return a relevant error.
| "alertmanagerUid", "authType", "defaultRegion", "profile", | ||
| } { | ||
| if v, ok := raw[key]; ok { | ||
| if num, ok := v.(float64); ok { |
There was a problem hiding this comment.
If the coercion is not ok we should return an error.
| } | ||
|
|
||
| // *int64 fields: accept string numbers and float64. | ||
| if v, ok := raw["seriesLimit"]; ok { |
| if v, ok := raw["seriesLimit"]; ok { | ||
| switch val := v.(type) { | ||
| case string: | ||
| if n, err := strconv.ParseInt(val, 10, 64); err == nil { |
There was a problem hiding this comment.
The error here should be returned.
aangelisc
left a comment
aangelisc
left a comment
There was a problem hiding this comment.
Left some comments, could you also add tests please 😊
2 participants
Description
Fixes grafana/grafana#130119
Since promlib v0.0.13,
ParsePromOptions()uses strict JSON unmarshaling. Datasources provisioned through the API or Terraform can store values with off-spec types (e.g., string"true"for a bool field, number15for a string field, or a single object instead of a one-element array). When that happens, instance creation fails and every query/resource call against that datasource errors immediately.This PR adds a lenient fallback with type coercion:
"true"/"false"/"1"/"0"/"yes"/"no"and number 0/1"timeInterval": 15)The coercion only runs when strict unmarshal fails, so it has zero overhead for datasources with correctly typed fields.
Verification
manageAlerts: "true"(string→bool) → no longer failsseriesLimit: "100"(string→*int64) → no longer failstimeInterval: 15(number→string) → no longer failsexemplarTraceIdDestinations: {...}(object→array) → no longer fails