diff --git a/.nextchanges/bundles/grants-rekey.md b/.nextchanges/bundles/grants-rekey.md new file mode 100644 index 00000000000..7c1953a5b82 --- /dev/null +++ b/.nextchanges/bundles/grants-rekey.md @@ -0,0 +1 @@ +* Fix grants of a renamed or recreated catalog, schema, volume or other UC resource staying recorded under the old name, which made the next `bundle plan` propose to apply them again. ([#7345](https://github.com/databricks/cli/pull/7345)) diff --git a/acceptance/bundle/resources/grants/schemas/change_name/databricks.yml.tmpl b/acceptance/bundle/resources/grants/schemas/change_name/databricks.yml.tmpl new file mode 100644 index 00000000000..594df21f5b0 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/change_name/databricks.yml.tmpl @@ -0,0 +1,12 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + schemas: + schema1: + catalog_name: main + name: test-schema-$UNIQUE_NAME + grants: + - principal: deco-test-user@databricks.com + privileges: + - USE_SCHEMA diff --git a/acceptance/bundle/resources/grants/schemas/change_name/out.test.toml b/acceptance/bundle/resources/grants/schemas/change_name/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/change_name/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/grants/schemas/change_name/output.txt b/acceptance/bundle/resources/grants/schemas/change_name/output.txt new file mode 100644 index 00000000000..6c4e069ee0c --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/change_name/output.txt @@ -0,0 +1,123 @@ + +=== Deploy a schema with grants +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Created schemas.schema1 +Created schemas.schema1.grants +Files: 2 uploaded, 0 deleted +Resources: 2 created, 0 changed, 0 deleted, 0 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/schemas", + "body": { + "catalog_name": "main", + "name": "test-schema-[UNIQUE_NAME]" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.test-schema-[UNIQUE_NAME]", + "body": { + "changes": [ + { + "add": [ + "USE_SCHEMA" + ], + "principal": "deco-test-user@databricks.com", + "remove": [ + "ALL_PRIVILEGES" + ] + } + ] + } +} + +>>> read_state.py schemas schema1.grants id +schemas schema1.grants id='schema/main.test-schema-[UNIQUE_NAME]' + +=== Rename the schema: its ID changes, and so does the ID of its grants +>>> update_file.py databricks.yml test-schema- test-schema-renamed- + +>>> [CLI] bundle plan --output json +{ + "full_name": { + "action": "update_id", + "reason": "id_changes", + "old": "main.test-schema-[UNIQUE_NAME]", + "new": "", + "remote": "main.test-schema-[UNIQUE_NAME]" + } +} + +=== Deploy moves the schema to the new name and applies the grants there +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... + +This action will result in the deletion or recreation of the following UC schemas. Any underlying data may be lost: + recreate resources.schemas.schema1 +Recreated schemas.schema1 +Updated schemas.schema1.grants +Files: 1 uploaded, 0 deleted +Resources: 1 created, 1 changed, 1 deleted, 0 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "DELETE", + "path": "/api/2.1/unity-catalog/schemas/main.test-schema-[UNIQUE_NAME]", + "q": { + "force": "true" + } +} +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/schemas", + "body": { + "catalog_name": "main", + "name": "test-schema-renamed-[UNIQUE_NAME]" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/schema/main.test-schema-renamed-[UNIQUE_NAME]", + "body": { + "changes": [ + { + "add": [ + "USE_SCHEMA" + ], + "principal": "deco-test-user@databricks.com", + "remove": [ + "ALL_PRIVILEGES" + ] + } + ] + } +} + +>>> [CLI] grants get schema main.test-schema-renamed-[UNIQUE_NAME] +{ + "principal": "deco-test-user@databricks.com", + "privileges": [ + "USE_SCHEMA" + ] +} + +=== The grants are recorded under the new name, so the next plan is a no-op +>>> read_state.py schemas schema1.grants id +schemas schema1.grants id='schema/main.test-schema-renamed-[UNIQUE_NAME]' + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 2 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 1 deleted diff --git a/acceptance/bundle/resources/grants/schemas/change_name/script b/acceptance/bundle/resources/grants/schemas/change_name/script new file mode 100644 index 00000000000..93360d06552 --- /dev/null +++ b/acceptance/bundle/resources/grants/schemas/change_name/script @@ -0,0 +1,29 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +NEW_SCHEMA="main.test-schema-renamed-${UNIQUE_NAME}" + +cleanup() { + trace $CLI bundle destroy --auto-approve + + # Consume the destroy's requests so none are left recorded. + print_requests.py //unity-catalog &> LOG.destroy_requests +} +trap cleanup EXIT + +title "Deploy a schema with grants" +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace read_state.py schemas schema1.grants id + +title "Rename the schema: its ID changes, and so does the ID of its grants" +trace update_file.py databricks.yml test-schema- test-schema-renamed- +trace $CLI bundle plan --output json | jq '.plan["resources.schemas.schema1.grants"].changes' + +title "Deploy moves the schema to the new name and applies the grants there" +trace $CLI bundle deploy --auto-approve +trace print_requests.py //unity-catalog +trace $CLI grants get schema "$NEW_SCHEMA" | jq '.privilege_assignments[] | select(.principal == "deco-test-user@databricks.com")' + +title "The grants are recorded under the new name, so the next plan is a no-op" +trace read_state.py schemas schema1.grants id +trace $CLI bundle plan diff --git a/acceptance/bundle/resources/grants/volume_change_name/databricks.yml.tmpl b/acceptance/bundle/resources/grants/volume_change_name/databricks.yml.tmpl new file mode 100644 index 00000000000..a8c9a407fda --- /dev/null +++ b/acceptance/bundle/resources/grants/volume_change_name/databricks.yml.tmpl @@ -0,0 +1,17 @@ +bundle: + name: test-bundle-$UNIQUE_NAME + +resources: + schemas: + schema1: + catalog_name: main + name: test-schema-$UNIQUE_NAME + volumes: + volume1: + catalog_name: main + schema_name: ${resources.schemas.schema1.name} + name: myvolume + grants: + - principal: deco-test-user@databricks.com + privileges: + - READ_VOLUME diff --git a/acceptance/bundle/resources/grants/volume_change_name/out.test.toml b/acceptance/bundle/resources/grants/volume_change_name/out.test.toml new file mode 100644 index 00000000000..0f167effef1 --- /dev/null +++ b/acceptance/bundle/resources/grants/volume_change_name/out.test.toml @@ -0,0 +1,2 @@ +Cloud = true +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/resources/grants/volume_change_name/output.txt b/acceptance/bundle/resources/grants/volume_change_name/output.txt new file mode 100644 index 00000000000..ffe41036393 --- /dev/null +++ b/acceptance/bundle/resources/grants/volume_change_name/output.txt @@ -0,0 +1,131 @@ + +=== Deploy a volume with grants +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Created schemas.schema1 +Created volumes.volume1 +Created volumes.volume1.grants +Files: 2 uploaded, 0 deleted +Resources: 3 created, 0 changed, 0 deleted, 0 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/schemas", + "body": { + "catalog_name": "main", + "name": "test-schema-[UNIQUE_NAME]" + } +} +{ + "method": "POST", + "path": "/api/2.1/unity-catalog/volumes", + "body": { + "catalog_name": "main", + "name": "myvolume", + "schema_name": "test-schema-[UNIQUE_NAME]", + "volume_type": "MANAGED" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/volume/main.test-schema-[UNIQUE_NAME].myvolume", + "body": { + "changes": [ + { + "add": [ + "READ_VOLUME" + ], + "principal": "deco-test-user@databricks.com", + "remove": [ + "ALL_PRIVILEGES" + ] + } + ] + } +} + +>>> read_state.py volumes volume1.grants id +volumes volume1.grants id='volume/main.test-schema-[UNIQUE_NAME].myvolume' + +=== Rename the volume: its ID changes, and so does the ID of its grants +>>> update_file.py databricks.yml myvolume myvolume_renamed + +>>> [CLI] bundle plan --output json +{ + "full_name": { + "action": "update_id", + "reason": "id_changes", + "old": "main.test-schema-[UNIQUE_NAME].myvolume", + "new": "", + "remote": "main.test-schema-[UNIQUE_NAME].myvolume" + } +} + +=== Deploy renames the volume and applies the grants under the new name +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default/files... +Updated volumes.volume1 +Updated volumes.volume1.grants +Files: 1 uploaded, 0 deleted +Resources: 0 created, 2 changed, 0 deleted, 1 unchanged + +>>> print_requests.py //unity-catalog +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/volumes/main.test-schema-[UNIQUE_NAME].myvolume", + "body": { + "comment": "", + "new_name": "myvolume_renamed" + } +} +{ + "method": "PATCH", + "path": "/api/2.1/unity-catalog/permissions/volume/main.test-schema-[UNIQUE_NAME].myvolume_renamed", + "body": { + "changes": [ + { + "add": [ + "READ_VOLUME" + ], + "principal": "deco-test-user@databricks.com", + "remove": [ + "ALL_PRIVILEGES" + ] + } + ] + } +} + +>>> [CLI] grants get volume main.test-schema-[UNIQUE_NAME].myvolume_renamed +{ + "principal": "deco-test-user@databricks.com", + "privileges": [ + "READ_VOLUME" + ] +} + +=== The grants are recorded under the new name, so the next plan is a no-op +>>> read_state.py volumes volume1.grants id +volumes volume1.grants id='volume/main.test-schema-[UNIQUE_NAME].myvolume_renamed' + +>>> [CLI] bundle plan +Plan: 0 to add, 0 to change, 0 to delete, 3 unchanged + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.schemas.schema1 + delete resources.volumes.volume1 + +This action will result in the deletion of the following UC schemas. Any underlying data may be lost: + delete resources.schemas.schema1 + +This action will result in the deletion of the following volumes. +For managed volumes, the files stored in the volume are also deleted from your +cloud tenant within 30 days. For external volumes, the metadata about the volume +is removed from the catalog, but the underlying files are not deleted: + delete resources.volumes.volume1 + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/test-bundle-[UNIQUE_NAME]/default + +Destroy: 2 deleted diff --git a/acceptance/bundle/resources/grants/volume_change_name/script b/acceptance/bundle/resources/grants/volume_change_name/script new file mode 100644 index 00000000000..3ae81092846 --- /dev/null +++ b/acceptance/bundle/resources/grants/volume_change_name/script @@ -0,0 +1,29 @@ +envsubst < databricks.yml.tmpl > databricks.yml + +NEW_VOLUME="main.test-schema-${UNIQUE_NAME}.myvolume_renamed" + +cleanup() { + trace $CLI bundle destroy --auto-approve + + # Consume the destroy's requests so none are left recorded. + print_requests.py //unity-catalog &> LOG.destroy_requests +} +trap cleanup EXIT + +title "Deploy a volume with grants" +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace read_state.py volumes volume1.grants id + +title "Rename the volume: its ID changes, and so does the ID of its grants" +trace update_file.py databricks.yml myvolume myvolume_renamed +trace $CLI bundle plan --output json | jq '.plan["resources.volumes.volume1.grants"].changes' + +title "Deploy renames the volume and applies the grants under the new name" +trace $CLI bundle deploy +trace print_requests.py //unity-catalog +trace $CLI grants get volume "$NEW_VOLUME" | jq '.privilege_assignments[] | select(.principal == "deco-test-user@databricks.com")' + +title "The grants are recorded under the new name, so the next plan is a no-op" +trace read_state.py volumes volume1.grants id +trace $CLI bundle plan diff --git a/bundle/direct/dresources/README.md b/bundle/direct/dresources/README.md index 5487042c965..0262a2d11e0 100644 --- a/bundle/direct/dresources/README.md +++ b/bundle/direct/dresources/README.md @@ -17,7 +17,7 @@ Each field with special plan/deploy behavior must be declared in the YAML file of its resource type, under `configs/`. Both files of a resource type are optional and are omitted when they would be empty: - - `configs/.yml` (e.g. `configs/jobs.yml`) — hand-written rules. + - `configs/.yml` (e.g. `configs/jobs.yml`) — hand-written rules. A file named after a sub-resource (`configs/grants.yml`) applies to that sub-resource of every parent. - `configs/.generated.yml` — API field behaviors from the OpenAPI schema. Generated by `./task generate-direct-resources`, do not edit. Choose the right category: diff --git a/bundle/direct/dresources/config.go b/bundle/direct/dresources/config.go index cc54d60a678..9552f935d1c 100644 --- a/bundle/direct/dresources/config.go +++ b/bundle/direct/dresources/config.go @@ -138,6 +138,8 @@ type Config struct { // hand-written rules and .generated.yml the ones derived from the // OpenAPI spec. A resource type without rules has no file. The file name is the // resource type, so each file holds the rules directly with no enclosing keys. +// A file named after a sub-resource (grants.yml) applies to that sub-resource of +// every parent (catalogs.grants, schemas.grants, ...). // //go:embed configs/*.yml var configFS embed.FS @@ -191,12 +193,32 @@ var loadConfigs = sync.OnceValues(func() (*Config, *Config) { panic(fmt.Errorf("%s: %w", name, err)) } - dst.Resources[resourceType] = rc + for _, rt := range configResourceTypes(resourceType) { + if _, ok := dst.Resources[rt]; ok { + panic(fmt.Errorf("%s: rules for %s are already defined by another file", name, rt)) + } + dst.Resources[rt] = rc + } } return handWritten, generated }) +// configResourceTypes returns the resource types a config file named name applies to: the +// resource type itself, or for a sub-resource name like "grants" every ".grants". +func configResourceTypes(name string) []string { + if _, ok := SupportedResources[name]; ok { + return []string{name} + } + var result []string + for resourceType := range SupportedResources { + if strings.HasSuffix(resourceType, "."+name) { + result = append(result, resourceType) + } + } + return result +} + // MustLoadConfig returns the configuration parsed from the configs/.yml files. func MustLoadConfig() *Config { handWritten, _ := loadConfigs() diff --git a/bundle/direct/dresources/config_test.go b/bundle/direct/dresources/config_test.go index 1446a14e8e2..4cf0d7d22be 100644 --- a/bundle/direct/dresources/config_test.go +++ b/bundle/direct/dresources/config_test.go @@ -28,7 +28,7 @@ func TestConfigFiles(t *testing.T) { for _, name := range names { resourceType := strings.TrimSuffix(strings.TrimSuffix(path.Base(name), ymlSuffix), generatedSuffix) - assert.Containsf(t, SupportedResources, resourceType, "%s: %q is not a supported resource type", name, resourceType) + assert.NotEmptyf(t, configResourceTypes(resourceType), "%s: %q is not a supported resource type or sub-resource", name, resourceType) } for _, cfg := range []*Config{MustLoadConfig(), MustLoadGeneratedConfig()} { @@ -43,6 +43,15 @@ func TestGetResourceConfig(t *testing.T) { assert.Empty(t, GetResourceConfig("nonexistent").RecreateOnChanges) } +// TestSubResourceConfig checks that grants.yml applies to the grants of every parent. +func TestSubResourceConfig(t *testing.T) { + for resourceType := range SupportedResources { + if strings.HasSuffix(resourceType, ".grants") { + assert.NotEmptyf(t, GetResourceConfig(resourceType).UpdatableIDFields, "%s: grants.yml does not apply", resourceType) + } + } +} + // TestFieldRuleOmittedIsRoot verifies that a FieldRule with no field is a root // rule matching every field, which the job_runs root rules rely on. func TestFieldRuleOmittedIsRoot(t *testing.T) { diff --git a/bundle/direct/dresources/configs/grants.yml b/bundle/direct/dresources/configs/grants.yml new file mode 100644 index 00000000000..2a1f2bed107 --- /dev/null +++ b/bundle/direct/dresources/configs/grants.yml @@ -0,0 +1,6 @@ +updatable_id_fields: + # The ID embeds the parent's full name, which changes when the parent is renamed or + # recreated under a new name. UpdateWithID records the new ID, so DoRead does not keep + # reading the grants of the old name. + - field: full_name + reason: id_changes diff --git a/bundle/direct/dresources/grants.go b/bundle/direct/dresources/grants.go index 3aa48c58279..378149b9a2f 100644 --- a/bundle/direct/dresources/grants.go +++ b/bundle/direct/dresources/grants.go @@ -126,7 +126,18 @@ func (r *ResourceGrants) DoCreate(ctx context.Context, state *GrantsState) (stri return "", nil, retrySafe(err) } - return state.SecurableType + "/" + state.FullName, nil, nil + return makeGrantsID(state), nil, nil +} + +// DoUpdateWithID records the new ID when the parent's ID changes (a rename, or a recreate +// under a new name). Otherwise DoRead keeps listing the grants of the old name and every +// later plan reports an update. +func (r *ResourceGrants) DoUpdateWithID(ctx context.Context, id string, state *GrantsState, entry *PlanEntry) (string, *GrantsState, error) { + _, err := r.DoUpdate(ctx, id, state, entry) + if err != nil { + return "", nil, err + } + return makeGrantsID(state), nil, nil } func (r *ResourceGrants) DoUpdate(ctx context.Context, _ string, state *GrantsState, entry *PlanEntry) (*GrantsState, error) { @@ -300,6 +311,10 @@ func normalizeAssignments(assignments []catalog.PrivilegeAssignment) { } } +func makeGrantsID(state *GrantsState) string { + return state.SecurableType + "/" + state.FullName +} + func parseGrantsID(id string) (string, string, error) { parts := strings.SplitN(id, "/", 2) if len(parts) != 2 {