diff --git a/ast/parser_ext.go b/ast/parser_ext.go index 413416da29..5103196a58 100644 --- a/ast/parser_ext.go +++ b/ast/parser_ext.go @@ -698,22 +698,7 @@ func parseModule(filename string, stmts []Statement, comments []*Comment, regoCo if mod.regoVersion == RegoV0CompatV1 || mod.regoVersion == RegoV1 { for _, rule := range mod.Rules { for r := rule; r != nil; r = r.Else { - var t string - if r.isFunction() { - t = "function" - } else { - t = "rule" - } - - if r.generatedBody && r.Head.generatedValue { - errs = append(errs, NewError(ParseErr, r.Location, "%s must have value assignment and/or body declaration", t)) - } - if r.Body != nil && !r.generatedBody && !ruleDeclarationHasKeyword(r, tokens.If) && !r.Default { - errs = append(errs, NewError(ParseErr, r.Location, "`if` keyword is required before %s body", t)) - } - if r.Head.RuleKind() == MultiValue && !ruleDeclarationHasKeyword(r, tokens.Contains) { - errs = append(errs, NewError(ParseErr, r.Location, "`contains` keyword is required for partial set rules")) - } + errs = append(errs, CheckRegoV1(r)...) } } } diff --git a/ast/rego_v1.go b/ast/rego_v1.go index 142a884488..ea3e907d70 100644 --- a/ast/rego_v1.go +++ b/ast/rego_v1.go @@ -1,5 +1,11 @@ package ast +import ( + "fmt" + + "github.com/open-policy-agent/opa/ast/internal/tokens" +) + func checkDuplicateImports(modules []*Module) (errors Errors) { for _, module := range modules { processedImports := map[Var]*Import{} @@ -116,11 +122,43 @@ func checkDeprecatedBuiltinsForCurrentVersion(node interface{}) Errors { return checkDeprecatedBuiltins(deprecatedBuiltins, node) } -// CheckRegoV1 checks the given module for errors that are specific to Rego v1 -func CheckRegoV1(module *Module) Errors { +// CheckRegoV1 checks the given module or rule for errors that are specific to Rego v1. +// Passing something other than an *ast.Rule or *ast.Module is considered a programming error, and will cause a panic. +func CheckRegoV1(x interface{}) Errors { + switch x := x.(type) { + case *Module: + return checkRegoV1Module(x) + case *Rule: + return checkRegoV1Rule(x) + } + panic(fmt.Sprintf("cannot check rego-v1 compatibility on type %T", x)) +} + +func checkRegoV1Module(module *Module) Errors { var errors Errors errors = append(errors, checkDuplicateImports([]*Module{module})...) errors = append(errors, checkRootDocumentOverrides(module)...) errors = append(errors, checkDeprecatedBuiltinsForCurrentVersion(module)...) return errors } + +func checkRegoV1Rule(rule *Rule) Errors { + t := "rule" + if rule.isFunction() { + t = "function" + } + + var errs Errors + + if rule.generatedBody && rule.Head.generatedValue { + errs = append(errs, NewError(ParseErr, rule.Location, "%s must have value assignment and/or body declaration", t)) + } + if rule.Body != nil && !rule.generatedBody && !ruleDeclarationHasKeyword(rule, tokens.If) && !rule.Default { + errs = append(errs, NewError(ParseErr, rule.Location, "`if` keyword is required before %s body", t)) + } + if rule.Head.RuleKind() == MultiValue && !ruleDeclarationHasKeyword(rule, tokens.Contains) { + errs = append(errs, NewError(ParseErr, rule.Location, "`contains` keyword is required for partial set rules")) + } + + return errs +} diff --git a/cmd/bench.go b/cmd/bench.go index 210597ce17..2d6ba62198 100644 --- a/cmd/bench.go +++ b/cmd/bench.go @@ -120,6 +120,7 @@ The optional "gobench" output format conforms to the Go Benchmark Data Format. addIgnoreFlag(benchCommand.Flags(), ¶ms.ignore) addSchemaFlags(benchCommand.Flags(), params.schema) addTargetFlag(benchCommand.Flags(), params.target) + addV1CompatibleFlag(benchCommand.Flags(), ¶ms.v1Compatible, false) // Shared benchmark flags addCountFlag(benchCommand.Flags(), ¶ms.count, "benchmark") @@ -299,6 +300,7 @@ func benchE2E(ctx context.Context, args []string, params benchmarkCommandParams, GracefulShutdownPeriod: params.gracefulShutdownPeriod, ShutdownWaitPeriod: params.shutdownWaitPeriod, ConfigFile: params.configFile, + V1Compatible: params.v1Compatible, } rt, err := runtime.NewRuntime(ctx, rtParams) diff --git a/cmd/bench_test.go b/cmd/bench_test.go index fcd80a7359..f09e66c74d 100644 --- a/cmd/bench_test.go +++ b/cmd/bench_test.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "errors" + "fmt" "os" "path/filepath" "strings" @@ -691,6 +692,122 @@ func TestBenchMainBadQueryE2E(t *testing.T) { } } +func TestBenchMainV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + query string + expErrs []string + }{ + // These tests are slow, so we're not being completely exhaustive here. + { + note: "v0.x, keywords not used", + module: `package test +a[4] { + 1 == 1 +}`, + query: `data.test.a`, + }, + { + note: "v0.x, no keywords imported", + module: `package test +a contains 4 if { + 1 == 1 +}`, + query: `data.test.a`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v1.0, keywords not used", + v1Compatible: true, + module: `package test +a[4] { + 1 == 1 +}`, + query: `data.test.a`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, no keywords imported", + v1Compatible: true, + module: `package test +a contains 4 if { + 1 == 1 +}`, + query: `data.test.a`, + }, + } + + modes := []struct { + name string + e2e bool + }{ + { + name: "run", + }, + { + name: "e2e", + e2e: true, + }, + } + + for _, mode := range modes { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s", tc.note, mode.name), func(t *testing.T) { + files := map[string]string{ + "mod.rego": tc.module, + } + + test.WithTempFS(files, func(path string) { + params := testBenchParams() + _ = params.outputFormat.Set(evalPrettyOutput) + params.v1Compatible = tc.v1Compatible + params.e2e = mode.e2e + + for n := range files { + err := params.dataPaths.Set(filepath.Join(path, n)) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + } + + args := []string{tc.query} + + var buf bytes.Buffer + rc, err := benchMain(args, params, &buf, &goBenchRunner{}) + + if len(tc.expErrs) > 0 { + if rc == 0 { + t.Fatalf("Expected non-zero return code") + } + + output := buf.String() + for _, expErr := range tc.expErrs { + if !strings.Contains(output, expErr) { + t.Fatalf("Expected error:\n\n%s\n\ngot:\n\n%s", expErr, output) + } + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + if rc != 0 { + t.Fatalf("Unexpected return code %d, expected 0", rc) + } + } + }) + }) + } + } +} + func TestRenderBenchmarkResultJSONOutput(t *testing.T) { params := testBenchParams() err := params.outputFormat.Set(evalJSONOutput) diff --git a/cmd/deps.go b/cmd/deps.go index 43fdd1d042..5aef2260a2 100644 --- a/cmd/deps.go +++ b/cmd/deps.go @@ -7,6 +7,7 @@ package cmd import ( "errors" "fmt" + "io" "os" "github.com/open-policy-agent/opa/dependencies" @@ -24,6 +25,14 @@ type depsCommandParams struct { outputFormat *util.EnumFlag ignore []string bundlePaths repeatedStringFlag + v1Compatible bool +} + +func (p *depsCommandParams) regoVersion() ast.RegoVersion { + if p.v1Compatible { + return ast.RegoV1 + } + return ast.RegoV0 } const ( @@ -31,14 +40,20 @@ const ( depsFormatJSON = "json" ) -func init() { - +func newDepsCommandParams() depsCommandParams { var params depsCommandParams params.outputFormat = util.NewEnumFlag(depsFormatPretty, []string{ depsFormatPretty, depsFormatJSON, }) + return params +} + +func init() { + + params := newDepsCommandParams() + depsCommand := &cobra.Command{ Use: "deps ", Short: "Analyze Rego query dependencies", @@ -81,7 +96,7 @@ data.policy.is_admin. return nil }, Run: func(cmd *cobra.Command, args []string) { - if err := deps(args, params); err != nil { + if err := deps(args, params, os.Stdout); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) } @@ -92,11 +107,12 @@ data.policy.is_admin. addDataFlag(depsCommand.Flags(), ¶ms.dataPaths) addBundleFlag(depsCommand.Flags(), ¶ms.bundlePaths) addOutputFormat(depsCommand.Flags(), params.outputFormat) + addV1CompatibleFlag(depsCommand.Flags(), ¶ms.v1Compatible, false) RootCommand.AddCommand(depsCommand) } -func deps(args []string, params depsCommandParams) error { +func deps(args []string, params depsCommandParams, w io.Writer) error { query, err := ast.ParseBody(args[0]) if err != nil { @@ -110,7 +126,9 @@ func deps(args []string, params depsCommandParams) error { Ignore: params.ignore, } - result, err := loader.NewFileLoader().Filtered(params.dataPaths.v, f.Apply) + result, err := loader.NewFileLoader(). + WithRegoVersion(params.regoVersion()). + Filtered(params.dataPaths.v, f.Apply) if err != nil { return err } @@ -157,8 +175,8 @@ func deps(args []string, params depsCommandParams) error { switch params.outputFormat.String() { case depsFormatJSON: - return presentation.JSON(os.Stdout, output) + return presentation.JSON(w, output) default: - return output.Pretty(os.Stdout) + return output.Pretty(w) } } diff --git a/cmd/deps_test.go b/cmd/deps_test.go new file mode 100644 index 0000000000..7fef1f8060 --- /dev/null +++ b/cmd/deps_test.go @@ -0,0 +1,140 @@ +// Copyright 2024 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 ( + "io" + "path/filepath" + "strings" + "testing" + + "github.com/open-policy-agent/opa/util/test" +) + +func TestDepsV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + query string + expErrs []string + }{ + { + note: "v0.x, no keywords", + module: `package test +p[3] { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v0.x, keywords not imported, but used", + module: `package test +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + module: `package test +import future.keywords +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v0.x, rego.v1 imported", + module: `package test +import rego.v1 +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v1.0, no keywords", + v1Compatible: true, + module: `package test +p[3] { + input.x = 1 +}`, + query: `data.test.p`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, no keyword imports", + v1Compatible: true, + module: `package test +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + module: `package test +import future.keywords +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + module: `package test +import rego.v1 +p contains 3 if { + input.x = 1 +}`, + query: `data.test.p`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "test.rego": tc.module, + } + + test.WithTempFS(files, func(rootPath string) { + params := newDepsCommandParams() + params.v1Compatible = tc.v1Compatible + _ = params.outputFormat.Set(depsFormatPretty) + + for f := range files { + _ = params.dataPaths.Set(filepath.Join(rootPath, f)) + } + + err := deps([]string{tc.query}, params, io.Discard) + + if len(tc.expErrs) > 0 { + if err == nil { + t.Fatalf("Expected error but got nil") + } + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("Expected error:\n\n%s\n\ngot:\n\n%s", expErr, err.Error()) + } + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + }) + }) + } +} diff --git a/cmd/exec.go b/cmd/exec.go index 2197af50c0..82f8b6a7c2 100644 --- a/cmd/exec.go +++ b/cmd/exec.go @@ -74,23 +74,31 @@ e.g., opa exec --decision /foo/bar/baz ...`, cmd.Flags().VarP(params.LogLevel, "log-level", "l", "set log level") cmd.Flags().Var(params.LogFormat, "log-format", "set log format") cmd.Flags().StringVar(¶ms.LogTimestampFormat, "log-timestamp-format", "", "set log timestamp format (OPA_LOG_TIMESTAMP_FORMAT environment variable)") + addV1CompatibleFlag(cmd.Flags(), ¶ms.V1Compatible, false) RootCommand.AddCommand(cmd) } func runExec(params *exec.Params) error { + return runExecWithContext(context.Background(), params) +} + +func runExecWithContext(ctx context.Context, params *exec.Params) error { stdLogger, consoleLogger, err := setupLogging(params.LogLevel.String(), params.LogFormat.String(), params.LogTimestampFormat) if err != nil { return fmt.Errorf("config error: %w", err) } + if params.Logger != nil { + stdLogger = params.Logger + } + config, err := setupConfig(params.ConfigFile, params.ConfigOverrides, params.ConfigOverrideFiles, params.BundlePaths) if err != nil { return fmt.Errorf("config error: %w", err) } - ctx := context.Background() ready := make(chan struct{}) opa, err := sdk.New(ctx, sdk.Options{ @@ -98,6 +106,7 @@ func runExec(params *exec.Params) error { Logger: stdLogger, ConsoleLogger: consoleLogger, Ready: ready, + V1Compatible: params.V1Compatible, }) if err != nil { return fmt.Errorf("runtime error: %w", err) diff --git a/cmd/exec_test.go b/cmd/exec_test.go index e6858bde61..8884708c67 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -4,9 +4,12 @@ import ( "bytes" "context" "reflect" + "strings" "testing" + "time" "github.com/open-policy-agent/opa/cmd/internal/exec" + loggingtest "github.com/open-policy-agent/opa/logging/test" sdk_test "github.com/open-policy-agent/opa/sdk/test" "github.com/open-policy-agent/opa/util" "github.com/open-policy-agent/opa/util/test" @@ -149,6 +152,166 @@ func TestExecBundleFlag(t *testing.T) { }) } +func TestExecV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x, no keywords used", + module: `package system +main["hello"] { + input.foo == "bar" +}`, + }, + { + note: "v0.x, no keywords imported", + module: `package system +main contains "hello" if { + input.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: string cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + module: `package system +import future.keywords +main contains "hello" if { + input.foo == "bar" +}`, + }, + { + note: "v0.x, rego.v1 imported", + module: `package system +import rego.v1 +main contains "hello" if { + input.foo == "bar" +}`, + }, + + { + note: "v1.0, no keywords used", + v1Compatible: true, + module: `package system +main["hello"] { + input.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, no keywords imported", + v1Compatible: true, + module: `package system +main contains "hello" if { + input.foo == "bar" +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + module: `package system +import future.keywords +main contains "hello" if { + input.foo == "bar" +}`, + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + module: `package system +import rego.v1 +main contains "hello" if { + input.foo == "bar" +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "test.json": `{"foo": "bar"}`, + } + + test.WithTempFS(files, func(dir string) { + s := sdk_test.MustNewServer( + sdk_test.MockBundle("/bundles/bundle.tar.gz", map[string]string{"test.rego": tc.module}), + sdk_test.RawBundles(true), + ) + + defer s.Stop() + + var buf bytes.Buffer + params := exec.NewParams(&buf) + params.V1Compatible = tc.v1Compatible + _ = params.OutputFormat.Set("json") + params.ConfigOverrides = []string{ + "services.test.url=" + s.URL(), + "bundles.test.resource=/bundles/bundle.tar.gz", + } + + params.Paths = append(params.Paths, dir) + + if len(tc.expErrs) > 0 { + testLogger := loggingtest.New() + params.Logger = testLogger + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + go func() { + err := runExecWithContext(ctx, params) + if err != nil { + t.Error(err) + return + } + }() + + if !test.Eventually(t, 5*time.Second, func() bool { + for _, expErr := range tc.expErrs { + found := false + for _, e := range testLogger.Entries() { + if strings.Contains(e.Message, expErr) { + found = true + break + } + } + if !found { + return false + } + } + return true + }) { + t.Fatalf("timed out waiting for logged errors:\n\n%v\n\ngot\n\n%v:", tc.expErrs, testLogger.Entries()) + } + } else { + err := runExec(params) + if err != nil { + t.Fatal(err) + } + + output := util.MustUnmarshalJSON(bytes.ReplaceAll(buf.Bytes(), []byte(dir), nil)) + + exp := util.MustUnmarshalJSON([]byte(`{"result": [{ + "path": "/test.json", + "result": ["hello"] + }]}`)) + + if !reflect.DeepEqual(output, exp) { + t.Fatal("Expected:", exp, "Got:", output) + } + } + }) + }) + } +} + func TestInvalidConfig(t *testing.T) { var buf bytes.Buffer params := exec.NewParams(&buf) diff --git a/cmd/inspect.go b/cmd/inspect.go index a23bc6c5b5..ead31d106f 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -29,6 +29,14 @@ const pageWidth = 80 type inspectCommandParams struct { outputFormat *util.EnumFlag listAnnotations bool + v1Compatible bool +} + +func (p *inspectCommandParams) regoVersion() ast.RegoVersion { + if p.v1Compatible { + return ast.RegoV1 + } + return ast.RegoV0 } func newInspectCommandParams() inspectCommandParams { @@ -83,11 +91,12 @@ referring to a directory, the 'inspect' command will load that path as a bundle addOutputFormat(inspectCommand.Flags(), params.outputFormat) addListAnnotations(inspectCommand.Flags(), ¶ms.listAnnotations) + addV1CompatibleFlag(inspectCommand.Flags(), ¶ms.v1Compatible, false) RootCommand.AddCommand(inspectCommand) } func doInspect(params inspectCommandParams, path string, out io.Writer) error { - info, err := ib.File(path, params.listAnnotations) + info, err := ib.FileForRegoVersion(params.regoVersion(), path, params.listAnnotations) if err != nil { return err } diff --git a/cmd/inspect_test.go b/cmd/inspect_test.go index 7b442c877b..817d15b06d 100644 --- a/cmd/inspect_test.go +++ b/cmd/inspect_test.go @@ -566,6 +566,133 @@ Custom: }) } +func TestDoInspectV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x, keywords not used", + module: `package test +p[v] { + v := input.x +}`, + }, + { + note: "v0.x, no keywords imported, but used", + module: `package test +p contains v if { + v := input.x +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + module: `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + { + note: "v0.x, rego.v1 imported", + module: `package test +import rego.v1 +p contains v if { + v := input.x +}`, + }, + { + note: "v1.0, keywords not used", + v1Compatible: true, + module: `package test +p[v] { + v := input.x +}`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, no keywords imported", + v1Compatible: true, + module: `package test +p contains v if { + v := input.x +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + module: `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + module: `package test +import rego.v1 +p contains v if { + v := input.x +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + test.WithTempFS(nil, func(rootDir string) { + buf := archive.MustWriteTarGz([][2]string{{"/policy.rego", tc.module}}) + + bundleFile := filepath.Join(rootDir, "bundle.tar.gz") + + bf, err := os.Create(bundleFile) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + var out bytes.Buffer + params := newInspectCommandParams() + params.v1Compatible = tc.v1Compatible + err = params.outputFormat.Set(evalJSONOutput) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + err = doInspect(params, bundleFile, &out) + + if len(tc.expErrs) > 0 { + if err == nil { + t.Fatalf("Expected error but got nil") + } + + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("Expected error:\n\n%v\n\nbut got:\n\n%v", expErr, err.Error()) + } + } + } else { + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + } + }) + }) + } +} + func TestCallToUnknownBuiltInFunction(t *testing.T) { files := [][2]string{ {"/policy.rego", `package test diff --git a/cmd/internal/exec/exec.go b/cmd/internal/exec/exec.go index a8490fdc1e..c447112bcf 100644 --- a/cmd/internal/exec/exec.go +++ b/cmd/internal/exec/exec.go @@ -11,6 +11,7 @@ import ( "path/filepath" "time" + "github.com/open-policy-agent/opa/logging" "github.com/open-policy-agent/opa/sdk" "github.com/open-policy-agent/opa/util" ) @@ -30,6 +31,8 @@ type Params struct { Fail bool // exits with non-zero exit code on undefined policy decision or empty policy decision result or other errors FailDefined bool // exits with non-zero exit code on 'not undefined policy decisiondefined' or 'not empty policy decision result' or other errors FailNonEmpty bool // exits with non-zero exit code on non-empty set (array) results + V1Compatible bool // use OPA 1.0 compatibility mode + Logger logging.Logger // Logger override. If set to nil, the default logger is used. } func NewParams(w io.Writer) *Params { diff --git a/cmd/parse.go b/cmd/parse.go index 700794d550..2935aa6aaa 100644 --- a/cmd/parse.go +++ b/cmd/parse.go @@ -26,8 +26,16 @@ const ( ) type parseParams struct { - format *util.EnumFlag - jsonInclude string + format *util.EnumFlag + jsonInclude string + v1Compatible bool +} + +func (p *parseParams) regoVersion() ast.RegoVersion { + if p.v1Compatible { + return ast.RegoV1 + } + return ast.RegoV0 } var configuredParseParams = parseParams{ @@ -71,7 +79,10 @@ func parse(args []string, params *parseParams, stdout io.Writer, stderr io.Write } } - parserOpts := ast.ParserOptions{ProcessAnnotation: true} + parserOpts := ast.ParserOptions{ + ProcessAnnotation: true, + RegoVersion: params.regoVersion(), + } if exposeLocation { parserOpts.JSONOptions = &astJSON.Options{ MarshalOptions: astJSON.MarshalOptions{ @@ -112,10 +123,10 @@ func parse(args []string, params *parseParams, stdout io.Writer, stderr io.Write return 1 } - fmt.Fprint(stdout, string(bs)+"\n") + _, _ = fmt.Fprint(stdout, string(bs)+"\n") default: if err != nil { - fmt.Fprintln(stderr, err) + _, _ = fmt.Fprintln(stderr, err) return 1 } ast.Pretty(stdout, result.Parsed) @@ -127,6 +138,7 @@ func parse(args []string, params *parseParams, stdout io.Writer, stderr io.Write func init() { parseCommand.Flags().VarP(configuredParseParams.format, "format", "f", "set output format") parseCommand.Flags().StringVarP(&configuredParseParams.jsonInclude, "json-include", "", "", "include or exclude optional elements. By default comments are included. Current options: locations, comments. E.g. --json-include locations,-comments will include locations and exclude comments.") + addV1CompatibleFlag(parseCommand.Flags(), &configuredParseParams.v1Compatible, false) RootCommand.AddCommand(parseCommand) } diff --git a/cmd/parse_test.go b/cmd/parse_test.go index b5371fbce4..5d3ff49cc4 100644 --- a/cmd/parse_test.go +++ b/cmd/parse_test.go @@ -930,6 +930,114 @@ func TestParseJSONOutputComments(t *testing.T) { } } +func TestParseV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + policy string + expErrs []string + }{ + { + note: "v0.x, keywords not used", + policy: `package test +p[v] { + v := input.x +}`, + }, + { + note: "v0.x, keywords not imported", + policy: `package test +p contains v if { + v := input.x +}`, + expErrs: []string{ + "var cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + policy: `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + { + note: "v0.x, rego.v1 imported", + policy: `package test +import rego.v1 +p contains v if { + v := input.x +}`, + }, + + { + note: "v1.0, keywords not used", + v1Compatible: true, + policy: `package test +p[v] { + v := input.x +}`, + expErrs: []string{ + "`if` keyword is required before rule body", + "`contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, keywords not imported", + v1Compatible: true, + policy: `package test +p contains v if { + v := input.x +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + policy: `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + policy: `package test +import rego.v1 +p contains v if { + v := input.x +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + files := map[string]string{ + "policy.rego": tc.policy, + } + + _, _, stderr, _ := testParse(t, files, &parseParams{ + format: util.NewEnumFlag(parseFormatPretty, []string{parseFormatPretty, parseFormatJSON}), + v1Compatible: tc.v1Compatible, + }) + + if len(tc.expErrs) > 0 { + errs := string(stderr) + for _, expErr := range tc.expErrs { + if !strings.Contains(errs, expErr) { + t.Fatalf("Expected error:\n\n%q\n\ngot:\n\n%s", expErr, errs) + } + } + } else { + if len(stderr) > 0 { + t.Fatalf("Expected no stderr output, got:\n%s\n", string(stderr)) + } + } + }) + } +} + // Runs parse and returns the exit code, stdout, and stderr contents func testParse(t *testing.T, files map[string]string, params *parseParams) (int, []byte, []byte, string) { t.Helper() diff --git a/cmd/test_test.go b/cmd/test_test.go index c6731fe0f3..70411bf8e6 100644 --- a/cmd/test_test.go +++ b/cmd/test_test.go @@ -10,7 +10,6 @@ import ( "path/filepath" "regexp" "strings" - "sync" "syscall" "testing" "time" @@ -438,7 +437,7 @@ func TestWatchMode(t *testing.T) { } test.WithTempFS(files, func(root string) { - buf := blockingWriter{} + buf := test.BlockingWriter{} testParams := newTestCommandParams() testParams.output = &buf @@ -539,7 +538,7 @@ func TestWatchModeWithDataFile(t *testing.T) { } test.WithTempFS(files, func(root string) { - buf := blockingWriter{} + buf := test.BlockingWriter{} testParams := newTestCommandParams() testParams.output = &buf @@ -619,7 +618,7 @@ func TestWatchModeWhenDataFileRemoved(t *testing.T) { } test.WithTempFS(files, func(root string) { - buf := blockingWriter{} + buf := test.BlockingWriter{} testParams := newTestCommandParams() testParams.output = &buf @@ -737,7 +736,7 @@ Watching for changes ...`, for _, tc := range tests { t.Run(tc.note, func(t *testing.T) { test.WithTempFS(files, func(root string) { - buf := blockingWriter{} + buf := test.BlockingWriter{} testParams := newTestCommandParams() testParams.output = &buf @@ -1214,26 +1213,3 @@ test_l if { } } } - -type blockingWriter struct { - m sync.Mutex - buf bytes.Buffer -} - -func (w *blockingWriter) Write(p []byte) (n int, err error) { - w.m.Lock() - defer w.m.Unlock() - return w.buf.Write(p) -} - -func (w *blockingWriter) String() string { - w.m.Lock() - defer w.m.Unlock() - return w.buf.String() -} - -func (w *blockingWriter) Reset() { - w.m.Lock() - defer w.m.Unlock() - w.buf.Reset() -} diff --git a/docs/content/opa-1.md b/docs/content/opa-1.md index 545ff53d8e..41c70d06d8 100644 --- a/docs/content/opa-1.md +++ b/docs/content/opa-1.md @@ -226,13 +226,19 @@ OPA can be run in 1.0 compatibility mode by using the `--v1-compatible` flag. Wh The `--v1-compatible` flag is currently supported on the following commands: -* `build`: requires modules to be compatible with OPA v1.0 syntax -* `check`*: requires modules to be compatible with OPA v1.0 syntax +* `bench`: requires modules to be compatible with OPA v1.0 syntax. +* `build`: requires modules to be compatible with OPA v1.0 syntax. +* `deps`: requires modules to be compatible with OPA v1.0 syntax. +* `check`*: requires modules to be compatible with OPA v1.0 syntax. +* `eval`: requires modules to be compatible with OPA v1.0 syntax. +* `exec`: requires modules to be compatible with OPA v1.0 syntax. * `fmt`*: formats modules to be compatible with OPA v1.0 syntax, but not the current 0.x syntax. -* `eval`: requires modules to be compatible with OPA v1.0 syntax -* `test`: requires modules to be compatible with OPA v1.0 syntax +* `inspect`: requires modules to be compatible with OPA v1.0 syntax. +* `parse`: requires modules to be compatible with OPA v1.0 syntax. +* `run`: requires modules (including discovery bundle) to be compatible with OPA v1.0 syntax. Binds server listeners to the `localhost` interface by default. +* `test`: requires modules to be compatible with OPA v1.0 syntax. -Note: the `check` and `fmt` commands also support the `--rego-v1` flag, which will check/format Rego modules as if compatible with the Rego syntax of _both_ the current 0.x OPA version and OPA v1.0. +Note (*): the `check` and `fmt` commands also support the `--rego-v1` flag, which will check/format Rego modules as if compatible with the Rego syntax of _both_ the current 0.x OPA version and OPA v1.0. If both flags are used at the same time, `--rego-v1` takes precedence over `--v1-compatible`. {{< info >}} diff --git a/download/download.go b/download/download.go index f5b3c9c7c6..f3e8c94cb2 100644 --- a/download/download.go +++ b/download/download.go @@ -18,6 +18,7 @@ import ( "sync" "time" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/logging" "github.com/open-policy-agent/opa/metrics" @@ -65,6 +66,7 @@ type Downloader struct { longPollingEnabled bool lazyLoadingMode bool bundleName string + bundleParserOpts ast.ParserOptions } type downloaderResponse struct { @@ -134,6 +136,12 @@ func (d *Downloader) WithBundleName(bundleName string) *Downloader { return d } +// WithBundleParserOpts specifies the parser options to use when parsing downloaded bundles. +func (d *Downloader) WithBundleParserOpts(opts ast.ParserOptions) *Downloader { + d.bundleParserOpts = opts + return d +} + // ClearCache is deprecated. Use SetCache instead. func (d *Downloader) ClearCache() { d.etag = "" @@ -335,6 +343,7 @@ func (d *Downloader) download(ctx context.Context, m metrics.Metrics) (*download etag := resp.Header.Get("ETag") reader := bundle.NewCustomReader(loader). + WithRegoVersion(d.bundleParserOpts.RegoVersion). WithMetrics(m). WithBundleVerificationConfig(d.bvc). WithBundleEtag(etag). diff --git a/download/download_test.go b/download/download_test.go index f8508df33a..52f81b5fca 100644 --- a/download/download_test.go +++ b/download/download_test.go @@ -16,6 +16,7 @@ import ( "testing" "time" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/logging" "github.com/open-policy-agent/opa/logging/test" @@ -488,6 +489,106 @@ func TestOneShotWithBundleEtag(t *testing.T) { } } +func TestOneShotV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + bundlePath string + expErrs []string + }{ + { + note: "v0.x, keywords not used", + bundlePath: "/bundles/test/v1compat/keywords_not_used", + }, + { + note: "v0.x, keywords used, not imported", + bundlePath: "/bundles/test/v1compat/no_imports", + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords used, rego.v1 imported", + bundlePath: "/bundles/test/v1compat/rego_import", + }, + + { + note: "v1.0, keywords not used", + v1Compatible: true, + bundlePath: "/bundles/test/v1compat/keywords_not_used", + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, keywords used, not imported", + v1Compatible: true, + bundlePath: "/bundles/test/v1compat/no_imports", + }, + { + note: "v1.0, keywords used, rego.v1 imported", + v1Compatible: true, + bundlePath: "/bundles/test/v1compat/rego_import", + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + ctx := context.Background() + fixture := newTestFixture(t) + fixture.d = New(Config{}, fixture.client, tc.bundlePath). + WithBundleParserOpts(popts). + WithCallback(fixture.oneShot) + fixture.server.expEtag = "some etag value" + defer fixture.server.stop() + + // check etag on the downloader is empty + if fixture.d.etag != "" { + t.Fatalf("Expected empty downloader ETag but got %v", fixture.d.etag) + } + + // simulate successful bundle activation and check updated etag on the downloader + fixture.server.expCode = 0 + err := fixture.d.oneShot(ctx) + + if tc.expErrs != nil { + if err == nil { + t.Fatal("Expected error but got nil") + } + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("Expected error to contain:\n\n%v\n\nbut got\n\n%v", expErr, err) + } + } + } else { + if err != nil { + t.Fatal("Unexpected:", err) + } + + if fixture.d.etag != fixture.server.expEtag { + t.Fatalf("Expected downloader ETag %v but got %v", fixture.server.expEtag, fixture.d.etag) + } + + if fixture.updates[0].Bundle == nil { + // 200 response on first request, bundle should be present + t.Errorf("Expected bundle in response") + } + + if fixture.updates[0].Bundle.Etag != fixture.server.expEtag { + t.Fatalf("Expected bundle ETag %v but got %v", fixture.server.expEtag, fixture.updates[0].Bundle.Etag) + } + } + }) + } +} + func TestFailureAuthn(t *testing.T) { ctx := context.Background() diff --git a/download/oci_download.go b/download/oci_download.go index 8544df246f..2a3ac891d6 100644 --- a/download/oci_download.go +++ b/download/oci_download.go @@ -23,6 +23,7 @@ import ( oraslib "oras.land/oras-go/v2" "oras.land/oras-go/v2/content/oci" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/logging" "github.com/open-policy-agent/opa/metrics" @@ -80,6 +81,12 @@ func (d *OCIDownloader) WithBundlePersistence(persist bool) *OCIDownloader { return d } +// WithBundleParserOpts specifies the parser options to use when parsing downloaded bundles. +func (d *OCIDownloader) WithBundleParserOpts(opts ast.ParserOptions) *OCIDownloader { + d.bundleParserOpts = opts + return d +} + // ClearCache is deprecated. Use SetCache instead. func (d *OCIDownloader) ClearCache() { } @@ -256,7 +263,8 @@ func (d *OCIDownloader) download(ctx context.Context, m metrics.Metrics) (*downl reader := bundle.NewCustomReader(loader). WithMetrics(m). WithBundleVerificationConfig(d.bvc). - WithBundleEtag(etag) + WithBundleEtag(etag). + WithRegoVersion(d.bundleParserOpts.RegoVersion) bundleInfo, err := reader.Read() if err != nil { return &downloaderResponse{}, fmt.Errorf("unexpected error %w", err) diff --git a/download/oci_download_test.go b/download/oci_download_test.go index 2653cf021a..8e80bee7c3 100644 --- a/download/oci_download_test.go +++ b/download/oci_download_test.go @@ -7,6 +7,7 @@ import ( "context" "encoding/base64" "fmt" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "net/http" "strings" @@ -19,6 +20,7 @@ import ( // when changed the layer hash & size should be updated in signed.manifest //go:generate go run github.com/open-policy-agent/opa build -b --signing-alg HS256 --signing-key secret testdata/signed_bundle_data --output testdata/signed.tar.gz +//go:generate go run github.com/open-policy-agent/opa build --v1-compatible -b --signing-alg HS256 --signing-key secret testdata/rego_v1_bundle_data --output testdata/rego_v1.tar.gz func TestOCIDownloaderWithBundleVerificationConfig(t *testing.T) { vc := bundle.NewVerificationConfig(map[string]*bundle.KeyConfig{"default": {Key: "secret", Algorithm: "HS256"}}, "", "", nil) @@ -59,10 +61,85 @@ func TestOCIDownloaderWithBundleVerificationConfig(t *testing.T) { } +func TestOCIDownloaderWithRegoV1Bundle(t *testing.T) { + tests := []struct { + note string + regoVersion ast.RegoVersion + expErr string + }{ + { + note: "non-1.0 compatible OCI downloader", + expErr: "rego_parse_error", + }, + { + note: "1.0 compatible OCI downloader", + regoVersion: ast.RegoV1, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + vc := bundle.NewVerificationConfig(map[string]*bundle.KeyConfig{"default": {Key: "secret", Algorithm: "HS256"}}, "", "", nil) + ctx := context.Background() + fixture := newTestFixture(t) + fixture.server.expEtag = "sha256:c5834dbce332cabe6ae68a364de171a50bf5b08024c27d7c08cc72878b4df7ff" + + // We might get multiple updates, a buffered channel will make sure we save the first one. + updates := make(chan *Update, 1) + + config := Config{} + if err := config.ValidateAndInjectDefaults(); err != nil { + t.Fatal(err) + } + + d := NewOCI(config, fixture.client, "ghcr.io/org/repo:rego_v1", "/tmp/opa/"). + WithBundleParserOpts(ast.ParserOptions{RegoVersion: tc.regoVersion}). + WithCallback(func(_ context.Context, u Update) { + // We might get multiple updates before the test ends, and we don't want to block indefinitely. + select { + case updates <- &u: + } + }).WithBundleVerificationConfig(vc) + + d.Start(ctx) + + // Give time for some download events to occur + time.Sleep(1 * time.Second) + + // We only care about the first update + u1 := <-updates + + if tc.expErr != "" { + if u1.Error == nil { + t.Fatalf("expected error but got: %v", u1) + } else { + if !strings.Contains(u1.Error.Error(), tc.expErr) { + t.Fatalf("expected error:\n\n%v\n\nbut got:\n\n%v", tc.expErr, u1.Error) + } + } + } else { + if u1.Error != nil { + t.Fatalf("expected no error but got: %v", u1.Error) + } + + if u1.Bundle == nil || len(u1.Bundle.Modules) == 0 { + t.Fatal("expected bundle with at least one module but got:", u1) + } + + if !strings.HasSuffix(u1.Bundle.Modules[0].URL, u1.Bundle.Modules[0].Path) { + t.Fatalf("expected URL to have path as suffix but got %v and %v", u1.Bundle.Modules[0].URL, u1.Bundle.Modules[0].Path) + } + } + + d.Stop(ctx) + }) + } +} + func TestOCIStartStop(t *testing.T) { ctx := context.Background() fixture := newTestFixture(t) - fixture.server.expEtag = "sha256:c5834dbce332cabe6ae68a364de171a50bf5b08024c27d7c08cc72878b4df7ff" + fixture.server.expEtag = "sha256:cc09b0f5ac97b11637c96ff1b0fbbc287c5ba0169813edaa71fe58424e95f0b7" updates := make(chan *Update) diff --git a/download/oci_downloader.go b/download/oci_downloader.go index aec65ff0ca..8e39aa92e9 100644 --- a/download/oci_downloader.go +++ b/download/oci_downloader.go @@ -4,6 +4,7 @@ import ( "context" "sync" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/logging" "github.com/open-policy-agent/opa/plugins/rest" @@ -11,20 +12,21 @@ import ( ) type OCIDownloader struct { - config Config // downloader configuration for tuning polling and other downloader behaviour - client rest.Client // HTTP client to use for bundle downloading - path string // path for OCI image as //: - localStorePath string // path for the local OCI storage - trigger chan chan struct{} // channel to signal downloads when manual triggering is enabled - stop chan chan struct{} // used to signal plugin to stop running - f func(context.Context, Update) // callback function invoked when download updates occur - sizeLimitBytes *int64 // max bundle file size in bytes (passed to reader) - bvc *bundle.VerificationConfig - wg sync.WaitGroup - logger logging.Logger - mtx sync.Mutex - stopped bool - persist bool - store *oci.Store - etag string + config Config // downloader configuration for tuning polling and other downloader behaviour + client rest.Client // HTTP client to use for bundle downloading + path string // path for OCI image as //: + localStorePath string // path for the local OCI storage + trigger chan chan struct{} // channel to signal downloads when manual triggering is enabled + stop chan chan struct{} // used to signal plugin to stop running + f func(context.Context, Update) // callback function invoked when download updates occur + sizeLimitBytes *int64 // max bundle file size in bytes (passed to reader) + bvc *bundle.VerificationConfig + wg sync.WaitGroup + logger logging.Logger + mtx sync.Mutex + stopped bool + persist bool + store *oci.Store + etag string + bundleParserOpts ast.ParserOptions } diff --git a/download/testdata/rego_v1.manifest b/download/testdata/rego_v1.manifest new file mode 100644 index 0000000000..6b9eaeac10 --- /dev/null +++ b/download/testdata/rego_v1.manifest @@ -0,0 +1,19 @@ +{ + "schemaVersion":2, + "config":{ + "mediaType":"application/vnd.oci.image.config.v1+json", + "digest":"sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a", + "size":2 + }, + "layers":[ + { + "mediaType":"application/vnd.oci.image.layer.v1.tar+gzip", + "digest":"sha256:cc09b0f5ac97b11637c96ff1b0fbbc287c5ba0169813edaa71fe58424e95f0b7", + "size":695, + "annotations":{ + "org.opencontainers.image.created":"2022-02-11T09:00:07Z", + "org.opencontainers.image.title":"dani/testpol" + } + } + ] +} \ No newline at end of file diff --git a/download/testdata/rego_v1.tar.gz b/download/testdata/rego_v1.tar.gz new file mode 100644 index 0000000000..7c0bf87638 Binary files /dev/null and b/download/testdata/rego_v1.tar.gz differ diff --git a/download/testdata/rego_v1_bundle_data/a/b/c/data.json b/download/testdata/rego_v1_bundle_data/a/b/c/data.json new file mode 100644 index 0000000000..3a26a2e5e9 --- /dev/null +++ b/download/testdata/rego_v1_bundle_data/a/b/c/data.json @@ -0,0 +1 @@ +[1,2,3] \ No newline at end of file diff --git a/download/testdata/rego_v1_bundle_data/http/policy/policy.rego b/download/testdata/rego_v1_bundle_data/http/policy/policy.rego new file mode 100644 index 0000000000..65bf71b531 --- /dev/null +++ b/download/testdata/rego_v1_bundle_data/http/policy/policy.rego @@ -0,0 +1,9 @@ +package example + +violations contains msg if { + msg := "hello" +} + +allow if { + count(violations) == 0 +} \ No newline at end of file diff --git a/download/testharness.go b/download/testharness.go index b262bc1da5..c0e99d34c1 100644 --- a/download/testharness.go +++ b/download/testharness.go @@ -299,6 +299,56 @@ func newTestServer(t *testing.T) *testServer { }, }}, }, + "test/v1compat/no_imports": { + Manifest: bundle.Manifest{ + Revision: "quickbrownfaux", + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: `/example.rego`, + Raw: []byte(`package test +import data.foo +import data.bar as foo +p contains 1 if { + input.x == 2 +}`), + }, + }, + }, + "test/v1compat/rego_import": { + Manifest: bundle.Manifest{ + Revision: "quickbrownfaux", + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: `/example.rego`, + Raw: []byte(`package test +import rego.v1 +import data.foo +import data.bar as foo +p contains 1 if { + input.x == 2 +}`), + }, + }, + }, + "test/v1compat/keywords_not_used": { + Manifest: bundle.Manifest{ + Revision: "quickbrownfaux", + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: `/example.rego`, + Raw: []byte(`package test +p[1] { + input.x == 2 +}`), + }, + }, + }, }, } } diff --git a/internal/bundle/inspect/inspect.go b/internal/bundle/inspect/inspect.go index 66325fd814..b5857df334 100644 --- a/internal/bundle/inspect/inspect.go +++ b/internal/bundle/inspect/inspect.go @@ -31,7 +31,12 @@ type Info struct { } func File(path string, includeAnnotations bool) (*Info, error) { + return FileForRegoVersion(ast.RegoV0, path, includeAnnotations) +} + +func FileForRegoVersion(regoVersion ast.RegoVersion, path string, includeAnnotations bool) (*Info, error) { b, err := loader.NewFileLoader(). + WithRegoVersion(regoVersion). WithSkipBundleVerification(true). WithProcessAnnotation(true). // Always process annotations, for enriching namespace listing WithJSONOptions(&json.Options{ diff --git a/internal/bundle/utils.go b/internal/bundle/utils.go index a541c45f1f..89f3e2a6f4 100644 --- a/internal/bundle/utils.go +++ b/internal/bundle/utils.go @@ -89,6 +89,10 @@ func LoadWasmResolversFromStore(ctx context.Context, store storage.Store, txn st // LoadBundleFromDisk loads a previously persisted activated bundle from disk func LoadBundleFromDisk(path, name string, bvc *bundle.VerificationConfig) (*bundle.Bundle, error) { + return LoadBundleFromDiskForRegoVersion(ast.RegoV0, path, name, bvc) +} + +func LoadBundleFromDiskForRegoVersion(regoVersion ast.RegoVersion, path, name string, bvc *bundle.VerificationConfig) (*bundle.Bundle, error) { bundlePath := filepath.Join(path, name, "bundle.tar.gz") if _, err := os.Stat(bundlePath); err == nil { @@ -98,7 +102,8 @@ func LoadBundleFromDisk(path, name string, bvc *bundle.VerificationConfig) (*bun } defer f.Close() - r := bundle.NewCustomReader(bundle.NewTarballLoaderWithBaseURL(f, "")) + r := bundle.NewCustomReader(bundle.NewTarballLoaderWithBaseURL(f, "")). + WithRegoVersion(regoVersion) if bvc != nil { r = r.WithBundleVerificationConfig(bvc) diff --git a/internal/pathwatcher/utils.go b/internal/pathwatcher/utils.go index e6b22837de..29a64c9079 100644 --- a/internal/pathwatcher/utils.go +++ b/internal/pathwatcher/utils.go @@ -42,7 +42,12 @@ func CreatePathWatcher(rootPaths []string) (*fsnotify.Watcher, error) { // ProcessWatcherUpdate handles an occurrence of a watcher event func ProcessWatcherUpdate(ctx context.Context, paths []string, removed string, store storage.Store, filter loader.Filter, asBundle bool, f func(context.Context, storage.Transaction, *initload.LoadPathsResult) error) error { - loaded, err := initload.LoadPaths(paths, filter, asBundle, nil, true, false, nil, nil) + return ProcessWatcherUpdateForRegoVersion(ctx, ast.RegoV0, paths, removed, store, filter, asBundle, f) +} + +func ProcessWatcherUpdateForRegoVersion(ctx context.Context, regoVersion ast.RegoVersion, paths []string, removed string, store storage.Store, filter loader.Filter, asBundle bool, + f func(context.Context, storage.Transaction, *initload.LoadPathsResult) error) error { + loaded, err := initload.LoadPathsForRegoVersion(regoVersion, paths, filter, asBundle, nil, true, false, nil, nil) if err != nil { return err } diff --git a/internal/runtime/init/init.go b/internal/runtime/init/init.go index ba316efcb8..88e8bc4e0b 100644 --- a/internal/runtime/init/init.go +++ b/internal/runtime/init/init.go @@ -28,6 +28,7 @@ type InsertAndCompileOptions struct { Bundles map[string]*bundle.Bundle MaxErrors int EnablePrintStatements bool + ParserOptions ast.ParserOptions } // InsertAndCompileResult contains the output of the operation. @@ -58,13 +59,14 @@ func InsertAndCompile(ctx context.Context, opts InsertAndCompileOptions) (*Inser m := metrics.New() activation := &bundle.ActivateOpts{ - Ctx: ctx, - Store: opts.Store, - Txn: opts.Txn, - Compiler: compiler, - Metrics: m, - Bundles: opts.Bundles, - ExtraModules: policies, + Ctx: ctx, + Store: opts.Store, + Txn: opts.Txn, + Compiler: compiler, + Metrics: m, + Bundles: opts.Bundles, + ExtraModules: policies, + ParserOptions: opts.ParserOptions, } err := bundle.Activate(activation) diff --git a/loader/loader.go b/loader/loader.go index 2960cababb..cb15060379 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -190,6 +190,7 @@ func (fl *fileLoader) WithJSONOptions(opts *astJSON.Options) FileLoader { // WithRegoV1Compatible enforces Rego v0 with Rego v1 compatibility. // See ParserOptions.RegoV1Compatible for more details. // Deprecated: use WithRegoVersion instead +// TODO: Remove, this is an internal type, so won't be a breaking change. func (fl *fileLoader) WithRegoV1Compatible(compatible bool) FileLoader { fl.opts.RegoV1Compatible = compatible return fl diff --git a/logging/test/test.go b/logging/test/test.go index fa9a05cd0d..5fd2dd47c4 100644 --- a/logging/test/test.go +++ b/logging/test/test.go @@ -19,7 +19,7 @@ type Logger struct { level logging.Level fields map[string]interface{} entries *[]LogEntry - mtx sync.Mutex + mtx *sync.Mutex } // New instantiates new Logger. @@ -27,6 +27,7 @@ func New() *Logger { return &Logger{ level: logging.Info, entries: &[]LogEntry{}, + mtx: &sync.Mutex{}, } } @@ -39,6 +40,7 @@ func (l *Logger) WithFields(fields map[string]interface{}) logging.Logger { level: l.level, entries: l.entries, fields: l.fields, + mtx: l.mtx, } flds := make(map[string]interface{}) for k, v := range cp.fields { diff --git a/plugins/bundle/plugin.go b/plugins/bundle/plugin.go index 6694cf07ec..818952d9a9 100644 --- a/plugins/bundle/plugin.go +++ b/plugins/bundle/plugin.go @@ -353,7 +353,7 @@ func (p *Plugin) loadAndActivateBundlesFromDisk(ctx context.Context) { for name, src := range p.config.Bundles { if p.persistBundle(name) { - b, err := loadBundleFromDisk(p.bundlePersistPath, name, src) + b, err := p.loadBundleFromDisk(p.bundlePersistPath, name, src) if err != nil { p.log(name).Error("Failed to load bundle from disk: %v", err) p.status[name].SetError(err) @@ -407,11 +407,12 @@ func (p *Plugin) newDownloader(name string, source *Source) Loader { switch u.Scheme { case "file": return &fileLoader{ - name: name, - path: u.Path, - bvc: source.Signing, - sizeLimitBytes: source.SizeLimitBytes, - f: p.oneShot, + name: name, + path: u.Path, + bvc: source.Signing, + sizeLimitBytes: source.SizeLimitBytes, + f: p.oneShot, + bundleParserOpts: p.manager.ParserOptions(), } } } @@ -432,14 +433,17 @@ func (p *Plugin) newDownloader(name string, source *Source) Loader { WithCallback(callback). WithBundleVerificationConfig(source.Signing). WithSizeLimitBytes(source.SizeLimitBytes). - WithBundlePersistence(p.persistBundle(name)) + WithBundlePersistence(p.persistBundle(name)). + WithBundleParserOpts(p.manager.ParserOptions()) } return download.New(conf, client, path). WithCallback(callback). WithBundleVerificationConfig(source.Signing). WithSizeLimitBytes(source.SizeLimitBytes). WithBundlePersistence(p.persistBundle(name)). - WithLazyLoadingMode(true).WithBundleName(name) + WithLazyLoadingMode(true). + WithBundleName(name). + WithBundleParserOpts(p.manager.ParserOptions()) } func (p *Plugin) oneShot(ctx context.Context, name string, u download.Update) { @@ -696,11 +700,11 @@ func saveCurrentBundleToDisk(path string, raw io.Reader) (string, error) { return bundleUtils.SaveBundleToDisk(path, raw) } -func loadBundleFromDisk(path, name string, src *Source) (*bundle.Bundle, error) { +func (p *Plugin) loadBundleFromDisk(path, name string, src *Source) (*bundle.Bundle, error) { if src != nil { - return bundleUtils.LoadBundleFromDisk(path, name, src.Signing) + return bundleUtils.LoadBundleFromDiskForRegoVersion(p.manager.ParserOptions().RegoVersion, path, name, src.Signing) } - return bundleUtils.LoadBundleFromDisk(path, name, nil) + return bundleUtils.LoadBundleFromDiskForRegoVersion(p.manager.ParserOptions().RegoVersion, path, name, nil) } func (p *Plugin) log(name string) logging.Logger { @@ -720,11 +724,12 @@ func (p *Plugin) getBundlePersistPath() (string, error) { } type fileLoader struct { - name string - path string - bvc *bundle.VerificationConfig - sizeLimitBytes int64 - f func(context.Context, string, download.Update) + name string + path string + bvc *bundle.VerificationConfig + sizeLimitBytes int64 + f func(context.Context, string, download.Update) + bundleParserOpts ast.ParserOptions } func (fl *fileLoader) Start(ctx context.Context) { @@ -779,7 +784,9 @@ func (fl *fileLoader) oneShot(ctx context.Context) { b, err := reader. WithMetrics(u.Metrics). WithBundleVerificationConfig(fl.bvc). - WithSizeLimitBytes(fl.sizeLimitBytes).Read() + WithSizeLimitBytes(fl.sizeLimitBytes). + WithRegoVersion(fl.bundleParserOpts.RegoVersion). + Read() u.Error = err if err == nil { u.Bundle = &b diff --git a/plugins/bundle/plugin_test.go b/plugins/bundle/plugin_test.go index 037b9aaeff..9fe5e85845 100644 --- a/plugins/bundle/plugin_test.go +++ b/plugins/bundle/plugin_test.go @@ -111,6 +111,145 @@ func TestPluginOneShot(t *testing.T) { } } +func TestPluginOneShotV1Compatible(t *testing.T) { + // Note: modules are parsed before passed to plugin, so any expected errors must be triggered by the compiler stage. + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x", + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x, shadowed import (no error)", + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0", + v1Compatible: true, + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0, shadowed import", + v1Compatible: true, + module: `package foo +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + + ctx := context.Background() + manager := getTestManager() + plugin := New(&Config{}, manager) + bundleName := "test-bundle" + plugin.status[bundleName] = &Status{Name: bundleName, Metrics: metrics.New()} + plugin.downloaders[bundleName] = download.New(download.Config{}, plugin.manager.Client(""), bundleName) + + ensurePluginState(t, plugin, plugins.StateNotReady) + + b := bundle.Bundle{ + Manifest: bundle.Manifest{Revision: "quickbrownfaux"}, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "/foo/bar", + Parsed: ast.MustParseModuleWithOpts(tc.module, popts), + Raw: []byte(tc.module), + }, + }, + Etag: "foo", + } + + b.Manifest.Init() + + plugin.oneShot(ctx, bundleName, download.Update{Bundle: &b, Metrics: metrics.New(), Size: snapshotBundleSize}) + + if tc.expErrs != nil { + ensurePluginState(t, plugin, plugins.StateNotReady) + + if status, ok := plugin.status[bundleName]; !ok { + t.Fatalf("Expected to find status for %s, found nil", bundleName) + } else if status.Type != bundle.SnapshotBundleType { + t.Fatalf("expected snapshot bundle but got %v", status.Type) + } else if errs := status.Errors; len(errs) != len(tc.expErrs) { + t.Fatalf("expected errors:\n\n%v\n\nbut got:\n\n%v", tc.expErrs, errs) + } else { + for _, expErr := range tc.expErrs { + found := false + for _, err := range errs { + if strings.Contains(err.Error(), expErr) { + found = true + break + } + } + if !found { + t.Fatalf("expected error:\n\n%v\n\nbut got:\n\n%v", expErr, errs) + } + } + } + } else { + ensurePluginState(t, plugin, plugins.StateOK) + + if status, ok := plugin.status[bundleName]; !ok { + t.Fatalf("Expected to find status for %s, found nil", bundleName) + } else if status.Type != bundle.SnapshotBundleType { + t.Fatalf("expected snapshot bundle but got %v", status.Type) + } else if status.Size != snapshotBundleSize { + t.Fatalf("expected snapshot bundle size %d but got %d", snapshotBundleSize, status.Size) + } + + txn := storage.NewTransactionOrDie(ctx, manager.Store) + defer manager.Store.Abort(ctx, txn) + + ids, err := manager.Store.ListPolicies(ctx, txn) + if err != nil { + t.Fatal(err) + } else if len(ids) != 1 { + t.Fatal("Expected 1 policy") + } + + bs, err := manager.Store.GetPolicy(ctx, txn, ids[0]) + exp := []byte(tc.module) + if err != nil { + t.Fatal(err) + } else if !bytes.Equal(bs, exp) { + t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs)) + } + } + }) + } +} + func TestPluginOneShotWithAuthzSchemaVerification(t *testing.T) { ctx := context.Background() @@ -831,7 +970,7 @@ func TestPluginOneShotBundlePersistence(t *testing.T) { ensurePluginState(t, plugin, plugins.StateOK) - result, err := loadBundleFromDisk(plugin.bundlePersistPath, bundleName, nil) + result, err := plugin.loadBundleFromDisk(plugin.bundlePersistPath, bundleName, nil) if err != nil { t.Fatal("unexpected error:", err) } @@ -872,6 +1011,191 @@ func TestPluginOneShotBundlePersistence(t *testing.T) { } } +func TestPluginOneShotBundlePersistenceV1Compatible(t *testing.T) { + // Note: modules are parsed before passed to plugin, so any expected errors must be triggered by the compiler stage. + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x", + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x, shadowed import (no error)", + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0", + v1Compatible: true, + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0, shadowed import", + v1Compatible: true, + module: `package foo +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + + ctx := context.Background() + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal("unexpected error:", err) + } + + dir := t.TempDir() + + bundleName := "test-bundle" + bundleSource := Source{ + Persist: true, + } + + bundles := map[string]*Source{} + bundles[bundleName] = &bundleSource + + plugin := New(&Config{Bundles: bundles}, manager) + + plugin.status[bundleName] = &Status{Name: bundleName, Metrics: metrics.New()} + plugin.downloaders[bundleName] = download.New(download.Config{}, plugin.manager.Client(""), bundleName) + plugin.bundlePersistPath = filepath.Join(dir, ".opa") + + ensurePluginState(t, plugin, plugins.StateNotReady) + + // simulate a bundle download error with no bundle on disk + plugin.oneShot(ctx, bundleName, download.Update{Error: fmt.Errorf("unknown error")}) + + if plugin.status[bundleName].Message == "" { + t.Fatal("expected error but got none") + } + + ensurePluginState(t, plugin, plugins.StateNotReady) + + // download a bundle and persist to disk. Then verify the bundle persisted to disk + b := bundle.Bundle{ + Manifest: bundle.Manifest{Revision: "quickbrownfaux"}, + Data: util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}}`)).(map[string]interface{}), + Modules: []bundle.ModuleFile{ + { + URL: "/foo/bar.rego", + Path: "/foo/bar.rego", + Parsed: ast.MustParseModuleWithOpts(tc.module, popts), + Raw: []byte(tc.module), + }, + }, + Etag: "foo", + } + + b.Manifest.Init() + expBndl := b.Copy() // We're opting out of roundtripping in storage/inmem, so we copy ourselves. + + var buf bytes.Buffer + if err := bundle.NewWriter(&buf).UseModulePath(true).Write(b); err != nil { + t.Fatal("unexpected error:", err) + } + + plugin.oneShot(ctx, bundleName, download.Update{Bundle: &b, Metrics: metrics.New(), Raw: &buf}) + + if tc.expErrs != nil { + ensurePluginState(t, plugin, plugins.StateNotReady) + + if status, ok := plugin.status[bundleName]; !ok { + t.Fatalf("Expected to find status for %s, found nil", bundleName) + } else if status.Type != bundle.SnapshotBundleType { + t.Fatalf("expected snapshot bundle but got %v", status.Type) + } else if errs := status.Errors; len(errs) != len(tc.expErrs) { + t.Fatalf("expected errors:\n\n%v\n\nbut got:\n\n%v", tc.expErrs, errs) + } else { + for _, expErr := range tc.expErrs { + found := false + for _, err := range errs { + if strings.Contains(err.Error(), expErr) { + found = true + break + } + } + if !found { + t.Fatalf("expected error:\n\n%v\n\nbut got:\n\n%v", expErr, errs) + } + } + } + } else { + ensurePluginState(t, plugin, plugins.StateOK) + + result, err := plugin.loadBundleFromDisk(plugin.bundlePersistPath, bundleName, nil) + if err != nil { + t.Fatal("unexpected error:", err) + } + + if !result.Equal(expBndl) { + t.Fatalf("expected the downloaded bundle to be equal to the one loaded from disk: result=%v, exp=%v", result, expBndl) + } + + // simulate a bundle download error and verify that the bundle on disk is activated + plugin.oneShot(ctx, bundleName, download.Update{Error: fmt.Errorf("unknown error")}) + + ensurePluginState(t, plugin, plugins.StateOK) + + txn := storage.NewTransactionOrDie(ctx, manager.Store) + defer manager.Store.Abort(ctx, txn) + + ids, err := manager.Store.ListPolicies(ctx, txn) + if err != nil { + t.Fatal(err) + } else if len(ids) != 1 { + t.Fatal("Expected 1 policy") + } + + bs, err := manager.Store.GetPolicy(ctx, txn, ids[0]) + exp := []byte(tc.module) + if err != nil { + t.Fatal(err) + } else if !bytes.Equal(bs, exp) { + t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs)) + } + + data, err := manager.Store.Read(ctx, txn, storage.Path{}) + expData := util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}}}`)) + if err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(data, expData) { + t.Fatalf("Bad data content. Exp:\n%v\n\nGot:\n\n%v", expData, data) + } + } + }) + } +} + func TestPluginOneShotSignedBundlePersistence(t *testing.T) { ctx := context.Background() @@ -933,7 +1257,7 @@ func TestPluginOneShotSignedBundlePersistence(t *testing.T) { ensurePluginState(t, plugin, plugins.StateOK) // load signed bundle from disk - result, err := loadBundleFromDisk(plugin.bundlePersistPath, bundleName, bundles[bundleName]) + result, err := plugin.loadBundleFromDisk(plugin.bundlePersistPath, bundleName, bundles[bundleName]) if err != nil { t.Fatal("unexpected error:", err) } @@ -1049,6 +1373,168 @@ func TestLoadAndActivateBundlesFromDisk(t *testing.T) { } } +func TestLoadAndActivateBundlesFromDiskV1Compatible(t *testing.T) { + // Note: modules are parsed before passed to plugin, so any expected errors must be triggered by the compiler stage. + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x", + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x, shadowed import (no error)", + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0", + v1Compatible: true, + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0, shadowed import", + v1Compatible: true, + module: `package foo +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + + ctx := context.Background() + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal("unexpected error:", err) + } + + dir := t.TempDir() + + bundleName := "test-bundle" + bundleSource := Source{ + Persist: true, + } + + bundleNameOther := "test-bundle-other" + bundleSourceOther := Source{} + + bundles := map[string]*Source{} + bundles[bundleName] = &bundleSource + bundles[bundleNameOther] = &bundleSourceOther + + plugin := New(&Config{Bundles: bundles}, manager) + plugin.bundlePersistPath = filepath.Join(dir, ".opa") + + plugin.loadAndActivateBundlesFromDisk(ctx) + + // persist a bundle to disk and then load it + b := bundle.Bundle{ + Manifest: bundle.Manifest{Revision: "quickbrownfaux"}, + Data: util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}}`)).(map[string]interface{}), + Modules: []bundle.ModuleFile{ + { + URL: "/foo/bar.rego", + Path: "/foo/bar.rego", + Parsed: ast.MustParseModuleWithOpts(tc.module, popts), + Raw: []byte(tc.module), + }, + }, + } + + b.Manifest.Init() + + var buf bytes.Buffer + if err := bundle.NewWriter(&buf).UseModulePath(true).Write(b); err != nil { + t.Fatal("unexpected error:", err) + } + + err = plugin.saveBundleToDisk(bundleName, &buf) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + + plugin.loadAndActivateBundlesFromDisk(ctx) + + if tc.expErrs != nil { + if status, ok := plugin.status[bundleName]; !ok { + t.Fatalf("Expected to find status for %s, found nil", bundleName) + } else if status.Type != bundle.SnapshotBundleType { + t.Fatalf("expected snapshot bundle but got %v", status.Type) + } else if errs := status.Errors; len(errs) != len(tc.expErrs) { + t.Fatalf("expected errors:\n\n%v\n\nbut got:\n\n%v", tc.expErrs, errs) + } else { + for _, expErr := range tc.expErrs { + found := false + for _, err := range errs { + if strings.Contains(err.Error(), expErr) { + found = true + break + } + } + if !found { + t.Fatalf("expected error:\n\n%v\n\nbut got:\n\n%v", expErr, errs) + } + } + } + } else { + txn := storage.NewTransactionOrDie(ctx, manager.Store) + defer manager.Store.Abort(ctx, txn) + + ids, err := manager.Store.ListPolicies(ctx, txn) + if err != nil { + t.Fatal(err) + } else if len(ids) != 1 { + t.Fatal("Expected 1 policy") + } + + bs, err := manager.Store.GetPolicy(ctx, txn, ids[0]) + exp := []byte(tc.module) + if err != nil { + t.Fatal(err) + } else if !bytes.Equal(bs, exp) { + t.Fatalf("Bad policy content. Exp:\n%v\n\nGot:\n\n%v", string(exp), string(bs)) + } + + data, err := manager.Store.Read(ctx, txn, storage.Path{}) + expData := util.MustUnmarshalJSON([]byte(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux", "roots": [""]}}}}}`)) + if err != nil { + t.Fatal(err) + } else if !reflect.DeepEqual(data, expData) { + t.Fatalf("Bad data content. Exp:\n%v\n\nGot:\n\n%v", expData, data) + } + } + }) + } +} + func TestLoadAndActivateDepBundlesFromDisk(t *testing.T) { ctx := context.Background() manager := getTestManager() @@ -2728,7 +3214,7 @@ func TestSaveBundleToDiskOverWrite(t *testing.T) { t.Fatalf("unexpected error %v", err) } - actual, err := loadBundleFromDisk(plugin.bundlePersistPath, "foo", nil) + actual, err := plugin.loadBundleFromDisk(plugin.bundlePersistPath, "foo", nil) if err != nil { t.Fatalf("unexpected error %v", err) } @@ -2763,8 +3249,11 @@ func TestSaveCurrentBundleToDisk(t *testing.T) { func TestLoadBundleFromDisk(t *testing.T) { + manager := getTestManager() + plugin := New(&Config{}, manager) + // no bundle on disk - _, err := loadBundleFromDisk("foo", "bar", nil) + _, err := plugin.loadBundleFromDisk("foo", "bar", nil) if err != nil { t.Fatalf("unexpected error %v", err) } @@ -2782,7 +3271,67 @@ func TestLoadBundleFromDisk(t *testing.T) { b := writeTestBundleToDisk(t, bundleDir, false) - result, err := loadBundleFromDisk(dir, bundleName, nil) + result, err := plugin.loadBundleFromDisk(dir, bundleName, nil) + if err != nil { + t.Fatal("unexpected error:", err) + } + + if !result.Equal(b) { + t.Fatal("expected the test bundle to be equal to the one loaded from disk") + } +} + +func TestLoadBundleFromDiskV1Compatible(t *testing.T) { + popts := ast.ParserOptions{RegoVersion: ast.RegoV1} + + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal("unexpected error:", err) + } + plugin := New(&Config{}, manager) + + // create a test bundle and load it from disk + dir := t.TempDir() + + bundleName := "foo" + bundleDir := filepath.Join(dir, bundleName) + + err = os.MkdirAll(bundleDir, os.ModePerm) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + + // v1.0 policy + policy := `package test +p contains 1 if { + input.x == 2 +}` + + b := bundle.Bundle{ + Manifest: bundle.Manifest{Revision: "test-revision"}, + Modules: []bundle.ModuleFile{ + { + URL: `policy.rego`, + Path: `/policy.rego`, + Raw: []byte(policy), + Parsed: ast.MustParseModuleWithOpts(policy, popts), + }, + }, + Data: map[string]interface{}{}, + } + + b.Manifest.Init() + + var buf bytes.Buffer + if err := bundle.NewWriter(&buf).UseModulePath(true).Write(b); err != nil { + t.Fatalf("unexpected error %v", err) + } + + if err := os.WriteFile(filepath.Join(bundleDir, "bundle.tar.gz"), buf.Bytes(), 0644); err != nil { + t.Fatalf("unexpected error %v", err) + } + + result, err := plugin.loadBundleFromDisk(dir, bundleName, nil) if err != nil { t.Fatal("unexpected error:", err) } @@ -2793,9 +3342,11 @@ func TestLoadBundleFromDisk(t *testing.T) { } func TestLoadSignedBundleFromDisk(t *testing.T) { + manager := getTestManager() + plugin := New(&Config{}, manager) // no bundle on disk - _, err := loadBundleFromDisk("foo", "bar", nil) + _, err := plugin.loadBundleFromDisk("foo", "bar", nil) if err != nil { t.Fatalf("unexpected error %v", err) } @@ -2817,7 +3368,7 @@ func TestLoadSignedBundleFromDisk(t *testing.T) { Signing: bundle.NewVerificationConfig(map[string]*keys.Config{"foo": {Key: "secret", Algorithm: "HS256"}}, "foo", "", nil), } - result, err := loadBundleFromDisk(dir, bundleName, &src) + result, err := plugin.loadBundleFromDisk(dir, bundleName, &src) if err != nil { t.Fatal("unexpected error:", err) } @@ -2917,6 +3468,193 @@ func TestPluginUsingFileLoader(t *testing.T) { } +func TestPluginUsingFileLoaderV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x, keywords not used", + module: `package test +p[7] { + input.x == 2 +}`, + }, + { + note: "v0.x, shadowed import", + module: `package test +import future.keywords +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x, keywords not imported", + module: `package test +p contains 7 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x, rego.ve imported", + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + // parse-time error + { + note: "v1.0, keywords not used", + v1Compatible: true, + module: `package test +p[7] { + input.x == 2 +}`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + // compile-time error + { + note: "v1.0, shadowed import", + v1Compatible: true, + module: `package test +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + { + note: "v1.0, keywords not imported", + v1Compatible: true, + module: `package test +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0, rego.ve imported", + v1Compatible: true, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + + test.WithTempFS(map[string]string{}, func(dir string) { + + b := bundle.Bundle{ + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + URL: "test.rego", + Raw: []byte(tc.module), + }, + }, + } + + name := path.Join(dir, "bundle.tar.gz") + + f, err := os.Create(name) + if err != nil { + t.Fatal(err) + } + + if err := bundle.NewWriter(f).Write(b); err != nil { + t.Fatal(err) + } + + f.Close() + + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal("unexpected error:", err) + } + url := "file://" + name + + p := New(&Config{Bundles: map[string]*Source{ + "test": { + SizeLimitBytes: 1e5, + Resource: url, + }, + }}, manager) + + ch := make(chan Status) + + p.Register("test", func(s Status) { + ch <- s + }) + + if err := p.Start(context.Background()); err != nil { + t.Fatal(err) + } + + s := <-ch + + if tc.expErrs != nil { + for _, expErr := range tc.expErrs { + found := false + for _, e := range s.Errors { + if strings.Contains(e.Error(), expErr) { + found = true + break + } + } + if !found { + t.Fatalf("expected error:\n\n%s\n\nbut got:\n\n%v", expErr, s.Errors) + } + } + } else { + if s.LastSuccessfulActivation.IsZero() { + t.Fatal("expected successful activation") + } + } + }) + }) + } +} + func TestPluginUsingDirectoryLoader(t *testing.T) { test.WithTempFS(map[string]string{ "test.rego": `package test @@ -2952,6 +3690,172 @@ func TestPluginUsingDirectoryLoader(t *testing.T) { }) } +func TestPluginUsingDirectoryLoaderV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + expErrs []string + }{ + { + note: "v0.x, keywords not used", + module: `package test +p[7] { + input.x == 2 +}`, + }, + { + note: "v0.x, shadowed import", + module: `package test +import future.keywords +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x, keywords not imported", + module: `package test +p contains 7 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x, rego.ve imported", + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + // parse-time error + { + note: "v1.0, keywords not used", + v1Compatible: true, + module: `package test +p[7] { + input.x == 2 +}`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + // compile-time error + { + note: "v1.0, shadowed import", + v1Compatible: true, + module: `package test +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + { + note: "v1.0, keywords not imported", + v1Compatible: true, + module: `package test +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0, rego.ve imported", + v1Compatible: true, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + + test.WithTempFS(map[string]string{ + "test.rego": tc.module, + }, func(dir string) { + + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal("unexpected error:", err) + } + url := "file://" + dir + + p := New(&Config{Bundles: map[string]*Source{ + "test": { + SizeLimitBytes: 1e5, + Resource: url, + }, + }}, manager) + + ch := make(chan Status) + + p.Register("test", func(s Status) { + ch <- s + }) + + if err := p.Start(context.Background()); err != nil { + t.Fatal(err) + } + + s := <-ch + + if tc.expErrs != nil { + for _, expErr := range tc.expErrs { + found := false + for _, e := range s.Errors { + if strings.Contains(e.Error(), expErr) { + found = true + break + } + } + if !found { + t.Fatalf("expected error:\n\n%s\n\nbut got:\n\n%v", expErr, s.Errors) + } + } + } else { + if s.LastSuccessfulActivation.IsZero() { + t.Fatal("expected successful activation") + } + } + }) + }) + } +} + func TestPluginReadBundleEtagFromDiskStore(t *testing.T) { // setup fake http server with mock bundle diff --git a/plugins/discovery/discovery.go b/plugins/discovery/discovery.go index 88eb8d427a..ae26ab36b5 100644 --- a/plugins/discovery/discovery.go +++ b/plugins/discovery/discovery.go @@ -121,12 +121,15 @@ func New(manager *plugins.Manager, opts ...func(*Discovery)) (*Discovery, error) result.downloader = download.NewOCI(config.Config, restClient, config.path, ociStorePath). WithCallback(result.oneShot). WithBundleVerificationConfig(config.Signing). - WithBundlePersistence(config.Persist) + WithBundlePersistence(config.Persist). + WithBundleParserOpts(manager.ParserOptions()) } else { - result.downloader = download.New(config.Config, restClient, config.path). + d := download.New(config.Config, restClient, config.path). WithCallback(result.oneShot). WithBundleVerificationConfig(config.Signing). - WithBundlePersistence(config.Persist) + WithBundlePersistence(config.Persist). + WithBundleParserOpts(manager.ParserOptions()) + result.downloader = d } result.status = &bundle.Status{ Name: Name, @@ -263,7 +266,8 @@ func (c *Discovery) loadAndActivateBundleFromDisk(ctx context.Context) { } func (c *Discovery) loadBundleFromDisk() (*bundleApi.Bundle, error) { - return bundleUtils.LoadBundleFromDisk(c.bundlePersistPath, c.discoveryBundleDirName(), c.config.Signing) + return bundleUtils.LoadBundleFromDiskForRegoVersion(c.manager.ParserOptions().RegoVersion, + c.bundlePersistPath, c.discoveryBundleDirName(), c.config.Signing) } func (c *Discovery) saveBundleToDisk(raw io.Reader) error { diff --git a/plugins/discovery/discovery_test.go b/plugins/discovery/discovery_test.go index 3f73c23434..21d3758ee5 100644 --- a/plugins/discovery/discovery_test.go +++ b/plugins/discovery/discovery_test.go @@ -17,6 +17,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" "time" @@ -184,6 +185,128 @@ func TestProcessBundle(t *testing.T) { } +func TestProcessBundleV1Compatible(t *testing.T) { + ctx := context.Background() + popts := ast.ParserOptions{RegoVersion: ast.RegoV1} + + manager, err := plugins.New([]byte(`{ + "services": { + "default": { + "url": "http://localhost:8181" + } + }, + "discovery": {"name": "config"} + }`), "test-id", + inmem.New(), + plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal(err) + } + + initialBundle := makeModuleBundle(1, `package config +bundle.name := "test1" +status := {} +decision_logs := {} if { 3 == 3 } +`, popts) + + disco, err := New(manager) + if err != nil { + t.Fatal(err) + } + + ps, err := disco.processBundle(ctx, initialBundle) + if err != nil { + t.Fatal(err) + } + + if len(ps.Start) != 3 || len(ps.Reconfig) != 0 { + t.Fatalf("Expected exactly three start events but got %v", ps) + } + + actualConfig, err := manager.Config.ActiveConfig() + if err != nil { + t.Fatal(err) + } + assertConfig(t, actualConfig, fmt.Sprintf(`{ + "bundle": { + "name": "test1" + }, + "decision_logs": {}, + "default_authorization_decision": "/system/authz/allow", + "default_decision": "/system/main", + "discovery": { + "name": "config" + }, + "labels": { + "id": "test-id", + "version": %v + }, + "status": {} +}`, version.Version)) + + // The bundle is parsed outside the discovery service, but is still compiled by it during processing. + // As such, it is impossible to pass it a module that doesn't pass the parsing step. + // We first pass it a valid v1.0 policy ... + updatedBundle := makeModuleBundle(1, `package config +bundle.name := "test2" if { 1 == 1 } +status.partition_name := "foo" if { 2 == 2 } +decision_logs.partition_name := "bar" if { 3 == 3 } +`, popts) + + ps, err = disco.processBundle(ctx, updatedBundle) + if err != nil { + t.Fatal(err) + } + + if len(ps.Start) != 0 || len(ps.Reconfig) != 3 { + t.Fatalf("Expected exactly three start events but got %v", ps) + } + + actualConfig, err = manager.Config.ActiveConfig() + if err != nil { + t.Fatal(err) + } + assertConfig(t, actualConfig, fmt.Sprintf(`{ + "bundle": { + "name": "test2" + }, + "decision_logs": { + "partition_name": "bar" + }, + "default_authorization_decision": "/system/authz/allow", + "default_decision": "/system/main", + "discovery": { + "name": "config" + }, + "labels": { + "id": "test-id", + "version": %v + }, + "status": { + "partition_name": "foo" + } +}`, version.Version)) + + // ... and then an invalid v1.0 policy, where we expect the compiler to complain about shadowed imports (which passes the parsing step). + updatedBundle = makeModuleBundle(1, `package config +import data.foo +import data.bar as foo + +bundle.name := "test2" if { 1 == 1 } +status.partition_name := "foo" if { 2 == 2 } +decision_logs.partition_name := "bar" if { 3 == 3 } +`, popts) + + _, err = disco.processBundle(ctx, updatedBundle) + if err == nil { + t.Fatal("Expected error but got none") + } + expErr := `rego_compile_error: import must not shadow import data.foo` + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("Expected error:\n\n%v\n\nbut got:\n\n%v", expErr, err) + } +} + func TestProcessBundleWithActiveConfig(t *testing.T) { ctx := context.Background() @@ -278,14 +401,7 @@ func TestProcessBundleWithActiveConfig(t *testing.T) { "discovery": {"name": "config"} }`, version.Version) - var expected map[string]interface{} - if err := util.Unmarshal([]byte(expectedConfig), &expected); err != nil { - t.Fatal(err) - } - - if !reflect.DeepEqual(actual, expected) { - t.Fatalf("want %v got %v", expected, actual) - } + assertConfig(t, actual, expectedConfig) initialBundle = makeDataBundle(2, ` { @@ -345,13 +461,19 @@ func TestProcessBundleWithActiveConfig(t *testing.T) { "discovery": {"name": "config"} }`, version.Version) - var expected2 map[string]interface{} - if err := util.Unmarshal([]byte(expectedConfig2), &expected2); err != nil { + assertConfig(t, actual, expectedConfig2) +} + +func assertConfig(t *testing.T, actualConfig interface{}, expectedConfig string) { + t.Helper() + + var expected map[string]interface{} + if err := util.Unmarshal([]byte(expectedConfig), &expected); err != nil { t.Fatal(err) } - if !reflect.DeepEqual(actual, expected2) { - t.Fatalf("want %v got %v", expected, actual) + if !reflect.DeepEqual(actualConfig, expected) { + t.Fatalf("expected config:\n\n%v\n\ngot:\n\n%v", expectedConfig, actualConfig) } } @@ -816,6 +938,132 @@ func TestLoadAndActivateBundleFromDiskMaxAttempts(t *testing.T) { } } +func TestLoadAndActivateBundleFromDiskV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + bundle string + }{ + { + note: "v0.x", + bundle: `package config +import future.keywords + +labels.x := "label value changed" +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v if { + v := {"a": "b"} +} +services.acmecorp.url := v if { + v := "http://localhost:8181" +} +bundles.authz.service := v if { + v := "localhost" +} +`, + }, + { + note: "v1.0", + v1Compatible: true, + // no future.keywords import + bundle: `package config +labels.x := "label value changed" +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v if { + v := {"a": "b"} +} +services.acmecorp.url := v if { + v := "http://localhost:8181" +} +bundles.authz.service := v if { + v := "localhost" +} +`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + regoVersion := ast.RegoV0 + if tc.v1Compatible { + regoVersion = ast.RegoV1 + } + popts := ast.ParserOptions{RegoVersion: regoVersion} + dir := t.TempDir() + + manager, err := plugins.New([]byte(`{ + "labels": {"x": "y"}, + "services": { + "localhost": { + "url": "http://localhost:9999" + } + }, + "discovery": {"name": "config", "persist": true}, + }`), "test-id", + inmem.New(), + plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal(err) + } + + testPlugin := &reconfigureTestPlugin{counts: map[string]int{}} + testFactory := testFactory{p: testPlugin} + + disco, err := New(manager, Factories(map[string]plugins.Factory{"test_plugin": testFactory})) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + + disco.bundlePersistPath = filepath.Join(dir, ".opa") + + ensurePluginState(t, disco, plugins.StateNotReady) + + // persist a bundle to disk and then load it + initialBundle := makeModuleBundle(1, tc.bundle, popts) + + initialBundle.Manifest.Init() + + var buf bytes.Buffer + if err := bundleApi.NewWriter(&buf).Write(*initialBundle); err != nil { + t.Fatal("unexpected error:", err) + } + + err = disco.saveBundleToDisk(&buf) + if err != nil { + t.Fatalf("unexpected error %v", err) + } + + disco.loadAndActivateBundleFromDisk(ctx) + + ensurePluginState(t, disco, plugins.StateOK) + + // verify the test plugin was registered on the manager + if plugin := manager.Plugin("test_plugin"); plugin == nil { + t.Fatalf("expected \"test_plugin\" to be regsitered with the plugin manager") + } + + // verify the test plugin was started + count, ok := testPlugin.counts["start"] + if !ok { + t.Fatal("expected test plugin to have start counter") + } + + if count != 1 { + t.Fatalf("expected test plugin to have a start count of 1 but got %v", count) + } + + // verify the bundle plugin was registered on the manager + if plugin := bundlePlugin.Lookup(disco.manager); plugin == nil { + t.Fatalf("expected bundle plugin to be regsitered with the plugin manager") + } + }) + } +} + func TestSaveBundleToDiskNew(t *testing.T) { dir := t.TempDir() @@ -1034,6 +1282,105 @@ func TestReconfigure(t *testing.T) { } } +func TestReconfigureV1Compatible(t *testing.T) { + popts := ast.ParserOptions{RegoVersion: ast.RegoV1} + + manager, err := plugins.New([]byte(`{ + "labels": {"x": "y"}, + "services": { + "localhost": { + "url": "http://localhost:9999" + } + }, + "discovery": {"name": "config"}, + }`), "test-id", + inmem.New(), + plugins.WithParserOptions(popts)) + if err != nil { + t.Fatal(err) + } + + testPlugin := &reconfigureTestPlugin{counts: map[string]int{}} + testFactory := testFactory{p: testPlugin} + + disco, err := New(manager, Factories(map[string]plugins.Factory{"test_plugin": testFactory})) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + + initialBundle := makeModuleBundle(1, `package config +labels := v if { + v := {"x": "label value changed", "y": "new label"} +} +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v if { + v := {"a": "b"} +}`, popts) + + disco.oneShot(ctx, download.Update{Bundle: initialBundle, Size: snapshotBundleSize}) + + if disco.status == nil { + t.Fatal("Expected to find status, found nil") + } else if disco.status.Type != bundleApi.SnapshotBundleType { + t.Fatalf("expected snapshot bundle but got %v", disco.status.Type) + } else if disco.status.Size != snapshotBundleSize { + t.Fatalf("expected snapshot bundle size %d but got %d", snapshotBundleSize, disco.status.Size) + } + + // Verify labels are unchanged but allow additions + exp := map[string]string{"x": "y", "y": "new label", "id": "test-id", "version": version.Version} + if !reflect.DeepEqual(manager.Labels(), exp) { + t.Errorf("Expected labels to be unchanged (%v) but got %v", exp, manager.Labels()) + } + + // Verify decision ids set + expDecision := ast.MustParseTerm("data.bar.baz") + expAuthzDecision := ast.MustParseTerm("data.baz.qux") + if !manager.Config.DefaultDecisionRef().Equal(expDecision.Value) { + t.Errorf("Expected default decision to be %v but got %v", expDecision, manager.Config.DefaultDecisionRef()) + } + if !manager.Config.DefaultAuthorizationDecisionRef().Equal(expAuthzDecision.Value) { + t.Errorf("Expected default authz decision to be %v but got %v", expAuthzDecision, manager.Config.DefaultAuthorizationDecisionRef()) + } + + // Verify plugins started + if !reflect.DeepEqual(testPlugin.counts, map[string]int{"start": 1}) { + t.Errorf("Expected exactly one plugin start but got %v", testPlugin) + } + + // Verify plugins reconfigured + updatedBundle := makeModuleBundle(2, `package config +labels := v if { + v := {"x": "label value changed", "z": "another added label"} +} +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v if { + v := {"a": "plugin parameter value changed"} +}`, popts) + + disco.oneShot(ctx, download.Update{Bundle: updatedBundle}) + + // Verify label additions are always on top of bootstrap config with multiple discovery documents + exp = map[string]string{"x": "y", "z": "another added label", "id": "test-id", "version": version.Version} + if !reflect.DeepEqual(manager.Labels(), exp) { + t.Errorf("Expected labels to be unchanged (%v) but got %v", exp, manager.Labels()) + } + + if disco.status == nil { + t.Fatal("Expected to find status, found nil") + } else if disco.status.Type != bundleApi.SnapshotBundleType { + t.Fatalf("expected snapshot bundle but got %v", disco.status.Type) + } + + if !reflect.DeepEqual(testPlugin.counts, map[string]int{"start": 1, "reconfig": 1}) { + t.Errorf("Expected one plugin start and one reconfig but got %v", testPlugin) + } +} + func TestReconfigureWithUpdates(t *testing.T) { ctx := context.Background() @@ -1919,6 +2266,21 @@ func makeDataBundle(n int, s string) *bundleApi.Bundle { } } +func makeModuleBundle(n int, s string, popts ast.ParserOptions) *bundleApi.Bundle { + return &bundleApi.Bundle{ + Manifest: bundleApi.Manifest{Revision: fmt.Sprintf("test-revision-%v", n)}, + Modules: []bundleApi.ModuleFile{ + { + URL: `policy.rego`, + Path: `/policy.rego`, + Raw: []byte(s), + Parsed: ast.MustParseModuleWithOpts(s, popts), + }, + }, + Data: map[string]interface{}{}, + } +} + func getTestManager(t *testing.T, conf string) *plugins.Manager { t.Helper() store := inmem.New() diff --git a/plugins/plugins.go b/plugins/plugins.go index 7bd47075e9..283fcc4591 100644 --- a/plugins/plugins.go +++ b/plugins/plugins.go @@ -210,6 +210,7 @@ type Manager struct { reporter *report.Reporter opaReportNotifyCh chan struct{} stop chan chan struct{} + parserOptions ast.ParserOptions } type managerContextKey string @@ -395,6 +396,13 @@ func WithHooks(hs hooks.Hooks) func(*Manager) { } } +// WithParserOptions sets the parser options to be used by the plugin manager. +func WithParserOptions(opts ast.ParserOptions) func(*Manager) { + return func(m *Manager) { + m.parserOptions = opts + } +} + // WithEnableTelemetry controls whether OPA will send telemetry reports to an external service. func WithEnableTelemetry(enableTelemetry bool) func(*Manager) { return func(m *Manager) { @@ -876,7 +884,7 @@ func (m *Manager) onCommit(ctx context.Context, txn storage.Transaction, event s // compiler on the context but the server does not (nor would users // implementing their own policy loading.) if compiler == nil && event.PolicyChanged() { - compiler, _ = loadCompilerFromStore(ctx, m.Store, txn, m.enablePrintStatements) + compiler, _ = loadCompilerFromStore(ctx, m.Store, txn, m.enablePrintStatements, m.ParserOptions()) } if compiler != nil { @@ -913,7 +921,7 @@ func (m *Manager) onCommit(ctx context.Context, txn storage.Transaction, event s } } -func loadCompilerFromStore(ctx context.Context, store storage.Store, txn storage.Transaction, enablePrintStatements bool) (*ast.Compiler, error) { +func loadCompilerFromStore(ctx context.Context, store storage.Store, txn storage.Transaction, enablePrintStatements bool, popts ast.ParserOptions) (*ast.Compiler, error) { policies, err := store.ListPolicies(ctx, txn) if err != nil { return nil, err @@ -925,7 +933,7 @@ func loadCompilerFromStore(ctx context.Context, store storage.Store, txn storage if err != nil { return nil, err } - module, err := ast.ParseModule(policy, string(bs)) + module, err := ast.ParseModuleWithOpts(policy, string(bs), popts) if err != nil { return nil, err } @@ -1085,3 +1093,7 @@ func (m *Manager) sendOPAUpdateLoop(ctx context.Context) { } } } + +func (m *Manager) ParserOptions() ast.ParserOptions { + return m.parserOptions +} diff --git a/repl/repl.go b/repl/repl.go index b7a82d7fa0..fe101144dc 100644 --- a/repl/repl.go +++ b/repl/repl.go @@ -51,6 +51,7 @@ type REPL struct { profiler bool strictBuiltinErrors bool capabilities *ast.Capabilities + v1Compatible bool // TODO(tsandall): replace this state with rule definitions // inside the default module. @@ -344,6 +345,11 @@ func (r *REPL) WithRuntime(term *ast.Term) *REPL { return r } +func (r *REPL) WithV1Compatible(v1Compatible bool) *REPL { + r.v1Compatible = v1Compatible + return r +} + // SetOPAVersionReport sets the information about the latest OPA release. func (r *REPL) SetOPAVersionReport(report [][2]string) { r.mtx.Lock() @@ -762,6 +768,12 @@ func (r *REPL) compileRule(ctx context.Context, rule *ast.Rule) error { var unset bool + if r.v1Compatible { + if errs := ast.CheckRegoV1(rule); errs != nil { + return errs + } + } + if rule.Head.Assign { var err error unset, err = r.unsetRule(ctx, rule.Head.Name) @@ -892,8 +904,19 @@ func (r *REPL) evalBufferMulti(ctx context.Context) error { } func (r *REPL) parserOptions() (ast.ParserOptions, error) { + if r.v1Compatible { + return ast.ParserOptions{RegoVersion: ast.RegoV1}, nil + } if r.currentModuleID != "" { - return future.ParserOptionsFromFutureImports(r.modules[r.currentModuleID].Imports) + opts, err := future.ParserOptionsFromFutureImports(r.modules[r.currentModuleID].Imports) + if err == nil { + for _, i := range r.modules[r.currentModuleID].Imports { + if ast.Compare(i.Path.Value, ast.RegoV1CompatibleRef) == 0 { + opts.RegoVersion = ast.RegoV1 + } + } + } + return opts, err } return ast.ParserOptions{}, nil } @@ -1241,7 +1264,12 @@ func (r *REPL) loadModules(ctx context.Context, txn storage.Transaction) (map[st return nil, err } - parsed, err := ast.ParseModule(id, string(bs)) + popts := ast.ParserOptions{} + if r.v1Compatible { + popts.RegoVersion = ast.RegoV1 + } + + parsed, err := ast.ParseModuleWithOpts(id, string(bs), popts) if err != nil { return nil, err } diff --git a/repl/repl_test.go b/repl/repl_test.go index 8b740b44fd..44366156fb 100644 --- a/repl/repl_test.go +++ b/repl/repl_test.go @@ -1039,6 +1039,264 @@ func TestOneShotJSON(t *testing.T) { } } +func TestOneShotV1Compatible(t *testing.T) { + type action struct { + line string + expOutput string + expErrs []string + } + tests := []struct { + note string + actions []action + v1Compatible bool + }{ + { + note: "v0.x, keywords used", + actions: []action{ + { + line: "a contains 2 if { true }", + expErrs: []string{"rego_unsafe_var_error: var a is unsafe"}, + }, + }, + }, + { + note: "v0.x, keywords not used", + actions: []action{ + { + line: "a[2] { true }", + expOutput: "Rule 'a' defined in package repl. Type 'show' to see rules.\n", + }, + }, + }, + { + note: "v0.x, keywords imported", + actions: []action{ + { + line: "import future.keywords", + }, + { + line: "a contains 2 if { true }", + expOutput: "Rule 'a' defined in package repl. Type 'show' to see rules.\n", + }, + }, + }, + { + note: "v0.x, rego.v1 imported", + actions: []action{ + { + line: "import rego.v1", + }, + { + line: "a contains 2 if { true }", + expOutput: "Rule 'a' defined in package repl. Type 'show' to see rules.\n", + }, + }, + }, + { + note: "v1.0, keywords not used", + v1Compatible: true, + actions: []action{ + { + line: "a[2] { true }", + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + }, + }, + { + note: "v1.0, keywords used, not imported", + v1Compatible: true, + actions: []action{ + { + line: "a contains 2 if { true }", + expOutput: "Rule 'a' defined in package repl. Type 'show' to see rules.\n", + }, + }, + }, + { + note: "v1.0, keywords used, keywords imported", + v1Compatible: true, + actions: []action{ + { + line: "import future.keywords", + }, + { + line: "a contains 2 if { true }", + expOutput: "Rule 'a' defined in package repl. Type 'show' to see rules.\n", + }, + }, + }, + { + note: "v1.0, keywords used, rego.v1 imported", + v1Compatible: true, + actions: []action{ + { + line: "import rego.v1", + }, + { + line: "a contains 2 if { true }", + expOutput: "Rule 'a' defined in package repl. Type 'show' to see rules.\n", + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx := context.Background() + store := newTestStore() + var buffer bytes.Buffer + repl := newRepl(store, &buffer). + WithV1Compatible(tc.v1Compatible) + + for _, action := range tc.actions { + err := repl.OneShot(ctx, action.line) + + if len(action.expErrs) != 0 { + if err == nil { + t.Fatalf("Expected error but got: %s", buffer.String()) + } + + for _, e := range action.expErrs { + if !strings.Contains(err.Error(), e) { + t.Fatalf("Expected error to contain:\n\n%q\n\nbut got:\n\n%v", e, err) + } + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expectOutput(t, buffer.String(), action.expOutput) + } + } + }) + } +} + +func TestStoredModuleV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + module string + line string + expOutput string + expErrs []string + }{ + { + note: "v0.x keywords not used", + module: `package example +p[2] { 1 == 1 }`, + line: "data.example.p", + expOutput: "[\n 2\n]\n", + }, + { + note: "v0.x, keywords not imported but used", + module: `package example +p contains 2 if { 1 == 1 }`, + line: "data.example.p", + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + module: `package example +import future.keywords +p contains 2 if { 1 == 1 }`, + line: "data.example.p", + expOutput: "[\n 2\n]\n", + }, + { + note: "v0.x, rego.v1 imported", + module: `package example +import rego.v1 +p contains 2 if { 1 == 1 }`, + line: "data.example.p", + expOutput: "[\n 2\n]\n", + }, + { + note: "v1.0, keywords not used", + v1Compatible: true, + module: `package example +p[2] { 1 == 1 }`, + line: "data.example.p", + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, keywords not imported", + v1Compatible: true, + module: `package example +p contains 2 if { 1 == 1 }`, + line: "data.example.p", + expOutput: "[\n 2\n]\n", + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + module: `package example +import future.keywords +p contains 2 if { 1 == 1 }`, + line: "data.example.p", + expOutput: "[\n 2\n]\n", + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + module: `package example +import rego.v1 +p contains 2 if { 1 == 1 }`, + line: "data.example.p", + expOutput: "[\n 2\n]\n", + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx := context.Background() + store := newTestStore() + + txn := storage.NewTransactionOrDie(ctx, store, storage.WriteParams) + if err := store.UpsertPolicy(ctx, txn, "policy", []byte(tc.module)); err != nil { + t.Fatalf("Unexpected error upserting policy: %v", err) + } + + if err := store.Commit(ctx, txn); err != nil { + t.Fatalf("Unexpected error committing store transaction: %v", err) + } + + var buffer bytes.Buffer + repl := newRepl(store, &buffer). + WithV1Compatible(tc.v1Compatible) + + err := repl.OneShot(ctx, tc.line) + + if len(tc.expErrs) != 0 { + if err == nil { + t.Fatalf("Expected error but got: %s", buffer.String()) + } + + for _, e := range tc.expErrs { + if !strings.Contains(err.Error(), e) { + t.Fatalf("Expected error to contain:\n\n%q\n\nbut got:\n\n%v", e, err) + } + } + } else { + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + expectOutput(t, buffer.String(), tc.expOutput) + } + }) + } +} + func TestEvalData(t *testing.T) { ctx := context.Background() store := newTestStore() diff --git a/runtime/runtime.go b/runtime/runtime.go index 848d7a883f..8dc95cafe9 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -28,6 +28,7 @@ import ( "go.opentelemetry.io/otel/propagation" "go.uber.org/automaxprocs/maxprocs" + "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" opa_config "github.com/open-policy-agent/opa/config" "github.com/open-policy-agent/opa/internal/compiler" @@ -336,7 +337,13 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { } } - loaded, err := initload.LoadPaths(params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, false, nil, nil) + var regoVersion ast.RegoVersion + if params.V1Compatible { + regoVersion = ast.RegoV1 + } else { + regoVersion = ast.RegoV0 + } + loaded, err := initload.LoadPathsForRegoVersion(regoVersion, params.Paths, params.Filter, params.BundleMode, params.BundleVerificationConfig, params.SkipBundleVerification, false, nil, nil) if err != nil { return nil, fmt.Errorf("load error: %w", err) } @@ -408,7 +415,8 @@ func NewRuntime(ctx context.Context, params Params) (*Runtime, error) { plugins.WithRouter(params.Router), plugins.WithPrometheusRegister(metrics), plugins.WithTracerProvider(tracerProvider), - plugins.WithEnableTelemetry(params.EnableVersionCheck)) + plugins.WithEnableTelemetry(params.EnableVersionCheck), + plugins.WithParserOptions(ast.ParserOptions{RegoVersion: regoVersion})) if err != nil { return nil, fmt.Errorf("config error: %w", err) } @@ -700,7 +708,8 @@ func (rt *Runtime) StartREPL(ctx context.Context) { banner := rt.getBanner() repl := repl.New(rt.Store, rt.Params.HistoryPath, rt.Params.Output, rt.Params.OutputFormat, rt.Params.ErrorLimit, banner). - WithRuntime(rt.Manager.Info) + WithRuntime(rt.Manager.Info). + WithV1Compatible(rt.Params.V1Compatible) if rt.Params.Watch { if err := rt.startWatcher(ctx, rt.Params.Paths, onReloadPrinter(rt.Params.Output)); err != nil { @@ -811,13 +820,14 @@ func (rt *Runtime) readWatcher(ctx context.Context, watcher *fsnotify.Watcher, p func (rt *Runtime) processWatcherUpdate(ctx context.Context, paths []string, removed string) error { - return pathwatcher.ProcessWatcherUpdate(ctx, paths, removed, rt.Store, rt.Params.Filter, rt.Params.BundleMode, func(ctx context.Context, txn storage.Transaction, loaded *initload.LoadPathsResult) error { + return pathwatcher.ProcessWatcherUpdateForRegoVersion(ctx, rt.Manager.ParserOptions().RegoVersion, paths, removed, rt.Store, rt.Params.Filter, rt.Params.BundleMode, func(ctx context.Context, txn storage.Transaction, loaded *initload.LoadPathsResult) error { _, err := initload.InsertAndCompile(ctx, initload.InsertAndCompileOptions{ - Store: rt.Store, - Txn: txn, - Files: loaded.Files, - Bundles: loaded.Bundles, - MaxErrors: -1, + Store: rt.Store, + Txn: txn, + Files: loaded.Files, + Bundles: loaded.Bundles, + MaxErrors: -1, + ParserOptions: rt.Manager.ParserOptions(), }) return err diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index dc90cff457..a2531b163c 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -8,6 +8,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "os" @@ -18,6 +19,8 @@ import ( "testing" "time" + "github.com/open-policy-agent/opa/loader" + "github.com/open-policy-agent/opa/internal/report" "github.com/open-policy-agent/opa/logging" testLog "github.com/open-policy-agent/opa/logging/test" @@ -231,6 +234,317 @@ func testRuntimeProcessWatchEventPolicyError(t *testing.T, asBundle bool) { }) } +func TestRuntimeReplProcessWatchV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + policy string + expErrs []string + expOutput string + }{ + { + note: "v0.x, keywords not used", + policy: `package test +p[1] { + data.foo == "bar" +}`, + }, + { + note: "v0.x, keywords not imported", + policy: `package test +p contains 1 if { + data.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + policy: `package test +import future.keywords +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v0.x, rego.v1 imported", + policy: `package test +import rego.v1 +p contains 1 if { + data.foo == "bar" +}`, + }, + + { + note: "v1.0, keywords not used", + v1Compatible: true, + policy: `package test +p[1] { + data.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, keywords not imported", + v1Compatible: true, + policy: `package test +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + policy: `package test +import future.keywords +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + policy: `package test +import rego.v1 +p contains 1 if { + data.foo == "bar" +}`, + }, + } + + fs := map[string]string{ + "test/data.json": `{"foo": "bar"}`, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + test.WithTempFS(fs, func(rootDir string) { + // Prefix the directory intended to be watched with at least one + // directory to avoid permission issues on the local host. Otherwise, we + // cannot always watch the tmp directory's parent. + rootDir = filepath.Join(rootDir, "test") + + output := test.BlockingWriter{} + + params := NewParams() + params.Output = &output + params.Paths = []string{rootDir} + params.Watch = true + params.V1Compatible = tc.v1Compatible + + rt, err := NewRuntime(ctx, params) + if err != nil { + t.Fatal(err) + } + + go rt.StartREPL(ctx) + + if !test.Eventually(t, 5*time.Second, func() bool { + return strings.Contains(output.String(), "Run 'help' to see a list of commands and check for updates.") + }) { + t.Fatal("Timed out waiting for REPL to start") + } + output.Reset() + + // write new policy to disk, to trigger the watcher + if err := os.WriteFile(path.Join(rootDir, "authz.rego"), []byte(tc.policy), 0644); err != nil { + t.Fatal(err) + } + + if !test.Eventually(t, 5*time.Second, func() bool { + if tc.expErrs != nil { + return strings.Contains(output.String(), "# reload error") + } + return strings.Contains(output.String(), "# reloaded files") + }) { + t.Fatal("Timed out waiting for watcher") + } + + for _, expErr := range tc.expErrs { + if !strings.Contains(output.String(), expErr) { + t.Fatalf("Expected error:\n\n%v\n\ngot output:\n\n%s", expErr, output.String()) + } + } + }) + }) + } +} + +func TestRuntimeServerProcessWatchV1Compatible(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + policy string + expErrs []string + expOutput string + }{ + { + note: "v0.x, keywords not used", + policy: `package test +p[1] { + data.foo == "bar" +}`, + }, + { + note: "v0.x, keywords not imported", + policy: `package test +p contains 1 if { + data.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "v0.x, keywords imported", + policy: `package test +import future.keywords +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v0.x, rego.v1 imported", + policy: `package test +import rego.v1 +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v1.0, keywords not used", + v1Compatible: true, + policy: `package test +p[1] { + data.foo == "bar" +}`, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0, keywords not imported", + v1Compatible: true, + policy: `package test +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v1.0, keywords imported", + v1Compatible: true, + policy: `package test +import future.keywords +p contains 1 if { + data.foo == "bar" +}`, + }, + { + note: "v1.0, rego.v1 imported", + v1Compatible: true, + policy: `package test +import rego.v1 +p contains 1 if { + data.foo == "bar" +}`, + }, + } + + fs := map[string]string{ + "test/data.json": `{"foo": "bar"}`, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + test.WithTempFS(fs, func(rootDir string) { + // Prefix the directory intended to be watched with at least one + // directory to avoid permission issues on the local host. Otherwise, we + // cannot always watch the tmp directory's parent. + rootDir = filepath.Join(rootDir, "test") + + testLogger := testLog.New() + + params := NewParams() + params.Logger = testLogger + params.Addrs = &[]string{"localhost:0"} + params.AddrSetByUser = true + params.Paths = []string{rootDir} + params.Watch = true + params.V1Compatible = tc.v1Compatible + + rt, err := NewRuntime(ctx, params) + if err != nil { + t.Fatal(err) + } + + go rt.StartServer(ctx) + + if !test.Eventually(t, 5*time.Second, func() bool { + found := false + for _, e := range testLogger.Entries() { + found = strings.Contains(e.Message, "Server initialized.") || found + } + return found + }) { + t.Fatal("Timed out waiting for server to start") + } + + // write new policy to disk, to trigger the watcher + if err := os.WriteFile(path.Join(rootDir, "authz.rego"), []byte(tc.policy), 0644); err != nil { + t.Fatal(err) + } + + if tc.expErrs != nil { + // wait for errors + if !test.Eventually(t, 5*time.Second, func() bool { + for _, expErr := range tc.expErrs { + found := false + for _, e := range testLogger.Entries() { + if errs, ok := e.Fields["err"].(loader.Errors); ok { + for _, err := range errs { + found = strings.Contains(err.Error(), expErr) || found + } + } + } + if !found { + return false + } + } + return true + }) { + t.Fatalf("Timed out waiting for watcher. Expected errors:\n\n%v\n\ngot output:\n\n%v", + tc.expErrs, testLogger.Entries()) + } + } else { + // wait for successful reload + if !test.Eventually(t, 5*time.Second, func() bool { + found := false + for _, e := range testLogger.Entries() { + found = strings.Contains(e.Message, "Processed file watch event.") || found + } + return found + }) { + t.Fatal("Timed out waiting for watcher") + } + } + }) + }) + } +} + func TestCheckOPAUpdateBadURL(t *testing.T) { testCheckOPAUpdate(t, "http://foo:8112", nil) } @@ -443,6 +757,159 @@ func TestServerInitialized(t *testing.T) { t.Fatal("expected ServerInitializedChannel to be closed") } } + +func TestServerInitializedWithRegoV1(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + files map[string]string + expErr string + }{ + { + note: "Rego v0, keywords not imported", + files: map[string]string{ + "policy.rego": `package test + p if { + input.x == 1 + } + `, + }, + expErr: "rego_parse_error: var cannot be used for rule name", + }, + { + note: "Rego v0, rego.v1 imported", + files: map[string]string{ + "policy.rego": `package test + import rego.v1 + p if { + input.x == 1 + } + `, + }, + }, + { + note: "Rego v0, future.keywords imported", + files: map[string]string{ + "policy.rego": `package test + import future.keywords.if + p if { + input.x == 1 + } + `, + }, + }, + { + note: "Rego v0, no keywords used", + files: map[string]string{ + "policy.rego": `package test + p { + input.x == 1 + } + `, + }, + }, + { + note: "Rego v1, keywords not imported", + v1Compatible: true, + files: map[string]string{ + "policy.rego": `package test + p if { + input.x == 1 + } + `, + }, + }, + { + note: "Rego v1, rego.v1 imported", + v1Compatible: true, + files: map[string]string{ + "policy.rego": `package test + import rego.v1 + p if { + input.x == 1 + } + `, + }, + }, + { + note: "Rego v1, future.keywords imported", + v1Compatible: true, + files: map[string]string{ + "policy.rego": `package test + import future.keywords.if + p if { + input.x == 1 + } + `, + }, + }, + { + note: "Rego v1, no keywords used", + v1Compatible: true, + files: map[string]string{ + "policy.rego": `package test + p { + input.x == 1 + } + `, + }, + expErr: "rego_parse_error: `if` keyword is required before rule body", + }, + } + + bundle := []bool{false, true} + + for _, tc := range tests { + for _, b := range bundle { + t.Run(fmt.Sprintf("%s; bundle=%v", tc.note, b), func(t *testing.T) { + test.WithTempFS(tc.files, func(root string) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Millisecond) + defer cancel() + var output bytes.Buffer + + params := NewParams() + params.Output = &output + params.Paths = []string{root} + params.BundleMode = b + params.Addrs = &[]string{"localhost:0"} + params.GracefulShutdownPeriod = 1 + params.Logger = logging.NewNoOpLogger() + params.V1Compatible = tc.v1Compatible + + rt, err := NewRuntime(ctx, params) + + if tc.expErr != "" { + if err == nil { + t.Fatal("Expected error but got nil") + } + if !strings.Contains(err.Error(), tc.expErr) { + t.Fatalf("Expected error:\n\n%v\n\ngot:\n\n%v", tc.expErr, err.Error()) + } + } else { + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + + initChannel := rt.Manager.ServerInitializedChannel() + done := make(chan struct{}) + go func() { + rt.StartServer(ctx) + close(done) + }() + <-done + select { + case <-initChannel: + return + default: + t.Fatal("expected ServerInitializedChannel to be closed") + } + } + }) + }) + } + } +} + func TestUrlPathToConfigOverride(t *testing.T) { params := NewParams() params.Paths = []string{"https://www.example.com/bundles/bundle.tar.gz"} diff --git a/sdk/opa.go b/sdk/opa.go index 93ad613896..f4bf540379 100644 --- a/sdk/opa.go +++ b/sdk/opa.go @@ -38,15 +38,16 @@ import ( // OPA represents an instance of the policy engine. OPA can be started with // several options that control configuration, logging, and lifecycle. type OPA struct { - id string - state *state - mtx sync.Mutex - logger logging.Logger - console logging.Logger - plugins map[string]plugins.Factory - store storage.Store - hooks hooks.Hooks - config []byte + id string + state *state + mtx sync.Mutex + logger logging.Logger + console logging.Logger + plugins map[string]plugins.Factory + store storage.Store + hooks hooks.Hooks + config []byte + v1Compatible bool } type state struct { @@ -86,6 +87,7 @@ func New(ctx context.Context, opts Options) (*OPA, error) { opa.logger = opts.Logger opa.console = opts.ConsoleLogger opa.plugins = opts.Plugins + opa.v1Compatible = opts.V1Compatible return opa, opa.configure(ctx, opa.config, opts.Ready, opts.block) } @@ -128,16 +130,22 @@ func (opa *OPA) configure(ctx context.Context, bs []byte, ready chan struct{}, b return err } - manager, err := plugins.New( - bs, - opa.id, - opa.store, + opts := []func(*plugins.Manager){ plugins.Info(info), plugins.Logger(opa.logger), plugins.ConsoleLogger(opa.console), plugins.EnablePrintStatements(opa.logger.GetLevel() >= logging.Info), plugins.PrintHook(loggingPrintHook{logger: opa.logger}), plugins.WithHooks(opa.hooks), + } + if opa.v1Compatible { + opts = append(opts, plugins.WithParserOptions(ast.ParserOptions{RegoVersion: ast.RegoV1})) + } + manager, err := plugins.New( + bs, + opa.id, + opa.store, + opts..., ) if err != nil { return err diff --git a/sdk/opa_test.go b/sdk/opa_test.go index 4f1f8291b8..10138b9579 100644 --- a/sdk/opa_test.go +++ b/sdk/opa_test.go @@ -11,6 +11,8 @@ import ( "fmt" "net/http" "net/http/httptest" + "os" + "path/filepath" "reflect" "strings" "testing" @@ -23,6 +25,7 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/config" "github.com/open-policy-agent/opa/hooks" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/logging" loggingtest "github.com/open-policy-agent/opa/logging/test" "github.com/open-policy-agent/opa/metrics" @@ -35,6 +38,7 @@ import ( "github.com/open-policy-agent/opa/topdown" "github.com/open-policy-agent/opa/topdown/builtins" "github.com/open-policy-agent/opa/topdown/lineage" + "github.com/open-policy-agent/opa/util/test" "github.com/open-policy-agent/opa/version" ) @@ -1796,6 +1800,434 @@ main = 7 } +func TestDiscoveryBundleRegoV1(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + discoveryBundle map[string]string + policyBundle map[string]string + expErr string + }{ + { + note: "0.x compatible, keywords not imported ind disco bundle", + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +foo contains "bar" + +# This will be interpreted as two rules - 'test.resource' and 'if' - so we have foo above to force an error +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +import future.keywords + +main := v if { v := 7 }`, + }, + expErr: "rego_parse_error", + }, + + { + note: "0.x compatible, keywords not imported ind policy bundle", + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import future.keywords + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +foo contains "bar" + +# This will be interpreted as two rules - 'main' and 'if' - so we have foo above to force an error +main := v if { v := 7 }`, + }, + expErr: "rego_parse_error", + }, + + { + note: "0.x compatible, keywords imported", + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import future.keywords + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +import future.keywords + +main := v if { v := 7 }`, + }, + }, + { + note: "0.x compatible, rego.v1 imported", + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import rego.v1 + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +import rego.v1 + +main := v if { v := 7 }`, + }, + }, + { + note: "1.0 compatible, keywords not imported", + v1Compatible: true, + // Discovery and policy bundles are rego-v1 compatible, but rego-v0 incompatible (if keyword used without import) + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +main := v if { v := 7 }`, + }, + }, + { + note: "1.0 compatible, keywords imported", + v1Compatible: true, + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import future.keywords + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +import future.keywords + +main := v if { v := 7 }`, + }, + }, + { + note: "1.0 compatible, rego.v1 imported", + v1Compatible: true, + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import rego.v1 + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +import rego.v1 + +main := v if { v := 7 }`, + }, + }, + { + note: "1.0 compatible, keywords not used in discovery bundle", + v1Compatible: true, + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import rego.v1 + +test.resource := b { + b := "/bundles/bundle.tar.gz" +}`, + }, + expErr: "rego_parse_error", + }, + { + note: "1.0 compatible, keywords not used in policy bundle", + v1Compatible: true, + discoveryBundle: map[string]string{ + "bundles.rego": ` +package bundles + +import rego.v1 + +test.resource := b if { + b := "/bundles/bundle.tar.gz" +}`, + }, + policyBundle: map[string]string{ + "main.rego": ` +package system + +import rego.v1 + +main := v { v := 7 }`, + }, + expErr: "rego_parse_error", + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx := context.Background() + + serverOpts := []func(*sdktest.Server) error{ + sdktest.MockBundle("/bundles/discovery.tar.gz", tc.discoveryBundle), + sdktest.MockBundle("/bundles/bundle.tar.gz", tc.policyBundle), + sdktest.RawBundles(true), + } + server := sdktest.MustNewServer(serverOpts...) + defer server.Stop() + + c := fmt.Sprintf(`{ + "services": { + "test": { + "url": %q + } + }, + "discovery": { + "resource": "/bundles/discovery.tar.gz" + } + }`, server.URL()) + + var readyCh chan struct{} + var logger logging.Logger + if tc.expErr != "" { + logger = loggingtest.New() + logger.SetLevel(logging.Info) + readyCh = make(chan struct{}) + } else { + logger = logging.NewNoOpLogger() + } + + opa, err := sdk.New(ctx, sdk.Options{ + Logger: logger, + Ready: readyCh, + Config: strings.NewReader(c), + V1Compatible: tc.v1Compatible, + }) + if err != nil { + t.Fatal(err) + } + + defer opa.Stop(ctx) + + if tc.expErr != "" { + l := logger.(*loggingtest.Logger) + if !test.Eventually(t, 5*time.Second, func() bool { + for _, e := range l.Entries() { + if strings.Contains(e.Message, tc.expErr) { + return true + } + } + return false + }) { + t.Fatalf("timed out waiting for logged error:\n\n%s\n\ngot\n\n%v:", tc.expErr, l.Entries()) + } + } else { + exp := json.Number("7") + + if result, err := opa.Decision(ctx, sdk.DecisionOptions{}); err != nil { + t.Fatal(err) + } else if result.Result != exp { + t.Fatalf("expected %v but got %v", exp, result.Result) + } + } + }) + } +} + +func TestRegoV1WithConfiguredLocalBundle(t *testing.T) { + tests := []struct { + note string + v1Compatible bool + policy string + expErr string + }{ + { + note: "0.x compatible, keywords not imported", + policy: ` +package system + +l contains 7 + +main := v if { + v := l[0] +} +`, + expErr: "rego_parse_error", + }, + { + note: "0.x compatible, keywords imported", + policy: ` +package system + +import future.keywords + +main := 7 if { + true +} +`, + }, + { + note: "0.x compatible, rego.v1 imported", + policy: ` +package system + +import rego.v1 + +main := 7 if { + true +} +`, + }, + { + note: "1.0 compatible, keywords not imported", + v1Compatible: true, + policy: ` +package system + +main := 7 if { + true +} +`, + }, + { + note: "1.0 compatible, keywords imported", + v1Compatible: true, + policy: ` +package system + +import future.keywords + +main := 7 if { + true +} +`, + }, + { + note: "1.0 compatible, rego.v1 imported", + v1Compatible: true, + policy: ` +package system + +import rego.v1 + +main := 7 if { + true +} +`, + }, + { + note: "1.0 compatible, keywords not used", + v1Compatible: true, + policy: ` +package system + +main := 7 { + true +} +`, + expErr: "rego_parse_error", + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + test.WithTempFS(map[string]string{}, func(rootDir string) { + f, err := os.Create(filepath.Join(rootDir, "bundle.tar.gz")) + if err != nil { + t.Fatal(err) + } + buf := archive.MustWriteTarGz([][2]string{{"main.rego", tc.policy}}) + _, err = f.Write(buf.Bytes()) + if err != nil { + t.Fatal(err) + } + + c := fmt.Sprintf(`services: +bundles: + test: + resource: "file://%s/bundle.tar.gz"`, rootDir) + + var readyCh chan struct{} + logger := loggingtest.New() + logger.SetLevel(logging.Info) + if tc.expErr != "" { + readyCh = make(chan struct{}) + } + + ctx := context.Background() + opa, err := sdk.New(ctx, sdk.Options{ + Logger: logger, + Ready: readyCh, + Config: strings.NewReader(c), + V1Compatible: tc.v1Compatible, + }) + if err != nil { + t.Fatal(err) + } + + if tc.expErr != "" { + if !test.Eventually(t, 5*time.Second, func() bool { + entries := logger.Entries() + for _, e := range entries { + if strings.Contains(e.Message, tc.expErr) { + return true + } + } + return false + }) { + t.Fatalf("timed out waiting for logged error:\n\n%s\n\ngot\n\n%v:", tc.expErr, logger.Entries()) + } + } else { + exp := json.Number("7") + + if result, err := opa.Decision(ctx, sdk.DecisionOptions{}); err != nil { + t.Fatal(err) + } else if result.Result != exp { + t.Fatalf("expected %v but got %v", exp, result.Result) + } + } + }) + }) + } +} + func TestAsync(t *testing.T) { ctx := context.Background() diff --git a/sdk/options.go b/sdk/options.go index 4c39b842ee..3a25ba6ce5 100644 --- a/sdk/options.go +++ b/sdk/options.go @@ -55,6 +55,8 @@ type Options struct { // Hooks allows hooking into the internals of SDK operations (TODO(sr): find better words) Hooks hooks.Hooks + V1Compatible bool + config []byte block bool } diff --git a/sdk/test/test.go b/sdk/test/test.go index 4c4bb2f78d..d70a03e80d 100644 --- a/sdk/test/test.go +++ b/sdk/test/test.go @@ -18,6 +18,7 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/compile" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -59,9 +60,10 @@ func Ready(ch chan struct{}) func(*Server) error { // Server provides a mock HTTP server for testing the SDK and integrations. type Server struct { - server *httptest.Server - ready chan struct{} - bundles map[string]map[string]string + server *httptest.Server + ready chan struct{} + bundles map[string]map[string]string + rawBundles bool } // MustNewServer returns a new Server for test purposes or panics if an error occurs. @@ -91,6 +93,13 @@ func NewServer(opts ...func(*Server) error) (*Server, error) { return s, nil } +func RawBundles(raw bool) func(*Server) error { + return func(s *Server) error { + s.rawBundles = raw + return nil + } +} + // WithTestBundle adds a bundle to the server at the specified endpoint. func (s *Server) WithTestBundle(endpoint string, policies map[string]string) *Server { s.bundles[endpoint] = policies @@ -228,7 +237,11 @@ func (s *Server) handle(w http.ResponseWriter, r *http.Request) { } if strings.HasPrefix(r.URL.Path, "/bundles") { - s.handleBundles(w, r) + if s.rawBundles { + s.handleRawBundles(w, r) + } else { + s.handleBundles(w, r) + } return } @@ -436,3 +449,22 @@ func (s *Server) handleBundles(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) _, _ = io.Copy(w, buf) } + +func (s *Server) handleRawBundles(w http.ResponseWriter, r *http.Request) { + // Return 404 if bundle path does not exist. + b, ok := s.bundles[r.URL.Path] + if !ok { + w.WriteHeader(http.StatusNotFound) + return + } + + files := make([][2]string, 0, len(b)) + for url, str := range b { + files = append(files, [2]string{url, str}) + } + buf := archive.MustWriteTarGz(files) + + // Write out the bundle + w.WriteHeader(http.StatusOK) + _, _ = io.Copy(w, buf) +} diff --git a/util/test/tempus.go b/util/test/tempus.go index 6ada673598..cd53d9a9b6 100644 --- a/util/test/tempus.go +++ b/util/test/tempus.go @@ -5,6 +5,8 @@ package test import ( + "bytes" + "sync" "testing" "time" ) @@ -20,3 +22,26 @@ func Eventually(t *testing.T, timeout time.Duration, f func() bool) bool { } return false } + +type BlockingWriter struct { + m sync.Mutex + buf bytes.Buffer +} + +func (w *BlockingWriter) Write(p []byte) (n int, err error) { + w.m.Lock() + defer w.m.Unlock() + return w.buf.Write(p) +} + +func (w *BlockingWriter) String() string { + w.m.Lock() + defer w.m.Unlock() + return w.buf.String() +} + +func (w *BlockingWriter) Reset() { + w.m.Lock() + defer w.m.Unlock() + w.buf.Reset() +}