Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 19 additions & 71 deletions validator/validate.go
Original file line number Diff line number Diff line change
@@ -1,99 +1,47 @@
package validator

import (
"cuelang.org/go/cue"
cueerrors "cuelang.org/go/cue/errors"
"encoding/json"
"fmt"
"io"

"cuelang.org/go/cue"
cueerrors "cuelang.org/go/cue/errors"
"github.com/meshery/meshkit/errors"
"github.com/meshery/meshkit/utils"
// "github.com/meshery/schemas"
// "sync"
"github.com/meshery/schemas"
)

var (
ErrValidateCode = ""
_ = "components.schemas"
// cueschema cue.Value
// mx sync.Mutex
// isSchemaLoaded bool
)

// func loadSchema() error {
// mx.Lock()
// defer func() {
// mx.Unlock()
// }()

// if isSchemaLoaded {
// return nil
// }

// file, err := schemas.Schemas.Open("schemas/constructs/v1beta1/design/design.json")
// if err != nil {
// return utils.ErrReadFile(err, "schemas/constructs/v1beta1/design/design.json")
// }

// cueschema, err = utils.ConvertoCue(file)
// if err == nil {
// isSchemaLoaded = true
// }
// return err
// }

func GetSchemaFor(resourceName string) (cue.Value, error) {
var schema cue.Value
var schemaJSON string

// Use simplified schemas for testing without external references
var sch string
switch resourceName {
case "design":
schemaJSON = `{
"type": "object",
"properties": {
"name": {"type": "string"},
"services": {"type": "object"},
"schemaVersion": {"type": "string"}
},
"required": ["name"]
}`
sch = "constructs/v1beta1/design/design.json"
case "catalog_data":
schemaJSON = `{
"type": "object",
"properties": {
"publishedVersion": {"type": "string", "pattern": "^v[0-9]+\\.[0-9]+\\.[0-9]+$"},
"contentClass": {"type": "string"},
"compatibility": {"type": "array"},
"patternCaveats": {"type": "string"},
"patternInfo": {"type": "string"},
"type": {"type": "string", "enum": ["Deployment", "Service", "ConfigMap"]}
},
"required": ["publishedVersion", "contentClass", "compatibility", "patternCaveats", "patternInfo", "type"]
}`
sch = "constructs/v1alpha1/catalog_data.json"
case "models":
schemaJSON = `{
"type": "object",
"properties": {
"schemaVersion": {"type": "string"},
"version": {"type": "string"},
"category": {"type": "object"},
"model": {"type": "object"},
"status": {"type": "string"},
"displayName": {"type": "string"},
"description": {"type": "string"}
},
"required": ["schemaVersion", "version", "displayName", "description"]
}`
sch = "constructs/v1beta1/model/model.json"
default:
return schema, fmt.Errorf("no schema defined for resource: %s", resourceName)
}

schema, err := utils.JsonSchemaToCue(schemaJSON)
file, err := schemas.Schemas.Open(fmt.Sprintf("schemas/%s", sch))
if err != nil {
return schema, err
}

return schema, nil
byt, err := io.ReadAll(file)
if err != nil {
return schema, err
}
val, err := utils.JsonToCue(byt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There appears to be a logic error in how the schema is being processed. The byt variable contains the content of a JSON Schema file. To correctly convert a JSON Schema into a CUE value that can be used for validation, you should use utils.JsonSchemaToCue. The current implementation uses utils.JsonToCue, which simply converts the JSON Schema document itself into a CUE value, rather than interpreting it as a set of validation rules. This will cause validations to not work as intended.

Suggested change
val, err := utils.JsonToCue(byt)
val, err := utils.JsonSchemaToCue(string(byt))

if err != nil {
return schema, err
}
return val, nil
}

func Validate(schema cue.Value, resourceValue interface{}) error {
Expand Down
69 changes: 28 additions & 41 deletions validator/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,80 +4,67 @@ import (
"fmt"
"testing"

"github.com/meshery/meshkit/models/catalog/v1alpha1"
"github.com/meshery/schemas/models/v1alpha2"
"github.com/meshery/schemas/models/v1beta1"
"github.com/meshery/schemas/models/v1beta1/category"
"github.com/meshery/schemas/models/v1beta1/model"
"cuelang.org/go/cue"
)

type ValidationCases struct {
Path string
Resource interface{}
Resource any

ShouldPass bool
}

func generateValidResource(t *testing.T, schema cue.Value) any {
var resource map[string]any
err := schema.Decode(&resource)
if err != nil {
t.Fatal(err)
}
return resource
}
Comment on lines +17 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The generateValidResource function's logic seems to be based on a misunderstanding of cue.Value.Decode. This function is for decoding a concrete CUE value into a Go type, not for generating a valid data instance from a schema definition (which is typically non-concrete). If the schema has required fields without default values, Decode will not populate them, and the resulting resource will be invalid. This makes the tests for "design" and "models" that rely on this function unreliable. You should manually construct valid resource instances for your test cases to ensure they are deterministic and correct.


func TestValidator(t *testing.T) {
tests := []ValidationCases{
{
Path: "design",
Resource: v1alpha2.PatternFile{
Name: "test-design",
Services: make(map[string]*v1alpha2.Service),
},
Path: "design",
ShouldPass: true,
},
{
Path: "catalog_data",
Resource: v1alpha1.CatalogData{
PublishedVersion: "v.10.9",
ContentClass: "sdsds",
Compatibility: []v1alpha1.CatalogDataCompatibility{
"kubernetes",
},
PatternCaveats: "NA",
PatternInfo: "NA",
Type: v1alpha1.CatalogDataType("Dployment"),
Resource: map[string]any{
"pattern_caveats": "NA",
"pattern_info": "NA",
"type": "Deployment",
},
ShouldPass: false,
},
{
Path: "models",
Resource: model.ModelDefinition{

SchemaVersion: v1beta1.ModelSchemaVersion,
Version: "1.0.0",

Category: category.CategoryDefinition{
Name: "test",
},
Model: model.Model{
Version: "1.0.0",
},
Status: "",
DisplayName: "",
Description: "",
},
ShouldPass: false,
Path: "models",
ShouldPass: true,
},
}

for _, test := range tests {
t.Run("validaion", func(_t *testing.T) {
t.Run(test.Path, func(_t *testing.T) {
schema, err := GetSchemaFor(test.Path)
if err != nil {
t.Errorf("%v", err)

}
var resource any
if test.Resource != nil {
resource = test.Resource
} else {
resource = generateValidResource(t, schema)
}

err = Validate(schema, test.Resource)
err = Validate(schema, resource)
fmt.Println(err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This fmt.Println(err) appears to be a leftover debugging statement. It should be removed to keep the test output clean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This fmt.Println call seems to be a leftover from debugging. It adds unnecessary noise to the test output and should be removed to keep the test suite clean.

if test.ShouldPass && err != nil {
t.Errorf("test failed for %s, got %s, want %t, error: %v", test.Path, "false", test.ShouldPass, err)
t.Errorf("test failed for %s, got %t, want %t, error: %v", test.Path, false, test.ShouldPass, err)

} else if !test.ShouldPass && err == nil {
t.Errorf("test failed for %s, got %s, want %t error: %v", test.Path, "true", !test.ShouldPass, err)
t.Errorf("test failed for %s, got %t, want %t error: %v", test.Path, true, !test.ShouldPass, err)
}

})
Expand Down