Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .nextchanges/bundles/grants-before-dependents.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* Resources that reference another resource are now deployed after that resource's `grants`, fixing intermittent `User does not have CREATE TABLE on Schema` errors when a pipeline's `run_as` principal is granted access to its schema in the same bundle.
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
bundle:
name: schema-grants-pipeline-$UNIQUE_NAME

resources:
catalogs:
pipeline_catalog:
name: catalog_grants_pipeline_$UNIQUE_NAME
grants:
- principal: $RUN_AS_SP
privileges:
- USE_CATALOG

schemas:
src:
catalog_name: ${resources.catalogs.pipeline_catalog.name}
name: src
grants:
- principal: $RUN_AS_SP
privileges:
- USE_SCHEMA
- CREATE_TABLE
- CREATE_MATERIALIZED_VIEW

pipelines:
load:
name: schema-grants-pipeline-$UNIQUE_NAME
catalog: ${resources.schemas.src.catalog_name}
schema: ${resources.schemas.src.name}
serverless: true
libraries:
- file:
path: ./transformation.py
run_as:
service_principal_name: $RUN_AS_SP
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Cloud = false
EnvMatrix.DMS = ["", "true"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@

>>> [CLI] bundle plan -o json
{
"resources.catalogs.pipeline_catalog": null,
"resources.catalogs.pipeline_catalog.grants": [
{
"node": "resources.catalogs.pipeline_catalog",
"label": "${resources.catalogs.pipeline_catalog.id}"
}
],
"resources.pipelines.load": [
{
"node": "resources.schemas.src",
"label": "${resources.schemas.src.catalog_name}"
},
{
"node": "resources.schemas.src",
"label": "${resources.schemas.src.name}"
},
{
"node": "resources.schemas.src.grants",
"label": "${resources.schemas.src.catalog_name}"
}
],
"resources.schemas.src": [
{
"node": "resources.catalogs.pipeline_catalog",
"label": "${resources.catalogs.pipeline_catalog.name}"
},
{
"node": "resources.catalogs.pipeline_catalog.grants",
"label": "${resources.catalogs.pipeline_catalog.name}"
}
],
"resources.schemas.src.grants": [
{
"node": "resources.schemas.src",
"label": "${resources.schemas.src.id}"
}
]
}

>>> [CLI] bundle deploy
Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/schema-grants-pipeline-[UNIQUE_NAME]/default/files...
Created catalogs.pipeline_catalog
Created catalogs.pipeline_catalog.grants
Created pipelines.load
Created schemas.src
Created schemas.src.grants
Files: 3 uploaded, 0 deleted
Resources: 5 created, 0 changed, 0 deleted, 0 unchanged

>>> print_requests.py --summary //unity-catalog/catalogs //unity-catalog/schemas //unity-catalog/permissions //pipelines
POST /api/2.1/unity-catalog/catalogs
PATCH /api/2.1/unity-catalog/permissions/catalog/catalog_grants_pipeline_[UNIQUE_NAME]
POST /api/2.1/unity-catalog/schemas
PATCH /api/2.1/unity-catalog/permissions/schema/catalog_grants_pipeline_[UNIQUE_NAME].src
POST /api/2.0/pipelines

>>> [CLI] bundle plan
Plan: 0 to add, 0 to change, 0 to delete, 5 unchanged

>>> [CLI] bundle destroy --auto-approve
The following resources will be deleted:
delete resources.catalogs.pipeline_catalog
delete resources.pipelines.load
delete resources.schemas.src

This action will result in the deletion of the following UC schemas. Any underlying data may be lost:
delete resources.schemas.src

This action will result in the deletion of the following Lakeflow Spark Declarative Pipelines along with the
Streaming Tables (STs) and Materialized Views (MVs) managed by them. Set 'cascade_on_destroy: false' on a pipeline to retain datasets on pipeline deletion:
delete resources.pipelines.load

All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/schema-grants-pipeline-[UNIQUE_NAME]/default

Destroy: 3 deleted
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Creating the pipeline fails unless its run_as principal can already create tables in the
# schema, so the pipeline must be deployed after the grants it relies on.
# See https://github.com/databricks/cli/issues/4573
export RUN_AS_SP="${TEST_SP_APPLICATION_ID:-6f1c2b3a-4d5e-4f60-8a7b-9c0d1e2f3a4b}"

envsubst < databricks.yml.tmpl > databricks.yml

cleanup() {
trace $CLI bundle destroy --auto-approve
}
trap cleanup EXIT

trace $CLI bundle plan -o json | jq '.plan | map_values(.depends_on)'

trace $CLI bundle deploy
trace print_requests.py --summary //unity-catalog/catalogs //unity-catalog/schemas //unity-catalog/permissions //pipelines

trace $CLI bundle plan
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Local only: on cloud the deploying identity lacks the servicePrincipal.user role on
# TEST_SP_APPLICATION_ID, so it cannot set that service principal as the pipeline's run_as.
Cloud = false
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
print("Hello, World!")
70 changes: 64 additions & 6 deletions bundle/direct/bundle_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -1285,12 +1285,7 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root
}
}

slices.SortFunc(dependsOn, func(a, b deployplan.DependsOnEntry) int {
if a.Node != b.Node {
return strings.Compare(a.Node, b.Node)
}
return strings.Compare(a.Label, b.Label)
})
slices.SortFunc(dependsOn, compareDependsOn)

// Store an unredacted copy in the cache so Apply can deploy with the
// actual values. The original newStateConfig is redacted below and used
Expand Down Expand Up @@ -1330,6 +1325,8 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root
p.Plan[node] = &e
}

addGrantsDependencies(p)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, I'm also adding lifecycle.depends_on #7120

Worth evaluating if it changes anything there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding lifecycle.depends_on

This would require manually configuring that schemas depends on the grants, right? I think it's less preferred and it makes more sense to always wait for grants being provisioned


for n, entry := range existingKeys {
if p.Plan[n] != nil {
panic("unexpected node " + n)
Expand All @@ -1344,6 +1341,67 @@ func (b *DeploymentBundle) makePlan(ctx context.Context, configRoot *config.Root
return p, nil
}

func compareDependsOn(a, b deployplan.DependsOnEntry) int {
if a.Node != b.Node {
return strings.Compare(a.Node, b.Node)
}
return strings.Compare(a.Label, b.Label)
}

// addGrantsDependencies makes resources that refer to another resource also depend on that
// resource's grants, because creating some resources requires the grants to be in place: a
// pipeline cannot be created before its run_as principal has CREATE TABLE on the target schema.
// A grants dependency that would create a cycle is skipped, e.g. when a schema grants privileges
// to the service principal of an app that refers to the schema.
func addGrantsDependencies(plan *deployplan.Plan) {
for _, node := range slices.Sorted(maps.Keys(plan.Plan)) {
entry := plan.Plan[node]

seen := make(map[string]bool, len(entry.DependsOn))
for _, dep := range entry.DependsOn {
seen[dep.Node] = true
}

var grantsDeps []deployplan.DependsOnEntry
for _, dep := range entry.DependsOn {
grantsNode := dep.Node + ".grants"
if seen[grantsNode] || plan.Plan[grantsNode] == nil || dependsOnTransitively(plan, grantsNode, node) {
continue
}
seen[grantsNode] = true
grantsDeps = append(grantsDeps, deployplan.DependsOnEntry{Node: grantsNode, Label: dep.Label})
}

if len(grantsDeps) > 0 {
entry.DependsOn = append(entry.DependsOn, grantsDeps...)
slices.SortFunc(entry.DependsOn, compareDependsOn)
}
}
}

// dependsOnTransitively reports whether node is target or depends on it, directly or through other nodes.
func dependsOnTransitively(plan *deployplan.Plan, node, target string) bool {
visited := make(map[string]bool)
stack := []string{node}
for len(stack) > 0 {
n := stack[len(stack)-1]
stack = stack[:len(stack)-1]
if n == target {
return true
}
// A dependency on a node missing from the plan is reported by makeGraph.
entry := plan.Plan[n]
if visited[n] || entry == nil {
continue
}
visited[n] = true
for _, dep := range entry.DependsOn {
stack = append(stack, dep.Node)
}
}
return false
}

// ExtractReferences extracts variable references from the config subtree rooted at node,
// keeping only those whose field path exists in stateType (references in input-only or
// bundle:"readonly" fields, such as volumes' computed volume_path, are skipped).
Expand Down
125 changes: 125 additions & 0 deletions bundle/direct/bundle_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,3 +795,128 @@ func TestLadderBackendDefaultBeforeRemoteAddition(t *testing.T) {
assert.Equal(t, deployplan.Skip, changes["enable_elastic_disk"].Action)
assert.Equal(t, deployplan.ReasonBackendDefault, changes["enable_elastic_disk"].Reason)
}

func TestAddGrantsDependencies(t *testing.T) {
const (
catalog = "resources.catalogs.main"
catalogGrants = "resources.catalogs.main.grants"
schema = "resources.schemas.src"
schemaGrants = "resources.schemas.src.grants"
pipeline = "resources.pipelines.load"
app = "resources.apps.app"
job = "resources.jobs.job"
)

schemaID := deployplan.DependsOnEntry{Node: schema, Label: "${resources.schemas.src.id}"}
schemaCatalogName := deployplan.DependsOnEntry{Node: schema, Label: "${resources.schemas.src.catalog_name}"}
schemaName := deployplan.DependsOnEntry{Node: schema, Label: "${resources.schemas.src.name}"}
appSP := deployplan.DependsOnEntry{Node: app, Label: "${resources.apps.app.service_principal_client_id}"}

tests := []struct {
name string
plan map[string][]deployplan.DependsOnEntry
expected map[string][]deployplan.DependsOnEntry
}{
{
name: "resource referring to a resource with grants depends on the grants",
plan: map[string][]deployplan.DependsOnEntry{
schema: nil,
schemaGrants: {schemaID},
pipeline: {schemaCatalogName, schemaName},
},
expected: map[string][]deployplan.DependsOnEntry{
schema: nil,
schemaGrants: {schemaID},
pipeline: {
schemaCatalogName,
schemaName,
{Node: schemaGrants, Label: "${resources.schemas.src.catalog_name}"},
},
},
},
{
name: "resource without grants",
plan: map[string][]deployplan.DependsOnEntry{
schema: nil,
pipeline: {schemaName},
},
expected: map[string][]deployplan.DependsOnEntry{
schema: nil,
pipeline: {schemaName},
},
},
{
name: "each resource depends on the grants of the resource it refers to",
plan: map[string][]deployplan.DependsOnEntry{
catalog: nil,
catalogGrants: {{Node: catalog, Label: "${resources.catalogs.main.id}"}},
schema: {{Node: catalog, Label: "${resources.catalogs.main.name}"}},
schemaGrants: {schemaID},
pipeline: {schemaName},
},
expected: map[string][]deployplan.DependsOnEntry{
catalog: nil,
catalogGrants: {{Node: catalog, Label: "${resources.catalogs.main.id}"}},
schema: {
{Node: catalog, Label: "${resources.catalogs.main.name}"},
{Node: catalogGrants, Label: "${resources.catalogs.main.name}"},
},
schemaGrants: {schemaID},
pipeline: {
schemaName,
{Node: schemaGrants, Label: "${resources.schemas.src.name}"},
},
},
},
{
name: "grants that depend on the referring resource",
plan: map[string][]deployplan.DependsOnEntry{
schema: nil,
schemaGrants: {appSP, schemaID},
app: {schemaName},
pipeline: {schemaName},
},
expected: map[string][]deployplan.DependsOnEntry{
schema: nil,
schemaGrants: {appSP, schemaID},
app: {schemaName},
pipeline: {
schemaName,
{Node: schemaGrants, Label: "${resources.schemas.src.name}"},
},
},
},
{
name: "grants that transitively depend on the referring resource",
plan: map[string][]deployplan.DependsOnEntry{
schema: nil,
schemaGrants: {{Node: job, Label: "${resources.jobs.job.run_as.service_principal_name}"}, schemaID},
job: {{Node: app, Label: "${resources.apps.app.name}"}},
app: {schemaName},
},
expected: map[string][]deployplan.DependsOnEntry{
schema: nil,
schemaGrants: {{Node: job, Label: "${resources.jobs.job.run_as.service_principal_name}"}, schemaID},
job: {{Node: app, Label: "${resources.apps.app.name}"}},
app: {schemaName},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
plan := deployplan.NewPlanDirect()
for node, dependsOn := range tt.plan {
plan.Plan[node] = &deployplan.PlanEntry{DependsOn: dependsOn}
}

addGrantsDependencies(plan)

actual := make(map[string][]deployplan.DependsOnEntry, len(plan.Plan))
for node, entry := range plan.Plan {
actual[node] = entry.DependsOn
}
assert.Equal(t, tt.expected, actual)
})
}
}
Loading
Loading