diff --git a/.nextchanges/bundles/sdk-native-type-errors.md b/.nextchanges/bundles/sdk-native-type-errors.md new file mode 100644 index 00000000000..0d7c35a7938 --- /dev/null +++ b/.nextchanges/bundles/sdk-native-type-errors.md @@ -0,0 +1 @@ +* Report malformed timestamp and duration values in bundle configuration with their path and location instead of a bare protobuf error, and fix a crash when the value comes from a variable. ([#6945](https://github.com/databricks/cli/pull/6945)) diff --git a/acceptance/bundle/validate/sdk_native_types/databricks.yml b/acceptance/bundle/validate/sdk_native_types/databricks.yml new file mode 100644 index 00000000000..80365c9248c --- /dev/null +++ b/acceptance/bundle/validate/sdk_native_types/databricks.yml @@ -0,0 +1,23 @@ +bundle: + name: sdk-native-types + +resources: + postgres_projects: + my_project: + project_id: test-pg-proj + display_name: Test + pg_version: 16 + + postgres_branches: + bad_expire_time: + parent: ${resources.postgres_projects.my_project.id} + branch_id: bad-expire-time + expire_time: "2030-01-01" + bad_ttl: + parent: ${resources.postgres_projects.my_project.id} + branch_id: bad-ttl + ttl: 1h + valid: + parent: ${resources.postgres_projects.my_project.id} + branch_id: valid + expire_time: 2030-01-01T00:00:00Z diff --git a/acceptance/bundle/validate/sdk_native_types/out.test.toml b/acceptance/bundle/validate/sdk_native_types/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/validate/sdk_native_types/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/validate/sdk_native_types/output.txt b/acceptance/bundle/validate/sdk_native_types/output.txt new file mode 100644 index 00000000000..afe8e8c0565 --- /dev/null +++ b/acceptance/bundle/validate/sdk_native_types/output.txt @@ -0,0 +1,32 @@ + +>>> [CLI] bundle validate +Error: cannot parse "2030-01-01" as an RFC 3339 timestamp (e.g. "[TIMESTAMP]") + at resources.postgres_branches.bad_expire_time.expire_time + in databricks.yml:15:20 + +Error: cannot parse "1h" as a duration in seconds (e.g. "300s") + at resources.postgres_branches.bad_ttl.ttl + in databricks.yml:19:12 + + +Found 2 errors + +Exit code: 1 + +>>> [CLI] bundle validate +Error: cannot parse "2030-01-01" as an RFC 3339 timestamp (e.g. "[TIMESTAMP]") + at resources.postgres_branches.dev.expire_time + in databricks.yml:19:20 + +Name: sdk-native-types-var-ref +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/sdk-native-types-var-ref/default + +Found 1 error + +Exit code: 1 + +>>> [CLI] bundle validate --var expire_time=[TIMESTAMP] -o json +"[TIMESTAMP]" diff --git a/acceptance/bundle/validate/sdk_native_types/script b/acceptance/bundle/validate/sdk_native_types/script new file mode 100644 index 00000000000..afce4eea01c --- /dev/null +++ b/acceptance/bundle/validate/sdk_native_types/script @@ -0,0 +1,5 @@ +errcode trace $CLI bundle validate + +cd var_ref +errcode trace $CLI bundle validate +trace $CLI bundle validate --var expire_time=2030-01-01T00:00:00Z -o json | jq .resources.postgres_branches.dev.expire_time diff --git a/acceptance/bundle/validate/sdk_native_types/test.toml b/acceptance/bundle/validate/sdk_native_types/test.toml new file mode 100644 index 00000000000..e8cdb964df8 --- /dev/null +++ b/acceptance/bundle/validate/sdk_native_types/test.toml @@ -0,0 +1 @@ +Ignore = ["var_ref/.databricks"] diff --git a/acceptance/bundle/validate/sdk_native_types/var_ref/databricks.yml b/acceptance/bundle/validate/sdk_native_types/var_ref/databricks.yml new file mode 100644 index 00000000000..7a383949ebd --- /dev/null +++ b/acceptance/bundle/validate/sdk_native_types/var_ref/databricks.yml @@ -0,0 +1,19 @@ +bundle: + name: sdk-native-types-var-ref + +variables: + expire_time: + default: "2030-01-01" + +resources: + postgres_projects: + my_project: + project_id: test-pg-proj + display_name: Test + pg_version: 16 + + postgres_branches: + dev: + parent: ${resources.postgres_projects.my_project.id} + branch_id: dev + expire_time: ${var.expire_time} diff --git a/libs/dyn/convert/from_typed.go b/libs/dyn/convert/from_typed.go index 66451124293..b36e77e1a1b 100644 --- a/libs/dyn/convert/from_typed.go +++ b/libs/dyn/convert/from_typed.go @@ -60,7 +60,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, switch srcv.Kind() { case reflect.Struct: // Handle SDK native types using JSON marshaling. - if slices.Contains(sdkNativeTypes, srcv.Type()) { + if isSDKNativeType(srcv.Type()) { v, err = fromTypedSDKNative(srcv, ref, options...) } else { v, err = fromTypedStruct(srcv, ref, options...) diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index 5f60af9474a..0fecf04b6b4 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -55,8 +55,8 @@ func (n normalizeOptions) normalizeType(typ reflect.Type, src dyn.Value, seen [] switch typ.Kind() { case reflect.Struct: // Handle SDK native types as strings since they use custom JSON marshaling. - if slices.Contains(sdkNativeTypes, typ) { - return n.normalizeString(reflect.TypeFor[string](), src, path) + if isSDKNativeType(typ) { + return n.normalizeSDKNative(typ, src, path) } return n.normalizeStruct(typ, src, append(seen, typ), path) case reflect.Map: diff --git a/libs/dyn/convert/sdk_native_types.go b/libs/dyn/convert/sdk_native_types.go index b67d9b2b1e3..cd83db2f4ee 100644 --- a/libs/dyn/convert/sdk_native_types.go +++ b/libs/dyn/convert/sdk_native_types.go @@ -6,6 +6,7 @@ import ( "reflect" "slices" + "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/dyn/dynvar" sdkduration "github.com/databricks/databricks-sdk-go/common/types/duration" @@ -13,13 +14,19 @@ import ( sdktime "github.com/databricks/databricks-sdk-go/common/types/time" ) -// sdkNativeTypes is a list of SDK native types that use custom JSON marshaling -// and should be treated as strings in dyn.Value. These types all implement -// json.Marshaler and json.Unmarshaler interfaces. -var sdkNativeTypes = []reflect.Type{ - reflect.TypeFor[sdkduration.Duration](), // Protobuf duration format (e.g., "300s") - reflect.TypeFor[sdktime.Time](), // RFC3339 timestamp format (e.g., "2023-12-25T10:30:00Z") - reflect.TypeFor[sdkfieldmask.FieldMask](), // Comma-separated paths (e.g., "name,age,email") +// sdkNativeTypes maps SDK native types that use custom JSON marshaling and +// should be treated as strings in dyn.Value to a description of their string +// format. These types all implement json.Marshaler and json.Unmarshaler interfaces. +var sdkNativeTypes = map[reflect.Type]string{ + reflect.TypeFor[sdkduration.Duration](): `a duration in seconds (e.g. "300s")`, + reflect.TypeFor[sdktime.Time](): `an RFC 3339 timestamp (e.g. "2023-12-25T10:30:00Z")`, + reflect.TypeFor[sdkfieldmask.FieldMask](): `a comma-separated list of field paths (e.g. "name,age,email")`, +} + +// isSDKNativeType reports whether typ is one of [sdkNativeTypes]. +func isSDKNativeType(typ reflect.Type) bool { + _, ok := sdkNativeTypes[typ] + return ok } // fromTypedSDKNative converts SDK native types to dyn.Value. @@ -65,6 +72,26 @@ func fromTypedSDKNative(src reflect.Value, ref dyn.Value, options ...fromTypedOp return dyn.V(str), nil } +// normalizeSDKNative normalizes an SDK native type as a string and checks that it +// parses, so a malformed value is reported with its location instead of failing +// later in [ToTyped]. +func (n normalizeOptions) normalizeSDKNative(typ reflect.Type, src dyn.Value, path dyn.Path) (dyn.Value, diag.Diagnostics) { + v, diags := n.normalizeString(reflect.TypeFor[string](), src, path) + if !v.IsValid() || dynvar.IsPureVariableReference(v.MustString()) { + return v, diags + } + + if err := toTypedSDKNative(reflect.New(typ).Elem(), v); err != nil { + return dyn.InvalidValue, diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("cannot parse %q as %s", v.MustString(), sdkNativeTypes[typ]), + Locations: []dyn.Location{src.Location()}, + Paths: []dyn.Path{path}, + }) + } + return v, diags +} + // toTypedSDKNative converts a dyn.Value to an SDK native type. // SDK native types (duration.Duration, time.Time, fieldmask.FieldMask) use // custom JSON marshaling with string representations. diff --git a/libs/dyn/convert/to_typed.go b/libs/dyn/convert/to_typed.go index 41fab3d7323..669b03b74bd 100644 --- a/libs/dyn/convert/to_typed.go +++ b/libs/dyn/convert/to_typed.go @@ -3,7 +3,6 @@ package convert import ( "fmt" "reflect" - "slices" "strconv" "github.com/databricks/cli/libs/dyn" @@ -41,7 +40,7 @@ func ToTyped(dst any, src dyn.Value) error { switch dstv.Kind() { case reflect.Struct: // Handle SDK native types using JSON unmarshaling. - if slices.Contains(sdkNativeTypes, dstv.Type()) { + if isSDKNativeType(dstv.Type()) { return toTypedSDKNative(dstv, src) } return toTypedStruct(dstv, src)