ewilliams28/releases
1
0
Fork 0
mirror of https://github.com/open-policy-agent/opa.git synced 2026-08-12 19:32:48 -06:00
Code Issues Packages Projects Releases Wiki Activity

cmd/{build,check}: respect capabilities for parsing (#5326)

Browse Source 
Before, the capabilities were plumbled through in most places:

1. checking which builtins exist
2. passed along to the optimizer
3. passed along to the planner

But they hadn't been passed along to the file loader. As such, it could not
pass the caps along to the parser either. This is now done, but adding a new
method to the FileLoader interface.

Fixes #5323.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
This commit is contained in:
Stephan Renatus
2022-11-01 18:50:03 +01:00
committed by GitHub
parent c2c9e536fc
commit d238828776
10 changed files with 344 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
bundle/bundle.go
+15 -1
View File
@@ -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
cmd/build_test.go
+109 -42
View File
@@ -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) {
cmd/check.go
+65 -59
View File
@@ -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> [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> [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")
cmd/check_test.go
+122
View File
@@ -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)
}
})
})
}
}
cmd/parse.go
+1 -1
View File
@@ -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))
},
}
compile/compile.go
+1 -1
View File
@@ -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)
}
internal/runtime/init/init.go
+13 -1
View File
@@ -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
internal/runtime/init/init_test.go
+2 -3
View File
@@ -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)
}
loader/loader.go
+14 -6
View File
@@ -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.
runtime/runtime.go
+2 -2
View File
@@ -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
}