diff --git a/pkg/sqlcache/informer/factory/informer_factory_test.go b/pkg/sqlcache/informer/factory/informer_factory_test.go index 875f412b..80a02d8b 100644 --- a/pkg/sqlcache/informer/factory/informer_factory_test.go +++ b/pkg/sqlcache/informer/factory/informer_factory_test.go @@ -496,4 +496,4 @@ func TestCacheFor(t *testing.T) { for _, test := range tests { t.Run(test.description, func(t *testing.T) { test.test(t) }) } -} \ No newline at end of file +} diff --git a/pkg/sqlcache/store/store.go b/pkg/sqlcache/store/store.go index c8e66295..ea7748ed 100644 --- a/pkg/sqlcache/store/store.go +++ b/pkg/sqlcache/store/store.go @@ -203,7 +203,7 @@ func (s *Store) updateExternalInfo(tx transaction.Client, key string, externalUp } } for _, nonLabelDep := range externalUpdateInfo.ExternalDependencies { - rawGetStmt := fmt.Sprintf(`SELECT f.key, ex2."%s" + rawGetStmt := fmt.Sprintf(`SELECT DISTINCT f.key, ex2."%s" FROM "%s_fields" f JOIN "%s_fields" ex2 ON f."%s" = ex2."%s" WHERE f."%s" != ex2."%s"`, nonLabelDep.TargetFinalFieldName, @@ -216,7 +216,7 @@ func (s *Store) updateExternalInfo(tx transaction.Client, key string, externalUp // TODO: Try to fold the two blocks together getStmt := s.Prepare(rawGetStmt) - rows, err := s.QueryForRows(s.ctx, getStmt, nonLabelDep.SourceFieldName) + rows, err := s.QueryForRows(s.ctx, getStmt) if err != nil { if !isDBError(err) { logrus.Infof("Error getting external info for table %s, key %s: %v", nonLabelDep.TargetGVK, key, &db.QueryError{QueryString: rawGetStmt, Err: err}) @@ -286,6 +286,8 @@ func (s *Store) overrideCheck(finalFieldName, sourceGVK, sourceKey, finalTargetV return false, nil } +/* Core methods */ + // deleteByKey deletes the object associated with key, if it exists in this Store func (s *Store) deleteByKey(key string, obj any) error { return s.WithTransaction(s.ctx, true, func(tx transaction.Client) error { diff --git a/pkg/sqlcache/store/store_test.go b/pkg/sqlcache/store/store_test.go index 18cb3557..2042f6a1 100644 --- a/pkg/sqlcache/store/store_test.go +++ b/pkg/sqlcache/store/store_test.go @@ -743,8 +743,8 @@ func TestAddWithOneUpdate(t *testing.T) { JOIN "management.cattle.io_v3_Project_fields" ex2 ON lt1.value = ex2."metadata.name" WHERE lt1.label = ? AND f."spec.displayName" != ex2."spec.displayName"` c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)) - results1 := "field.cattle.io/projectId" - c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), results1) + args1 := []any{"field.cattle.io/projectId"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1) c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil) // Override check: rawStmt2 := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?` @@ -757,12 +757,12 @@ func TestAddWithOneUpdate(t *testing.T) { txC.EXPECT().Stmt(gomock.Any()).Return(stmts) stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1") - rawStmt3 := `SELECT f.key, ex2."spec.projectName" FROM "_v1_Pods_fields" f + rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName" FROM "_v1_Pods_fields" f JOIN "provisioner.cattle.io_v3_Cluster_fields" ex2 ON f."field.cattle.io/fixer" = ex2."metadata.name" WHERE f."spec.projectName" != ex2."spec.projectName"` c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)) - results2 := []any{"lego.cattle.io/fields2"} - c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), results2) + args2 := []any{} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2) c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields2", "moose2"}}, nil) // Override check: @@ -782,6 +782,152 @@ func TestAddWithOneUpdate(t *testing.T) { } } +func TestAddWithExternalUpdates(t *testing.T) { + type testCase struct { + description string + test func(t *testing.T) + } + testObject := testStoreObject{Id: "testStoreObject", Val: "a"} + var tests []testCase + tests = append(tests, testCase{description: "Add with no DB client errors", test: func(t *testing.T) { + c, txC := SetupMockDB(t) + stmts := NewMockStmt(gomock.NewController(t)) + store := SetupStoreWithExternalDependencies(t, c, true, false) + + c.EXPECT().Upsert(txC, store.upsertStmt, "testStoreObject", testObject, store.shouldEncrypt).Return(nil) + c.EXPECT().WithTransaction(gomock.Any(), true, gomock.Any()).Return(nil).Do( + func(ctx context.Context, shouldEncrypt bool, f db.WithTransactionFunction) { + err := f(txC) + if err != nil { + t.Fail() + } + }).Times(2) + rawStmt := `SELECT DISTINCT f.key, ex2."spec.displayName" FROM "_v1_Namespace_fields" f + LEFT OUTER JOIN "_v1_Namespace_labels" lt1 ON f.key = lt1.key + JOIN "management.cattle.io_v3_Project_fields" ex2 ON lt1.value = ex2."metadata.name" + WHERE lt1.label = ? AND f."spec.displayName" != ex2."spec.displayName"` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)) + args1 := []any{"field.cattle.io/projectId"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1) + c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil) + + rawStmt1b := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt1b)) + args1b := []any{"lego.cattle.io/fields1"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1b) + c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"flipper"}, nil) + + rawStmt2 := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?` + c.EXPECT().Prepare(rawStmt2) + txC.EXPECT().Stmt(gomock.Any()).Return(stmts) + stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1") + + rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName" + FROM "_v1_Pods_fields" f JOIN "provisioner.cattle.io_v3_Cluster_fields" ex2 + ON f."field.cattle.io/fixer" = ex2."metadata.name" + WHERE f."spec.projectName" != ex2."spec.projectName"` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)) + args2 := []any{} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2) + c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields2", "moose2"}}, nil) + + rawStmt3b := `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3b)) + args3b := []any{"lego.cattle.io/fields2"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args3b) + c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"snorkel"}, nil) + + rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?` + c.EXPECT().Prepare(rawStmt4) + txC.EXPECT().Stmt(gomock.Any()).Return(stmts) + stmts.EXPECT().Exec("moose2", "lego.cattle.io/fields2") + + err := store.Add(testObject) + assert.Nil(t, err) + }, + }) + + t.Parallel() + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + test.test(t) + }) + } +} + +func TestAddWithSelfUpdates(t *testing.T) { + type testCase struct { + description string + test func(t *testing.T) + } + testObject := testStoreObject{Id: "testStoreObject", Val: "a"} + var tests []testCase + tests = append(tests, testCase{description: "Add with no DB client errors", test: func(t *testing.T) { + c, txC := SetupMockDB(t) + stmts := NewMockStmt(gomock.NewController(t)) + store := SetupStoreWithExternalDependencies(t, c, false, true) + + c.EXPECT().Upsert(txC, store.upsertStmt, "testStoreObject", testObject, store.shouldEncrypt).Return(nil) + c.EXPECT().WithTransaction(gomock.Any(), true, gomock.Any()).Return(nil).Do( + func(ctx context.Context, shouldEncrypt bool, f db.WithTransactionFunction) { + err := f(txC) + if err != nil { + t.Fail() + } + }).Times(2) + rawStmt := `SELECT DISTINCT f.key, ex2."spec.displayName" FROM "_v1_Namespace_fields" f + LEFT OUTER JOIN "_v1_Namespace_labels" lt1 ON f.key = lt1.key + JOIN "management.cattle.io_v3_Project_fields" ex2 ON lt1.value = ex2."metadata.name" + WHERE lt1.label = ? AND f."spec.displayName" != ex2."spec.displayName"` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)) + args1 := []any{"field.cattle.io/projectId"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1) + c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil) + + rawStmt1b := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt1b)) + args1b := []any{"lego.cattle.io/fields1"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1b) + c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"flipper"}, nil) + + rawStmt2 := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?` + c.EXPECT().Prepare(rawStmt2) + txC.EXPECT().Stmt(gomock.Any()).Return(stmts) + stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1") + + rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName" + FROM "_v1_Pods_fields" f JOIN "provisioner.cattle.io_v3_Cluster_fields" ex2 + ON f."field.cattle.io/fixer" = ex2."metadata.name" + WHERE f."spec.projectName" != ex2."spec.projectName"` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)) + args2 := []any{} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2) + c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"field.cattle.io/fixer", "moose1"}}, nil) + + rawStmt3b := `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?` + c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3b)) + args3b := []any{"field.cattle.io/fixer"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args3b) + c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"snorkel"}, nil) + + rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?` + c.EXPECT().Prepare(rawStmt4) + txC.EXPECT().Stmt(gomock.Any()).Return(stmts) + stmts.EXPECT().Exec("moose1", "field.cattle.io/fixer") + + err := store.Add(testObject) + assert.Nil(t, err) + }, + }) + + t.Parallel() + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + test.test(t) + }) + } +} + func TestAddWithBothUpdates(t *testing.T) { type testCase struct { description string @@ -798,7 +944,7 @@ func TestAddWithBothUpdates(t *testing.T) { LEFT OUTER JOIN "_v1_Namespace_labels" lt1 ON f.key = lt1.key JOIN "management.cattle.io_v3_Project_fields" ex2 ON lt1.value = ex2."metadata.name" WHERE lt1.label = ? AND f."spec.displayName" != ex2."spec.displayName"` - rawStmt3 := `SELECT f.key, ex2."spec.projectName" FROM "_v1_Pods_fields" f + rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName" FROM "_v1_Pods_fields" f JOIN "provisioner.cattle.io_v3_Cluster_fields" ex2 ON f."field.cattle.io/fixer" = ex2."metadata.name" WHERE f."spec.projectName" != ex2."spec.projectName"` @@ -819,13 +965,14 @@ func TestAddWithBothUpdates(t *testing.T) { } }) c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)) - results1 := "field.cattle.io/projectId" - c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), results1) + args1 := []any{"field.cattle.io/projectId"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1) c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil) // Override check: rawStmt2 := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?` c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2)) - c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), gomock.Any()) + args1b := []any{"lego.cattle.io/fields1"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1b) c.EXPECT().ReadStrings(gomock.Any()) rawStmt2a := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?` @@ -834,20 +981,21 @@ func TestAddWithBothUpdates(t *testing.T) { stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1") c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)) - results2 := []any{"field.cattle.io/fixer"} - c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), results2) + args2 := []any{} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2) - c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"lego.cattle.io/fields2", "moose2"}}, nil) + c.EXPECT().ReadStrings2(gomock.Any()).Return([][]string{{"field.cattle.io/fixer", "moose1"}}, nil) // Override check: rawStmt2 = `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?` c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2)) - c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), gomock.Any()) + args3b := []any{"field.cattle.io/fixer"} + c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args3b) c.EXPECT().ReadStrings(gomock.Any()) rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?` c.EXPECT().Prepare(rawStmt4) txC.EXPECT().Stmt(gomock.Any()).Return(stmts) - stmts.EXPECT().Exec("moose2", "lego.cattle.io/fields2") + stmts.EXPECT().Exec("moose1", "field.cattle.io/fixer") // And again for the other object }