From 5bf95534ad9643b35ebf35a307f6fd92bbf6d926 Mon Sep 17 00:00:00 2001 From: Naman Verma Date: Sat, 25 Jul 2026 18:31:34 +0530 Subject: [PATCH 1/2] fix: reencode svg images to base64 when migrating dashboards (#12274) --- .../dashboardtypes/perses_v1_to_v2_icons.go | 35 ++++++++++ .../dashboardtypes/perses_v1_to_v2_test.go | 65 +++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_icons.go b/pkg/types/dashboardtypes/perses_v1_to_v2_icons.go index 3aaf5963bee..df0cf019d69 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_icons.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_icons.go @@ -1,5 +1,11 @@ package dashboardtypes +import ( + "encoding/base64" + "net/url" + "strings" +) + // v1 dashboards stored their `image` as one of a fixed set of base64 icon data // URIs. v2 references the same icons by a stable asset path (dashboardIconPathPrefix // + name). This maps each preset v1 base64 blob to its v2 icon path so a migrated @@ -45,8 +51,37 @@ func resolveV1Image(image string) string { if path, ok := v1Base64IconToAssetPath[image]; ok { return path } + if reencoded, ok := reencodeInlineSVGImage(image); ok { + image = reencoded + } if (DashboardV2MetadataBase{Image: image}).validateImage() == nil { return image } return defaultDashboardIconPath } + +// reencodeInlineSVGImage rewrites a percent-encoded inline SVG data URI into the +// base64 form the v2 schema accepts, preserving an icon that would otherwise fail +// validation. Returns (image, false) unchanged for anything else. +func reencodeInlineSVGImage(image string) (string, bool) { + const mediaType = "data:image/svg+xml" + if !strings.HasPrefix(image, mediaType) { + return image, false + } + rest := image[len(mediaType):] + // The media type must be followed by its params/data separator, not more type + // text (guards against a longer type that merely shares this prefix). + if len(rest) == 0 || (rest[0] != ',' && rest[0] != ';') { + return image, false + } + params, encoded, found := strings.Cut(rest, ",") + // A base64 URI carries ";base64" before the comma and is already valid — leave it. + if !found || strings.Contains(params, "base64") { + return image, false + } + svg, err := url.PathUnescape(encoded) + if err != nil { + return image, false // malformed escaping — leave it; conversion falls back to the default icon + } + return "data:image/svg+xml;base64," + base64.StdEncoding.EncodeToString([]byte(svg)), true +} diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_test.go b/pkg/types/dashboardtypes/perses_v1_to_v2_test.go index e9ab2a31123..872b5c5f94d 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_test.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_test.go @@ -429,6 +429,11 @@ func TestResolveV1Image(t *testing.T) { assert.Equal(t, passthrough, resolveV1Image(passthrough)) } + // A percent-encoded inline SVG icon is preserved by re-encoding it to base64 + // (the form v2 accepts) rather than being dropped to the default. + svgResolved := resolveV1Image("data:image/svg+xml,%3Csvg%2F%3E") + assert.Equal(t, "data:image/svg+xml;base64,PHN2Zy8+", svgResolved) + // A value that v2 would reject (a URL, a corrupt data URI) falls back to the // default eight-ball icon rather than failing the dashboard migration. for _, invalid := range []string{ @@ -440,6 +445,66 @@ func TestResolveV1Image(t *testing.T) { } } +func TestReencodeInlineSVGImage(t *testing.T) { + testCases := []struct { + scenario string + image string + wantRewritten bool + want string + }{ + { + scenario: "percent-encoded inline svg", + image: "data:image/svg+xml,%3Csvg%2F%3E", + wantRewritten: true, + want: "data:image/svg+xml;base64,PHN2Zy8+", + }, + { + scenario: "percent-encoded inline svg with charset param", + image: "data:image/svg+xml;charset=utf-8,%3Csvg%2F%3E", + wantRewritten: true, + want: "data:image/svg+xml;base64,PHN2Zy8+", + }, + { + scenario: "already base64 svg is left unchanged", + image: "data:image/svg+xml;base64,PHN2Zy8+", + wantRewritten: false, + want: "data:image/svg+xml;base64,PHN2Zy8+", + }, + { + scenario: "non-svg data uri is left unchanged", + image: "data:image/png,%89PNG", + wantRewritten: false, + want: "data:image/png,%89PNG", + }, + { + scenario: "an icon path is left unchanged", + image: "/assets/Icons/eight-ball", + wantRewritten: false, + want: "/assets/Icons/eight-ball", + }, + { + scenario: "a longer media type merely sharing the svg prefix is left unchanged", + image: "data:image/svg+xmlish,%3Csvg%2F%3E", + wantRewritten: false, + want: "data:image/svg+xmlish,%3Csvg%2F%3E", + }, + { + scenario: "empty is left unchanged", + image: "", + wantRewritten: false, + want: "", + }, + } + + for _, testCase := range testCases { + t.Run(testCase.scenario, func(t *testing.T) { + got, rewritten := reencodeInlineSVGImage(testCase.image) + assert.Equal(t, testCase.wantRewritten, rewritten) + assert.Equal(t, testCase.want, got) + }) + } +} + func TestConvertV1ToV2ReplacesInvalidImage(t *testing.T) { // An image v2 would reject must not sink the whole migration — it falls back // to the default icon so the dashboard still converts. From e728942fec39378a2afcec67af7b92124ba24c2d Mon Sep 17 00:00:00 2001 From: Naman Verma Date: Sat, 25 Jul 2026 20:27:29 +0530 Subject: [PATCH 2/2] fix: handle malformed reduceTo + pick correct columns for traces in list panel (#12275) --- .../dashboardtypes/perses_v1_to_v2_panels.go | 20 ++++++++- .../perses_v1_to_v2_queries_malformed.go | 29 ++++++++++++- .../dashboardtypes/perses_v1_to_v2_test.go | 42 +++++++++++++++++++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_panels.go b/pkg/types/dashboardtypes/perses_v1_to_v2_panels.go index aa1fe41c04d..b5123e4e1e9 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_panels.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_panels.go @@ -266,11 +266,27 @@ func (d *v1Decoder) legendFromWidget(w map[string]any) Legend { } } +// widgetBuilderSignal returns the signal of the widget's first builder query. Empty +// for non-builder (promql/clickhouse) widgets, which have no per-signal selected fields. +func (d *v1Decoder) widgetBuilderSignal(w map[string]any) telemetrytypes.Signal { + builder := d.readObject(d.readObject(w, "query"), "builder") + for _, q := range d.readObjects(builder, "queryData") { + return signalFromDataSource(q["dataSource"]) + } + return telemetrytypes.Signal{} +} + func (d *v1Decoder) mapV1SelectFields(w map[string]any) []telemetrytypes.TelemetryFieldKey { - field := "selectedLogFields" + // Read the array matching the query's signal: traces and logs each keep their own + // selected columns, and a widget can carry a stale array for the other signal. + primary, fallback := "selectedLogFields", "selectedTracesFields" + if d.widgetBuilderSignal(w) == telemetrytypes.SignalTraces { + primary, fallback = "selectedTracesFields", "selectedLogFields" + } + field := primary raw := d.readArray(w, field) if len(raw) == 0 { - field = "selectedTracesFields" + field = fallback raw = d.readArray(w, field) } if len(raw) == 0 { diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go b/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go index d96374ddf16..fa1817d633f 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_queries_malformed.go @@ -33,6 +33,7 @@ var preV5Migrator = transition.NewDashboardMigrateV5(slog.New(slog.DiscardHandle func normalizePreV5QueryData(query map[string]any, widgetType string) { dropLegacyFilter(query) normalizeFilterItemOps(query) + foldReduceToIntoMetricAggregations(query) preV5Migrator.MigrateQueryDataShapeSafe(context.Background(), query, widgetType) normalizePreV5LogTraceAggregations(query) normalizeMetricAggregations(query) @@ -226,7 +227,7 @@ func dropLegacyFilter(query map[string]any) { // normalizeFilterItemOps lowercases exists/nexists filter ops (frontend stores them // uppercase) to the spelling transition's buildCondition (pkg/transition/migrate_common.go) -// matches; otherwise it appends a spurious empty value ("svc EXISTS ''"). Value +// matches; otherwise it appends a spurious empty value ("svc EXISTS ”"). Value // operators already round-trip via that switch's default case. func normalizeFilterItemOps(query map[string]any) { filters, ok := query["filters"].(map[string]any) @@ -303,6 +304,32 @@ func normalizeMetricAggregations(query map[string]any) { } } +// foldReduceToIntoMetricAggregations moves a metric query's top-level reduceTo onto +// its existing aggregations[] (where v5 wants it), which the shared migrator only does +// when building an aggregation from flat fields. Runs before it so the value survives. +func foldReduceToIntoMetricAggregations(query map[string]any) { + if signalFromDataSource(query["dataSource"]) != telemetrytypes.SignalMetrics { + return + } + reduceTo, ok := query["reduceTo"].(string) + if !ok || reduceTo == "" { + return + } + aggs, ok := query["aggregations"].([]any) + if !ok || len(aggs) == 0 { + return + } + for _, a := range aggs { + agg, ok := a.(map[string]any) + if !ok { + continue + } + if _, exists := agg["reduceTo"]; !exists { + agg["reduceTo"] = reduceTo + } + } +} + // normalizePreV5LogTraceAggregations reshapes an existing logs/traces aggregations[] // via parseAggregations (extract func(args), lift inline "as alias", split // multi-part, drop metric-only fields; empty → count()). Covers the case the diff --git a/pkg/types/dashboardtypes/perses_v1_to_v2_test.go b/pkg/types/dashboardtypes/perses_v1_to_v2_test.go index 872b5c5f94d..9067d2369fd 100644 --- a/pkg/types/dashboardtypes/perses_v1_to_v2_test.go +++ b/pkg/types/dashboardtypes/perses_v1_to_v2_test.go @@ -524,6 +524,48 @@ func TestConvertV1ToV2ReplacesInvalidImage(t *testing.T) { assert.Equal(t, "/assets/Icons/eight-ball", dashboard.Image) } +func TestFoldReduceToIntoMetricAggregations(t *testing.T) { + // A metric query's top-level reduceTo lands on its aggregation (where v5 wants it). + query := map[string]any{ + "dataSource": "metrics", + "reduceTo": "sum", + "aggregations": []any{map[string]any{"metricName": "m", "spaceAggregation": "sum"}}, + } + foldReduceToIntoMetricAggregations(query) + assert.Equal(t, "sum", query["aggregations"].([]any)[0].(map[string]any)["reduceTo"]) + + // An aggregation that already carries a reduceTo is not overwritten. + query = map[string]any{ + "dataSource": "metrics", + "reduceTo": "sum", + "aggregations": []any{map[string]any{"metricName": "m", "reduceTo": "avg"}}, + } + foldReduceToIntoMetricAggregations(query) + assert.Equal(t, "avg", query["aggregations"].([]any)[0].(map[string]any)["reduceTo"]) +} + +func TestMapV1SelectFieldsPicksBySignal(t *testing.T) { + d := &v1Decoder{} + // A list widget carrying both arrays; the query's signal must decide which wins. + widget := func(dataSource string) map[string]any { + return map[string]any{ + "query": map[string]any{ + "builder": map[string]any{"queryData": []any{map[string]any{"dataSource": dataSource}}}, + }, + "selectedLogFields": []any{map[string]any{"name": "log_field"}}, + "selectedTracesFields": []any{map[string]any{"name": "trace_field"}}, + } + } + + logs := d.mapV1SelectFields(widget("logs")) + require.Len(t, logs, 1) + assert.Equal(t, "log_field", logs[0].Name) + + traces := d.mapV1SelectFields(widget("traces")) + require.Len(t, traces, 1) + assert.Equal(t, "trace_field", traces[0].Name) +} + func TestConvertV1ToV2RejectsAlreadyV2(t *testing.T) { storable := &StorableDashboard{ Identifiable: types.Identifiable{ID: valuer.GenerateUUID()},