test: external change ID collision prevention

flanksource · Sep 5, 2024 · 1a513a1 · 1a513a1
1 parent d476606
commit 1a513a1
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 8 deletions.
diff --git a/changes/externalIDCache.go b/changes/externalIDCache.go
@@ -3,7 +3,8 @@ package changes
 import (
 	"sync"
 
-	"github.com/flanksource/config-db/api"
+	"github.com/flanksource/duty/context"
+
 	"github.com/flanksource/config-db/db/models"
 )
 
@@ -17,7 +18,7 @@ func AddToExteranlChangeIDCache(changes []*models.ConfigChange) {
 	}
 }
 
-func InitExternalChangeIDCache(ctx api.ScrapeContext) error {
+func InitExternalChangeIDCache(ctx context.Context) error {
 	var externalIDs []string
 	if err := ctx.DB().Select("external_change_id").Model(&models.ConfigChange{}).Where("external_change_id IS NOT NULL").Find(&externalIDs).Error; err != nil {
 		return err

diff --git a/cmd/operator.go b/cmd/operator.go
@@ -58,11 +58,11 @@ func run(cmd *cobra.Command, args []string) error {
 	logger.SetLogLevel(k8sLogLevel)
 
 	dedupWindow := api.DefaultContext.Properties().Duration("changes.dedup.window", time.Hour)
-	if err := db.InitChangeFingerprintCache(api.DefaultContext, dedupWindow); err != nil {
+	if err := db.InitChangeFingerprintCache(api.DefaultContext.Context, dedupWindow); err != nil {
 		return fmt.Errorf("failed to initialize change fingerprint cache: %w", err)
 	}
 
-	if err := changes.InitExternalChangeIDCache(api.DefaultContext); err != nil {
+	if err := changes.InitExternalChangeIDCache(api.DefaultContext.Context); err != nil {
 		return fmt.Errorf("failed to initialize external change ID cache: %w", err)
 	}
 

diff --git a/cmd/server.go b/cmd/server.go
@@ -48,11 +48,11 @@ var Serve = &cobra.Command{
 		api.DefaultContext = api.NewScrapeContext(dutyCtx)
 
 		dedupWindow := api.DefaultContext.Properties().Duration("changes.dedup.window", time.Hour)
-		if err := db.InitChangeFingerprintCache(api.DefaultContext, dedupWindow); err != nil {
+		if err := db.InitChangeFingerprintCache(api.DefaultContext.Context, dedupWindow); err != nil {
 			return fmt.Errorf("failed to initialize change fingerprint cache: %w", err)
 		}
 
-		if err := changes.InitExternalChangeIDCache(api.DefaultContext); err != nil {
+		if err := changes.InitExternalChangeIDCache(api.DefaultContext.Context); err != nil {
 			return fmt.Errorf("failed to initialize external change ID cache: %w", err)
 		}
 

diff --git a/db/changes.go b/db/changes.go
@@ -4,9 +4,11 @@ import (
 	"fmt"
 	"time"
 
+	"github.com/flanksource/duty/context"
+	"github.com/patrickmn/go-cache"
+
 	"github.com/flanksource/config-db/api"
 	"github.com/flanksource/config-db/db/models"
-	"github.com/patrickmn/go-cache"
 )
 
 var changeCacheByFingerprint = cache.New(time.Hour, time.Hour)
@@ -15,7 +17,7 @@ func changeFingeprintCacheKey(configID, fingerprint string) string {
 	return fmt.Sprintf("%s:%s", configID, fingerprint)
 }
 
-func InitChangeFingerprintCache(ctx api.ScrapeContext, window time.Duration) error {
+func InitChangeFingerprintCache(ctx context.Context, window time.Duration) error {
 	var changes []*models.ConfigChange
 	if err := ctx.DB().Where("fingerprint IS NOT NULL").Where("NOW() - created_at <= ?", window).Find(&changes).Error; err != nil {
 		return err

diff --git a/scrapers/runscrapers_test.go b/scrapers/runscrapers_test.go
@@ -10,11 +10,13 @@ import (
 	"github.com/flanksource/commons/logger"
 	"github.com/flanksource/config-db/api"
 	v1 "github.com/flanksource/config-db/api/v1"
+	"github.com/flanksource/config-db/changes"
 	"github.com/flanksource/config-db/db"
 	"github.com/flanksource/config-db/db/models"
 	"github.com/flanksource/duty"
 	dutymodels "github.com/flanksource/duty/models"
 	"github.com/flanksource/duty/query"
+	"github.com/flanksource/duty/tests/fixtures/dummy"
 	"github.com/flanksource/duty/types"
 	"github.com/google/uuid"
 	. "github.com/onsi/ginkgo/v2"
@@ -76,6 +78,67 @@ var _ = Describe("CRD Sync test", Ordered, func() {
 	})
 })
 
+var _ = Describe("prevent config change external_change_id collision on dedup", Ordered, func() {
+	var createdChanges []*models.ConfigChange
+	var scraperCtx api.ScrapeContext
+
+	BeforeAll(func() {
+		scraperCtx = api.NewScrapeContext(DefaultContext).WithScrapeConfig(&v1.ScrapeConfig{})
+	})
+
+	AfterAll(func() {
+		for _, change := range createdChanges {
+			err := DefaultContext.DB().Delete(change).Error
+			Expect(err).NotTo(HaveOccurred(), "failed to delete config change")
+		}
+	})
+
+	It("should create a new change", func() {
+		firstChange := models.ConfigChange{
+			ConfigID:         dummy.EKSCluster.ID.String(),
+			CreatedAt:        time.Now().Add(-time.Minute * 5),
+			ExternalChangeID: lo.ToPtr("first"),
+			Details:          v1.JSON{"id": 1},
+			Fingerprint:      lo.ToPtr("642d77b89087c2c1b898880f2ab8321f"),
+		}
+		err := DefaultContext.DB().Create(&firstChange).Error
+		Expect(err).NotTo(HaveOccurred())
+
+		createdChanges = append(createdChanges, &firstChange)
+	})
+
+	It("should create a second change", func() {
+		secondChange := models.ConfigChange{
+			ConfigID:         dummy.EKSCluster.ID.String(),
+			CreatedAt:        time.Now().Add(-time.Minute),
+			ExternalChangeID: lo.ToPtr("second"),
+			Details:          v1.JSON{"id": 100},
+			Fingerprint:      lo.ToPtr("642d77b89087c2c1b898880f2ab8321f"),
+		}
+		err := DefaultContext.DB().Create(&secondChange).Error
+		Expect(err).NotTo(HaveOccurred())
+
+		createdChanges = append(createdChanges, &secondChange)
+	})
+
+	It("should save without deduping", func() {
+		changes.InitExternalChangeIDCache(DefaultContext)
+		err := db.InitChangeFingerprintCache(DefaultContext, time.Minute*2)
+		Expect(err).NotTo(HaveOccurred())
+
+		_, err = db.SaveResults(scraperCtx, []v1.ScrapeResult{
+			{Changes: []v1.ChangeResult{
+				{
+					ConfigID:         dummy.EKSCluster.ID.String(),
+					ExternalChangeID: "first",
+					Details:          v1.JSON{"id": 1},
+				},
+			}},
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+})
+
 var _ = Describe("Dedup test", Ordered, func() {
 	configA := apiv1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{