diff --git a/.nextchanges/bundles/secret-scope-permission-principal-required.md b/.nextchanges/bundles/secret-scope-permission-principal-required.md new file mode 100644 index 00000000000..171c007448b --- /dev/null +++ b/.nextchanges/bundles/secret-scope-permission-principal-required.md @@ -0,0 +1 @@ +Reject secret scope permissions that name no principal, instead of failing after the scope is created. diff --git a/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml b/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml new file mode 100644 index 00000000000..0e33e50deca --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml @@ -0,0 +1,27 @@ +bundle: + name: test-bundle + +resources: + secret_scopes: + no_principal: + name: test-scope-no-principal + permissions: + # Missing principal: rejected. + - level: READ + empty_principal: + name: test-scope-empty-principal + permissions: + # Empty principal: rejected. + - level: READ + group_name: "" + wrong_type_principal: + name: test-scope-wrong-type-principal + permissions: + # Wrong-typed principal: normalization drops it, so treated as missing. + - level: READ + group_name: [] + valid: + name: test-scope-valid + permissions: + - level: READ + group_name: users diff --git a/acceptance/bundle/validate/secret_scope_required_principal/out.test.toml b/acceptance/bundle/validate/secret_scope_required_principal/out.test.toml new file mode 100644 index 00000000000..98ea5040486 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/validate/secret_scope_required_principal/output.txt b/acceptance/bundle/validate/secret_scope_required_principal/output.txt new file mode 100644 index 00000000000..987ce601cb6 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/output.txt @@ -0,0 +1,57 @@ + +>>> [CLI] bundle validate +Warning: expected string, found sequence + at resources.secret_scopes.wrong_type_principal.permissions[0].group_name + in databricks.yml:22:23 + +Error: secret scope permission principal is required + at resources.secret_scopes.empty_principal.permissions[0] + in databricks.yml:12:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.wrong_type_principal.permissions[0] + in databricks.yml:18:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.no_principal.permissions[0] + in databricks.yml:7:7 + +Set one of user_name, group_name or service_principal_name + +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Found 3 errors and 1 warning + +>>> [CLI] bundle deploy +Warning: expected string, found sequence + at resources.secret_scopes.wrong_type_principal.permissions[0].group_name + in databricks.yml:22:23 + +Error: secret scope permission principal is required + at resources.secret_scopes.empty_principal.permissions[0] + in databricks.yml:12:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.wrong_type_principal.permissions[0] + in databricks.yml:18:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.no_principal.permissions[0] + in databricks.yml:7:7 + +Set one of user_name, group_name or service_principal_name + + +>>> print_requests.py //secrets diff --git a/acceptance/bundle/validate/secret_scope_required_principal/script b/acceptance/bundle/validate/secret_scope_required_principal/script new file mode 100644 index 00000000000..056ede5170d --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/script @@ -0,0 +1,9 @@ +# Without the fix, validate only warns; deploy fails later in SecretScopeFixups (direct) +# or leaves empty-principal ACLs that destroy cannot remove (terraform). +musterr trace $CLI bundle validate + +# Deploy must abort at validation: no scope is created, so no partial deploy. +musterr trace $CLI bundle deploy +trace print_requests.py //secrets + +rm -f out.requests.txt diff --git a/acceptance/bundle/validate/secret_scope_required_principal/test.toml b/acceptance/bundle/validate/secret_scope_required_principal/test.toml new file mode 100644 index 00000000000..159efe02696 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/test.toml @@ -0,0 +1 @@ +RecordRequests = true diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index 26223630e77..b886c2c1d73 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -83,8 +83,13 @@ func sortDiagnostics(diags diag.Diagnostics) { return n } - // Finally sort by locations as a tie breaker if summaries are the same. - return cmp.Compare(fmt.Sprintf("%v", a.Locations), fmt.Sprintf("%v", b.Locations)) + // Then sort by locations as a tie breaker if summaries are the same. + if n := cmp.Compare(fmt.Sprintf("%v", a.Locations), fmt.Sprintf("%v", b.Locations)); n != 0 { + return n + } + + // Sibling entries can share a location; fall back to path for a stable order. + return cmp.Compare(fmt.Sprintf("%v", a.Paths), fmt.Sprintf("%v", b.Paths)) }) } @@ -184,6 +189,34 @@ func errorForInvalidGrants(ctx context.Context, b *bundle.Bundle) diag.Diagnosti return diags } +// errorForInvalidSecretScopePermissions errors when a permission names no principal. +// Wrong-typed values are already empty here (normalization only warns and drops them). +func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + diags := diag.Diagnostics{} + + for key, scope := range b.Config.Resources.SecretScopes { + for i, perm := range scope.Permissions { + if perm.UserName != "" || perm.GroupName != "" || perm.ServicePrincipalName != "" { + continue + } + path := fmt.Sprintf("resources.secret_scopes.%s.permissions[%d]", key, i) + // ApplyBundlePermissions rebuilds permissions via convert.FromTyped and drops + // per-entry locations, so point at the scope. + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: "secret scope permission principal is required", + Detail: "Set one of user_name, group_name or service_principal_name", + Locations: b.Config.GetLocations("resources.secret_scopes." + key), + Paths: []dyn.Path{dyn.MustPathFromString(path)}, + }) + } + } + + sortDiagnostics(diags) + + return diags +} + // isMissingOrEmptyString reports whether v is unset, null, or an empty string. func isMissingOrEmptyString(v dyn.Value) bool { switch v.Kind() { @@ -211,6 +244,7 @@ func isMissingOrEmptySequence(v dyn.Value) bool { func (f *required) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := errorForMissingFields(ctx, b) diags = diags.Extend(errorForInvalidGrants(ctx, b)) + diags = diags.Extend(errorForInvalidSecretScopePermissions(ctx, b)) if diags.HasError() { return diags }