Skip to content

Commit c86fe00

Browse files
mgyuchtclaudealexott
authored
Fix databricks_grant & databricks_grants failing with empty permissions diff for shares (#5095)
## Changes When using `databricks_grant` or `databricks_grants` on a share where permissions are already correctly configured, the provider would make a PATCH API call with an empty changes array. For shares, this gets serialized as an empty body {}, causing the API to return: "Missing required field: permissions_diff". This fix skips the UpdatePermissions call when diff is empty. This also has the side effect of this skipping any waiting for permissions to update, improving performance. We can do this for all permission types, so the performance improvement applies to all securable types, not just shares. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Tests Added an additional unit test for this case. --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Alex Ott <[email protected]>
1 parent a5146e5 commit c86fe00

File tree

5 files changed

+97
-3
lines changed

5 files changed

+97
-3
lines changed

NEXT_CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22

33
## Release v1.95.0
44

5-
### Breaking Changes
5+
### New Features and Improvements
6+
7+
* Add `arm` option to `databricks_node_type` instead of `graviton` ([#5028](https://github.com/databricks/terraform-provider-databricks/pull/5028))
8+
* Optimize `databricks_grant` and `databricks_grants` to not call the `Update` API if the requested permissions are already granted ([#5095](https://github.com/databricks/terraform-provider-databricks/pull/5095))
69

710
### New Features and Improvements
811

catalog/resource_grant.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,12 @@ func replacePermissionsForPrincipal(a permissions.UnityCatalogPermissionsAPI, se
7272
if err != nil {
7373
return err
7474
}
75-
err = a.UpdatePermissions(securableType, name, diffPermissionsForPrincipal(principal, list.PrivilegeAssignments, existing.PrivilegeAssignments))
75+
diff := diffPermissionsForPrincipal(principal, list.PrivilegeAssignments, existing.PrivilegeAssignments)
76+
if len(diff) == 0 {
77+
// The permissions are already correct, no need to update or wait
78+
return nil
79+
}
80+
err = a.UpdatePermissions(securableType, name, diff)
7681
if err != nil {
7782
return err
7883
}

catalog/resource_grant_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,3 +849,43 @@ func TestResourceGrantModelGrantCreate(t *testing.T) {
849849
`,
850850
}.ApplyNoError(t)
851851
}
852+
853+
func TestResourceGrantShareGrantCreateNoChange(t *testing.T) {
854+
qa.ResourceFixture{
855+
Fixtures: []qa.HTTPFixture{
856+
{
857+
Method: "GET",
858+
Resource: "/api/2.1/unity-catalog/shares/myshare/permissions?",
859+
Response: catalog.GetPermissionsResponse{
860+
PrivilegeAssignments: []catalog.PrivilegeAssignment{
861+
{
862+
Principal: "recipient1",
863+
Privileges: []catalog.Privilege{"SELECT"},
864+
},
865+
},
866+
},
867+
},
868+
// No PATCH request should be made since permissions are already correct
869+
{
870+
Method: "GET",
871+
Resource: "/api/2.1/unity-catalog/shares/myshare/permissions?",
872+
Response: catalog.GetPermissionsResponse{
873+
PrivilegeAssignments: []catalog.PrivilegeAssignment{
874+
{
875+
Principal: "recipient1",
876+
Privileges: []catalog.Privilege{"SELECT"},
877+
},
878+
},
879+
},
880+
},
881+
},
882+
Resource: ResourceGrant(),
883+
Create: true,
884+
HCL: `
885+
share = "myshare"
886+
887+
principal = "recipient1"
888+
privileges = ["SELECT"]
889+
`,
890+
}.ApplyNoError(t)
891+
}

catalog/resource_grants.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,12 @@ func replaceAllPermissions(a permissions.UnityCatalogPermissionsAPI, securable s
8080
if err != nil {
8181
return err
8282
}
83-
err = a.UpdatePermissions(securableType, name, diffPermissions(list.PrivilegeAssignments, existing.PrivilegeAssignments))
83+
diff := diffPermissions(list.PrivilegeAssignments, existing.PrivilegeAssignments)
84+
if len(diff) == 0 {
85+
// The permissions are already correct, no need to update or wait
86+
return nil
87+
}
88+
err = a.UpdatePermissions(securableType, name, diff)
8489
if err != nil {
8590
return err
8691
}

catalog/resource_grants_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,47 @@ func TestShareGrantUpdate(t *testing.T) {
581581
}.ApplyNoError(t)
582582
}
583583

584+
func TestShareGrantCreateNoChange(t *testing.T) {
585+
qa.ResourceFixture{
586+
Fixtures: []qa.HTTPFixture{
587+
{
588+
Method: "GET",
589+
Resource: "/api/2.1/unity-catalog/shares/myshare/permissions?",
590+
Response: catalog.GetPermissionsResponse{
591+
PrivilegeAssignments: []catalog.PrivilegeAssignment{
592+
{
593+
Principal: "recipient1",
594+
Privileges: []catalog.Privilege{"SELECT"},
595+
},
596+
},
597+
},
598+
},
599+
// No PATCH request should be made since permissions are already correct
600+
{
601+
Method: "GET",
602+
Resource: "/api/2.1/unity-catalog/shares/myshare/permissions?",
603+
Response: catalog.GetPermissionsResponse{
604+
PrivilegeAssignments: []catalog.PrivilegeAssignment{
605+
{
606+
Principal: "recipient1",
607+
Privileges: []catalog.Privilege{"SELECT"},
608+
},
609+
},
610+
},
611+
},
612+
},
613+
Resource: ResourceGrants(),
614+
Create: true,
615+
HCL: `
616+
share = "myshare"
617+
618+
grant {
619+
principal = "recipient1"
620+
privileges = ["SELECT"]
621+
}`,
622+
}.ApplyNoError(t)
623+
}
624+
584625
func TestConnectionGrantCreate(t *testing.T) {
585626
qa.ResourceFixture{
586627
Fixtures: []qa.HTTPFixture{

0 commit comments

Comments
 (0)