fix: use correct primary key columns in config_access_logs OnConflict (#1877)

Author: adityathebe

db/update.go

@@ -886,8 +886,7 @@ func saveResults(ctx api.ScrapeContext, results []v1.ScrapeResult) (v1.ScrapeSum
 			accessLog.ConfigID = uuid.MustParse(config)
 		}
 
-		if err := ctx.DB().Clauses(clause.OnConflict{Columns: []clause.Column{{Name: "id"}}, DoNothing: true}).
-			Save(&accessLog.ConfigAccessLog).Error; err != nil {
+		if err := SaveConfigAccessLog(ctx, &accessLog.ConfigAccessLog); err != nil {
 			return summary, fmt.Errorf("failed to save access log: %w", err)
 		}
 	}
@@ -1395,6 +1394,20 @@ type configExternalKey struct {
 	parentType string
 }
 
+func SaveConfigAccessLog(ctx api.ScrapeContext, accessLog *dutyModels.ConfigAccessLog) error {
+	if accessLog == nil {
+		return nil
+	}
+
+	return ctx.DB().Clauses(clause.OnConflict{
+		Columns:   []clause.Column{{Name: "config_id"}, {Name: "external_user_id"}, {Name: "scraper_id"}},
+		DoUpdates: clause.AssignmentColumns([]string{"created_at", "mfa", "properties"}),
+		Where: clause.Where{Exprs: []clause.Expression{
+			clause.Expr{SQL: "excluded.created_at > config_access_logs.created_at"},
+		}},
+	}).Create(accessLog).Error
+}
+
 // extractResult holds the extracted configs & changes from the scrape result
 type extractResult struct {
 	newConfigs      []*models.ConfigItem

scrapers/run_test.go

@@ -3,6 +3,7 @@ package scrapers
 import (
 	gocontext "context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math/rand/v2"
 	"os"
@@ -1628,3 +1629,131 @@ var _ = Describe("External groups with aliases e2e test", Ordered, func() {
 		Expect(developersID).To(Equal(devsID)) // Same group, same ID
 	})
 })
+
+var _ = Describe("Config access logs upsert", Ordered, func() {
+	fetchAccessLogRefs := func() (dutymodels.ConfigItem, uuid.UUID, api.ScrapeContext, uuid.UUID, func()) {
+		var configItem dutymodels.ConfigItem
+		err := DefaultContext.DB().Where("scraper_id IS NOT NULL").First(&configItem).Error
+		Expect(err).NotTo(HaveOccurred(), "failed to find config item with scraper_id")
+
+		scraperIDValue := lo.FromPtr(configItem.ScraperID)
+		Expect(scraperIDValue).NotTo(BeEmpty())
+
+		scraperID, err := uuid.Parse(scraperIDValue)
+		Expect(err).NotTo(HaveOccurred(), "failed to parse scraper id")
+
+		err = DefaultContext.DB().First(&dutymodels.ConfigScraper{}, "id = ?", scraperID).Error
+		Expect(err).NotTo(HaveOccurred(), "failed to find scraper")
+
+		cleanupExternalUser := func() {}
+		var externalUser dutymodels.ExternalUser
+		err = DefaultContext.DB().Where("scraper_id = ?", scraperID).First(&externalUser).Error
+		if err != nil {
+			if errors.Is(err, gorm.ErrRecordNotFound) {
+				externalUser = dutymodels.ExternalUser{
+					ID:        uuid.New(),
+					Name:      "access-log-user",
+					AccountID: "test-account",
+					UserType:  "user",
+					ScraperID: scraperID,
+					CreatedAt: time.Now(),
+				}
+				err = DefaultContext.DB().Create(&externalUser).Error
+				Expect(err).NotTo(HaveOccurred(), "failed to create external user")
+				cleanupExternalUser = func() {
+					err := DefaultContext.DB().Delete(&externalUser).Error
+					Expect(err).NotTo(HaveOccurred(), "failed to delete external user")
+				}
+			} else {
+				Expect(err).NotTo(HaveOccurred(), "failed to fetch external user")
+			}
+		}
+
+		return configItem, scraperID, api.NewScrapeContext(DefaultContext), externalUser.ID, cleanupExternalUser
+	}
+
+	cleanupAccessLog := func(configID, externalUserID, scraperID uuid.UUID) {
+		err := DefaultContext.DB().Where("config_id = ? AND external_user_id = ? AND scraper_id = ?",
+			configID, externalUserID, scraperID).Delete(&dutymodels.ConfigAccessLog{}).Error
+		Expect(err).NotTo(HaveOccurred(), "failed to delete access logs")
+	}
+
+	It("should update access log when newer", func() {
+		configItem, scraperID, scraperCtx, externalUserID, cleanupExternalUser := fetchAccessLogRefs()
+		DeferCleanup(func() {
+			cleanupAccessLog(configItem.ID, externalUserID, scraperID)
+			cleanupExternalUser()
+		})
+
+		olderTime := time.Now().Add(-2 * time.Hour)
+		latestAccessTime := time.Now().Add(-1 * time.Hour)
+
+		olderLog := dutymodels.ConfigAccessLog{
+			ConfigID:       configItem.ID,
+			ExternalUserID: externalUserID,
+			ScraperID:      scraperID,
+			CreatedAt:      olderTime,
+			MFA:            false,
+		}
+
+		err := db.SaveConfigAccessLog(scraperCtx, &olderLog)
+		Expect(err).NotTo(HaveOccurred())
+
+		newerLog := dutymodels.ConfigAccessLog{
+			ConfigID:       configItem.ID,
+			ExternalUserID: externalUserID,
+			ScraperID:      scraperID,
+			CreatedAt:      latestAccessTime,
+			MFA:            true,
+		}
+
+		err = db.SaveConfigAccessLog(scraperCtx, &newerLog)
+		Expect(err).NotTo(HaveOccurred())
+
+		var storedLog dutymodels.ConfigAccessLog
+		err = DefaultContext.DB().Where("config_id = ? AND external_user_id = ? AND scraper_id = ?",
+			configItem.ID, externalUserID, scraperID).First(&storedLog).Error
+		Expect(err).NotTo(HaveOccurred())
+		Expect(storedLog.MFA).To(BeTrue())
+		Expect(storedLog.CreatedAt).To(BeTemporally("~", latestAccessTime, time.Second))
+	})
+
+	It("should ignore older access log", func() {
+		configItem, scraperID, scraperCtx, externalUserID, cleanupExternalUser := fetchAccessLogRefs()
+		DeferCleanup(func() {
+			cleanupAccessLog(configItem.ID, externalUserID, scraperID)
+			cleanupExternalUser()
+		})
+
+		latestAccessTime := time.Now().Add(-1 * time.Hour)
+		latestLog := dutymodels.ConfigAccessLog{
+			ConfigID:       configItem.ID,
+			ExternalUserID: externalUserID,
+			ScraperID:      scraperID,
+			CreatedAt:      latestAccessTime,
+			MFA:            true,
+		}
+
+		err := db.SaveConfigAccessLog(scraperCtx, &latestLog)
+		Expect(err).NotTo(HaveOccurred())
+
+		olderTime := time.Now().Add(-3 * time.Hour)
+		olderLog := dutymodels.ConfigAccessLog{
+			ConfigID:       configItem.ID,
+			ExternalUserID: externalUserID,
+			ScraperID:      scraperID,
+			CreatedAt:      olderTime,
+			MFA:            false,
+		}
+
+		err = db.SaveConfigAccessLog(scraperCtx, &olderLog)
+		Expect(err).NotTo(HaveOccurred())
+
+		var storedLog dutymodels.ConfigAccessLog
+		err = DefaultContext.DB().Where("config_id = ? AND external_user_id = ? AND scraper_id = ?",
+			configItem.ID, externalUserID, scraperID).First(&storedLog).Error
+		Expect(err).NotTo(HaveOccurred())
+		Expect(storedLog.MFA).To(BeTrue())
+		Expect(storedLog.CreatedAt).To(BeTemporally("~", latestAccessTime, time.Second))
+	})
+})