Dashboards: Fix history list for dashboard uids that end in - (#107073)

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,
 	})

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

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{

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) {