Skip to content

Drop empty strings on omitempty fields before deploy - #6088

Open
denik wants to merge 6 commits into
mainfrom
denik/empty-string-cleanup
Open

Drop empty strings on omitempty fields before deploy#6088
denik wants to merge 6 commits into
mainfrom
denik/empty-string-cleanup

Conversation

@denik

@denik denik commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Why

An optional (omitempty) resource field set to "" — often via a variable that resolved to an empty string, e.g. policy_id: ${var.x} — was force-sent to the backend and rejected with '' is not a valid cluster policy ID. Terraform dropped these via omitempty, but the direct engine sent them.

Changes

A new convert.DropEmptyStrings normalize option removes empty-string values on omitempty fields, applied by a DropEmptyStrings mutator in the initialize phase after variable resolution. Required fields without omitempty (e.g. spark_version) are kept, and apps.description is exempt to match terraform's existing behavior. The result is engine-agnostic and reflected in bundle validate -o json.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

13 files changed
Suggested: @pietern
Also eligible: @janniklasrose, @andrewnester, @shreyas-goenka, @lennartkats-db, @anton-107

/bundle/ - needs approval

Files: bundle/config/mutator/drop_empty_strings.go, bundle/phases/initialize.go
Suggested: @pietern
Also eligible: @janniklasrose, @andrewnester, @shreyas-goenka, @lennartkats-db, @anton-107

General files (require maintainer)

Files: .nextchanges/bundles/drop-empty-strings.md, libs/dyn/convert/normalize.go, libs/dyn/convert/normalize_test.go
Based on git history:

  • @pietern -- recent work in bundle/config/mutator/, libs/dyn/convert/, .nextchanges/bundles/

Any maintainer (@andrewnester, @anton-107, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db, @janniklasrose, @lennartkats-db) can approve all areas.
See OWNERS for ownership rules.

denik added 5 commits July 28, 2026 18:44
An optional (omitempty) resource field set to "" -- often via a variable that
resolved to an empty string, e.g. policy_id: ${var.x} -- was force-sent to the
backend and rejected ("'' is not a valid cluster policy ID"). A new
DropEmptyStrings normalize option removes such values, applied by a
DropEmptyStrings mutator in the initialize phase after variable resolution.

Fields without omitempty (required, e.g. spark_version) are kept. The fix is
engine-agnostic and reflected in bundle validate -o json.

Co-authored-by: Isaac
Apps API always returns "" for an unset description, so tfdyn/convert_app.go
injects it to avoid terraform drift. Exempt apps.description from DropEmptyStrings
so the direct and terraform engines send the same create request.

Co-authored-by: Isaac
Co-authored-by: Isaac
The script passes leading-slash path filters to print_requests.py; Git Bash on
Windows rewrites them into Windows paths, so the filter is rejected. Set
MSYS_NO_PATHCONV=1 for this test.

Co-authored-by: Isaac
The fix drops empty omitempty strings, so the direct-only gap closes to empty
and those fields move to dropped-by-both. apps.description stays sent by both.

Co-authored-by: Isaac
@denik
denik force-pushed the denik/empty-string-cleanup branch from 4a35603 to f679d8f Compare July 28, 2026 16:47
MSYS_NO_PATHCONV=1 broke print_requests.py itself on Windows (its /c/... script
path became unopenable). The script already uses double-slash filters
(^//api/2.0/workspace), which is the correct Windows-safe form, so the env
override was both unnecessary and harmful.

Co-authored-by: Isaac
@eng-dev-ecosystem-bot

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: 9b8d8f8

Run: 30381533504

Env ❌​FAIL 🟨​KNOWN 🔄​flaky 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
❌​ aws linux 3 1 2 3 4 317 1064 6:33
❌​ aws windows 3 1 3 4 321 1062 9:46
❌​ azure linux 3 1 3 4 319 1063 6:09
❌​ azure windows 3 1 3 4 321 1061 6:29
❌​ gcp linux 3 1 5 318 1065 6:02
❌​ gcp windows 3 1 5 320 1063 6:29
13 interesting tests: 4 SKIP, 3 FAIL, 3 RECOVERED, 2 flaky, 1 KNOWN
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
🟨​ TestAccept 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K 🟨​K
❌​ TestAccept/bundle/deploy/mlops-stacks ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=direct ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
❌​ TestAccept/bundle/deploy/mlops-stacks/DATABRICKS_BUNDLE_ENGINE=terraform ❌​F ❌​F ❌​F ❌​F ❌​F ❌​F
🙈​ TestAccept/bundle/invariant/no_drift 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_endpoints/drift/recreated_same_name 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/vector_search_indexes/recreate/embedding_dimension 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🔄​ TestFsCpFileToNonExistentDir 🔄​f ✅​p ✅​p ✅​p ✅​p ✅​p
🔄​ TestFsCpFileToNonExistentDir/uc-volumes_to_dbfs 🔄​f ✅​p ✅​p ✅​p ✅​p ✅​p
💚​ TestFetchRepositoryInfoAPI_FromRepo 💚​R 💚​R 💚​R 💚​R 🙈​S 🙈​S
💚​ TestFetchRepositoryInfoAPI_FromRepo/root 💚​R 💚​R 💚​R 💚​R
💚​ TestFetchRepositoryInfoAPI_FromRepo/subdir 💚​R 💚​R 💚​R 💚​R

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants