implement resource databricks_service_pricnipal #386
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
One question about the SCIM service principal schema about |
|
Hey @tcz001, TravisBuddy Request Identifier: d7d35b20-17e3-11eb-bc4d-2726ba7bdf50 |
Codecov Report
@@ Coverage Diff @@
## master #386 +/- ##
==========================================
+ Coverage 76.32% 80.86% +4.54%
==========================================
Files 65 63 -2
Lines 6636 6011 -625
==========================================
- Hits 5065 4861 -204
+ Misses 1172 759 -413
+ Partials 399 391 -8
|
|
Hey @tcz001, TravisBuddy Request Identifier: 4053db80-1806-11eb-bc4d-2726ba7bdf50 |
|
Hi @nfx, any feedback? |
nfx
requested changes
Oct 30, 2020
| } | ||
|
|
||
| resource "databricks_service_principal" "me" { | ||
| application_id = "00000000-0000-0000-0000-000000000000" |
There was a problem hiding this comment.
i'm currently running this by our engineering folks to agree on name for this field, because service principal identification might look differently on AWS
There was a problem hiding this comment.
still internal discussions :) i'll try to run this PR against AWS deployment this week
identity/scim.go
Outdated
| @@ -45,6 +46,13 @@ type UserPatchOperations struct { | |||
| Value *GroupsValue `json:"value,omitempty"` | |||
| } | |||
|
|
|||
| // ServicePrincipalPatchOperations is a list of path operations for add or removing service principal attributes | |||
| type ServicePrincipalPatchOperations struct { | |||
There was a problem hiding this comment.
there's no need to create a dedicated *PatchOperations struct, you can reuse the user one.
There was a problem hiding this comment.
sounds good, will change it, but should we rename the struct of UserPatchOperation to make it more abstract?
There was a problem hiding this comment.
@tcz001 yes, it can be just PathOperations
|
+change to databricks_permissions resource doc in this particular case |
…concept into docs
Travis tests have failedHey @tcz001, 1st Buildcurl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsumcurl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0time make linttime make test2nd Buildcurl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsumcurl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0time make lintTravisBuddy Request Identifier: 67ea7ff0-1d51-11eb-8505-818d591f0ed7 |
Travis tests have failedHey @tcz001, 1st Buildcurl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsumcurl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0time make lint2nd Buildcurl -sSL "https://github.com/gotestyourself/gotestsum/releases/download/v0.4.2/gotestsum_0.4.2_linux_amd64.tar.gz" | sudo tar -xz -C /usr/local/bin gotestsumcurl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.25.0time make lintTravisBuddy Request Identifier: 6c03de20-1d64-11eb-8505-818d591f0ed7 |
|
Hey @tcz001, TravisBuddy Request Identifier: 8eafb0f0-1d75-11eb-8505-818d591f0ed7 |
|
Hey @tcz001, TravisBuddy Request Identifier: a296b680-200a-11eb-9dab-419bf2960c30 |
|
Hey @tcz001, TravisBuddy Request Identifier: 34a11d70-2062-11eb-9dab-419bf2960c30 |
|
Hey @tcz001, TravisBuddy Request Identifier: 20f81f60-22bc-11eb-b1e0-17a3b2b69400 |
|
Hi @nfx any further feedback? |
nfx
requested changes
Nov 10, 2020
docs/resources/permissions.md
Outdated
| @@ -92,7 +92,7 @@ resource "databricks_permissions" "policy_usage" { | |||
|
|
|||
| ## Instance Pool usage | |||
|
|
|||
| [Instance Pools](instance_pool.md) access control [allows to](https://docs.databricks.com/security/access-control/pool-acl.html) assign `CAN_ATTACH_TO` and `CAN_MANAGE` permissions to users and groups. It's also possible to grant creation of Instance Pools to individual [groups](group.md#allow_instance_pool_create) and [users](user.md#allow_instance_pool_create). | |||
| [Instance Pools](instance_pool.md) access control [allows to](https://docs.databricks.com/security/access-control/pool-acl.html) assign `CAN_ATTACH_TO` and `CAN_MANAGE` permissions to users, service principals and groups. It's also possible to grant creation of Instance Pools to individual [groups](group.md#allow_instance_pool_create) and [users](user.md#allow_instance_pool_create), [service principals](service_principal.md#allow_instance_pool_create). | |||
There was a problem hiding this comment.
to use consistent punctuation, please add coma before and
There was a problem hiding this comment.
Hi @nfx , sorry I'm a bit lost on these comments because I'm not english native speaker, to save the time, can you maybe directly push to my branch, if you don't have access to push please let me know. https://github.com/tcz001/terraform-provider-databricks-1/invitations
docs/resources/permissions.md
Outdated
| @@ -52,7 +52,7 @@ resource "databricks_permissions" "cluster_usage" { | |||
|
|
|||
| ## Cluster Policy usage | |||
|
|
|||
| Cluster policies allow creation of [clusters](cluster.md), that match [given policy](https://docs.databricks.com/administration-guide/clusters/policies.html). It's possible to assign `CAN_USE` permission to users and groups: | |||
| Cluster policies allow creation of [clusters](cluster.md), that match [given policy](https://docs.databricks.com/administration-guide/clusters/policies.html). It's possible to assign `CAN_USE` permission to users, service principals and groups: | |||
There was a problem hiding this comment.
to use consistent punctuation, please add coma before and
| @@ -128,12 +128,12 @@ resource "databricks_permissions" "pool_usage" { | |||
|
|
|||
| ## Job usage | |||
|
|
|||
There was a problem hiding this comment.
to use consistent punctuation, please add coma before and
docs/resources/permissions.md
Outdated
| @@ -128,12 +128,12 @@ resource "databricks_permissions" "pool_usage" { | |||
|
|
|||
| ## Job usage | |||
|
|
|||
| There are four assignable [permission levels](https://docs.databricks.com/security/access-control/jobs-acl.html#job-permissions) for [databricks_job](job.md): `CAN_VIEW`, `CAN_MANAGE_RUN`, `IS_OWNER`, and `CAN_MANAGE`. Admins are granted the `CAN_MANAGE` permission by default, and they can assign that permission to non-admin users. | |||
| There are four assignable [permission levels](https://docs.databricks.com/security/access-control/jobs-acl.html#job-permissions) for [databricks_job](job.md): `CAN_VIEW`, `CAN_MANAGE_RUN`, `IS_OWNER`, and `CAN_MANAGE`. Admins are granted the `CAN_MANAGE` permission by default, and they can assign that permission to non-admin users and service principals. | |||
There was a problem hiding this comment.
to use consistent punctuation, please add coma before and
docs/resources/permissions.md
Outdated
|
|
||
| * The creator of a job has `IS_OWNER` permission. Destroying `databricks_permissions` resource for a job would revert ownership to creator. | ||
| * A job must have exactly one owner. If resource is changed and no owner is specified, currently authenticated principal would become new owner of the job. Nothing would change, per se, if the job was created through Terraform. | ||
| * A job cannot have a group as an owner. | ||
| * Jobs triggered through *Run Now* assume the permissions of the job owner and not the user who issued Run Now. | ||
| * Jobs triggered through *Run Now* assume the permissions of the job owner and not the user and service principal who issued Run Now. |
There was a problem hiding this comment.
to use consistent punctuation, please add coma before and
docs/resources/permissions.md
Outdated
| * All users have `CAN_MANAGE` permission for items in the Workspace > Shared Icon Shared folder. You can grant `CAN_MANAGE` permission to notebooks and folders by moving them to the Shared Icon Shared folder. | ||
| * All users have `CAN_MANAGE` permission for objects the user creates. | ||
| * User home directory - The user has `CAN_MANAGE` permission. All other users can list their directories. | ||
| * All users(or service principals) have `CAN_MANAGE` permission for items in the Workspace > Shared Icon Shared folder. You can grant `CAN_MANAGE` permission to notebooks and folders by moving them to the Shared Icon Shared folder. |
docs/resources/permissions.md
Outdated
| @@ -273,7 +273,7 @@ resource "databricks_permissions" "folder_usage" { | |||
|
|
|||
| ## Passwords usage | |||
|
|
|||
| By default on AWS deployments, all admin users can sign in to Databricks using either SSO or their username and password, and all API users can authenticate to the Databricks REST APIs using their username and password. As an admin, you [can limit](https://docs.databricks.com/administration-guide/users-groups/single-sign-on/index.html#optional-configure-password-access-control) admin users’ and API users’ ability to authenticate with their username and password by configuring `CAN_USE` permissions using password access control. | |||
| By default on AWS deployments, all admin users(or service principals) can sign in to Databricks using either SSO or their username and password, and all API users(or service principals) can authenticate to the Databricks REST APIs using their username and password. As an admin, you [can limit](https://docs.databricks.com/administration-guide/users-groups/single-sign-on/index.html#optional-configure-password-access-control) admin users’ and API users’ ability to authenticate with their username and password by configuring `CAN_USE` permissions using password access control. | |||
|
@tcz001 good news! we can merge changes with application_id field. please finish up existing comments and let's get ready to merge. |
|
Hi @nfx, we pushed some changes based on your comments regarding the documentation. We are colleagues of @tcz001 and he is away for a while, so our team will take care of the changes in the meantime. Let us know if there is anything else missing from the comments or if there's anything else we need to take care of. Thanks |
nfx
approved these changes
Nov 24, 2020
|
@avillalain please resolve conflicts in |
|
@nfx conflicts are resolved. Thanks |
nfx
added a commit
that referenced
this pull request
Dec 3, 2020
Directly creates service principal, that could be added to a group within workspace.
```hcl
resource "databricks_service_principal" "sp" {
application_id = "00000000-0000-0000-0000-000000000000"
}
```
Co-authored-by: tcz001 <fan.torchz@gmail.com>
Co-authored-by: Angel Villalain Garcia <avillalaingarcia@humana.com>
Co-authored-by: Serge Smertin <serge.smertin@databricks.com>
|
congratulations @tcz001 , @avillalain , it's merged in #432 and is going to be part of 0.3.0 release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should address #28