Skip to content

Conversation

AuraReaper
Copy link

Description

This PR fixes #576

Notes for Reviewers
replaced the hardcoeded schemas values to use the original schemas from the schemas repo

Signed commits

  • Yes, I signed my commits.

Copy link
Contributor

Summary of Changes

Hello @AuraReaper, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue with unknown domain references in the validator by refactoring the schema loading mechanism. It transitions from using inline, hardcoded JSON schema definitions to dynamically fetching schemas from the meshery/schemas repository. This change improves the robustness and maintainability of the validation process by ensuring that the validator always uses the most current and authoritative schema definitions.

Highlights

  • Dynamic Schema Loading: The validator now dynamically loads schemas from the github.com/meshery/schemas repository instead of using hardcoded JSON strings. This ensures the use of canonical and up-to-date schema definitions.
  • Test Refactoring: Validation test cases have been updated to align with the new dynamic schema loading. A helper function was introduced to generate valid resources for testing, simplifying test data structures and improving maintainability.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a good step towards improving maintainability by loading validation schemas from the schemas repository instead of hardcoding them. However, I've identified a critical issue in the implementation where a schema file is parsed as data instead of as a schema, which will prevent validation from working correctly. I've also found that the new test helper function generateValidResource is based on a flawed assumption and will lead to unreliable tests. My review includes suggestions to fix these problems.

Comment on lines +17 to +24
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
}
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.


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.

AuraReaper and others added 2 commits September 22, 2025 22:42
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Yash Kumar Gupta <[email protected]>
Signed-off-by: Yash Kumar Gupta <[email protected]>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly refactors the validator to load schemas from the meshery/schemas package instead of using hardcoded strings, which is a great improvement for maintainability. The associated tests are also nicely refactored to be more robust. However, there is a critical issue in validator/validate.go where utils.JsonToCue is used instead of utils.JsonSchemaToCue, which means the validation is not being performed against the actual schema rules. Additionally, a minor cleanup is needed in the test file to remove a debugging print statement.

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))


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 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.

Copy link
Contributor

Thank you for the summary. I will now proceed with a detailed review of the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Golang Test Validator Errors
1 participant