From 7a0e8f3ea3793bde2c545afd44de24d1d0175bb3 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 8 Jun 2026 17:09:10 +0200 Subject: [PATCH 1/2] Trigger recreate for immutable Lakebase database fields The direct engine attempted in-place updates that the Database API can't honor: database_instances has no rename endpoint, and database_catalogs and synced_database_tables have no update endpoint at all (their Update* SDK methods are stubs the backend rejects with 501 NOT_IMPLEMENTED). Mark the affected fields recreate_on_changes so a change goes through delete + create: name for database_instances, and all settable fields for database_catalogs and synced_database_tables (whose now-dead DoUpdate methods are removed). The testserver catalog/synced-table PATCH handlers return the real 501. A new test asserts every no-DoUpdate resource classifies every settable field, plus direct-engine recreate acceptance tests. Co-authored-by: Isaac --- .../recreate/databricks.yml.tmpl | 14 ++++ .../recreate/out.requests.recreate.txt | 14 ++++ .../database_catalogs/recreate/out.test.toml | 5 ++ .../database_catalogs/recreate/output.txt | 32 +++++++++ .../database_catalogs/recreate/script | 22 ++++++ .../database_catalogs/recreate/test.toml | 10 +++ .../recreate/databricks.yml.tmpl | 11 +++ .../recreate/out.requests.rename.txt | 15 ++++ .../database_instances/recreate/out.test.toml | 5 ++ .../database_instances/recreate/output.txt | 35 +++++++++ .../database_instances/recreate/script | 22 ++++++ .../database_instances/recreate/test.toml | 11 +++ .../recreate/databricks.yml.tmpl | 24 +++++++ .../recreate/out.requests.recreate.txt | 20 ++++++ .../recreate/out.test.toml | 5 ++ .../recreate/output.txt | 41 +++++++++++ .../synced_database_tables/recreate/script | 22 ++++++ .../synced_database_tables/recreate/test.toml | 10 +++ bundle/direct/dresources/all_test.go | 72 +++++++++++++++++++ bundle/direct/dresources/database_catalog.go | 11 --- bundle/direct/dresources/resources.yml | 56 +++++++++++++++ .../dresources/synced_database_table.go | 11 --- libs/testserver/database_catalogs.go | 44 +++--------- libs/testserver/synced_database_tables.go | 38 +++------- 24 files changed, 466 insertions(+), 84 deletions(-) create mode 100644 acceptance/bundle/resources/database_catalogs/recreate/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/database_catalogs/recreate/out.requests.recreate.txt create mode 100644 acceptance/bundle/resources/database_catalogs/recreate/out.test.toml create mode 100644 acceptance/bundle/resources/database_catalogs/recreate/output.txt create mode 100644 acceptance/bundle/resources/database_catalogs/recreate/script create mode 100644 acceptance/bundle/resources/database_catalogs/recreate/test.toml create mode 100644 acceptance/bundle/resources/database_instances/recreate/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/database_instances/recreate/out.requests.rename.txt create mode 100644 acceptance/bundle/resources/database_instances/recreate/out.test.toml create mode 100644 acceptance/bundle/resources/database_instances/recreate/output.txt create mode 100644 acceptance/bundle/resources/database_instances/recreate/script create mode 100644 acceptance/bundle/resources/database_instances/recreate/test.toml create mode 100644 acceptance/bundle/resources/synced_database_tables/recreate/databricks.yml.tmpl create mode 100644 acceptance/bundle/resources/synced_database_tables/recreate/out.requests.recreate.txt create mode 100644 acceptance/bundle/resources/synced_database_tables/recreate/out.test.toml create mode 100644 acceptance/bundle/resources/synced_database_tables/recreate/output.txt create mode 100644 acceptance/bundle/resources/synced_database_tables/recreate/script create mode 100644 acceptance/bundle/resources/synced_database_tables/recreate/test.toml diff --git a/acceptance/bundle/resources/database_catalogs/recreate/databricks.yml.tmpl b/acceptance/bundle/resources/database_catalogs/recreate/databricks.yml.tmpl new file mode 100644 index 00000000000..8d19b470d8b --- /dev/null +++ b/acceptance/bundle/resources/database_catalogs/recreate/databricks.yml.tmpl @@ -0,0 +1,14 @@ +bundle: + name: deploy-lakebase-catalog-recreate-$UNIQUE_NAME + +resources: + database_instances: + my_instance: + name: test-db-catalog-recreate-$UNIQUE_NAME + capacity: CU_1 + database_catalogs: + my_catalog: + database_instance_name: ${resources.database_instances.my_instance.name} + database_name: DBNAME_PLACEHOLDER + name: my_catalog_$UNIQUE_NAME + create_database_if_not_exists: true diff --git a/acceptance/bundle/resources/database_catalogs/recreate/out.requests.recreate.txt b/acceptance/bundle/resources/database_catalogs/recreate/out.requests.recreate.txt new file mode 100644 index 00000000000..f310847f609 --- /dev/null +++ b/acceptance/bundle/resources/database_catalogs/recreate/out.requests.recreate.txt @@ -0,0 +1,14 @@ +{ + "method": "DELETE", + "path": "/api/2.0/database/catalogs/my_catalog_[UNIQUE_NAME]" +} +{ + "body": { + "create_database_if_not_exists": true, + "database_instance_name": "test-db-catalog-recreate-[UNIQUE_NAME]", + "database_name": "my_database_v2", + "name": "my_catalog_[UNIQUE_NAME]" + }, + "method": "POST", + "path": "/api/2.0/database/catalogs" +} diff --git a/acceptance/bundle/resources/database_catalogs/recreate/out.test.toml b/acceptance/bundle/resources/database_catalogs/recreate/out.test.toml new file mode 100644 index 00000000000..c777e3ce206 --- /dev/null +++ b/acceptance/bundle/resources/database_catalogs/recreate/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false +RequiresUnityCatalog = true +CloudEnvs.gcp = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/database_catalogs/recreate/output.txt b/acceptance/bundle/resources/database_catalogs/recreate/output.txt new file mode 100644 index 00000000000..e63cb50093d --- /dev/null +++ b/acceptance/bundle/resources/database_catalogs/recreate/output.txt @@ -0,0 +1,32 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-catalog-recreate-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Change an immutable catalog field: no update API, so the plan recreates it +>>> [CLI] bundle plan +recreate database_catalogs.my_catalog + +Plan: 1 to add, 0 to change, 1 to delete, 1 unchanged + +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-catalog-recreate-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.database_catalogs.my_catalog + delete resources.database_instances.my_instance + +This action will result in the deletion of the following Lakebase database instances. +All data stored in them will be permanently lost: + delete resources.database_instances.my_instance + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-catalog-recreate-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/database_catalogs/recreate/script b/acceptance/bundle/resources/database_catalogs/recreate/script new file mode 100644 index 00000000000..78243de7042 --- /dev/null +++ b/acceptance/bundle/resources/database_catalogs/recreate/script @@ -0,0 +1,22 @@ +cleanup() { + trace $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT + +envsubst < databricks.yml.tmpl | sed "s/DBNAME_PLACEHOLDER/my_database/" > databricks.yml + +trace $CLI bundle deploy +rm -f out.requests.txt + +title "Change an immutable catalog field: no update API, so the plan recreates it" +sed "s/my_database/my_database_v2/" databricks.yml > databricks.yml.new +mv databricks.yml.new databricks.yml + +trace $CLI bundle plan +trace $CLI bundle deploy --auto-approve + +# The change must be a DELETE of the old catalog plus a POST creating the new +# one. A PATCH would return 501 (the bug this guards against). +jq --sort-keys 'select(.method != "GET" and (.path | contains("/database/catalogs")))' < out.requests.txt > out.requests.recreate.txt +rm -f out.requests.txt diff --git a/acceptance/bundle/resources/database_catalogs/recreate/test.toml b/acceptance/bundle/resources/database_catalogs/recreate/test.toml new file mode 100644 index 00000000000..731a0a503cb --- /dev/null +++ b/acceptance/bundle/resources/database_catalogs/recreate/test.toml @@ -0,0 +1,10 @@ +# The Database API has no UpdateDatabaseCatalog endpoint (PATCH returns 501), so +# any field change must go through delete + create. The direct engine handles +# this; the terraform path would need an upstream provider fix and is excluded. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +RecordRequests = true + +Ignore = [ + "databricks.yml", + ".databricks", +] diff --git a/acceptance/bundle/resources/database_instances/recreate/databricks.yml.tmpl b/acceptance/bundle/resources/database_instances/recreate/databricks.yml.tmpl new file mode 100644 index 00000000000..e814a11b2d8 --- /dev/null +++ b/acceptance/bundle/resources/database_instances/recreate/databricks.yml.tmpl @@ -0,0 +1,11 @@ +bundle: + name: deploy-lakebase-recreate-$UNIQUE_NAME + +sync: + paths: [] + +resources: + database_instances: + my_database: + name: NAME_PLACEHOLDER + capacity: CU_1 diff --git a/acceptance/bundle/resources/database_instances/recreate/out.requests.rename.txt b/acceptance/bundle/resources/database_instances/recreate/out.requests.rename.txt new file mode 100644 index 00000000000..ee03f77e80d --- /dev/null +++ b/acceptance/bundle/resources/database_instances/recreate/out.requests.rename.txt @@ -0,0 +1,15 @@ +{ + "method": "DELETE", + "path": "/api/2.0/database/instances/test-db-old-[UNIQUE_NAME]", + "q": { + "purge": "true" + } +} +{ + "body": { + "capacity": "CU_1", + "name": "test-db-new-[UNIQUE_NAME]" + }, + "method": "POST", + "path": "/api/2.0/database/instances" +} diff --git a/acceptance/bundle/resources/database_instances/recreate/out.test.toml b/acceptance/bundle/resources/database_instances/recreate/out.test.toml new file mode 100644 index 00000000000..c777e3ce206 --- /dev/null +++ b/acceptance/bundle/resources/database_instances/recreate/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false +RequiresUnityCatalog = true +CloudEnvs.gcp = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/database_instances/recreate/output.txt b/acceptance/bundle/resources/database_instances/recreate/output.txt new file mode 100644 index 00000000000..3d40bbc5b09 --- /dev/null +++ b/acceptance/bundle/resources/database_instances/recreate/output.txt @@ -0,0 +1,35 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-recreate-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Rename the instance: name is immutable, so the plan recreates it +>>> [CLI] bundle plan +recreate database_instances.my_database + +Plan: 1 to add, 0 to change, 1 to delete, 0 unchanged + +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-recreate-[UNIQUE_NAME]/default/files... + +This action will result in the deletion or recreation of the following Lakebase database instances. +All data stored in them will be permanently lost: + recreate resources.database_instances.my_database +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.database_instances.my_database + +This action will result in the deletion of the following Lakebase database instances. +All data stored in them will be permanently lost: + delete resources.database_instances.my_database + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-recreate-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/database_instances/recreate/script b/acceptance/bundle/resources/database_instances/recreate/script new file mode 100644 index 00000000000..e331f9ee6ac --- /dev/null +++ b/acceptance/bundle/resources/database_instances/recreate/script @@ -0,0 +1,22 @@ +cleanup() { + trace $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT + +envsubst < databricks.yml.tmpl | sed "s/NAME_PLACEHOLDER/test-db-old-${UNIQUE_NAME}/" > databricks.yml + +trace $CLI bundle deploy +rm -f out.requests.txt + +title "Rename the instance: name is immutable, so the plan recreates it" +sed "s/test-db-old-${UNIQUE_NAME}/test-db-new-${UNIQUE_NAME}/" databricks.yml > databricks.yml.new +mv databricks.yml.new databricks.yml + +trace $CLI bundle plan +trace $CLI bundle deploy --auto-approve + +# The rename must be a DELETE of the old instance plus a POST creating the new +# one. A PATCH to the renamed instance would 404 (the bug this guards against). +jq --sort-keys 'select(.method != "GET" and (.path | contains("/database/instances")))' < out.requests.txt > out.requests.rename.txt +rm -f out.requests.txt diff --git a/acceptance/bundle/resources/database_instances/recreate/test.toml b/acceptance/bundle/resources/database_instances/recreate/test.toml new file mode 100644 index 00000000000..d928c965cec --- /dev/null +++ b/acceptance/bundle/resources/database_instances/recreate/test.toml @@ -0,0 +1,11 @@ +# A database instance name is immutable and the Database API has no rename +# endpoint, so changing it must go through delete + create. Only the direct +# engine handles this correctly; the terraform path needs an upstream provider +# fix (marking name immutable in the spec) and is excluded here. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +RecordRequests = true + +Ignore = [ + "databricks.yml", + ".databricks", +] diff --git a/acceptance/bundle/resources/synced_database_tables/recreate/databricks.yml.tmpl b/acceptance/bundle/resources/synced_database_tables/recreate/databricks.yml.tmpl new file mode 100644 index 00000000000..243df2f1033 --- /dev/null +++ b/acceptance/bundle/resources/synced_database_tables/recreate/databricks.yml.tmpl @@ -0,0 +1,24 @@ +bundle: + name: deploy-lakebase-synced-recreate-$UNIQUE_NAME + +resources: + database_instances: + my_instance: + name: test-db-synced-recreate-$UNIQUE_NAME + capacity: CU_1 + database_catalogs: + my_catalog: + database_instance_name: ${resources.database_instances.my_instance.name} + database_name: my_database + name: my_catalog_$UNIQUE_NAME + create_database_if_not_exists: true + synced_database_tables: + my_synced_table: + name: ${resources.database_catalogs.my_catalog.name}.${resources.database_catalogs.my_catalog.database_name}.my_synced_table + database_instance_name: ${resources.database_instances.my_instance.name} + logical_database_name: ${resources.database_catalogs.my_catalog.database_name} + spec: + source_table_full_name: "main.test_schema.trips_source" + scheduling_policy: POLICY_PLACEHOLDER + primary_key_columns: + - id diff --git a/acceptance/bundle/resources/synced_database_tables/recreate/out.requests.recreate.txt b/acceptance/bundle/resources/synced_database_tables/recreate/out.requests.recreate.txt new file mode 100644 index 00000000000..04f7c103547 --- /dev/null +++ b/acceptance/bundle/resources/synced_database_tables/recreate/out.requests.recreate.txt @@ -0,0 +1,20 @@ +{ + "method": "DELETE", + "path": "/api/2.0/database/synced_tables/my_catalog_[UNIQUE_NAME].my_database.my_synced_table" +} +{ + "body": { + "database_instance_name": "test-db-synced-recreate-[UNIQUE_NAME]", + "logical_database_name": "my_database", + "name": "my_catalog_[UNIQUE_NAME].my_database.my_synced_table", + "spec": { + "primary_key_columns": [ + "id" + ], + "scheduling_policy": "TRIGGERED", + "source_table_full_name": "main.test_schema.trips_source" + } + }, + "method": "POST", + "path": "/api/2.0/database/synced_tables" +} diff --git a/acceptance/bundle/resources/synced_database_tables/recreate/out.test.toml b/acceptance/bundle/resources/synced_database_tables/recreate/out.test.toml new file mode 100644 index 00000000000..c777e3ce206 --- /dev/null +++ b/acceptance/bundle/resources/synced_database_tables/recreate/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false +RequiresUnityCatalog = true +CloudEnvs.gcp = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/bundle/resources/synced_database_tables/recreate/output.txt b/acceptance/bundle/resources/synced_database_tables/recreate/output.txt new file mode 100644 index 00000000000..49b3db00e4e --- /dev/null +++ b/acceptance/bundle/resources/synced_database_tables/recreate/output.txt @@ -0,0 +1,41 @@ + +>>> [CLI] bundle deploy +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-synced-recreate-[UNIQUE_NAME]/default/files... +Deploying resources... +Updating deployment state... +Deployment complete! + +=== Change an immutable synced-table field: no update API, so the plan recreates it +>>> [CLI] bundle plan +recreate synced_database_tables.my_synced_table + +Plan: 1 to add, 0 to change, 1 to delete, 2 unchanged + +>>> [CLI] bundle deploy --auto-approve +Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-synced-recreate-[UNIQUE_NAME]/default/files... + +This action will result in the deletion or recreation of the following synced database tables. +The synced data in the destination database will be lost (the source table is preserved): + recreate resources.synced_database_tables.my_synced_table +Deploying resources... +Updating deployment state... +Deployment complete! + +>>> [CLI] bundle destroy --auto-approve +The following resources will be deleted: + delete resources.database_catalogs.my_catalog + delete resources.database_instances.my_instance + delete resources.synced_database_tables.my_synced_table + +This action will result in the deletion of the following Lakebase database instances. +All data stored in them will be permanently lost: + delete resources.database_instances.my_instance + +This action will result in the deletion of the following synced database tables. +The synced data in the destination database will be lost (the source table is preserved): + delete resources.synced_database_tables.my_synced_table + +All files and directories at the following location will be deleted: /Workspace/Users/[USERNAME]/.bundle/deploy-lakebase-synced-recreate-[UNIQUE_NAME]/default + +Deleting files... +Destroy complete! diff --git a/acceptance/bundle/resources/synced_database_tables/recreate/script b/acceptance/bundle/resources/synced_database_tables/recreate/script new file mode 100644 index 00000000000..f77695bcd53 --- /dev/null +++ b/acceptance/bundle/resources/synced_database_tables/recreate/script @@ -0,0 +1,22 @@ +cleanup() { + trace $CLI bundle destroy --auto-approve + rm -f out.requests.txt +} +trap cleanup EXIT + +envsubst < databricks.yml.tmpl | sed "s/POLICY_PLACEHOLDER/SNAPSHOT/" > databricks.yml + +trace $CLI bundle deploy +rm -f out.requests.txt + +title "Change an immutable synced-table field: no update API, so the plan recreates it" +sed "s/scheduling_policy: SNAPSHOT/scheduling_policy: TRIGGERED/" databricks.yml > databricks.yml.new +mv databricks.yml.new databricks.yml + +trace $CLI bundle plan +trace $CLI bundle deploy --auto-approve + +# The change must be a DELETE of the old synced table plus a POST creating the +# new one. A PATCH would return 501 (the bug this guards against). +jq --sort-keys 'select(.method != "GET" and (.path | contains("/database/synced_tables")))' < out.requests.txt > out.requests.recreate.txt +rm -f out.requests.txt diff --git a/acceptance/bundle/resources/synced_database_tables/recreate/test.toml b/acceptance/bundle/resources/synced_database_tables/recreate/test.toml new file mode 100644 index 00000000000..664eda83b1b --- /dev/null +++ b/acceptance/bundle/resources/synced_database_tables/recreate/test.toml @@ -0,0 +1,10 @@ +# The Database API has no UpdateSyncedDatabaseTable endpoint (PATCH returns 501), +# so any field change must go through delete + create. The direct engine handles +# this; the terraform path would need an upstream provider fix and is excluded. +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +RecordRequests = true + +Ignore = [ + "databricks.yml", + ".databricks", +] diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index f18a84d0efc..8ecdc4e73a5 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -976,6 +976,78 @@ func validateResourceConfig(t *testing.T, stateType reflect.Type, cfg *ResourceL } } +// TestNoUpdateResourcesCoverAllFields ensures that resources which do not +// implement DoUpdate classify every settable field. Without DoUpdate the +// planner rejects any "update" action ("resource does not support update +// action but plan produced update"), so a field a user can change must be +// declared recreate_on_changes (or ignore_local_changes) to recreate instead. +// An output-only field is fine under ignore_remote_changes because the user +// never sets it, so it can never produce a user-driven change. +// +// The check walks the state type and fails on any uncovered scalar leaf. This +// keeps the recreate_on_changes lists exhaustive as the SDK adds fields. +func TestNoUpdateResourcesCoverAllFields(t *testing.T) { + for resourceType, resource := range SupportedResources { + adapter, err := NewAdapter(resource, resourceType, nil) + require.NoError(t, err) + if adapter.HasDoUpdate() { + continue + } + if adapter.HasOverrideChangeDesc() { + // OverrideChangeDesc classifies changes programmatically (e.g. + // vector_search_indexes handles endpoint_uuid there), which this + // static walk can't see into. + continue + } + + // A user change is only neutralized by recreate_on_changes or + // ignore_local_changes; output-only fields are covered by + // ignore_remote_changes since the user never sets them. + covered := map[string]bool{} + for _, cfg := range []*ResourceLifecycleConfig{adapter.ResourceConfig(), adapter.GeneratedResourceConfig()} { + if cfg == nil { + continue + } + for _, r := range cfg.RecreateOnChanges { + covered[r.Field.String()] = true + } + for _, r := range cfg.IgnoreLocalChanges { + covered[r.Field.String()] = true + } + for _, r := range cfg.IgnoreRemoteChanges { + if strings.HasSuffix(r.Reason, "output_only") { + covered[r.Field.String()] = true + } + } + } + + t.Run(resourceType, func(t *testing.T) { + err := structwalk.WalkType(adapter.StateType(), func(path *structpath.PatternNode, typ reflect.Type, _ *reflect.StructField) bool { + if path.IsRoot() { + return true + } + if covered[path.String()] { + // This field (or its enclosing object) is classified; the + // whole subtree is covered, so stop descending. + return false + } + for typ.Kind() == reflect.Pointer { + typ = typ.Elem() + } + switch typ.Kind() { + case reflect.Struct, reflect.Slice, reflect.Array, reflect.Map: + // Intermediate node: descend and check its children. + return true + default: + t.Errorf("field %q is not classified; a change to it would plan an unsupported update. Add it to recreate_on_changes.", path) + return false + } + }) + require.NoError(t, err) + }) + } +} + func setupTestServerClient(t *testing.T) (*testserver.Server, *databricks.WorkspaceClient) { server := testserver.New(t) testserver.AddDefaultHandlers(server) diff --git a/bundle/direct/dresources/database_catalog.go b/bundle/direct/dresources/database_catalog.go index 9bffa708d73..a1a7ebdef44 100644 --- a/bundle/direct/dresources/database_catalog.go +++ b/bundle/direct/dresources/database_catalog.go @@ -34,17 +34,6 @@ func (r *ResourceDatabaseCatalog) DoCreate(ctx context.Context, config *database return result.Name, nil, nil } -func (r *ResourceDatabaseCatalog) DoUpdate(ctx context.Context, id string, config *database.DatabaseCatalog, _ *PlanEntry) (*database.DatabaseCatalog, error) { - request := database.UpdateDatabaseCatalogRequest{ - DatabaseCatalog: *config, - Name: id, - UpdateMask: "*", - } - - _, err := r.client.Database.UpdateDatabaseCatalog(ctx, request) - return nil, err -} - func (r *ResourceDatabaseCatalog) DoDelete(ctx context.Context, id string, _ *database.DatabaseCatalog) error { return r.client.Database.DeleteDatabaseCatalog(ctx, database.DeleteDatabaseCatalogRequest{ Name: id, diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index 75a60e0dc62..e6e32ac638c 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -492,6 +492,62 @@ resources: # m["enable_serverless_compute"].Computed = true - field: enable_serverless_compute + database_instances: + recreate_on_changes: + # name is the instance identifier (PATCH /api/2.0/database/instances/{name}) + # and the Database API has no rename endpoint, so changing it requires + # delete + create. The OpenAPI spec only marks parent_instance_ref as + # IMMUTABLE (handled by autogen in resources.generated.yml); name needs an + # explicit entry here. + - field: name + reason: immutable + + database_catalogs: + # The Database API has no UpdateDatabaseCatalog endpoint (the generated SDK + # method is a stub that returns 501 NOT_IMPLEMENTED), so this resource + # implements no DoUpdate and every settable field must recreate. The + # complementary ignore_remote_changes block in resources.generated.yml + # handles the read side (create_database_if_not_exists is input-only, uid is + # output-only) so no-op deploys stay idempotent. + recreate_on_changes: + - field: name + reason: immutable + - field: database_instance_name + reason: immutable + - field: database_name + reason: immutable + - field: create_database_if_not_exists + reason: immutable + + synced_database_tables: + # The Database API has no UpdateSyncedDatabaseTable endpoint (the generated + # SDK method is a stub that returns 501 NOT_IMPLEMENTED), so this resource + # implements no DoUpdate and every settable field must recreate. The + # complementary ignore_remote_changes block in resources.generated.yml + # handles the read side (input-only and output-only fields) so no-op deploys + # stay idempotent. Same pattern as postgres_synced_tables. + recreate_on_changes: + - field: name + reason: immutable + - field: database_instance_name + reason: immutable + - field: logical_database_name + reason: immutable + - field: spec.source_table_full_name + reason: immutable + - field: spec.primary_key_columns + reason: immutable + - field: spec.timeseries_key + reason: immutable + - field: spec.scheduling_policy + reason: immutable + - field: spec.create_database_objects_if_missing + reason: immutable + - field: spec.new_pipeline_spec + reason: immutable + - field: spec.existing_pipeline_id + reason: immutable + postgres_projects: recreate_on_changes: # project_id is immutable (part of hierarchical name, not in API spec) diff --git a/bundle/direct/dresources/synced_database_table.go b/bundle/direct/dresources/synced_database_table.go index d45c6fb3fc7..e1c0df12f00 100644 --- a/bundle/direct/dresources/synced_database_table.go +++ b/bundle/direct/dresources/synced_database_table.go @@ -34,17 +34,6 @@ func (r *ResourceSyncedDatabaseTable) DoCreate(ctx context.Context, config *data return result.Name, nil, nil } -func (r *ResourceSyncedDatabaseTable) DoUpdate(ctx context.Context, id string, config *database.SyncedDatabaseTable, _ *PlanEntry) (*database.SyncedDatabaseTable, error) { - request := database.UpdateSyncedDatabaseTableRequest{ - SyncedTable: *config, - Name: id, - UpdateMask: "*", - } - - _, err := r.client.Database.UpdateSyncedDatabaseTable(ctx, request) - return nil, err -} - func (r *ResourceSyncedDatabaseTable) DoDelete(ctx context.Context, id string, _ *database.SyncedDatabaseTable) error { return r.client.Database.DeleteSyncedDatabaseTable(ctx, database.DeleteSyncedDatabaseTableRequest{ Name: id, diff --git a/libs/testserver/database_catalogs.go b/libs/testserver/database_catalogs.go index bd02a6442ff..d6dbdc49687 100644 --- a/libs/testserver/database_catalogs.go +++ b/libs/testserver/database_catalogs.go @@ -62,42 +62,16 @@ func (s *FakeWorkspace) DatabaseCatalogGet(name string) Response { return Response{Body: result} } +// DatabaseCatalogUpdate models the real Database API, which has no update +// endpoint: PATCH /api/2.0/database/catalogs/{name} returns 501 NOT_IMPLEMENTED. +// The direct engine recreates database_catalogs on any change and never calls +// this; modeling the 501 guards against reintroducing an update path. func (s *FakeWorkspace) DatabaseCatalogUpdate(req Request, name string) Response { - defer s.LockUnlock()() - - var updateRequest database.UpdateDatabaseCatalogRequest - err := json.Unmarshal(req.Body, &updateRequest) - if err != nil { - return Response{ - Body: fmt.Sprintf("cannot unmarshal request body: %v", err), - StatusCode: 400, - } - } - - // Check if the catalog exists - existingCatalog, exists := s.DatabaseCatalogs[name] - if !exists { - return Response{ - Body: fmt.Sprintf("database catalog with name '%s' not found", name), - StatusCode: 404, - } - } - - // Update the catalog with the new config - updatedCatalog := updateRequest.DatabaseCatalog - if updatedCatalog.Name == "" { - updatedCatalog.Name = existingCatalog.Name - } - if updatedCatalog.DatabaseInstanceName == "" { - updatedCatalog.DatabaseInstanceName = existingCatalog.DatabaseInstanceName - } - if updatedCatalog.DatabaseName == "" { - updatedCatalog.DatabaseName = existingCatalog.DatabaseName - } - - s.DatabaseCatalogs[name] = updatedCatalog - return Response{ - Body: updatedCatalog, + StatusCode: 501, + Body: map[string]string{ + "error_code": "NOT_IMPLEMENTED", + "message": "Update Database Catalog is not yet implemented", + }, } } diff --git a/libs/testserver/synced_database_tables.go b/libs/testserver/synced_database_tables.go index 9ac81483945..13c81539909 100644 --- a/libs/testserver/synced_database_tables.go +++ b/libs/testserver/synced_database_tables.go @@ -43,33 +43,17 @@ func (s *FakeWorkspace) SyncedDatabaseTableCreate(req Request) Response { } } +// SyncedDatabaseTableUpdate models the real Database API, which has no update +// endpoint: PATCH /api/2.0/database/synced_tables/{name} returns 501 +// NOT_IMPLEMENTED. The direct engine recreates synced_database_tables on any +// change and never calls this; modeling the 501 guards against reintroducing an +// update path. func (s *FakeWorkspace) SyncedDatabaseTableUpdate(req Request, name string) Response { - defer s.LockUnlock()() - - var updateReq database.UpdateSyncedDatabaseTableRequest - if err := json.Unmarshal(req.Body, &updateReq); err != nil { - return Response{ - Body: fmt.Sprintf("cannot unmarshal request body: %v", err), - StatusCode: 400, - } - } - - // Ensure the resource exists - existing, ok := s.SyncedDatabaseTables[name] - if !ok { - return Response{ - Body: fmt.Sprintf("synced database table with name '%s' not found", name), - StatusCode: 404, - } - } - - // Apply updates: shallow replace with provided struct while preserving name - updated := updateReq.SyncedTable - if updated.Name == "" { - updated.Name = existing.Name + return Response{ + StatusCode: 501, + Body: map[string]string{ + "error_code": "NOT_IMPLEMENTED", + "message": "Update Synced Database Table is not yet implemented.", + }, } - - s.SyncedDatabaseTables[name] = updated - - return Response{Body: updated} } From 02dd7ff543c697fee7a2537fcd73e192fa13636a Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 17 Jun 2026 10:28:14 +0200 Subject: [PATCH 2/2] Fix duplicate resource keys after merge of origin/main The merge left two entries each for database_instances, database_catalogs, and synced_database_tables in resources.yml: provided_id_fields from main and recreate_on_changes from this branch. yaml.Unmarshal rejects duplicate mapping keys, so MustLoadConfig panicked at init on every direct-engine deploy. Merge each pair into one entry. name is now classified only via provided_id_fields (classifyIDField already plans Recreate on change, and runs before the recreate_on_changes check), so the explicit recreate_on_changes name entries were redundant. database_instances keeps just provided_id_fields; the other two keep recreate_on_changes for their remaining immutable fields. Count provided_id_fields as covered in TestNoUpdateResourcesCoverAllFields, since a change to such a field recreates rather than planning an unsupported update. Co-authored-by: Isaac --- bundle/direct/dresources/all_test.go | 14 ++-- bundle/direct/dresources/resources.yml | 94 +++++++++++--------------- 2 files changed, 47 insertions(+), 61 deletions(-) diff --git a/bundle/direct/dresources/all_test.go b/bundle/direct/dresources/all_test.go index a9485a7a17a..bfbe0b163c1 100644 --- a/bundle/direct/dresources/all_test.go +++ b/bundle/direct/dresources/all_test.go @@ -1058,8 +1058,9 @@ func validateResourceConfig(t *testing.T, stateType reflect.Type, cfg *ResourceL // planner rejects any "update" action ("resource does not support update // action but plan produced update"), so a field a user can change must be // declared recreate_on_changes (or ignore_local_changes) to recreate instead. -// An output-only field is fine under ignore_remote_changes because the user -// never sets it, so it can never produce a user-driven change. +// A provided_id_field also recreates on change (classifyIDField), so it counts +// as covered too. An output-only field is fine under ignore_remote_changes +// because the user never sets it, so it can never produce a user-driven change. // // The check walks the state type and fails on any uncovered scalar leaf. This // keeps the recreate_on_changes lists exhaustive as the SDK adds fields. @@ -1077,9 +1078,9 @@ func TestNoUpdateResourcesCoverAllFields(t *testing.T) { continue } - // A user change is only neutralized by recreate_on_changes or - // ignore_local_changes; output-only fields are covered by - // ignore_remote_changes since the user never sets them. + // A user change is only neutralized by recreate_on_changes, + // provided_id_fields, or ignore_local_changes; output-only fields are + // covered by ignore_remote_changes since the user never sets them. covered := map[string]bool{} for _, cfg := range []*ResourceLifecycleConfig{adapter.ResourceConfig(), adapter.GeneratedResourceConfig()} { if cfg == nil { @@ -1088,6 +1089,9 @@ func TestNoUpdateResourcesCoverAllFields(t *testing.T) { for _, r := range cfg.RecreateOnChanges { covered[r.Field.String()] = true } + for _, r := range cfg.ProvidedIDFields { + covered[r.Field.String()] = true + } for _, r := range cfg.IgnoreLocalChanges { covered[r.Field.String()] = true } diff --git a/bundle/direct/dresources/resources.yml b/bundle/direct/dresources/resources.yml index dff618edd4e..5b48218ecb9 100644 --- a/bundle/direct/dresources/resources.yml +++ b/bundle/direct/dresources/resources.yml @@ -420,11 +420,49 @@ resources: provided_id_fields: - field: name reason: id_field + # The Database API has no UpdateDatabaseCatalog endpoint (the generated SDK + # method is a stub that returns 501 NOT_IMPLEMENTED), so this resource + # implements no DoUpdate and every settable field must recreate. The + # complementary ignore_remote_changes block in resources.generated.yml + # handles the read side (create_database_if_not_exists is input-only, uid is + # output-only) so no-op deploys stay idempotent. + recreate_on_changes: + - field: database_instance_name + reason: immutable + - field: database_name + reason: immutable + - field: create_database_if_not_exists + reason: immutable synced_database_tables: provided_id_fields: - field: name reason: id_field + # The Database API has no UpdateSyncedDatabaseTable endpoint (the generated + # SDK method is a stub that returns 501 NOT_IMPLEMENTED), so this resource + # implements no DoUpdate and every settable field must recreate. The + # complementary ignore_remote_changes block in resources.generated.yml + # handles the read side (input-only and output-only fields) so no-op deploys + # stay idempotent. Same pattern as postgres_synced_tables. + recreate_on_changes: + - field: database_instance_name + reason: immutable + - field: logical_database_name + reason: immutable + - field: spec.source_table_full_name + reason: immutable + - field: spec.primary_key_columns + reason: immutable + - field: spec.timeseries_key + reason: immutable + - field: spec.scheduling_policy + reason: immutable + - field: spec.create_database_objects_if_missing + reason: immutable + - field: spec.new_pipeline_spec + reason: immutable + - field: spec.existing_pipeline_id + reason: immutable apps: provided_id_fields: @@ -560,62 +598,6 @@ resources: # m["enable_serverless_compute"].Computed = true - field: enable_serverless_compute - database_instances: - recreate_on_changes: - # name is the instance identifier (PATCH /api/2.0/database/instances/{name}) - # and the Database API has no rename endpoint, so changing it requires - # delete + create. The OpenAPI spec only marks parent_instance_ref as - # IMMUTABLE (handled by autogen in resources.generated.yml); name needs an - # explicit entry here. - - field: name - reason: immutable - - database_catalogs: - # The Database API has no UpdateDatabaseCatalog endpoint (the generated SDK - # method is a stub that returns 501 NOT_IMPLEMENTED), so this resource - # implements no DoUpdate and every settable field must recreate. The - # complementary ignore_remote_changes block in resources.generated.yml - # handles the read side (create_database_if_not_exists is input-only, uid is - # output-only) so no-op deploys stay idempotent. - recreate_on_changes: - - field: name - reason: immutable - - field: database_instance_name - reason: immutable - - field: database_name - reason: immutable - - field: create_database_if_not_exists - reason: immutable - - synced_database_tables: - # The Database API has no UpdateSyncedDatabaseTable endpoint (the generated - # SDK method is a stub that returns 501 NOT_IMPLEMENTED), so this resource - # implements no DoUpdate and every settable field must recreate. The - # complementary ignore_remote_changes block in resources.generated.yml - # handles the read side (input-only and output-only fields) so no-op deploys - # stay idempotent. Same pattern as postgres_synced_tables. - recreate_on_changes: - - field: name - reason: immutable - - field: database_instance_name - reason: immutable - - field: logical_database_name - reason: immutable - - field: spec.source_table_full_name - reason: immutable - - field: spec.primary_key_columns - reason: immutable - - field: spec.timeseries_key - reason: immutable - - field: spec.scheduling_policy - reason: immutable - - field: spec.create_database_objects_if_missing - reason: immutable - - field: spec.new_pipeline_spec - reason: immutable - - field: spec.existing_pipeline_id - reason: immutable - postgres_projects: provided_id_fields: # project_id is immutable (part of hierarchical name, not in API spec)