Skip to content

Commit cb2f98b

Browse files
shreyas-goenkashreyas-goenka
authored
Excludes roles in scim API list calls to reduce load on databricks scim service (#2181)
* Exclude roles in scim API list calls * more test fixes
1 parent 424c609 commit cb2f98b

14 files changed

+88
-40
lines changed

‎exporter/exporter_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ func TestImportingUsersGroupsSecretScopes(t *testing.T) {
340340
},
341341
{
342342
Method: "GET",
343-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%20eq%20%27spn%27",
343+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=applicationId%20eq%20%27spn%27",
344344
Response: scim.User{ID: "321", DisplayName: "spn", ApplicationID: "spn",
345345
Groups: []scim.ComplexValue{
346346
{Display: "admins", Value: "a", Ref: "Groups/a", Type: "direct"},
@@ -394,7 +394,7 @@ func TestImportingUsersGroupsSecretScopes(t *testing.T) {
394394
},
395395
{
396396
Method: "GET",
397-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27test%40test.com%27",
397+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27test%40test.com%27",
398398
Response: scim.UserList{
399399
Resources: []scim.User{
400400
{ID: "123", DisplayName: "[email protected]", UserName: "[email protected]"},
@@ -1275,7 +1275,7 @@ func TestImportingUser(t *testing.T) {
12751275
{
12761276
Method: "GET",
12771277
ReuseRequest: true,
1278-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27me%27",
1278+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27me%27",
12791279
Response: scim.UserList{
12801280
Resources: []scim.User{
12811281
{
@@ -1601,7 +1601,7 @@ func TestImportingDLTPipelines(t *testing.T) {
16011601
},
16021602
{
16031603
Method: "GET",
1604-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27user%40domain.com%27",
1604+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27user%40domain.com%27",
16051605
Response: scim.UserList{
16061606
Resources: []scim.User{
16071607
{ID: "123", DisplayName: "[email protected]", UserName: "[email protected]"},

‎exporter/importables_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ func TestUserSearchFails(t *testing.T) {
444444
{
445445
ReuseRequest: true,
446446
Method: "GET",
447-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27dbc%27",
447+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27dbc%27",
448448
Status: 404,
449449
Response: apierr.NotFound("nope"),
450450
},
@@ -473,7 +473,7 @@ func TestSpnSearchFails(t *testing.T) {
473473
{
474474
ReuseRequest: true,
475475
Method: "GET",
476-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%20eq%20%27dbc%27",
476+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=applicationId%20eq%20%27dbc%27",
477477
Status: 404,
478478
Response: apierr.NotFound("nope"),
479479
},
@@ -502,7 +502,7 @@ func TestSpnSearchSuccess(t *testing.T) {
502502
{
503503
ReuseRequest: true,
504504
Method: "GET",
505-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%20eq%20%27dbc%27",
505+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=applicationId%20eq%20%27dbc%27",
506506
Response: scim.UserList{Resources: []scim.User{
507507
{ID: "321", DisplayName: "spn", ApplicationID: "dbc"},
508508
}},
@@ -556,7 +556,7 @@ func TestUserImportSkipNonDirectGroups(t *testing.T) {
556556
{
557557
ReuseRequest: true,
558558
Method: "GET",
559-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27dbc%27",
559+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27dbc%27",
560560
Response: scim.UserList{
561561
Resources: []scim.User{
562562
{

‎exporter/util.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ func (ic *importContext) cacheGroups() error {
200200
if len(ic.allGroups) == 0 {
201201
log.Printf("[INFO] Caching groups in memory ...")
202202
groupsAPI := scim.NewGroupsAPI(ic.Context, ic.Client)
203-
g, err := groupsAPI.Filter("")
203+
g, err := groupsAPI.Filter("", true)
204204
if err != nil {
205205
return err
206206
}

‎scim/data_group_test.go‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func TestDataSourceGroup(t *testing.T) {
1818
Fixtures: []qa.HTTPFixture{
1919
{
2020
Method: "GET",
21-
Resource: "/api/2.0/preview/scim/v2/Groups?filter=displayName%20eq%20%27ds%27",
21+
Resource: "/api/2.0/preview/scim/v2/Groups?excludedAttributes=roles&filter=displayName%20eq%20%27ds%27",
2222
Response: GroupList{
2323
Resources: []Group{
2424
{
@@ -29,11 +29,6 @@ func TestDataSourceGroup(t *testing.T) {
2929
Value: "allow-cluster-create",
3030
},
3131
},
32-
Roles: []ComplexValue{
33-
{
34-
Value: "a",
35-
},
36-
},
3732
Members: []ComplexValue{
3833
{
3934
Ref: "Users/1112",
@@ -57,6 +52,43 @@ func TestDataSourceGroup(t *testing.T) {
5752
},
5853
},
5954
},
55+
{
56+
Method: "GET",
57+
Resource: "/api/2.0/preview/scim/v2/Groups/eerste",
58+
Response: Group{
59+
DisplayName: "ds",
60+
ID: "eerste",
61+
Entitlements: []ComplexValue{
62+
{
63+
Value: "allow-cluster-create",
64+
},
65+
},
66+
Roles: []ComplexValue{
67+
{
68+
Value: "a",
69+
},
70+
},
71+
Members: []ComplexValue{
72+
{
73+
Ref: "Users/1112",
74+
Value: "1112",
75+
},
76+
{
77+
Ref: "ServicePrincipals/1113",
78+
Value: "1113",
79+
},
80+
{
81+
Ref: "Groups/1114",
82+
Value: "1114",
83+
},
84+
},
85+
Groups: []ComplexValue{
86+
{
87+
Value: "abc",
88+
},
89+
},
90+
},
91+
},
6092
{
6193
Method: "GET",
6294
Resource: "/api/2.0/preview/scim/v2/Groups/abc",

‎scim/data_service_principal_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ func TestDataServicePrincipalReadByAppId(t *testing.T) {
1212
Fixtures: []qa.HTTPFixture{
1313
{
1414
Method: "GET",
15-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%20eq%20%27abc%27",
15+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=applicationId%20eq%20%27abc%27",
1616
Response: UserList{
1717
Resources: []User{
1818
{
@@ -56,7 +56,7 @@ func TestDataServicePrincipalReadNotFound(t *testing.T) {
5656
Fixtures: []qa.HTTPFixture{
5757
{
5858
Method: "GET",
59-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%20eq%20%27abc%27",
59+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=applicationId%20eq%20%27abc%27",
6060
Response: UserList{},
6161
},
6262
},
@@ -74,7 +74,7 @@ func TestDataServicePrincipalReadError(t *testing.T) {
7474
Fixtures: []qa.HTTPFixture{
7575
{
7676
Method: "GET",
77-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=applicationId%20eq%20%27abc%27",
77+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=applicationId%20eq%20%27abc%27",
7878
Status: 500,
7979
},
8080
},

‎scim/data_service_principals_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ func TestDataServicePrincipalsReadByDisplayName(t *testing.T) {
1212
Fixtures: []qa.HTTPFixture{
1313
{
1414
Method: "GET",
15-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=displayName%20co%20%27def%27",
15+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=displayName%20co%20%27def%27",
1616
Response: UserList{
1717
Resources: []User{
1818
{
@@ -46,7 +46,7 @@ func TestDataServicePrincipalsReadNotFound(t *testing.T) {
4646
Fixtures: []qa.HTTPFixture{
4747
{
4848
Method: "GET",
49-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=displayName%20co%20%27def%27",
49+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=displayName%20co%20%27def%27",
5050
Response: UserList{},
5151
},
5252
},
@@ -64,7 +64,7 @@ func TestDataServicePrincipalsReadNoFilter(t *testing.T) {
6464
Fixtures: []qa.HTTPFixture{
6565
{
6666
Method: "GET",
67-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?",
67+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles",
6868
Response: UserList{
6969
Resources: []User{
7070
{
@@ -98,7 +98,7 @@ func TestDataServicePrincipalsReadError(t *testing.T) {
9898
Fixtures: []qa.HTTPFixture{
9999
{
100100
Method: "GET",
101-
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?filter=displayName%20co%20%27def%27",
101+
Resource: "/api/2.0/preview/scim/v2/ServicePrincipals?excludedAttributes=roles&filter=displayName%20co%20%27def%27",
102102
Status: 500,
103103
},
104104
},

‎scim/data_user_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func TestDataSourceUser(t *testing.T) {
1616
Fixtures: []qa.HTTPFixture{
1717
{
1818
Method: "GET",
19-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27ds%27",
19+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27ds%27",
2020
Response: UserList{
2121
Resources: []User{
2222
{
@@ -53,15 +53,15 @@ func TestDataSourceUserGerUser(t *testing.T) {
5353
},
5454
{
5555
Method: "GET",
56-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27searching_error%27",
56+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27searching_error%27",
5757
Status: 404,
5858
Response: apierr.APIError{
5959
Message: "searching_error",
6060
},
6161
},
6262
{
6363
Method: "GET",
64-
Resource: "/api/2.0/preview/scim/v2/Users?filter=userName%20eq%20%27empty_search%27",
64+
Resource: "/api/2.0/preview/scim/v2/Users?excludedAttributes=roles&filter=userName%20eq%20%27empty_search%27",
6565
Response: UserList{},
6666
},
6767
}, func(ctx context.Context, client *common.DatabricksClient) {

‎scim/groups.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,27 +39,32 @@ func (a GroupsAPI) Read(groupID string) (group Group, err error) {
3939
}
4040

4141
// Filter returns groups matching the filter
42-
func (a GroupsAPI) Filter(filter string) (GroupList, error) {
42+
func (a GroupsAPI) Filter(filter string, includeRoles bool) (GroupList, error) {
4343
var groups GroupList
4444
req := map[string]string{}
4545
if filter != "" {
4646
req["filter"] = filter
4747
}
48+
if !includeRoles {
49+
// We exclude roles to reduce load on the scim service
50+
req["excludedAttributes"] = "roles"
51+
}
4852
err := a.client.Scim(a.context, http.MethodGet, "/preview/scim/v2/Groups", req, &groups)
4953
return groups, err
5054
}
5155

5256
func (a GroupsAPI) ReadByDisplayName(displayName string) (group Group, err error) {
53-
groupList, err := a.Filter(fmt.Sprintf("displayName eq '%s'", displayName))
57+
groupList, err := a.Filter(fmt.Sprintf("displayName eq '%s'", displayName), false)
5458
if err != nil {
5559
return
5660
}
5761
if len(groupList.Resources) == 0 {
5862
err = fmt.Errorf("cannot find group: %s", displayName)
5963
return
6064
}
61-
group = groupList.Resources[0]
62-
return
65+
// We GET the group again to fetch any fields that were excluded the first
66+
// time around
67+
return a.Read(groupList.Resources[0].ID)
6368
}
6469

6570
func (a GroupsAPI) Patch(groupID string, r patchRequest) error {

‎scim/resource_group_test.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ func TestCreateForceOverwriteCannotListGroups(t *testing.T) {
354354
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
355355
{
356356
Method: "GET",
357-
Resource: "/api/2.0/preview/scim/v2/Groups?filter=displayName%20eq%20%27abc%27",
357+
Resource: "/api/2.0/preview/scim/v2/Groups?excludedAttributes=roles&filter=displayName%20eq%20%27abc%27",
358358
Status: 417,
359359
Response: apierr.APIError{
360360
Message: "cannot find group",
@@ -376,7 +376,7 @@ func TestCreateForceOverwriteFindsAndSetsGroupID(t *testing.T) {
376376
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
377377
{
378378
Method: "GET",
379-
Resource: "/api/2.0/preview/scim/v2/Groups?filter=displayName%20eq%20%27abc%27",
379+
Resource: "/api/2.0/preview/scim/v2/Groups?excludedAttributes=roles&filter=displayName%20eq%20%27abc%27",
380380
Response: GroupList{
381381
Resources: []Group{
382382
{
@@ -392,6 +392,13 @@ func TestCreateForceOverwriteFindsAndSetsGroupID(t *testing.T) {
392392
ID: "123",
393393
},
394394
},
395+
{
396+
Method: "GET",
397+
Resource: "/api/2.0/preview/scim/v2/Groups/123",
398+
Response: Group{
399+
ID: "123",
400+
},
401+
},
395402
{
396403
Method: "PUT",
397404
Resource: "/api/2.0/preview/scim/v2/Groups/123",

‎scim/resource_service_principal.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ func (a ServicePrincipalsAPI) Filter(filter string) (u []User, err error) {
4545
if filter != "" {
4646
req["filter"] = filter
4747
}
48+
// We exclude roles to reduce load on the scim service
49+
req["excludedAttributes"] = "roles"
4850
err = a.client.Scim(a.context, http.MethodGet, "/preview/scim/v2/ServicePrincipals", req, &sps)
4951
if err != nil {
5052
return

0 commit comments

Comments
 (0)