Skip to content

Commit

Permalink
Explicitly include SCIM attributes for databricks_group, `databrick…
Browse files Browse the repository at this point in the history
…s_user`, `databricks_user_role`, `databricks_group_role`, `databricks_group_member`, `databricks_group_instance_profile`, `databricks_user` data, `databricks_group` data, and `databricks_entitlement` resources (#2200)

Reduce the possibility of platform errors
  • Loading branch information
nfx committed Apr 6, 2023
1 parent e0e641b commit 4709988
Show file tree
Hide file tree
Showing 28 changed files with 184 additions and 94 deletions.
2 changes: 1 addition & 1 deletion aws/resource_group_instance_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func ResourceGroupInstanceProfile() *schema.Resource {
return m
}).BindResource(common.BindResource{
ReadContext: func(ctx context.Context, groupID, roleARN string, c *common.DatabricksClient) error {
group, err := scim.NewGroupsAPI(ctx, c).Read(groupID)
group, err := scim.NewGroupsAPI(ctx, c).Read(groupID, "roles")
hasRole := scim.ComplexValues(group.Roles).HasValue(roleARN)
if err == nil && !hasRole {
return apierr.NotFound("Group has no instance profile")
Expand Down
10 changes: 5 additions & 5 deletions aws/resource_group_instance_profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestResourceGroupInstanceProfileCreate(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/abc",
Resource: "/api/2.0/preview/scim/v2/Groups/abc?attributes=roles",
Response: scim.Group{
Schemas: []scim.URN{"urn:ietf:params:scim:schemas:core:2.0:Group"},
DisplayName: "Data Scientists",
Expand Down Expand Up @@ -125,7 +125,7 @@ func TestResourceGroupInstanceProfileRead(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/abc",
Resource: "/api/2.0/preview/scim/v2/Groups/abc?attributes=roles",
Response: scim.Group{
Schemas: []scim.URN{"urn:ietf:params:scim:schemas:core:2.0:Group"},
DisplayName: "Data Scientists",
Expand All @@ -151,7 +151,7 @@ func TestResourceGroupInstanceProfileRead_NotFound(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/abc",
Resource: "/api/2.0/preview/scim/v2/Groups/abc?attributes=roles",
Response: apierr.APIErrorBody{
ErrorCode: "NOT_FOUND",
Message: "Item not found",
Expand All @@ -171,7 +171,7 @@ func TestResourceGroupInstanceProfileRead_NotFound_Role(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/abc",
Resource: "/api/2.0/preview/scim/v2/Groups/abc?attributes=roles",
Response: scim.Group{
Schemas: []scim.URN{"urn:ietf:params:scim:schemas:core:2.0:Group"},
DisplayName: "Data Scientists",
Expand All @@ -191,7 +191,7 @@ func TestResourceGroupInstanceProfileRead_Error(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/abc",
Resource: "/api/2.0/preview/scim/v2/Groups/abc?attributes=roles",
Response: apierr.APIErrorBody{
ErrorCode: "INVALID_REQUEST",
Message: "Internal error happened",
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_user_instance_profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func ResourceUserInstanceProfile() *schema.Resource {
return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequest("add", "roles", roleARN))
},
ReadContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
user, err := scim.NewUsersAPI(ctx, c).Read(userID)
user, err := scim.NewUsersAPI(ctx, c).Read(userID, "roles")
hasRole := scim.ComplexValues(user.Roles).HasValue(roleARN)
if err == nil && !hasRole {
return apierr.NotFound("User has no role")
Expand Down
10 changes: 5 additions & 5 deletions aws/resource_user_instance_profile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestResourceUserInstanceProfileCreate(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/abc",
Resource: "/api/2.0/preview/scim/v2/Users/abc?attributes=roles",
Response: scim.User{
Schemas: []scim.URN{"urn:ietf:params:scim:schemas:core:2.0:User"},
DisplayName: "Data Scientists",
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestResourceUserInstanceProfileRead(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/abc",
Resource: "/api/2.0/preview/scim/v2/Users/abc?attributes=roles",
Response: scim.User{
Schemas: []scim.URN{"urn:ietf:params:scim:schemas:core:2.0:User"},
DisplayName: "Data Scientists",
Expand All @@ -117,7 +117,7 @@ func TestResourceUserInstanceProfileRead_NoRole(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/abc",
Resource: "/api/2.0/preview/scim/v2/Users/abc?attributes=roles",
Response: scim.User{
Schemas: []scim.URN{"urn:ietf:params:scim:schemas:core:2.0:User"},
DisplayName: "Data Scientists",
Expand All @@ -137,7 +137,7 @@ func TestResourceUserInstanceProfileRead_NotFound(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/abc",
Resource: "/api/2.0/preview/scim/v2/Users/abc?attributes=roles",
Response: apierr.APIErrorBody{
ErrorCode: "NOT_FOUND",
Message: "Item not found",
Expand All @@ -157,7 +157,7 @@ func TestResourceUserInstanceProfileRead_Error(t *testing.T) {
Fixtures: []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/abc",
Resource: "/api/2.0/preview/scim/v2/Users/abc?attributes=roles",
Response: apierr.APIErrorBody{
ErrorCode: "INVALID_REQUEST",
Message: "Internal error happened",
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_user_role.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func ResourceUserRole() *schema.Resource {
return scim.NewUsersAPI(ctx, c).Patch(userID, scim.PatchRequest("add", "roles", role))
},
ReadContext: func(ctx context.Context, userID, roleARN string, c *common.DatabricksClient) error {
user, err := scim.NewUsersAPI(ctx, c).Read(userID)
user, err := scim.NewUsersAPI(ctx, c).Read(userID, "roles")
hasRole := scim.ComplexValues(user.Roles).HasValue(roleARN)
if err == nil && !hasRole {
return apierr.NotFound("User has no role")
Expand Down
2 changes: 1 addition & 1 deletion aws/resource_user_role_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestUserRoleCreate_AndGetResourceDrift(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/a",
Resource: "/api/2.0/preview/scim/v2/Users/a?attributes=roles",
Response: scim.User{},
},
},
Expand Down
24 changes: 18 additions & 6 deletions exporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ func TestImportingUsersGroupsSecretScopes(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/a",
Resource: "/api/2.0/preview/scim/v2/Groups/a?attributes=members",
Response: scim.Group{ID: "a", DisplayName: "admins",
Members: []scim.ComplexValue{
{Display: "test@test.com", Value: "123", Ref: "Users/123"},
Expand All @@ -369,12 +369,24 @@ func TestImportingUsersGroupsSecretScopes(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/b",
Resource: "/api/2.0/preview/scim/v2/Groups/a?attributes=displayName,externalId,entitlements",
Response: scim.Group{ID: "a", DisplayName: "admins",
Members: []scim.ComplexValue{
{Display: "test@test.com", Value: "123", Ref: "Users/123"},
{Display: "Test group", Value: "f", Ref: "Groups/f"},
{Display: "spn", Value: "spn", Ref: "ServicePrincipals/spn"},
},
},
ReuseRequest: true,
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/b?attributes=displayName,externalId,entitlements",
Response: scim.Group{ID: "b", DisplayName: "users"},
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/c",
Resource: "/api/2.0/preview/scim/v2/Groups/c?attributes=displayName,externalId,entitlements",
Response: scim.Group{ID: "c", DisplayName: "test",
Groups: []scim.ComplexValue{
{Display: "admins", Value: "a", Ref: "Groups/a", Type: "direct"},
Expand All @@ -383,13 +395,13 @@ func TestImportingUsersGroupsSecretScopes(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/f",
Resource: "/api/2.0/preview/scim/v2/Groups/f?attributes=displayName,externalId,entitlements",
Response: scim.Group{ID: "f", DisplayName: "nested"},
},
// TODO: add groups to the output
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/123",
Resource: "/api/2.0/preview/scim/v2/Users/123?attributes=userName,displayName,active,externalID,entitlements",
Response: scim.User{ID: "123", DisplayName: "test@test.com", UserName: "test@test.com"},
},
{
Expand Down Expand Up @@ -1614,7 +1626,7 @@ func TestImportingDLTPipelines(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/123",
Resource: "/api/2.0/preview/scim/v2/Users/123?attributes=userName,displayName,active,externalID,entitlements",
Response: scim.User{ID: "123", DisplayName: "user@domain.com", UserName: "user@domain.com"},
},
{
Expand Down
40 changes: 40 additions & 0 deletions internal/acceptance/data_user_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package acceptance

import (
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const userDataSourceTemplate = `
resource "databricks_user" "this" {
user_name = "tf-{var.RANDOM}@example.com"
}
data "databricks_user" "this" {
user_name = databricks_user.this.user_name
}`

func checkUserDataSourcePopulated(t *testing.T) func(s *terraform.State) error {
return func(s *terraform.State) error {
r, ok := s.Modules[0].Resources["data.databricks_user.this"]
require.True(t, ok, "data.databricks_user.this has to be there")
assert.Equal(t, s.Modules[0].Resources["databricks_user.this"].Primary.ID, r.Primary.ID)
return nil
}
}

func TestMwsAccUserData(t *testing.T) {
accountLevel(t, step{
Template: userDataSourceTemplate,
Check: checkUserDataSourcePopulated(t),
})
}

func TestAccUserData(t *testing.T) {
workspaceLevel(t, step{
Template: userDataSourceTemplate,
Check: checkUserDataSourcePopulated(t),
})
}
4 changes: 2 additions & 2 deletions internal/acceptance/group_member_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestMwsAccGroupMemberResource(t *testing.T) {
accountLevel(t, step{
Template: groupMemberTest,
Callback: func(ctx context.Context, client *common.DatabricksClient, id string) error {
g, err := scim.NewGroupsAPI(ctx, client).Read(id)
g, err := scim.NewGroupsAPI(ctx, client).Read(id, "members")
if err != nil {
return err
}
Expand All @@ -46,7 +46,7 @@ func TestAccGroupMemberResource(t *testing.T) {
workspaceLevel(t, step{
Template: groupMemberTest,
Callback: func(ctx context.Context, client *common.DatabricksClient, id string) error {
g, err := scim.NewGroupsAPI(ctx, client).Read(id)
g, err := scim.NewGroupsAPI(ctx, client).Read(id, "members")
if err != nil {
return err
}
Expand Down
18 changes: 18 additions & 0 deletions internal/acceptance/group_role_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package acceptance

import (
"testing"
)

func TestAccGroupRole(t *testing.T) {
workspaceLevel(t, step{
Template: `
resource "databricks_group" "this" {
display_name = "tf-{var.RANDOM}"
}
resource "databricks_group_role" "this" {
group_id = databricks_group.this.id
role = "arn:aws:iam::999999999999:role/foo"
}`,
})
}
4 changes: 2 additions & 2 deletions internal/acceptance/group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func TestMwsAccGroupsExternalIdAndScimProvisioning(t *testing.T) {
// duplicate code between workspace level and account level, because clients
// might get different
groupsAPI := scim.NewGroupsAPI(ctx, client)
group, err := groupsAPI.Read(id)
group, err := groupsAPI.Read(id, "displayName,entitlements")
if err != nil {
return err
}
Expand Down Expand Up @@ -52,7 +52,7 @@ func TestAccGroupsExternalIdAndScimProvisioning(t *testing.T) {
resourceCheck("databricks_group.this",
func(ctx context.Context, client *common.DatabricksClient, id string) error {
groupsAPI := scim.NewGroupsAPI(ctx, client)
group, err := groupsAPI.Read(id)
group, err := groupsAPI.Read(id, "displayName,entitlements")
if err != nil {
return err
}
Expand Down
18 changes: 18 additions & 0 deletions internal/acceptance/user_role_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package acceptance

import (
"testing"
)

func TestAccUserRole(t *testing.T) {
workspaceLevel(t, step{
Template: `
resource "databricks_user" "this" {
user_name = "{var.RANDOM}@example.com"
}
resource "databricks_user_role" "this" {
user_id = databricks_user.this.id
role = "arn:aws:iam::999999999999:role/foo"
}`,
})
}
5 changes: 3 additions & 2 deletions scim/data_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ func DataSourceGroup() *schema.Resource {
var this entity
common.DataToStructPointer(d, s, &this)
groupsAPI := NewGroupsAPI(ctx, m)
group, err := groupsAPI.ReadByDisplayName(this.DisplayName)
groupAttributes := "members,roles,entitlements,externalId"
group, err := groupsAPI.ReadByDisplayName(this.DisplayName, groupAttributes)
if err != nil {
return diag.FromErr(err)
}
Expand Down Expand Up @@ -69,7 +70,7 @@ func DataSourceGroup() *schema.Resource {
for _, x := range current.Groups {
this.Groups = append(this.Groups, x.Value)
if this.Recursive {
childGroup, err := groupsAPI.Read(x.Value)
childGroup, err := groupsAPI.Read(x.Value, groupAttributes)
if err != nil {
return diag.FromErr(err)
}
Expand Down
4 changes: 2 additions & 2 deletions scim/data_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestDataSourceGroup(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/eerste",
Resource: "/api/2.0/preview/scim/v2/Groups/eerste?attributes=members,roles,entitlements,externalId",
Response: Group{
DisplayName: "ds",
ID: "eerste",
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestDataSourceGroup(t *testing.T) {
},
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Groups/abc",
Resource: "/api/2.0/preview/scim/v2/Groups/abc?attributes=members,roles,entitlements,externalId",
Response: Group{
DisplayName: "product",
ID: "abc",
Expand Down
2 changes: 1 addition & 1 deletion scim/data_user.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

func getUser(usersAPI UsersAPI, id, name string) (user User, err error) {
if id != "" {
return usersAPI.Read(id)
return usersAPI.Read(id, "userName,displayName,externalId,applicationId")
}
userList, err := usersAPI.Filter(fmt.Sprintf("userName eq '%s'", name))
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion scim/data_user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestDataSourceUserGerUser(t *testing.T) {
qa.HTTPFixturesApply(t, []qa.HTTPFixture{
{
Method: "GET",
Resource: "/api/2.0/preview/scim/v2/Users/a",
Resource: "/api/2.0/preview/scim/v2/Users/a?attributes=userName,displayName,externalId,applicationId",
Response: User{
ID: "a",
},
Expand Down
11 changes: 6 additions & 5 deletions scim/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ func (a GroupsAPI) Create(scimGroupRequest Group) (group Group, err error) {
}

// Read reads and returns a Group object via SCIM api
func (a GroupsAPI) Read(groupID string) (group Group, err error) {
err = a.client.Scim(a.context, http.MethodGet, fmt.Sprintf("/preview/scim/v2/Groups/%v", groupID), nil, &group)
func (a GroupsAPI) Read(groupID, attributes string) (group Group, err error) {
err = a.client.Scim(a.context, http.MethodGet, fmt.Sprintf(
"/preview/scim/v2/Groups/%v?attributes=%s", groupID, attributes), nil, &group)
if err != nil {
return
}
Expand All @@ -53,7 +54,7 @@ func (a GroupsAPI) Filter(filter string, includeRoles bool) (GroupList, error) {
return groups, err
}

func (a GroupsAPI) ReadByDisplayName(displayName string) (group Group, err error) {
func (a GroupsAPI) ReadByDisplayName(displayName, attributes string) (group Group, err error) {
groupList, err := a.Filter(fmt.Sprintf("displayName eq '%s'", displayName), false)
if err != nil {
return
Expand All @@ -64,15 +65,15 @@ func (a GroupsAPI) ReadByDisplayName(displayName string) (group Group, err error
}
// We GET the group again to fetch any fields that were excluded the first
// time around
return a.Read(groupList.Resources[0].ID)
return a.Read(groupList.Resources[0].ID, attributes)
}

func (a GroupsAPI) Patch(groupID string, r patchRequest) error {
return a.client.Scim(a.context, http.MethodPatch, fmt.Sprintf("/preview/scim/v2/Groups/%v", groupID), r, nil)
}

func (a GroupsAPI) UpdateNameAndEntitlements(groupID string, name string, externalID string, e entitlements) error {
g, err := a.Read(groupID)
g, err := a.Read(groupID, "displayName,entitlements,groups,members,externalId")
if err != nil {
return err
}
Expand Down

0 comments on commit 4709988

Please sign in to comment.