diff --git a/pkg/services/dashboardversion/dashverimpl/dashver.go b/pkg/services/dashboardversion/dashverimpl/dashver.go index 11c440b3352..c36288073b3 100644 --- a/pkg/services/dashboardversion/dashverimpl/dashver.go +++ b/pkg/services/dashboardversion/dashverimpl/dashver.go @@ -226,11 +226,13 @@ func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboa // generation id in unified storage, so we cannot query for the dashboard version directly, and we cannot use search as history is not indexed. // use batches to make sure we don't load too much data at once. const batchSize = 50 - labelSelector := utils.LabelKeyGetHistory + "=" + dashboardUID + labelSelector := utils.LabelKeyGetHistory + "=true" + fieldSelector := "metadata.name=" + dashboardUID var continueToken string for { out, err := s.k8sclient.List(ctx, orgID, v1.ListOptions{ LabelSelector: labelSelector, + FieldSelector: fieldSelector, Limit: int64(batchSize), Continue: continueToken, }) @@ -260,8 +262,11 @@ func (s *Service) getHistoryThroughK8s(ctx context.Context, orgID int64, dashboa } func (s *Service) listHistoryThroughK8s(ctx context.Context, orgID int64, dashboardUID string, limit int64, continueToken string) (*dashver.DashboardVersionResponse, error) { + labelSelector := utils.LabelKeyGetHistory + "=true" + fieldSelector := "metadata.name=" + dashboardUID out, err := s.k8sclient.List(ctx, orgID, v1.ListOptions{ - LabelSelector: utils.LabelKeyGetHistory + "=" + dashboardUID, + LabelSelector: labelSelector, + FieldSelector: fieldSelector, Limit: limit, Continue: continueToken, }) diff --git a/pkg/storage/unified/apistore/util.go b/pkg/storage/unified/apistore/util.go index 6e8bebf0b28..d3763f652a6 100644 --- a/pkg/storage/unified/apistore/util.go +++ b/pkg/storage/unified/apistore/util.go @@ -70,9 +70,6 @@ func toListRequest(k *resourcepb.ResourceKey, opts storage.ListOptions) (*resour if len(requirements) != 1 { return nil, predicate, apierrors.NewBadRequest("single label supported with: " + v) } - if opts.Predicate.Field != nil && !opts.Predicate.Field.Empty() { - return nil, predicate, apierrors.NewBadRequest("field selector not supported with: " + v) - } if r.Operator() != selection.Equals { return nil, predicate, apierrors.NewBadRequest("only = operator supported with: " + v) } @@ -90,7 +87,21 @@ func toListRequest(k *resourcepb.ResourceKey, opts storage.ListOptions) (*resour } case utils.LabelKeyGetHistory: req.Source = resourcepb.ListRequest_HISTORY - req.Options.Key.Name = vals[0] + if opts.Predicate.Field == nil || opts.Predicate.Field.Empty() { + return nil, predicate, apierrors.NewBadRequest("metadata.name field selector required for history requests") + } + + fieldRequirements := opts.Predicate.Field.Requirements() + if len(fieldRequirements) != 1 { + return nil, predicate, apierrors.NewBadRequest("only one field selector supported for history requests") + } + + fieldReq := fieldRequirements[0] + if fieldReq.Field != "metadata.name" { + return nil, predicate, apierrors.NewBadRequest("metadata.name field selector required for history requests") + } + + req.Options.Key.Name = fieldReq.Value } req.Options.Labels = nil diff --git a/pkg/storage/unified/apistore/util_test.go b/pkg/storage/unified/apistore/util_test.go index 222dc1cab69..0bee8adf75e 100644 --- a/pkg/storage/unified/apistore/util_test.go +++ b/pkg/storage/unified/apistore/util_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/selection" "k8s.io/apiserver/pkg/storage" @@ -153,7 +154,8 @@ func TestToListRequest(t *testing.T) { }, opts: storage.ListOptions{ Predicate: storage.SelectionPredicate{ - Label: labels.SelectorFromSet(labels.Set{utils.LabelKeyGetHistory: "test-name"}), + Label: labels.SelectorFromSet(labels.Set{utils.LabelKeyGetHistory: "true"}), + Field: fields.SelectorFromSet(fields.Set{"metadata.name": "test-name"}), }, }, want: &resourcepb.ListRequest{ diff --git a/pkg/tests/apis/dashboard/integration/api_validation_test.go b/pkg/tests/apis/dashboard/integration/api_validation_test.go index 8b0e1c1cbb1..5c808c7df64 100644 --- a/pkg/tests/apis/dashboard/integration/api_validation_test.go +++ b/pkg/tests/apis/dashboard/integration/api_validation_test.go @@ -431,6 +431,31 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) { err = adminClient.Resource.Delete(context.Background(), dashUID, v1.DeleteOptions{}) require.NoError(t, err) }) + + t.Run("dashboard version history available, even for UIDs ending in hyphen", func(t *testing.T) { + dashboardUID := "test-dashboard-" + dash, err := createDashboard(t, adminClient, "Dashboard with uid ending in hyphen", nil, &dashboardUID) + require.NoError(t, err) + + updatedDash, err := updateDashboard(t, adminClient, dash, "Updated dashboard with uid ending in hyphen", nil) + require.NoError(t, err) + require.NotNil(t, updatedDash) + + labelSelector := utils.LabelKeyGetHistory + "=true" + fieldSelector := "metadata.name=" + dashboardUID + versions, err := adminClient.Resource.List(context.Background(), v1.ListOptions{ + LabelSelector: labelSelector, + FieldSelector: fieldSelector, + Limit: 10, + }) + require.NoError(t, err) + require.NotNil(t, versions) + // one from initial save, one from update + require.Equal(t, len(versions.Items), 2) + + err = adminClient.Resource.Delete(context.Background(), dashboardUID, v1.DeleteOptions{}) + require.NoError(t, err) + }) }) t.Run("Dashboard provisioning validations", func(t *testing.T) {