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

SQL Endpoint Permissions - Unable to remove permissions #1163

Closed
jdafoe1 opened this issue Mar 7, 2022 · 1 comment · Fixed by #1172
Closed

SQL Endpoint Permissions - Unable to remove permissions #1163

jdafoe1 opened this issue Mar 7, 2022 · 1 comment · Fixed by #1172

Comments

@jdafoe1
Copy link

jdafoe1 commented Mar 7, 2022

databricks_permissions resource doesn't remove permissions when applied to SQL Endpoints

Configuration

The following configuration comments out GROUP_TO_REMOVE. On Terarform apply, the Group should be removed from SQL endpoint permissions

resource "databricks_permissions" "sql_endpoint_test" {
  sql_endpoint_id = databricks_sql_endpoint.test.id

  access_control {
    group_name       = "GROUP_TO_KEEP"
    permission_level = "CAN_USE"
  }
  
  # REMOVE THIS GROUP
  #access_control {
  #  group_name       = "GROUP_TO_REMOVE"
  #  permission_level = "CAN_USE"
  #}

}

Terarform apply shows changes successfully applied:

Terraform will perform the following actions:

  # databricks_permissions.sql_endpoint_test will be updated in-place
  ~ resource "databricks_permissions" "sql_endpoint_test" {
        id              = "/sql/endpoints/f9e5be351fb914fb"
        # (2 unchanged attributes hidden)

      - access_control {
          - group_name       = "GROUP_TO_REMOVE" -> null
          - permission_level = "CAN_USE" -> null
        }
      + access_control {
          + group_name       = "GROUP_TO_KEEP"
          + permission_level = "CAN_USE"
        }
      - access_control {
          - group_name       = "GROUP_TO_KEEP" -> null
          - permission_level = "CAN_USE" -> null
        }
    }
	
Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

Apply complete! Resources: 0 added, 1 changed, 0 destroyed.

Expected Behaviour

After a Terraform apply, GROUP_TO_REMOVE is removed from permissions on Databricks SQL Endpoint

Actual Behaviour

GROUP_TO_REMOVE doesn't get removed on the Databricks SQL Endpoint. Performing a subsequent plan/apply shows same changes need to be made

Terraform will perform the following actions:

  # databricks_permissions.sql_endpoint_test will be updated in-place
  ~ resource "databricks_permissions" "sql_endpoint_test" {
        id              = "/sql/endpoints/f9e5be351fb914fb"
        # (2 unchanged attributes hidden)

      - access_control {
          - group_name       = "GROUP_TO_REMOVE" -> null
          - permission_level = "CAN_USE" -> null
        }
      + access_control {
          + group_name       = "GROUP_TO_KEEP"
          + permission_level = "CAN_USE"
        }
      - access_control {
          - group_name       = "GROUP_TO_KEEP" -> null
          - permission_level = "CAN_USE" -> null
        }
    }
	
Plan: 0 to add, 1 to change, 0 to destroy.

Steps to reproduce

Apply a databricks_permissions resource on an sql endpoint with an initial set of group permissions. Remove a group permission and run a Terraform apply. The group is not removed.

Terraform and provider versions

Terarform Version 1.0.11
Databricks Provider 0.5.2

Important Factoids

Looking at the DEBUG logs, it seems a PATCH API call is done. Reading the Databricks API docs, shouldn't that be a PUT API call, so that user permisions are overwritten?

2022-03-07T13:22:40.401-0500 [DEBUG] provider.terraform-provider-databricks_v0.5.2: PATCH /api/2.0/permissions/sql/endpoints/f9e5be351fb914fb {
  "access_control_list": [
    {
      "group_name": "GROUP_TO_KEEP",
      "permission_level": "CAN_USE"
    },
    {
      "permission_level": "CAN_MANAGE",
      "user_name": "<redacted>"
    }
  ]
}: timestamp=2022-03-07T13:22:40.401-0500
2022-03-07T13:22:40.801-0500 [DEBUG] provider.terraform-provider-databricks_v0.5.2: 200 OK  {
  "access_control_list": [
    {
      "all_permissions": [
        {
          "inherited": false,
          "permission_level": "CAN_MANAGE"
        }
      ],
      "service_principal_name": "<redacted>"
    },
    {
      "all_permissions": [
        {
          "inherited": false,
          "permission_level": "CAN_USE"
        }
      ],
      "group_name": "GROUP_TO_REMOVE"
    },
    {
      "all_permissions": [
        {
          "inherited": false,
          "permission_level": "CAN_USE"
        }
      ],
      "group_name": "GROUP_TO_KEEP"
    },
      {
      "all_permissions": [
      ... (295 more bytes) <- PATCH /api/2.0/permissions/sql/endpoints/f9e5be351fb914fb: timestamp=2022-03-07T13:22:40.801-0500

PATCH /permissions/sql/endpoints/{endpoint_id}
https://docs.databricks.com/dev-tools/api/latest/permissions.html#operation/set-sqlendpoint-permissions
Grant SQL endpoint permissions for one or more users, groups, or service principals.
This request only grants (adds) permissions. To revoke, use the replace all SQL endpoint permissions operation.

PUT /permissions/sql/endpoints/{endpoint_id}
https://docs.databricks.com/dev-tools/api/latest/permissions.html#operation/update-all-sqlendpoint-permissions
Update all permissions for a specific SQL endpoint, specifying all users, groups or service principal.
WARNING: This request overwrites all existing direct (non-inherited) permissions on the SQL endpoint and replaces it with the new permissions specified in the request body.

@nfx
Copy link
Contributor

nfx commented Mar 8, 2022

https://github.com/databrickslabs/terraform-provider-databricks/blame/master/permissions/resource_permissions.go#L135-L140 we're using PATCH for historical reasons. we'll look into it, given the cycles allowing.

nfx added a commit that referenced this issue Mar 10, 2022
Use correct HTTP verb for `databricks_permissions` on `databricks_sql_endpoint`. Authorized user, assumingly part of `admins` group, is no longer sending `CAN_MANAGE` permission in the HTTP PUT request.

Fixes #1163
nfx added a commit that referenced this issue Mar 10, 2022
Use correct HTTP verb for `databricks_permissions` on `databricks_sql_endpoint`. Authorized user, assumingly part of `admins` group, is no longer sending `CAN_MANAGE` permission in the HTTP PUT request.

Fixes #1163
@nfx nfx closed this as completed in #1172 Mar 11, 2022
nfx added a commit that referenced this issue Mar 11, 2022
* Use correct verb for permissions on SQL Endpoint

Use correct HTTP verb for `databricks_permissions` on `databricks_sql_endpoint`. Authorized user, assumingly part of `admins` group, is no longer sending `CAN_MANAGE` permission in the HTTP PUT request.

Fixes #1163
@nfx nfx mentioned this issue Mar 14, 2022
michael-berk pushed a commit to michael-berk/terraform-provider-databricks that referenced this issue Feb 15, 2023
* Use correct verb for permissions on SQL Endpoint

Use correct HTTP verb for `databricks_permissions` on `databricks_sql_endpoint`. Authorized user, assumingly part of `admins` group, is no longer sending `CAN_MANAGE` permission in the HTTP PUT request.

Fixes databricks#1163
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants