Unistore: Propagate DeprecatedLegacyID on upsert (#112619)

Unistore: Fix label propogation on upsert

pkg/registry/apis/dashboard/legacy/storage.go

@@ -119,8 +119,13 @@ func (a *dashboardSqlAccess) WriteEvent(ctx context.Context, event resource.Writ
 				}
 
 				// dashboard version is the RV in legacy storage
+				// and deprecatedInternalID must be set here (as SaveDashboard does below for non-provisioned dashboards)
 				if after != nil {
 					rv = int64(after.Version)
+					access := GetLegacyAccess(ctx)
+					if access != nil {
+						access.DashboardID = after.ID
+					}
 				}
 			} else {
 				failOnExisting := event.Type == resourcepb.WatchEvent_ADDED

pkg/registry/apis/dashboard/legacy/storage_test.go

@@ -83,6 +83,7 @@ func TestWriteProvisioningEvent(t *testing.T) {
 	dashData := &dashboards.Dashboard{
 		Title:   "Test Dashboard",
 		Version: 2,
+		ID:      3,
 	}
 	dashBytes, err := json.Marshal(dashData)
 	require.NoError(t, err)
@@ -125,8 +126,12 @@ func TestWriteProvisioningEvent(t *testing.T) {
 	}
 
 	ctx := identity.WithRequester(context.Background(), &user.SignedInUser{})
+	ctx = WithLegacyAccess(ctx)
 	rv, err := access.WriteEvent(ctx, event)
 	require.NoError(t, err)
 	require.Equal(t, int64(2), rv)
+	a := GetLegacyAccess(ctx)
+	require.NotNil(t, a)
+	require.Equal(t, int64(3), a.DashboardID)
 	mockStore.AssertExpectations(t)
 }

pkg/storage/legacysql/dualwrite/dualwriter.go

@@ -332,10 +332,10 @@ func (d *dualWriter) Update(ctx context.Context, name string, objInfo rest.Updat
 	unifiedInfo := objInfo
 	unifiedForceCreate := forceAllowCreate
 	if d.readUnified {
-		legacyInfo = &wrappedUpdateInfo{objInfo}
+		legacyInfo = &wrappedUpdateInfo{objInfo: objInfo}
 		legacyForceCreate = true
 	} else {
-		unifiedInfo = &wrappedUpdateInfo{objInfo}
+		unifiedInfo = &wrappedUpdateInfo{objInfo: objInfo}
 		unifiedForceCreate = true
 	}
 
@@ -345,6 +345,21 @@ func (d *dualWriter) Update(ctx context.Context, name string, objInfo rest.Updat
 		return nil, false, err
 	}
 
+	// add any metadata returned from legacy to what is saved in unified storage when forceCreate is used.
+	// this is especially needed for legacy internal IDs
+	if createdLegacy {
+		legacyMeta, err := utils.MetaAccessor(objFromLegacy)
+		if err != nil {
+			log.With("object", objFromLegacy).Error("could not get meta accessor for legacy object", "err", err)
+			return nil, false, err
+		}
+		unifiedInfo = &wrappedUpdateInfo{
+			objInfo:           objInfo,
+			legacyLabels:      legacyMeta.GetLabels(),
+			legacyAnnotations: legacyMeta.GetAnnotations(),
+		}
+	}
+
 	if d.readUnified {
 		return d.unified.Update(ctx, name, unifiedInfo, createValidation, updateValidation, unifiedForceCreate, options)
 	} else if d.errorIsOK {
@@ -427,7 +442,9 @@ func (d *dualWriter) ConvertToTable(ctx context.Context, object runtime.Object,
 }
 
 type wrappedUpdateInfo struct {
-	objInfo rest.UpdatedObjectInfo
+	objInfo           rest.UpdatedObjectInfo
+	legacyLabels      map[string]string
+	legacyAnnotations map[string]string
 }
 
 // Preconditions implements rest.UpdatedObjectInfo.
@@ -445,6 +462,29 @@ func (w *wrappedUpdateInfo) UpdatedObject(ctx context.Context, oldObj runtime.Ob
 	if err != nil {
 		return nil, err
 	}
+
+	// add any labels or annotations set by legacy storage
+	if len(w.legacyLabels) > 0 {
+		existingLabels := meta.GetLabels()
+		if existingLabels == nil {
+			existingLabels = make(map[string]string)
+		}
+		for key, value := range w.legacyLabels {
+			existingLabels[key] = value
+		}
+		meta.SetLabels(existingLabels)
+	}
+	if len(w.legacyAnnotations) > 0 {
+		existingAnnotations := meta.GetAnnotations()
+		if existingAnnotations == nil {
+			existingAnnotations = make(map[string]string)
+		}
+		for key, value := range w.legacyAnnotations {
+			existingAnnotations[key] = value
+		}
+		meta.SetAnnotations(existingAnnotations)
+	}
+
 	meta.SetResourceVersion("")
 	meta.SetUID("")
 	return obj, err

pkg/tests/apis/dashboard/integration/api_validation_test.go

@@ -20,6 +20,7 @@ import (
 	dashboardV2beta1 "github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2beta1"
 	foldersV1 "github.com/grafana/grafana/apps/folder/pkg/apis/folder/v1beta1"
 	"github.com/grafana/grafana/pkg/apiserver/rest"
+	"github.com/grafana/grafana/pkg/infra/db"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/folder"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
@@ -30,6 +31,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/apimachinery/identity"
 	"github.com/grafana/grafana/pkg/apimachinery/utils"
+	"github.com/grafana/grafana/pkg/components/simplejson"
 	"github.com/grafana/grafana/pkg/services/dashboards" // TODO: Check if we can remove this import
 	"github.com/grafana/grafana/pkg/services/quota"
 	"github.com/grafana/grafana/pkg/util"
@@ -779,6 +781,76 @@ func runDashboardValidationTests(t *testing.T, ctx TestContext) {
 			require.NoError(t, err)
 		})
 	})
+
+	t.Run("Dashboard upsert propagates legacy id", func(t *testing.T) {
+		// ensures that the internal ID is propogated from legacy to unified on upsert even in mode 3
+		if ctx.DualWriterMode != rest.Mode3 {
+			t.Skip("Skipping upsert metadata test")
+		}
+
+		// create via the service, so that upsert is used
+		specificUID := "upsert-metadata-test"
+		dashboardService := ctx.Helper.GetEnv().Server.HTTPServer.DashboardService
+		require.NotNil(t, dashboardService)
+		provisioningService, ok := dashboardService.(dashboards.DashboardProvisioningService)
+		require.True(t, ok, "DashboardService should also implement DashboardProvisioningService")
+		dashboardData := simplejson.NewFromAny(map[string]interface{}{
+			"title": "Dashboard for Upsert Metadata Test",
+			"uid":   specificUID,
+		})
+		result, err := provisioningService.SaveProvisionedDashboard(context.Background(), &dashboards.SaveDashboardDTO{
+			OrgID: ctx.OrgID,
+			Dashboard: &dashboards.Dashboard{
+				Title: "Dashboard for Upsert Metadata Test",
+				UID:   specificUID,
+				Data:  dashboardData,
+			},
+		}, &dashboards.DashboardProvisioning{
+			Name:       "test-provisioner",
+			ExternalID: "/test/path/dashboard.json",
+			CheckSum:   "abc123",
+		})
+		require.NoError(t, err)
+		require.NotNil(t, result)
+
+		// get the internal id from legacy directly from the db
+		sqlStore := ctx.Helper.GetEnv().Server.HTTPServer.SQLStore
+		require.NotNil(t, sqlStore)
+
+		var legacyID int64
+		err = sqlStore.WithDbSession(context.Background(), func(sess *db.Session) error {
+			has, innerErr := sess.Table("dashboard").
+				Where("uid = ? AND org_id = ?", specificUID, ctx.OrgID).
+				Cols("id").
+				Get(&legacyID)
+			if innerErr != nil {
+				return innerErr
+			}
+			if !has {
+				return fmt.Errorf("dashboard not found in legacy storage")
+			}
+			return nil
+		})
+		require.NoError(t, err)
+		require.NotZero(t, legacyID)
+
+		// then compare what unistore returns
+		dashboardObj, err := adminClient.Resource.Get(context.Background(), specificUID, v1.GetOptions{})
+		require.NoError(t, err)
+		require.NotNil(t, dashboardObj)
+		labels := dashboardObj.GetLabels()
+		require.NotNil(t, labels)
+		deprecatedIDStr, exists := labels[utils.LabelKeyDeprecatedInternalID]
+		require.True(t, exists)
+
+		legacyIDStr := strconv.FormatInt(legacyID, 10)
+		require.Equal(t, legacyIDStr, deprecatedIDStr)
+
+		// clean up - with force so the provisioned dashboard deletion check is skipped
+		zeroPtr := int64(0)
+		err = adminClient.Resource.Delete(context.Background(), specificUID, v1.DeleteOptions{GracePeriodSeconds: &zeroPtr})
+		require.NoError(t, err)
+	})
 }
 
 // Run tests for quota validation