Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Extends the attribute of the databricks_group data source #1085

Closed
yb-yu opened this issue Feb 3, 2022 · 8 comments · Fixed by #1103
Closed

[FEATURE] Extends the attribute of the databricks_group data source #1085

yb-yu opened this issue Feb 3, 2022 · 8 comments · Fixed by #1103
Labels
feature New feature or request

Comments

@yb-yu
Copy link

yb-yu commented Feb 3, 2022

Currently, the members field of the databricks_group data source holds not only the user id but also the sub-group id. (Unlike the document, the groups field actually represents the parent group to which the group belongs.)

The corresponding data source uses SCIM Group API, and it also returns the user id and sub-group id by putting them in the samemembers field. The following is the part of response of SCIM Groups API.

{
  "members": [
    {
      "display": "<Display Name of the User>,
      "value": "<ID of the User>,
      "$ref": "Users/<ID of the User>
    },
    {
      "display": "<Display Name of the Group>,
      "value": "<ID of the Group>,
      "$ref": "Groups/<ID of the Group>
    },
  ]
}

My suggestion is to expand the databricks_group data source to put the entire object of response as well as the id of the resource in the members field and parse the ref field to add type, or add the ref field as it is so that the user can parse.

Current Behavior

> data.databricks_group.admins.members
[
  "<id of the user A>",
  "<id of the user B>",
  "<id of the group A>",
  "<id of the group B>",
]

Expected Behavior 1

> data.databricks_group.admins.members
[
  {
    "display_name": "<display name of user A>",
    "id": "<id of user A>",
    "type": "USER"
  },
  {
    "display_name": "<display name of group A>",
    "id": "<id of group A>",
    "type": "Group"
  },
]

Expected Behavior 2

> data.databricks_group.admins.members
[
  {
    "display_name": "<display name of user A>",
    "id": "<id of user A>",
    "$ref": "Users/<id of user A>"
  },
  {
    "display_name": "<display name of group A>",
    "id": "<id of group A>",
    "$ref": "Groups/<id of group A>"
  },
]
@yb-yu yb-yu changed the title [FEATURE] Extends the attribute of the data source databricks_group [FEATURE] Extends the attribute of the databricks_group data source Feb 3, 2022
@nfx
Copy link
Contributor

nfx commented Feb 3, 2022

@Kloty there's user_id field for databricks_user data resource - https://registry.terraform.io/providers/databrickslabs/databricks/latest/docs/data-sources/user

we won't change the datatype of members attribute for backwards compatibility reasons and this field would always contain ids of members.

@yb-yu
Copy link
Author

yb-yu commented Feb 3, 2022

@nfx However, if you put the wrong user_id (e.g. group id) into the databricks_user data source, it spits out the error. So it can't be used to distinguish between user and group.

@yb-yu
Copy link
Author

yb-yu commented Feb 3, 2022

AFAIK, there is no way to distinguish member's type from the databricks_group data source at this moment.
Then how about adding a field that can filter the members field as follows?

data "databricks_group" "admins" {
  display_name = "admins"
  members_type = "USER"
}

data "databricks_group" "admins" {
  display_name = "admins"
  members_type = "GROUP"
}

@nfx
Copy link
Contributor

nfx commented Feb 3, 2022

However, if you put the wrong user_id (e.g. group id) into the databricks_user data source, it spits out the error. So it can't be used to distinguish between user and group.

could you specify that error?

@yb-yu
Copy link
Author

yb-yu commented Feb 3, 2022

It's an error similar to the following. Error: User with id 5432154321 not found.
If I call SCIM Group API with it's ID, I can get a normal response, and I knew it was a group.

Full hcl code is similar to the following.

data "databricks_group" "this" {
  display_name = "some group name"
}

data "databricks_user" "department_members" {
  for_each = data.databricks_group.this.members
  user_id  = each.value
}

@yb-yu
Copy link
Author

yb-yu commented Feb 4, 2022

Like the azuread_users data source, it would be nice to have a data source that can fetch multiple users and ignore errors. There is field named ignore_missing to ignore missing users and return users that were found.

@nfx
Copy link
Contributor

nfx commented Feb 4, 2022

@Kloty it's a separate feature request :) there are two ways of getting your feature request implemented sooner: have plenty of customers asking for it or making a pull request.

features are prioritised by two things: number of customers requesting that and complexity of implementation - often it makes very little sense to implement something just for one customer.

if you decide to to send a pull request - we'll guide and align you through reviews.

@yb-yu
Copy link
Author

yb-yu commented Feb 4, 2022

Ok, I understand what you're saying.

I want to go back to your question. For databricks_users data source, did you ask because there is a way to handle it without spitting error or am I using it in the wrong way?

@nfx nfx added the feature New feature or request label Feb 9, 2022
@nfx nfx closed this as completed in #1103 Feb 11, 2022
nfx added a commit that referenced this issue Feb 11, 2022
# Version changelog

## 0.4.9

* Prevent creation of `databricks_group` with `users` and `admins` reserved names ([#1089](#1089)).
* Added support for shared clusters in multi-task `databricks_job` ([#1082](#1082)).
* Added diff suppression for `external_id` in `databricks_group` ([#1099](#1099)).
* Added diff suppression for `external_id` in `databricks_user` ([#1097](#1097)).
* Added `users`, `service_principals`, and `child_groups` exported properties to `databricks_group` data resource ([#1085](#1085)).
* Added various documentation improvements.
@nfx nfx mentioned this issue Feb 11, 2022
nfx added a commit that referenced this issue Feb 11, 2022
* Release v0.4.9

# Version changelog

## 0.4.9

* Prevent creation of `databricks_group` with `users` and `admins` reserved names ([#1089](#1089)).
* Added support for shared clusters in multi-task `databricks_job` ([#1082](#1082)).
* Added diff suppression for `external_id` in `databricks_group` ([#1099](#1099)).
* Added diff suppression for `external_id` in `databricks_user` ([#1097](#1097)).
* Added `users`, `service_principals`, and `child_groups` exported properties to `databricks_group` data resource ([#1085](#1085)).
* Added various documentation improvements.
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this issue Feb 15, 2023
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this issue Feb 15, 2023
* Release v0.4.9

# Version changelog

## 0.4.9

* Prevent creation of `databricks_group` with `users` and `admins` reserved names ([databricks#1089](databricks#1089)).
* Added support for shared clusters in multi-task `databricks_job` ([databricks#1082](databricks#1082)).
* Added diff suppression for `external_id` in `databricks_group` ([databricks#1099](databricks#1099)).
* Added diff suppression for `external_id` in `databricks_user` ([databricks#1097](databricks#1097)).
* Added `users`, `service_principals`, and `child_groups` exported properties to `databricks_group` data resource ([databricks#1085](databricks#1085)).
* Added various documentation improvements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants