Skip to content

Commit aa1ea9f

Browse files
authored
feat: add support for external user aliases (#1847)
* feat: add support for external user aliases * chore: fix tests + logic * chore: remove scraper id from external user alias * chore: update schema * chore: fix lint
1 parent 42b36d6 commit aa1ea9f

File tree

8 files changed

+138
-37
lines changed

8 files changed

+138
-37
lines changed

chart/crds/configs.flanksource.com_scrapeconfigs.yaml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2567,6 +2567,8 @@ spec:
25672567
type: object
25682568
connection:
25692569
type: string
2570+
depth:
2571+
type: integer
25702572
destination:
25712573
description: |-
25722574
Destination is the full path to where the contents of the URL should be downloaded to.
@@ -7855,6 +7857,8 @@ spec:
78557857
properties:
78567858
address:
78577859
type: string
7860+
connection:
7861+
type: string
78587862
index:
78597863
type: string
78607864
limit:
@@ -7948,7 +7952,6 @@ spec:
79487952
type: object
79497953
type: object
79507954
required:
7951-
- address
79527955
- index
79537956
- query
79547957
type: object

config/schemas/config_exec.schema.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,9 @@
477477
"branch": {
478478
"type": "string"
479479
},
480+
"depth": {
481+
"type": "integer"
482+
},
480483
"destination": {
481484
"type": "string"
482485
}

config/schemas/config_logs.schema.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,9 @@
485485
},
486486
"OpenSearchConfig": {
487487
"properties": {
488+
"connection": {
489+
"type": "string"
490+
},
488491
"address": {
489492
"type": "string"
490493
},
@@ -507,7 +510,6 @@
507510
"additionalProperties": false,
508511
"type": "object",
509512
"required": [
510-
"address",
511513
"index",
512514
"query"
513515
]

config/schemas/scrape_config.schema.json

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1444,6 +1444,9 @@
14441444
"branch": {
14451445
"type": "string"
14461446
},
1447+
"depth": {
1448+
"type": "integer"
1449+
},
14471450
"destination": {
14481451
"type": "string"
14491452
}
@@ -2410,6 +2413,9 @@
24102413
},
24112414
"OpenSearchConfig": {
24122415
"properties": {
2416+
"connection": {
2417+
"type": "string"
2418+
},
24132419
"address": {
24142420
"type": "string"
24152421
},
@@ -2432,7 +2438,6 @@
24322438
"additionalProperties": false,
24332439
"type": "object",
24342440
"required": [
2435-
"address",
24362441
"index",
24372442
"query"
24382443
]

db/update.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,44 @@ func extractChanges(ctx api.ScrapeContext, result *v1.ScrapeResult, ci *models.C
397397

398398
var OrphanCache = cache.New(60*time.Minute, 10*time.Minute)
399399

400+
// ExternalUserCache stores alias -> external_user_id mapping
401+
var ExternalUserCache = cache.New(time.Hour, 10*time.Minute)
402+
403+
// findExternalUserIDByAliases looks up an external user ID by aliases
404+
// It first checks the cache, then queries the DB. Returns the ID if found, uuid.Nil otherwise.
405+
func findExternalUserIDByAliases(ctx api.ScrapeContext, aliases []string) (uuid.UUID, error) {
406+
for _, alias := range aliases {
407+
if cachedID, ok := ExternalUserCache.Get(alias); ok {
408+
return cachedID.(uuid.UUID), nil
409+
}
410+
}
411+
412+
// Query DB for any matching alias
413+
for _, alias := range aliases {
414+
var existingUser dutyModels.ExternalUser
415+
err := ctx.DB().
416+
Select("id").
417+
Where("? = ANY(aliases)", alias).
418+
Where("deleted_at IS NULL").
419+
First(&existingUser).Error
420+
421+
if err != nil {
422+
if !errors.Is(err, gorm.ErrRecordNotFound) {
423+
return uuid.Nil, fmt.Errorf("failed to query external user by alias: %w", err)
424+
}
425+
continue
426+
}
427+
428+
// Found in DB, populate cache for all aliases and return
429+
for _, a := range aliases {
430+
ExternalUserCache.Set(a, existingUser.ID, cache.DefaultExpiration)
431+
}
432+
return existingUser.ID, nil
433+
}
434+
435+
return uuid.Nil, nil
436+
}
437+
400438
func upsertAnalysis(ctx api.ScrapeContext, result *v1.ScrapeResult) error {
401439
var ci *models.ConfigItem
402440
var err error
@@ -624,9 +662,25 @@ func saveResults(ctx api.ScrapeContext, results []v1.ScrapeResult) (v1.ScrapeSum
624662

625663
for _, externalUser := range extractResult.externalUsers {
626664
externalUser.ScraperID = lo.Ternary(externalUser.ScraperID == uuid.Nil, lo.FromPtr(scraperID), externalUser.ScraperID)
665+
666+
if len(externalUser.Aliases) > 0 {
667+
existingID, err := findExternalUserIDByAliases(ctx, externalUser.Aliases)
668+
if err != nil {
669+
return summary, fmt.Errorf("failed to find external user by aliases: %w", err)
670+
}
671+
if existingID != uuid.Nil {
672+
externalUser.ID = existingID
673+
}
674+
}
675+
627676
if err := ctx.DB().Save(&externalUser).Error; err != nil {
628677
return summary, fmt.Errorf("failed to save external user: %w", err)
629678
}
679+
680+
// Update cache for all aliases
681+
for _, alias := range externalUser.Aliases {
682+
ExternalUserCache.Set(alias, externalUser.ID, cache.DefaultExpiration)
683+
}
630684
}
631685

632686
for _, externalGroup := range extractResult.externalGroups {

fixtures/data/external_users_test.json

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,17 @@
22
"id": "test-org-1",
33
"external_users": [
44
{
5-
"id": "018e4c6a-1111-7000-8000-000000000001",
65
"name": "John Doe",
76
"account_id": "org-123",
87
"user_type": "human",
9-
"email": "[email protected]"
8+
"email": "[email protected]",
9+
"aliases": ["john-doe", "[email protected]"]
1010
},
1111
{
12-
"id": "018e4c6a-1111-7000-8000-000000000002",
1312
"name": "Service Bot",
1413
"account_id": "org-123",
15-
"user_type": "service"
14+
"user_type": "service",
15+
"aliases": ["service-bot", "bot-001"]
1616
}
1717
],
1818
"external_groups": [
@@ -44,15 +44,5 @@
4444
"role_type": "custom",
4545
"description": "Read-only access"
4646
}
47-
],
48-
"external_user_groups": [
49-
{
50-
"external_user_id": "018e4c6a-1111-7000-8000-000000000001",
51-
"external_group_id": "018e4c6a-2222-7000-8000-000000000001"
52-
},
53-
{
54-
"external_user_id": "018e4c6a-1111-7000-8000-000000000002",
55-
"external_group_id": "018e4c6a-2222-7000-8000-000000000002"
56-
}
5747
]
5848
}

scrapers/aws/cloudtrail.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ var LastEventTime = sync.Map{}
5151
type CloudTrailEvent struct {
5252
AWSRegion string `json:"awsRegion"`
5353
RecipientAccountID string `json:"recipientAccountId"`
54-
UserIdentity struct {
54+
UserIdentity struct {
5555
Type string `json:"type"`
5656
Arn string `json:"arn"`
5757
Username string `json:"userName"`

scrapers/run_test.go

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -723,16 +723,8 @@ var _ = Describe("External users e2e test", Ordered, func() {
723723
})
724724

725725
AfterAll(func() {
726-
// Clean up external_user_groups first (foreign key constraint)
727-
err := DefaultContext.DB().Where("external_user_id IN (?)",
728-
[]string{
729-
"018e4c6a-1111-7000-8000-000000000001",
730-
"018e4c6a-1111-7000-8000-000000000002",
731-
}).Delete(&dutymodels.ExternalUserGroup{}).Error
732-
Expect(err).NotTo(HaveOccurred(), "failed to delete external user groups")
733-
734726
// Clean up external users
735-
err = DefaultContext.DB().Where("scraper_id = ?", scraperModel.ID).Delete(&dutymodels.ExternalUser{}).Error
727+
err := DefaultContext.DB().Where("scraper_id = ?", scraperModel.ID).Delete(&dutymodels.ExternalUser{}).Error
736728
Expect(err).NotTo(HaveOccurred(), "failed to delete external users")
737729

738730
// Clean up external groups
@@ -748,19 +740,29 @@ var _ = Describe("External users e2e test", Ordered, func() {
748740
Expect(err).NotTo(HaveOccurred(), "failed to delete scrape config")
749741
})
750742

751-
It("should scrape and save external users, groups, roles, and user-group mappings", func() {
743+
It("should scrape and save external users, groups, and roles", func() {
752744
_, err := RunScraper(scraperCtx)
753745
Expect(err).To(BeNil())
754746
})
755747

756-
It("should have saved external users to the database", func() {
748+
It("should have saved external users to the database with aliases", func() {
757749
var users []dutymodels.ExternalUser
758750
err := DefaultContext.DB().Where("scraper_id = ?", scraperModel.ID).Find(&users).Error
759751
Expect(err).NotTo(HaveOccurred())
760752
Expect(users).To(HaveLen(2))
761753

762754
userNames := lo.Map(users, func(u dutymodels.ExternalUser, _ int) string { return u.Name })
763755
Expect(userNames).To(ContainElements("John Doe", "Service Bot"))
756+
757+
// Verify aliases are saved
758+
for _, user := range users {
759+
switch user.Name {
760+
case "John Doe":
761+
Expect(user.Aliases).To(ContainElements("john-doe", "[email protected]"))
762+
case "Service Bot":
763+
Expect(user.Aliases).To(ContainElements("service-bot", "bot-001"))
764+
}
765+
}
764766
})
765767

766768
It("should have saved external groups to the database", func() {
@@ -783,14 +785,56 @@ var _ = Describe("External users e2e test", Ordered, func() {
783785
Expect(roleNames).To(ContainElements("Admin", "Reader"))
784786
})
785787

786-
It("should have saved external user groups to the database", func() {
787-
var userGroups []dutymodels.ExternalUserGroup
788-
err := DefaultContext.DB().Where("external_user_id IN (?)",
789-
[]string{
790-
"018e4c6a-1111-7000-8000-000000000001",
791-
"018e4c6a-1111-7000-8000-000000000002",
792-
}).Find(&userGroups).Error
788+
It("should upsert external users by alias on second scrape (not create duplicates)", func() {
789+
// Get existing user IDs before second scrape
790+
var usersBefore []dutymodels.ExternalUser
791+
err := DefaultContext.DB().Where("scraper_id = ?", scraperModel.ID).Find(&usersBefore).Error
792+
Expect(err).NotTo(HaveOccurred())
793+
Expect(usersBefore).To(HaveLen(2))
794+
795+
userIDsBefore := lo.Map(usersBefore, func(u dutymodels.ExternalUser, _ int) uuid.UUID { return u.ID })
796+
797+
// Clear cache to ensure we test DB lookup path
798+
db.ExternalUserCache.Flush()
799+
800+
// Run scraper again
801+
_, err = RunScraper(scraperCtx)
802+
Expect(err).To(BeNil())
803+
804+
// Verify same number of users (no duplicates)
805+
var usersAfter []dutymodels.ExternalUser
806+
err = DefaultContext.DB().Where("scraper_id = ?", scraperModel.ID).Find(&usersAfter).Error
793807
Expect(err).NotTo(HaveOccurred())
794-
Expect(userGroups).To(HaveLen(2))
808+
Expect(usersAfter).To(HaveLen(2))
809+
810+
// Verify same IDs were used (upsert, not insert)
811+
userIDsAfter := lo.Map(usersAfter, func(u dutymodels.ExternalUser, _ int) uuid.UUID { return u.ID })
812+
Expect(userIDsAfter).To(ConsistOf(userIDsBefore))
813+
})
814+
815+
It("should use cache for external user lookup on subsequent scrapes", func() {
816+
// Clear cache first
817+
db.ExternalUserCache.Flush()
818+
819+
// Run scraper to populate cache
820+
_, err := RunScraper(scraperCtx)
821+
Expect(err).To(BeNil())
822+
823+
// Verify cache is populated for all aliases (key is just the alias)
824+
johnDoeID, found := db.ExternalUserCache.Get("john-doe")
825+
Expect(found).To(BeTrue())
826+
Expect(johnDoeID).NotTo(Equal(uuid.Nil))
827+
828+
jdoeID, found := db.ExternalUserCache.Get("[email protected]")
829+
Expect(found).To(BeTrue())
830+
Expect(jdoeID).To(Equal(johnDoeID)) // Same user, same ID
831+
832+
serviceBotID, found := db.ExternalUserCache.Get("service-bot")
833+
Expect(found).To(BeTrue())
834+
Expect(serviceBotID).NotTo(Equal(uuid.Nil))
835+
836+
bot001ID, found := db.ExternalUserCache.Get("bot-001")
837+
Expect(found).To(BeTrue())
838+
Expect(bot001ID).To(Equal(serviceBotID)) // Same user, same ID
795839
})
796840
})

0 commit comments

Comments
 (0)