Skip to content

Commit 0451da3

Browse files
rauchyalexott
andauthored
[Fix] Fix Inconsistent Plan Errors in Permissions Resource (#5091)
## Changes <!-- Summary of your changes that are easy to understand --> This PR attempts to fix an issue where Terraform was intermittently throwing "inconsistent final plan" errors when managing permissions, where `access_control` elements appeared to be removed and re-added even though their actual values were identical. The hypothesis is that Terraform's `TypeSet` was treating `{group_name: "X", permission_level: "Y"}` and `{group_name: "X", permission_level: "Y", user_name: "", service_principal_name: ""}` as different elements because the default hash function considers the presence of keys with empty string values as distinct from absent keys, even though these are semantically equivalent The fix adds a custom hash function that normalizes elements by only hashing non-empty string fields, so both representations produce the same hash and are correctly recognized as identical. ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> Existing test coverage + testing of the new hash function. - [x] `make test` run locally - [ ] relevant change in `docs/` folder - [ ] covered with integration tests in `internal/acceptance` - [ ] using Go SDK - [ ] using TF Plugin Framework - [ ] has entry in `NEXT_CHANGELOG.md` file --------- Co-authored-by: Omer Lachish <[email protected]> Co-authored-by: Alex Ott <[email protected]>
1 parent c464993 commit 0451da3

File tree

3 files changed

+78
-0
lines changed

3 files changed

+78
-0
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010

1111
### Bug Fixes
1212

13+
* Fix Inconsistent Plan Errors in Permissions Resource ([#5091](https://github.com/databricks/terraform-provider-databricks/pull/5091))
14+
1315
### Documentation
1416

1517
### Exporter

permissions/resource_permissions.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,32 @@ func ResourcePermissions() common.Resource {
181181
}
182182
}
183183
s["access_control"].MinItems = 1
184+
185+
// Use a custom hash function that only considers non-empty fields.
186+
// This prevents spurious diffs when comparing {group_name: "X", permission_level: "Y"}
187+
// with {group_name: "X", permission_level: "Y", service_principal_name: "", user_name: ""}
188+
acSchema := s["access_control"].Elem.(*schema.Resource).Schema
189+
s["access_control"].Set = func(v interface{}) int {
190+
m, ok := v.(map[string]interface{})
191+
if !ok {
192+
return 0
193+
}
194+
// Build a normalized map with only non-empty string fields for hashing
195+
normalized := make(map[string]interface{})
196+
for key, val := range m {
197+
if _, exists := acSchema[key]; !exists {
198+
continue // Skip fields not in schema
199+
}
200+
if strVal, ok := val.(string); ok {
201+
if strVal != "" {
202+
normalized[key] = strVal
203+
}
204+
}
205+
}
206+
// Use HashResource with a schema that only includes the fields we care about
207+
hashSchema := &schema.Resource{Schema: acSchema}
208+
return schema.HashResource(hashSchema)(normalized)
209+
}
184210
return s
185211
})
186212
return common.Resource{

permissions/resource_permissions_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1868,3 +1868,53 @@ func TestResourcePermissionsRootDirectory(t *testing.T) {
18681868
assert.Equal(t, TestingUser, firstElem["user_name"])
18691869
assert.Equal(t, "CAN_READ", firstElem["permission_level"])
18701870
}
1871+
1872+
// TestAccessControlHashFunction verifies that the custom hash function treats
1873+
// access_control elements with and without empty fields as equal.
1874+
// This prevents "inconsistent final plan" errors reported in ES-1587799.
1875+
func TestAccessControlHashFunction(t *testing.T) {
1876+
resource := ResourcePermissions()
1877+
s := resource.ToResource().Schema
1878+
acSchema := s["access_control"]
1879+
1880+
require.NotNil(t, acSchema.Set, "access_control should have a custom Set function")
1881+
1882+
// Test case 1: Elements with only populated fields should hash the same
1883+
// as elements with explicit empty fields
1884+
elem1 := map[string]interface{}{
1885+
"group_name": "RSDB_Databricks_TSR_Residential_RW",
1886+
"permission_level": "CAN_USE",
1887+
}
1888+
elem2 := map[string]interface{}{
1889+
"group_name": "RSDB_Databricks_TSR_Residential_RW",
1890+
"permission_level": "CAN_USE",
1891+
"service_principal_name": "",
1892+
"user_name": "",
1893+
}
1894+
hash1 := acSchema.Set(elem1)
1895+
hash2 := acSchema.Set(elem2)
1896+
assert.Equal(t, hash1, hash2, "Elements with and without empty fields should have the same hash")
1897+
1898+
// Test case 2: Different elements should have different hashes
1899+
elem3 := map[string]interface{}{
1900+
"user_name": "different_user",
1901+
"permission_level": "CAN_USE",
1902+
}
1903+
hash3 := acSchema.Set(elem3)
1904+
assert.NotEqual(t, hash1, hash3, "Different elements should have different hashes")
1905+
1906+
// Test case 3: Service principal with and without empty fields
1907+
elem4 := map[string]interface{}{
1908+
"service_principal_name": "6d1fc824-191d-4a99-9fac-f62a27822946",
1909+
"permission_level": "CAN_USE",
1910+
}
1911+
elem5 := map[string]interface{}{
1912+
"service_principal_name": "6d1fc824-191d-4a99-9fac-f62a27822946",
1913+
"permission_level": "CAN_USE",
1914+
"group_name": "",
1915+
"user_name": "",
1916+
}
1917+
hash4 := acSchema.Set(elem4)
1918+
hash5 := acSchema.Set(elem5)
1919+
assert.Equal(t, hash4, hash5, "Service principal elements with and without empty fields should have the same hash")
1920+
}

0 commit comments

Comments
 (0)