From aa83eb76e0e8ce777290c97f6c00108f76f5653d Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 24 Apr 2024 15:19:35 +0200 Subject: [PATCH 1/2] Fix bundle schema for variables --- bundle/config/mutator/set_variables.go | 2 - bundle/schema/schema.go | 54 ++++++++++++++++ libs/jsonschema/schema.go | 38 ++++++++++- libs/jsonschema/schema_test.go | 90 ++++++++++++++++++++++++++ 4 files changed, 181 insertions(+), 3 deletions(-) diff --git a/bundle/config/mutator/set_variables.go b/bundle/config/mutator/set_variables.go index bb88379e0f6..eae1fe2ab72 100644 --- a/bundle/config/mutator/set_variables.go +++ b/bundle/config/mutator/set_variables.go @@ -53,8 +53,6 @@ func setVariable(ctx context.Context, v *variable.Variable, name string) diag.Di } // We should have had a value to set for the variable at this point. - // TODO: use cmdio to request values for unassigned variables if current - // terminal is a tty. Tracked in https://github.com/databricks/cli/issues/379 return diag.Errorf(`no value assigned to required variable %s. Assignment can be done through the "--var" flag or by setting the %s environment variable`, name, bundleVarPrefix+name) } diff --git a/bundle/schema/schema.go b/bundle/schema/schema.go index b37f72d9b68..59a1ded5974 100644 --- a/bundle/schema/schema.go +++ b/bundle/schema/schema.go @@ -48,9 +48,63 @@ func New(golangType reflect.Type, docs *Docs) (*jsonschema.Schema, error) { if err != nil { return nil, tracker.errWithTrace(err.Error(), "root") } + + err = overrideVariables(schema) + if err != nil { + return nil, err + } + return schema, nil } +func overrideVariables(s *jsonschema.Schema) error { + // Override schema for default values to allow for multiple primitive types. + // These are normalized to strings when converted to the typed representation. + err := s.SetByPath("variables.*.default", jsonschema.Schema{ + AnyOf: []*jsonschema.Schema{ + { + Type: jsonschema.StringType, + }, + { + Type: jsonschema.BooleanType, + }, + { + Type: jsonschema.NumberType, + }, + { + Type: jsonschema.IntegerType, + }, + }, + }) + if err != nil { + return err + } + + // Override schema for variables in targets to allow just specifying the value + // along side overriding the variable definition if needed. + ns, err := s.GetByPath("variables.*") + if err != nil { + return err + } + return s.SetByPath("targets.*.variables.*", jsonschema.Schema{ + AnyOf: []*jsonschema.Schema{ + { + Type: jsonschema.StringType, + }, + { + Type: jsonschema.BooleanType, + }, + { + Type: jsonschema.NumberType, + }, + { + Type: jsonschema.IntegerType, + }, + &ns, + }, + }) +} + func jsonSchemaType(golangType reflect.Type) (jsonschema.Type, error) { switch golangType.Kind() { case reflect.Bool: diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 967e2e9cdc5..f1e223ec73e 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -6,6 +6,7 @@ import ( "os" "regexp" "slices" + "strings" "github.com/databricks/cli/internal/build" "golang.org/x/mod/semver" @@ -81,6 +82,41 @@ func (s *Schema) ParseString(v string) (any, error) { return fromString(v, s.Type) } +func (s *Schema) getByPath(path string) (*Schema, error) { + p := strings.Split(path, ".") + + res := s + for _, node := range p { + if node == "*" { + res = res.AdditionalProperties.(*Schema) + continue + } + var ok bool + res, ok = res.Properties[node] + if !ok { + return nil, fmt.Errorf("property %q not found in schema. Query path: %s", node, path) + } + } + return res, nil +} + +func (s *Schema) GetByPath(path string) (Schema, error) { + v, err := s.getByPath(path) + if err != nil { + return Schema{}, err + } + return *v, nil +} + +func (s *Schema) SetByPath(path string, v Schema) error { + dst, err := s.getByPath(path) + if err != nil { + return err + } + *dst = v + return nil +} + type Type string const ( @@ -97,7 +133,7 @@ const ( func (schema *Schema) validateSchemaPropertyTypes() error { for _, v := range schema.Properties { switch v.Type { - case NumberType, BooleanType, StringType, IntegerType: + case NumberType, BooleanType, StringType, IntegerType, ObjectType, ArrayType: continue case "int", "int32", "int64": return fmt.Errorf("type %s is not a recognized json schema type. Please use \"integer\" instead", v.Type) diff --git a/libs/jsonschema/schema_test.go b/libs/jsonschema/schema_test.go index cf1f1276720..c365cf23521 100644 --- a/libs/jsonschema/schema_test.go +++ b/libs/jsonschema/schema_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSchemaValidateTypeNames(t *testing.T) { @@ -305,3 +306,92 @@ func TestValidateSchemaSkippedPropertiesHaveDefaults(t *testing.T) { err = s.validate() assert.NoError(t, err) } + +func testSchema() *Schema { + return &Schema{ + Type: "object", + Properties: map[string]*Schema{ + "int_val": { + Type: "integer", + Default: int64(123), + }, + "string_val": { + Type: "string", + }, + "object_val": { + Type: "object", + Properties: map[string]*Schema{ + "bar": { + Type: "string", + Default: "baz", + }, + }, + AdditionalProperties: &Schema{ + Type: "object", + Properties: map[string]*Schema{ + "foo": { + Type: "string", + Default: "zab", + }, + }, + }, + }, + }, + } + +} + +func TestSchemaGetByPath(t *testing.T) { + s := testSchema() + + ss, err := s.GetByPath("int_val") + require.NoError(t, err) + assert.Equal(t, Schema{ + Type: IntegerType, + Default: int64(123), + }, ss) + + ss, err = s.GetByPath("string_val") + require.NoError(t, err) + assert.Equal(t, Schema{ + Type: StringType, + }, ss) + + ss, err = s.GetByPath("object_val.bar") + require.NoError(t, err) + assert.Equal(t, Schema{ + Type: StringType, + Default: "baz", + }, ss) + + ss, err = s.GetByPath("object_val.*.foo") + require.NoError(t, err) + assert.Equal(t, Schema{ + Type: StringType, + Default: "zab", + }, ss) +} + +func TestSchemaSetByPath(t *testing.T) { + s := testSchema() + + err := s.SetByPath("int_val", Schema{ + Type: IntegerType, + Default: int64(456), + }) + require.NoError(t, err) + assert.Equal(t, int64(456), s.Properties["int_val"].Default) + + err = s.SetByPath("object_val.*.foo", Schema{ + Type: StringType, + Default: "zooby", + }) + require.NoError(t, err) + + ns, err := s.GetByPath("object_val.*.foo") + require.NoError(t, err) + assert.Equal(t, Schema{ + Type: StringType, + Default: "zooby", + }, ns) +} From 81a298743719d9010f4c3a94fd2c583a58cdcf12 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 24 Apr 2024 15:37:04 +0200 Subject: [PATCH 2/2] fix tests --- bundle/schema/schema.go | 54 --------------------------------------- cmd/bundle/schema.go | 56 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 54 deletions(-) diff --git a/bundle/schema/schema.go b/bundle/schema/schema.go index 59a1ded5974..b37f72d9b68 100644 --- a/bundle/schema/schema.go +++ b/bundle/schema/schema.go @@ -48,63 +48,9 @@ func New(golangType reflect.Type, docs *Docs) (*jsonschema.Schema, error) { if err != nil { return nil, tracker.errWithTrace(err.Error(), "root") } - - err = overrideVariables(schema) - if err != nil { - return nil, err - } - return schema, nil } -func overrideVariables(s *jsonschema.Schema) error { - // Override schema for default values to allow for multiple primitive types. - // These are normalized to strings when converted to the typed representation. - err := s.SetByPath("variables.*.default", jsonschema.Schema{ - AnyOf: []*jsonschema.Schema{ - { - Type: jsonschema.StringType, - }, - { - Type: jsonschema.BooleanType, - }, - { - Type: jsonschema.NumberType, - }, - { - Type: jsonschema.IntegerType, - }, - }, - }) - if err != nil { - return err - } - - // Override schema for variables in targets to allow just specifying the value - // along side overriding the variable definition if needed. - ns, err := s.GetByPath("variables.*") - if err != nil { - return err - } - return s.SetByPath("targets.*.variables.*", jsonschema.Schema{ - AnyOf: []*jsonschema.Schema{ - { - Type: jsonschema.StringType, - }, - { - Type: jsonschema.BooleanType, - }, - { - Type: jsonschema.NumberType, - }, - { - Type: jsonschema.IntegerType, - }, - &ns, - }, - }) -} - func jsonSchemaType(golangType reflect.Type) (jsonschema.Type, error) { switch golangType.Kind() { case reflect.Bool: diff --git a/cmd/bundle/schema.go b/cmd/bundle/schema.go index 0f27142bd56..b0d6b3dd527 100644 --- a/cmd/bundle/schema.go +++ b/cmd/bundle/schema.go @@ -7,9 +7,58 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/schema" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/jsonschema" "github.com/spf13/cobra" ) +func overrideVariables(s *jsonschema.Schema) error { + // Override schema for default values to allow for multiple primitive types. + // These are normalized to strings when converted to the typed representation. + err := s.SetByPath("variables.*.default", jsonschema.Schema{ + AnyOf: []*jsonschema.Schema{ + { + Type: jsonschema.StringType, + }, + { + Type: jsonschema.BooleanType, + }, + { + Type: jsonschema.NumberType, + }, + { + Type: jsonschema.IntegerType, + }, + }, + }) + if err != nil { + return err + } + + // Override schema for variables in targets to allow just specifying the value + // along side overriding the variable definition if needed. + ns, err := s.GetByPath("variables.*") + if err != nil { + return err + } + return s.SetByPath("targets.*.variables.*", jsonschema.Schema{ + AnyOf: []*jsonschema.Schema{ + { + Type: jsonschema.StringType, + }, + { + Type: jsonschema.BooleanType, + }, + { + Type: jsonschema.NumberType, + }, + { + Type: jsonschema.IntegerType, + }, + &ns, + }, + }) +} + func newSchemaCommand() *cobra.Command { cmd := &cobra.Command{ Use: "schema", @@ -30,6 +79,13 @@ func newSchemaCommand() *cobra.Command { return err } + // Override schema for variables to take into account normalization of default + // variable values and variable overrides in a target. + err = overrideVariables(schema) + if err != nil { + return err + } + // Print the JSON schema to stdout. result, err := json.MarshalIndent(schema, "", " ") if err != nil {