From 05f70c126d2153b627d911cd59b7d56f2071bc3d Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 13 Sep 2024 15:58:02 -0600 Subject: [PATCH 01/27] #3468: added initial data_users.go resource --- scim/data_users.go | 52 +++++++++++++++++++++++++++++++++++++++++ scim/data_users_test.go | 29 +++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100755 scim/data_users.go create mode 100755 scim/data_users_test.go diff --git a/scim/data_users.go b/scim/data_users.go new file mode 100755 index 0000000000..f3a8b873bc --- /dev/null +++ b/scim/data_users.go @@ -0,0 +1,52 @@ +package scim + +import ( + "context" + "fmt" + + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/service/iam" + "github.com/databricks/terraform-provider-databricks/common" +) + +func DataSourceDataUsers() common.Resource { + + type DataUsers struct { + Id string `json:"id,omitempty" tf:"computed"` + DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` + Users []map[string]interface{} `json:"users,omitempty" tf:"computed"` + } + + return common.AccountData(func(ctx context.Context, data *DataUsers, acc *databricks.AccountClient) error { + listRequest := iam.ListAccountUsersRequest{} + + if data.DisplayNameContains != "" { + listRequest.Filter = fmt.Sprintf("displayName co \"%s\"", data.DisplayNameContains) + } + + userList, err := acc.Users.ListAll(ctx, listRequest) + + if err != nil { + return err + } + + if len(userList) == 0 { + return fmt.Errorf("cannot find users with display name containing %s", data.DisplayNameContains) + } + + var users []map[string]interface{} + + for _, u := range userList { + user := map[string]interface{}{ + "id": u.Id, + "user_name": u.UserName, + "display_name": u.DisplayName, + } + users = append(users, user) + } + + data.Users = users + + return nil + }) +} diff --git a/scim/data_users_test.go b/scim/data_users_test.go new file mode 100755 index 0000000000..0d45fa9a0c --- /dev/null +++ b/scim/data_users_test.go @@ -0,0 +1,29 @@ +package scim + +import ( + "testing" + + "github.com/databricks/terraform-provider-databricks/qa" +) + +func TestDataSourceDataUsers(t *testing.T) { + qa.ResourceFixture{ + Fixtures: []qa.HTTPFixture{ + // TODO: run this test to get fixtures + }, + Resource: DataSourceDataUsers(), + Read: true, + NonWritable: true, + ID: "_", + }.ApplyNoError(t) +} + +func TestCatalogsData_Error(t *testing.T) { + qa.ResourceFixture{ + Fixtures: qa.HTTPFailures, + Resource: DataSourceDataUsers(), + Read: true, + NonWritable: true, + ID: "_", + }.ExpectError(t, "i'm a teapot") +} From f2dd1c2bf57e436d3d8f8358d251901f49051878 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Mon, 16 Sep 2024 17:16:13 -0600 Subject: [PATCH 02/27] #3468: made changes to data_users.go to explicitly define schema, and only request the id, userName, and displayName attributes. added succesful initial test_case on data_users_test.go --- scim/data_users.go | 25 ++++++++++++++++--------- scim/data_users_test.go | 37 ++++++++++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/scim/data_users.go b/scim/data_users.go index f3a8b873bc..576ed2ebb8 100755 --- a/scim/data_users.go +++ b/scim/data_users.go @@ -11,14 +11,21 @@ import ( func DataSourceDataUsers() common.Resource { + type UserInfo struct { + Id string `json:"id,omitempty" tf:"computed"` + UserName string `json:"user_name,omitempty" tf:"computed"` + DisplayName string `json:"display_name,omitempty" tf:"computed"` + } + type DataUsers struct { - Id string `json:"id,omitempty" tf:"computed"` - DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` - Users []map[string]interface{} `json:"users,omitempty" tf:"computed"` + DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` + Users []UserInfo `json:"users,omitempty" tf:"computed"` } return common.AccountData(func(ctx context.Context, data *DataUsers, acc *databricks.AccountClient) error { - listRequest := iam.ListAccountUsersRequest{} + listRequest := iam.ListAccountUsersRequest{ + Attributes: "id,userName,displayName", + } if data.DisplayNameContains != "" { listRequest.Filter = fmt.Sprintf("displayName co \"%s\"", data.DisplayNameContains) @@ -34,13 +41,13 @@ func DataSourceDataUsers() common.Resource { return fmt.Errorf("cannot find users with display name containing %s", data.DisplayNameContains) } - var users []map[string]interface{} + var users []UserInfo for _, u := range userList { - user := map[string]interface{}{ - "id": u.Id, - "user_name": u.UserName, - "display_name": u.DisplayName, + user := UserInfo{ + Id: u.Id, + UserName: u.UserName, + DisplayName: u.DisplayName, } users = append(users, user) } diff --git a/scim/data_users_test.go b/scim/data_users_test.go index 0d45fa9a0c..13fecdc47e 100755 --- a/scim/data_users_test.go +++ b/scim/data_users_test.go @@ -3,19 +3,50 @@ package scim import ( "testing" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/terraform-provider-databricks/qa" + "github.com/stretchr/testify/mock" ) func TestDataSourceDataUsers(t *testing.T) { qa.ResourceFixture{ - Fixtures: []qa.HTTPFixture{ - // TODO: run this test to get fixtures + MockAccountClientFunc: func(m *mocks.MockAccountClient) { + e := m.GetMockAccountUsersAPI() + e.On("ListAll", + mock.Anything, + mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { + return req.Filter == `displayName co "testuser"` && + req.Attributes == "id,userName,displayName" + })).Return([]iam.User{ + { + Id: "123", + UserName: "testuser@example.com", + DisplayName: "testuser", + }, + { + Id: "456", + UserName: "testuser2@example.com", + DisplayName: "testuser2", + }, + }, nil) }, Resource: DataSourceDataUsers(), Read: true, NonWritable: true, ID: "_", - }.ApplyNoError(t) + HCL: ` + display_name_contains = "testuser" + `, + }.ApplyAndExpectData(t, map[string]any{ + "users.#": 2, + "users.0.id": "123", + "users.0.user_name": "testuser@example.com", + "users.0.display_name": "testuser", + "users.1.id": "456", + "users.1.user_name": "testuser2@example.com", + "users.1.display_name": "testuser2", + }) } func TestCatalogsData_Error(t *testing.T) { From 2d594a9355b7658562acda2a62df7d0fb99cdc4e Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Tue, 17 Sep 2024 15:57:19 -0600 Subject: [PATCH 03/27] #3468: added 'user_name_contains' attribute to data_users.go to allow filtering by userName, and added additional test cases on data_users_test.go --- scim/data_users.go | 17 ++++- scim/data_users_test.go | 136 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 147 insertions(+), 6 deletions(-) diff --git a/scim/data_users.go b/scim/data_users.go index 576ed2ebb8..9c1b3783cf 100755 --- a/scim/data_users.go +++ b/scim/data_users.go @@ -19,6 +19,7 @@ func DataSourceDataUsers() common.Resource { type DataUsers struct { DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` + UserNameContains string `json:"user_name_contains,omitempty" tf:"computed"` Users []UserInfo `json:"users,omitempty" tf:"computed"` } @@ -27,7 +28,13 @@ func DataSourceDataUsers() common.Resource { Attributes: "id,userName,displayName", } - if data.DisplayNameContains != "" { + if data.DisplayNameContains != "" && data.UserNameContains != "" { + return fmt.Errorf("exactly one of display_name_contains or user_name_contains should be specified, not both") + } + + if data.UserNameContains != "" { + listRequest.Filter = fmt.Sprintf("userName co \"%s\"", data.UserNameContains) + } else if data.DisplayNameContains != "" { listRequest.Filter = fmt.Sprintf("displayName co \"%s\"", data.DisplayNameContains) } @@ -38,7 +45,13 @@ func DataSourceDataUsers() common.Resource { } if len(userList) == 0 { - return fmt.Errorf("cannot find users with display name containing %s", data.DisplayNameContains) + if data.DisplayNameContains != "" { + return fmt.Errorf("cannot find users with display name containing %s", data.DisplayNameContains) + } else if data.UserNameContains != "" { + return fmt.Errorf("cannot find users with username containing %s", data.UserNameContains) + } else { + return fmt.Errorf("no users found") + } } var users []UserInfo diff --git a/scim/data_users_test.go b/scim/data_users_test.go index 13fecdc47e..5c19f8342c 100755 --- a/scim/data_users_test.go +++ b/scim/data_users_test.go @@ -1,6 +1,7 @@ package scim import ( + "fmt" "testing" "github.com/databricks/databricks-sdk-go/experimental/mocks" @@ -9,7 +10,7 @@ import ( "github.com/stretchr/testify/mock" ) -func TestDataSourceDataUsers(t *testing.T) { +func TestDataSourceDataUsers_DisplayNameContains(t *testing.T) { qa.ResourceFixture{ MockAccountClientFunc: func(m *mocks.MockAccountClient) { e := m.GetMockAccountUsersAPI() @@ -49,12 +50,139 @@ func TestDataSourceDataUsers(t *testing.T) { }) } -func TestCatalogsData_Error(t *testing.T) { +func TestDataSourceDataUsers_UserNameContains(t *testing.T) { qa.ResourceFixture{ - Fixtures: qa.HTTPFailures, + MockAccountClientFunc: func(m *mocks.MockAccountClient) { + e := m.GetMockAccountUsersAPI() + e.On("ListAll", + mock.Anything, + mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { + return req.Filter == `userName co "testuser"` && + req.Attributes == "id,userName,displayName" + })).Return([]iam.User{ + { + Id: "123", + UserName: "testuser@example.com", + DisplayName: "testuser", + }, + { + Id: "456", + UserName: "testuser2@example.com", + DisplayName: "testuser2", + }, + }, nil) + }, + Resource: DataSourceDataUsers(), + Read: true, + NonWritable: true, + ID: "_", + HCL: ` + user_name_contains = "testuser" + `, + }.ApplyAndExpectData(t, map[string]any{ + "users.#": 2, + "users.0.id": "123", + "users.0.user_name": "testuser@example.com", + "users.0.display_name": "testuser", + "users.1.id": "456", + "users.1.user_name": "testuser2@example.com", + "users.1.display_name": "testuser2", + }) +} + +func TestDataSourceDataUsers_BothFiltersSpecified(t *testing.T) { + qa.ResourceFixture{ + Resource: DataSourceDataUsers(), + Read: true, + NonWritable: true, + ID: "_", + HCL: ` + display_name_contains = "user" + user_name_contains = "example.com" + `, + }.ExpectError(t, "exactly one of display_name_contains or user_name_contains should be specified, not both") +} + +func TestDataSourceDataUsersNoUsers(t *testing.T) { + qa.ResourceFixture{ + MockAccountClientFunc: func(m *mocks.MockAccountClient) { + e := m.GetMockAccountUsersAPI() + e.On("ListAll", + mock.Anything, + mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { + return req.Filter == `displayName co "testuser"` && + req.Attributes == "id,userName,displayName" + })).Return([]iam.User{}, nil) + }, Resource: DataSourceDataUsers(), Read: true, NonWritable: true, ID: "_", - }.ExpectError(t, "i'm a teapot") + HCL: ` + display_name_contains = "testuser" + `, + }.ExpectError(t, "cannot find users with display name containing testuser") +} + +func TestDataSourceDataUsersNoFilter(t *testing.T) { + qa.ResourceFixture{ + MockAccountClientFunc: func(m *mocks.MockAccountClient) { + e := m.GetMockAccountUsersAPI() + e.On("ListAll", + mock.Anything, + mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { + return req.Attributes == "id,userName,displayName" + })).Return([]iam.User{ + { + Id: "123", + UserName: "testuser@example.com", + DisplayName: "testuser", + }, + { + Id: "456", + UserName: "testuser2@example.com", + DisplayName: "testuser2", + }, + { + Id: "789", + UserName: "testuser3@example.com", + DisplayName: "testuser3", + }, + }, nil) + }, + Resource: DataSourceDataUsers(), + Read: true, + NonWritable: true, + ID: "_", + }.ApplyAndExpectData(t, map[string]any{ + "users.#": 3, + "users.0.id": "123", + "users.0.user_name": "testuser@example.com", + "users.0.display_name": "testuser", + "users.1.id": "456", + "users.1.user_name": "testuser2@example.com", + "users.1.display_name": "testuser2", + "users.2.id": "789", + "users.2.user_name": "testuser3@example.com", + "users.2.display_name": "testuser3", + }) +} + +func TestDataSourceDataUsers_APIError(t *testing.T) { + qa.ResourceFixture{ + MockAccountClientFunc: func(m *mocks.MockAccountClient) { + usersAPI := m.GetMockAccountUsersAPI() + usersAPI.On("ListAll", + mock.Anything, + mock.Anything, + ).Return(nil, fmt.Errorf("api error")) + }, + Resource: DataSourceDataUsers(), + Read: true, + NonWritable: true, + ID: "_", + HCL: ` + display_name_contains = "testuser" + `, + }.ExpectError(t, "api error") } From 0d6f280c6e57b507e633654fda0ea59c252f2d6f Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Tue, 17 Sep 2024 16:24:46 -0600 Subject: [PATCH 04/27] #3468: added acceptance tests for data_users --- internal/acceptance/data_users_test.go | 67 ++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 internal/acceptance/data_users_test.go diff --git a/internal/acceptance/data_users_test.go b/internal/acceptance/data_users_test.go new file mode 100644 index 0000000000..9c624e28d2 --- /dev/null +++ b/internal/acceptance/data_users_test.go @@ -0,0 +1,67 @@ +package acceptance + +import ( + "fmt" + "testing" + + "github.com/hashicorp/terraform-plugin-testing/terraform" +) + +func TestAccDataSourceDataUsers_DisplayNameContains(t *testing.T) { + accountLevel(t, step{ + Template: ` + data "databricks_data_users" "this" { + display_name_contains = "testuser" + }`, + Check: func(s *terraform.State) error { + r, ok := s.RootModule().Resources["data.databricks_data_users.this"] + if !ok { + return fmt.Errorf("data not found in state") + } + ids := r.Primary.Attributes["users.#"] + if ids == "" { + return fmt.Errorf("users is empty: %v", r.Primary.Attributes) + } + return nil + }, + }) +} + +func TestAccDataSourceDataUsers_UserNameContains(t *testing.T) { + accountLevel(t, step{ + Template: ` + data "databricks_data_users" "this" { + user_name_contains = "example.com" + }`, + Check: func(s *terraform.State) error { + r, ok := s.RootModule().Resources["data.databricks_data_users.this"] + if !ok { + return fmt.Errorf("data not found in state") + } + usersCount := r.Primary.Attributes["users.#"] + if usersCount == "" || usersCount == "0" { + return fmt.Errorf("users list is empty: %v", r.Primary.Attributes) + } + return nil + }, + }) +} + +func TestAccDataSourceDataUsers_NoFilters(t *testing.T) { + accountLevel(t, step{ + Template: ` + data "databricks_data_users" "this" { + }`, + Check: func(s *terraform.State) error { + r, ok := s.RootModule().Resources["data.databricks_data_users.this"] + if !ok { + return fmt.Errorf("data not found in state") + } + usersCount := r.Primary.Attributes["users.#"] + if usersCount == "" || usersCount == "0" { + return fmt.Errorf("users list is empty: %v", r.Primary.Attributes) + } + return nil + }, + }) +} From b6380f80b9766d7f8ac3af5542f2f09b6fcdff11 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Tue, 17 Sep 2024 16:49:09 -0600 Subject: [PATCH 05/27] #3468: added documentation for databricks_users data source. --- docs/data-sources/users.md | 63 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 docs/data-sources/users.md diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md new file mode 100644 index 0000000000..b3c2cbcd96 --- /dev/null +++ b/docs/data-sources/users.md @@ -0,0 +1,63 @@ +--- +subcategory: "Security" +--- + +# databricks_users Data Source + +->**Note** This is an account-level data source. + +-> **Note** If you have a fully automated setup with workspaces created by [databricks_mws_workspaces](../resources/mws_workspaces.md) or [azurerm_databricks_workspace](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace), please make sure to add [depends_on attribute](../guides/troubleshooting.md#data-resources-and-authentication-is-not-configured-errors) in order to prevent _default auth: cannot configure default credentials_ errors. + +Retrieves information about multiple [databricks_user](../resources/user.md) resources. + +## Example Usage + +Adding a subset of users to a group + +```hcl +data "databricks_users" "company_users" { + user_name_contains = "@domain.org" +} + +resource "databricks_group" "data_users_group" { + display_name = "Data Users" +} + +resource "databricks_group_member" "add_users_to_group" { + for_each = { for user in data.databricks_users.company_users.users : user.id => user } + group_id = databricks_group.data_users_group.id + member_id = each.value.id +} +``` + +## Argument Reference + +This data source allows you to filter the list of users using the following optional arguments: + +- `display_name_contains` - (Optional) A substring to filter users by their display name. Only users whose display names contain this substring will be included in the results. +- `user_name_contains` - (Optional) A substring to filter users by their username. Only users whose usernames contain this substring will be included in the results. + +->**Note** You can specify **exactly one** of `display_name_contains` or `user_name_contains`. If neither is specified, all users will be returned. + +## Attribute Reference + +This data source exposes the following attributes: + +- `users` - A list of users matching the specified criteria. Each user has the following attributes: + - `id` - The ID of the user. + - `user_name` - The username of the user. + - `display_name` - The display name of the user. + +## Related Resources + +The following resources are used in the same context: + +- [**databricks_user**](../resources/user.md): Resource to manage individual users in Databricks. + +- [**databricks_group**](../resources/group.md): Resource to manage groups in Databricks. + +- [**databricks_group_member**](../resources/group_member.md): Resource to manage group memberships by adding users to groups. + +- [**databricks_permissions**](../resources/permissions.md): Resource to manage access control in the Databricks workspace. + +- [**databricks_current_user**](current_user.md): Data source to retrieve information about the user or service principal that is calling the Databricks REST API. From 1c703b430f88a2accbdab48a8f4e234b05414435 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 20 Sep 2024 10:07:31 -0600 Subject: [PATCH 06/27] renamed resource func to DataSourceUsers, removed various acceptance tests, and added the resource definition into the provider --- internal/acceptance/data_users_test.go | 41 +------------------------- internal/providers/sdkv2/sdkv2.go | 1 + scim/data_users.go | 2 +- scim/data_users_test.go | 24 +++++++-------- 4 files changed, 15 insertions(+), 53 deletions(-) diff --git a/internal/acceptance/data_users_test.go b/internal/acceptance/data_users_test.go index 9c624e28d2..0107203bc9 100644 --- a/internal/acceptance/data_users_test.go +++ b/internal/acceptance/data_users_test.go @@ -7,7 +7,7 @@ import ( "github.com/hashicorp/terraform-plugin-testing/terraform" ) -func TestAccDataSourceDataUsers_DisplayNameContains(t *testing.T) { +func TestAccDataSourceDataUsers(t *testing.T) { accountLevel(t, step{ Template: ` data "databricks_data_users" "this" { @@ -26,42 +26,3 @@ func TestAccDataSourceDataUsers_DisplayNameContains(t *testing.T) { }, }) } - -func TestAccDataSourceDataUsers_UserNameContains(t *testing.T) { - accountLevel(t, step{ - Template: ` - data "databricks_data_users" "this" { - user_name_contains = "example.com" - }`, - Check: func(s *terraform.State) error { - r, ok := s.RootModule().Resources["data.databricks_data_users.this"] - if !ok { - return fmt.Errorf("data not found in state") - } - usersCount := r.Primary.Attributes["users.#"] - if usersCount == "" || usersCount == "0" { - return fmt.Errorf("users list is empty: %v", r.Primary.Attributes) - } - return nil - }, - }) -} - -func TestAccDataSourceDataUsers_NoFilters(t *testing.T) { - accountLevel(t, step{ - Template: ` - data "databricks_data_users" "this" { - }`, - Check: func(s *terraform.State) error { - r, ok := s.RootModule().Resources["data.databricks_data_users.this"] - if !ok { - return fmt.Errorf("data not found in state") - } - usersCount := r.Primary.Attributes["users.#"] - if usersCount == "" || usersCount == "0" { - return fmt.Errorf("users list is empty: %v", r.Primary.Attributes) - } - return nil - }, - }) -} diff --git a/internal/providers/sdkv2/sdkv2.go b/internal/providers/sdkv2/sdkv2.go index 8136901ddf..40ba5d3d1a 100644 --- a/internal/providers/sdkv2/sdkv2.go +++ b/internal/providers/sdkv2/sdkv2.go @@ -125,6 +125,7 @@ func DatabricksProvider() *schema.Provider { "databricks_volume": catalog.DataSourceVolume().ToResource(), "databricks_volumes": catalog.DataSourceVolumes().ToResource(), "databricks_user": scim.DataSourceUser().ToResource(), + "databricks_users": scim.DataSourceUsers().ToResource(), "databricks_zones": clusters.DataSourceClusterZones().ToResource(), }, ResourcesMap: map[string]*schema.Resource{ // must be in alphabetical order diff --git a/scim/data_users.go b/scim/data_users.go index 9c1b3783cf..b3fcd03782 100755 --- a/scim/data_users.go +++ b/scim/data_users.go @@ -9,7 +9,7 @@ import ( "github.com/databricks/terraform-provider-databricks/common" ) -func DataSourceDataUsers() common.Resource { +func DataSourceUsers() common.Resource { type UserInfo struct { Id string `json:"id,omitempty" tf:"computed"` diff --git a/scim/data_users_test.go b/scim/data_users_test.go index 5c19f8342c..6c11e97594 100755 --- a/scim/data_users_test.go +++ b/scim/data_users_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/mock" ) -func TestDataSourceDataUsers_DisplayNameContains(t *testing.T) { +func TestDataSourceUsers_DisplayNameContains(t *testing.T) { qa.ResourceFixture{ MockAccountClientFunc: func(m *mocks.MockAccountClient) { e := m.GetMockAccountUsersAPI() @@ -32,7 +32,7 @@ func TestDataSourceDataUsers_DisplayNameContains(t *testing.T) { }, }, nil) }, - Resource: DataSourceDataUsers(), + Resource: DataSourceUsers(), Read: true, NonWritable: true, ID: "_", @@ -50,7 +50,7 @@ func TestDataSourceDataUsers_DisplayNameContains(t *testing.T) { }) } -func TestDataSourceDataUsers_UserNameContains(t *testing.T) { +func TestDataSourceUsers_UserNameContains(t *testing.T) { qa.ResourceFixture{ MockAccountClientFunc: func(m *mocks.MockAccountClient) { e := m.GetMockAccountUsersAPI() @@ -72,7 +72,7 @@ func TestDataSourceDataUsers_UserNameContains(t *testing.T) { }, }, nil) }, - Resource: DataSourceDataUsers(), + Resource: DataSourceUsers(), Read: true, NonWritable: true, ID: "_", @@ -90,9 +90,9 @@ func TestDataSourceDataUsers_UserNameContains(t *testing.T) { }) } -func TestDataSourceDataUsers_BothFiltersSpecified(t *testing.T) { +func TestDataSourceUsers_BothFiltersSpecified(t *testing.T) { qa.ResourceFixture{ - Resource: DataSourceDataUsers(), + Resource: DataSourceUsers(), Read: true, NonWritable: true, ID: "_", @@ -103,7 +103,7 @@ func TestDataSourceDataUsers_BothFiltersSpecified(t *testing.T) { }.ExpectError(t, "exactly one of display_name_contains or user_name_contains should be specified, not both") } -func TestDataSourceDataUsersNoUsers(t *testing.T) { +func TestDataSourceUsersNoUsers(t *testing.T) { qa.ResourceFixture{ MockAccountClientFunc: func(m *mocks.MockAccountClient) { e := m.GetMockAccountUsersAPI() @@ -114,7 +114,7 @@ func TestDataSourceDataUsersNoUsers(t *testing.T) { req.Attributes == "id,userName,displayName" })).Return([]iam.User{}, nil) }, - Resource: DataSourceDataUsers(), + Resource: DataSourceUsers(), Read: true, NonWritable: true, ID: "_", @@ -124,7 +124,7 @@ func TestDataSourceDataUsersNoUsers(t *testing.T) { }.ExpectError(t, "cannot find users with display name containing testuser") } -func TestDataSourceDataUsersNoFilter(t *testing.T) { +func TestDataSourceUsersNoFilter(t *testing.T) { qa.ResourceFixture{ MockAccountClientFunc: func(m *mocks.MockAccountClient) { e := m.GetMockAccountUsersAPI() @@ -150,7 +150,7 @@ func TestDataSourceDataUsersNoFilter(t *testing.T) { }, }, nil) }, - Resource: DataSourceDataUsers(), + Resource: DataSourceUsers(), Read: true, NonWritable: true, ID: "_", @@ -168,7 +168,7 @@ func TestDataSourceDataUsersNoFilter(t *testing.T) { }) } -func TestDataSourceDataUsers_APIError(t *testing.T) { +func TestDataSourceUsers_APIError(t *testing.T) { qa.ResourceFixture{ MockAccountClientFunc: func(m *mocks.MockAccountClient) { usersAPI := m.GetMockAccountUsersAPI() @@ -177,7 +177,7 @@ func TestDataSourceDataUsers_APIError(t *testing.T) { mock.Anything, ).Return(nil, fmt.Errorf("api error")) }, - Resource: DataSourceDataUsers(), + Resource: DataSourceUsers(), Read: true, NonWritable: true, ID: "_", From dbd79d80a61642d8c6d7ba63e3bf43abf5d18862 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 20 Sep 2024 10:33:52 -0600 Subject: [PATCH 07/27] added correct reference to data resource 'databricks_users' on the acceptance test. --- internal/acceptance/data_users_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/acceptance/data_users_test.go b/internal/acceptance/data_users_test.go index 0107203bc9..c8730eee96 100644 --- a/internal/acceptance/data_users_test.go +++ b/internal/acceptance/data_users_test.go @@ -10,7 +10,7 @@ import ( func TestAccDataSourceDataUsers(t *testing.T) { accountLevel(t, step{ Template: ` - data "databricks_data_users" "this" { + data "databricks_users" "this" { display_name_contains = "testuser" }`, Check: func(s *terraform.State) error { From 4ed08d4b537c1be7ce27a7e790e814232fced344 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 20 Sep 2024 10:37:18 -0600 Subject: [PATCH 08/27] name format changes to acceptance tests --- internal/acceptance/data_users_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/acceptance/data_users_test.go b/internal/acceptance/data_users_test.go index c8730eee96..29c6098099 100644 --- a/internal/acceptance/data_users_test.go +++ b/internal/acceptance/data_users_test.go @@ -14,7 +14,7 @@ func TestAccDataSourceDataUsers(t *testing.T) { display_name_contains = "testuser" }`, Check: func(s *terraform.State) error { - r, ok := s.RootModule().Resources["data.databricks_data_users.this"] + r, ok := s.RootModule().Resources["data.databricks_users.this"] if !ok { return fmt.Errorf("data not found in state") } From 072123a501ae68b7f4f750510b37673248166fbd Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Wed, 2 Oct 2024 10:05:12 -0600 Subject: [PATCH 09/27] modified acceptance test --- internal/acceptance/data_users_test.go | 58 +++++++++++++++++++------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/internal/acceptance/data_users_test.go b/internal/acceptance/data_users_test.go index 29c6098099..d758200c29 100644 --- a/internal/acceptance/data_users_test.go +++ b/internal/acceptance/data_users_test.go @@ -1,28 +1,54 @@ package acceptance import ( - "fmt" "testing" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +const dataSourceTemplate = ` + resource "databricks_user" "user1" { + user_name = "testuser1@databricks.com" + } + + resource "databricks_user" "user2" { + user_name = "testuser2@databricks.com" + } + + data "databricks_users" "this" { + user_name_contains = "testuser" + } +` + +func checkUsersDataSourcePopulated(t *testing.T) func(s *terraform.State) error { + return func(s *terraform.State) error { + ds, ok := s.Modules[0].Resources["data.databricks_users.this"] + require.True(t, ok, "data.databricks_users.this has to be there") + + usersCount := ds.Primary.Attributes["users.#"] + require.Equal(t, "2", usersCount, "expected two users") + + userIds := []string{ + ds.Primary.Attributes["users.0.id"], + ds.Primary.Attributes["users.1.id"], + } + + expectedUserIDs := []string{ + s.Modules[0].Resources["databricks_user.user1"].Primary.ID, + s.Modules[0].Resources["databricks_user.user2"].Primary.ID, + } + + assert.ElementsMatch(t, expectedUserIDs, userIds, "expected user ids to match") + + return nil + } +} + func TestAccDataSourceDataUsers(t *testing.T) { accountLevel(t, step{ - Template: ` - data "databricks_users" "this" { - display_name_contains = "testuser" - }`, - Check: func(s *terraform.State) error { - r, ok := s.RootModule().Resources["data.databricks_users.this"] - if !ok { - return fmt.Errorf("data not found in state") - } - ids := r.Primary.Attributes["users.#"] - if ids == "" { - return fmt.Errorf("users is empty: %v", r.Primary.Attributes) - } - return nil - }, + Template: dataSourceTemplate, + Check: checkUsersDataSourcePopulated(t), }) } From bffcf544a87bc5f943d8d863844c3a13b84fa5b1 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Wed, 2 Oct 2024 10:09:49 -0600 Subject: [PATCH 10/27] fixed integration test to point to correct import --- internal/acceptance/data_users_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/acceptance/data_users_test.go b/internal/acceptance/data_users_test.go index d758200c29..21c1764798 100644 --- a/internal/acceptance/data_users_test.go +++ b/internal/acceptance/data_users_test.go @@ -47,7 +47,7 @@ func checkUsersDataSourcePopulated(t *testing.T) func(s *terraform.State) error } func TestAccDataSourceDataUsers(t *testing.T) { - accountLevel(t, step{ + AccountLevel(t, Step{ Template: dataSourceTemplate, Check: checkUsersDataSourcePopulated(t), }) From 7fa5823101bd7fbc051dd5304d7cdc3b33e81af1 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Wed, 23 Oct 2024 12:23:22 -0600 Subject: [PATCH 11/27] #3468: started migrating to the plugin framework, adding support for workspace-level and account-level provider. --- .../pluginfw/resources/user/data_users.go | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 internal/providers/pluginfw/resources/user/data_users.go diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/resources/user/data_users.go new file mode 100644 index 0000000000..851cb77f53 --- /dev/null +++ b/internal/providers/pluginfw/resources/user/data_users.go @@ -0,0 +1,93 @@ +package user + +import ( + "context" + + "github.com/databricks/terraform-provider-databricks/common" + pluginfwcommon "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/common" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/tfschema" + "github.com/hashicorp/terraform-plugin-framework/datasource" + "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/types" +) + +func DataSourceUsers() datasource.DataSource { + return &UsersDataSource{} +} + +var _ datasource.DataSourceWithConfigure = &UsersDataSource{} + +type UsersDataSource struct { + Client *common.DatabricksClient +} + +type UserData struct { + Id types.String `tfsdk:"id,omitempty" tf:"computed"` + UserName types.String `tfsdk:"user_name,omitempty" tf:"computed"` + DisplayName types.String `tfsdk:"display_name,omitempty" tf:"computed"` +} + +type UsersInfo struct { + DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` + UserNameContains string `json:"user_name_contains,omitempty" tf:"computed"` + Users []UserData `json:"users,omitempty" tf:"computed"` // TODO: use UserData[] or []iam_tf.ListAccountUserResponse / []iam_tf.ListUserResponse? +} + +func (d *UsersDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { + resp.TypeName = "databricks_users" +} + +func (d *UsersDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { + attrs, blocks := tfschema.DataSourceStructToSchemaMap(UsersInfo{}, nil) + resp.Schema = schema.Schema{ + Attributes: attrs, + Blocks: blocks, + } +} + +func (d *UsersDataSource) Configure(_ context.Context, req datasource.ConfigureRequest, resp *datasource.ConfigureResponse) { + if d.Client == nil { + d.Client = pluginfwcommon.ConfigureDataSource(req, resp) + } +} + +// AppendDiagAndCheckErrors is a helper function that simplifies error handling by combining the appending of diagnostics and the checking of errors in a single step. +// It is particularly useful in data source and resource read operations where you want to append diagnostics and immediately determine if an error has occurred. +func AppendDiagAndCheckErrors(resp *datasource.ReadResponse, diags diag.Diagnostics) bool { + resp.Diagnostics.Append(diags...) + return resp.Diagnostics.HasError() +} + +func validateFilters(data *UsersInfo) diag.Diagnostics { + if data.DisplayNameContains != "" && data.UserNameContains != "" { + return diag.Diagnostics{diag.NewErrorDiagnostic("Invalid configuration", "Exactly one of display_name_contains or user_name_contains should be specified, not both.")} + } + return nil +} + +func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { + var userInfo UsersInfo + + diags := req.Config.Get(ctx, &userInfo) + diags = append(diags, validateFilters(&userInfo)...) + + if AppendDiagAndCheckErrors(resp, diags) { + return + } + + if d.Client.Config.IsAccountClient() { + a, diags := d.Client.GetAccountClient() + if AppendDiagAndCheckErrors(resp, diags) { + return + } + // TODO: Add retrieval of iterator at the account level. + } else { + w, diags := d.Client.GetWorkspaceClient() + if AppendDiagAndCheckErrors(resp, diags) { + return + } + // TODO: Add retreival of iterator at the workspace level. + } + // TODO: Continue setting the datasource. +} From 1c58eaa4629775fd99603b478c6c5f96a206ea86 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 16:31:26 -0600 Subject: [PATCH 12/27] migrated data_users to plugin framework and added support for both providers. --- .../pluginfw/resources/user/data_users.go | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/resources/user/data_users.go index 851cb77f53..ff4742d2f2 100644 --- a/internal/providers/pluginfw/resources/user/data_users.go +++ b/internal/providers/pluginfw/resources/user/data_users.go @@ -2,14 +2,15 @@ package user import ( "context" + "fmt" + "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/terraform-provider-databricks/common" pluginfwcommon "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/common" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/tfschema" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/datasource/schema" "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/hashicorp/terraform-plugin-framework/types" ) func DataSourceUsers() datasource.DataSource { @@ -22,16 +23,10 @@ type UsersDataSource struct { Client *common.DatabricksClient } -type UserData struct { - Id types.String `tfsdk:"id,omitempty" tf:"computed"` - UserName types.String `tfsdk:"user_name,omitempty" tf:"computed"` - DisplayName types.String `tfsdk:"display_name,omitempty" tf:"computed"` -} - type UsersInfo struct { DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` UserNameContains string `json:"user_name_contains,omitempty" tf:"computed"` - Users []UserData `json:"users,omitempty" tf:"computed"` // TODO: use UserData[] or []iam_tf.ListAccountUserResponse / []iam_tf.ListUserResponse? + Users []iam.User `json:"users,omitempty" tf:"computed"` } func (d *UsersDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { @@ -59,6 +54,13 @@ func AppendDiagAndCheckErrors(resp *datasource.ReadResponse, diags diag.Diagnost return resp.Diagnostics.HasError() } +// DiagsFromError helps us create an error diagnostic from an error. +func DiagsFromError(summary string, err error) diag.Diagnostics { + return diag.Diagnostics{ + diag.NewErrorDiagnostic(summary, err.Error()), + } +} + func validateFilters(data *UsersInfo) diag.Diagnostics { if data.DisplayNameContains != "" && data.UserNameContains != "" { return diag.Diagnostics{diag.NewErrorDiagnostic("Invalid configuration", "Exactly one of display_name_contains or user_name_contains should be specified, not both.")} @@ -71,23 +73,41 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, diags := req.Config.Get(ctx, &userInfo) diags = append(diags, validateFilters(&userInfo)...) - if AppendDiagAndCheckErrors(resp, diags) { return } + var filter string + + if userInfo.DisplayNameContains != "" { + filter = fmt.Sprintf("displayName co \"%s\"", userInfo.DisplayNameContains) + } else if userInfo.UserNameContains != "" { + filter = fmt.Sprintf("userName co \"%s\"", userInfo.UserNameContains) + } + + var users []iam.User + var err error + if d.Client.Config.IsAccountClient() { a, diags := d.Client.GetAccountClient() if AppendDiagAndCheckErrors(resp, diags) { return } - // TODO: Add retrieval of iterator at the account level. + users, err = a.Users.ListAll(ctx, iam.ListAccountUsersRequest{Filter: filter}) + if err != nil && AppendDiagAndCheckErrors(resp, DiagsFromError("Error listing account users", err)) { + return + } } else { w, diags := d.Client.GetWorkspaceClient() if AppendDiagAndCheckErrors(resp, diags) { return } - // TODO: Add retreival of iterator at the workspace level. + users, err = w.Users.ListAll(ctx, iam.ListUsersRequest{Filter: filter}) + if err != nil && AppendDiagAndCheckErrors(resp, DiagsFromError("Error listing workspace users", err)) { + return + } } - // TODO: Continue setting the datasource. + + userInfo.Users = users + resp.Diagnostics.Append(resp.State.Set(ctx, userInfo)...) } From c2d70b9ebe8528c008b7dc001bcc482ebbbef661 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 16:33:39 -0600 Subject: [PATCH 13/27] deleted sdkv2 'data_users' and removed it from sdkv2.go --- internal/providers/sdkv2/sdkv2.go | 1 - scim/data_users.go | 72 ------------------------------- 2 files changed, 73 deletions(-) delete mode 100755 scim/data_users.go diff --git a/internal/providers/sdkv2/sdkv2.go b/internal/providers/sdkv2/sdkv2.go index 40ba5d3d1a..8136901ddf 100644 --- a/internal/providers/sdkv2/sdkv2.go +++ b/internal/providers/sdkv2/sdkv2.go @@ -125,7 +125,6 @@ func DatabricksProvider() *schema.Provider { "databricks_volume": catalog.DataSourceVolume().ToResource(), "databricks_volumes": catalog.DataSourceVolumes().ToResource(), "databricks_user": scim.DataSourceUser().ToResource(), - "databricks_users": scim.DataSourceUsers().ToResource(), "databricks_zones": clusters.DataSourceClusterZones().ToResource(), }, ResourcesMap: map[string]*schema.Resource{ // must be in alphabetical order diff --git a/scim/data_users.go b/scim/data_users.go deleted file mode 100755 index b3fcd03782..0000000000 --- a/scim/data_users.go +++ /dev/null @@ -1,72 +0,0 @@ -package scim - -import ( - "context" - "fmt" - - "github.com/databricks/databricks-sdk-go" - "github.com/databricks/databricks-sdk-go/service/iam" - "github.com/databricks/terraform-provider-databricks/common" -) - -func DataSourceUsers() common.Resource { - - type UserInfo struct { - Id string `json:"id,omitempty" tf:"computed"` - UserName string `json:"user_name,omitempty" tf:"computed"` - DisplayName string `json:"display_name,omitempty" tf:"computed"` - } - - type DataUsers struct { - DisplayNameContains string `json:"display_name_contains,omitempty" tf:"computed"` - UserNameContains string `json:"user_name_contains,omitempty" tf:"computed"` - Users []UserInfo `json:"users,omitempty" tf:"computed"` - } - - return common.AccountData(func(ctx context.Context, data *DataUsers, acc *databricks.AccountClient) error { - listRequest := iam.ListAccountUsersRequest{ - Attributes: "id,userName,displayName", - } - - if data.DisplayNameContains != "" && data.UserNameContains != "" { - return fmt.Errorf("exactly one of display_name_contains or user_name_contains should be specified, not both") - } - - if data.UserNameContains != "" { - listRequest.Filter = fmt.Sprintf("userName co \"%s\"", data.UserNameContains) - } else if data.DisplayNameContains != "" { - listRequest.Filter = fmt.Sprintf("displayName co \"%s\"", data.DisplayNameContains) - } - - userList, err := acc.Users.ListAll(ctx, listRequest) - - if err != nil { - return err - } - - if len(userList) == 0 { - if data.DisplayNameContains != "" { - return fmt.Errorf("cannot find users with display name containing %s", data.DisplayNameContains) - } else if data.UserNameContains != "" { - return fmt.Errorf("cannot find users with username containing %s", data.UserNameContains) - } else { - return fmt.Errorf("no users found") - } - } - - var users []UserInfo - - for _, u := range userList { - user := UserInfo{ - Id: u.Id, - UserName: u.UserName, - DisplayName: u.DisplayName, - } - users = append(users, user) - } - - data.Users = users - - return nil - }) -} From 4ab1e276a3c80b00dc4c9900e7dabf92b8c8e39d Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 16:48:05 -0600 Subject: [PATCH 14/27] migrated test --- .../resources/user/data_users_test.go | 19 ++ scim/data_users_test.go | 188 ------------------ 2 files changed, 19 insertions(+), 188 deletions(-) create mode 100755 internal/providers/pluginfw/resources/user/data_users_test.go delete mode 100755 scim/data_users_test.go diff --git a/internal/providers/pluginfw/resources/user/data_users_test.go b/internal/providers/pluginfw/resources/user/data_users_test.go new file mode 100755 index 0000000000..08be8522da --- /dev/null +++ b/internal/providers/pluginfw/resources/user/data_users_test.go @@ -0,0 +1,19 @@ +package user + +import ( + "testing" + + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/stretchr/testify/assert" +) + +func TestValidateFilters(t *testing.T) { + userInfo := UsersInfo{ + DisplayNameContains: "filter", + UserNameContains: "another_filter", + } + actualDiagnostics := validateFilters(&userInfo) + expectedDiagnostics := diag.Diagnostics{diag.NewErrorDiagnostic("Invalid configuration", "Exactly one of display_name_contains or user_name_contains should be specified, not both.")} + assert.True(t, actualDiagnostics.HasError()) + assert.Equal(t, expectedDiagnostics, actualDiagnostics) +} diff --git a/scim/data_users_test.go b/scim/data_users_test.go deleted file mode 100755 index 6c11e97594..0000000000 --- a/scim/data_users_test.go +++ /dev/null @@ -1,188 +0,0 @@ -package scim - -import ( - "fmt" - "testing" - - "github.com/databricks/databricks-sdk-go/experimental/mocks" - "github.com/databricks/databricks-sdk-go/service/iam" - "github.com/databricks/terraform-provider-databricks/qa" - "github.com/stretchr/testify/mock" -) - -func TestDataSourceUsers_DisplayNameContains(t *testing.T) { - qa.ResourceFixture{ - MockAccountClientFunc: func(m *mocks.MockAccountClient) { - e := m.GetMockAccountUsersAPI() - e.On("ListAll", - mock.Anything, - mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { - return req.Filter == `displayName co "testuser"` && - req.Attributes == "id,userName,displayName" - })).Return([]iam.User{ - { - Id: "123", - UserName: "testuser@example.com", - DisplayName: "testuser", - }, - { - Id: "456", - UserName: "testuser2@example.com", - DisplayName: "testuser2", - }, - }, nil) - }, - Resource: DataSourceUsers(), - Read: true, - NonWritable: true, - ID: "_", - HCL: ` - display_name_contains = "testuser" - `, - }.ApplyAndExpectData(t, map[string]any{ - "users.#": 2, - "users.0.id": "123", - "users.0.user_name": "testuser@example.com", - "users.0.display_name": "testuser", - "users.1.id": "456", - "users.1.user_name": "testuser2@example.com", - "users.1.display_name": "testuser2", - }) -} - -func TestDataSourceUsers_UserNameContains(t *testing.T) { - qa.ResourceFixture{ - MockAccountClientFunc: func(m *mocks.MockAccountClient) { - e := m.GetMockAccountUsersAPI() - e.On("ListAll", - mock.Anything, - mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { - return req.Filter == `userName co "testuser"` && - req.Attributes == "id,userName,displayName" - })).Return([]iam.User{ - { - Id: "123", - UserName: "testuser@example.com", - DisplayName: "testuser", - }, - { - Id: "456", - UserName: "testuser2@example.com", - DisplayName: "testuser2", - }, - }, nil) - }, - Resource: DataSourceUsers(), - Read: true, - NonWritable: true, - ID: "_", - HCL: ` - user_name_contains = "testuser" - `, - }.ApplyAndExpectData(t, map[string]any{ - "users.#": 2, - "users.0.id": "123", - "users.0.user_name": "testuser@example.com", - "users.0.display_name": "testuser", - "users.1.id": "456", - "users.1.user_name": "testuser2@example.com", - "users.1.display_name": "testuser2", - }) -} - -func TestDataSourceUsers_BothFiltersSpecified(t *testing.T) { - qa.ResourceFixture{ - Resource: DataSourceUsers(), - Read: true, - NonWritable: true, - ID: "_", - HCL: ` - display_name_contains = "user" - user_name_contains = "example.com" - `, - }.ExpectError(t, "exactly one of display_name_contains or user_name_contains should be specified, not both") -} - -func TestDataSourceUsersNoUsers(t *testing.T) { - qa.ResourceFixture{ - MockAccountClientFunc: func(m *mocks.MockAccountClient) { - e := m.GetMockAccountUsersAPI() - e.On("ListAll", - mock.Anything, - mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { - return req.Filter == `displayName co "testuser"` && - req.Attributes == "id,userName,displayName" - })).Return([]iam.User{}, nil) - }, - Resource: DataSourceUsers(), - Read: true, - NonWritable: true, - ID: "_", - HCL: ` - display_name_contains = "testuser" - `, - }.ExpectError(t, "cannot find users with display name containing testuser") -} - -func TestDataSourceUsersNoFilter(t *testing.T) { - qa.ResourceFixture{ - MockAccountClientFunc: func(m *mocks.MockAccountClient) { - e := m.GetMockAccountUsersAPI() - e.On("ListAll", - mock.Anything, - mock.MatchedBy(func(req iam.ListAccountUsersRequest) bool { - return req.Attributes == "id,userName,displayName" - })).Return([]iam.User{ - { - Id: "123", - UserName: "testuser@example.com", - DisplayName: "testuser", - }, - { - Id: "456", - UserName: "testuser2@example.com", - DisplayName: "testuser2", - }, - { - Id: "789", - UserName: "testuser3@example.com", - DisplayName: "testuser3", - }, - }, nil) - }, - Resource: DataSourceUsers(), - Read: true, - NonWritable: true, - ID: "_", - }.ApplyAndExpectData(t, map[string]any{ - "users.#": 3, - "users.0.id": "123", - "users.0.user_name": "testuser@example.com", - "users.0.display_name": "testuser", - "users.1.id": "456", - "users.1.user_name": "testuser2@example.com", - "users.1.display_name": "testuser2", - "users.2.id": "789", - "users.2.user_name": "testuser3@example.com", - "users.2.display_name": "testuser3", - }) -} - -func TestDataSourceUsers_APIError(t *testing.T) { - qa.ResourceFixture{ - MockAccountClientFunc: func(m *mocks.MockAccountClient) { - usersAPI := m.GetMockAccountUsersAPI() - usersAPI.On("ListAll", - mock.Anything, - mock.Anything, - ).Return(nil, fmt.Errorf("api error")) - }, - Resource: DataSourceUsers(), - Read: true, - NonWritable: true, - ID: "_", - HCL: ` - display_name_contains = "testuser" - `, - }.ExpectError(t, "api error") -} From 8db0188a275b74c841f6993ca6cfe9878eec715f Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 16:53:44 -0600 Subject: [PATCH 15/27] added acceptance tests and added the data source to pluginfw.go --- internal/providers/pluginfw/pluginfw.go | 2 ++ .../pluginfw/resources/user/data_users_acc_test.go} | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) rename internal/{acceptance/data_users_test.go => providers/pluginfw/resources/user/data_users_acc_test.go} (80%) diff --git a/internal/providers/pluginfw/pluginfw.go b/internal/providers/pluginfw/pluginfw.go index db811d5ae2..f7b8dbfa6f 100644 --- a/internal/providers/pluginfw/pluginfw.go +++ b/internal/providers/pluginfw/pluginfw.go @@ -21,6 +21,7 @@ import ( "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/notificationdestinations" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/qualitymonitor" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/registered_model" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/volume" "github.com/hashicorp/terraform-plugin-framework/datasource" @@ -56,6 +57,7 @@ func (p *DatabricksProviderPluginFramework) DataSources(ctx context.Context) []f volume.DataSourceVolumes, registered_model.DataSourceRegisteredModel, notificationdestinations.DataSourceNotificationDestinations, + user.DataSourceUsers, } } diff --git a/internal/acceptance/data_users_test.go b/internal/providers/pluginfw/resources/user/data_users_acc_test.go similarity index 80% rename from internal/acceptance/data_users_test.go rename to internal/providers/pluginfw/resources/user/data_users_acc_test.go index 21c1764798..1223d99ebe 100644 --- a/internal/acceptance/data_users_test.go +++ b/internal/providers/pluginfw/resources/user/data_users_acc_test.go @@ -1,8 +1,9 @@ -package acceptance +package user_test import ( "testing" + "github.com/databricks/terraform-provider-databricks/internal/acceptance" "github.com/hashicorp/terraform-plugin-testing/terraform" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -47,7 +48,14 @@ func checkUsersDataSourcePopulated(t *testing.T) func(s *terraform.State) error } func TestAccDataSourceDataUsers(t *testing.T) { - AccountLevel(t, Step{ + acceptance.AccountLevel(t, acceptance.Step{ + Template: dataSourceTemplate, + Check: checkUsersDataSourcePopulated(t), + }) +} + +func TestWorkspaceDataSourceDataUsers(t *testing.T) { + acceptance.WorkspaceLevel(t, acceptance.Step{ Template: dataSourceTemplate, Check: checkUsersDataSourcePopulated(t), }) From 2e46921f7891405c93054ad2efe6cd2d9c19c0e7 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 16:58:38 -0600 Subject: [PATCH 16/27] modified docs to reflect changes --- docs/data-sources/users.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index b3c2cbcd96..2fba6db474 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -4,8 +4,6 @@ subcategory: "Security" # databricks_users Data Source -->**Note** This is an account-level data source. - -> **Note** If you have a fully automated setup with workspaces created by [databricks_mws_workspaces](../resources/mws_workspaces.md) or [azurerm_databricks_workspace](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace), please make sure to add [depends_on attribute](../guides/troubleshooting.md#data-resources-and-authentication-is-not-configured-errors) in order to prevent _default auth: cannot configure default credentials_ errors. Retrieves information about multiple [databricks_user](../resources/user.md) resources. From 6e4b8edc1751f05131deeac4ace2860513833bf6 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 17:08:40 -0600 Subject: [PATCH 17/27] added correct attributes to the docs --- docs/data-sources/users.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index 2fba6db474..336927bff5 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -43,8 +43,13 @@ This data source exposes the following attributes: - `users` - A list of users matching the specified criteria. Each user has the following attributes: - `id` - The ID of the user. - - `user_name` - The username of the user. - - `display_name` - The display name of the user. + - `userName` - The username of the user. + - `emails` - All the emails associated with the Databricks user. + - `name` + - `givenName` - Given name of the Databricks user. + - `familyName` - Family name of the Databricks user. + - `active` - Boolean that represents if this user is active. + - `displayName` - The display name of the user. ## Related Resources From 5604c6809d7e7463e9a468cea1c40cc52285b273 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 24 Oct 2024 17:09:59 -0600 Subject: [PATCH 18/27] added correct attributes to the docs --- docs/data-sources/users.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index 336927bff5..e8f31b9941 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -48,8 +48,15 @@ This data source exposes the following attributes: - `name` - `givenName` - Given name of the Databricks user. - `familyName` - Family name of the Databricks user. - - `active` - Boolean that represents if this user is active. - `displayName` - The display name of the user. + - `roles` - Indicates if the user has the admin role. + - `$ref` + - `value` + - `display` + - `primary` + - `type` + - `externalId` - reserved for future use. + - `active` - Boolean that represents if this user is active. ## Related Resources From d5ac8b46c3589bf097c801c87a2636a6b93d9861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20G=C3=B3mez=20Moreno?= <69928678+dgomez04@users.noreply.github.com> Date: Thu, 31 Oct 2024 16:21:45 -0600 Subject: [PATCH 19/27] Fix `fmt`on pluginfw.go Fix `fmt`. --- internal/providers/pluginfw/pluginfw.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/providers/pluginfw/pluginfw.go b/internal/providers/pluginfw/pluginfw.go index 35659b706b..20deb3ee55 100644 --- a/internal/providers/pluginfw/pluginfw.go +++ b/internal/providers/pluginfw/pluginfw.go @@ -23,7 +23,7 @@ import ( "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/qualitymonitor" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/registered_model" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/sharing" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/volume" "github.com/hashicorp/terraform-plugin-framework/datasource" @@ -63,7 +63,7 @@ func (p *DatabricksProviderPluginFramework) DataSources(ctx context.Context) []f sharing.DataSourceShare, sharing.DataSourceShares, catalog.DataSourceFunctions, - user.DataSourceUsers, + user.DataSourceUsers, } } From 32e79f605223813d19f0782626df4c6d23801f8e Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Thu, 31 Oct 2024 16:48:51 -0600 Subject: [PATCH 20/27] merged upstream --- internal/providers/pluginfw/pluginfw.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/providers/pluginfw/pluginfw.go b/internal/providers/pluginfw/pluginfw.go index 20deb3ee55..992606087f 100644 --- a/internal/providers/pluginfw/pluginfw.go +++ b/internal/providers/pluginfw/pluginfw.go @@ -23,7 +23,7 @@ import ( "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/qualitymonitor" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/registered_model" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/sharing" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/volume" "github.com/hashicorp/terraform-plugin-framework/datasource" @@ -63,7 +63,7 @@ func (p *DatabricksProviderPluginFramework) DataSources(ctx context.Context) []f sharing.DataSourceShare, sharing.DataSourceShares, catalog.DataSourceFunctions, - user.DataSourceUsers, + user.DataSourceUsers, } } From 88608c1c13ed7ff3ad93f982ec0af9ac1485f29d Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 1 Nov 2024 12:01:23 -0600 Subject: [PATCH 21/27] removed unnecesary tests, switched to use a filter field to leverage REST API's flexibility --- .../pluginfw/resources/user/data_users.go | 11 +++++++---- .../resources/user/data_users_acc_test.go | 2 +- .../resources/user/data_users_test.go | 19 ------------------- 3 files changed, 8 insertions(+), 24 deletions(-) delete mode 100755 internal/providers/pluginfw/resources/user/data_users_test.go diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/resources/user/data_users.go index c5fbdc8f6e..ce15767cc0 100644 --- a/internal/providers/pluginfw/resources/user/data_users.go +++ b/internal/providers/pluginfw/resources/user/data_users.go @@ -2,7 +2,6 @@ package user import ( "context" - "go/types" "github.com/databricks/databricks-sdk-go/service/iam" "github.com/databricks/terraform-provider-databricks/common" @@ -12,6 +11,7 @@ import ( "github.com/databricks/terraform-provider-databricks/internal/service/iam_tf" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/datasource/schema" + "github.com/hashicorp/terraform-plugin-framework/types" ) func DataSourceUsers() datasource.DataSource { @@ -55,6 +55,9 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } + var users []iam.User + var err error + if d.Client.Config.IsAccountClient() { a, diags := d.Client.GetAccountClient() resp.Diagnostics.Append(diags...) @@ -62,7 +65,7 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } - users, err := a.Users.ListAll(ctx, iam.ListAccountUsersRequest{Filter: usersInfo.Filter}) + users, err = a.Users.ListAll(ctx, iam.ListAccountUsersRequest{Filter: usersInfo.Filter.ValueString()}) if err != nil { resp.Diagnostics.AddError("Error listing account users", err.Error()) } @@ -73,7 +76,7 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } - users, err := w.Users.ListAll(ctx, iam.ListUsersRequest{Filter: filter}) + users, err = w.Users.ListAll(ctx, iam.ListUsersRequest{Filter: usersInfo.Filter.ValueString()}) if err != nil { resp.Diagnostics.AddError("Error listing workspace users", err.Error()) } @@ -85,7 +88,7 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, if resp.Diagnostics.HasError() { return } - usersInfo.Users = append(usersInfo.Users, user) + usersInfo.Users = append(usersInfo.Users, tfUser) } resp.Diagnostics.Append(resp.State.Set(ctx, usersInfo)...) diff --git a/internal/providers/pluginfw/resources/user/data_users_acc_test.go b/internal/providers/pluginfw/resources/user/data_users_acc_test.go index eb76926b14..a71b8cd9e4 100644 --- a/internal/providers/pluginfw/resources/user/data_users_acc_test.go +++ b/internal/providers/pluginfw/resources/user/data_users_acc_test.go @@ -19,7 +19,7 @@ const dataSourceTemplate = ` } data "databricks_users" "this" { - user_name_contains = "testuser" + filter = "userName co \"testuser\"" depends_on = [databricks_user.user1, databricks_user.user2] } ` diff --git a/internal/providers/pluginfw/resources/user/data_users_test.go b/internal/providers/pluginfw/resources/user/data_users_test.go deleted file mode 100755 index 08be8522da..0000000000 --- a/internal/providers/pluginfw/resources/user/data_users_test.go +++ /dev/null @@ -1,19 +0,0 @@ -package user - -import ( - "testing" - - "github.com/hashicorp/terraform-plugin-framework/diag" - "github.com/stretchr/testify/assert" -) - -func TestValidateFilters(t *testing.T) { - userInfo := UsersInfo{ - DisplayNameContains: "filter", - UserNameContains: "another_filter", - } - actualDiagnostics := validateFilters(&userInfo) - expectedDiagnostics := diag.Diagnostics{diag.NewErrorDiagnostic("Invalid configuration", "Exactly one of display_name_contains or user_name_contains should be specified, not both.")} - assert.True(t, actualDiagnostics.HasError()) - assert.Equal(t, expectedDiagnostics, actualDiagnostics) -} From 26c69fbd0d250cafc2819c8d770676bc097557ed Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 1 Nov 2024 12:16:59 -0600 Subject: [PATCH 22/27] added resource to rollout utils --- internal/providers/pluginfw/pluginfw.go | 19 +------------------ .../pluginfw/pluginfw_rollout_utils.go | 2 ++ .../pluginfw/resources/user/data_users.go | 4 +++- .../resources/user/data_users_acc_test.go | 4 ++-- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/internal/providers/pluginfw/pluginfw.go b/internal/providers/pluginfw/pluginfw.go index bf1b4cce90..4eaecd9938 100644 --- a/internal/providers/pluginfw/pluginfw.go +++ b/internal/providers/pluginfw/pluginfw.go @@ -16,14 +16,6 @@ import ( "github.com/databricks/terraform-provider-databricks/commands" "github.com/databricks/terraform-provider-databricks/common" providercommon "github.com/databricks/terraform-provider-databricks/internal/providers/common" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/catalog" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/cluster" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/notificationdestinations" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/registered_model" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/sharing" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/volume" - "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" @@ -50,16 +42,7 @@ func (p *DatabricksProviderPluginFramework) Resources(ctx context.Context) []fun } func (p *DatabricksProviderPluginFramework) DataSources(ctx context.Context) []func() datasource.DataSource { - return []func() datasource.DataSource{ - cluster.DataSourceCluster, - volume.DataSourceVolumes, - registered_model.DataSourceRegisteredModel, - notificationdestinations.DataSourceNotificationDestinations, - sharing.DataSourceShare, - sharing.DataSourceShares, - catalog.DataSourceFunctions, - user.DataSourceUsers, - } + return getPluginFrameworkDataSourcesToRegister(p.sdkV2Fallbacks...) } func (p *DatabricksProviderPluginFramework) Schema(ctx context.Context, req provider.SchemaRequest, resp *provider.SchemaResponse) { diff --git a/internal/providers/pluginfw/pluginfw_rollout_utils.go b/internal/providers/pluginfw/pluginfw_rollout_utils.go index 90b782a511..5a74df8e7d 100644 --- a/internal/providers/pluginfw/pluginfw_rollout_utils.go +++ b/internal/providers/pluginfw/pluginfw_rollout_utils.go @@ -19,6 +19,7 @@ import ( "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/qualitymonitor" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/registered_model" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/sharing" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/volume" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/resource" @@ -45,6 +46,7 @@ var pluginFwOnlyResources = []func() resource.Resource{ var pluginFwOnlyDataSources = []func() datasource.DataSource{ registered_model.DataSourceRegisteredModel, notificationdestinations.DataSourceNotificationDestinations, + user.DataSourceUsers, catalog.DataSourceFunctions, // TODO: Add DataSourceCluster into migratedDataSources after fixing unit tests. cluster.DataSourceCluster, // Using the staging name (with pluginframework suffix) diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/resources/user/data_users.go index ce15767cc0..5117d3bc53 100644 --- a/internal/providers/pluginfw/resources/user/data_users.go +++ b/internal/providers/pluginfw/resources/user/data_users.go @@ -14,6 +14,8 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" ) +const dataSourceName = "users" + func DataSourceUsers() datasource.DataSource { return &UsersDataSource{} } @@ -30,7 +32,7 @@ type UsersInfo struct { } func (d *UsersDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { - resp.TypeName = "databricks_users" + resp.TypeName = pluginfwcommon.GetDatabricksProductionName(dataSourceName) } func (d *UsersDataSource) Schema(ctx context.Context, req datasource.SchemaRequest, resp *datasource.SchemaResponse) { diff --git a/internal/providers/pluginfw/resources/user/data_users_acc_test.go b/internal/providers/pluginfw/resources/user/data_users_acc_test.go index a71b8cd9e4..79f9eaeba7 100644 --- a/internal/providers/pluginfw/resources/user/data_users_acc_test.go +++ b/internal/providers/pluginfw/resources/user/data_users_acc_test.go @@ -11,11 +11,11 @@ import ( const dataSourceTemplate = ` resource "databricks_user" "user1" { - user_name = "user-datasource-{var.STICKY_RANDOM}-1@databricks.com" + user_name = "tf-{var.STICKY_RANDOM}-1@databricks.com" } resource "databricks_user" "user2" { - user_name = "user-datasource-{var.STICKY_RANDOM}-2@databricks.com" + user_name = "tf-{var.STICKY_RANDOM}-2@databricks.com" } data "databricks_users" "this" { From 9e541372c09183befe92e5121eb22c63eb5b74db Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Fri, 1 Nov 2024 12:37:12 -0600 Subject: [PATCH 23/27] updated docs to reflect changes --- docs/data-sources/users.md | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index e8f31b9941..d9aef49a6c 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -4,7 +4,7 @@ subcategory: "Security" # databricks_users Data Source --> **Note** If you have a fully automated setup with workspaces created by [databricks_mws_workspaces](../resources/mws_workspaces.md) or [azurerm_databricks_workspace](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace), please make sure to add [depends_on attribute](../guides/troubleshooting.md#data-resources-and-authentication-is-not-configured-errors) in order to prevent _default auth: cannot configure default credentials_ errors. +-> If you have a fully automated setup with workspaces created by [databricks_mws_workspaces](../resources/mws_workspaces.md) or [azurerm_databricks_workspace](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace), please make sure to add [depends_on attribute](../guides/troubleshooting.md#data-resources-and-authentication-is-not-configured-errors) in order to prevent _default auth: cannot configure default credentials_ errors. Retrieves information about multiple [databricks_user](../resources/user.md) resources. @@ -14,7 +14,7 @@ Adding a subset of users to a group ```hcl data "databricks_users" "company_users" { - user_name_contains = "@domain.org" + filter = "userName co \"@domain.org\"" } resource "databricks_group" "data_users_group" { @@ -32,10 +32,17 @@ resource "databricks_group_member" "add_users_to_group" { This data source allows you to filter the list of users using the following optional arguments: -- `display_name_contains` - (Optional) A substring to filter users by their display name. Only users whose display names contain this substring will be included in the results. -- `user_name_contains` - (Optional) A substring to filter users by their username. Only users whose usernames contain this substring will be included in the results. - -->**Note** You can specify **exactly one** of `display_name_contains` or `user_name_contains`. If neither is specified, all users will be returned. +- `filter` - (Optional) Query by which the results have to be filtered. If not specified, all users will be returned. Supported operators are equals (`eq`), contains (`co`), starts with (`sw`), and not equals (`ne`). Additionally, simple expressions can be formed using logical operators `and` and `or`. + + **Examples:** + - User whose `displayName` equals "john": + ```hcl + filter = "displayName eq \"john\"" + ``` + - User whose `displayName` contains "john" or `userName` contains "@domain.org": + ```hcl + filter = "displayName co \"john\" or userName co \"@domain.org\"" + ``` ## Attribute Reference From 43b66ec15a3ef2059de771721bac591e648e1aff Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Wed, 20 Nov 2024 11:13:12 -0600 Subject: [PATCH 24/27] added requested changes --- docs/data-sources/users.md | 23 +++++++- .../pluginfw/resources/user/data_users.go | 12 +++- .../resources/user/data_users_acc_test.go | 59 +++++++++++++++++++ 3 files changed, 89 insertions(+), 5 deletions(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index d9aef49a6c..0c0e2d8060 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -4,6 +4,8 @@ subcategory: "Security" # databricks_users Data Source +-> This data source works with both the account-level and workspace-level provider. + -> If you have a fully automated setup with workspaces created by [databricks_mws_workspaces](../resources/mws_workspaces.md) or [azurerm_databricks_workspace](https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs/resources/databricks_workspace), please make sure to add [depends_on attribute](../guides/troubleshooting.md#data-resources-and-authentication-is-not-configured-errors) in order to prevent _default auth: cannot configure default credentials_ errors. Retrieves information about multiple [databricks_user](../resources/user.md) resources. @@ -32,6 +34,8 @@ resource "databricks_group_member" "add_users_to_group" { This data source allows you to filter the list of users using the following optional arguments: +-> Attribute names and operators used in filters are case-insensitive. Find more information [here](https://datatracker.ietf.org/doc/html/rfc7644#section-3.4.2.2). + - `filter` - (Optional) Query by which the results have to be filtered. If not specified, all users will be returned. Supported operators are equals (`eq`), contains (`co`), starts with (`sw`), and not equals (`ne`). Additionally, simple expressions can be formed using logical operators `and` and `or`. **Examples:** @@ -44,6 +48,8 @@ This data source allows you to filter the list of users using the following opti filter = "displayName co \"john\" or userName co \"@domain.org\"" ``` +- `extra_attributes` - (Optional) A list of additional user attributes to include in the results. By default, the data source returns the following attributes: `id`, `userName`, `displayName`, and `externalId`. Use this argument to request additional attributes as needed. The list of all available attributes can be found in the [API reference](https://docs.databricks.com/api/workspace/users/list). + ## Attribute Reference This data source exposes the following attributes: @@ -56,13 +62,26 @@ This data source exposes the following attributes: - `givenName` - Given name of the Databricks user. - `familyName` - Family name of the Databricks user. - `displayName` - The display name of the user. - - `roles` - Indicates if the user has the admin role. + - `groups` - Indicates if the user is part of any groups. + - `$ref` + - `value` + - `display` + - `primary` + - `type` + - `entitlements` - Entitlements assigned to the user. + - `$ref` + - `value` + - `display` + - `primary` + - `type` + - `roles` - Indicates if the user has any associated roles. - `$ref` - `value` - `display` - `primary` - `type` - - `externalId` - reserved for future use. + - `schemas` - The schema of the user. + - `externalId` - Reserved for future use. - `active` - Boolean that represents if this user is active. ## Related Resources diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/resources/user/data_users.go index 5117d3bc53..4663d212c8 100644 --- a/internal/providers/pluginfw/resources/user/data_users.go +++ b/internal/providers/pluginfw/resources/user/data_users.go @@ -27,8 +27,9 @@ type UsersDataSource struct { } type UsersInfo struct { - Filter types.String `json:"filter,omitempty"` - Users []iam_tf.User `json:"users,omitempty" tf:"computed"` + Filter types.String `json:"filter,omitempty"` + ExtraAttributes types.String `json:"extra_attributes,omitempty"` + Users []iam_tf.User `json:"users,omitempty" tf:"computed"` } func (d *UsersDataSource) Metadata(ctx context.Context, req datasource.MetadataRequest, resp *datasource.MetadataResponse) { @@ -51,12 +52,17 @@ func (d *UsersDataSource) Configure(_ context.Context, req datasource.ConfigureR func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { var usersInfo UsersInfo + var attributes string = "id,userName,displayName,externalId," resp.Diagnostics.Append(req.Config.Get(ctx, &usersInfo)...) if resp.Diagnostics.HasError() { return } + if !(usersInfo.ExtraAttributes.IsNull()) { + attributes += usersInfo.ExtraAttributes.String() + } + var users []iam.User var err error @@ -67,7 +73,7 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, return } - users, err = a.Users.ListAll(ctx, iam.ListAccountUsersRequest{Filter: usersInfo.Filter.ValueString()}) + users, err = a.Users.ListAll(ctx, iam.ListAccountUsersRequest{Filter: usersInfo.Filter.ValueString(), Attributes: attributes}) if err != nil { resp.Diagnostics.AddError("Error listing account users", err.Error()) } diff --git a/internal/providers/pluginfw/resources/user/data_users_acc_test.go b/internal/providers/pluginfw/resources/user/data_users_acc_test.go index 79f9eaeba7..7f656e8eeb 100644 --- a/internal/providers/pluginfw/resources/user/data_users_acc_test.go +++ b/internal/providers/pluginfw/resources/user/data_users_acc_test.go @@ -24,6 +24,27 @@ const dataSourceTemplate = ` } ` +const dataSourceTemplateExtraAttributes = ` + resource "databricks_group" "admins" { + display_name = "admins-{var.STICKY_RANDOM}" + } + + resource "databricks_user" "user1" { + user_name = "tf-{var.STICKY_RANDOM}-1@databricks.com" + } + + resource "databricks_group_member" "membership" { + group_id = databricks_group.admins.id + member_id = databricks_user.user1.id + } + + data "databricks_users" "this" { + filter = "userName eq \"me-{var.STICKY_RANDOM}@example.com\"" + extra_attributes = "groups" + depends_on = [databricks_group_member.membership] + } +` + func checkUsersDataSourcePopulated(t *testing.T) func(s *terraform.State) error { return func(s *terraform.State) error { ds, ok := s.Modules[0].Resources["data.databricks_users.this"] @@ -48,6 +69,30 @@ func checkUsersDataSourcePopulated(t *testing.T) func(s *terraform.State) error } } +func checkUsersDataSourceWithGroups(t *testing.T) func(s *terraform.State) error { + return func(s *terraform.State) error { + ds, ok := s.Modules[0].Resources["data.databricks_users.this"] + require.True(t, ok, "data.databricks_users.this must be present") + + usersCount := ds.Primary.Attributes["users.#"] + require.Equal(t, "1", usersCount, "expected one user") + + userPrefix := "users.0." + + groupsCountAttr := userPrefix + "groups.#" + groupsCount, exists := ds.Primary.Attributes[groupsCountAttr] + require.True(t, exists, "attribute groups.# should be present") + require.Equal(t, "1", groupsCount, "expected one group membership") + + groupIdAttr := userPrefix + "groups.0.value" + groupId, exists := ds.Primary.Attributes[groupIdAttr] + require.True(t, exists, "attribute group.0.value should be present") + + expectedGroupId := s.Modules[0].Resources["databricks_group.admins"].Primary.ID + assert.Equal(t, expectedGroupId, groupId, "group id should match the admins group id") + } +} + func TestAccDataSourceDataUsers(t *testing.T) { acceptance.AccountLevel(t, acceptance.Step{ Template: dataSourceTemplate, @@ -61,3 +106,17 @@ func TestWorkspaceDataSourceDataUsers(t *testing.T) { Check: checkUsersDataSourcePopulated(t), }) } + +func TestAccDataSourceUsers_WithGroups(t *testing.T) { + acceptance.WorkspaceLevel(t, acceptance.Step{ + Template: dataSourceTemplateExtraAttributes, + Check: checkUsersDataSourceWithGroups(t), + }) +} + +func TestWorkspaceDataSourceUsers_WithGroups(t *testing.T) { + acceptance.WorkspaceLevel(t, acceptance.Step{ + Template: dataSourceTemplateExtraAttributes, + Check: checkUsersDataSourceWithGroups(t), + }) +} From 7949b7d5ce9f2c31481e34cebabeec6510c47056 Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Mon, 25 Nov 2024 10:57:58 -0600 Subject: [PATCH 25/27] merged latest changes from main branch and fixed a test definition --- .../providers/pluginfw/resources/user/data_users_acc_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/providers/pluginfw/resources/user/data_users_acc_test.go b/internal/providers/pluginfw/resources/user/data_users_acc_test.go index 7f656e8eeb..3ecd83d30b 100644 --- a/internal/providers/pluginfw/resources/user/data_users_acc_test.go +++ b/internal/providers/pluginfw/resources/user/data_users_acc_test.go @@ -108,7 +108,7 @@ func TestWorkspaceDataSourceDataUsers(t *testing.T) { } func TestAccDataSourceUsers_WithGroups(t *testing.T) { - acceptance.WorkspaceLevel(t, acceptance.Step{ + acceptance.AccountLevel(t, acceptance.Step{ Template: dataSourceTemplateExtraAttributes, Check: checkUsersDataSourceWithGroups(t), }) From 03fad81fb54846b604140445a3bcfa297a2c93fd Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Wed, 27 Nov 2024 10:15:09 -0600 Subject: [PATCH 26/27] made requested changes --- docs/data-sources/users.md | 2 +- internal/providers/pluginfw/resources/user/data_users.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/data-sources/users.md b/docs/data-sources/users.md index 0c0e2d8060..809a316e10 100644 --- a/docs/data-sources/users.md +++ b/docs/data-sources/users.md @@ -48,7 +48,7 @@ This data source allows you to filter the list of users using the following opti filter = "displayName co \"john\" or userName co \"@domain.org\"" ``` -- `extra_attributes` - (Optional) A list of additional user attributes to include in the results. By default, the data source returns the following attributes: `id`, `userName`, `displayName`, and `externalId`. Use this argument to request additional attributes as needed. The list of all available attributes can be found in the [API reference](https://docs.databricks.com/api/workspace/users/list). +- `extra_attributes` - (Optional) A comma-separated list of additional user attributes to include in the results. By default, the data source returns the following attributes: `id`, `userName`, `displayName`, and `externalId`. Use this argument to request additional attributes as needed. The list of all available attributes can be found in the [API reference](https://docs.databricks.com/api/workspace/users/list). ## Attribute Reference diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/resources/user/data_users.go index 4663d212c8..8dc90d5614 100644 --- a/internal/providers/pluginfw/resources/user/data_users.go +++ b/internal/providers/pluginfw/resources/user/data_users.go @@ -52,7 +52,7 @@ func (d *UsersDataSource) Configure(_ context.Context, req datasource.ConfigureR func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) { var usersInfo UsersInfo - var attributes string = "id,userName,displayName,externalId," + attributes := "id,userName,displayName,externalId" resp.Diagnostics.Append(req.Config.Get(ctx, &usersInfo)...) if resp.Diagnostics.HasError() { @@ -60,6 +60,7 @@ func (d *UsersDataSource) Read(ctx context.Context, req datasource.ReadRequest, } if !(usersInfo.ExtraAttributes.IsNull()) { + attributes += "," attributes += usersInfo.ExtraAttributes.String() } From 99d7d2cec3b9130c86c03398619a04885c89c3ef Mon Sep 17 00:00:00 2001 From: dgomez04 Date: Wed, 27 Nov 2024 10:29:05 -0600 Subject: [PATCH 27/27] added files into the products folder and fixed tests --- internal/providers/pluginfw/pluginfw_rollout_utils.go | 2 +- .../pluginfw/{resources => products}/user/data_users.go | 0 .../{resources => products}/user/data_users_acc_test.go | 2 ++ 3 files changed, 3 insertions(+), 1 deletion(-) rename internal/providers/pluginfw/{resources => products}/user/data_users.go (100%) rename internal/providers/pluginfw/{resources => products}/user/data_users_acc_test.go (99%) diff --git a/internal/providers/pluginfw/pluginfw_rollout_utils.go b/internal/providers/pluginfw/pluginfw_rollout_utils.go index 639abc13e6..942a9bf212 100644 --- a/internal/providers/pluginfw/pluginfw_rollout_utils.go +++ b/internal/providers/pluginfw/pluginfw_rollout_utils.go @@ -20,8 +20,8 @@ import ( "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/products/registered_model" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/products/serving" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/products/sharing" + "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/products/user" "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/products/volume" - "github.com/databricks/terraform-provider-databricks/internal/providers/pluginfw/resources/user" "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/resource" ) diff --git a/internal/providers/pluginfw/resources/user/data_users.go b/internal/providers/pluginfw/products/user/data_users.go similarity index 100% rename from internal/providers/pluginfw/resources/user/data_users.go rename to internal/providers/pluginfw/products/user/data_users.go diff --git a/internal/providers/pluginfw/resources/user/data_users_acc_test.go b/internal/providers/pluginfw/products/user/data_users_acc_test.go similarity index 99% rename from internal/providers/pluginfw/resources/user/data_users_acc_test.go rename to internal/providers/pluginfw/products/user/data_users_acc_test.go index 3ecd83d30b..133547b09b 100644 --- a/internal/providers/pluginfw/resources/user/data_users_acc_test.go +++ b/internal/providers/pluginfw/products/user/data_users_acc_test.go @@ -90,6 +90,8 @@ func checkUsersDataSourceWithGroups(t *testing.T) func(s *terraform.State) error expectedGroupId := s.Modules[0].Resources["databricks_group.admins"].Primary.ID assert.Equal(t, expectedGroupId, groupId, "group id should match the admins group id") + + return nil } }