diff --git a/bundle/bundle.go b/bundle/bundle.go index a1600463e9..b5ff532fbf 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -355,6 +355,7 @@ type Reader struct { verificationConfig *VerificationConfig skipVerify bool processAnnotations bool + capabilities *ast.Capabilities files map[string]FileInfo // files in the bundle signature payload sizeLimitBytes int64 etag string @@ -417,6 +418,12 @@ func (r *Reader) WithProcessAnnotations(yes bool) *Reader { return r } +// WithCapabilities sets the supported capabilities when loading the files +func (r *Reader) WithCapabilities(caps *ast.Capabilities) *Reader { + r.capabilities = caps + return r +} + // WithSizeLimitBytes sets the size limit to apply to files in the bundle. If files are larger // than this, an error will be returned by the reader. func (r *Reader) WithSizeLimitBytes(n int64) *Reader { @@ -445,6 +452,13 @@ func (r *Reader) WithLazyLoadingMode(yes bool) *Reader { return r } +func (r *Reader) ParserOptions() ast.ParserOptions { + return ast.ParserOptions{ + ProcessAnnotation: r.processAnnotations, + Capabilities: r.capabilities, + } +} + // Read returns a new Bundle loaded from the reader. func (r *Reader) Read() (Bundle, error) { @@ -511,7 +525,7 @@ func (r *Reader) Read() (Bundle, error) { } r.metrics.Timer(metrics.RegoModuleParse).Start() - module, err := ast.ParseModuleWithOpts(fullPath, buf.String(), ast.ParserOptions{ProcessAnnotation: r.processAnnotations}) + module, err := ast.ParseModuleWithOpts(fullPath, buf.String(), r.ParserOptions()) r.metrics.Timer(metrics.RegoModuleParse).Stop() if err != nil { return bundle, err diff --git a/cmd/build_test.go b/cmd/build_test.go index b49ec0bc61..bcd1d81936 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -3,6 +3,7 @@ package cmd import ( "archive/tar" "compress/gzip" + "encoding/json" "fmt" "io" "os" @@ -11,6 +12,7 @@ import ( "strings" "testing" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/loader" "github.com/open-policy-agent/opa/util/test" @@ -69,52 +71,117 @@ func TestBuildProducesBundle(t *testing.T) { } func TestBuildRespectsCapabilities(t *testing.T) { - capabilitiesJSON := `{ - "builtins": [ - { - "name": "is_foo", - "decl": { - "args": [ - { - "type": "string" - } - ], - "result": { - "type": "boolean" - }, - "type": "function" - } - } - ] - }` - - files := map[string]string{ - "capabilities.json": capabilitiesJSON, - "test.rego": ` - package test - p { is_foo("bar") } - `, + tests := []struct { + note string + caps string + policy string + err string + bundleMode bool // build with "-b" flag + }{ + { + note: "builtin defined in caps", + caps: `{ + "builtins": [ + { + "name": "is_foo", + "decl": { + "args": [ + { + "type": "string" + } + ], + "result": { + "type": "boolean" + }, + "type": "function" + } + } + ] + }`, + policy: `package test +p { is_foo("bar") }`, + }, + { + note: "future kw NOT defined in caps", + caps: func() string { + c := ast.CapabilitiesForThisVersion() + c.FutureKeywords = []string{"in"} + j, err := json.Marshal(c) + if err != nil { + panic(err) + } + return string(j) + }(), + policy: `package test +import future.keywords.if +import future.keywords.in +p if "opa" in input.tools`, + err: "rego_parse_error: unexpected keyword, must be one of [in]", + }, + { + note: "future kw are defined in caps", + caps: func() string { + c := ast.CapabilitiesForThisVersion() + c.FutureKeywords = []string{"in", "if"} + j, err := json.Marshal(c) + if err != nil { + panic(err) + } + return string(j) + }(), + policy: `package test +import future.keywords.if +import future.keywords.in +p if "opa" in input.tools`, + }, } - test.WithTempFS(files, func(root string) { - caps := newcapabilitiesFlag() - if err := caps.Set(path.Join(root, "capabilities.json")); err != nil { - t.Fatal(err) - } - params := newBuildParams() - params.outputFile = path.Join(root, "bundle.tar.gz") - params.capabilities = caps + // add same tests for bundle-mode == true: + for i := range tests { + tc := tests[i] + tc.bundleMode = true + tc.note = tc.note + " (as bundle)" + tests = append(tests, tc) + } - err := dobuild(params, []string{root}) - if err != nil { - t.Fatal(err) - } + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "capabilities.json": tc.caps, + "test.rego": tc.policy, + } - _, err = loader.NewFileLoader().AsBundle(params.outputFile) - if err != nil { - t.Fatal(err) - } - }) + test.WithTempFS(files, func(root string) { + caps := newcapabilitiesFlag() + if err := caps.Set(path.Join(root, "capabilities.json")); err != nil { + t.Fatal(err) + } + params := newBuildParams() + params.outputFile = path.Join(root, "bundle.tar.gz") + params.capabilities = caps + params.bundleMode = tc.bundleMode + + err := dobuild(params, []string{root}) + switch { + case err != nil && tc.err != "": + if !strings.Contains(err.Error(), tc.err) { + t.Fatalf("expected err %v, got %v", tc.err, err) + } + return // don't read back bundle below + case err != nil && tc.err == "": + t.Fatalf("unexpected error: %v", err) + case err == nil && tc.err != "": + t.Fatalf("expected error %v, got nil", tc.err) + } + + // check that the resulting bundle is readable + _, err = loader.NewFileLoader().AsBundle(params.outputFile) + if err != nil { + t.Fatal(err) + } + }) + }) + } } func TestBuildFilesystemModeIgnoresTarGz(t *testing.T) { diff --git a/cmd/check.go b/cmd/check.go index 3e576555ba..bbe6264381 100644 --- a/cmd/check.go +++ b/cmd/check.go @@ -17,7 +17,7 @@ import ( "github.com/open-policy-agent/opa/util" ) -var checkParams = struct { +type checkParams struct { format *util.EnumFlag errLimit int ignore []string @@ -25,12 +25,16 @@ var checkParams = struct { capabilities *capabilitiesFlag schema *schemaFlags strict bool -}{ - format: util.NewEnumFlag(checkFormatPretty, []string{ - checkFormatPretty, checkFormatJSON, - }), - capabilities: newcapabilitiesFlag(), - schema: &schemaFlags{}, +} + +func newCheckParams() checkParams { + return checkParams{ + format: util.NewEnumFlag(checkFormatPretty, []string{ + checkFormatPretty, checkFormatJSON, + }), + capabilities: newcapabilitiesFlag(), + schema: &schemaFlags{}, + } } const ( @@ -38,46 +42,34 @@ const ( checkFormatJSON = "json" ) -var checkCommand = &cobra.Command{ - Use: "check [path [...]]", - Short: "Check Rego source files", - Long: `Check Rego source files for parse and compilation errors. - -If the 'check' command succeeds in parsing and compiling the source file(s), no output -is produced. If the parsing or compiling fails, 'check' will output the errors -and exit with a non-zero exit code.`, - - PreRunE: func(Cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return fmt.Errorf("specify at least one file") - } - return nil - }, - - Run: func(cmd *cobra.Command, args []string) { - os.Exit(checkModules(args)) - }, -} - -func checkModules(args []string) int { +func checkModules(params checkParams, args []string) error { modules := map[string]*ast.Module{} - ss, err := loader.Schemas(checkParams.schema.path) - if err != nil { - outputErrors(err) - return 1 + var capabilities *ast.Capabilities + // if capabilities are not provided as a cmd flag, + // then ast.CapabilitiesForThisVersion must be called + // within checkModules to ensure custom builtins are properly captured + if params.capabilities.C != nil { + capabilities = params.capabilities.C + } else { + capabilities = ast.CapabilitiesForThisVersion() } - if checkParams.bundleMode { + ss, err := loader.Schemas(params.schema.path) + if err != nil { + return err + } + + if params.bundleMode { for _, path := range args { b, err := loader.NewFileLoader(). WithSkipBundleVerification(true). WithProcessAnnotation(ss != nil). + WithCapabilities(capabilities). AsBundle(path) if err != nil { - outputErrors(err) - return 1 + return err } for name, mod := range b.ParsedModules(path) { modules[name] = mod @@ -85,49 +77,37 @@ func checkModules(args []string) int { } } else { f := loaderFilter{ - Ignore: checkParams.ignore, + Ignore: params.ignore, } result, err := loader.NewFileLoader(). WithProcessAnnotation(ss != nil). + WithCapabilities(capabilities). Filtered(args, f.Apply) if err != nil { - outputErrors(err) - return 1 + return err } for _, m := range result.Modules { modules[m.Name] = m.Parsed } } - var capabilities *ast.Capabilities - // if capabilities are not provided as a cmd flag, - // then ast.CapabilitiesForThisVersion must be called - // within checkModules to ensure custom builtins are properly captured - if checkParams.capabilities.C != nil { - capabilities = checkParams.capabilities.C - } else { - capabilities = ast.CapabilitiesForThisVersion() - } + compiler := ast.NewCompiler(). - SetErrorLimit(checkParams.errLimit). + SetErrorLimit(params.errLimit). WithCapabilities(capabilities). WithSchemas(ss). WithEnablePrintStatements(true). - WithStrict(checkParams.strict) + WithStrict(params.strict) compiler.Compile(modules) - - if !compiler.Failed() { - return 0 + if compiler.Failed() { + return compiler.Errors } - - outputErrors(compiler.Errors) - - return 1 + return nil } -func outputErrors(err error) { +func outputErrors(format string, err error) { var out io.Writer if err != nil { out = os.Stderr @@ -135,7 +115,7 @@ func outputErrors(err error) { out = os.Stdout } - switch checkParams.format.String() { + switch format { case checkFormatJSON: result := pr.Output{ Errors: pr.NewOutputErrors(err), @@ -150,6 +130,32 @@ func outputErrors(err error) { } func init() { + checkParams := newCheckParams() + + checkCommand := &cobra.Command{ + Use: "check [path [...]]", + Short: "Check Rego source files", + Long: `Check Rego source files for parse and compilation errors. + + If the 'check' command succeeds in parsing and compiling the source file(s), no output + is produced. If the parsing or compiling fails, 'check' will output the errors + and exit with a non-zero exit code.`, + + PreRunE: func(_ *cobra.Command, args []string) error { + if len(args) == 0 { + return fmt.Errorf("specify at least one file") + } + return nil + }, + + Run: func(_ *cobra.Command, args []string) { + if err := checkModules(checkParams, args); err != nil { + outputErrors(checkParams.format.String(), err) + os.Exit(1) + } + }, + } + addMaxErrorsFlag(checkCommand.Flags(), &checkParams.errLimit) addIgnoreFlag(checkCommand.Flags(), &checkParams.ignore) checkCommand.Flags().VarP(checkParams.format, "format", "f", "set output format") diff --git a/cmd/check_test.go b/cmd/check_test.go new file mode 100644 index 0000000000..1ef05e34e1 --- /dev/null +++ b/cmd/check_test.go @@ -0,0 +1,122 @@ +// Copyright 2022 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +package cmd + +import ( + "encoding/json" + "path" + "strings" + "testing" + + "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/util/test" +) + +func TestCheckRespectsCapabilities(t *testing.T) { + tests := []struct { + note string + caps string + policy string + err string + bundleMode bool // check with "-b" flag + }{ + { + note: "builtin defined in caps", + caps: `{ + "builtins": [ + { + "name": "is_foo", + "decl": { + "args": [ + { + "type": "string" + } + ], + "result": { + "type": "boolean" + }, + "type": "function" + } + } + ] + }`, + policy: `package test +p { is_foo("bar") }`, + }, + { + note: "future kw NOT defined in caps", + caps: func() string { + c := ast.CapabilitiesForThisVersion() + c.FutureKeywords = []string{"in"} + j, err := json.Marshal(c) + if err != nil { + panic(err) + } + return string(j) + }(), + policy: `package test +import future.keywords.if +import future.keywords.in +p if "opa" in input.tools`, + err: "rego_parse_error: unexpected keyword, must be one of [in]", + }, + { + note: "future kw are defined in caps", + caps: func() string { + c := ast.CapabilitiesForThisVersion() + c.FutureKeywords = []string{"in", "if"} + j, err := json.Marshal(c) + if err != nil { + panic(err) + } + return string(j) + }(), + policy: `package test +import future.keywords.if +import future.keywords.in +p if "opa" in input.tools`, + }, + } + + // add same tests for bundle-mode == true: + for i := range tests { + tc := tests[i] + tc.bundleMode = true + tc.note = tc.note + " (as bundle)" + tests = append(tests, tc) + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "capabilities.json": tc.caps, + "test.rego": tc.policy, + } + + test.WithTempFS(files, func(root string) { + caps := newcapabilitiesFlag() + if err := caps.Set(path.Join(root, "capabilities.json")); err != nil { + t.Fatal(err) + } + params := newCheckParams() + params.capabilities = caps + params.bundleMode = tc.bundleMode + + err := checkModules(params, []string{root}) + switch { + case err != nil && tc.err != "": + if !strings.Contains(err.Error(), tc.err) { + t.Fatalf("expected err %v, got %v", tc.err, err) + } + return // don't read back bundle below + case err != nil && tc.err == "": + t.Fatalf("unexpected error: %v", err) + case err == nil && tc.err != "": + t.Fatalf("expected error %v, got nil", tc.err) + } + }) + }) + } +} diff --git a/cmd/parse.go b/cmd/parse.go index 5ab073b895..28c37ae343 100644 --- a/cmd/parse.go +++ b/cmd/parse.go @@ -39,7 +39,7 @@ var parseCommand = &cobra.Command{ } return nil }, - Run: func(cmd *cobra.Command, args []string) { + Run: func(_ *cobra.Command, args []string) { os.Exit(parse(args, os.Stdout, os.Stderr)) }, } diff --git a/compile/compile.go b/compile/compile.go index 86e0002377..eab5141972 100644 --- a/compile/compile.go +++ b/compile/compile.go @@ -409,7 +409,7 @@ func (c *Compiler) initBundle() error { // TODO(tsandall): the metrics object should passed through here so we that // we can track read and parse times. - load, err := initload.LoadPaths(c.paths, c.filter, c.asBundle, c.bvc, false, c.useRegoAnnotationEntrypoints) + load, err := initload.LoadPaths(c.paths, c.filter, c.asBundle, c.bvc, false, c.useRegoAnnotationEntrypoints, c.capabilities) if err != nil { return fmt.Errorf("load error: %w", err) } diff --git a/internal/runtime/init/init.go b/internal/runtime/init/init.go index 20ad91428a..e91bc9d45d 100644 --- a/internal/runtime/init/init.go +++ b/internal/runtime/init/init.go @@ -113,7 +113,17 @@ type Descriptor struct { // LoadPaths reads data and policy from the given paths and returns a set of bundles or // raw loader file results. -func LoadPaths(paths []string, filter loader.Filter, asBundle bool, bvc *bundle.VerificationConfig, skipVerify bool, processAnnotations bool) (*LoadPathsResult, error) { +func LoadPaths(paths []string, + filter loader.Filter, + asBundle bool, + bvc *bundle.VerificationConfig, + skipVerify bool, + processAnnotations bool, + caps *ast.Capabilities) (*LoadPathsResult, error) { + + if caps == nil { + caps = ast.CapabilitiesForThisVersion() + } var result LoadPathsResult var err error @@ -126,6 +136,7 @@ func LoadPaths(paths []string, filter loader.Filter, asBundle bool, bvc *bundle. WithSkipBundleVerification(skipVerify). WithFilter(filter). WithProcessAnnotation(processAnnotations). + WithCapabilities(caps). AsBundle(path) if err != nil { return nil, err @@ -136,6 +147,7 @@ func LoadPaths(paths []string, filter loader.Filter, asBundle bool, bvc *bundle. files, err := loader.NewFileLoader(). WithProcessAnnotation(processAnnotations). + WithCapabilities(caps). Filtered(paths, filter) if err != nil { return nil, err diff --git a/internal/runtime/init/init_test.go b/internal/runtime/init/init_test.go index d42b7d455c..6310906f08 100644 --- a/internal/runtime/init/init_test.go +++ b/internal/runtime/init/init_test.go @@ -14,7 +14,6 @@ import ( "testing" "github.com/open-policy-agent/opa/loader" - "github.com/open-policy-agent/opa/storage" inmem "github.com/open-policy-agent/opa/storage/inmem/test" "github.com/open-policy-agent/opa/util" @@ -121,7 +120,7 @@ p = true { 1 = 2 }` err := storage.Txn(ctx, store, storage.WriteParams, func(txn storage.Transaction) error { - loaded, err := LoadPaths(paths, nil, tc.asBundle, nil, true, false) + loaded, err := LoadPaths(paths, nil, tc.asBundle, nil, true, false, nil) if err != nil { return err } @@ -270,7 +269,7 @@ func TestLoadPathsBundleModeWithFilter(t *testing.T) { // bundle mode loaded, err := LoadPaths(paths, func(abspath string, info os.FileInfo, depth int) bool { return loader.GlobExcludeName("*_test.rego", 1)(abspath, info, depth) - }, true, nil, true, false) + }, true, nil, true, false, nil) if err != nil { t.Fatalf("Unexpected error: %s", err) } diff --git a/loader/loader.go b/loader/loader.go index 332b5c065a..1c2a04d06b 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -91,12 +91,13 @@ type FileLoader interface { All(paths []string) (*Result, error) Filtered(paths []string, filter Filter) (*Result, error) AsBundle(path string) (*bundle.Bundle, error) - WithFS(fsys fs.FS) FileLoader - WithMetrics(m metrics.Metrics) FileLoader - WithFilter(filter Filter) FileLoader + WithFS(fs.FS) FileLoader + WithMetrics(metrics.Metrics) FileLoader + WithFilter(Filter) FileLoader WithBundleVerificationConfig(*bundle.VerificationConfig) FileLoader - WithSkipBundleVerification(skipVerify bool) FileLoader - WithProcessAnnotation(processAnnotation bool) FileLoader + WithSkipBundleVerification(bool) FileLoader + WithProcessAnnotation(bool) FileLoader + WithCapabilities(*ast.Capabilities) FileLoader } // NewFileLoader returns a new FileLoader instance. @@ -155,6 +156,12 @@ func (fl *fileLoader) WithProcessAnnotation(processAnnotation bool) FileLoader { return fl } +// WithCapabilities sets the supported capabilities when loading the files +func (fl *fileLoader) WithCapabilities(caps *ast.Capabilities) FileLoader { + fl.opts.Capabilities = caps + return fl +} + // All returns a Result object loaded (recursively) from the specified paths. func (fl fileLoader) All(paths []string) (*Result, error) { return fl.Filtered(paths, nil) @@ -214,7 +221,8 @@ func (fl fileLoader) AsBundle(path string) (*bundle.Bundle, error) { WithMetrics(fl.metrics). WithBundleVerificationConfig(fl.bvc). WithSkipBundleVerification(fl.skipVerify). - WithProcessAnnotations(fl.opts.ProcessAnnotation) + WithProcessAnnotations(fl.opts.ProcessAnnotation). + WithCapabilities(fl.opts.Capabilities) // For bundle directories add the full path in front of module file names // to simplify debugging. diff --git a/runtime/runtime.go b/runtime/runtime.go index 4e18c9a269..7f41715cbb 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -298,7 +298,7 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { } } - loaded, err := initload.LoadPaths(params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, false) + loaded, err := initload.LoadPaths(params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, false, nil) if err != nil { return nil, fmt.Errorf("load error: %w", err) } @@ -716,7 +716,7 @@ func (rt *Runtime) readWatcher(ctx context.Context, watcher *fsnotify.Watcher, p func (rt *Runtime) processWatcherUpdate(ctx context.Context, paths []string, removed string) error { - loaded, err := initload.LoadPaths(paths, rt.Params.Filter, rt.Params.BundleMode, nil, true, false) + loaded, err := initload.LoadPaths(paths, rt.Params.Filter, rt.Params.BundleMode, nil, true, false, nil) if err != nil { return err }