From 2e5a87906e1c21a9c9349c8dceb90651ac0df4a7 Mon Sep 17 00:00:00 2001 From: Neko Ayaka Date: Tue, 28 Nov 2023 14:47:50 +0800 Subject: [PATCH] fix: direct []byte write when updating resources Signed-off-by: Neko Ayaka --- .../internalstorage/resource_storage.go | 3 +- .../internalstorage/resource_storage_test.go | 105 ++++++++++++++++++ pkg/storage/internalstorage/storage_test.go | 20 ++++ pkg/storage/internalstorage/util_test.go | 2 + 4 files changed, 129 insertions(+), 1 deletion(-) diff --git a/pkg/storage/internalstorage/resource_storage.go b/pkg/storage/internalstorage/resource_storage.go index 4848bc5d4..ff2692b05 100644 --- a/pkg/storage/internalstorage/resource_storage.go +++ b/pkg/storage/internalstorage/resource_storage.go @@ -8,6 +8,7 @@ import ( "reflect" "strconv" + "gorm.io/datatypes" "gorm.io/gorm" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -107,7 +108,7 @@ func (s *ResourceStorage) Update(ctx context.Context, cluster string, obj runtim "owner_uid": ownerUID, "uid": metaobj.GetUID(), "resource_version": metaobj.GetResourceVersion(), - "object": buffer.Bytes(), + "object": datatypes.JSON(buffer.Bytes()), } if deletedAt := metaobj.GetDeletionTimestamp(); deletedAt != nil { updatedResource["deleted_at"] = sql.NullTime{Time: deletedAt.Time, Valid: true} diff --git a/pkg/storage/internalstorage/resource_storage_test.go b/pkg/storage/internalstorage/resource_storage_test.go index 4e4175a3e..4e44168d9 100644 --- a/pkg/storage/internalstorage/resource_storage_test.go +++ b/pkg/storage/internalstorage/resource_storage_test.go @@ -1,15 +1,22 @@ package internalstorage import ( + "bytes" "context" "fmt" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "gorm.io/gorm" appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" internal "github.com/clusterpedia-io/api/clusterpedia" + "github.com/clusterpedia-io/clusterpedia/pkg/storageconfig" ) func testApplyListOptionsToResourceQuery(t *testing.T, name string, options *internal.ListOptions, expected expected) { @@ -358,6 +365,104 @@ func TestResourceStorage_deleteObject(t *testing.T) { } } +func TestResourceStorage_Update(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + db, cleanup, err := newSQLiteDB() + require.NoError(err) + require.NotNil(db) + defer cleanup() + + rs := newTestResourceStorage(db, appsv1.SchemeGroupVersion.WithResource("deployments")) + + factory := storageconfig.NewStorageConfigFactory() + require.NotNil(factory) + + config, err := factory.NewLegacyResourceConfig(schema.GroupResource{Group: appsv1.SchemeGroupVersion.Group, Resource: "deployments"}, true) + require.NoError(err) + require.NotNil(config) + + rs.codec = config.Codec + trueRef := true + + obj := &appsv1.Deployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "Deployment", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Namespace: "foobar", + OwnerReferences: []metav1.OwnerReference{ + {UID: "fooer-id", Name: "fooer", Controller: &trueRef}, + }, + }, + Spec: appsv1.DeploymentSpec{ + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + }, + } + + metaObj, err := meta.Accessor(obj) + require.NoError(err) + ownerRef := metaObj.GetOwnerReferences() + require.Len(ownerRef, 1) + + var buffer bytes.Buffer + err = rs.codec.Encode(obj, &buffer) + require.NoError(err) + + owner := metav1.GetControllerOfNoCopy(metaObj) + require.NotNil(owner) + + ownerUID := owner.UID + require.NotEmpty(ownerUID) + + clusterName := "test" + + err = rs.Create(context.Background(), clusterName, obj) + require.NoError(err) + + var resourcesAfterCreation []Resource + err = db. + Where(Resource{ + Cluster: clusterName, + Name: "foo", + Namespace: "foobar", + }). + Find(&resourcesAfterCreation). + Error + require.NoError(err) + require.Len(resourcesAfterCreation, 1) + assert.NotEmpty(resourcesAfterCreation[0].Object) + + obj.Spec.Template.ObjectMeta.Labels = map[string]string{ + "foo2": "bar2", + } + + err = rs.Update(context.Background(), clusterName, obj) + require.NoError(err) + + var resourcesAfterUpdates []Resource + err = db. + Where(Resource{ + Cluster: clusterName, + Name: "foo", + Namespace: "foobar", + }). + Find(&resourcesAfterUpdates). + Error + require.NoError(err) + require.Len(resourcesAfterUpdates, 1) + assert.NotEmpty(resourcesAfterUpdates[0].Object) + assert.NotEqual(resourcesAfterUpdates[0].Object, resourcesAfterCreation[0].Object) +} + func newTestResourceStorage(db *gorm.DB, storageGVK schema.GroupVersionResource) *ResourceStorage { return &ResourceStorage{ db: db, diff --git a/pkg/storage/internalstorage/storage_test.go b/pkg/storage/internalstorage/storage_test.go index 7ba1933f9..2aa4d377e 100644 --- a/pkg/storage/internalstorage/storage_test.go +++ b/pkg/storage/internalstorage/storage_test.go @@ -8,6 +8,7 @@ import ( "github.com/DATA-DOG/go-sqlmock" gmysql "gorm.io/driver/mysql" gpostgres "gorm.io/driver/postgres" + gsqlite "gorm.io/driver/sqlite" "gorm.io/gorm" ) @@ -34,6 +35,25 @@ func newMockedPostgresDB() (*gorm.DB, sqlmock.Sqlmock, error) { return gormDB, mock, nil } +func newSQLiteDB() (*gorm.DB, func(), error) { + db, err := gorm.Open(gsqlite.Open("test.db")) + if err != nil { + return nil, func() {}, err + } + + err = db.AutoMigrate(&Resource{}) + if err != nil { + return nil, func() {}, err + } + + return db, func() { + err := os.Remove("test.db") + if err != nil { + panic(err) + } + }, nil +} + func newMockedMySQLDB(version string) (*gorm.DB, sqlmock.Sqlmock, error) { mockedDB, mock, err := sqlmock.New() if err != nil { diff --git a/pkg/storage/internalstorage/util_test.go b/pkg/storage/internalstorage/util_test.go index 29cdf04ac..49136e7c1 100644 --- a/pkg/storage/internalstorage/util_test.go +++ b/pkg/storage/internalstorage/util_test.go @@ -121,6 +121,7 @@ func assertPostgresDatabaseExecutedSQL[P any]( db, mock, err := newMockedPostgresDB() require.NoError(t, err) require.NotNil(t, db) + require.NotNil(t, mock) assertDatabaseExecutedSQL(t, db, mock, options, applyFn, expectedQuery, args, expectedError) } @@ -137,6 +138,7 @@ func assertMySQLDatabaseExecutedSQL[P any]( db, mock, err := newMockedMySQLDB(version) require.NoError(t, err) require.NotNil(t, db) + require.NotNil(t, mock) assertDatabaseExecutedSQL(t, db, mock, options, applyFn, expectedQuery, args, expectedError) }