Skip to content

Reject empty and control-character identifiers during bundle validate - #6305

Open
radakam wants to merge 8 commits into
mainfrom
fix-illegal-empty-identifiers
Open

Reject empty and control-character identifiers during bundle validate#6305
radakam wants to merge 8 commits into
mainfrom
fix-illegal-empty-identifiers

Conversation

@radakam

@radakam radakam commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Changes

Reject empty, blank, and control-character resource identifiers (and incomplete pipeline library paths) during bundle validate, so these configs fail at validate instead of later with API 400 / invalid-URL errors.

  • Identifier checks walk typed resource fields (an explicit allowlist), separate from OpenAPI required-field warnings.
  • Missing resource names that previously only warned (e.g. models, apps) now error.
  • For UC parent fields (catalog_name, schema_name, ...): blank and control-character values error. Omitted parents are left to the existing OpenAPI required-field warnings, which still run only when validate has no errors.
  • Incomplete pipeline file / notebook / glob entries without paths are rejected.
  • Diagnostics use structural dyn.NewPath (no panic on resource keys with path metacharacters) and name the offending control character in the detail.

Why

These configs used to pass databricks bundle validate and only fail later at the API.

Before this fix (databricks bundle validate on a bundle with empty names, control-character names, and file: {}):

Name: invalid-identifiers
Target: default
Workspace:
  User: [USERNAME]
  Path: /Workspace/Users/[USERNAME]/.bundle/invalid-identifiers/default

Validation OK!

Explicit "" on non-omitempty parents (e.g. volumes) also errors. On omitempty parents (e.g. registered models), DropEmptyStrings treats "" as unset before validate.

Found by fuzz testing.

Tests

  • Unit: invalid_identifiers_test.go, pipeline_libraries_test.go
  • Acceptance: invalid_identifiers, required, empty_resources, dashboard/sql_warehouse required-name cases, volume_defaults, models validate

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Integration test report

Commit: c74f179

Run: 33167520926

Env 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
💚​ aws linux 1 1 274 1204 4:50
💚​ aws windows 1 1 276 1202 3:48
💚​ azure linux 1 1 273 1204 4:36
💚​ azure windows 1 1 275 1202 4:16
💚​ gcp linux 1 1 274 1204 4:46
💚​ gcp windows 1 1 276 1202 4:37
Test Name aws linux aws windows azure linux azure windows gcp linux gcp windows
💚​ TestAccept 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/ssh/connection 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
Top 3 slowest tests (at least 2 minutes):
duration env testname
4:32 gcp windows TestAccept
3:43 aws windows TestAccept
3:27 azure windows TestAccept

@radakam
radakam marked this pull request as ready for review August 19, 2026 08:56
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Approval status: pending

/acceptance/bundle/ - needs approval

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

/bundle/ - needs approval

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

General files (require maintainer)

Files: .nextchanges/bundles/invalid-identifiers.md
Based on git history:

  • @denik -- recent work in bundle/config/validate/, .nextchanges/bundles/, acceptance/bundle/validate/models/user_id/

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

@radakam radakam changed the title Reject empty/control-char names and incomplete pipeline libraries at … Reject empty and control-character identifiers during bundle validate Aug 19, 2026
@radakam
radakam requested a review from denik August 21, 2026 11:07
…validate

validate --strict previously accepted empty model names, UC/serving names with
control characters, and pipeline file: {}. Those fail later with API 400 or
invalid-URL errors; fail during initialize instead.
Keep omitted UC parent fields as warnings, point diagnostics at the field path, tighten tests, and drop duplicate empty-name acceptance coverage.
Build diagnostic paths structurally with dyn.NewPath instead of parsing
concatenated strings: a resource key containing a path metacharacter (e.g.
"weird[0]key") made MustPathFromString panic during bundle validate.

Drive the identifier checks off generated.RequiredFields rather than a
hand-picked list of resource types, so control characters and blank values are
caught consistently (vector search endpoints, apps, experiments and others were
previously silent). Explicit empty strings on UC parent fields are now rejected
too; omitted parents keep warning.

Stop short-circuiting before warnForMissingFields so one validate run reports
every issue, and name the offending character in the diagnostic detail since
control characters are invisible in terminal output.
Collapse OpenAPI warnings, backend-only errors, and blank/control-char
checks into a compiled rule set so nested fields keep accurate messages
and validate stays a single config walk.
Split OpenAPI required-field warnings from identifier checks and drop the
heuristic rule engine so validation policy stays readable and intentional.
Comment thread bundle/config/validate/pipeline_libraries.go
Comment thread bundle/config/validate/invalid_identifiers.go Outdated
GetLocations parses a string, so resource keys with '[' or '.' lose their YAML source location.
@radakam
radakam requested a review from denik August 27, 2026 14:05
Comment thread bundle/config/validate/required.go Outdated
Keep the original warn path, HasError early return, and warehouse_id check; only use structural paths where GetLocations panics.
@radakam
radakam requested a review from denik August 28, 2026 11:35
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.

3 participants