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 37e35be0..5349f2e4 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,12 +81,12 @@ 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) + d, err := decode.DecoderFromURL(u) if err != nil { return err } - var p validationfile.ValidationFile - if _, _, err := decoder(&p); err != nil { + p, err := d.UnmarshalYAMLValidationFile() + if err != nil { return err } diff --git a/internal/cmd/preview-test/composable-schema-invalid-root.zed b/internal/cmd/preview-test/composable-schema-invalid-root.zed index bc5138dd..67889974 100644 --- a/internal/cmd/preview-test/composable-schema-invalid-root.zed +++ b/internal/cmd/preview-test/composable-schema-invalid-root.zed @@ -1,6 +1,9 @@ +use import + import "./composable-schema-user.zed" definition resource { - relation and: user - permission viewer = and -} \ No newline at end of file + // definition is a keyword, so we expect it to fail here. + relation definition: user + permission viewer = definition +} 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/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-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..e8245043 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" @@ -14,12 +16,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" "github.com/authzed/zed/internal/commands" @@ -71,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) @@ -93,12 +92,10 @@ 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\")") + validateCmd.Flags().String("type", "", "the type of the validated file. valid options are \"zed\" and \"yaml\"") 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 +105,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,10 +116,26 @@ 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") + fileTypeArg = cobrautil.MustGetString(cmd, "type") + fileType = decode.FileTypeUnknown ) + if fileTypeArg != "" { + switch fileTypeArg { + case "yaml", "yml", "zaml": + 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 { @@ -142,56 +144,51 @@ 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) + d, err := decode.DecoderFromURL(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) - 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 - } + contents := d.Contents + + var parsed *validationfile.ValidationFile + switch fileType { + case decode.FileTypeYaml: + parsed, err = d.UnmarshalYAMLValidationFile() + case decode.FileTypeZed: + parsed = d.UnmarshalSchemaValidationFile() + case decode.FileTypeUnknown: + parsed, err = d.UnmarshalAsYAMLOrSchema() 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 - } + return "", true, fmt.Errorf("invalid file type %q", fileType) + } + // This block handles the error regardless of which case statement is hit + if err != nil { + // TODO: what should this string be? + return "", true, err } - if otherErrs != nil { - return "", false, otherErrs + // 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) + 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(contentsOfSchema) } + // 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 @@ -205,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(filesystem)) if err != nil { return "", false, err } @@ -213,12 +210,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 - 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 @@ -227,7 +221,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++ @@ -238,7 +232,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 @@ -250,7 +244,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") } @@ -277,11 +271,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) @@ -404,28 +393,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..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,33 +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_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{ - 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{ @@ -289,20 +227,12 @@ 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`: { files: []string{ filepath.Join("validate-test", "warnings-point-at-right-line.zed"), @@ -327,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) @@ -337,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") }) } } @@ -353,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")}) @@ -365,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 028eeaf5..041bf821 100644 --- a/internal/decode/decoder.go +++ b/internal/decode/decoder.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net/http" "net/url" "os" @@ -15,70 +16,76 @@ 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/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*:`) + 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.*$") ) -// 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"` -} +const ( + FileTypeUnknown = iota + FileTypeYaml + FileTypeZed +) -// Func will decode into the supplied object. -type Func func(out any) ([]byte, bool, 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 +} -// 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) { +// 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": - d = fileDecoder(u) + return decoderFromFile(u) case "http", "https": - d = httpDecoder(u) + return decoderFromHTTP(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 decoderFromFile(u *url.URL) (*Decoder, error) { + filePath := u.Path + data, err := os.ReadFile(filePath) + if err != nil { + return nil, err } + dir := filepath.Dir(filePath) + return &Decoder{ + Contents: data, + FS: os.DirFS(dir), + }, nil } -func httpDecoder(u *url.URL) Func { +func decoderFromHTTP(u *url.URL) (*Decoder, error) { rewriteURL(u) - return directHTTPDecoder(u) + data, err := fetchHTTPDirectly(u) + if err != nil { + return nil, err + } + return &Decoder{ + Contents: data, + FS: nil, + }, nil } func rewriteURL(u *url.URL) { @@ -101,88 +108,92 @@ 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 - } +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 + } + defer r.Body.Close() + data, err := io.ReadAll(r.Body) + if err != nil { + return nil, err + } + + return data, err +} - isOnlySchema, err := unmarshalAsYAMLOrSchema("", data, out) - return data, isOnlySchema, err +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 } + + return d.UnmarshalSchemaValidationFile(), nil } -// 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") - } +// 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) + } + + var validationFile validationfile.ValidationFile + err := yaml.Unmarshal(d.Contents, &validationFile) + if err != nil { + return nil, err + } - // 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 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) { - // 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) + return nil, fmt.Errorf("schema filepath %q must be local to where the command was invoked", schemaPath) } - fs := os.DirFS(".") - file, err := fs.Open(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 false, err + return nil, err } - data, err = io.ReadAll(file) + schemaBytes, err := io.ReadAll(file) if err != nil { - return false, err + return nil, 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 + validationFile.SchemaFile = "" + validationFile.Schema = blocks.SchemaWithPosition{ + SourcePosition: spiceerrors.SourcePosition{LineNumber: 1, ColumnPosition: 1}, + Schema: string(schemaBytes), } - 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 - } - // Try to unparse as YAML for the validation file format. - if err := yaml.Unmarshal(data, out); err != nil { - return false, err - } + return &validationFile, nil +} - return false, 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. @@ -194,54 +205,3 @@ func hasYAMLSchemaKey(input string) bool { func hasYAMLSchemaFileKey(input string) bool { return yamlSchemaFileKeyPattern.MatchString(input) } - -// 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) -} - -// 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. -func compileSchemaFromData(filename, schemaString string, out any) error { - var ( - standardCompileErr error - composableCompiled *composable.CompiledSchema - composableCompileErr error - vfile validationfile.ValidationFile - ) - - vfile = *out.(*validationfile.ValidationFile) - 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{ - 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 - } - - err := errors.Join(standardCompileErr, composableCompileErr) - - *out.(*validationfile.ValidationFile) = vfile - return err -} diff --git a/internal/decode/decoder_test.go b/internal/decode/decoder_test.go index c789edc2..08f254a5 100644 --- a/internal/decode/decoder_test.go +++ b/internal/decode/decoder_test.go @@ -1,14 +1,152 @@ package decode import ( + "net/http" + "net/http/httptest" "net/url" + "os" + "path/filepath" "testing" "github.com/stretchr/testify/require" - - "github.com/authzed/spicedb/pkg/validationfile" + "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 @@ -131,11 +269,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 string }{ { name: "valid yaml", @@ -143,23 +280,23 @@ func TestUnmarshalAsYAMLOrSchema(t *testing.T) { schema: definition user {} `), - outSchema: `definition user {}`, - isOnlySchema: false, - wantErr: false, + outSchema: `definition user {}`, }, { - name: "valid schema", - in: []byte(`definition user {}`), - isOnlySchema: true, - outSchema: `definition user {}`, - wantErr: false, + name: "valid schema", + in: []byte(`definition user {}`), + outSchema: `definition user {}`, }, { - 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: "yaml: line 2: found character that cannot start any token", }, { name: "schema with relation named schema", @@ -176,7 +313,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user @@ -190,7 +326,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "schema with permission named schema", @@ -207,7 +342,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user @@ -221,7 +355,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "schema with relation named something_schema", @@ -236,7 +369,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user permission manage = owner @@ -248,7 +380,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "schema with relation named relationships", @@ -263,7 +394,6 @@ definition child { } definition user {}`), - isOnlySchema: true, outSchema: `definition parent { relation owner: user permission manage = owner @@ -275,7 +405,6 @@ definition child { } definition user {}`, - wantErr: false, }, { name: "valid yaml with relation named schema inside", @@ -292,7 +421,6 @@ definition user {}`, definition user {} `), - isOnlySchema: false, outSchema: `definition parent { relation owner: user permission manage = owner @@ -304,141 +432,18 @@ definition child { } definition user {}`, - wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - block := validationfile.ValidationFile{} - isOnlySchema, err := unmarshalAsYAMLOrSchema("", tt.in, &block) - require.Equal(t, tt.wantErr, err != nil) - require.Equal(t, tt.isOnlySchema, isOnlySchema) - if !tt.wantErr { - require.Equal(t, tt.outSchema, block.Schema.Schema) + d := &Decoder{Contents: tt.in} + vFile, err := d.UnmarshalAsYAMLOrSchema() + if tt.wantErr != "" { + require.ErrorContains(t, err, tt.wantErr) + return } - }) - } -} - -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) + require.NoError(t, err) + require.Equal(t, tt.outSchema, vFile.Schema.Schema) }) } }