Skip to content

Commit

Permalink
Fix schema merging on additionalProperties
Browse files Browse the repository at this point in the history
  • Loading branch information
MacroPower committed Jan 3, 2025
1 parent dfd5764 commit 193e6aa
Show file tree
Hide file tree
Showing 6 changed files with 247 additions and 14 deletions.
4 changes: 4 additions & 0 deletions pkg/jsonschema/gen_auto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func TestAutoGenerator(t *testing.T) {
filePaths: []string{"input/values.yaml", "input/schema.json"},
expectedPath: "output/schema.json",
},
"DeepSchema": {
filePaths: []string{"input/values.yaml", "input/deep.schema.json"},
expectedPath: "output/deep.schema.json",
},
}

for name, tc := range testCases {
Expand Down
15 changes: 9 additions & 6 deletions pkg/jsonschema/gen_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,25 +120,28 @@ func (g *ReaderGenerator) FromData(data []byte, refBasePath string) ([]byte, err
return nil, fmt.Errorf("failed to unmarshal JSON Schema: %w", err)
}

// Remove the ID to keep downstream KCL schema generation consistent.
hs.Id = ""

if err := hs.Validate(); err != nil {
return nil, fmt.Errorf("invalid schema: %w", err)
}

// Remove the ID to keep downstream KCL schema generation consistent.
hs.Id = ""

if err := handleSchemaRefs(hs, refBasePath); err != nil {
return nil, fmt.Errorf("failed to handle schema refs: %w", err)
}

if err := hs.Validate(); err != nil {
mhs := &helmschema.Schema{}
mhs = mergeHelmSchemas(mhs, hs, true)

if err := mhs.Validate(); err != nil {
return nil, fmt.Errorf("invalid schema: %w", err)
}
if len(hs.Properties) == 0 {
if len(mhs.Properties) == 0 {
return nil, errors.New("empty schema")
}

resolvedData, err := hs.ToJson()
resolvedData, err := mhs.ToJson()
if err != nil {
return nil, fmt.Errorf("failed to convert schema to JSON: %w", err)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/jsonschema/gen_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ func TestReaderGenerator(t *testing.T) {
filePaths: []string{"input/refs.schema.json"},
expectedPath: "output/schema.json",
},
"DeepSchema": {
filePaths: []string{"input/deep.schema.json"},
expectedPath: "output/deep.schema.json",
},
}

for name, tc := range testCases {
Expand Down
79 changes: 71 additions & 8 deletions pkg/jsonschema/gen_value_inference.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ package jsonschema

import (
"bytes"
"encoding/json"
"errors"
"fmt"
"os"
"regexp"
"slices"
"strings"

helmschema "github.com/dadav/helm-schema/pkg/schema"
Expand Down Expand Up @@ -301,15 +303,17 @@ func mergeHelmSchemas(dest, src *helmschema.Schema, setDefaults bool) *helmschem
if src.ReadOnly {
dest.ReadOnly = src.ReadOnly
}
if src.AdditionalProperties != nil {
dest.AdditionalProperties = src.AdditionalProperties
}
if src.Id != "" {
dest.Id = src.Id
}

// Merge 'enum' field (assuming that maintaining order doesn't matter)
dest.Enum = append(dest.Enum, src.Enum...)
dest.Enum = slices.Compact(append(dest.Enum, src.Enum...))

dest.Required = helmschema.BoolOrArrayOfString{
Bool: dest.Required.Bool || src.Required.Bool,
Strings: intersectStringSlices(dest.Required.Strings, src.Required.Strings),
}

// Recursive calls for nested structures
if src.Properties != nil {
Expand All @@ -325,6 +329,13 @@ func mergeHelmSchemas(dest, src *helmschema.Schema, setDefaults bool) *helmschem
}
}

if src.AdditionalProperties != nil {
err := mergeSchemaAdditionalProperties(dest, src, setDefaults)
if err != nil {
dest.AdditionalProperties = true
}
}

// Merge 'items' if they exist (assuming they're not arrays)
if src.Items != nil {
dest.Items = mergeHelmSchemas(dest.Items, src.Items, setDefaults)
Expand All @@ -348,17 +359,69 @@ func mergeHelmSchemas(dest, src *helmschema.Schema, setDefaults bool) *helmschem
}

if src.If != nil {
dest.If = mergeHelmSchemas(dest.If, src.If, setDefaults)
dest = mergeHelmSchemas(dest, src.If, setDefaults)
}
if src.Else != nil {
dest.Else = mergeHelmSchemas(dest.Else, src.Else, setDefaults)
dest = mergeHelmSchemas(dest, src.Else, setDefaults)
}
if src.Then != nil {
dest.Then = mergeHelmSchemas(dest.Then, src.Then, setDefaults)
dest = mergeHelmSchemas(dest, src.Then, setDefaults)
}
if src.Not != nil {
dest.Not = mergeHelmSchemas(dest.Not, src.Not, setDefaults)
dest = mergeHelmSchemas(dest, src.Not, setDefaults)
}

return dest
}

func intersectStringSlices(a, b []string) []string {
intersection := []string{}
for _, x := range a {
if slices.Contains(b, x) {
intersection = append(intersection, x)
}
}
return intersection
}

func mergeSchemaAdditionalProperties(dest, src *helmschema.Schema, setDefaults bool) error {
if src.AdditionalProperties == true || src.AdditionalProperties == false {
dest.AdditionalProperties = src.AdditionalProperties
return nil
}

srcData, err := json.Marshal(src.AdditionalProperties)
if err != nil {
return err //nolint:wrapcheck
}
destData, err := json.Marshal(dest.AdditionalProperties)
if err != nil {
return err //nolint:wrapcheck
}

srcSubSchema := &helmschema.Schema{}
var jsonSrcNode yaml.Node
if err := yaml.Unmarshal(srcData, &jsonSrcNode); err != nil {
return err //nolint:wrapcheck
}
if err := srcSubSchema.UnmarshalYAML(&jsonSrcNode); err != nil {
return err //nolint:wrapcheck
}
destSubSchema := &helmschema.Schema{}
var jsonDestNode yaml.Node
if err := yaml.Unmarshal(destData, &jsonDestNode); err != nil {
return err //nolint:wrapcheck
}
if err := destSubSchema.UnmarshalYAML(&jsonDestNode); err != nil {
return err //nolint:wrapcheck
}

subSchema := mergeHelmSchemas(destSubSchema, srcSubSchema, setDefaults)
if err := subSchema.Validate(); err != nil {
return err //nolint:wrapcheck
}

dest.AdditionalProperties = subSchema

return nil
}
85 changes: 85 additions & 0 deletions pkg/jsonschema/testdata/input/deep.schema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
{
"$schema": "http://json-schema.org/draft-07/schema",
"properties": {
"configMaps": {
"additionalProperties": {
"additionalProperties": false,
"oneOf": [
{
"required": [
"data"
]
},
{
"required": [
"binaryData"
]
}
],
"properties": {
"annotations": {
"additionalProperties": {
"required": [],
"type": [
"string",
"null"
]
},
"required": [],
"type": [
"object",
"null"
]
},
"binaryData": {
"additionalProperties": {
"required": [],
"type": "string"
},
"required": [],
"type": "object"
},
"data": {
"additionalProperties": {
"required": [],
"type": "string"
},
"required": [],
"type": "object"
},
"enabled": {
"default": true,
"required": [],
"type": "boolean"
},
"includeInChecksum": {
"default": true,
"required": [],
"type": "boolean"
},
"labels": {
"additionalProperties": {
"required": [],
"type": [
"string",
"null"
]
},
"required": [],
"type": [
"object",
"null"
]
},
"nameOverride": {
"required": [],
"type": "string"
}
},
"required": [],
"type": "object"
},
"required": []
}
}
}
74 changes: 74 additions & 0 deletions pkg/jsonschema/testdata/output/deep.schema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
{
"$schema": "http://json-schema.org/draft-07/schema",
"properties": {
"configMaps": {
"additionalProperties": {
"additionalProperties": false,
"properties": {
"annotations": {
"additionalProperties": {
"required": [],
"type": [
"string",
"null"
]
},
"required": [],
"type": [
"object",
"null"
]
},
"binaryData": {
"additionalProperties": {
"required": [],
"type": "string"
},
"required": [],
"type": "object"
},
"data": {
"additionalProperties": {
"required": [],
"type": "string"
},
"required": [],
"type": "object"
},
"enabled": {
"default": true,
"required": [],
"type": "boolean"
},
"includeInChecksum": {
"default": true,
"required": [],
"type": "boolean"
},
"labels": {
"additionalProperties": {
"required": [],
"type": [
"string",
"null"
]
},
"required": [],
"type": [
"object",
"null"
]
},
"nameOverride": {
"required": [],
"type": "string"
}
},
"required": [],
"type": "object"
},
"required": []
}
},
"required": []
}

0 comments on commit 193e6aa

Please sign in to comment.