diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 4064c7eb89..f1d710c3ed 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -11,5 +11,6 @@ ### Bundles * For secret scopes, no longer remove current user's permissions ([#3780](https://github.com/databricks/cli/pull/3780)) * Automatically add owner permissions during bundle initialization, this makes final permissions visible in 'bundle validate -o json' ([#3780](https://github.com/databricks/cli/pull/3780)) +* Fix permissions for 'models' resource ([#3786](https://github.com/databricks/cli/pull/3786)) ### API Changes diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/databricks.yml b/acceptance/bundle/resources/permissions/models/current_can_manage/databricks.yml new file mode 100644 index 0000000000..311dd52ba2 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/databricks.yml @@ -0,0 +1,13 @@ +resources: + models: + foo: + name: test-model + permissions: + - level: CAN_READ + user_name: viewer@example.com + - level: CAN_MANAGE + group_name: data-team + - level: CAN_MANAGE + service_principal_name: f37d18cd-98a8-4db5-8112-12dd0a6bfe38 + - level: CAN_MANAGE + user_name: tester@databricks.com diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.direct-exp.json b/acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.direct-exp.json new file mode 100644 index 0000000000..4e5919cf6d --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.direct-exp.json @@ -0,0 +1,12 @@ +{ + "plan": { + "resources.models.foo": { + "action": "create", + "new_state": { + "config": { + "name": "test-model" + } + } + } + } +} diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.terraform.json b/acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.terraform.json new file mode 100644 index 0000000000..504da4ab9c --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/out.plan.terraform.json @@ -0,0 +1,7 @@ +{ + "plan": { + "resources.models.foo": { + "action": "create" + } + } +} diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.direct-exp.json b/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.direct-exp.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.terraform.json new file mode 100644 index 0000000000..9caf90946a --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.deploy.terraform.json @@ -0,0 +1,24 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/registered-models/test-model", + "body": { + "access_control_list": [ + { + "permission_level": "CAN_READ", + "user_name": "viewer@example.com" + }, + { + "permission_level": "CAN_MANAGE", + "service_principal_name": "[UUID]" + }, + { + "group_name": "data-team", + "permission_level": "CAN_MANAGE" + }, + { + "permission_level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.destroy.direct-exp.json b/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.destroy.direct-exp.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.destroy.terraform.json b/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.destroy.terraform.json new file mode 100644 index 0000000000..220f23c019 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/out.requests.destroy.terraform.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/registered-models/test-model", + "body": { + "access_control_list": [ + { + "permission_level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/out.test.toml b/acceptance/bundle/resources/permissions/models/current_can_manage/out.test.toml new file mode 100644 index 0000000000..e092fd5ed6 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct-exp"] diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/output.txt b/acceptance/bundle/resources/permissions/models/current_can_manage/output.txt new file mode 100644 index 0000000000..6d73184cf9 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/output.txt @@ -0,0 +1,35 @@ + +>>> [CLI] bundle validate -o json +[ + { + "level": "CAN_READ", + "user_name": "viewer@example.com" + }, + { + "group_name": "data-team", + "level": "CAN_MANAGE" + }, + { + "level": "CAN_MANAGE", + "service_principal_name": "[UUID]" + }, + { + "level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } +] + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete model foo + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/script b/acceptance/bundle/resources/permissions/models/current_can_manage/script new file mode 100644 index 0000000000..7d1e9fc8e2 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/current_can_manage/script @@ -0,0 +1 @@ +source $TESTDIR/../../_script diff --git a/acceptance/bundle/resources/permissions/models/current_can_manage/test.toml b/acceptance/bundle/resources/permissions/models/current_can_manage/test.toml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/databricks.yml b/acceptance/bundle/resources/permissions/models/other_can_manage/databricks.yml new file mode 100644 index 0000000000..c12d605d2f --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/databricks.yml @@ -0,0 +1,7 @@ +resources: + models: + foo: + name: test-model + permissions: + - level: CAN_MANAGE + service_principal_name: f37d18cd-98a8-4db5-8112-12dd0a6bfe38 diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.plan.direct-exp.json b/acceptance/bundle/resources/permissions/models/other_can_manage/out.plan.direct-exp.json new file mode 100644 index 0000000000..4e5919cf6d --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/out.plan.direct-exp.json @@ -0,0 +1,12 @@ +{ + "plan": { + "resources.models.foo": { + "action": "create", + "new_state": { + "config": { + "name": "test-model" + } + } + } + } +} diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.plan.terraform.json b/acceptance/bundle/resources/permissions/models/other_can_manage/out.plan.terraform.json new file mode 100644 index 0000000000..504da4ab9c --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/out.plan.terraform.json @@ -0,0 +1,7 @@ +{ + "plan": { + "resources.models.foo": { + "action": "create" + } + } +} diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.deploy.direct-exp.json b/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.deploy.direct-exp.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.deploy.terraform.json new file mode 100644 index 0000000000..4b085b72fa --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.deploy.terraform.json @@ -0,0 +1,16 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/registered-models/test-model", + "body": { + "access_control_list": [ + { + "permission_level": "CAN_MANAGE", + "service_principal_name": "[UUID]" + }, + { + "permission_level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.destroy.direct-exp.json b/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.destroy.direct-exp.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.destroy.terraform.json b/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.destroy.terraform.json new file mode 100644 index 0000000000..220f23c019 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/out.requests.destroy.terraform.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/registered-models/test-model", + "body": { + "access_control_list": [ + { + "permission_level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/out.test.toml b/acceptance/bundle/resources/permissions/models/other_can_manage/out.test.toml new file mode 100644 index 0000000000..e092fd5ed6 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct-exp"] diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/output.txt b/acceptance/bundle/resources/permissions/models/other_can_manage/output.txt new file mode 100644 index 0000000000..1970522e9e --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/output.txt @@ -0,0 +1,27 @@ + +>>> [CLI] bundle validate -o json +[ + { + "level": "CAN_MANAGE", + "service_principal_name": "[UUID]" + }, + { + "level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } +] + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete model foo + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/script b/acceptance/bundle/resources/permissions/models/other_can_manage/script new file mode 100644 index 0000000000..7d1e9fc8e2 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/other_can_manage/script @@ -0,0 +1 @@ +source $TESTDIR/../../_script diff --git a/acceptance/bundle/resources/permissions/models/other_can_manage/test.toml b/acceptance/bundle/resources/permissions/models/other_can_manage/test.toml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/test.toml b/acceptance/bundle/resources/permissions/models/test.toml new file mode 100644 index 0000000000..2669d8b2b4 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/test.toml @@ -0,0 +1 @@ +Env.RESOURCE = "models" # for ../_script diff --git a/acceptance/bundle/resources/permissions/models/viewer/databricks.yml b/acceptance/bundle/resources/permissions/models/viewer/databricks.yml new file mode 100644 index 0000000000..e8ff008690 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/databricks.yml @@ -0,0 +1,7 @@ +resources: + models: + foo: + name: test-model + permissions: + - level: CAN_READ + user_name: viewer@example.com diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.plan.direct-exp.json b/acceptance/bundle/resources/permissions/models/viewer/out.plan.direct-exp.json new file mode 100644 index 0000000000..4e5919cf6d --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/out.plan.direct-exp.json @@ -0,0 +1,12 @@ +{ + "plan": { + "resources.models.foo": { + "action": "create", + "new_state": { + "config": { + "name": "test-model" + } + } + } + } +} diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.plan.terraform.json b/acceptance/bundle/resources/permissions/models/viewer/out.plan.terraform.json new file mode 100644 index 0000000000..504da4ab9c --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/out.plan.terraform.json @@ -0,0 +1,7 @@ +{ + "plan": { + "resources.models.foo": { + "action": "create" + } + } +} diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.requests.deploy.direct-exp.json b/acceptance/bundle/resources/permissions/models/viewer/out.requests.deploy.direct-exp.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.requests.deploy.terraform.json b/acceptance/bundle/resources/permissions/models/viewer/out.requests.deploy.terraform.json new file mode 100644 index 0000000000..d2ffa103da --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/out.requests.deploy.terraform.json @@ -0,0 +1,16 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/registered-models/test-model", + "body": { + "access_control_list": [ + { + "permission_level": "CAN_READ", + "user_name": "viewer@example.com" + }, + { + "permission_level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.requests.destroy.direct-exp.json b/acceptance/bundle/resources/permissions/models/viewer/out.requests.destroy.direct-exp.json new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.requests.destroy.terraform.json b/acceptance/bundle/resources/permissions/models/viewer/out.requests.destroy.terraform.json new file mode 100644 index 0000000000..220f23c019 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/out.requests.destroy.terraform.json @@ -0,0 +1,12 @@ +{ + "method": "PUT", + "path": "/api/2.0/permissions/registered-models/test-model", + "body": { + "access_control_list": [ + { + "permission_level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } + ] + } +} diff --git a/acceptance/bundle/resources/permissions/models/viewer/out.test.toml b/acceptance/bundle/resources/permissions/models/viewer/out.test.toml new file mode 100644 index 0000000000..e092fd5ed6 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct-exp"] diff --git a/acceptance/bundle/resources/permissions/models/viewer/output.txt b/acceptance/bundle/resources/permissions/models/viewer/output.txt new file mode 100644 index 0000000000..87589749a2 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/output.txt @@ -0,0 +1,27 @@ + +>>> [CLI] bundle validate -o json +[ + { + "level": "CAN_READ", + "user_name": "viewer@example.com" + }, + { + "level": "CAN_MANAGE", + "user_name": "[USERNAME]" + } +] + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete model foo + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/permissions/models/viewer/script b/acceptance/bundle/resources/permissions/models/viewer/script new file mode 100644 index 0000000000..7d1e9fc8e2 --- /dev/null +++ b/acceptance/bundle/resources/permissions/models/viewer/script @@ -0,0 +1 @@ +source $TESTDIR/../../_script diff --git a/acceptance/bundle/resources/permissions/models/viewer/test.toml b/acceptance/bundle/resources/permissions/models/viewer/test.toml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/bundle/deploy/terraform/tfdyn/convert_model.go b/bundle/deploy/terraform/tfdyn/convert_model.go index 722f3aa636..fd988069c9 100644 --- a/bundle/deploy/terraform/tfdyn/convert_model.go +++ b/bundle/deploy/terraform/tfdyn/convert_model.go @@ -38,7 +38,7 @@ func (modelConverter) Convert(ctx context.Context, key string, vin dyn.Value, ou // Configure permissions for this resource. if permissions := convertPermissionsResource(ctx, vin); permissions != nil { - permissions.RegisteredModelId = fmt.Sprintf("${databricks_mlflow_model.%s.registered_model_id}", key) + permissions.RegisteredModelId = fmt.Sprintf("${databricks_mlflow_model.%s.id}", key) out.Permissions["mlflow_model_"+key] = permissions } diff --git a/bundle/deploy/terraform/tfdyn/convert_model_test.go b/bundle/deploy/terraform/tfdyn/convert_model_test.go index 0b36034514..58e368681f 100644 --- a/bundle/deploy/terraform/tfdyn/convert_model_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_model_test.go @@ -63,7 +63,7 @@ func TestConvertModel(t *testing.T) { // Assert equality on the permissions assert.Equal(t, &schema.ResourcePermissions{ - RegisteredModelId: "${databricks_mlflow_model.my_model.registered_model_id}", + RegisteredModelId: "${databricks_mlflow_model.my_model.id}", AccessControl: []schema.ResourcePermissionsAccessControl{ { PermissionLevel: "CAN_READ",