Use global function to compare any entity to both stores (grafana#89282)

* WIP implement generic compare interface

* Use global compare fn for all entities

* Lint

* Update pkg/apiserver/rest/dualwriter.go

Co-authored-by: Dan Cech <[email protected]>

* Don't need to hash, just compare bytes

* Fix tests

---------

Co-authored-by: Dan Cech <[email protected]>

Author: leonorfmartins

pkg/apiserver/rest/dualwriter.go

@@ -1,7 +1,9 @@
 package rest
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 
@@ -35,8 +37,6 @@ type Storage interface {
 	rest.CreaterUpdater
 	rest.GracefulDeleter
 	rest.CollectionDeleter
-	// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-	Compare(storageObj, legacyObj runtime.Object) bool
 }
 
 // LegacyStorage is a storage implementation that writes to the Grafana SQL database.
@@ -207,3 +207,25 @@ func SetDualWritingMode(
 
 	return NewDualWriter(currentMode, legacy, storage, reg), nil
 }
+
+var defaultConverter = runtime.UnstructuredConverter(runtime.DefaultUnstructuredConverter)
+
+// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
+func Compare(storageObj, legacyObj runtime.Object) bool {
+	return bytes.Equal(removeMeta(storageObj), removeMeta(legacyObj))
+}
+
+func removeMeta(obj runtime.Object) []byte {
+	cpy := obj.DeepCopyObject()
+	unstObj, err := defaultConverter.ToUnstructured(cpy)
+	if err != nil {
+		return nil
+	}
+	// we don't want to compare meta fields
+	delete(unstObj, "meta")
+	jsonObj, err := json.Marshal(cpy)
+	if err != nil {
+		return nil
+	}
+	return jsonObj
+}

pkg/apiserver/rest/dualwriter_mode1.go

@@ -243,7 +243,3 @@ func (d *DualWriterMode1) NewList() runtime.Object {
 func (d *DualWriterMode1) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
 	return d.Legacy.ConvertToTable(ctx, object, tableOptions)
 }
-
-func (d *DualWriterMode1) Compare(storageObj, legacyObj runtime.Object) bool {
-	return d.Storage.Compare(storageObj, legacyObj)
-}

pkg/apiserver/rest/dualwriter_mode1_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"testing"
+	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
@@ -15,10 +16,10 @@ import (
 	"k8s.io/apiserver/pkg/apis/example"
 )
 
-var exampleObj = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: example.PodSpec{}, Status: example.PodStatus{}}
-var exampleObjNoRV = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: ""}, Spec: example.PodSpec{}, Status: example.PodStatus{}}
+var exampleObj = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1", CreationTimestamp: metav1.Time{}}, Spec: example.PodSpec{}, Status: example.PodStatus{StartTime: &metav1.Time{Time: time.Now()}}}
+var exampleObjNoRV = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "", CreationTimestamp: metav1.Time{}}, Spec: example.PodSpec{}, Status: example.PodStatus{StartTime: &metav1.Time{Time: time.Now()}}}
 var exampleObjDifferentRV = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "3"}, Spec: example.PodSpec{}, Status: example.PodStatus{}}
-var anotherObj = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "bar", ResourceVersion: "2"}, Spec: example.PodSpec{}, Status: example.PodStatus{}}
+var anotherObj = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "bar", ResourceVersion: "2"}, Spec: example.PodSpec{}, Status: example.PodStatus{StartTime: &metav1.Time{Time: time.Now()}}}
 var failingObj = &example.Pod{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ObjectMeta: metav1.ObjectMeta{Name: "object-fail", ResourceVersion: "2"}, Spec: example.PodSpec{}, Status: example.PodStatus{}}
 var exampleList = &example.PodList{TypeMeta: metav1.TypeMeta{Kind: "foo"}, ListMeta: metav1.ListMeta{}, Items: []example.Pod{*exampleObj}}
 var anotherList = &example.PodList{Items: []example.Pod{*anotherObj}}

pkg/apiserver/rest/dualwriter_mode2.go

@@ -306,10 +306,6 @@ func (d *DualWriterMode2) ConvertToTable(ctx context.Context, object runtime.Obj
 	return d.Storage.ConvertToTable(ctx, object, tableOptions)
 }
 
-func (d *DualWriterMode2) Compare(storageObj, legacyObj runtime.Object) bool {
-	return d.Storage.Compare(storageObj, legacyObj)
-}
-
 func parseList(legacyList []runtime.Object) (metainternalversion.ListOptions, map[string]int, error) {
 	options := metainternalversion.ListOptions{}
 	originKeys := []string{}

pkg/apiserver/rest/dualwriter_mode3.go

@@ -155,7 +155,3 @@ func (d *DualWriterMode3) NewList() runtime.Object {
 func (d *DualWriterMode3) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
 	return d.Storage.ConvertToTable(ctx, object, tableOptions)
 }
-
-func (d *DualWriterMode3) Compare(storageObj, legacyObj runtime.Object) bool {
-	return d.Storage.Compare(storageObj, legacyObj)
-}

pkg/apiserver/rest/dualwriter_mode4.go

@@ -83,7 +83,3 @@ func (d *DualWriterMode4) NewList() runtime.Object {
 func (d *DualWriterMode4) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) {
 	return d.Storage.ConvertToTable(ctx, object, tableOptions)
 }
-
-func (d *DualWriterMode4) Compare(storageObj, legacyObj runtime.Object) bool {
-	return d.Storage.Compare(storageObj, legacyObj)
-}

pkg/apiserver/rest/dualwriter_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"k8s.io/apimachinery/pkg/runtime"
 )
 
 func TestSetDualWritingMode(t *testing.T) {
@@ -58,3 +59,26 @@ func TestSetDualWritingMode(t *testing.T) {
 		assert.Equal(t, val, fmt.Sprint(tt.expectedMode))
 	}
 }
+
+func TestCompare(t *testing.T) {
+	testCase := []struct {
+		name     string
+		input    runtime.Object
+		expected bool
+	}{
+		{
+			name:     "should return true when both objects are the same",
+			input:    exampleObj,
+			expected: true,
+		},
+		{
+			name:  "should return false when objects are different",
+			input: anotherObj,
+		},
+	}
+	for _, tt := range testCase {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, Compare(tt.input, exampleObj))
+		})
+	}
+}

pkg/registry/apis/dashboard/storage.go

@@ -66,9 +66,3 @@ func newStorage(scheme *runtime.Scheme) (*storage, error) {
 		})
 	return &storage{Store: store}, nil
 }
-
-// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-func (s *storage) Compare(storageObj, legacyObj runtime.Object) bool {
-	//TODO: define the comparison logic between a dashboard returned by the storage and a dashboard returned by the legacy storage
-	return false
-}

pkg/registry/apis/folders/storage.go

@@ -40,9 +40,3 @@ func newStorage(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter, le
 	}
 	return &storage{Store: store}, nil
 }
-
-// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-func (s *storage) Compare(storageObj, legacyObj runtime.Object) bool {
-	//TODO: define the comparison logic between a folder returned by the storage and a folder returned by the legacy storage
-	return false
-}

pkg/registry/apis/peakq/storage.go

@@ -62,9 +62,3 @@ func newStorage(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter) (*
 	}
 	return &storage{Store: store}, nil
 }
-
-// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-func (s *storage) Compare(storageObj, legacyObj runtime.Object) bool {
-	//TODO: define the comparison logic between a query template returned by the storage and a query template returned by the legacy storage
-	return false
-}

pkg/registry/apis/playlist/storage.go

@@ -1,7 +1,6 @@
 package playlist
 
 import (
-	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/generic"
 	genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
@@ -41,17 +40,3 @@ func newStorage(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter, le
 	}
 	return &storage{Store: store}, nil
 }
-
-// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-func (s *storage) Compare(storageObj, legacyObj runtime.Object) bool {
-	accStr, err := meta.Accessor(storageObj)
-	if err != nil {
-		return false
-	}
-	accLegacy, err := meta.Accessor(legacyObj)
-	if err != nil {
-		return false
-	}
-
-	return accStr.GetName() == accLegacy.GetName()
-}

pkg/registry/apis/scope/storage.go

@@ -207,9 +207,3 @@ func SelectableScopeNodeFields(obj *scope.ScopeNode) fields.Set {
 		"spec.parentName": parentName,
 	})
 }
-
-// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-func (s *storage) Compare(storageObj, legacyObj runtime.Object) bool {
-	//TODO: define the comparison logic between a scope returned by the storage and a scope returned by the legacy storage
-	return false
-}

pkg/registry/apis/service/storage.go

@@ -62,9 +62,3 @@ func newStorage(scheme *runtime.Scheme, optsGetter generic.RESTOptionsGetter) (*
 	}
 	return &storage{Store: store}, nil
 }
-
-// Compare asserts on the equality of objects returned from both stores	(object storage and legacy storage)
-func (s *storage) Compare(storageObj, legacyObj runtime.Object) bool {
-	//TODO: define the comparison logic between a generic object returned by the storage and a generic object returned by the legacy storage
-	return false
-}