From 918978590cfdea3734f901b2f0639d8e21fa4d84 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Mon, 2 Mar 2026 14:35:14 -0700 Subject: [PATCH 1/3] chore: bring in spicedb and update compilation --- .../composable-schema-invalid-root.zed | 4 +- .../preview-test/composable-schema-root.zed | 4 +- internal/cmd/schema.go | 24 ++--- .../validate-test/composable-schema-root.zed | 2 + .../composable-schema-warning-root.zed | 4 +- .../validate-test/only-passes-composable.zed | 5 -- .../validate-test/only-passes-standard.zed | 2 - internal/cmd/validate.go | 87 ++----------------- internal/cmd/validate_test.go | 21 +---- internal/decode/decoder.go | 47 +++------- 10 files changed, 41 insertions(+), 159 deletions(-) delete mode 100644 internal/cmd/validate-test/only-passes-composable.zed delete mode 100644 internal/cmd/validate-test/only-passes-standard.zed diff --git a/internal/cmd/preview-test/composable-schema-invalid-root.zed b/internal/cmd/preview-test/composable-schema-invalid-root.zed index bc5138dd..f31a6f3e 100644 --- a/internal/cmd/preview-test/composable-schema-invalid-root.zed +++ b/internal/cmd/preview-test/composable-schema-invalid-root.zed @@ -1,6 +1,8 @@ +use import + import "./composable-schema-user.zed" definition resource { relation and: user permission viewer = and -} \ No newline at end of file +} diff --git a/internal/cmd/preview-test/composable-schema-root.zed b/internal/cmd/preview-test/composable-schema-root.zed index b58c62a3..40885370 100644 --- a/internal/cmd/preview-test/composable-schema-root.zed +++ b/internal/cmd/preview-test/composable-schema-root.zed @@ -1,6 +1,8 @@ +use import + import "./composable-schema-user.zed" definition resource { relation user: user permission view = user -} \ No newline at end of file +} diff --git a/internal/cmd/schema.go b/internal/cmd/schema.go index 912b1dbd..54ba2688 100644 --- a/internal/cmd/schema.go +++ b/internal/cmd/schema.go @@ -18,8 +18,6 @@ import ( v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/pkg/caveats/types" - newcompiler "github.com/authzed/spicedb/pkg/composableschemadsl/compiler" - newinput "github.com/authzed/spicedb/pkg/composableschemadsl/input" "github.com/authzed/spicedb/pkg/diff" "github.com/authzed/spicedb/pkg/schemadsl/compiler" "github.com/authzed/spicedb/pkg/schemadsl/generator" @@ -435,27 +433,17 @@ func schemaCompileInner(args []string, writer io.Writer) error { return errors.New("attempted to compile empty schema") } - compiled, err := newcompiler.Compile(newcompiler.InputSchema{ - Source: newinput.Source(inputFilepath), + compiled, err := compiler.Compile(compiler.InputSchema{ + Source: input.Source(inputFilepath), SchemaString: string(schemaBytes), - }, newcompiler.AllowUnprefixedObjectType(), - newcompiler.SourceFolder(inputSourceFolder)) + }, compiler.AllowUnprefixedObjectType(), + compiler.SourceFolder(inputSourceFolder)) if err != nil { return err } - // Attempt to cast one kind of OrderedDefinition to another - oldDefinitions := make([]compiler.SchemaDefinition, 0, len(compiled.OrderedDefinitions)) - for _, definition := range compiled.OrderedDefinitions { - oldDefinition, ok := definition.(compiler.SchemaDefinition) - if !ok { - return fmt.Errorf("could not convert definition to old schemadefinition: %v", oldDefinition) - } - oldDefinitions = append(oldDefinitions, oldDefinition) - } - - // This is where we functionally assert that the two systems are compatible - generated, _, err := generator.GenerateSchema(oldDefinitions) + // Generate the schema, which compiles over import and partial syntax + generated, _, err := generator.GenerateSchema(compiled.OrderedDefinitions) if err != nil { return fmt.Errorf("could not generate resulting schema: %w", err) } diff --git a/internal/cmd/validate-test/composable-schema-root.zed b/internal/cmd/validate-test/composable-schema-root.zed index 264c4314..40885370 100644 --- a/internal/cmd/validate-test/composable-schema-root.zed +++ b/internal/cmd/validate-test/composable-schema-root.zed @@ -1,3 +1,5 @@ +use import + import "./composable-schema-user.zed" definition resource { diff --git a/internal/cmd/validate-test/composable-schema-warning-root.zed b/internal/cmd/validate-test/composable-schema-warning-root.zed index 892f3472..50346ba9 100644 --- a/internal/cmd/validate-test/composable-schema-warning-root.zed +++ b/internal/cmd/validate-test/composable-schema-warning-root.zed @@ -1,7 +1,9 @@ +use partial + partial edit_partial { permission edit = edit } definition resource { ...edit_partial -} \ No newline at end of file +} diff --git a/internal/cmd/validate-test/only-passes-composable.zed b/internal/cmd/validate-test/only-passes-composable.zed deleted file mode 100644 index 57cbf2fc..00000000 --- a/internal/cmd/validate-test/only-passes-composable.zed +++ /dev/null @@ -1,5 +0,0 @@ -partial foo {} - -definition bar { - ...foo -} diff --git a/internal/cmd/validate-test/only-passes-standard.zed b/internal/cmd/validate-test/only-passes-standard.zed deleted file mode 100644 index 87044628..00000000 --- a/internal/cmd/validate-test/only-passes-standard.zed +++ /dev/null @@ -1,2 +0,0 @@ -// "and" is a reserved keyword in composable schemas -definition and {} diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 6cc981a2..c34906e7 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -14,11 +14,9 @@ import ( "github.com/muesli/termenv" "github.com/spf13/cobra" - composable "github.com/authzed/spicedb/pkg/composableschemadsl/compiler" "github.com/authzed/spicedb/pkg/development" core "github.com/authzed/spicedb/pkg/proto/core/v1" devinterface "github.com/authzed/spicedb/pkg/proto/developer/v1" - "github.com/authzed/spicedb/pkg/schemadsl/compiler" "github.com/authzed/spicedb/pkg/spiceerrors" "github.com/authzed/spicedb/pkg/validationfile" @@ -93,12 +91,9 @@ func registerValidateCmd(cmd *cobra.Command) { validateCmd.Flags().Bool("force-color", false, "force color code output even in non-tty environments") validateCmd.Flags().Bool("fail-on-warn", false, "treat warnings as errors during validation") - validateCmd.Flags().String("schema-type", "", "force validation according to specific schema syntax (\"\", \"composable\", \"standard\")") cmd.AddCommand(validateCmd) } -var validSchemaTypes = []string{"", "standard", "composable"} - func validatePreRunE(cmd *cobra.Command, _ []string) error { // Override lipgloss's autodetection of whether it's in a terminal environment // and display things in color anyway. This can be nice in CI environments that @@ -108,17 +103,6 @@ func validatePreRunE(cmd *cobra.Command, _ []string) error { lipgloss.SetColorProfile(termenv.ANSI256) } - schemaType := cobrautil.MustGetString(cmd, "schema-type") - schemaTypeValid := false - for _, validType := range validSchemaTypes { - if schemaType == validType { - schemaTypeValid = true - } - } - if !schemaTypeValid { - return fmt.Errorf("schema-type must be one of \"\", \"standard\", \"composable\". received: %s", schemaType) - } - return nil } @@ -130,7 +114,6 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro successfullyValidatedFiles = 0 shouldExit = false toPrint = &strings.Builder{} - schemaType = cobrautil.MustGetString(cmd, "schema-type") failOnWarn = cobrautil.MustGetBool(cmd, "fail-on-warn") ) @@ -153,43 +136,14 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro var parsed validationfile.ValidationFile // the decoder is also where compilation happens. validateContents, isOnlySchema, err := decoder(&parsed) - standardErrors, composableErrs, otherErrs := classifyErrors(err) - - switch schemaType { - case "standard": - if standardErrors != nil { - var errWithSource spiceerrors.WithSourceError - if errors.As(standardErrors, &errWithSource) { - outputErrorWithSource(toPrint, validateContents, errWithSource) - shouldExit = true - } - return "", shouldExit, standardErrors - } - case "composable": - if composableErrs != nil { - var errWithSource spiceerrors.WithSourceError - if errors.As(composableErrs, &errWithSource) { - outputErrorWithSource(toPrint, validateContents, errWithSource) - shouldExit = true - } - return "", shouldExit, composableErrs - } - default: - // By default, validate will attempt to validate a schema first according to composable schema rules, - // then standard schema rules, - // and if both fail it will show the errors from composable schema. - if composableErrs != nil && standardErrors != nil { - var errWithSource spiceerrors.WithSourceError - if errors.As(composableErrs, &errWithSource) { - outputErrorWithSource(toPrint, validateContents, errWithSource) - shouldExit = true - } - return "", shouldExit, composableErrs - } - } - if otherErrs != nil { - return "", false, otherErrs + if err != nil { + var errWithSource spiceerrors.WithSourceError + if errors.As(err, &errWithSource) { + outputErrorWithSource(toPrint, validateContents, errWithSource) + shouldExit = true + } + return "", shouldExit, err } tuples := make([]*core.RelationTuple, 0) @@ -213,6 +167,8 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro // Calculate the schema offset, used for outputting errors and warnings // and having them point to the right place regardless of zed vs yaml schemaOffset := parsed.Schema.SourcePosition.LineNumber + fmt.Println("offset before reset") + fmt.Println(schemaOffset) if isOnlySchema { schemaOffset = 0 } @@ -404,28 +360,3 @@ func renderLine(sb *strings.Builder, lines []string, index int, highlight string highlightedSourceStyle().Render(strings.Repeat("~", highlightLength))) } } - -// classifyErrors returns errors from the composable DSL, the standard DSL, and any other parsing errors. -func classifyErrors(err error) (error, error, error) { - if err == nil { - return nil, nil, nil - } - var standardErr compiler.BaseCompilerError - var composableErr composable.BaseCompilerError - var retStandard, retComposable, allOthers error - - ok := errors.As(err, &standardErr) - if ok { - retStandard = standardErr - } - ok = errors.As(err, &composableErr) - if ok { - retComposable = composableErr - } - - if retStandard == nil && retComposable == nil { - allOthers = err - } - - return retStandard, retComposable, allOthers -} diff --git a/internal/cmd/validate_test.go b/internal/cmd/validate_test.go index 39b4f32c..38a8225f 100644 --- a/internal/cmd/validate_test.go +++ b/internal/cmd/validate_test.go @@ -254,26 +254,6 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida }, expectStr: "Success! - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", }, - `composable_schema_only_without_flag_passes`: { - files: []string{ - filepath.Join("validate-test", "only-passes-composable.zed"), - }, - expectStr: "Success! - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", - }, - `standard_only_with_composable_flag_fails`: { - schemaTypeFlag: "composable", - files: []string{ - filepath.Join("validate-test", "only-passes-standard.zed"), - }, - expectErr: "Expected identifier, found token TokenTypeKeyword", - }, - `composable_only_with_standard_flag_fails`: { - schemaTypeFlag: "standard", - files: []string{ - filepath.Join("validate-test", "only-passes-composable.zed"), - }, - expectErr: "Unexpected token at root level", - }, `composable_in_validation_yaml_with_standard_fails`: { schemaTypeFlag: "standard", files: []string{ @@ -303,6 +283,7 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida complete - 0 relationships loaded, 0 assertions run, 0 expected relations validated `, }, + // TODO: it's not showing the pointer for whatever reason. `warnings_point_at_correct_line_in_zed`: { files: []string{ filepath.Join("validate-test", "warnings-point-at-right-line.zed"), diff --git a/internal/decode/decoder.go b/internal/decode/decoder.go index 028eeaf5..f0445682 100644 --- a/internal/decode/decoder.go +++ b/internal/decode/decoder.go @@ -15,10 +15,8 @@ import ( "github.com/rs/zerolog/log" "gopkg.in/yaml.v3" - composable "github.com/authzed/spicedb/pkg/composableschemadsl/compiler" - "github.com/authzed/spicedb/pkg/composableschemadsl/generator" "github.com/authzed/spicedb/pkg/schemadsl/compiler" - "github.com/authzed/spicedb/pkg/schemadsl/input" + "github.com/authzed/spicedb/pkg/schemadsl/generator" "github.com/authzed/spicedb/pkg/spiceerrors" "github.com/authzed/spicedb/pkg/validationfile" "github.com/authzed/spicedb/pkg/validationfile/blocks" @@ -200,47 +198,30 @@ func hasYAMLRelationshipsKey(input string) bool { return yamlRelationshipsKeyPattern.MatchString(input) } -// compileSchemaFromData attempts to compile using the old DSL and the new composable DSL, -// but prefers the new DSL. -// It returns the errors returned by both compilations. +// TODO: is this still in use? +// compileSchemaFromData attempts to compile a schema to a form that can be written to SpiceDB. func compileSchemaFromData(filename, schemaString string, out any) error { - var ( - standardCompileErr error - composableCompiled *composable.CompiledSchema - composableCompileErr error - vfile validationfile.ValidationFile - ) + var vfile validationfile.ValidationFile vfile = *out.(*validationfile.ValidationFile) + // TODO: figure out if this is still necessary vfile.Schema = blocks.SchemaWithPosition{ SourcePosition: spiceerrors.SourcePosition{LineNumber: 1, ColumnPosition: 1}, } - _, standardCompileErr = compiler.Compile(compiler.InputSchema{ - Source: input.Source("schema"), - SchemaString: schemaString, - }, compiler.AllowUnprefixedObjectType()) - - if standardCompileErr == nil { - vfile.Schema.Schema = schemaString - } - inputSourceFolder := filepath.Dir(filename) - composableCompiled, composableCompileErr = composable.Compile(composable.InputSchema{ + compiled, err := compiler.Compile(compiler.InputSchema{ SchemaString: schemaString, - }, composable.AllowUnprefixedObjectType(), composable.SourceFolder(inputSourceFolder)) - - // We'll only attempt to generate the composable schema string if we don't already - // have one from standard schema compilation - if composableCompileErr == nil && vfile.Schema.Schema == "" { - compiledSchemaString, _, err := generator.GenerateSchema(composableCompiled.OrderedDefinitions) - if err != nil { - return fmt.Errorf("could not generate string schema: %w", err) - } - vfile.Schema.Schema = compiledSchemaString + }, compiler.AllowUnprefixedObjectType(), compiler.SourceFolder(inputSourceFolder)) + if err != nil { + return err } - err := errors.Join(standardCompileErr, composableCompileErr) + compiledSchemaString, _, err := generator.GenerateSchema(compiled.OrderedDefinitions) + if err != nil { + return fmt.Errorf("could not generate string schema: %w", err) + } + vfile.Schema.Schema = compiledSchemaString *out.(*validationfile.ValidationFile) = vfile return err From 5327e9a1101789411c758e2c50d2b5205b4651d9 Mon Sep 17 00:00:00 2001 From: Tanner Stirrat Date: Thu, 5 Mar 2026 10:30:20 -0700 Subject: [PATCH 2/3] chore: fix test --- internal/cmd/import.go | 8 +- .../composable-schema-invalid-root.zed | 5 +- internal/cmd/schema_test.go | 2 +- internal/cmd/validate.go | 86 ++++--- internal/cmd/validate_test.go | 70 +----- internal/decode/decoder.go | 212 +++++------------- internal/decode/decoder_test.go | 174 ++------------ 7 files changed, 154 insertions(+), 403 deletions(-) diff --git a/internal/cmd/import.go b/internal/cmd/import.go index 37e35be0..f20e5bc0 100644 --- a/internal/cmd/import.go +++ b/internal/cmd/import.go @@ -13,7 +13,6 @@ import ( v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/pkg/tuple" - "github.com/authzed/spicedb/pkg/validationfile" "github.com/authzed/zed/internal/client" "github.com/authzed/zed/internal/commands" @@ -82,14 +81,11 @@ func registerImportCmd(rootCmd *cobra.Command) { func importCmdFunc(cmd *cobra.Command, schemaClient v1.SchemaServiceClient, relationshipsClient v1.PermissionsServiceClient, prefix string, u *url.URL) error { prefix = strings.TrimRight(prefix, "/") - decoder, err := decode.DecoderForURL(u) + contents, err := decode.FetchURL(u) if err != nil { return err } - var p validationfile.ValidationFile - if _, _, err := decoder(&p); err != nil { - return err - } + p, err := decode.UnmarshalYAMLValidationFile(contents) if cobrautil.MustGetBool(cmd, "schema") { if err := importSchema(cmd.Context(), schemaClient, p.Schema.Schema, prefix); err != nil { diff --git a/internal/cmd/preview-test/composable-schema-invalid-root.zed b/internal/cmd/preview-test/composable-schema-invalid-root.zed index f31a6f3e..67889974 100644 --- a/internal/cmd/preview-test/composable-schema-invalid-root.zed +++ b/internal/cmd/preview-test/composable-schema-invalid-root.zed @@ -3,6 +3,7 @@ use import import "./composable-schema-user.zed" definition resource { - relation and: user - permission viewer = and + // definition is a keyword, so we expect it to fail here. + relation definition: user + permission viewer = definition } diff --git a/internal/cmd/schema_test.go b/internal/cmd/schema_test.go index e4bef19d..ad658113 100644 --- a/internal/cmd/schema_test.go +++ b/internal/cmd/schema_test.go @@ -18,7 +18,7 @@ import ( "google.golang.org/grpc" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" - "github.com/authzed/spicedb/pkg/composableschemadsl/compiler" + "github.com/authzed/spicedb/pkg/schemadsl/compiler" "github.com/authzed/zed/internal/zedtesting" ) diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index c34906e7..f617de39 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -3,8 +3,10 @@ package cmd import ( "errors" "fmt" + "io/fs" "net/url" "os" + "path/filepath" "strconv" "strings" @@ -17,7 +19,6 @@ import ( "github.com/authzed/spicedb/pkg/development" core "github.com/authzed/spicedb/pkg/proto/core/v1" devinterface "github.com/authzed/spicedb/pkg/proto/developer/v1" - "github.com/authzed/spicedb/pkg/spiceerrors" "github.com/authzed/spicedb/pkg/validationfile" "github.com/authzed/zed/internal/commands" @@ -69,7 +70,7 @@ func registerValidateCmd(cmd *cobra.Command) { From a devtools instance: zed validate https://localhost:8443/download`, Args: commands.ValidationWrapper(cobra.MinimumNArgs(1)), - ValidArgsFunction: commands.FileExtensionCompletions("zed", "yaml", "zaml"), + ValidArgsFunction: commands.FileExtensionCompletions("zed", "yaml", "yml", "zaml"), PreRunE: validatePreRunE, RunE: func(cmd *cobra.Command, filenames []string) error { result, shouldExit, err := validateCmdFunc(cmd, filenames) @@ -91,6 +92,7 @@ func registerValidateCmd(cmd *cobra.Command) { validateCmd.Flags().Bool("force-color", false, "force color code output even in non-tty environments") validateCmd.Flags().Bool("fail-on-warn", false, "treat warnings as errors during validation") + validateCmd.Flags().String("type", "", "the type of the validated file. valid options are \"zed\" and \"yaml\"") cmd.AddCommand(validateCmd) } @@ -115,8 +117,25 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro shouldExit = false toPrint = &strings.Builder{} failOnWarn = cobrautil.MustGetBool(cmd, "fail-on-warn") + fileTypeArg = cobrautil.MustGetString(cmd, "type") + fileType = decode.FileTypeUnknown ) + if fileTypeArg != "" { + switch fileTypeArg { + case "yaml": + fileType = decode.FileTypeYaml + case "zed": + fileType = decode.FileTypeZed + default: + return "", true, fmt.Errorf("invalid value \"%s\" for --type. valid options are \"zed\" and \"yaml\"", fileTypeArg) + } + } + + // Get a handle on a filesystem rooted where the command was invoked. This + // ensures that we don't traverse outside of where the command was invoked. + filesystem := os.DirFS(".") + for _, filename := range filenames { // If we're running over multiple files, print the filename for context/debugging purposes if totalFiles > 1 { @@ -125,27 +144,48 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro u, err := url.Parse(filename) if err != nil { - return "", false, err + return "", true, err } - decoder, err := decode.DecoderForURL(u) + contents, err := decode.FetchURL(u) if err != nil { - return "", false, err + return "", true, err } var parsed validationfile.ValidationFile - // the decoder is also where compilation happens. - validateContents, isOnlySchema, err := decoder(&parsed) - + switch fileType { + case decode.FileTypeYaml: + parsed, err = decode.UnmarshalYAMLValidationFile(contents) + case decode.FileTypeZed: + parsed = decode.UnmarshalSchemaValidationFile(contents) + case decode.FileTypeUnknown: + parsed, err = decode.UnmarshalValidationFile(contents) + } + // This block handles the error regardless of which case statement is hit if err != nil { - var errWithSource spiceerrors.WithSourceError - if errors.As(err, &errWithSource) { - outputErrorWithSource(toPrint, validateContents, errWithSource) - shouldExit = true + // TODO: what should this string be? + return "", true, err + } + + // Ensure that either schema or schemaFile is present + if parsed.Schema.Schema == "" { + if parsed.SchemaFile == "" { + return "", false, errors.New("either schema or schemaFile must be present") + } + + // If schema is not defined and schemaFile is, we go and grab the schemaFile and + // stick it in the schema field. + fileDir := filepath.Dir(filename) + schemaFileName := filepath.Join(fileDir, parsed.SchemaFile) + contents, err := fs.ReadFile(filesystem, schemaFileName) + if err != nil { + return "", false, fmt.Errorf("could not read schemaFile %s at path %s", parsed.SchemaFile, schemaFileName) } - return "", shouldExit, err + parsed.Schema.Schema = string(contents) } + // This logic will use the zero value of the struct, so we don't need + // to do it conditionally. tuples := make([]*core.RelationTuple, 0) totalAssertions := 0 totalRelationsValidated := 0 @@ -159,7 +199,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro devCtx, devErrs, err := development.NewDevContext(ctx, &devinterface.RequestContext{ Schema: parsed.Schema.Schema, Relationships: tuples, - }) + }, development.WithSourceFS(os.DirFS("."))) if err != nil { return "", false, err } @@ -167,14 +207,9 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro // Calculate the schema offset, used for outputting errors and warnings // and having them point to the right place regardless of zed vs yaml schemaOffset := parsed.Schema.SourcePosition.LineNumber - fmt.Println("offset before reset") - fmt.Println(schemaOffset) - if isOnlySchema { - schemaOffset = 0 - } // Output errors - outputDeveloperErrorsWithLineOffset(toPrint, validateContents, devErrs.InputErrors, schemaOffset) + outputDeveloperErrorsWithLineOffset(toPrint, contents, devErrs.InputErrors, schemaOffset) return toPrint.String(), true, nil } // Run assertions @@ -183,7 +218,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro return "", false, aerr } if adevErrs != nil { - outputDeveloperErrors(toPrint, validateContents, adevErrs) + outputDeveloperErrors(toPrint, contents, adevErrs) return toPrint.String(), true, nil } successfullyValidatedFiles++ @@ -194,7 +229,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro return "", false, rerr } if erDevErrs != nil { - outputDeveloperErrors(toPrint, validateContents, erDevErrs) + outputDeveloperErrors(toPrint, contents, erDevErrs) return toPrint.String(), true, nil } // Print out any warnings for file @@ -206,7 +241,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro if len(warnings) > 0 { for _, warning := range warnings { fmt.Fprintf(toPrint, "%s%s\n", warningPrefix(), warning.Message) - outputForLine(toPrint, validateContents, uint64(warning.Line), warning.SourceCode, uint64(warning.Column)) // warning.LineNumber is 1-indexed + outputForLine(toPrint, contents, uint64(warning.Line), warning.SourceCode, uint64(warning.Column)) // warning.LineNumber is 1-indexed toPrint.WriteString("\n") } @@ -233,11 +268,6 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro return toPrint.String(), shouldExit, nil } -func outputErrorWithSource(sb *strings.Builder, validateContents []byte, errWithSource spiceerrors.WithSourceError) { - fmt.Fprintf(sb, "%s%s\n", errorPrefix(), errorMessageStyle().Render(errWithSource.Error())) - outputForLine(sb, validateContents, errWithSource.LineNumber, errWithSource.SourceCodeString, 0) // errWithSource.LineNumber is 1-indexed -} - func outputForLine(sb *strings.Builder, validateContents []byte, oneIndexedLineNumber uint64, sourceCodeString string, oneIndexedColumnPosition uint64) { lines := strings.Split(string(validateContents), "\n") intLineNumber, err := safecast.Convert[int](oneIndexedLineNumber) diff --git a/internal/cmd/validate_test.go b/internal/cmd/validate_test.go index 38a8225f..b4aec113 100644 --- a/internal/cmd/validate_test.go +++ b/internal/cmd/validate_test.go @@ -47,42 +47,13 @@ func normalizeNewlines(s string) string { func TestValidatePreRun(t *testing.T) { t.Parallel() - testCases := map[string]struct { - schemaTypeFlag string - expectErr bool - }{ - `invalid`: { - schemaTypeFlag: "invalid", - expectErr: true, - }, - `composable`: { - schemaTypeFlag: "composable", - expectErr: false, - }, - `standard`: { - schemaTypeFlag: "standard", - expectErr: false, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - t.Parallel() - - cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, - zedtesting.BoolFlag{FlagName: "force-color", FlagValue: false}, - zedtesting.StringFlag{FlagName: "schema-type", FlagValue: tc.schemaTypeFlag}, - zedtesting.BoolFlag{FlagName: "fail-on-warn", FlagValue: false}, - ) + cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, + zedtesting.BoolFlag{FlagName: "force-color", FlagValue: false}, + zedtesting.BoolFlag{FlagName: "fail-on-warn", FlagValue: false}, + ) - err := validatePreRunE(cmd, []string{}) - if tc.expectErr { - require.ErrorContains(t, err, "schema-type must be one of \"\", \"standard\", \"composable\"") - } else { - require.NoError(t, err) - } - }) - } + err := validatePreRunE(cmd, []string{}) + require.NoError(t, err) } func TestValidate(t *testing.T) { @@ -144,12 +115,6 @@ total files: 2, successfully validated files: 2 }, expectErr: "Unexpected token at root level", }, - `standard_only_without_flag_passes`: { - files: []string{ - filepath.Join("validate-test", "only-passes-standard.zed"), - }, - expectStr: "Success! - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", - }, `without_schema_fails`: { files: []string{ filepath.Join("validate-test", "missing-schema.yaml"), @@ -254,13 +219,6 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida }, expectStr: "Success! - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", }, - `composable_in_validation_yaml_with_standard_fails`: { - schemaTypeFlag: "standard", - files: []string{ - filepath.Join("validate-test", "external-and-composable.yaml"), - }, - expectErr: "Unexpected token at root level", - }, `composable_in_validation_yaml_with_composable_passes`: { schemaTypeFlag: "composable", files: []string{ @@ -269,19 +227,10 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida expectStr: "Success! - 1 relationships loaded, 2 assertions run, 0 expected relations validated\n", }, `warnings_in_composable_fail`: { - schemaTypeFlag: "composable", files: []string{ filepath.Join("validate-test", "composable-schema-warning-root.zed"), }, - expectStr: `warning: Permission "edit" references itself, which will cause an error to be raised due to infinite recursion (permission-references-itself) - 1 | partial edit_partial { - 2 > permission edit = edit - > ^~~~ - 3 | } - 4 | - -complete - 0 relationships loaded, 0 assertions run, 0 expected relations validated -`, + expectStr: "warning: Permission \"edit\" references itself, which will cause an error to be raised due to infinite recursion (permission-references-itself)\n 1 | use partial\n 2 | \n 3 | partial edit_partial {\n 4 > permission edit = edit\n > ^~~~\n 5 | }\n 6 | \n\ncomplete - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", }, // TODO: it's not showing the pointer for whatever reason. `warnings_point_at_correct_line_in_zed`: { @@ -308,6 +257,7 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100}, zedtesting.IntFlag{FlagName: "workers", FlagValue: 1}, zedtesting.BoolFlag{FlagName: "fail-on-warn", FlagValue: false}, + zedtesting.StringFlag{FlagName: "type", FlagValue: ""}, ) res, shouldError, err := validateCmdFunc(cmd, tc.files) @@ -318,7 +268,7 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida require.Error(err) require.Contains(err.Error(), tc.expectErr) } - require.Equal(tc.expectNonZeroStatusCode, shouldError) + require.Equal(tc.expectNonZeroStatusCode, shouldError, "non-zero status code value didn't match expectation") }) } } @@ -334,6 +284,7 @@ func TestFailOnWarn(t *testing.T) { zedtesting.BoolFlag{FlagName: "force-color", FlagValue: false}, zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100}, zedtesting.IntFlag{FlagName: "workers", FlagValue: 1}, zedtesting.BoolFlag{FlagName: "fail-on-warn", FlagValue: false}, + zedtesting.StringFlag{FlagName: "type", FlagValue: ""}, ) _, shouldError, _ := validateCmdFunc(cmd, []string{filepath.Join("validate-test", "schema-with-warnings.zed")}) @@ -346,6 +297,7 @@ func TestFailOnWarn(t *testing.T) { zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100}, zedtesting.IntFlag{FlagName: "workers", FlagValue: 1}, zedtesting.BoolFlag{FlagName: "fail-on-warn", FlagValue: true}, + zedtesting.StringFlag{FlagName: "type", FlagValue: ""}, ) _, shouldError, _ = validateCmdFunc(cmd, []string{filepath.Join("validate-test", "schema-with-warnings.zed")}) diff --git a/internal/decode/decoder.go b/internal/decode/decoder.go index f0445682..51eb2c08 100644 --- a/internal/decode/decoder.go +++ b/internal/decode/decoder.go @@ -1,82 +1,62 @@ package decode import ( - "errors" "fmt" "io" "net/http" "net/url" "os" "path" - "path/filepath" "regexp" "strings" "github.com/rs/zerolog/log" "gopkg.in/yaml.v3" - "github.com/authzed/spicedb/pkg/schemadsl/compiler" - "github.com/authzed/spicedb/pkg/schemadsl/generator" "github.com/authzed/spicedb/pkg/spiceerrors" "github.com/authzed/spicedb/pkg/validationfile" "github.com/authzed/spicedb/pkg/validationfile/blocks" ) -var playgroundPattern = regexp.MustCompile("^.*/s/.*/schema|relationships|assertions|expected.*$") - -// yamlKeyPatterns match YAML top-level keys that indicate a validation file format. -// These patterns look for the key at the start of a line (column 0), followed by a colon. -// This avoids false positives like "relation schema: parent" in a schema file being -// mistaken for the "schema:" YAML key. -var ( - yamlSchemaKeyPattern = regexp.MustCompile(`(?m)^schema\s*:`) - yamlSchemaFileKeyPattern = regexp.MustCompile(`(?m)^schemaFile\s*:`) - yamlRelationshipsKeyPattern = regexp.MustCompile(`(?m)^relationships\s*:`) +const ( + FileTypeUnknown = iota + FileTypeYaml + FileTypeZed ) -// SchemaRelationships holds the schema (as a string) and a list of -// relationships (as a string) in the format from the devtools download API. -type SchemaRelationships struct { - Schema string `yaml:"schema"` - Relationships string `yaml:"relationships"` -} +var playgroundPattern = regexp.MustCompile("^.*/s/.*/schema|relationships|assertions|expected.*$") // Func will decode into the supplied object. -type Func func(out any) ([]byte, bool, error) +type Func func(out any) ([]byte, error) -// DecoderForURL returns the appropriate decoder for a given URL. -// Some URLs have special handling to dereference to the actual file. -func DecoderForURL(u *url.URL) (d Func, err error) { +// FetchURL interprets the URL and fetches using the appropriate transport. +func FetchURL(u *url.URL) (contents []byte, err error) { switch s := u.Scheme; s { case "", "file": - d = fileDecoder(u) + return fetchFile(u) case "http", "https": - d = httpDecoder(u) + return fetchHTTP(u) default: - err = fmt.Errorf("%s scheme not supported", s) + return nil, fmt.Errorf("%s scheme not supported", s) } - return d, err } -func fileDecoder(u *url.URL) Func { - return func(out any) ([]byte, bool, error) { - fs := os.DirFS(".") - file, err := fs.Open(u.Path) - if err != nil { - return nil, false, err - } - data, err := io.ReadAll(file) - if err != nil { - return nil, false, err - } - isOnlySchema, err := unmarshalAsYAMLOrSchemaWithFile(data, out, u.Path) - return data, isOnlySchema, err +func fetchFile(u *url.URL) ([]byte, error) { + fs := os.DirFS(".") + file, err := fs.Open(u.Path) + if err != nil { + return nil, err } + data, err := io.ReadAll(file) + if err != nil { + return nil, err + } + return data, err } -func httpDecoder(u *url.URL) Func { +func fetchHTTP(u *url.URL) ([]byte, error) { rewriteURL(u) - return directHTTPDecoder(u) + return fetchHTTPDirectly(u) } func rewriteURL(u *url.URL) { @@ -99,130 +79,52 @@ func rewriteURL(u *url.URL) { } } -func directHTTPDecoder(u *url.URL) Func { - return func(out any) ([]byte, bool, error) { - log.Debug().Stringer("url", u).Send() - r, err := http.Get(u.String()) - if err != nil { - return nil, false, err - } - defer r.Body.Close() - data, err := io.ReadAll(r.Body) - if err != nil { - return nil, false, err - } - - isOnlySchema, err := unmarshalAsYAMLOrSchema("", data, out) - return data, isOnlySchema, err - } -} - -// Uses the files passed in the args and looks for the specified schemaFile to parse the YAML. -func unmarshalAsYAMLOrSchemaWithFile(data []byte, out any, filename string) (bool, error) { - inputString := string(data) - if hasYAMLSchemaFileKey(inputString) && !hasYAMLSchemaKey(inputString) { - if err := yaml.Unmarshal(data, out); err != nil { - return false, err - } - validationFile, ok := out.(*validationfile.ValidationFile) - if !ok { - return false, errors.New("could not cast unmarshalled file to validationfile") - } - - // Need to join the original filepath with the requested filepath - // to construct the path to the referenced schema file. - // NOTE: This does not allow for yaml files to transitively reference - // each other's schemaFile fields. - // TODO: enable this behavior - schemaPath := filepath.Join(filepath.Dir(filename), validationFile.SchemaFile) - - if !filepath.IsLocal(schemaPath) { - // We want to prevent access of files that are outside of the folder - // where the command was originally invoked. This should do that. - return false, fmt.Errorf("schema filepath %s must be local to where the command was invoked", schemaPath) - } - - fs := os.DirFS(".") - file, err := fs.Open(schemaPath) - if err != nil { - return false, err - } - data, err = io.ReadAll(file) - if err != nil { - return false, err - } - } - return unmarshalAsYAMLOrSchema(filename, data, out) -} - -func unmarshalAsYAMLOrSchema(filename string, data []byte, out any) (bool, error) { - inputString := string(data) - - // Check for indications of a schema-only file by looking for YAML top-level keys. - // We use regex patterns to match keys at the start of a line to avoid false positives - // like "relation schema: parent" in a schema file being mistaken for the "schema:" YAML key. - if !hasYAMLSchemaKey(inputString) && !hasYAMLRelationshipsKey(inputString) { - if err := compileSchemaFromData(filename, inputString, out); err != nil { - return false, err - } - return true, nil - } - - if !hasYAMLSchemaKey(inputString) && !hasYAMLSchemaFileKey(inputString) { - // If there is no schema and no schemaFile and it doesn't compile then it must be yaml with missing fields - if err := compileSchemaFromData(filename, inputString, out); err != nil { - return false, errors.New("either schema or schemaFile must be present") - } - return true, nil +func fetchHTTPDirectly(u *url.URL) ([]byte, error) { + log.Debug().Stringer("url", u).Send() + r, err := http.Get(u.String()) + if err != nil { + return nil, err } - // Try to unparse as YAML for the validation file format. - if err := yaml.Unmarshal(data, out); err != nil { - return false, err + defer r.Body.Close() + data, err := io.ReadAll(r.Body) + if err != nil { + return nil, err } - return false, nil + return data, err } -// hasYAMLSchemaKey returns true if the input contains a "schema:" YAML key at the start of a line. -func hasYAMLSchemaKey(input string) bool { - return yamlSchemaKeyPattern.MatchString(input) -} +func UnmarshalValidationFile(contents []byte) (validationfile.ValidationFile, error) { + // Try first to unmarshal as yaml. If it can't be unmarshalled, we assume it's a schema. + // TODO: make sure this doesn't attempt to unmarshal relation schema: user + vFile, err := UnmarshalYAMLValidationFile(contents) + if err == nil { + return vFile, nil + } -// hasYAMLSchemaFileKey returns true if the input contains a "schemaFile:" YAML key at the start of a line. -func hasYAMLSchemaFileKey(input string) bool { - return yamlSchemaFileKeyPattern.MatchString(input) + // yaml unmarshalling was unsuccessful, so we construct a vfile that assumes the schema + // is the bytes and return it. + return UnmarshalSchemaValidationFile(contents), nil } -// hasYAMLRelationshipsKey returns true if the input contains a "relationships:" YAML key at the start of a line. -func hasYAMLRelationshipsKey(input string) bool { - return yamlRelationshipsKeyPattern.MatchString(input) +// TODO: use os.OpenRoot to avoid needing to check whether the file is local +// or DirFS(). Not sure which. +func UnmarshalYAMLValidationFile(yamlBytes []byte) (validationfile.ValidationFile, error) { + var vFile validationfile.ValidationFile + // Try to unmarshal as YAML for the validation file format. + err := yaml.Unmarshal(yamlBytes, &vFile) + return vFile, err } -// TODO: is this still in use? -// compileSchemaFromData attempts to compile a schema to a form that can be written to SpiceDB. -func compileSchemaFromData(filename, schemaString string, out any) error { - var vfile validationfile.ValidationFile +// TODO: does this actually do anything? +// I think the idea is that we'd just pass it to the validation logic. +func UnmarshalSchemaValidationFile(schemaBytes []byte) validationfile.ValidationFile { + var vFile validationfile.ValidationFile - vfile = *out.(*validationfile.ValidationFile) - // TODO: figure out if this is still necessary - vfile.Schema = blocks.SchemaWithPosition{ + vFile.Schema = blocks.SchemaWithPosition{ SourcePosition: spiceerrors.SourcePosition{LineNumber: 1, ColumnPosition: 1}, + Schema: string(schemaBytes), } - inputSourceFolder := filepath.Dir(filename) - compiled, err := compiler.Compile(compiler.InputSchema{ - SchemaString: schemaString, - }, compiler.AllowUnprefixedObjectType(), compiler.SourceFolder(inputSourceFolder)) - if err != nil { - return err - } - - compiledSchemaString, _, err := generator.GenerateSchema(compiled.OrderedDefinitions) - if err != nil { - return fmt.Errorf("could not generate string schema: %w", err) - } - vfile.Schema.Schema = compiledSchemaString - - *out.(*validationfile.ValidationFile) = vfile - return err + return vFile } diff --git a/internal/decode/decoder_test.go b/internal/decode/decoder_test.go index c789edc2..cf1ec71c 100644 --- a/internal/decode/decoder_test.go +++ b/internal/decode/decoder_test.go @@ -5,8 +5,6 @@ import ( "testing" "github.com/stretchr/testify/require" - - "github.com/authzed/spicedb/pkg/validationfile" ) func TestRewriteURL(t *testing.T) { @@ -131,11 +129,10 @@ func TestRewriteURL(t *testing.T) { func TestUnmarshalAsYAMLOrSchema(t *testing.T) { tests := []struct { - name string - in []byte - isOnlySchema bool - outSchema string - wantErr bool + name string + in []byte + outSchema string + wantErr bool }{ { name: "valid yaml", @@ -143,23 +140,25 @@ func TestUnmarshalAsYAMLOrSchema(t *testing.T) { schema: definition user {} `), - outSchema: `definition user {}`, - isOnlySchema: false, - wantErr: false, + outSchema: `definition user {}`, + wantErr: false, }, { - name: "valid schema", - in: []byte(`definition user {}`), - isOnlySchema: true, - outSchema: `definition user {}`, - wantErr: false, + name: "valid schema", + in: []byte(`definition user {}`), + outSchema: `definition user {}`, + wantErr: false, }, { - name: "invalid yaml", - in: []byte(`invalid yaml`), - isOnlySchema: false, - outSchema: "", - wantErr: true, + name: "invalid yaml", + in: []byte(` +schema: "" +relationships: + some: key + bad: indentation + `), + outSchema: "", + wantErr: true, }, { name: "schema with relation named schema", @@ -176,7 +175,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user @@ -207,7 +205,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user @@ -236,7 +233,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user permission manage = owner @@ -263,7 +259,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user permission manage = owner @@ -292,7 +287,6 @@ definition user {}`, definition user {} `), - isOnlySchema: false, outSchema: `definition parent { relation owner: user permission manage = owner @@ -309,136 +303,12 @@ definition user {}`, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - block := validationfile.ValidationFile{} - isOnlySchema, err := unmarshalAsYAMLOrSchema("", tt.in, &block) + vFile, err := UnmarshalValidationFile(tt.in) require.Equal(t, tt.wantErr, err != nil) - require.Equal(t, tt.isOnlySchema, isOnlySchema) if !tt.wantErr { - require.Equal(t, tt.outSchema, block.Schema.Schema) + // TODO: this test has gotten kinda vacuous for non-yaml stuff. + require.Equal(t, tt.outSchema, vFile.Schema.Schema) } }) } } - -func TestHasYAMLSchemaKey(t *testing.T) { - tests := []struct { - name string - input string - expected bool - }{ - { - name: "yaml schema key at start of line", - input: "schema:\n definition user {}", - expected: true, - }, - { - name: "yaml schema key with space before colon", - input: "schema :\n definition user {}", - expected: true, - }, - { - name: "relation named schema in definition", - input: "definition child {\n\trelation schema: parent\n}", - expected: false, - }, - { - name: "relation named something_schema in definition", - input: "definition child {\n\trelation something_schema: parent\n}", - expected: false, - }, - { - name: "schema arrow expression", - input: "definition child {\n\tpermission access = schema->manage\n}", - expected: false, - }, - { - name: "no schema at all", - input: "definition user {}", - expected: false, - }, - { - name: "schema in single line comment should not trigger", - input: "// schema: this is a comment\ndefinition user {}", - expected: false, - }, - { - name: "schema in block comment should not trigger", - input: "/* schema: block comment */\ndefinition user {}", - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := hasYAMLSchemaKey(tt.input) - require.Equal(t, tt.expected, result) - }) - } -} - -func TestHasYAMLRelationshipsKey(t *testing.T) { - tests := []struct { - name string - input string - expected bool - }{ - { - name: "yaml relationships key at start of line", - input: "schema:\n definition user {}\nrelationships:\n user:1#member@user:2", - expected: true, - }, - { - name: "no relationships key", - input: "schema:\n definition user {}", - expected: false, - }, - { - name: "relationships in middle of line", - input: " relationships: user:1#member@user:2", - expected: false, - }, - { - name: "relation named relationships in definition", - input: "definition child {\n\trelation relationships: parent\n}", - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := hasYAMLRelationshipsKey(tt.input) - require.Equal(t, tt.expected, result) - }) - } -} - -func TestHasYAMLSchemaFileKey(t *testing.T) { - tests := []struct { - name string - input string - expected bool - }{ - { - name: "yaml schemaFile key at start of line", - input: "schemaFile: ./schema.zed\nrelationships:\n user:1#member@user:2", - expected: true, - }, - { - name: "no schemaFile key", - input: "schema:\n definition user {}", - expected: false, - }, - { - name: "relation named schemaFile in definition", - input: "definition child {\n\trelation schemaFile: parent\n}", - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := hasYAMLSchemaFileKey(tt.input) - require.Equal(t, tt.expected, result) - }) - } -} From 2a7434f8ed1daa6dd1eb3502056a1606230fbf2a Mon Sep 17 00:00:00 2001 From: Maria Ines Parnisari Date: Thu, 12 Mar 2026 16:14:10 -0700 Subject: [PATCH 3/3] refactor: decoder --- docs/zed.md | 6 +- go.mod | 2 +- internal/cmd/import.go | 7 +- internal/cmd/validate.go | 21 ++-- internal/decode/decoder.go | 159 +++++++++++++++++++++-------- internal/decode/decoder_test.go | 173 ++++++++++++++++++++++++++++---- 6 files changed, 293 insertions(+), 75 deletions(-) diff --git a/docs/zed.md b/docs/zed.md index 848825e8..2fb00de2 100644 --- a/docs/zed.md +++ b/docs/zed.md @@ -1431,9 +1431,9 @@ zed validate [flags] ### Options ``` - --fail-on-warn treat warnings as errors during validation - --force-color force color code output even in non-tty environments - --schema-type string force validation according to specific schema syntax ("", "composable", "standard") + --fail-on-warn treat warnings as errors during validation + --force-color force color code output even in non-tty environments + --type string the type of the validated file. valid options are "zed" and "yaml" ``` ### Options Inherited From Parent Flags diff --git a/go.mod b/go.mod index 96446e26..dfe376ff 100644 --- a/go.mod +++ b/go.mod @@ -42,6 +42,7 @@ require ( github.com/spf13/pflag v1.0.10 github.com/stretchr/testify v1.11.1 github.com/xlab/treeprint v1.2.0 + go.uber.org/goleak v1.3.0 go.uber.org/mock v0.6.0 golang.org/x/exp v0.0.0-20250819193227-8b4c13bb791b golang.org/x/mod v0.32.0 @@ -406,7 +407,6 @@ require ( go.opentelemetry.io/proto/otlp v1.7.0 // indirect go.uber.org/atomic v1.11.0 // indirect go.uber.org/automaxprocs v1.6.0 // indirect - go.uber.org/goleak v1.3.0 // indirect go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect diff --git a/internal/cmd/import.go b/internal/cmd/import.go index f20e5bc0..5349f2e4 100644 --- a/internal/cmd/import.go +++ b/internal/cmd/import.go @@ -81,11 +81,14 @@ func registerImportCmd(rootCmd *cobra.Command) { func importCmdFunc(cmd *cobra.Command, schemaClient v1.SchemaServiceClient, relationshipsClient v1.PermissionsServiceClient, prefix string, u *url.URL) error { prefix = strings.TrimRight(prefix, "/") - contents, err := decode.FetchURL(u) + d, err := decode.DecoderFromURL(u) + if err != nil { + return err + } + p, err := d.UnmarshalYAMLValidationFile() if err != nil { return err } - p, err := decode.UnmarshalYAMLValidationFile(contents) if cobrautil.MustGetBool(cmd, "schema") { if err := importSchema(cmd.Context(), schemaClient, p.Schema.Schema, prefix); err != nil { diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index f617de39..e8245043 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -123,7 +123,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro if fileTypeArg != "" { switch fileTypeArg { - case "yaml": + case "yaml", "yml", "zaml": fileType = decode.FileTypeYaml case "zed": fileType = decode.FileTypeZed @@ -147,19 +147,22 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro return "", true, err } - contents, err := decode.FetchURL(u) + d, err := decode.DecoderFromURL(u) if err != nil { return "", true, err } + contents := d.Contents - var parsed validationfile.ValidationFile + var parsed *validationfile.ValidationFile switch fileType { case decode.FileTypeYaml: - parsed, err = decode.UnmarshalYAMLValidationFile(contents) + parsed, err = d.UnmarshalYAMLValidationFile() case decode.FileTypeZed: - parsed = decode.UnmarshalSchemaValidationFile(contents) + parsed = d.UnmarshalSchemaValidationFile() case decode.FileTypeUnknown: - parsed, err = decode.UnmarshalValidationFile(contents) + parsed, err = d.UnmarshalAsYAMLOrSchema() + default: + return "", true, fmt.Errorf("invalid file type %q", fileType) } // This block handles the error regardless of which case statement is hit if err != nil { @@ -177,11 +180,11 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro // stick it in the schema field. fileDir := filepath.Dir(filename) schemaFileName := filepath.Join(fileDir, parsed.SchemaFile) - contents, err := fs.ReadFile(filesystem, schemaFileName) + contentsOfSchema, err := fs.ReadFile(filesystem, schemaFileName) if err != nil { return "", false, fmt.Errorf("could not read schemaFile %s at path %s", parsed.SchemaFile, schemaFileName) } - parsed.Schema.Schema = string(contents) + parsed.Schema.Schema = string(contentsOfSchema) } // This logic will use the zero value of the struct, so we don't need @@ -199,7 +202,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro devCtx, devErrs, err := development.NewDevContext(ctx, &devinterface.RequestContext{ Schema: parsed.Schema.Schema, Relationships: tuples, - }, development.WithSourceFS(os.DirFS("."))) + }, development.WithSourceFS(filesystem)) if err != nil { return "", false, err } diff --git a/internal/decode/decoder.go b/internal/decode/decoder.go index 51eb2c08..041bf821 100644 --- a/internal/decode/decoder.go +++ b/internal/decode/decoder.go @@ -1,12 +1,15 @@ package decode import ( + "errors" "fmt" "io" + "io/fs" "net/http" "net/url" "os" "path" + "path/filepath" "regexp" "strings" @@ -18,45 +21,71 @@ import ( "github.com/authzed/spicedb/pkg/validationfile/blocks" ) +// yamlKeyPatterns match YAML top-level keys that indicate a validation file format. +// These patterns look for the key at the start of a line (column 0), followed by a colon. +// This avoids false positives like "relation schema: parent" in a schema file being +// mistaken for the "schema:" YAML key. +var ( + yamlSchemaKeyPattern = regexp.MustCompile(`(?m)^\s*schema\s*:`) + yamlSchemaFileKeyPattern = regexp.MustCompile(`(?m)^\s*schemaFile\s*:`) + yamlRelationshipsKeyPattern = regexp.MustCompile(`(?m)^\s*relationships\s*:`) + + playgroundPattern = regexp.MustCompile("^.*/s/.*/schema|relationships|assertions|expected.*$") +) + const ( FileTypeUnknown = iota FileTypeYaml FileTypeZed ) -var playgroundPattern = regexp.MustCompile("^.*/s/.*/schema|relationships|assertions|expected.*$") - -// Func will decode into the supplied object. -type Func func(out any) ([]byte, error) +// Decoder holds fetched file contents along with a filesystem for resolving +// relative paths (e.g. schemaFile references). For remote URLs the filesystem +// is nil because relative file references are not supported. +type Decoder struct { + Contents []byte + // FS is rooted at the directory of the fetched file. It is non-nil only + // for local (file://) URLs. + FS fs.FS +} -// FetchURL interprets the URL and fetches using the appropriate transport. -func FetchURL(u *url.URL) (contents []byte, err error) { +// DecoderFromURL interprets the URL, fetches the content, and returns a +// Decoder. For local files the Decoder's FS is rooted at the file's directory +// so that relative schemaFile paths can be resolved. +func DecoderFromURL(u *url.URL) (*Decoder, error) { switch s := u.Scheme; s { case "", "file": - return fetchFile(u) + return decoderFromFile(u) case "http", "https": - return fetchHTTP(u) + return decoderFromHTTP(u) default: return nil, fmt.Errorf("%s scheme not supported", s) } } -func fetchFile(u *url.URL) ([]byte, error) { - fs := os.DirFS(".") - file, err := fs.Open(u.Path) +func decoderFromFile(u *url.URL) (*Decoder, error) { + filePath := u.Path + data, err := os.ReadFile(filePath) if err != nil { return nil, err } - data, err := io.ReadAll(file) - if err != nil { - return nil, err - } - return data, err + dir := filepath.Dir(filePath) + return &Decoder{ + Contents: data, + FS: os.DirFS(dir), + }, nil } -func fetchHTTP(u *url.URL) ([]byte, error) { +func decoderFromHTTP(u *url.URL) (*Decoder, error) { rewriteURL(u) - return fetchHTTPDirectly(u) + data, err := fetchHTTPDirectly(u) + if err != nil { + return nil, err + } + return &Decoder{ + Contents: data, + FS: nil, + }, nil } func rewriteURL(u *url.URL) { @@ -94,37 +123,85 @@ func fetchHTTPDirectly(u *url.URL) ([]byte, error) { return data, err } -func UnmarshalValidationFile(contents []byte) (validationfile.ValidationFile, error) { - // Try first to unmarshal as yaml. If it can't be unmarshalled, we assume it's a schema. - // TODO: make sure this doesn't attempt to unmarshal relation schema: user - vFile, err := UnmarshalYAMLValidationFile(contents) +var ErrInvalidYamlTryZed = errors.New("invalid yaml") + +// UnmarshalAsYAMLOrSchema tries to unmarshal as YAML first, falling back to +// treating the contents as a raw schema. +func (d *Decoder) UnmarshalAsYAMLOrSchema() (*validationfile.ValidationFile, error) { + vFile, err := d.UnmarshalYAMLValidationFile() if err == nil { return vFile, nil } + if !errors.Is(err, ErrInvalidYamlTryZed) { + return nil, err + } - // yaml unmarshalling was unsuccessful, so we construct a vfile that assumes the schema - // is the bytes and return it. - return UnmarshalSchemaValidationFile(contents), nil + return d.UnmarshalSchemaValidationFile(), nil } -// TODO: use os.OpenRoot to avoid needing to check whether the file is local -// or DirFS(). Not sure which. -func UnmarshalYAMLValidationFile(yamlBytes []byte) (validationfile.ValidationFile, error) { - var vFile validationfile.ValidationFile - // Try to unmarshal as YAML for the validation file format. - err := yaml.Unmarshal(yamlBytes, &vFile) - return vFile, err -} +// UnmarshalYAMLValidationFile unmarshals YAML validation file contents. If the +// YAML contains a schemaFile reference, the Decoder's FS is used to resolve it. +func (d *Decoder) UnmarshalYAMLValidationFile() (*validationfile.ValidationFile, error) { + inputString := string(d.Contents) + + // Only attempt YAML unmarshaling if the input looks like a YAML validation file. + if !hasYAMLSchemaKey(inputString) && !hasYAMLSchemaFileKey(inputString) && !yamlRelationshipsKeyPattern.MatchString(inputString) { + return nil, fmt.Errorf("%w: input does not appear to be a YAML validation file", ErrInvalidYamlTryZed) + } -// TODO: does this actually do anything? -// I think the idea is that we'd just pass it to the validation logic. -func UnmarshalSchemaValidationFile(schemaBytes []byte) validationfile.ValidationFile { - var vFile validationfile.ValidationFile + var validationFile validationfile.ValidationFile + err := yaml.Unmarshal(d.Contents, &validationFile) + if err != nil { + return nil, err + } + + // If schemaFile is specified, resolve it using the Decoder's filesystem. + if validationFile.SchemaFile != "" { + // Clean the path for use with fs.FS (which doesn't accept ./ prefix). + schemaPath := filepath.Clean(validationFile.SchemaFile) + + if !filepath.IsLocal(schemaPath) { + return nil, fmt.Errorf("schema filepath %q must be local to where the command was invoked", schemaPath) + } + + if d.FS == nil { + return nil, fmt.Errorf("cannot resolve schemaFile %q: no local filesystem context (remote URL?)", schemaPath) + } + + file, err := d.FS.Open(schemaPath) + if err != nil { + return nil, err + } + schemaBytes, err := io.ReadAll(file) + if err != nil { + return nil, err + } + validationFile.SchemaFile = "" + validationFile.Schema = blocks.SchemaWithPosition{ + SourcePosition: spiceerrors.SourcePosition{LineNumber: 1, ColumnPosition: 1}, + Schema: string(schemaBytes), + } + } - vFile.Schema = blocks.SchemaWithPosition{ - SourcePosition: spiceerrors.SourcePosition{LineNumber: 1, ColumnPosition: 1}, - Schema: string(schemaBytes), + return &validationFile, nil +} + +// UnmarshalSchemaValidationFile wraps raw schema bytes into a ValidationFile. +func (d *Decoder) UnmarshalSchemaValidationFile() *validationfile.ValidationFile { + return &validationfile.ValidationFile{ + Schema: blocks.SchemaWithPosition{ + SourcePosition: spiceerrors.SourcePosition{LineNumber: 1, ColumnPosition: 1}, + Schema: string(d.Contents), + }, } +} + +// hasYAMLSchemaKey returns true if the input contains a "schema:" YAML key at the start of a line. +func hasYAMLSchemaKey(input string) bool { + return yamlSchemaKeyPattern.MatchString(input) +} - return vFile +// hasYAMLSchemaFileKey returns true if the input contains a "schemaFile:" YAML key at the start of a line. +func hasYAMLSchemaFileKey(input string) bool { + return yamlSchemaFileKeyPattern.MatchString(input) } diff --git a/internal/decode/decoder_test.go b/internal/decode/decoder_test.go index cf1ec71c..08f254a5 100644 --- a/internal/decode/decoder_test.go +++ b/internal/decode/decoder_test.go @@ -1,12 +1,152 @@ package decode import ( + "net/http" + "net/http/httptest" "net/url" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/require" + "go.uber.org/goleak" ) +func TestDecoderFromURL(t *testing.T) { + t.Cleanup(func() { + goleak.VerifyNone(t) + }) + yamlContent := `--- +schema: |- + definition user {} +relationships: |- + resource:1#reader@user:1 +` + invalidYamlContent := `--- +schemaFile: "./external-schema.zed" +relationships: |- + resource:1#reader@user:1 +` + schemaContent := "definition user {}\n" + + // Spin up a test HTTP server that serves the file contents based on path. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/valid.yaml": + _, _ = w.Write([]byte(yamlContent)) + case "/invalid.yaml": + _, _ = w.Write([]byte(invalidYamlContent)) + case "/valid.zed": + _, _ = w.Write([]byte(schemaContent)) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + + t.Run("valid yaml file over http", func(t *testing.T) { + u, err := url.Parse(srv.URL + "/valid.yaml") + require.NoError(t, err) + + d, err := DecoderFromURL(u) + require.NoError(t, err) + require.Nil(t, d.FS) + require.YAMLEq(t, yamlContent, string(d.Contents)) + + vFile, err := d.UnmarshalAsYAMLOrSchema() + require.NoError(t, err) + require.Equal(t, "definition user {}", vFile.Schema.Schema) + require.Equal(t, "resource:1#reader@user:1", vFile.Relationships.RelationshipsString) + }) + + t.Run("valid zed schema file over http", func(t *testing.T) { + u, err := url.Parse(srv.URL + "/valid.zed") + require.NoError(t, err) + + d, err := DecoderFromURL(u) + require.NoError(t, err) + require.Nil(t, d.FS) + require.Equal(t, []byte(schemaContent), d.Contents) + + vFile, err := d.UnmarshalAsYAMLOrSchema() + require.NoError(t, err) + require.Equal(t, schemaContent, vFile.Schema.Schema) + }) + + t.Run("invalid yaml file over http", func(t *testing.T) { + u, err := url.Parse(srv.URL + "/invalid.yaml") + require.NoError(t, err) + + d, err := DecoderFromURL(u) + require.NoError(t, err) + require.Nil(t, d.FS) + require.YAMLEq(t, invalidYamlContent, string(d.Contents)) + + vFile, err := d.UnmarshalAsYAMLOrSchema() + // arbitrary decision: we could fetch the remote URL, but I don't want to. + require.ErrorContains(t, err, "cannot resolve schemaFile \"external-schema.zed\": no local filesystem context (remote URL?)") + require.Nil(t, vFile) + }) +} + +func TestUnmarshalYAMLValidationFile(t *testing.T) { + schemaContent := "definition user {}\ndefinition resource {\nrelation reader: user\n}\n" + + // Write real files to a temp directory so DecoderFromURL -> decoderFromFile is exercised. + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "schema.zed"), []byte(schemaContent), 0o600)) + + tests := []struct { + name string + yamlContent string + expectedSchema string + expectedRels string + expectedErrText string + }{ + { + name: "resolves_local_schemaFile", + yamlContent: `--- +schemaFile: "./schema.zed" +relationships: |- + resource:1#reader@user:1 +`, + expectedSchema: schemaContent, + expectedRels: "resource:1#reader@user:1", + }, + { + name: "rejects_schemaFile_pointing_to_above_directory", + yamlContent: `--- +schemaFile: "../secret/schema.zed" +relationships: |- + resource:1#reader@user:1 +`, + expectedErrText: "must be local to where the command was invoked", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := filepath.Join(dir, tt.name+".yaml") + require.NoError(t, os.WriteFile(f, []byte(tt.yamlContent), 0o600)) + u, err := url.Parse(f) + require.NoError(t, err) + + d, err := DecoderFromURL(u) + require.NoError(t, err) + + vFile, err := d.UnmarshalYAMLValidationFile() + if tt.expectedErrText != "" { + require.ErrorContains(t, err, tt.expectedErrText) + return + } + require.NoError(t, err) + require.Equal(t, tt.expectedSchema, vFile.Schema.Schema) + require.Empty(t, vFile.SchemaFile) + require.Equal(t, tt.expectedRels, vFile.Relationships.RelationshipsString) + }) + } +} + func TestRewriteURL(t *testing.T) { tests := []struct { name string @@ -132,7 +272,7 @@ func TestUnmarshalAsYAMLOrSchema(t *testing.T) { name string in []byte outSchema string - wantErr bool + wantErr string }{ { name: "valid yaml", @@ -141,24 +281,22 @@ schema: definition user {} `), outSchema: `definition user {}`, - wantErr: false, }, { name: "valid schema", in: []byte(`definition user {}`), outSchema: `definition user {}`, - wantErr: false, }, { name: "invalid yaml", in: []byte(` -schema: "" -relationships: - some: key - bad: indentation - `), + schema: "" + relationships: + some: key + bad: indentation + `), outSchema: "", - wantErr: true, + wantErr: "yaml: line 2: found character that cannot start any token", }, { name: "schema with relation named schema", @@ -188,7 +326,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "schema with permission named schema", @@ -218,7 +355,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "schema with relation named something_schema", @@ -244,7 +380,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "schema with relation named relationships", @@ -270,7 +405,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "valid yaml with relation named schema inside", @@ -298,17 +432,18 @@ definition child { } definition user {}`, - wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - vFile, err := UnmarshalValidationFile(tt.in) - require.Equal(t, tt.wantErr, err != nil) - if !tt.wantErr { - // TODO: this test has gotten kinda vacuous for non-yaml stuff. - require.Equal(t, tt.outSchema, vFile.Schema.Schema) + d := &Decoder{Contents: tt.in} + vFile, err := d.UnmarshalAsYAMLOrSchema() + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return } + require.NoError(t, err) + require.Equal(t, tt.outSchema, vFile.Schema.Schema) }) } }