Validate required/unit-constrained UC fields early in bundle validate#5818
Merged
radakam merged 15 commits intoJul 16, 2026
Merged
Conversation
Collaborator
Integration test reportCommit: 1ed565c
10 interesting tests: 4 SKIP, 4 flaky, 2 RECOVERED
Top 9 slowest tests (at least 2 minutes):
|
cfbbb09 to
998cbdf
Compare
998cbdf to
6eb36c8
Compare
6eb36c8 to
81814b5
Compare
Reject missing sql_warehouse name, missing grant principal, and out-of-range catalog/schema custom_max_retention_hours at validate/plan time instead of letting them pass and fail later at deploy with low-context backend errors. DECO-27550
The retention range (0 or 168-720 hours) is a backend policy value that could change server-side; hardcoding it in the CLI risks rejecting configs the backend would accept. Keep only the stable presence checks (sql_warehouse name, grant principal).
d10fb7a to
556d8e5
Compare
Iterate b.Config.Resources.SqlWarehouses directly instead of dyn, matching the adjacent dashboards validation. Addresses review feedback.
Iterate the six grant-bearing resource types directly instead of dyn, and drop the now-unused isMissingOrEmptyString helper. Addresses review feedback.
shreyas-goenka
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me. Just one minor comment.
Revert the static per-resource-type grant principal validation back to the dyn-based walk over resources.*.*.grants[*], so any resource with a grants field is validated without enumerating types. Rename warnForMissingBackendFields to warnForMissingGrantPrincipals for clarity.
shreyas-goenka
approved these changes
Jul 16, 2026
Collaborator
Integration test reportCommit: c8c397f
114 interesting tests: 100 FAIL, 6 flaky, 4 RECOVERED, 2 KNOWN, 2 SKIP
Top 50 slowest tests (at least 2 minutes):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
bundle validate(and thereforeplan/deploy) now reports two UC fields that the SDK models as optional but the backend treats differently:sql_warehouses.*.name— errorsql_warehouse name is required(backend-confirmed as required; whitespace-only names are also rejected, matching the backend'sname.trim.nonEmpty).grants[*].principal— warninggrant principal is required, on every securable that supports grants (catalogs, schemas, volumes, external_locations, registered_models, vector_search_indexes). Kept as a warning since the backend contract is unconfirmed.Implemented in the existing
validate:requiredmutator (runs inphases.Initialize(), covering validate/plan/deploy), following the existing dashboard bespoke-validation precedent. Validation diagnostics are sorted deterministically so multiple messages have stable ordering.Why
These fields are modeled as loosely-typed/optional (
json:"...,omitempty"), sobundle validateandbundle planpass while the deploy is rejected by the backend with late, low-context 400s. Discovered via fuzz testing; affects both the Terraform and direct engines. Reporting the offending field by name at validate time is much more actionable.Tests
acceptance/bundle/validate/:sql_warehouse_required_name/(both engines; covers a missing name and a whitespace-only name)grants_required_principal/(verifies a missing principal warns while a valid grant passes)empty_resourcesgolden files that surface thesql_warehouse name is requirederror.