Skip to content

Commit 0a4a5be

Browse files
alexotttanmay-db
andauthored
[Exporter] Adding more retries for SCIM API calls (#3807)
## Changes <!-- Summary of your changes that are easy to understand --> There were issue when SCIM API failed to return list of users (because of the incorrect status code or something like that, so Go SDK didn't retry it) and this lead to marking all workspace objects (notebooks/files/...) as ignored because user wasn't found. Also, when the list of users wasn't retrieved, the process didn't stop, and we generated incomplete Terraform code. This PR improves the situation by following: - Adding retries around Users/SPs/Groups SCIM API calls - Exit with the error code when we aren't able to fetch the list of the users or service principals ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] relevant acceptance tests are passing - [ ] using Go SDK Co-authored-by: Tanmay Rustagi <[email protected]>
1 parent 324ac5a commit 0a4a5be

File tree

2 files changed

+106
-68
lines changed

2 files changed

+106
-68
lines changed

exporter/importables.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,7 +1023,7 @@ var resourcesMap map[string]importable = map[string]importable{
10231023
if err != nil {
10241024
return err
10251025
}
1026-
ic.emitGroups(u)
1026+
ic.emitGroups(*u)
10271027
ic.emitRoles("user", u.ID, u.Roles)
10281028
return nil
10291029
},
@@ -1081,7 +1081,7 @@ var resourcesMap map[string]importable = map[string]importable{
10811081
if err != nil {
10821082
return err
10831083
}
1084-
ic.emitGroups(u)
1084+
ic.emitGroups(*u)
10851085
ic.emitRoles("service_principal", u.ID, u.Roles)
10861086
if ic.accountLevel {
10871087
ic.Emit(&resource{

exporter/util.go

Lines changed: 104 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -452,32 +452,48 @@ func (ic *importContext) cacheGroups() error {
452452
defer ic.groupsMutex.Unlock()
453453
if ic.allGroups == nil {
454454
log.Printf("[INFO] Caching groups in memory ...")
455-
var groups []iam.Group
455+
var groups *[]iam.Group
456456
var err error
457-
if ic.accountLevel {
458-
groups, err = ic.accountClient.Groups.ListAll(ic.Context, iam.ListAccountGroupsRequest{
459-
Attributes: "id",
460-
})
461-
} else {
462-
groups, err = ic.workspaceClient.Groups.ListAll(ic.Context, iam.ListGroupsRequest{
463-
Attributes: "id",
464-
})
465-
}
457+
err = runWithRetries(func() error {
458+
var grps []iam.Group
459+
var err error
460+
if ic.accountLevel {
461+
grps, err = ic.accountClient.Groups.ListAll(ic.Context, iam.ListAccountGroupsRequest{
462+
Attributes: "id",
463+
})
464+
} else {
465+
grps, err = ic.workspaceClient.Groups.ListAll(ic.Context, iam.ListGroupsRequest{
466+
Attributes: "id",
467+
})
468+
}
469+
if err != nil {
470+
return err
471+
}
472+
groups = &grps
473+
return nil
474+
}, "error fetching full list of groups")
466475
if err != nil {
467-
log.Printf("[ERROR] can't fetch list of groups")
476+
log.Printf("[ERROR] can't fetch list of groups. Error: %v", err)
468477
return err
469478
}
470479
api := scim.NewGroupsAPI(ic.Context, ic.Client)
471-
ic.allGroups = make([]scim.Group, 0, len(groups))
472-
for i, g := range groups {
473-
group, err := api.Read(g.Id, "id,displayName,active,externalId,entitlements,groups,roles,members")
480+
groupsCount := len(*groups)
481+
ic.allGroups = make([]scim.Group, 0, groupsCount)
482+
for i, g := range *groups {
483+
err = runWithRetries(func() error {
484+
group, err := api.Read(g.Id, "id,displayName,active,externalId,entitlements,groups,roles,members")
485+
if err != nil {
486+
return err
487+
}
488+
ic.allGroups = append(ic.allGroups, group)
489+
return nil
490+
}, "error reading group with ID "+g.Id)
474491
if err != nil {
475-
log.Printf("[ERROR] Error reading group with ID %s", g.Id)
492+
log.Printf("[ERROR] Error reading group with ID %s: %v", g.Id, err)
476493
continue
477494
}
478-
ic.allGroups = append(ic.allGroups, group)
479495
if (i+1)%10 == 0 {
480-
log.Printf("[DEBUG] Read %d out of %d groups", i+1, len(groups))
496+
log.Printf("[DEBUG] Read %d out of %d groups", i+1, groupsCount)
481497
}
482498
}
483499
log.Printf("[INFO] Cached %d groups", len(ic.allGroups))
@@ -506,30 +522,34 @@ func (ic *importContext) getUsersMapping() {
506522
return
507523
}
508524
ic.allUsersMapping = make(map[string]string)
509-
var users []iam.User
510-
var err error
511-
if ic.accountLevel {
512-
users, err = ic.accountClient.Users.ListAll(ic.Context, iam.ListAccountUsersRequest{
513-
Attributes: "id,userName",
514-
})
515-
} else {
516-
users, err = ic.workspaceClient.Users.ListAll(ic.Context, iam.ListUsersRequest{
517-
Attributes: "id,userName",
518-
})
519-
}
525+
err := runWithRetries(func() error {
526+
var users []iam.User
527+
var err error
528+
if ic.accountLevel {
529+
users, err = ic.accountClient.Users.ListAll(ic.Context, iam.ListAccountUsersRequest{
530+
Attributes: "id,userName",
531+
})
532+
} else {
533+
users, err = ic.workspaceClient.Users.ListAll(ic.Context, iam.ListUsersRequest{
534+
Attributes: "id,userName",
535+
})
536+
}
537+
if err != nil {
538+
return err
539+
}
540+
for _, user := range users {
541+
ic.allUsersMapping[user.UserName] = user.Id
542+
}
543+
log.Printf("[DEBUG] users are copied")
544+
return nil
545+
}, "error fetching full list of users")
520546
if err != nil {
521-
log.Printf("[ERROR] can't fetch list of users")
522-
return
523-
}
524-
for _, user := range users {
525-
// log.Printf("[DEBUG] adding user %v into the map. %d out of %d", user, i+1, len(users))
526-
ic.allUsersMapping[user.UserName] = user.Id
547+
log.Fatalf("[ERROR] can't fetch list of users after few retries: error=%v", err)
527548
}
528-
log.Printf("[DEBUG] users are copied")
529549
}
530550
}
531551

532-
func (ic *importContext) findUserByName(name string, fastCheck bool) (u scim.User, err error) {
552+
func (ic *importContext) findUserByName(name string, fastCheck bool) (u *scim.User, err error) {
533553
log.Printf("[DEBUG] Looking for user %s", name)
534554
ic.usersMutex.RLocker().Lock()
535555
user, exists := ic.allUsers[name]
@@ -540,7 +560,7 @@ func (ic *importContext) findUserByName(name string, fastCheck bool) (u scim.Use
540560
err = fmt.Errorf("user %s is not found", name)
541561
} else {
542562
log.Printf("[DEBUG] existing user %s is found in the cache", name)
543-
u = user
563+
u = &user
544564
}
545565
return
546566
}
@@ -550,21 +570,28 @@ func (ic *importContext) findUserByName(name string, fastCheck bool) (u scim.Use
550570
ic.allUsersMutex.RLocker().Unlock()
551571
if !exists {
552572
err = fmt.Errorf("there is no user '%s'", name)
553-
u = scim.User{UserName: nonExistingUserOrSp}
573+
u = &scim.User{UserName: nonExistingUserOrSp}
554574
} else {
555575
if fastCheck {
556-
return scim.User{UserName: name}, nil
576+
return &scim.User{UserName: name}, nil
557577
}
558578
a := scim.NewUsersAPI(ic.Context, ic.Client)
559-
u, err = a.Read(userId, "id,userName,displayName,active,externalId,entitlements,groups,roles")
579+
err = runWithRetries(func() error {
580+
usr, err := a.Read(userId, "id,userName,displayName,active,externalId,entitlements,groups,roles")
581+
if err != nil {
582+
return err
583+
}
584+
u = &usr
585+
return nil
586+
}, fmt.Sprintf("error reading user with name '%s', user ID: %s", name, userId))
560587
if err != nil {
561588
log.Printf("[WARN] error reading user with name '%s', user ID: %s", name, userId)
562-
u = scim.User{UserName: nonExistingUserOrSp}
589+
u = &scim.User{UserName: nonExistingUserOrSp}
563590
}
564591
}
565592
ic.usersMutex.Lock()
566593
defer ic.usersMutex.Unlock()
567-
ic.allUsers[name] = u
594+
ic.allUsers[name] = *u
568595
return
569596
}
570597

@@ -573,24 +600,28 @@ func (ic *importContext) getSpsMapping() {
573600
defer ic.spsMutex.Unlock()
574601
if ic.allSpsMapping == nil {
575602
ic.allSpsMapping = make(map[string]string)
576-
var sps []iam.ServicePrincipal
577-
var err error
578-
// Reimplement it myself
579-
if ic.accountLevel {
580-
sps, err = ic.accountClient.ServicePrincipals.ListAll(ic.Context, iam.ListAccountServicePrincipalsRequest{
581-
Attributes: "id,userName",
582-
})
583-
} else {
584-
sps, err = ic.workspaceClient.ServicePrincipals.ListAll(ic.Context, iam.ListServicePrincipalsRequest{
585-
Attributes: "id,userName",
586-
})
587-
}
603+
err := runWithRetries(func() error {
604+
var sps []iam.ServicePrincipal
605+
var err error
606+
if ic.accountLevel {
607+
sps, err = ic.accountClient.ServicePrincipals.ListAll(ic.Context, iam.ListAccountServicePrincipalsRequest{
608+
Attributes: "id,userName",
609+
})
610+
} else {
611+
sps, err = ic.workspaceClient.ServicePrincipals.ListAll(ic.Context, iam.ListServicePrincipalsRequest{
612+
Attributes: "id,userName",
613+
})
614+
}
615+
if err != nil {
616+
return err
617+
}
618+
for _, sp := range sps {
619+
ic.allSpsMapping[sp.ApplicationId] = sp.Id
620+
}
621+
return nil
622+
}, "error fetching full list of service principals")
588623
if err != nil {
589-
log.Printf("[ERROR] can't fetch list of service principals")
590-
return
591-
}
592-
for _, sp := range sps {
593-
ic.allSpsMapping[sp.ApplicationId] = sp.Id
624+
log.Fatalf("[ERROR] can't fetch list of service principals after few retries: error=%v", err)
594625
}
595626
}
596627
}
@@ -621,7 +652,7 @@ func (ic *importContext) getBuiltinPolicyFamilies() map[string]compute.PolicyFam
621652
return ic.builtInPolicies
622653
}
623654

624-
func (ic *importContext) findSpnByAppID(applicationID string, fastCheck bool) (u scim.User, err error) {
655+
func (ic *importContext) findSpnByAppID(applicationID string, fastCheck bool) (u *scim.User, err error) {
625656
log.Printf("[DEBUG] Looking for SP %s", applicationID)
626657
ic.spsMutex.RLocker().Lock()
627658
sp, exists := ic.allSps[applicationID]
@@ -632,7 +663,7 @@ func (ic *importContext) findSpnByAppID(applicationID string, fastCheck bool) (u
632663
err = fmt.Errorf("service principal %s is not found", applicationID)
633664
} else {
634665
log.Printf("[DEBUG] existing SP %s is found in the cache", applicationID)
635-
u = sp
666+
u = &sp
636667
}
637668
return
638669
}
@@ -642,21 +673,28 @@ func (ic *importContext) findSpnByAppID(applicationID string, fastCheck bool) (u
642673
ic.spsMutex.RLocker().Unlock()
643674
if !exists {
644675
err = fmt.Errorf("there is no service principal '%s'", applicationID)
645-
u = scim.User{ApplicationID: nonExistingUserOrSp}
676+
u = &scim.User{ApplicationID: nonExistingUserOrSp}
646677
} else {
647678
if fastCheck {
648-
return scim.User{ApplicationID: applicationID}, nil
679+
return &scim.User{ApplicationID: applicationID}, nil
649680
}
650681
a := scim.NewServicePrincipalsAPI(ic.Context, ic.Client)
651-
u, err = a.Read(spId, "userName,displayName,active,externalId,entitlements,groups,roles")
682+
err = runWithRetries(func() error {
683+
usr, err := a.Read(spId, "userName,displayName,active,externalId,entitlements,groups,roles")
684+
if err != nil {
685+
return err
686+
}
687+
u = &usr
688+
return nil
689+
}, fmt.Sprintf("error reading service principal with AppID '%s', SP ID: %s", applicationID, spId))
652690
if err != nil {
653691
log.Printf("[WARN] error reading service principal with AppID '%s', SP ID: %s", applicationID, spId)
654-
u = scim.User{ApplicationID: nonExistingUserOrSp}
692+
u = &scim.User{ApplicationID: nonExistingUserOrSp}
655693
}
656694
}
657695
ic.spsMutex.Lock()
658696
defer ic.spsMutex.Unlock()
659-
ic.allSps[applicationID] = u
697+
ic.allSps[applicationID] = *u
660698

661699
return
662700
}

0 commit comments

Comments
 (0)