Skip to content

Adding validation against the Cue schema#16

Closed
trumant wants to merge 2 commits intoossf:mainfrom
trumant:add-schema-validation
Closed

Adding validation against the Cue schema#16
trumant wants to merge 2 commits intoossf:mainfrom
trumant:add-schema-validation

Conversation

@trumant
Copy link
Contributor

@trumant trumant commented Mar 28, 2025

This PR adds a Validate func to the v2/si/SecurityInsights struct that compares the value of the struct against those specified by the schema.

@trumant trumant force-pushed the add-schema-validation branch from 2630c48 to f25983c Compare March 28, 2025 17:43
This PR adds a Validate func to the `v2/si/SecurityInsights` struct that
compares the value of the struct against those specified by the schema.

Signed-off-by: Travis Truman <trumant@gmail.com>
@trumant trumant force-pushed the add-schema-validation branch from f25983c to 11b4265 Compare March 29, 2025 14:48
Co-authored-by: Evan Anderson <evan.k.anderson@gmail.com>
Signed-off-by: Travis Truman <trumant@gmail.com>
@eddie-knight
Copy link
Contributor

I'm concerned by the 50+ new dependencies added here. Let's hold off on merging this until I can explore any possible alternatives.

@trumant
Copy link
Contributor Author

trumant commented Apr 3, 2025

I'm concerned by the 50+ new dependencies added here. Let's hold off on merging this until I can explore any possible alternatives.

Yeah, its a bit much.

While I appreciate the expressiveness of cue for the schema, it feels like that choice is proving quite a bit of a friction in terms of serving as the foundational source of truth. The ideal foundational source of truth schema would allow us to easily generate documentation directly from comments/descriptions within the schema file as well as easily generate go types corresponding to the schema type definitions.

The changes in https://github.com/ossf/security-insights-spec/pull/111/files helpfully illustrates the complexity we see today.

Curious if others have felt this pain and considered alternatives to the cue schema, like JSON schema

@trumant
Copy link
Contributor Author

trumant commented Apr 4, 2025

Well, I just found ossf/security-insights#96 so clearly jsonschema had its own pain

@eddie-knight
Copy link
Contributor

I met on Friday with @myitcv, who is a maintainer of cue-lang. We chatted about the reputational risk of a long requirements list, and the need to keep that tight for our use case.

Let's leave this PR on standby for a bit until we can lock in the best next steps.

@trumant
Copy link
Contributor Author

trumant commented Apr 6, 2025

cue-lang/cue@51c5fdf looks promising in terms of allowing us to generate rather than manually maintain go types

@myitcv
Copy link

myitcv commented Apr 7, 2025

@eddie-knight great to catch up last week!

We chatted about the reputational risk of a long requirements list, and the need to keep that tight for our use case.

Indeed, let us report back on this PR once we have had a chance to do some analysis.

cue-lang/cue@51c5fdf looks promising in terms of allowing us to generate rather than manually maintain go types

Yes, @trumant - we have some work-in-progress docs we can share on this very soon! But the command is already live in the latest version of CUE: https://cuelang.org/docs/reference/command/cue-help-exp-gengotypes/

@trumant trumant closed this Apr 29, 2025
@myitcv
Copy link

myitcv commented Apr 29, 2025

@trumant - just out of interest, did you close the issue here in favour of generating Go types?

From my perspective, the goal of adding real CUE validation to a process is an entirely reasonable and legitimate goal, and so the dependencies need to reflect that process being lightweight.

We are working on means of cutting down the actual dependencies used by the core CUE evaluator (some work due to land today), and per my discussion with @eddie-knight we also need to look at how to handle the unused modules that appear because of module dependencies being just that, at a module level rather than package level.

@trumant
Copy link
Contributor Author

trumant commented Apr 29, 2025

@trumant - just out of interest, did you close the issue here in favour of generating Go types?

From my perspective, the goal of adding real CUE validation to a process is an entirely reasonable and legitimate goal, and so the dependencies need to reflect that process being lightweight.

We are working on means of cutting down the actual dependencies used by the core CUE evaluator (some work due to land today), and per my discussion with @eddie-knight we also need to look at how to handle the unused modules that appear because of module dependencies being just that, at a module level rather than package level.

Yes, this is closed in favor of #21

I appreciate the work happening in the cue-lang community to try to trim down the dependencies here.

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.

4 participants