Skip to content

Commit e4e26cb

Browse files
pieternclaude
andcommitted
Add depends_on chain for secret scope ACLs to fix flaky permission removal
The secret scope permissions test fails ~33% of the time when using the Terraform bundle engine. The root cause is a backend API race condition where parallel ACL modifications return inconsistent results. The direct bundle engine already works around this by sequentializing ACL operations (see #3886): // Set ACLs. The service returns inconsistent results for parallel // API calls. That's why we do them sequentially here to maintain // correctness. The Terraform provider has a similar workaround for creates via robustPutACL with retry/verification (terraform-provider-databricks#4885, issue #4195), but deletions have no such protection. This change adds a depends_on chain between ACL resources, forcing Terraform to execute them sequentially: ACL_0 → depends_on: [scope] ACL_1 → depends_on: [scope, ACL_0] ACL_2 → depends_on: [scope, ACL_1] This avoids triggering the backend race condition without requiring changes to the Terraform provider. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent ec58ff2 commit e4e26cb

File tree

2 files changed

+115
-1
lines changed

2 files changed

+115
-1
lines changed

bundle/deploy/terraform/tfdyn/convert_secret_scope.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ type resourceSecretAcl struct {
1919
type secretScopeConverter struct{}
2020

2121
func convertPermissionsSecretScope(key, scopeName string, permissions []dyn.Value, out *schema.Resources) {
22+
var previousAclKey string
23+
2224
for idx, permission := range permissions {
2325
level, _ := permission.Get("level").AsString()
2426
userName, _ := permission.Get("user_name").AsString()
@@ -34,17 +36,27 @@ func convertPermissionsSecretScope(key, scopeName string, permissions []dyn.Valu
3436
principal = servicePrincipalName
3537
}
3638

39+
// Build depends_on list - always depend on scope, plus previous ACL if exists.
40+
// This forces Terraform to execute ACL operations sequentially, avoiding
41+
// a backend race condition where parallel ACL modifications return inconsistent results.
42+
dependsOn := []string{"databricks_secret_scope." + key}
43+
if previousAclKey != "" {
44+
dependsOn = append(dependsOn, "databricks_secret_acl."+previousAclKey)
45+
}
46+
3747
acl := &resourceSecretAcl{
3848
ResourceSecretAcl: schema.ResourceSecretAcl{
3949
Permission: level,
4050
Principal: principal,
4151
Scope: scopeName,
4252
},
43-
DependsOn: []string{"databricks_secret_scope." + key},
53+
DependsOn: dependsOn,
4454
}
4555

4656
aclKey := fmt.Sprintf("secret_acl_%s_%d", key, idx)
4757
out.SecretAcl[aclKey] = acl
58+
59+
previousAclKey = aclKey
4860
}
4961
}
5062

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package tfdyn
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/databricks/cli/bundle/config/resources"
8+
"github.com/databricks/cli/bundle/internal/tf/schema"
9+
"github.com/databricks/cli/libs/dyn"
10+
"github.com/databricks/cli/libs/dyn/convert"
11+
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestConvertSecretScopeWithPermissions(t *testing.T) {
16+
src := resources.SecretScope{
17+
Name: "my_scope",
18+
Permissions: []resources.SecretScopePermission{
19+
{UserName: "[email protected]", Level: resources.SecretScopePermissionLevelWrite},
20+
{GroupName: "data-team", Level: resources.SecretScopePermissionLevelRead},
21+
{ServicePrincipalName: "sp-uuid", Level: resources.SecretScopePermissionLevelManage},
22+
},
23+
}
24+
25+
vin, err := convert.FromTyped(src, dyn.NilValue)
26+
require.NoError(t, err)
27+
28+
ctx := context.Background()
29+
out := schema.NewResources()
30+
err = secretScopeConverter{}.Convert(ctx, "my_scope", vin, out)
31+
require.NoError(t, err)
32+
33+
// Verify ACL count
34+
assert.Len(t, out.SecretAcl, 3)
35+
36+
// Verify first ACL depends only on scope
37+
acl0 := out.SecretAcl["secret_acl_my_scope_0"].(*resourceSecretAcl)
38+
assert.Equal(t, "[email protected]", acl0.Principal)
39+
assert.Equal(t, "WRITE", acl0.Permission)
40+
assert.Equal(t, []string{"databricks_secret_scope.my_scope"}, acl0.DependsOn)
41+
42+
// Verify second ACL depends on scope + first ACL (sequential execution)
43+
acl1 := out.SecretAcl["secret_acl_my_scope_1"].(*resourceSecretAcl)
44+
assert.Equal(t, "data-team", acl1.Principal)
45+
assert.Equal(t, "READ", acl1.Permission)
46+
assert.Equal(t, []string{
47+
"databricks_secret_scope.my_scope",
48+
"databricks_secret_acl.secret_acl_my_scope_0",
49+
}, acl1.DependsOn)
50+
51+
// Verify third ACL depends on scope + second ACL (sequential execution)
52+
acl2 := out.SecretAcl["secret_acl_my_scope_2"].(*resourceSecretAcl)
53+
assert.Equal(t, "sp-uuid", acl2.Principal)
54+
assert.Equal(t, "MANAGE", acl2.Permission)
55+
assert.Equal(t, []string{
56+
"databricks_secret_scope.my_scope",
57+
"databricks_secret_acl.secret_acl_my_scope_1",
58+
}, acl2.DependsOn)
59+
}
60+
61+
func TestConvertSecretScopeSinglePermission(t *testing.T) {
62+
src := resources.SecretScope{
63+
Name: "single_scope",
64+
Permissions: []resources.SecretScopePermission{
65+
{UserName: "[email protected]", Level: resources.SecretScopePermissionLevelManage},
66+
},
67+
}
68+
69+
vin, err := convert.FromTyped(src, dyn.NilValue)
70+
require.NoError(t, err)
71+
72+
ctx := context.Background()
73+
out := schema.NewResources()
74+
err = secretScopeConverter{}.Convert(ctx, "single_scope", vin, out)
75+
require.NoError(t, err)
76+
77+
// Single ACL should only depend on scope (no chaining needed)
78+
assert.Len(t, out.SecretAcl, 1)
79+
acl := out.SecretAcl["secret_acl_single_scope_0"].(*resourceSecretAcl)
80+
assert.Equal(t, "[email protected]", acl.Principal)
81+
assert.Equal(t, []string{"databricks_secret_scope.single_scope"}, acl.DependsOn)
82+
}
83+
84+
func TestConvertSecretScopeNoPermissions(t *testing.T) {
85+
src := resources.SecretScope{
86+
Name: "no_permissions_scope",
87+
}
88+
89+
vin, err := convert.FromTyped(src, dyn.NilValue)
90+
require.NoError(t, err)
91+
92+
ctx := context.Background()
93+
out := schema.NewResources()
94+
err = secretScopeConverter{}.Convert(ctx, "no_permissions_scope", vin, out)
95+
require.NoError(t, err)
96+
97+
// No ACLs should be created
98+
assert.Len(t, out.SecretAcl, 0)
99+
100+
// But the scope should still be created
101+
assert.Contains(t, out.SecretScope, "no_permissions_scope")
102+
}

0 commit comments

Comments
 (0)