From d3468105d760ae0bc70389f8aa12aade21aed903 Mon Sep 17 00:00:00 2001 From: Marco de Abreu Date: Tue, 25 Mar 2025 15:28:24 +0100 Subject: [PATCH] Storage: Fix resource history sorting and pagination in Spanner backend (#102777) * Align spanner ListHistory implementation with sql one * Move migration logic * Move migration code --------- Co-authored-by: Marco de Abreu <18629099+marcoabreu@users.noreply.github.com> --- pkg/storage/unified/resource/migrations.go | 30 ++++++++++++++++++++++ pkg/storage/unified/sql/backend.go | 22 +++------------- 2 files changed, 34 insertions(+), 18 deletions(-) create mode 100644 pkg/storage/unified/resource/migrations.go diff --git a/pkg/storage/unified/resource/migrations.go b/pkg/storage/unified/resource/migrations.go new file mode 100644 index 00000000000..509cd3b8249 --- /dev/null +++ b/pkg/storage/unified/resource/migrations.go @@ -0,0 +1,30 @@ +package resource + +import ( + "fmt" + + "github.com/grafana/grafana/pkg/infra/log" +) + +// MigrateVersionMatch handles backwards compatibility for ResourceVersionMatch +// by migrating from the deprecated version_match to version_match_v2. +// It returns an error if the version match is unknown. +func MigrateListRequestVersionMatch(req *ListRequest, logger log.Logger) error { + if req.VersionMatch != nil && req.GetVersionMatchV2() == ResourceVersionMatchV2_UNKNOWN { + switch req.GetVersionMatch() { + case ResourceVersionMatch_DEPRECATED_NotOlderThan: + // This is not a typo. The old implementation actually did behave like Unset. + req.VersionMatchV2 = ResourceVersionMatchV2_Unset + case ResourceVersionMatch_DEPRECATED_Exact: + req.VersionMatchV2 = ResourceVersionMatchV2_Exact + default: + return fmt.Errorf("unknown version match: %v", req.GetVersionMatch()) + } + + // Log the migration to measure whether we have successfully migrated all clients + logger.Info("Old client request received, migrating from version_match to version_match_v2", + "oldValue", req.GetVersionMatch(), + "newValue", req.GetVersionMatchV2()) + } + return nil +} diff --git a/pkg/storage/unified/sql/backend.go b/pkg/storage/unified/sql/backend.go index d675998f97a..f3944720c00 100644 --- a/pkg/storage/unified/sql/backend.go +++ b/pkg/storage/unified/sql/backend.go @@ -525,6 +525,10 @@ func (b *backend) ListIterator(ctx context.Context, req *resource.ListRequest, c ctx, span := b.tracer.Start(ctx, tracePrefix+"List") defer span.End() + if err := resource.MigrateListRequestVersionMatch(req, b.log); err != nil { + return 0, err + } + if req.Options == nil || req.Options.Key.Group == "" || req.Options.Key.Resource == "" { return 0, fmt.Errorf("missing group or resource") } @@ -710,24 +714,6 @@ func (b *backend) listAtRevision(ctx context.Context, req *resource.ListRequest, // getHistory fetches the resources from the resource table. func (b *backend) getHistory(ctx context.Context, req *resource.ListRequest, cb func(resource.ListIterator) error) (int64, error) { - // Backwards compatibility for ResourceVersionMatch - if req.VersionMatch != nil && req.GetVersionMatchV2() == resource.ResourceVersionMatchV2_UNKNOWN { - switch req.GetVersionMatch() { - case resource.ResourceVersionMatch_DEPRECATED_NotOlderThan: - // This is not a typo. The old implementation actually did behave like Unset. - req.VersionMatchV2 = resource.ResourceVersionMatchV2_Unset - case resource.ResourceVersionMatch_DEPRECATED_Exact: - req.VersionMatchV2 = resource.ResourceVersionMatchV2_Exact - default: - return 0, fmt.Errorf("unknown version match: %v", req.GetVersionMatch()) - } - - // Log the migration for debugging purposes - b.log.Debug("Old client request received, migrating from version_match to version_match_v2", - "oldValue", req.GetVersionMatch(), - "newValue", req.GetVersionMatchV2()) - } - listReq := sqlGetHistoryRequest{ SQLTemplate: sqltemplate.New(b.dialect), Key: req.Options.Key,