Added multiple validation files parsing support #455

kartikaysaxena · 2025-01-13T15:39:46Z

zed validate now takes in multiple args to validate, also looks for schemaFile in given .yaml validation files to parse the schema. Example usage:-

(base) Kartikay2@Kartikays-MacBook-Pro-2 zed % ./zed validate validations/*
warning: Arrow `parent->reader` under permission "view" references relation "reader" on definition "folder"; it is recommended to point to a permission (arrow-references-relation)
 25 |       relation auditor: user with day_of_a_week
 26 | 
 27 | 
 28 >       // can user:kartikay view document:quartely_planning
 29 |       permission view = reader + owner + parent->reader + auditor
 30 |       permission edit = owner + parent->owner
 25 |     - "[user:owner_1] is <document:quaterly_planning#owner>"
 26 |     - "[user:owner_2] is <document:quaterly_planning#owner>"
 27 |   document:quaterly_planning#parent:
 28 >     - "[folder:planning] is <document:quaterly_planning#parent>"
 29 |   document:quaterly_planning#view:
 30 |     - "[user:auditorhere[...]] is <document:quaterly_planning#auditor>"
 25 | 
 26 | 
 27 |     // can user:kartikay view document:quartely_planning
 28 >     permission view = reader + owner + parent->reader + auditor
    >                                        ^~~~~~~~~~~~~~
 29 |     permission edit = owner + parent->owner
 30 |     permission delete = owner + parent->owner

warning: Arrow `parent->owner` under permission "edit" references relation "owner" on definition "folder"; it is recommended to point to a permission (arrow-references-relation)
 26 | 
 27 | 
 28 |       // can user:kartikay view document:quartely_planning
 29 >       permission view = reader + owner + parent->reader + auditor
 30 |       permission edit = owner + parent->owner
 31 |       permission delete = owner + parent->owner
 26 |     - "[user:owner_2] is <document:quaterly_planning#owner>"
 27 |   document:quaterly_planning#parent:
 28 |     - "[folder:planning] is <document:quaterly_planning#parent>"
 29 >   document:quaterly_planning#view:
 30 |     - "[user:auditorhere[...]] is <document:quaterly_planning#auditor>"
 31 |     - "[user:folder_reader_1] is <folder:planning#reader>"
 26 | 
 27 |     // can user:kartikay view document:quartely_planning
 28 |     permission view = reader + owner + parent->reader + auditor
 29 >     permission edit = owner + parent->owner
    >                               ^~~~~~~~~~~~~
 30 |     permission delete = owner + parent->owner
 31 | }

warning: Arrow `parent->owner` under permission "delete" references relation "owner" on definition "folder"; it is recommended to point to a permission (arrow-references-relation)
 27 | 
 28 |       // can user:kartikay view document:quartely_planning
 29 |       permission view = reader + owner + parent->reader + auditor
 30 >       permission edit = owner + parent->owner
    >                                 ^~~~~~~~~~~~~
 31 |       permission delete = owner + parent->owner
 32 |   }
 27 |   document:quaterly_planning#parent:
 28 |     - "[folder:planning] is <document:quaterly_planning#parent>"
 29 |   document:quaterly_planning#view:
 30 >     - "[user:auditorhere[...]] is <document:quaterly_planning#auditor>"
 31 |     - "[user:folder_reader_1] is <folder:planning#reader>"
 32 |     - "[user:owner_1] is <document:quaterly_planning#owner>"
 27 |     // can user:kartikay view document:quartely_planning
 28 |     permission view = reader + owner + parent->reader + auditor
 29 |     permission edit = owner + parent->owner
 30 >     permission delete = owner + parent->owner
    >                                 ^~~~~~~~~~~~~
 31 | }

complete - 26 relationships loaded, 2 assertions run, 8 expected relations validated
total files: 3, successfully validated files: 3

Edit: Raised authzed/spicedb#2206 to introduce schemaFile which would fix the tests here too

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

tstirrat15 · 2025-01-13T23:04:10Z

@kartikaysaxena as a temporary fix for the linting stuff, you can install a git SHA with go get github.com/authzed/spicedb@<git sha>. we can update the reference once there's a release to point at.

tstirrat15

See comments - the assumption about using the first schema file for the dev context is the only part that I'd like to change. The other points are more for discussion.

internal/cmd/validate.go

tstirrat15 · 2025-01-13T23:07:51Z

internal/decode/decoder.go

@@ -105,6 +105,30 @@ func directHTTPDecoder(u *url.URL) Func {
 	}
 }

+// Uses the files passed in the args and looks for the specified schemaFile to parse the YAML.
+func unmarshalAsYAMLOrSchemaWithFile(data []byte, out interface{}, args []string) (bool, error) {


Can we add validation for the case where a user specifies both the schema and the schemafile?

When both schema and schemaFIle are provided, it directly goes to yaml.Unmarshal() which prefers schema over schemaFile.

tstirrat15 · 2025-01-13T23:09:38Z

internal/cmd/validate.go

@@ -71,7 +71,7 @@ var validateCmd = &cobra.Command{

 	From a devtools instance:
 		zed validate https://localhost:8443/download`,
-	Args:              cobra.ExactArgs(1),
+	Args:              cobra.MinimumNArgs(1),


tstirrat15 · 2025-01-13T23:11:23Z

internal/decode/decoder.go

+				data, err = io.ReadAll(file)
+				if err != nil {
+					return false, err
+				}
+			}
+		}
+	}
+	return unmarshalAsYAMLOrSchema(data, out)


Hmm... I suppose that one implication of doing it this way is that one yaml file could reference another. Is that desirable? Genuine question; I don't know the answer. I think I'm fine with it, though.

We can add maybe some identifiers to prevent this if not desirable.

tstirrat15 · 2025-01-13T23:12:34Z

internal/decode/decoder.go

+		schema := out.(*validationfile.ValidationFile)
+
+		for _, arg := range args {
+			if strings.Contains(arg, schema.SchemaFile) {


What does this check do?

Checks if the file is present in the arguments provided to validate

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

tstirrat15 · 2025-01-14T20:56:35Z

This is looking good to me. Can you look into the lint failures? Then we'll get the SpiceDB PR merged and we can update this PR to point at that SpiceDB commit.

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

tstirrat15 · 2025-01-15T00:42:51Z

This looks great! Thank you!

kartikaysaxena and others added 2 commits January 13, 2025 19:38

added multiple validation files parsing support

87ea68a

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

Merge branch 'main' into multiple-validation-files

24fd2b1

tstirrat15 reviewed Jan 13, 2025

View reviewed changes

kartikaysaxena force-pushed the multiple-validation-files branch 2 times, most recently from 23bd84e to 2bb29e8 Compare January 14, 2025 20:10

merge loops and enhance logic

dc5852f

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

kartikaysaxena force-pushed the multiple-validation-files branch from 2bb29e8 to dc5852f Compare January 14, 2025 20:40

tstirrat15 mentioned this pull request Jan 14, 2025

add schemaFile to ValidationFile authzed/spicedb#2206

Merged

lint

c5908cf

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

kartikaysaxena force-pushed the multiple-validation-files branch 2 times, most recently from f6d132d to c5282b6 Compare January 14, 2025 22:46

added parser logic

2ee39b6

Signed-off-by: Kartikay <kartikay_2101ce32@iitp.ac.in>

kartikaysaxena force-pushed the multiple-validation-files branch from e7e9d0e to 2ee39b6 Compare January 14, 2025 23:13

tstirrat15 approved these changes Jan 15, 2025

View reviewed changes

tstirrat15 merged commit 753fe21 into authzed:main Jan 15, 2025
11 checks passed

github-actions bot locked and limited conversation to collaborators Jan 15, 2025

kartikaysaxena deleted the multiple-validation-files branch January 16, 2025 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added multiple validation files parsing support #455

Added multiple validation files parsing support #455

kartikaysaxena commented Jan 13, 2025 •

edited

Loading

tstirrat15 commented Jan 13, 2025

tstirrat15 left a comment

tstirrat15 Jan 13, 2025

kartikaysaxena Jan 14, 2025

tstirrat15 Jan 13, 2025

tstirrat15 Jan 13, 2025

kartikaysaxena Jan 14, 2025

tstirrat15 Jan 13, 2025

kartikaysaxena Jan 14, 2025

tstirrat15 commented Jan 14, 2025

tstirrat15 commented Jan 15, 2025

Added multiple validation files parsing support #455

Added multiple validation files parsing support #455

Conversation

kartikaysaxena commented Jan 13, 2025 • edited Loading

tstirrat15 commented Jan 13, 2025

tstirrat15 left a comment

Choose a reason for hiding this comment

tstirrat15 Jan 13, 2025

Choose a reason for hiding this comment

kartikaysaxena Jan 14, 2025

Choose a reason for hiding this comment

tstirrat15 Jan 13, 2025

Choose a reason for hiding this comment

tstirrat15 Jan 13, 2025

Choose a reason for hiding this comment

kartikaysaxena Jan 14, 2025

Choose a reason for hiding this comment

tstirrat15 Jan 13, 2025

Choose a reason for hiding this comment

kartikaysaxena Jan 14, 2025

Choose a reason for hiding this comment

tstirrat15 commented Jan 14, 2025

tstirrat15 commented Jan 15, 2025

kartikaysaxena commented Jan 13, 2025 •

edited

Loading