Mask only leaf paths in postgres update_mask - #6440
Conversation
|
Verified that test is regression test, results on main: Regression Test ReportTested commit: 0308da2 Shorten the changelog entry and link the PR
TestAccept/bundle/resources/postgres_projects/update_default_endpoint_autoscaling/DATABRICKS_BUNDLE_ENGINE=direct ✅ | main (0a8aae1) ❌ | latest ➖main (0a8aae1): TestAccept/bundle/resources/postgres_projects/update_default_endpoint_suspend/DATABRICKS_BUNDLE_ENGINE=direct ✅ | main (0a8aae1) ❌ | latest ➖main (0a8aae1): |
| @@ -0,0 +1,3 @@ | |||
| Fixed deploying a change to a field nested inside a Lakebase message, such as | |||
| `postgres_projects.default_endpoint_settings.autoscaling_limit_max_cu`, when the bundle | |||
| declares no suspension field ([#6440](https://github.com/databricks/cli/pull/6440)). | |||
There was a problem hiding this comment.
No need to pre-wrap. This is copied verbatim to the changelog: https://github.com/databricks/cli/actions/runs/33405684720?pr=6440
There was a problem hiding this comment.
The message itself is quite cryptic. Can we make it simpler, just "Fix deploying updates to default_endpoint_settings" or something?
There was a problem hiding this comment.
Unwrapped, now one line.
There was a problem hiding this comment.
Took your wording:
direct: Fix deploying an update to `postgres_projects.default_endpoint_settings` ([#6440](https://github.com/databricks/cli/pull/6440)).
Same treatment applied to the two entries in the PRs stacked on this one.
| title "Change one field inside default_endpoint_settings" | ||
| # The bundle declares no suspension field, which is what makes this a regression test: | ||
| # masking the enclosing message asks the API to replace it wholesale, and the API then | ||
| # requires the suspension oneof to be populated in the body. Only the leaf may be masked. |
There was a problem hiding this comment.
This is also problematic; why would the backend accept POST without this field but reject PATCH without this field... Hints at asymmetry in the backend code.
There was a problem hiding this comment.
Claude did some probing there is a default for this field on the backend.
| request | suspension field in body | result |
|---|---|---|
POST /projects |
absent | accepted, backend defaults it to 86400s |
PATCH ?update_mask=spec |
absent | accepted |
PATCH ?update_mask=spec.default_endpoint_settings.autoscaling_limit_max_cu |
absent | accepted |
PATCH ?update_mask=spec.default_endpoint_settings |
absent | 400 Field 'spec.default_endpoint_settings.suspension' is in update_mask but not provided in request |
There were two helpers building update_mask from the plan's change paths, one that kept parent paths and one that dropped them, and the postgres resources were split between them. Keep the one that drops parents. Masking a message asks the API to replace it wholesale, and the API then requires every oneof group beneath that message to be populated in the request body. Probed against a real workspace: update_mask=spec.default_endpoint_settings with a body carrying only the autoscaling limits is rejected with "Field 'spec.default_endpoint_settings.suspension' is in update_mask but not provided in request". A bundle only sends the fields it declares, so masking the parent can never be right for us. Co-authored-by: Isaac
Masking a nested message makes the API demand the oneof groups directly beneath it, so a bundle that changes default_endpoint_settings.autoscaling_limit_max_cu without declaring a suspension field could not deploy at all. The fake accepted it, which is why nothing caught this. Teach the fake the rule, in exactly the three shapes probed against a real workspace: masking a nested message requires the groups below it, masking the top-level spec does not, at either depth. The new test fails without the previous commit and passes with it, locally and on cloud. Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
Co-authored-by: Isaac
76bec58 to
964c675
Compare
Stacked on #6441 (which is stacked on #6440) — review those first. Four changes a bundle can express could not be deployed at all: | resource | field | |---|---| | `postgres_branches` | `expire_time`, `ttl` | | `postgres_endpoints` | `suspend_timeout_duration` | | `postgres_projects` | `default_endpoint_settings.suspend_timeout_duration` | `expire_time` / `ttl` / `no_expiry` are one oneof and `suspend_timeout_duration` / `no_suspension` another, and the API accepts them in `update_mask` only under the group name — masking the field itself is answered with `Unknown field path in update_mask`. All four now apply and their tests drop `Badness`. The group names are in neither the OpenAPI spec nor the SDK doc comments, so each map is hand-written from what the backend accepts, probed on 2026-08-31. Two members of one group collapse onto a single mask entry. `remove_suspend_timeout` still fails, and the mask is no longer why: the API requires a masked field to be populated in the body, so a removal has nothing to send. An absent value and an explicit `null` are both rejected; the supported way to express it is `no_suspension: true`. That test keeps a `Badness` saying so. This pull request and its description were written by Isaac.
Stacked on databricks#6440 — review that one first. A bundle that changes one key of `postgres_endpoints.settings.pg_settings` cannot deploy: ``` Unknown field path in update_mask: 'spec.settings.pg_settings['statement_timeout']' ``` The plan diffs maps entry by entry, so the change path carries the map key and the mask repeated it verbatim. A map or repeated field is addressable only as a whole. Probed against a real endpoint on 2026-08-31: `spec.settings` and `spec.settings.pg_settings` are both accepted, the indexed form is not. So drop everything from the first subscript on, and dedupe — two changed entries of one map collapse onto the same path. Terraform is unaffected; it masks the whole spec. Two tests, both local and cloud: - `update_pg_settings` — edit a key. Fails without this change. - `add_settings` — add the whole block, which leaves one change path and masks the message itself. That path was missing from the fake's allowed list, so a case the real API accepts was failing locally. This pull request and its description were written by Isaac.
## Release v1.15.0 ### CLI * When `uv python install` fails, `databricks environments setup-local` now falls back to a compatible Python interpreter already installed on the machine. ([#6457](#6457)) * Allow `databricks environments setup-local` to update `pyproject.toml` files containing TOML multi-line strings. ([#6445](#6445)) ### Bundles * Before committing the automatic terraform→direct migration, run a deployment plan against the converted state; if the plan fails the migration is abandoned. ([#6486](#6486)) * The `dbt-sql` bundle template now uses Databricks Runtime 16.4 LTS (up from 15.4 LTS) for classic (non-serverless) compute. ([#6418](#6418)) * Fixed the direct engine silently ignoring edits to duration and timestamp fields, such as a Lakebase endpoint's `suspend_timeout_duration`. Such a change planned `0 to change` and was never applied. ([#6377](#6377)) * Fixed `$${...}` not escaping a literal `${...}` on the direct engine, which failed with an `invalid dependency` error. ([#6484](#6484), [#6489](#6489)) * direct: Fix deploying an update to `postgres_projects.default_endpoint_settings`. ([#6440](#6440)) * direct: Fix deploying an update to `postgres_endpoints.settings.pg_settings`. ([#6441](#6441)) * direct: Fix deploying an update to `expire_time`, `ttl` or `suspend_timeout_duration` on Lakebase resources. ([#6443](#6443)) * Added PyDABs (Python) support for catalogs: `Resources.add_catalog` and the `catalog_mutator` decorator. ([#6408](#6408)) * Bundle templates now use serverless [environment version 5](https://docs.databricks.com/aws/en/release-notes/serverless/environment-version/five), which offers better performance, and `databricks-connect` 16.4. ([#6378](#6378)) * Fixed a job with a `table_update` trigger never converging on the direct engine. ([#6442](#6442)) ### Dependency Updates * Bump Go toolchain to 1.26.8. ([#6476](#6476))
## Release v1.15.0 ### CLI * When `uv python install` fails, `databricks environments setup-local` now falls back to a compatible Python interpreter already installed on the machine. ([#6457](#6457)) * Allow `databricks environments setup-local` to update `pyproject.toml` files containing TOML multi-line strings. ([#6445](#6445)) ### Bundles * Before committing the automatic terraform→direct migration, run a deployment plan against the converted state; if the plan fails the migration is abandoned. ([#6486](#6486)) * The `dbt-sql` bundle template now uses Databricks Runtime 16.4 LTS (up from 15.4 LTS) for classic (non-serverless) compute. ([#6418](#6418)) * Fixed the direct engine silently ignoring edits to duration and timestamp fields, such as a Lakebase endpoint's `suspend_timeout_duration`. Such a change planned `0 to change` and was never applied. ([#6377](#6377)) * Fixed `$${...}` not escaping a literal `${...}` on the direct engine, which failed with an `invalid dependency` error. ([#6484](#6484), [#6489](#6489)) * Remove forward_user_access_token from update_mask for Apps because it's not supported. Fixes regression in 1.14.1. ([#6510](#6510)) * direct: Fix deploying an update to `postgres_projects.default_endpoint_settings`. ([#6440](#6440)) * direct: Fix deploying an update to `postgres_endpoints.settings.pg_settings`. ([#6441](#6441)) * direct: Fix deploying an update to `expire_time`, `ttl` or `suspend_timeout_duration` on Lakebase resources. ([#6443](#6443)) * Added PyDABs (Python) support for catalogs: `Resources.add_catalog` and the `catalog_mutator` decorator. ([#6408](#6408)) * Bundle templates now use serverless [environment version 5](https://docs.databricks.com/aws/en/release-notes/serverless/environment-version/five), which offers better performance, and `databricks-connect` 16.4. ([#6378](#6378)) * Fixed a job with a `table_update` trigger never converging on the direct engine. ([#6442](#6442)) ### Dependency Updates * Bump Go toolchain to 1.26.8. ([#6476](#6476))
Growing a config with a nested block, e.g. adding default_endpoint_settings
with just an autoscaling limit, failed the deploy with
400 INVALID_PARAMETER_VALUE
Field 'spec.default_endpoint_settings.suspension' is in update_mask
but not provided in request
#6440 masks only leaves when a child of the message changed, but a block
added as a whole produces a single change on the message itself, and that
was masked as-is. Expand such a change to the fields the request body
actually carries.
A map, a repeated field and a wrapper like duration.Duration do not expand:
the first two are addressable only as a whole, and the last is a struct in
Go but a scalar on the wire.
Co-authored-by: Isaac
Growing a config with a nested block — adding default_endpoint_settings with
just an autoscaling limit — failed the deploy with
400 INVALID_PARAMETER_VALUE
Field 'spec.default_endpoint_settings.suspension' is in update_mask
but not provided in request
#6440 masks only leaves when a child of the message changed, but a block added
as a whole leaves a single change on the message itself, and that was masked
as-is. Expand such a change to the fields the request body carries.
Two things do not expand. A message the body populates completely, because no
requirement the API places on a masked field can then go unmet and replacing
the message is what the config declares — that keeps spec.settings as the mask
for endpoint settings. And a map, a repeated field or a wrapper like
duration.Duration: the first two are addressable only as a whole, the last is a
struct in Go but a scalar on the wire.
The expansion reads the spec the request body carries rather than the plan's own
copy of the new value, because a plan read back from disk carries that copy as
deserialized JSON with the types erased; deploying a saved plan would otherwise
still send the message path and fail.
Co-authored-by: Isaac
Growing a config with a nested block — adding default_endpoint_settings with
just autoscaling limits — failed the deploy with
400 INVALID_PARAMETER_VALUE
Field 'spec.default_endpoint_settings.suspension' is in update_mask
but not provided in request
#6440 masks only leaves when a child of the message changed, but a block added
as a whole leaves a single change on the message itself, and that was masked
as-is. Expand such a change to the fields the request body carries.
Two things do not expand. A message the body populates completely, because no
requirement the API places on a masked field can then go unmet and replacing
the message is what the config declares — that keeps spec.settings as the mask
for endpoint settings. And a map, a repeated field or a wrapper like
duration.Duration: the first two are addressable only as a whole, the last is a
struct in Go but a scalar on the wire.
The expansion reads the spec the request body carries rather than the plan's own
copy of the new value, because a plan read back from disk carries that copy as
deserialized JSON with the types erased; deploying a saved plan would otherwise
still send the message path and fail. The READPLAN cell of the acceptance test
covers that path.
This turns the postgres_projects/add_default_endpoint_settings case that #6566
recorded as broken into a passing one; its Badness marker is removed.
Co-authored-by: Isaac
A bundle that changes a field nested inside a Lakebase message — say `postgres_projects.default_endpoint_settings.autoscaling_limit_max_cu` — cannot deploy at all when it declares no suspension field: ``` Field 'spec.default_endpoint_settings.suspension' is in update_mask but not provided in request ``` There were two helpers building `update_mask` from the plan's change paths, one that kept parent paths and one that dropped them, and the postgres resources were split between them. The one that keeps parents masks `spec.default_endpoint_settings` alongside the leaf, which asks the API to replace that message wholesale — and the API then requires the oneof groups directly beneath it to be populated in the body. A bundle only sends the fields it declares, so masking the parent can never be right for us. Keep the helper that drops parents. `postgres_projects/update_default_endpoint_autoscaling` covers it and fails without the first commit, locally and on cloud. The fake accepted the broken request, which is why nothing caught this. It now models the rule in exactly the three shapes probed against a real workspace on 2026-08-31: | mask | body without a suspension field | |---|---| | `spec.default_endpoint_settings` | rejected | | `spec` (project, group two levels below) | accepted | | `spec` (endpoint, group one level below) | accepted | So the rule is neither "every group under the mask" nor "every group one level under it". The fake reproduces only what was measured; widening it needs another probe. `update_default_endpoint_suspend` still fails on the oneof group name — separate fix. This pull request and its description were written by Isaac.
Stacked on #6440 — review that one first. A bundle that changes one key of `postgres_endpoints.settings.pg_settings` cannot deploy: ``` Unknown field path in update_mask: 'spec.settings.pg_settings['statement_timeout']' ``` The plan diffs maps entry by entry, so the change path carries the map key and the mask repeated it verbatim. A map or repeated field is addressable only as a whole. Probed against a real endpoint on 2026-08-31: `spec.settings` and `spec.settings.pg_settings` are both accepted, the indexed form is not. So drop everything from the first subscript on, and dedupe — two changed entries of one map collapse onto the same path. Terraform is unaffected; it masks the whole spec. Two tests, both local and cloud: - `update_pg_settings` — edit a key. Fails without this change. - `add_settings` — add the whole block, which leaves one change path and masks the message itself. That path was missing from the fake's allowed list, so a case the real API accepts was failing locally. This pull request and its description were written by Isaac.
Stacked on #6441 (which is stacked on #6440) — review those first. Four changes a bundle can express could not be deployed at all: | resource | field | |---|---| | `postgres_branches` | `expire_time`, `ttl` | | `postgres_endpoints` | `suspend_timeout_duration` | | `postgres_projects` | `default_endpoint_settings.suspend_timeout_duration` | `expire_time` / `ttl` / `no_expiry` are one oneof and `suspend_timeout_duration` / `no_suspension` another, and the API accepts them in `update_mask` only under the group name — masking the field itself is answered with `Unknown field path in update_mask`. All four now apply and their tests drop `Badness`. The group names are in neither the OpenAPI spec nor the SDK doc comments, so each map is hand-written from what the backend accepts, probed on 2026-08-31. Two members of one group collapse onto a single mask entry. `remove_suspend_timeout` still fails, and the mask is no longer why: the API requires a masked field to be populated in the body, so a removal has nothing to send. An absent value and an explicit `null` are both rejected; the supported way to express it is `no_suspension: true`. That test keeps a `Badness` saying so. This pull request and its description were written by Isaac.
## Release v1.15.0 ### CLI * When `uv python install` fails, `databricks environments setup-local` now falls back to a compatible Python interpreter already installed on the machine. ([#6457](#6457)) * Allow `databricks environments setup-local` to update `pyproject.toml` files containing TOML multi-line strings. ([#6445](#6445)) ### Bundles * Before committing the automatic terraform→direct migration, run a deployment plan against the converted state; if the plan fails the migration is abandoned. ([#6486](#6486)) * The `dbt-sql` bundle template now uses Databricks Runtime 16.4 LTS (up from 15.4 LTS) for classic (non-serverless) compute. ([#6418](#6418)) * Fixed the direct engine silently ignoring edits to duration and timestamp fields, such as a Lakebase endpoint's `suspend_timeout_duration`. Such a change planned `0 to change` and was never applied. ([#6377](#6377)) * Fixed `$${...}` not escaping a literal `${...}` on the direct engine, which failed with an `invalid dependency` error. ([#6484](#6484), [#6489](#6489)) * direct: Fix deploying an update to `postgres_projects.default_endpoint_settings`. ([#6440](#6440)) * direct: Fix deploying an update to `postgres_endpoints.settings.pg_settings`. ([#6441](#6441)) * direct: Fix deploying an update to `expire_time`, `ttl` or `suspend_timeout_duration` on Lakebase resources. ([#6443](#6443)) * Added PyDABs (Python) support for catalogs: `Resources.add_catalog` and the `catalog_mutator` decorator. ([#6408](#6408)) * Bundle templates now use serverless [environment version 5](https://docs.databricks.com/aws/en/release-notes/serverless/environment-version/five), which offers better performance, and `databricks-connect` 16.4. ([#6378](#6378)) * Fixed a job with a `table_update` trigger never converging on the direct engine. ([#6442](#6442)) ### Dependency Updates * Bump Go toolchain to 1.26.8. ([#6476](#6476))
## Release v1.15.0 ### CLI * When `uv python install` fails, `databricks environments setup-local` now falls back to a compatible Python interpreter already installed on the machine. ([#6457](#6457)) * Allow `databricks environments setup-local` to update `pyproject.toml` files containing TOML multi-line strings. ([#6445](#6445)) ### Bundles * Before committing the automatic terraform→direct migration, run a deployment plan against the converted state; if the plan fails the migration is abandoned. ([#6486](#6486)) * The `dbt-sql` bundle template now uses Databricks Runtime 16.4 LTS (up from 15.4 LTS) for classic (non-serverless) compute. ([#6418](#6418)) * Fixed the direct engine silently ignoring edits to duration and timestamp fields, such as a Lakebase endpoint's `suspend_timeout_duration`. Such a change planned `0 to change` and was never applied. ([#6377](#6377)) * Fixed `$${...}` not escaping a literal `${...}` on the direct engine, which failed with an `invalid dependency` error. ([#6484](#6484), [#6489](#6489)) * Remove forward_user_access_token from update_mask for Apps because it's not supported. Fixes regression in 1.14.1. ([#6510](#6510)) * direct: Fix deploying an update to `postgres_projects.default_endpoint_settings`. ([#6440](#6440)) * direct: Fix deploying an update to `postgres_endpoints.settings.pg_settings`. ([#6441](#6441)) * direct: Fix deploying an update to `expire_time`, `ttl` or `suspend_timeout_duration` on Lakebase resources. ([#6443](#6443)) * Added PyDABs (Python) support for catalogs: `Resources.add_catalog` and the `catalog_mutator` decorator. ([#6408](#6408)) * Bundle templates now use serverless [environment version 5](https://docs.databricks.com/aws/en/release-notes/serverless/environment-version/five), which offers better performance, and `databricks-connect` 16.4. ([#6378](#6378)) * Fixed a job with a `table_update` trigger never converging on the direct engine. ([#6442](#6442)) ### Dependency Updates * Bump Go toolchain to 1.26.8. ([#6476](#6476))
A bundle that changes a field nested inside a Lakebase message — say
postgres_projects.default_endpoint_settings.autoscaling_limit_max_cu— cannot deploy at all when it declares no suspension field:There were two helpers building
update_maskfrom the plan's change paths, one that kept parent paths and one that dropped them, and the postgres resources were split between them. The one that keeps parents masksspec.default_endpoint_settingsalongside the leaf, which asks the API to replace that message wholesale — and the API then requires the oneof groups directly beneath it to be populated in the body. A bundle only sends the fields it declares, so masking the parent can never be right for us. Keep the helper that drops parents.postgres_projects/update_default_endpoint_autoscalingcovers it and fails without the first commit, locally and on cloud.The fake accepted the broken request, which is why nothing caught this. It now models the rule in exactly the three shapes probed against a real workspace on 2026-08-31:
spec.default_endpoint_settingsspec(project, group two levels below)spec(endpoint, group one level below)So the rule is neither "every group under the mask" nor "every group one level under it". The fake reproduces only what was measured; widening it needs another probe.
update_default_endpoint_suspendstill fails on the oneof group name — separate fix.This pull request and its description were written by Isaac.