From e23d771711df4de659f9e28aadd7d736431701df Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Wed, 10 Apr 2024 19:06:07 +0200 Subject: [PATCH] Add `rego_version` attribute to bundle manifest (#6579) Adding a global `rego_version` attribute to bundle manifest, to inform OPA runtime about what rego-version (v0/v1) to use to parse/compile contained Rego files. The rego-version of individual Rego files can be overridden through the `file_rego_versions` manifest attribute. Implements: #6578 Signed-off-by: Johan Fylling --- ast/parser.go | 14 + bundle/bundle.go | 248 ++++- bundle/bundle_test.go | 96 +- cmd/bench_test.go | 200 ++++ cmd/build_test.go | 653 ++++++++++++- cmd/check_test.go | 420 ++++++++ cmd/deps_test.go | 322 +++++++ cmd/eval_test.go | 304 ++++++ cmd/exec_test.go | 349 +++++++ cmd/inspect.go | 8 +- cmd/inspect_test.go | 335 +++++++ cmd/test_test.go | 334 +++++++ compile/compile.go | 9 +- compile/compile_test.go | 668 +++++++++++++ download/download_test.go | 129 +++ download/oci_download_test.go | 9 +- download/testdata/rego_v1.manifest | 4 +- download/testdata/rego_v1.tar.gz | Bin 695 -> 830 bytes download/testdata/signed.manifest | 4 +- download/testdata/signed.tar.gz | Bin 629 -> 764 bytes plugins/bundle/plugin_test.go | 1385 ++++++++++++++++++++++++++- plugins/discovery/discovery_test.go | 371 +++++++ runtime/runtime_test.go | 321 +++++++ 23 files changed, 6130 insertions(+), 53 deletions(-) diff --git a/ast/parser.go b/ast/parser.go index 75dc61a983..98f9c56bd6 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -43,6 +43,20 @@ const ( RegoV1 ) +func (v RegoVersion) Int() int { + if v == RegoV1 { + return 1 + } + return 0 +} + +func RegoVersionFromInt(i int) RegoVersion { + if i == 1 { + return RegoV1 + } + return RegoV0 +} + // Note: This state is kept isolated from the parser so that we // can do efficient shallow copies of these values when doing a // save() and restore(). diff --git a/bundle/bundle.go b/bundle/bundle.go index 035a1a9c81..09fd18d493 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -15,10 +15,12 @@ import ( "fmt" "io" "net/url" + "path" "path/filepath" "reflect" "strings" + "github.com/gobwas/glob" "github.com/open-policy-agent/opa/ast" astJSON "github.com/open-policy-agent/opa/ast/json" "github.com/open-policy-agent/opa/format" @@ -120,10 +122,28 @@ func NewFile(name, hash, alg string) FileInfo { // Manifest represents the manifest from a bundle. The manifest may contain // metadata such as the bundle revision. type Manifest struct { - Revision string `json:"revision"` - Roots *[]string `json:"roots,omitempty"` - WasmResolvers []WasmResolver `json:"wasm,omitempty"` - Metadata map[string]interface{} `json:"metadata,omitempty"` + Revision string `json:"revision"` + Roots *[]string `json:"roots,omitempty"` + WasmResolvers []WasmResolver `json:"wasm,omitempty"` + // RegoVersion is the global Rego version for the bundle described by this Manifest. + // The Rego version of individual files can be overridden in FileRegoVersions. + // We don't use ast.RegoVersion here, as this iota type's order isn't guaranteed to be stable over time. + // We use a pointer so that we can support hand-made bundles that don't have an explicit version appropriately. + // E.g. in OPA 0.x if --v1-compatible is used when consuming the bundle, and there is no specified version, + // we should default to v1; if --v1-compatible isn't used, we should default to v0. In OPA 1.0, no --x-compatible + // flag and no explicit bundle version should default to v1. + RegoVersion *int `json:"rego_version,omitempty"` + // FileRegoVersions is a map from file paths to Rego versions. + // This allows individual files to override the global Rego version specified by RegoVersion. + FileRegoVersions map[string]int `json:"file_rego_versions,omitempty"` + Metadata map[string]interface{} `json:"metadata,omitempty"` + + compiledFileRegoVersions []fileRegoVersion +} + +type fileRegoVersion struct { + path glob.Glob + version int } // WasmResolver maps a wasm module to an entrypoint ref. @@ -150,6 +170,15 @@ func (m *Manifest) AddRoot(r string) { } } +func (m *Manifest) SetRegoVersion(v ast.RegoVersion) { + m.Init() + regoVersion := 0 + if v == ast.RegoV1 { + regoVersion = 1 + } + m.RegoVersion = ®oVersion +} + // Equal returns true if m is semantically equivalent to other. func (m Manifest) Equal(other Manifest) bool { @@ -161,6 +190,19 @@ func (m Manifest) Equal(other Manifest) bool { return false } + if m.RegoVersion == nil && other.RegoVersion != nil { + return false + } + if m.RegoVersion != nil && other.RegoVersion == nil { + return false + } + if m.RegoVersion != nil && other.RegoVersion != nil && *m.RegoVersion != *other.RegoVersion { + return false + } + if !reflect.DeepEqual(m.FileRegoVersions, other.FileRegoVersions) { + return false + } + if !reflect.DeepEqual(m.Metadata, other.Metadata) { return false } @@ -197,7 +239,12 @@ func (m Manifest) Copy() Manifest { func (m Manifest) String() string { m.Init() - return fmt.Sprintf("", m.Revision, *m.Roots, m.WasmResolvers, m.Metadata) + if m.RegoVersion != nil { + return fmt.Sprintf("", + m.Revision, *m.RegoVersion, *m.Roots, m.WasmResolvers, m.Metadata) + } + return fmt.Sprintf("", + m.Revision, *m.Roots, m.WasmResolvers, m.Metadata) } func (m Manifest) rootSet() stringSet { @@ -358,10 +405,11 @@ func (m *Manifest) validateAndInjectDefaults(b Bundle) error { // ModuleFile represents a single module contained in a bundle. type ModuleFile struct { - URL string - Path string - Raw []byte - Parsed *ast.Module + URL string + Path string + RelativePath string + Raw []byte + Parsed *ast.Module } // WasmModuleFile represents a single wasm module contained in a bundle. @@ -543,6 +591,7 @@ func (r *Reader) Read() (Bundle, error) { bundle.Data = map[string]interface{}{} } + var modules []ModuleFile for _, f := range descriptors { buf, err := readFile(f, r.sizeLimitBytes) if err != nil { @@ -583,20 +632,14 @@ func (r *Reader) Read() (Bundle, error) { raw = append(raw, Raw{Path: p, Value: bs}) } - r.metrics.Timer(metrics.RegoModuleParse).Start() - module, err := ast.ParseModuleWithOpts(fullPath, buf.String(), r.ParserOptions()) - r.metrics.Timer(metrics.RegoModuleParse).Stop() - if err != nil { - return bundle, err - } - + // Modules are parsed after we've had a chance to read the manifest mf := ModuleFile{ - URL: f.URL(), - Path: fullPath, - Raw: bs, - Parsed: module, + URL: f.URL(), + Path: fullPath, + RelativePath: path, + Raw: bs, } - bundle.Modules = append(bundle.Modules, mf) + modules = append(modules, mf) } else if filepath.Base(path) == WasmFile { bundle.WasmModules = append(bundle.WasmModules, WasmModuleFile{ URL: f.URL(), @@ -656,6 +699,23 @@ func (r *Reader) Read() (Bundle, error) { } } + // Parse modules + popts := r.ParserOptions() + popts.RegoVersion = bundle.RegoVersion(popts.RegoVersion) + for _, mf := range modules { + modulePopts := popts + if modulePopts.RegoVersion, err = bundle.RegoVersionForFile(mf.RelativePath, popts.RegoVersion); err != nil { + return bundle, err + } + r.metrics.Timer(metrics.RegoModuleParse).Start() + mf.Parsed, err = ast.ParseModuleWithOpts(mf.Path, string(mf.Raw), modulePopts) + r.metrics.Timer(metrics.RegoModuleParse).Stop() + if err != nil { + return bundle, err + } + bundle.Modules = append(bundle.Modules, mf) + } + if bundle.Type() == DeltaBundleType { if len(bundle.Data) != 0 { return bundle, fmt.Errorf("delta bundle expected to contain only patch file but data files found") @@ -1012,7 +1072,7 @@ func hashBundleFiles(hash SignatureHasher, b *Bundle) ([]FileInfo, error) { } // FormatModules formats Rego modules -// Modules will be formatted to comply with rego-v1, but Rego compatibility of individual parsed modules will be respected (e.g. if 'rego.v1' is imported). +// Modules will be formatted to comply with rego-v0, but Rego compatibility of individual parsed modules will be respected (e.g. if 'rego.v1' is imported). func (b *Bundle) FormatModules(useModulePath bool) error { return b.FormatModulesForRegoVersion(ast.RegoV0, true, useModulePath) } @@ -1111,6 +1171,65 @@ func (b *Bundle) ParsedModules(bundleName string) map[string]*ast.Module { return mods } +func (b *Bundle) RegoVersion(def ast.RegoVersion) ast.RegoVersion { + if v := b.Manifest.RegoVersion; v != nil { + if *v == 0 { + return ast.RegoV0 + } else if *v == 1 { + return ast.RegoV1 + } + } + return def +} + +func (b *Bundle) SetRegoVersion(v ast.RegoVersion) { + b.Manifest.SetRegoVersion(v) +} + +// RegoVersionForFile returns the rego-version for the specified file path. +// If there is no defined version for the given path, the default version def is returned. +// If the version does not correspond to ast.RegoV0 or ast.RegoV1, an error is returned. +func (b *Bundle) RegoVersionForFile(path string, def ast.RegoVersion) (ast.RegoVersion, error) { + if version, err := b.Manifest.numericRegoVersionForFile(path); err != nil { + return def, err + } else if version == nil { + return def, nil + } else if *version == 0 { + return ast.RegoV0, nil + } else if *version == 1 { + return ast.RegoV1, nil + } else { + return def, fmt.Errorf("unknown bundle rego-version %d for file '%s'", *version, path) + } +} + +func (m *Manifest) numericRegoVersionForFile(path string) (*int, error) { + var version *int + + if len(m.FileRegoVersions) != len(m.compiledFileRegoVersions) { + m.compiledFileRegoVersions = make([]fileRegoVersion, 0, len(m.FileRegoVersions)) + for pattern, v := range m.FileRegoVersions { + compiled, err := glob.Compile(pattern) + if err != nil { + return nil, fmt.Errorf("failed to compile glob pattern %s: %s", pattern, err) + } + m.compiledFileRegoVersions = append(m.compiledFileRegoVersions, fileRegoVersion{compiled, v}) + } + } + + for _, fv := range m.compiledFileRegoVersions { + if fv.path.Match(path) { + version = &fv.version + break + } + } + + if version == nil { + version = m.RegoVersion + } + return version, nil +} + // Equal returns true if this bundle's contents equal the other bundle's // contents. func (b Bundle) Equal(other Bundle) bool { @@ -1261,13 +1380,33 @@ func mktree(path []string, value interface{}) (map[string]interface{}, error) { // will have an empty revision except in the special case where a single bundle is provided // (and in that case the bundle is just returned unmodified.) func Merge(bundles []*Bundle) (*Bundle, error) { + return MergeWithRegoVersion(bundles, ast.RegoV0, false) +} + +// MergeWithRegoVersion creates a merged bundle from the provided bundles, similar to Merge. +// If more than one bundle is provided, the rego version of the result bundle is set to the provided regoVersion. +// Any Rego files in a bundle of conflicting rego version will be marked in the result's manifest with the rego version +// of its original bundle. If the Rego file already had an overriding rego version, it will be preserved. +// If a single bundle is provided, it will retain any rego version information it already had. If it has none, the +// provided regoVersion will be applied to it. +// If usePath is true, per-file rego-versions will be calculated using the file's ModuleFile.Path; otherwise, the file's +// ModuleFile.URL will be used. +func MergeWithRegoVersion(bundles []*Bundle, regoVersion ast.RegoVersion, usePath bool) (*Bundle, error) { if len(bundles) == 0 { return nil, errors.New("expected at least one bundle") } if len(bundles) == 1 { - return bundles[0], nil + result := bundles[0] + // We respect the bundle rego-version, defaulting to the provided rego version if not set. + result.SetRegoVersion(result.RegoVersion(regoVersion)) + fileRegoVersions, err := bundleRegoVersions(result, result.RegoVersion(regoVersion), usePath) + if err != nil { + return nil, err + } + result.Manifest.FileRegoVersions = fileRegoVersions + return result, nil } var roots []string @@ -1296,8 +1435,24 @@ func Merge(bundles []*Bundle) (*Bundle, error) { result.WasmModules = append(result.WasmModules, b.WasmModules...) result.PlanModules = append(result.PlanModules, b.PlanModules...) + if b.Manifest.RegoVersion != nil || len(b.Manifest.FileRegoVersions) > 0 { + if result.Manifest.FileRegoVersions == nil { + result.Manifest.FileRegoVersions = map[string]int{} + } + + fileRegoVersions, err := bundleRegoVersions(b, regoVersion, usePath) + if err != nil { + return nil, err + } + for k, v := range fileRegoVersions { + result.Manifest.FileRegoVersions[k] = v + } + } } + // We respect the bundle rego-version, defaulting to the provided rego version if not set. + result.SetRegoVersion(result.RegoVersion(regoVersion)) + if result.Data == nil { result.Data = map[string]interface{}{} } @@ -1311,6 +1466,53 @@ func Merge(bundles []*Bundle) (*Bundle, error) { return &result, nil } +func bundleRegoVersions(bundle *Bundle, regoVersion ast.RegoVersion, usePath bool) (map[string]int, error) { + fileRegoVersions := map[string]int{} + + // we drop the bundle-global rego versions and record individual rego versions for each module. + for _, m := range bundle.Modules { + // We fetch rego-version by the path relative to the bundle root, as the complete path of the module might + // contain the path between OPA working directory and the bundle root. + v, err := bundle.RegoVersionForFile(bundleRelativePath(m, usePath), bundle.RegoVersion(regoVersion)) + if err != nil { + return nil, err + } + // only record the rego version if it's different from one applied globally to the result bundle + if v != regoVersion { + // We store the rego version by the absolute path to the bundle root, as this will be the - possibly new - path + // to the module inside the merged bundle. + fileRegoVersions[bundleAbsolutePath(m, usePath)] = v.Int() + } + } + + return fileRegoVersions, nil +} + +func bundleRelativePath(m ModuleFile, usePath bool) string { + p := m.RelativePath + if p == "" { + if usePath { + p = m.Path + } else { + p = m.URL + } + } + return p +} + +func bundleAbsolutePath(m ModuleFile, usePath bool) string { + var p string + if usePath { + p = m.Path + } else { + p = m.URL + } + if !path.IsAbs(p) { + p = "/" + p + } + return path.Clean(p) +} + // RootPathsOverlap takes in two bundle root paths and returns true if they overlap. func RootPathsOverlap(pathA string, pathB string) bool { a := rootPathSegments(pathA) diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index 6c2fa6f3e3..4be3db4249 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -80,6 +80,82 @@ func TestManifestEqual(t *testing.T) { "foo": "bar", } assertEqual() + + // rego-version + + n.RegoVersion = pointTo(1) + assertNotEqual() + + m.RegoVersion = pointTo(0) + assertNotEqual() + + m.RegoVersion = pointTo(1) + assertEqual() + + n.FileRegoVersions = map[string]int{ + "foo": 1, + } + assertNotEqual() + + m.FileRegoVersions = map[string]int{ + "foo": 1, + } + assertEqual() + + n.FileRegoVersions["*/bar"] = 0 + assertNotEqual() + + m.FileRegoVersions["*/bar"] = 0 + assertEqual() +} + +func TestBundleRegoVersion(t *testing.T) { + b := Bundle{} + + if b.Manifest.RegoVersion != nil { + t.Fatal("expected nil") + } + + // No rego-version set, expect default + if b.RegoVersion(ast.RegoV0) != ast.RegoV0 { + t.Fatal("expected v0") + } + if b.RegoVersion(ast.RegoV1) != ast.RegoV1 { + t.Fatal("expected v1") + } + + // Set rego-version to v0 + b.SetRegoVersion(ast.RegoV0) + + if b.Manifest.RegoVersion == nil || *b.Manifest.RegoVersion != 0 { + t.Fatal("expected v0") + } + + if b.RegoVersion(ast.RegoV1) != ast.RegoV0 { + t.Fatal("expected v0") + } + + // Set rego-version to v1 + b.SetRegoVersion(ast.RegoV1) + + if b.Manifest.RegoVersion == nil || *b.Manifest.RegoVersion != 1 { + t.Fatal("expected v1") + } + + if b.RegoVersion(ast.RegoV0) != ast.RegoV1 { + t.Fatal("expected v1") + } + + // Set rego-version to v0-compat1 + b.SetRegoVersion(ast.RegoV0CompatV1) + + if b.Manifest.RegoVersion == nil || *b.Manifest.RegoVersion != 0 { + t.Fatal("expected v0") + } + + if b.RegoVersion(ast.RegoV1) != ast.RegoV0 { + t.Fatal("expected v0") + } } func TestRead(t *testing.T) { @@ -1227,7 +1303,7 @@ func TestWriterSkipEmptyManifest(t *testing.T) { } if f.Name != "/data.json" { - t.Fatal("expected only /data.json but got:", f.Name) + t.Fatal("expected only /data.json and /.manifest but got:", f.Name) } } } @@ -1633,8 +1709,10 @@ func TestMerge(t *testing.T) { }, wantBundle: &Bundle{ Manifest: Manifest{ - Revision: "abcdef", - Roots: &[]string{""}, + Revision: "abcdef", + Roots: &[]string{""}, + RegoVersion: pointTo(0), // Default rego-version + FileRegoVersions: map[string]int{}, }, Modules: []ModuleFile{ { @@ -1671,6 +1749,7 @@ func TestMerge(t *testing.T) { "foo", "bar", }, + RegoVersion: pointTo(0), // Default rego-version }, Data: map[string]interface{}{}, }, @@ -1715,6 +1794,7 @@ func TestMerge(t *testing.T) { "logs", "authz", }, + RegoVersion: pointTo(0), // Default rego-version }, WasmModules: []WasmModuleFile{ { @@ -1771,6 +1851,7 @@ func TestMerge(t *testing.T) { "foo", "baz", }, + RegoVersion: pointTo(0), // Default rego-version }, Modules: []ModuleFile{ { @@ -1819,6 +1900,7 @@ func TestMerge(t *testing.T) { "foo/bar", "baz", }, + RegoVersion: pointTo(0), // Default rego-version }, Data: map[string]interface{}{ "foo": map[string]interface{}{ @@ -1854,6 +1936,7 @@ func TestMerge(t *testing.T) { "foo/bar", "baz", }, + RegoVersion: pointTo(0), // Default rego-version }, Data: map[string]interface{}{}, }, @@ -1889,7 +1972,8 @@ func TestMerge(t *testing.T) { wantBundle: &Bundle{ Data: map[string]interface{}{}, Manifest: Manifest{ - Roots: &[]string{"a", "b"}, + Roots: &[]string{"a", "b"}, + RegoVersion: pointTo(0), // Default rego-version }, PlanModules: []PlanModuleFile{ { @@ -1951,3 +2035,7 @@ func TestMerge(t *testing.T) { }) } } + +func pointTo[T any](x T) *T { + return &x +} diff --git a/cmd/bench_test.go b/cmd/bench_test.go index f09e66c74d..f370be9e02 100644 --- a/cmd/bench_test.go +++ b/cmd/bench_test.go @@ -7,6 +7,7 @@ package cmd import ( "bytes" "context" + "encoding/json" "errors" "fmt" "os" @@ -808,6 +809,205 @@ a contains 4 if { } } +func TestBenchMainWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + bundleRegoVersion int + bundleFileRegoVersions map[string]int + modules map[string]string + query string + expErrs []string + }{ + // These tests are slow, so we're not being completely exhaustive here. + { + note: "v0 bundle", + bundleRegoVersion: 0, + modules: map[string]string{ + "test.rego": `package test +a[4] { + 1 == 1 +}`, + }, + query: `data.test.a`, + }, + { + note: "v0 bundle, no keywords imported", + bundleRegoVersion: 0, + modules: map[string]string{ + "test.rego": `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: "v0 bundle, v1 per-file override", + bundleRegoVersion: 0, + bundleFileRegoVersions: map[string]int{ + "*/test2.rego": 1, + }, + modules: map[string]string{ + "test1.rego": `package test +a[4] { + 1 == 1 +}`, + "test2.rego": `package test +b contains 4 if { + 1 == 1 +}`, + }, + query: `data.test.a`, + }, + { + note: "v1 bundle, keywords not used", + bundleRegoVersion: 1, + modules: map[string]string{ + "test.rego": `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, no keywords imported", + bundleRegoVersion: 1, + modules: map[string]string{ + "test.rego": `package test +a contains 4 if { + 1 == 1 +}`, + }, + query: `data.test.a`, + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + modes := []struct { + name string + e2e bool + }{ + { + name: "run", + }, + { + name: "e2e", + e2e: true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, mode := range modes { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, tc.note, mode.name), func(t *testing.T) { + files := map[string]string{} + + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.modules { + files[k] = v + } + + manifest := bundle.Manifest{ + RegoVersion: &tc.bundleRegoVersion, + FileRegoVersions: tc.bundleFileRegoVersions, + } + manifest.Init() + if b, err := json.Marshal(manifest); err != nil { + t.Fatalf("Unexpected error: %s", err) + } else { + files[".manifest"] = string(b) + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + b := bundle.Bundle{ + Manifest: bundle.Manifest{ + RegoVersion: &tc.bundleRegoVersion, + FileRegoVersions: tc.bundleFileRegoVersions, + }, + Data: map[string]interface{}{}, + } + for k, v := range tc.modules { + b.Modules = append(b.Modules, bundle.ModuleFile{ + Path: k, + Raw: []byte(v), + }) + } + p = filepath.Join(root, "bundle.tar.gz") + f, err := os.OpenFile(p, os.O_WRONLY, os.ModePerm) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + err = bundle.Write(f, b) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + } + + params := testBenchParams() + _ = params.outputFormat.Set(evalPrettyOutput) + + params.e2e = mode.e2e + err := params.bundlePaths.Set(p) + 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/build_test.go b/cmd/build_test.go index 49a478b8e5..758df258ae 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -14,6 +14,7 @@ import ( "testing" "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/loader" "github.com/open-policy-agent/opa/util" "github.com/open-policy-agent/opa/util/test" @@ -63,7 +64,7 @@ func TestBuildProducesBundle(t *testing.T) { } else if err != nil { t.Fatal(err) } - if f.Name == "/data.json" || strings.HasSuffix(f.Name, "/test.rego") { + if f.Name == "/.manifest" || f.Name == "/data.json" || strings.HasSuffix(f.Name, "/test.rego") { continue } t.Fatal("unexpected file:", f.Name) @@ -382,7 +383,7 @@ func TestBuildPlanWithPruneUnused(t *testing.T) { switch { case f.Name == "/plan.json": found = true - case f.Name == "/data.json" || strings.HasSuffix(f.Name, "/test.rego"): // expected + case f.Name == "/.manifest" || f.Name == "/data.json" || strings.HasSuffix(f.Name, "/test.rego"): // expected default: t.Errorf("unexpected file: %s", f.Name) } @@ -646,6 +647,7 @@ p2 := 2 manifest: ` { "revision":"", + "rego_version": 0, "roots":[""], "wasm":[{ "entrypoint":"test/p2", @@ -678,6 +680,7 @@ p2 := 2 manifest: ` { "revision":"", + "rego_version": 0, "roots":[""], "wasm":[{ "entrypoint":"test/p2", @@ -729,6 +732,7 @@ bar := "baz" manifest: ` { "revision":"", + "rego_version": 0, "roots":[""], "wasm":[{ "entrypoint":"test/p3", @@ -773,6 +777,7 @@ p := 1 manifest: ` { "revision":"", + "rego_version": 0, "roots":[""], "wasm":[{ "entrypoint":"test/p", @@ -812,6 +817,7 @@ p2 := 2 manifest: ` { "revision":"", + "rego_version": 0, "roots":[""], "wasm":[{ "entrypoint":"test", @@ -839,6 +845,7 @@ p2 := 2 manifest: ` { "revision":"", + "rego_version": 0, "roots":[""], "wasm":[{ "entrypoint":"test", @@ -970,13 +977,602 @@ func TestBuildBundleModeIgnoreFlag(t *testing.T) { files = append(files, filepath.Base(f.Name)) } - expected := 4 + // We additionally expect a manifest file + expected := 5 if len(files) != expected { t.Fatalf("expected %v files but got %v", expected, len(files)) } }) } +func TestBuildBundleModeWithManifestRegoVersion(t *testing.T) { + tests := []struct { + note string + roots []string + files map[string]string + expManifest string + expErrs []string + }{ + { + note: "v0 bundle rego-version", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "test.rego": `package test + +p[42] { + input.x == 1 +}`, + }, + expManifest: `{"revision":"","roots":[""],"rego_version":0}`, + }, + { + note: "v1 bundle rego-version", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "test.rego": `package test + +p contains 42 if { + input.x == 1 +}`, + }, + expManifest: `{"revision":"","roots":[""],"rego_version":1}`, + }, + { + note: "v0 bundle rego-version, v1 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test + +p[1] { + input.x == 1 +}`, + "test2.rego": `package test + +p contains 2 if { + input.x == 1 +}`, + }, + expManifest: `{"revision":"","roots":[""],"rego_version":0,"file_rego_versions":{"%ROOT%/test2.rego":1}}`, + }, + { + note: "v0 bundle rego-version, v1 per-file override, missing v1 keywords in v1 file", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test + +p[1] { + input.x == 1 +}`, + "test2.rego": `package test + +p[2] { + input.x == 1 +}`, + }, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v0 bundle rego-version, v1 per-file override, v1 keywords but no v1 imports in v0 file", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test + +p contains 1 if { + input.x == 1 +}`, + "test2.rego": `package test + +p contains 2 if { + input.x == 1 +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + { + note: "multiple bundles with different rego-versions", + roots: []string{"bundle1", "bundle2"}, + files: map[string]string{ + "bundle1/.manifest": `{ + "roots": ["test1"], + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "bundle1/test1.rego": `package test1 +p[1] { + input.x == 1 +}`, + "bundle1/test2.rego": `package test1 +p contains 2 if { + input.x == 1 +}`, + "bundle2/.manifest": `{ + "roots": ["test2"], + "rego_version": 1, + "file_rego_versions": { + "*/test4.rego": 0 + } +}`, + "bundle2/test3.rego": `package test2 +p contains 3 if { + input.x == 1 +}`, + "bundle2/test4.rego": `package test2 +p[4] { + input.x == 1 +}`, + }, + expManifest: `{"revision":"","roots":["test1","test2"],"rego_version":0,"file_rego_versions":{"%ROOT%/bundle1/test2.rego":1,"%ROOT%/bundle2/test3.rego":1}}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + test.WithTempFS(tc.files, func(root string) { + params := newBuildParams() + params.outputFile = path.Join(root, "bundle.tar.gz") + params.bundleMode = true + + var roots []string + if len(tc.roots) == 0 { + roots = []string{root} + } else { + for _, r := range tc.roots { + roots = append(roots, path.Join(root, r)) + } + } + err := dobuild(params, roots) + if tc.expErrs != nil { + if err == nil { + t.Fatal("expected error but got none") + } + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected error:\n\n%q\n\nbut got:\n\n%v", expErr, err) + } + } + } else { + if err != nil { + t.Fatal(err) + } + + _, err = loader.NewFileLoader().AsBundle(params.outputFile) + if err != nil { + t.Fatal(err) + } + + f, err := os.Open(params.outputFile) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = f.Close() + }() + + gr, err := gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + + tr := tar.NewReader(gr) + + for { + f, err := tr.Next() + if err == io.EOF { + break + } else if err != nil { + t.Fatal(err) + } + + if f.Name == "/.manifest" { + b, err := io.ReadAll(tr) + if err != nil { + t.Fatal(err) + } + expManifest := strings.ReplaceAll(tc.expManifest, "%ROOT%", root) + if !strings.Contains(string(b), expManifest) { + t.Fatalf("expected manifest:\n\n%v\n\nbut got:\n\n%v", expManifest, string(b)) + } + } + } + } + }) + }) + } +} + +func TestBuildBundleFromOtherBundles(t *testing.T) { + type bundleInfo map[string]string + + tests := []struct { + note string + v1Compatible bool + bundles map[string]bundleInfo + expBundle bundleInfo + expErrs []string + }{ + { + note: "single bundle", + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + "policy.rego": `package test +p { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":0} +`, + "%ROOT%/bundle.tar.gz/policy.rego": `package test + +p { + input.x == 1 +} +`, + }, + }, + { + note: "single bundle, --v1-compatible", + v1Compatible: true, + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + "policy.rego": `package test +p if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":1} +`, + "%ROOT%/bundle.tar.gz/policy.rego": `package test + +p if { + input.x == 1 +} +`, + }, + }, + { + note: "single v0 bundle", + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":0} +`, + "%ROOT%/bundle.tar.gz/policy.rego": `package test + +p { + input.x == 1 +} +`, + }, + }, + { + note: "single v0 bundle, --v1-compatible", + v1Compatible: true, + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p { + input.x == 1 +}`, + }, + }, + // We don't expect parse/compile errors, as the bundle rego-version is 0, which overrides the --v1-compatible flag. + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":0} +`, + "%ROOT%/bundle.tar.gz/policy.rego": `package test + +p { + input.x == 1 +} +`, + }, + }, + { + note: "single v0 bundle, v1 per-file override", + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy_1.rego": 1 + } +}`, + "policy_0.rego": `package test +p { + input.x == 1 +}`, + "policy_1.rego": `package test +q contains 1 if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":0,"file_rego_versions":{"%ROOT%/bundle.tar.gz/policy_1.rego":1}} +`, + "%ROOT%/bundle.tar.gz/policy_0.rego": `package test + +p { + input.x == 1 +} +`, + "%ROOT%/bundle.tar.gz/policy_1.rego": `package test + +q contains 1 if { + input.x == 1 +} +`, + }, + }, + { + note: "single v0 bundle, v1 per-file override, --v1-compatible", + v1Compatible: true, + bundles: map[string]bundleInfo{ + "bundle.tar.gz": { + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy_1.rego": 1 + } +}`, + "policy_0.rego": `package test +p { + input.x == 1 +}`, + "policy_1.rego": `package test +q contains 1 if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":[""],"rego_version":0,"file_rego_versions":{"%ROOT%/bundle.tar.gz/policy_1.rego":1}} +`, + "%ROOT%/bundle.tar.gz/policy_0.rego": `package test + +p { + input.x == 1 +} +`, + "%ROOT%/bundle.tar.gz/policy_1.rego": `package test + +q contains 1 if { + input.x == 1 +} +`, + }, + }, + { + note: "v0 bundle + v1 bundle", + bundles: map[string]bundleInfo{ + "bundle_v0.tar.gz": { + ".manifest": `{"roots": ["test1"], "rego_version": 0}`, + "policy.rego": `package test1 +p { + input.x == 1 +}`, + }, + "bundle_v1.tar.gz": { + ".manifest": `{"roots": ["test2"], "rego_version": 1}`, + "policy.rego": `package test2 +q contains 1 if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + "/.manifest": `{"revision":"","roots":["test1","test2"],"rego_version":0,"file_rego_versions":{"%ROOT%/bundle_v1.tar.gz/policy.rego":1}} +`, + "%ROOT%/bundle_v0.tar.gz/policy.rego": `package test1 + +p { + input.x == 1 +} +`, + "%ROOT%/bundle_v1.tar.gz/policy.rego": `package test2 + +q contains 1 if { + input.x == 1 +} +`, + }, + }, + { + note: "v0 bundle + v1 bundle, --v1-compatible", + v1Compatible: true, + bundles: map[string]bundleInfo{ + "bundle_v0.tar.gz": { + ".manifest": `{"roots": ["test1"], "rego_version": 0}`, + "policy.rego": `package test1 +p { + input.x == 1 +}`, + }, + "bundle_v1.tar.gz": { + ".manifest": `{"roots": ["test2"], "rego_version": 1}`, + "policy.rego": `package test2 +q contains 1 if { + input.x == 1 +}`, + }, + }, + expBundle: bundleInfo{ + "/data.json": `{} +`, + // We get a v1 bundle with a v0 per-file override + "/.manifest": `{"revision":"","roots":["test1","test2"],"rego_version":1,"file_rego_versions":{"%ROOT%/bundle_v0.tar.gz/policy.rego":0}} +`, + "%ROOT%/bundle_v0.tar.gz/policy.rego": `package test1 + +p { + input.x == 1 +} +`, + "%ROOT%/bundle_v1.tar.gz/policy.rego": `package test2 + +q contains 1 if { + input.x == 1 +} +`, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + test.WithTempFS(nil, func(root string) { + var roots []string + for name, files := range tc.bundles { + p := filepath.Join(root, name) + roots = append(roots, p) + filePairs := make([][2]string, 0, len(files)) + for k, v := range files { + filePairs = append(filePairs, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(filePairs) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + params := newBuildParams() + params.outputFile = path.Join(root, "bundle.tar.gz") + params.bundleMode = true + params.v1Compatible = tc.v1Compatible + + err := dobuild(params, roots) + if tc.expErrs != nil { + if err == nil { + t.Fatal("expected error but got none") + } + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected error:\n\n%q\n\nbut got:\n\n%v", expErr, err) + } + } + } else { + if err != nil { + t.Fatal(err) + } + + _, err = loader.NewFileLoader().AsBundle(params.outputFile) + if err != nil { + t.Fatal(err) + } + + f, err := os.Open(params.outputFile) + if err != nil { + t.Fatal(err) + } + defer func() { + _ = f.Close() + }() + + gr, err := gzip.NewReader(f) + if err != nil { + t.Fatal(err) + } + + tr := tar.NewReader(gr) + + for { + f, err := tr.Next() + if err == io.EOF { + break + } else if err != nil { + t.Fatal(err) + } + + found := false + for expName, expVal := range tc.expBundle { + expName = strings.ReplaceAll(expName, "%ROOT%", root) + if f.Name == expName { + found = true + b, err := io.ReadAll(tr) + if err != nil { + t.Fatal(err) + } + expVal = strings.ReplaceAll(expVal, "%ROOT%", root) + if string(b) != expVal { + t.Fatalf("expected %v:\n\n%v\n\nbut got:\n\n%v", expName, expVal, string(b)) + } + break + } + } + if !found { + t.Fatalf("unexpected file in bundle: %v", f.Name) + } + + //if f.Name == "/policy.rego" { + // b, err := io.ReadAll(tr) + // if err != nil { + // t.Fatal(err) + // } + // if string(b) != tc.expBundle["policy.rego"] { + // t.Fatalf("expected policy.rego:\n\n%v\n\nbut got:\n\n%v", tc.expBundle["policy.rego"], string(b)) + // } + //} + } + } + }) + }) + } +} + func TestBuildWithV1CompatibleFlag(t *testing.T) { tests := []struct { note string @@ -1006,6 +1602,8 @@ func TestBuildWithV1CompatibleFlag(t *testing.T) { }, // Imports are preserved expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":0} +`, "test.rego": `package test import rego.v1 @@ -1027,6 +1625,8 @@ allow if { }, // Imports are preserved expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":0} +`, "test.rego": `package test import future.keywords.if @@ -1048,6 +1648,8 @@ allow if { }, // Imports are not added in expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":1} +`, "test.rego": `package test allow if { @@ -1068,6 +1670,8 @@ allow if { }, // the rego.v1 import is obsolete in rego-v1, and is removed expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":1} +`, "test.rego": `package test allow if { @@ -1088,6 +1692,8 @@ allow if { }, // future.keywords imports are obsolete in rego-v1, and are removed expectedFiles: map[string]string{ + ".manifest": `{"revision":"","roots":[""],"rego_version":1} +`, "test.rego": `package test allow if { @@ -1096,6 +1702,17 @@ allow if { `, }, }, + { + note: "1.0 compatibility: missing keywords", + v1Compatible: true, + files: map[string]string{ + "test.rego": `package test + allow[1] { + 1 < 2 + }`, + }, + expectedErr: "rego_parse_error", + }, } for _, tc := range tests { @@ -1142,6 +1759,7 @@ allow if { tr := tar.NewReader(gr) + foundFiles := map[string]struct{}{} for { f, err := tr.Next() if err == io.EOF { @@ -1149,6 +1767,7 @@ allow if { } else if err != nil { t.Fatal(err) } + foundFiles[path.Base(f.Name)] = struct{}{} expectedFile := tc.expectedFiles[path.Base(f.Name)] if expectedFile != "" { data, err := io.ReadAll(tr) @@ -1157,10 +1776,16 @@ allow if { } actualFile := string(data) if actualFile != expectedFile { - t.Fatalf("expected optimized module:\n\n%v\n\ngot:\n\n%v", expectedFile, actualFile) + t.Fatalf("expected file %s to be:\n\n%v\n\ngot:\n\n%v", f.Name, expectedFile, actualFile) } } } + + for expectedFile := range tc.expectedFiles { + if _, ok := foundFiles[expectedFile]; !ok { + t.Fatalf("expected file %s not found in bundle, got: %v", expectedFile, foundFiles) + } + } } }) }) @@ -1186,6 +1811,8 @@ p[k] contains v if { `, }, expectedFiles: map[string]string{ + "/.manifest": `{"revision":"","roots":[""],"rego_version":1} +`, "/optimized/test/p.rego": `package test.p foo contains __local1__1 if { @@ -1209,10 +1836,10 @@ p[k] contains v if { }, // Note: the rego.v1 import isn't added to the optimized module. // This is ok, as the bundle was built with the --v1-compatible flag, - // and is therefore only guaranteed to work with OPA 1.0 or when - // OPA 0.x is run with the --rego-v1 flag. - // TODO: add rego-v1 flag to bundle, so `opa run` etc. doesn't need the --rego-v1 flag to consume it. + // and is tagged with a rego-version to inform the consumer. expectedFiles: map[string]string{ + "/.manifest": `{"revision":"","roots":[""],"rego_version":1} +`, "/optimized/test/p.rego": `package test.p foo contains __local1__1 if { @@ -1235,6 +1862,8 @@ p[k] contains v if { `, }, expectedFiles: map[string]string{ + "/.manifest": `{"revision":"","roots":[""],"rego_version":1} +`, "/optimized/test/p.rego": `package test.p foo contains __local1__1 if { @@ -1272,6 +1901,7 @@ foo contains __local1__1 if { tr := tar.NewReader(gr) + foundFiles := map[string]struct{}{} for { f, err := tr.Next() if err == io.EOF { @@ -1279,6 +1909,7 @@ foo contains __local1__1 if { } else if err != nil { t.Fatal(err) } + foundFiles[f.Name] = struct{}{} expectedFile := tc.expectedFiles[f.Name] if expectedFile != "" { data, err := io.ReadAll(tr) @@ -1287,10 +1918,16 @@ foo contains __local1__1 if { } actualFile := string(data) if actualFile != expectedFile { - t.Fatalf("expected optimized module:\n\n%v\n\ngot:\n\n%v", expectedFile, actualFile) + t.Fatalf("expected file %s to be:\n\n%v\n\ngot:\n\n%v", f.Name, expectedFile, actualFile) } } } + + for expectedFile := range tc.expectedFiles { + if _, ok := foundFiles[expectedFile]; !ok { + t.Fatalf("expected file %s not found in bundle, got: %v", expectedFile, foundFiles) + } + } }) }) } diff --git a/cmd/check_test.go b/cmd/check_test.go index 6eedae4c4c..d19d3c7cf4 100644 --- a/cmd/check_test.go +++ b/cmd/check_test.go @@ -6,11 +6,15 @@ package cmd import ( "encoding/json" + "fmt" + "os" "path" + "path/filepath" "strings" "testing" "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/util/test" ) @@ -494,3 +498,419 @@ p contains x if { }) } } + +func TestCheckWithBundleRegoVersion(t *testing.T) { + cases := []struct { + note string + files map[string]string + expErrs []string + }{ + { + note: "v0.x bundle, illegal keywords", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p contains x if { + x := [1,2,3] +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + }, + }, + { + note: "v0.x bundle, rego.v1 imported, v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import rego.v1 +p contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v0.x bundle, rego.v1 imported, NOT v1 compliant (parser)", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import rego.v1 +p contains x { + x := [1,2,3] +} + +q.r`, + }, + expErrs: []string{ + "rego_parse_error: `if` keyword is required before rule body", + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v0.x bundle, rego.v1 imported, NOT v1 compliant (compiler)", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import rego.v1 + +import data.foo +import data.bar as foo +`, + }, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + { + note: "v0.x bundle, keywords imported, v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import future.keywords.if +import future.keywords.contains +p contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v0.x bundle, no imports, v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p := 1 +`, + }, + }, + { + note: "v0 bundle, v1 per-file overrides, compliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[x] { + x := [1,2,3] +}`, + "policy2.rego": `package test +q contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v0 bundle, v1 per-file overrides (glob), compliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[x] { + x := [1,2,3] +}`, + "policy2.rego": `package test +q contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v0 bundle, v1 per-file overrides, incompliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[x] { + x := [1,2,3] +}`, + "policy2.rego": `package test +q[x] { + x := [1,2,3] +}`, + }, + 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 bundle, keywords used but not imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v1.0 bundle, rego.v1 imported, v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import rego.v1 +p contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v1.0 bundle, rego.v1 imported, NOT v1 compliant (parser)", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import rego.v1 +p contains x { + x := [1,2,3] +} + +q.r`, + }, + 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 bundle, rego.v1 imported, NOT v1 compliant (compiler)", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import rego.v1 + +import data.foo +import data.bar as foo +`, + }, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + { + note: "v1.0 bundle, keywords imported, v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import future.keywords.if +import future.keywords.contains +p contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v1.0 bundle, keywords imported, NOT v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import future.keywords.contains +p contains x { + x := [1,2,3] +} + +q.r`, + }, + 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 bundle, keywords imported, NOT v1 compliant (compiler)", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import future.keywords.if + +input := 1 if { + 1 == 2 +}`, + }, + expErrs: []string{ + "rego_compile_error: rules must not shadow input (use a different rule name)", + }, + }, + { + note: "v1.0 bundle, no imports, v1 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p := 1 +`, + }, + }, + { + note: "v1.0 bundle, no imports, NOT v1 compliant but v0 compliant (compiler)", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p.x`, + }, + expErrs: []string{ + "rego_parse_error: `contains` keyword is required for partial set rules", + }, + }, + { + note: "v1.0 bundle, no imports, v1 compliant but NOT v0 compliant", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v1 bundle, v0 per-file overrides, compliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p[x] { + x := [1,2,3] +}`, + "policy2.rego": `package test +q contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v1 bundle, v0 per-file overrides (glob), compliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "*/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p[x] { + x := [1,2,3] +}`, + "policy2.rego": `package test +q contains x if { + x := [1,2,3] +}`, + }, + }, + { + note: "v1 bundle, v0 per-file overrides, incompliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p contains x if { + x := [1,2,3] +}`, + "policy2.rego": `package test +q contains x if { + x := [1,2,3] +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + }, + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + v1CompatibleFlagCases := []struct { + note string + used bool + }{ + { + "no --v1-compatible", false, + }, + { + "--v1-compatible", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, v1CompatibleFlag := range v1CompatibleFlagCases { + for _, tc := range cases { + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, v1CompatibleFlag.note, tc.note), func(t *testing.T) { + files := map[string]string{} + + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + params := newCheckParams() + params.bundleMode = true + params.v1Compatible = v1CompatibleFlag.used + + err := checkModules(params, []string{p}) + switch { + case err != nil && len(tc.expErrs) > 0: + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected err:\n\n%v\n\ngot:\n\n%v", expErr, err) + } + } + return // don't read back bundle below + case err != nil && len(tc.expErrs) == 0: + t.Fatalf("unexpected error: %v", err) + case err == nil && len(tc.expErrs) > 0: + t.Fatalf("expected error:\n\n%v\n\ngot: none", tc.expErrs) + } + }) + }) + } + } + } +} diff --git a/cmd/deps_test.go b/cmd/deps_test.go index 7fef1f8060..8f9750dccf 100644 --- a/cmd/deps_test.go +++ b/cmd/deps_test.go @@ -5,11 +5,14 @@ package cmd import ( + "fmt" "io" + "os" "path/filepath" "strings" "testing" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/util/test" ) @@ -138,3 +141,322 @@ p contains 3 if { }) } } + +func TestDepsV1WithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + files map[string]string + query string + expErrs []string + }{ + { + note: "v0.x bundle, no keywords", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p[3] { + input.x = 1 +}`, + }, + query: `data.test.p`, + }, + { + note: "v0.x bundle, keywords not imported, but used", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `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 bundle, keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import future.keywords +p contains 3 if { + input.x = 1 +}`, + }, + query: `data.test.p`, + }, + { + note: "v0.x bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import rego.v1 +p contains 3 if { + input.x = 1 +}`, + }, + query: `data.test.p`, + }, + { + note: "v0 bundle, v1 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[3] { + input.x = 1 +}`, + "policy2.rego": `package test +p contains 4 if { + input.x = 1 +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/bar/*.rego": 1 + } +}`, + "foo/policy1.rego": `package test +p[3] { + input.x = 1 +}`, + "bar/policy2.rego": `package test +p contains 4 if { + input.x = 1 +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override, incompliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[3] { + input.x = 1 +}`, + "policy2.rego": `package test +p[4] { + input.x = 1 +}`, + }, + 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 bundle, no keywords", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `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 bundle, no keyword imports", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p contains 3 if { + input.x = 1 +}`, + }, + query: `data.test.p`, + }, + { + note: "v1.0 bundle, keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import future.keywords +p contains 3 if { + input.x = 1 +}`, + }, + query: `data.test.p`, + }, + { + note: "v1.0 bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import rego.v1 +p contains 3 if { + input.x = 1 +}`, + }, + query: `data.test.p`, + }, + { + note: "v1 bundle, v0 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p[3] { + input.x = 1 +}`, + "policy2.rego": `package test +p contains 4 if { + input.x = 1 +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/foo/*.rego": 0 + } +}`, + "foo/policy1.rego": `package test +p[3] { + input.x = 1 +}`, + "bar/policy2.rego": `package test +p contains 4 if { + input.x = 1 +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override, incompliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p contains 3 if { + input.x = 1 +}`, + "policy2.rego": `package test +p contains 4 if { + input.x = 1 +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + v1CompatibleFlagCases := []struct { + note string + used bool + }{ + { + "no --v1-compatible", false, + }, + { + "--v1-compatible", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, v1CompatibleFlag := range v1CompatibleFlagCases { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, v1CompatibleFlag.note, tc.note), func(t *testing.T) { + files := map[string]string{} + + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + params := newDepsCommandParams() + if err := params.bundlePaths.Set(p); err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + params.v1Compatible = v1CompatibleFlag.used + _ = params.outputFormat.Set(depsFormatPretty) + + 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/eval_test.go b/cmd/eval_test.go index 1bb2ed9059..c5adbdc9c2 100755 --- a/cmd/eval_test.go +++ b/cmd/eval_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/internal/presentation" "github.com/open-policy-agent/opa/loader" "github.com/open-policy-agent/opa/rego" @@ -1990,3 +1991,306 @@ func TestEvalPolicyWithV1CompatibleFlag(t *testing.T) { } } } + +func TestEvalPolicyWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + files map[string]string + query string + expectedErr string + }{ + { + note: "v0.x bundle, no rego.v1 or future.keywords imports", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +allow if { + 1 < 2 +}`, + }, + query: "data.test.allow", + expectedErr: "rego_parse_error", + }, + { + note: "v0 bundle, v1 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[1] { + 1 < 2 +} +`, + "policy2.rego": `package test +p contains 2 if { + 1 < 2 +} +`, + }, + query: "data.test.p", + }, + { + note: "v0 bundle, v1 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/bar/*.rego": 1 + } +}`, + "foo/policy1.rego": `package test +p[1] { + 1 < 2 +} +`, + "bar/policy1.rego": `package test +p contains 2 if { + 1 < 2 +} +`, + "bar/policy2.rego": `package test +p contains 3 if { + 1 < 2 +} +`, + }, + query: "data.test.p", + }, + { + note: "v0 bundle, v1 per-file override, incompliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[1] { + 1 < 2 +} +`, + "policy2.rego": `package test +p[2] { + 1 < 2 +} +`, + }, + query: "data.test.p", + expectedErr: "rego_parse_error", + }, + + { + note: "v1.0 bundle, no rego.v1 or future.keywords imports", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +allow if { + 1 < 2 +}`, + }, + query: "data.test.allow", + }, + { + note: "v1.0 bundle, policy with rego.v1 import", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import rego.v1 +allow if { + 1 < 2 +}`, + }, + query: "data.test.allow", + }, + { + note: "v1.0 bundle, future.keywords import", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import future.keywords.if +allow if { + 1 < 2 +}`, + }, + query: "data.test.allow", + }, + { + note: "v1.0 bundle, keywords not used", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +allow { + 1 < 2 +}`, + }, + query: "data.test.allow", + expectedErr: "rego_parse_error", + }, + { + note: "v1 bundle, v0 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p[1] { + 1 < 2 +} +`, + "policy2.rego": `package test +p contains 2 if { + 1 < 2 +} +`, + }, + query: "data.test.p", + }, + { + note: "v1 bundle, v0 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/foo/*.rego": 0 + } +}`, + "foo/policy1.rego": `package test +p[1] { + 1 < 2 +} +`, + "foo/policy2.rego": `package test +p[2] { + 1 < 2 +} +`, + "bar/policy1.rego": `package test +p contains 3 if { + 1 < 2 +} +`, + }, + query: "data.test.p", + }, + { + note: "v1 bundle, v0 per-file override, incompliant", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "*/policy2.rego": 0 + } +}`, + "policy1.rego": `package test +p contains 1 if { + input.x == 1 +} +`, + "policy2.rego": `package test +p contains 2 if { + input.x == 1 +} +`, + }, + query: "data.test.p", + expectedErr: "rego_parse_error", + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + v1CompatibleFlagCases := []struct { + note string + used bool + }{ + { + "no --v1-compatible", false, + }, + { + "--v1-compatible", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, v1CompatibleFlag := range v1CompatibleFlagCases { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, v1CompatibleFlag.note, tc.note), func(t *testing.T) { + files := map[string]string{} + + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + params := newEvalCommandParams() + params.v1Compatible = v1CompatibleFlag.used + if err := params.bundlePaths.Set(p); err != nil { + t.Fatal(err) + } + + var buf bytes.Buffer + + defined, err := eval([]string{tc.query}, params, &buf) + + if tc.expectedErr == "" { + if err != nil { + t.Fatalf("Unexpected error: %v, buf: %s", err, buf.String()) + } else if !defined { + t.Fatal("expected result to be defined") + } + } else { + if err == nil { + t.Fatal("expected error, got none") + } + + actual := buf.String() + if !strings.Contains(actual, tc.expectedErr) { + t.Fatalf("expected error:\n\n%v\n\ngot\n\n%v", tc.expectedErr, actual) + } + } + }) + }) + } + } + } +} diff --git a/cmd/exec_test.go b/cmd/exec_test.go index 50b919bdf0..cf56004bf5 100644 --- a/cmd/exec_test.go +++ b/cmd/exec_test.go @@ -3,12 +3,16 @@ package cmd import ( "bytes" "context" + "fmt" + "os" + "path/filepath" "reflect" "strings" "testing" "time" "github.com/open-policy-agent/opa/cmd/internal/exec" + "github.com/open-policy-agent/opa/internal/file/archive" 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" @@ -314,6 +318,351 @@ main contains "hello" if { } } +func TestExecWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + files map[string]string + expErrs []string + }{ + { + note: "v0.x bundle, no keywords used", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package system +main["hello"] { + input.foo == "bar" +}`, + }, + }, + { + note: "v0.x bundle, no keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `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 bundle, keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package system +import future.keywords +main contains "hello" if { + input.foo == "bar" +}`, + }, + }, + { + note: "v0.x bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package system +import rego.v1 +main contains "hello" if { + input.foo == "bar" +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package system +p[42] { + input.foo == "bar" +}`, + "policy2.rego": `package system +main contains "hello" if { + 42 in p +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/policy2.rego": 1 + } +}`, + "policy1.rego": `package system +p[42] { + input.foo == "bar" +}`, + "policy2.rego": `package system +main contains "hello" if { + 42 in p +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override, incompatible", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package system +p[42] { + input.foo == "bar" +}`, + "policy2.rego": `package system +main["hello"] { + p[_] == 42 +}`, + }, + 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 bundle, no keywords used", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `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 bundle, no keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package system +main contains "hello" if { + input.foo == "bar" +}`, + }, + }, + { + note: "v1.0 bundle, keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package system +import future.keywords +main contains "hello" if { + input.foo == "bar" +}`, + }, + }, + { + note: "v1.0 bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package system +import rego.v1 +main contains "hello" if { + input.foo == "bar" +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package system +p[42] { + input.foo == "bar" +}`, + "policy2.rego": `package system +main contains "hello" if { + 42 in p +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "*/policy1.rego": 0 + } +}`, + "policy1.rego": `package system +p[42] { + input.foo == "bar" +}`, + "policy2.rego": `package system +main contains "hello" if { + 42 in p +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override, incompatible", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package system +p contains 42 { + input.foo == "bar" +}`, + "policy2.rego": `package system +main contains "hello" if { + 42 in p +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + "rego_parse_error: set cannot be used for rule name", + }, + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + v1CompatibleFlagCases := []struct { + note string + used bool + }{ + { + "no --v1-compatible", false, + }, + { + "--v1-compatible", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, v1CompatibleFlag := range v1CompatibleFlagCases { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, v1CompatibleFlag.note, tc.note), func(t *testing.T) { + files := map[string]string{ + "files/test.json": `{"foo": "bar"}`, + } + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + var buf bytes.Buffer + params := exec.NewParams(&buf) + params.Paths = append(params.Paths, root+"/files/") + params.BundlePaths = []string{p} + params.V1Compatible = v1CompatibleFlag.used + _ = params.OutputFormat.Set("json") + + 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) + // we cancelled the context, so we expect that error + if err != nil && err.Error() != "context canceled" { + 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(root), nil)) + + exp := util.MustUnmarshalJSON([]byte(`{"result": [{ + "path": "/files/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 5cd7f56e81..88c36d0a88 100644 --- a/cmd/inspect.go +++ b/cmd/inspect.go @@ -10,6 +10,7 @@ import ( "io" "os" "sort" + "strconv" "strings" "github.com/open-policy-agent/opa/ast" @@ -110,7 +111,8 @@ func doInspect(params inspectCommandParams, path string, out io.Writer) error { return pr.JSON(out, info) default: - if info.Manifest.Revision != "" || len(*info.Manifest.Roots) != 0 || len(info.Manifest.Metadata) != 0 { + if info.Manifest.Revision != "" || len(*info.Manifest.Roots) != 0 || len(info.Manifest.Metadata) != 0 || + info.Manifest.RegoVersion != nil { if err := populateManifest(out, info.Manifest); err != nil { return err } @@ -148,6 +150,10 @@ func populateManifest(out io.Writer, m bundle.Manifest) error { t := generateTableWithKeys(out, "field", "value") var lines [][]string + if m.RegoVersion != nil { + lines = append(lines, []string{"Rego Version", truncateTableStr(strconv.Itoa(*m.RegoVersion))}) + } + if m.Revision != "" { lines = append(lines, []string{"Revision", truncateTableStr(m.Revision)}) } diff --git a/cmd/inspect_test.go b/cmd/inspect_test.go index 1e10340796..6274a3cf36 100644 --- a/cmd/inspect_test.go +++ b/cmd/inspect_test.go @@ -693,6 +693,341 @@ p contains v if { } } +func TestDoInspectWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + bundleRegoVersion int + files map[string]string + expErrs []string + }{ + { + note: "v0.x bundle, keywords not used", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p[v] { + v := input.x +}`, + }, + }, + { + note: "v0.x bundle, no keywords imported, but used", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +p contains v if { + v := input.x +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + }, + }, + { + note: "v0.x bundle, keywords imported", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + }, + { + note: "v0.x bundle, rego.v1 imported", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test +import rego.v1 +p contains v if { + v := input.x +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[1] { + v := input.x +}`, + "policy2.rego": `package test +p contains 2 if { + v := input.x +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override (glob)", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/bar/*.rego": 1 + } +}`, + "foo/policy1.rego": `package test +p[1] { + v := input.x +}`, + "bar/policy2.rego": `package test +p contains 2 if { + v := input.x +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override, incompatible", + bundleRegoVersion: 0, + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +p[1] { + v := input.x +}`, + "policy2.rego": `package test +p[2] { + 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 bundle, keywords not used", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `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 bundle, no keywords imported", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +p contains v if { + v := input.x +}`, + }, + }, + { + note: "v1.0 bundle, keywords imported", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import future.keywords +p contains v if { + v := input.x +}`, + }, + }, + { + note: "v1.0 bundle, rego.v1 imported", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test +import rego.v1 +p contains v if { + v := input.x +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p[1] { + v := input.x +}`, + "policy2.rego": `package test +p contains 2 if { + v := input.x +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/foo/*.rego": 0 + } +}`, + "foo/policy1.rego": `package test +p[1] { + v := input.x +}`, + "bar/policy2.rego": `package test +p contains 2 if { + v := input.x +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override, incompatible", + bundleRegoVersion: 1, + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +p contains 1 if { + v := input.x +}`, + "policy2.rego": `package test +p contains 2 if { + v := input.x +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: number cannot be used for rule name", + }, + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + v1CompatibleFlagCases := []struct { + note string + used bool + }{ + { + "no --v1-compatible", false, + }, + { + "--v1-compatible", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, v1CompatibleFlag := range v1CompatibleFlagCases { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, v1CompatibleFlag.note, tc.note), func(t *testing.T) { + files := map[string]string{} + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + 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 = v1CompatibleFlag.used + err := params.outputFormat.Set(evalPrettyOutput) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + + err = doInspect(params, p, &out) + + if len(tc.expErrs) > 0 { + if err == nil { + t.Fatalf("Expected error but got output: %s", out.String()) + } + + 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) + } + + expOut := fmt.Sprintf(`MANIFEST: ++--------------+-------+ +| FIELD | VALUE | ++--------------+-------+ +| Rego Version | %d | ++--------------+-------+`, + tc.bundleRegoVersion) + if !strings.Contains(out.String(), expOut) { + t.Fatalf("Expected output to contain:\n\n%s\n\nbut got:\n\n%s", expOut, out.String()) + } + } + }) + }) + } + } + } +} + func TestCallToUnknownBuiltInFunction(t *testing.T) { files := [][2]string{ {"/policy.rego", `package test diff --git a/cmd/test_test.go b/cmd/test_test.go index 70411bf8e6..de906a6e0e 100644 --- a/cmd/test_test.go +++ b/cmd/test_test.go @@ -16,6 +16,7 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/rego" "github.com/open-policy-agent/opa/topdown" "github.com/open-policy-agent/opa/util/test" @@ -1213,3 +1214,336 @@ test_l if { } } } + +func TestWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + files map[string]string + expErr string + }{ + { + note: "v0.x bundle, no imports", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + expErr: "rego_parse_error", + }, + { + note: "v0.x bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + +import rego.v1 + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v0.x bundle, future.keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + +import future.keywords + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +l1 := {1, 3, 5} +l2[v] { + v := l1[_] +}`, + "policy2.rego": `package test +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +l1 := {1, 3, 5} +l2[v] { + v := l1[_] +}`, + "policy2.rego": `package test +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v0 bundle, v1 per-file override, incompatible", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } +}`, + "policy1.rego": `package test +l1 := {1, 3, 5} +l2[v] { + v := l1[_] +}`, + "policy2.rego": `package test +test_l { + l1 == l2 +}`, + }, + expErr: "rego_parse_error", + }, + + { + note: "v1.0 bundle, no imports", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v1.0 bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + +import rego.v1 + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v1.0 bundle, future.keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + +import future.keywords + +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +} + +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +l1 := {1, 3, 5} +l2[v] { + v := l1[_] +}`, + "policy2.rego": `package test +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "*/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +l1 := {1, 3, 5} +l2[v] { + v := l1[_] +}`, + "policy2.rego": `package test +test_l if { + l1 == l2 +}`, + }, + }, + { + note: "v1 bundle, v0 per-file override, incompatible", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } +}`, + "policy1.rego": `package test +l1 := {1, 3, 5} +l2 contains v if { + v := l1[_] +}`, + "policy2.rego": `package test +test_l if { + l1 == l2 +}`, + }, + expErr: "rego_parse_error", + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + v1CompatibleFlagCases := []struct { + note string + used bool + }{ + { + "no --v1-compatible", false, + }, + { + "--v1-compatible", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, v1CompatibleFlag := range v1CompatibleFlagCases { + for _, tc := range tests { + + t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, v1CompatibleFlag.note, tc.note), func(t *testing.T) { + files := map[string]string{} + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + var buf bytes.Buffer + var errBuf bytes.Buffer + + testParams := newTestCommandParams() + testParams.v1Compatible = v1CompatibleFlag.used + testParams.bundleMode = true + testParams.count = 1 + testParams.output = &buf + testParams.errOutput = &errBuf + + exitCode, _ := opaTest([]string{p}, testParams) + if tc.expErr != "" { + if exitCode == 0 { + t.Fatalf("expected non-zero exit code") + } + + if actual := errBuf.String(); !strings.Contains(actual, tc.expErr) { + t.Fatalf("expected error output to contain:\n\n%q\n\nbut got:\n\n%q", tc.expErr, actual) + } + } else { + if exitCode != 0 { + t.Fatalf("unexpected exit code: %d", exitCode) + } + + if errBuf.Len() > 0 { + t.Fatalf("expected no error output but got:\n\n%q", buf.String()) + } + + expected := "PASS: 1/1" + if actual := buf.String(); !strings.Contains(actual, expected) { + t.Fatalf("expected output to contain:\n\n%s\n\nbut got:\n\n%q", expected, actual) + } + } + }) + }) + } + } + } +} diff --git a/compile/compile.go b/compile/compile.go index 4f6245ac49..3f1f1d4212 100644 --- a/compile/compile.go +++ b/compile/compile.go @@ -298,7 +298,7 @@ func (c *Compiler) Build(ctx context.Context) error { } } - if err := c.initBundle(); err != nil { + if err := c.initBundle(false); err != nil { return err } @@ -369,7 +369,7 @@ func (c *Compiler) Build(ctx context.Context) error { } if c.regoVersion == ast.RegoV1 { - if err := c.bundle.FormatModulesForRegoVersion(c.regoVersion, false, false); err != nil { + if err := c.bundle.FormatModulesForRegoVersion(c.regoVersion, true, false); err != nil { return err } } else { @@ -462,7 +462,7 @@ func (c *Compiler) Bundle() *bundle.Bundle { return c.bundle } -func (c *Compiler) initBundle() error { +func (c *Compiler) initBundle(usePath bool) error { // If the bundle is already set, skip file loading. if c.bundle != nil { return nil @@ -490,7 +490,7 @@ func (c *Compiler) initBundle() error { bundles = append(bundles, load.Bundles[k]) } - result, err := bundle.Merge(bundles) + result, err := bundle.MergeWithRegoVersion(bundles, c.regoVersion, usePath) if err != nil { return fmt.Errorf("bundle merge failed: %v", err) } @@ -503,6 +503,7 @@ func (c *Compiler) initBundle() error { // contents. That would require changes to the loader to preserve the // locations where base documents were mounted under data. result := &bundle.Bundle{} + result.SetRegoVersion(c.regoVersion) if len(c.roots) > 0 { result.Manifest.Roots = &c.roots } diff --git a/compile/compile_test.go b/compile/compile_test.go index 787977bd69..85505281d3 100644 --- a/compile/compile_test.go +++ b/compile/compile_test.go @@ -9,6 +9,7 @@ import ( "io/fs" "os" "path" + "path/filepath" "reflect" "sort" "strconv" @@ -18,6 +19,7 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/bundle" "github.com/open-policy-agent/opa/format" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/internal/ref" "github.com/open-policy-agent/opa/ir" "github.com/open-policy-agent/opa/loader" @@ -155,6 +157,7 @@ func TestCompilerLoadAsBundleSuccess(t *testing.T) { expManifest := bundle.Manifest{ Roots: &expRoots, } + expManifest.SetRegoVersion(ast.RegoV0) if !compiler.bundle.Manifest.Equal(expManifest) { t.Fatalf("expected %v but got %v", compiler.bundle.Manifest, expManifest) @@ -163,6 +166,668 @@ func TestCompilerLoadAsBundleSuccess(t *testing.T) { } } +func TestCompilerLoadAsBundleWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + files map[string]string + expErrs []string + }{ + { + note: "No bundle rego version", + files: map[string]string{ + ".manifest": `{}`, + "test.rego": `package test +p[1] { + input.x == 2 +}`, + }, + }, + { + note: "v0 bundle rego version", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "test.rego": `package test +p[1] { + input.x == 2 +}`, + }, + }, + { + note: "v0 bundle rego version, missing keyword imports", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "test.rego": `package test +p contains 1 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: "v1 bundle rego version", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "test.rego": `package test +p contains 1 if { + input.x == 2 +}`, + }, + }, + { + note: "v1 bundle rego version, no keywords", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "test.rego": `package test +p[1] { + 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", + }, + }, + { + note: "v1 bundle rego version, duplicate imports", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "test.rego": `package test +import data.foo +import data.foo + +p contains 1 if { + input.x == 2 +}`, + }, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + // file overrides + { + note: "v0 bundle rego version, v1 file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test +p["A"] { + input.x == 1 +}`, + "test2.rego": `package test +p contains "B" if { + input.x == 2 +}`, + }, + }, + { + note: "v0 bundle rego version, v1 file override, missing file", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test +p["A"] { + input.x == 1 +}`, + }, + }, + { + note: "v0 bundle rego version, v1 file override, no keywords", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test +p["A"] { + input.x == 1 +}`, + "test2.rego": `package test +p["B"] { + 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", + }, + }, + { + note: "v0 bundle rego version, v1 file override, duplicate imports", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "*/test2.rego": 1 + } +}`, + "test1.rego": `package test +p["A"] { + input.x == 1 +}`, + "test2.rego": `package test +import data.foo +import data.foo + +p contains "B" if { + input.x == 2 +}`, + }, + expErrs: []string{ + "rego_compile_error: import must not shadow import data.foo", + }, + }, + { + note: "v1 bundle rego version, v0 file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "*/test1.rego": 0 + } +}`, + "test1.rego": `package test +p["A"] { + input.x == 1 +}`, + "test2.rego": `package test +p contains "B" if { + input.x == 2 +}`, + }, + }, + { + note: "v1 bundle rego version, v0 file override, no import", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "*/test1.rego": 0 + } +}`, + "test1.rego": `package test +p contains "A" if { + input.x == 1 +}`, + "test2.rego": `package test +p contains "B" if { + input.x == 2 +}`, + }, + expErrs: []string{ + "rego_parse_error: var cannot be used for rule name", + "rego_parse_error: string cannot be used for rule name", + }, + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, tc := range tests { + ctx := context.Background() + t.Run(fmt.Sprintf("%s, %s", bundleType.note, tc.note), func(t *testing.T) { + files := map[string]string{} + if bundleType.tar { + files["bundle.tar"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTestFS(tc.files, false, func(root string, fsys fs.FS) { + var path string + if bundleType.tar { + path = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + + bf, err := os.Create(path) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } else { + path = root + } + + compiler := New(). + WithFS(fsys). + WithPaths(path). + WithAsBundle(true) + + err := compiler.Build(ctx) + + if len(tc.expErrs) > 0 { + if err == nil { + t.Fatal("expected error, got none") + } + for _, expErr := range tc.expErrs { + if !strings.Contains(err.Error(), expErr) { + t.Fatalf("expected error to contain:\n\n%s\n\ngot:\n\n%v", expErr, err) + } + } + } else { + if err != nil { + t.Fatal(err) + } + } + }) + }) + } + } +} + +func pointTo[T any](x T) *T { + return &x +} + +func TestCompilerBundleMergeWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + bundles []*bundle.Bundle + regoVersion ast.RegoVersion + expGlobalRegoVersion *int + expFileRegoVersions map[string]int + }{ + { + note: "single bundle, no bundle rego version", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{}, + }, + }, + expGlobalRegoVersion: pointTo(0), + expFileRegoVersions: map[string]int{}, + }, + { + note: "single bundle, global rego version", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + RegoVersion: pointTo(1), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{}, + }, + }, + expGlobalRegoVersion: pointTo(1), + expFileRegoVersions: map[string]int{}, + }, + { + note: "no global rego versions", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{}, + }, + { + Manifest: bundle.Manifest{ + Roots: &[]string{"b"}, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{}, + }, + }, + regoVersion: ast.RegoV1, + expGlobalRegoVersion: pointTo(1), + }, + { + note: "global rego versions, v1 bundles, v0 provided", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + RegoVersion: pointTo(1), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "a/test1.rego", + URL: "a/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package a"), + }, + }, + }, + { + Manifest: bundle.Manifest{ + Roots: &[]string{"b"}, + RegoVersion: pointTo(1), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "b/test1.rego", + URL: "b/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package b"), + }, + }, + }, + }, + regoVersion: ast.RegoV0, + // global rego-version in bundles are dropped in favor of the provided rego-version + expGlobalRegoVersion: pointTo(0), + expFileRegoVersions: map[string]int{ + "/a/test1.rego": 1, + "/b/test1.rego": 1, + }, + }, + { + note: "global rego versions, v0 bundles, v1 provided", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + RegoVersion: pointTo(0), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "a/test1.rego", + URL: "a/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package a"), + }, + }, + }, + { + Manifest: bundle.Manifest{ + Roots: &[]string{"b"}, + RegoVersion: pointTo(0), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "b/test1.rego", + URL: "b/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package b"), + }, + }, + }, + }, + regoVersion: ast.RegoV1, + // global rego-version in bundles are dropped in favor of the provided rego-version + expGlobalRegoVersion: pointTo(1), + expFileRegoVersions: map[string]int{ + "/a/test1.rego": 0, + "/b/test1.rego": 0, + }, + }, + { + note: "different global rego versions", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + RegoVersion: pointTo(0), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "a/test1.rego", + URL: "a/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package a"), + }, + }, + }, + { + Manifest: bundle.Manifest{ + Roots: &[]string{"b"}, + RegoVersion: pointTo(1), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "b/test1.rego", + URL: "b/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package b"), + }, + }, + }, + }, + regoVersion: ast.RegoV0, + // global rego-version in bundles are dropped in favor of the provided rego-version + expGlobalRegoVersion: pointTo(0), + expFileRegoVersions: map[string]int{ + "/b/test1.rego": 1, + }, + }, + { + note: "different global rego versions, per-file overrides", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + RegoVersion: pointTo(1), + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "a/test1.rego", + URL: "a/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package a"), + }, + { + Path: "a/test2.rego", + URL: "a/test2.rego", + RelativePath: "/test2.rego", + Raw: []byte("package a"), + }, + }, + }, + { + Manifest: bundle.Manifest{ + Roots: &[]string{"b"}, + RegoVersion: pointTo(1), + FileRegoVersions: map[string]int{ + "/test1.rego": 0, + }, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + // we don't expect this file to get an individual rego-version in the result, as + // it has the same rego-version as the global rego-version + Path: "b/test1.rego", + URL: "b/test1.rego", + RelativePath: "/test1.rego", + Raw: []byte("package b"), + }, + { + Path: "b/test2.rego", + URL: "b/test2.rego", + RelativePath: "/test2.rego", + Raw: []byte("package b"), + }, + }, + }, + { + Manifest: bundle.Manifest{ + RegoVersion: pointTo(0), + Roots: &[]string{"c"}, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + // we don't expect these files to get individual rego-versions in the result, + // as they have the same rego-version as the global rego-version + { + Path: "c/test1.rego", + URL: "c/test1.rego", + RelativePath: "test1.rego", + Raw: []byte("package c"), + }, + { + Path: "c/test2.rego", + URL: "c/test2.rego", + RelativePath: "test2.rego", + Raw: []byte("package c"), + }, + }, + }, + }, + regoVersion: ast.RegoV0, + // global rego-version in bundles are dropped in favor of the provided rego-version + expGlobalRegoVersion: pointTo(0), + // rego-versions is expected for all modules with different rego-version than the global rego-version + expFileRegoVersions: map[string]int{ + "/a/test1.rego": 1, + "/a/test2.rego": 1, + "/b/test2.rego": 1, + }, + }, + { + note: "glob per-file overrides", + bundles: []*bundle.Bundle{ + { + Manifest: bundle.Manifest{ + Roots: &[]string{"a"}, + RegoVersion: pointTo(0), + FileRegoVersions: map[string]int{ + "a/*": 1, + }, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "a/foo/test.rego", + URL: "a/foo/test.rego", + Raw: []byte("package a"), + }, + { + Path: "a/bar/test.rego", + URL: "a/bar/test.rego", + Raw: []byte("package a"), + }, + { + Path: "a/baz/test.rego", + URL: "a/baz/test.rego", + Raw: []byte("package a"), + }, + }, + }, + { + Manifest: bundle.Manifest{ + Roots: &[]string{"b"}, + RegoVersion: pointTo(1), + FileRegoVersions: map[string]int{ + // glob should not affect files with matching path in the other bundle + "*/bar/*": 0, + }, + }, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "b/foo/test.rego", + URL: "b/foo/test.rego", + Raw: []byte("package b"), + }, + { + Path: "b/bar/test.rego", + URL: "b/bar/test.rego", + Raw: []byte("package b"), + }, + { + Path: "b/baz/test.rego", + URL: "b/baz/test.rego", + Raw: []byte("package b"), + }, + }, + }, + }, + regoVersion: ast.RegoV0, + expGlobalRegoVersion: pointTo(0), + expFileRegoVersions: map[string]int{ + "/a/foo/test.rego": 1, + "/a/bar/test.rego": 1, + "/a/baz/test.rego": 1, + "/b/foo/test.rego": 1, + "/b/baz/test.rego": 1, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + for _, b := range tc.bundles { + b.Manifest.Init() + for i, m := range b.Modules { + b.Modules[i].Parsed = ast.MustParseModule(string(m.Raw)) + } + } + + result, err := bundle.MergeWithRegoVersion(tc.bundles, tc.regoVersion, false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + compareRegoVersions(t, tc.expGlobalRegoVersion, result.Manifest.RegoVersion) + + if !reflect.DeepEqual(tc.expFileRegoVersions, result.Manifest.FileRegoVersions) { + t.Fatalf("expected file rego versions to be:\n\n%v\n\nbut got:\n\n%v", tc.expFileRegoVersions, result.Manifest.FileRegoVersions) + } + }) + } +} + +func compareRegoVersions(t *testing.T, exp, act *int) { + t.Helper() + if exp == nil { + if act != nil { + t.Errorf("expected no rego version, but got %v", *act) + } + } else { + if act == nil { + t.Errorf("expected rego version to be %v, but got none", *exp) + } else if *act != *exp { + t.Errorf("expected rego version to be %v, but got %v", *exp, *act) + } + } +} + func TestCompilerLoadAsBundleMergeError(t *testing.T) { ctx := context.Background() @@ -1122,6 +1787,7 @@ func TestCompilerWasmTargetMultipleEntrypoints(t *testing.T) { expManifest := bundle.Manifest{} expManifest.Init() + expManifest.SetRegoVersion(ast.RegoV0) expManifest.WasmResolvers = []bundle.WasmResolver{ { Entrypoint: "test/p", @@ -1261,6 +1927,7 @@ func TestCompilerWasmTargetEntrypointDependents(t *testing.T) { expManifest := bundle.Manifest{} expManifest.Init() + expManifest.SetRegoVersion(ast.RegoV0) expManifest.WasmResolvers = []bundle.WasmResolver{ { Entrypoint: "test/r", @@ -1859,6 +2526,7 @@ func TestCompilerOutput(t *testing.T) { p { input.x = data.foo }`))), "data.json": `{"foo": 1}`, + ".manifest": `{"rego_version": 0}`, } for _, useMemoryFS := range []bool{false, true} { diff --git a/download/download_test.go b/download/download_test.go index 498b0be033..2dea57b842 100644 --- a/download/download_test.go +++ b/download/download_test.go @@ -589,6 +589,135 @@ func TestOneShotV1Compatible(t *testing.T) { } } +func TestOneShotWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + bundleRegoVersion int + module string + expErrs []string + }{ + { + note: "v0.x bundle, keywords not used", + bundleRegoVersion: 0, + module: `package test +p[1] { + input.x == 2 +}`, + }, + { + note: "v0.x bundle, keywords used but not imported", + bundleRegoVersion: 0, + module: `package test +p contains 1 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 bundle, keywords used, rego.v1 imported", + bundleRegoVersion: 0, + module: `package test +import rego.v1 +p contains 1 if { + input.x == 2 +}`, + }, + + { + note: "v1.0 bundle, keywords not used", + bundleRegoVersion: 1, + module: `package test +p[1] { + 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", + }, + }, + { + note: "v1.0 bundle, keywords used, not imported", + bundleRegoVersion: 1, + module: `package test +p contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0, keywords used, rego.v1 imported", + bundleRegoVersion: 1, + module: `package test +import rego.v1 +p contains 1 if { + input.x == 2 +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx := context.Background() + fixture := newTestFixture(t) + fixture.d = New(Config{}, fixture.client, "bundles/custom"). + WithCallback(fixture.oneShot) + fixture.server.expEtag = "some etag value" + + fixture.server.bundles["custom"] = bundle.Bundle{ + Manifest: bundle.Manifest{RegoVersion: &tc.bundleRegoVersion}, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "test.rego", + Raw: []byte(tc.module), + }, + }, + } + + 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_test.go b/download/oci_download_test.go index 8e80bee7c3..6b5e2ff3cd 100644 --- a/download/oci_download_test.go +++ b/download/oci_download_test.go @@ -7,13 +7,14 @@ import ( "context" "encoding/base64" "fmt" - "github.com/open-policy-agent/opa/ast" - "github.com/open-policy-agent/opa/bundle" "net/http" "strings" "testing" "time" + "github.com/open-policy-agent/opa/ast" + "github.com/open-policy-agent/opa/bundle" + "github.com/open-policy-agent/opa/keys" "github.com/open-policy-agent/opa/plugins/rest" ) @@ -67,9 +68,9 @@ func TestOCIDownloaderWithRegoV1Bundle(t *testing.T) { regoVersion ast.RegoVersion expErr string }{ + // The bundle contains a v1 rego_version attr, so we expect no errors regardless of parser regoVersion. { - note: "non-1.0 compatible OCI downloader", - expErr: "rego_parse_error", + note: "non-1.0 compatible OCI downloader", }, { note: "1.0 compatible OCI downloader", diff --git a/download/testdata/rego_v1.manifest b/download/testdata/rego_v1.manifest index 6b9eaeac10..ec782ec915 100644 --- a/download/testdata/rego_v1.manifest +++ b/download/testdata/rego_v1.manifest @@ -8,8 +8,8 @@ "layers":[ { "mediaType":"application/vnd.oci.image.layer.v1.tar+gzip", - "digest":"sha256:cc09b0f5ac97b11637c96ff1b0fbbc287c5ba0169813edaa71fe58424e95f0b7", - "size":695, + "digest":"sha256:0f93a2c5964d7c8b676e3b507b6bc3b771c086428dece6f84256b9948cb3f256", + "size":830, "annotations":{ "org.opencontainers.image.created":"2022-02-11T09:00:07Z", "org.opencontainers.image.title":"dani/testpol" diff --git a/download/testdata/rego_v1.tar.gz b/download/testdata/rego_v1.tar.gz index 7c0bf876383461ab968565d60ae678dccfa6ff0d..5826c65170bcc074b508acf5ffc3f70ff9851a4d 100644 GIT binary patch literal 830 zcmV-E1Ht?siwFP!00000|Lm8~ZmT#H#_x7`3gp#{R3)|{X%nf7PDv=@s4_<53EIb^xF{|5hcuJ^_LB0K^Q^dEs={{*0ai%#%~argA}r2TAV z4PQY!53{`P0quPlrS}i$-ajNO8Qwq7ZSs8A-lb9m<@eC4|Mr79KJq^@yKnuEkqJAE z|G$Er7i_&K)WgD4I~i))gGi;9i!{mfAWd>lB$=+VNEd7UQTweP_0M&%38hSd_M~Z^ zlriWY$$yu2D-%)T<%fNkebzDH1OFp@`p*9do9MLh|5xy+ z>ELw-fWCgK={g9@(Z;s}F%~0dQ8|bTCv#N3cFbe&`>17o^nH`C3vF_Ikj)~k59X36rv?Lyw*>1JgQ9=xk53@&>|9~V1j zY##!H^gQocby}NFDinE zf>cXMl~b@vZdp7qiA{>}d?cvL3{ouxQHzpQTSRSw_;YE)jN@ME-Aq%z)-I_tnZc6I znOM(?Q%7|+zPqSD$4fnGSNYg{t8ISK;r60r^FhV!k*amEcyQ!%z9CjnP-{}qY9(kq z72JwUZWGMyrKDAas6|YsXiTl3q_K>7U6;!coBfD3=is};gXeM?7?U^J=$t^RSViLt zW9&{!q6j7y6-`yFG*f5U#r+vQW<$t&SEmzy z7M)s)IRu)1ccSTFCl8TGKwk&U;O+#xTnWPc%Q9-taSaU(4gVkh4gdiE|8W8P IP5>4F07vJWG5`Po literal 695 zcmV;o0!aNIiwFP!00000|Lm4qZ=*OI#rwK^3X7**shgNfvQ4Bu%;rKt#Z^)Qu}7oG zTmmNMqM9T?mG8cyJ5n{X57lbb&W^>Yb44uim`B(JuxA|8U9gh0^ ze*`5B4m6HHRMV{FktCg1!D+>cT!5g+E6Q>KG7$r|2hY+kU2A#^sDJJ1b5*GXtBPV{aKgW2Tlt96OB!znN( zf_i9lpLh4%u=|-+;4Lc5v|ta$Mj;N?$UJTy=0R&`3mvI$VjH+2o_V&@Ppz#knccb^ zWoC_Ae!%HMw+*{~a4d#Atl6#0>}=EE_MjlcR>+IBr6B60q1$!gciK5JSC6nk4rcqu z`VTW3CBC=mUdP{lvaL5=3C8nq!q>6(#w ztP{gliQ#h8#tNF-I?hmv=0Sr}o|4X&hZX$zhBg;=pM^&rKXe+1;bwU2ig0F429rcM z7M-s==&f!-v32T^H(Q*W-`zixqTVJtE+T#t>`r$=mcW;53CJZs#w?eoK;DJXo*sE6 dOP3!#LwkC9dV1c2zXAXN|Nmg!N@xHQ000!@Sfl^| diff --git a/download/testdata/signed.manifest b/download/testdata/signed.manifest index 4aecee2583..f5e5b6b98f 100644 --- a/download/testdata/signed.manifest +++ b/download/testdata/signed.manifest @@ -8,8 +8,8 @@ "layers":[ { "mediaType":"application/vnd.oci.image.layer.v1.tar+gzip", - "digest":"sha256:e060c7b9558fad3ec85df5ffa19d0d019f839c36d7ec146977c871dcbc70885e", - "size":629, + "digest":"sha256:7fccf82798e6e627afd04144889570d966583788473db2888f0d0d325904d273", + "size":764, "annotations":{ "org.opencontainers.image.created":"2022-02-11T09:00:07Z", "org.opencontainers.image.title":"dani/testpol" diff --git a/download/testdata/signed.tar.gz b/download/testdata/signed.tar.gz index f08d5185ca08f217b0de218ff3d06c9dadd601e7..9e8374cdd7cb4464f96ae5383d4bec8e692fc17c 100644 GIT binary patch literal 764 zcmV-MCBZSC& zxo_tU16t^;58uwx3sq72$20h!wZ1Rzf5Id1!SoRX?PUk1H^#smpxdXXr{1r2*6|*U z%BU#YJz!LbeHMjxL6wCvx_h3A%Q7G32Pt-^e}~~Q+8=&aG27+Dw--X=Y zrT#wZM~$y?8Fi`u%@_^a-1}v9jEc8v1769$Vg6x#_y0G?#-z*td+^xngO@J=O#3&z zejh}qc^o)9u@G~-(b%8@7g&`!|AZ%4tt3~r5nL_B{c;$c=F0aPxjW&BhYArWyX0#; z!P!dg&?;qLR`E5WIX5lI?Q;=&RvI|ld+ykftn z&4N=a8^=3y-&&Ex+0=AK8kR84-2Is7jX@Nysk?8exzxm*$Hb+I>hwetToYF&+}SFY ztR(epNoK5O8e0pGG~C%V+{0rmYQelF3(!0dow&rwL2O`raqYHc&(E)~zN$&B;?TLQ z{ER*X;%jf%A~TtC_gOz>iXraVg18rws>_<{OGsyzh?>}>&OBYr;*@Ega&LncvkfFV z7R)4&YHTh%s+hYsc(RRu%A2rC=2@gbLzHNoMJoCSl5yEvZfoW)e+Y8S{3H6P|K47c{}>HE&T u0FH-4S%B#c0Jj6sHv4xE(eaM5NcOd#lM2V z5LBQLs(kkm)ky7h7S(8!R-@s!;G(-Z=c0SC2cBKn{U|@A7iR+i07d}3RPb{JK!&gH zZC{drB3}sLe3o7*^TK|-0KcvEez-q`XVDfX&!W>lTyb(MUBXNG?sPi!zS>#GXD}%I zylDRcgPaAa@7+6P>T&=6r4<%MHpmW~xySFN|L6yYU)A_9{}B9`e<;bI%m1g4*=}qH zzTnq(l5xLF{a?nQ-{#&f%11vx*EQgS{7Yacy*vNm5GY;#KZEC7h~pGO6;in2dXXr*^Lk0Qm@wX^vO|qDp+=UX9nwH!~%A=&AG4PVtk& z-g-ldHsdihUi4#<5SdyFrWsR?lX-=kIl!~Ij3I(pUB#PO7?Xy^)H3DG%mi8!G6FS# z8w58NPKXgmG}?zBdAr$F%!Yf#wzlo}iK#_H&#D1RL7!>nzt{ix)2nQ$|L3F&x|wn3 zsce*?pT=J8?5_?t%)H(yH4uBk-DOV@GhPNP6(@pt@A(gQ$qTV}`sxMk=;-L^I1Aqb P0096094&Xe01^NILH{-S diff --git a/plugins/bundle/plugin_test.go b/plugins/bundle/plugin_test.go index 9fe5e85845..cfb3402530 100644 --- a/plugins/bundle/plugin_test.go +++ b/plugins/bundle/plugin_test.go @@ -250,6 +250,252 @@ corge contains 1 if { } } +func TestPluginOneShotWithBundleRegoVersion(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 + managerRegoVersion ast.RegoVersion + bundleRegoVersion *ast.RegoVersion + module string + expErrs []string + }{ + { + note: "v0.x manager, no bundle version", + managerRegoVersion: ast.RegoV0, + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, no bundle version, shadowed import (no error)", + managerRegoVersion: ast.RegoV0, + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + + { + note: "v0.x manager, v0.x bundle", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, shadowed import (no error)", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + + { + note: "v0.x manager, v1.0 bundle", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV1), + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, shadowed import (error)", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV1), + 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", + }, + }, + + { + note: "v1.0 manager, no bundle version", + managerRegoVersion: ast.RegoV1, + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, no bundle version, shadowed import (error)", + managerRegoVersion: ast.RegoV1, + module: `package foo +import future.keywords +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", + }, + }, + + { + note: "v1.0 manager, v0.x bundle", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, shadowed import (no error)", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + + { + note: "v1.0 manager, v1.0 bundle", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV1), + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle, shadowed import (error)", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV1), + 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) { + ctx := context.Background() + managerPopts := ast.ParserOptions{RegoVersion: tc.managerRegoVersion} + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), + plugins.WithParserOptions(managerPopts)) + if err != nil { + t.Fatal(err) + } + 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) + + var bundlePopts ast.ParserOptions + m := bundle.Manifest{Revision: "quickbrownfaux"} + if tc.bundleRegoVersion != nil { + m.SetRegoVersion(*tc.bundleRegoVersion) + bundlePopts = ast.ParserOptions{RegoVersion: *tc.bundleRegoVersion} + } else { + bundlePopts = managerPopts + } + b := bundle.Bundle{ + Manifest: m, + Data: map[string]interface{}{}, + Modules: []bundle.ModuleFile{ + { + Path: "/foo/bar", + Parsed: ast.MustParseModuleWithOpts(tc.module, bundlePopts), + 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() @@ -1196,6 +1442,298 @@ corge contains 1 if { } } +func TestPluginOneShotBundlePersistenceWithBundleRegoVersion(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 + managerRegoVersion ast.RegoVersion + bundleRegoVersion *ast.RegoVersion + module string + expErrs []string + }{ + { + note: "v0.x manager, no bundle rego version", + managerRegoVersion: ast.RegoV0, + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, no bundle rego version, shadowed import (no error)", + managerRegoVersion: ast.RegoV0, + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, shadowed import (no error)", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV1), + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, shadowed import", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV1), + 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", + }, + }, + + { + note: "v1.0 manager, no bundle rego version", + managerRegoVersion: ast.RegoV1, + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, no bundle rego version, shadowed import (no error)", + managerRegoVersion: ast.RegoV1, + module: `package foo +import future.keywords +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", + }, + }, + { + note: "v1.0 manager, v0.x bundle", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, shadowed import (no error)", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +import future.keywords +import data.foo +import data.bar as foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV1), + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle, shadowed import", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV1), + 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) { + ctx := context.Background() + managerPopts := ast.ParserOptions{RegoVersion: tc.managerRegoVersion} + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), + plugins.WithParserOptions(managerPopts)) + 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 + var bundlePopts ast.ParserOptions + m := bundle.Manifest{Revision: "quickbrownfaux"} + if tc.bundleRegoVersion != nil { + m.SetRegoVersion(*tc.bundleRegoVersion) + bundlePopts = ast.ParserOptions{RegoVersion: *tc.bundleRegoVersion} + } else { + bundlePopts = managerPopts + } + b := bundle.Bundle{ + Manifest: m, + 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, bundlePopts), + 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{}) + var regoVersion string + if tc.bundleRegoVersion != nil { + regoVersion = fmt.Sprintf(`, "rego_version": %d`, bundleRegoVersion(*tc.bundleRegoVersion)) + } else { + regoVersion = "" + } + expData := util.MustUnmarshalJSON([]byte(fmt.Sprintf(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "foo", "manifest": {"revision": "quickbrownfaux"%s, "roots": [""]}}}}}`, + regoVersion))) + 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() @@ -1376,10 +1914,11 @@ 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 string + v1Compatible bool + bundleRegoVersion *ast.RegoVersion + module string + expErrs []string }{ { note: "v0.x", @@ -1431,7 +1970,8 @@ corge contains 1 if { popts := ast.ParserOptions{RegoVersion: regoVersion} ctx := context.Background() - manager, err := plugins.New(nil, "test-instance-id", inmem.New(), plugins.WithParserOptions(popts)) + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), + plugins.WithParserOptions(popts)) if err != nil { t.Fatal("unexpected error:", err) } @@ -1535,6 +2075,234 @@ corge contains 1 if { } } +func TestLoadAndActivateBundlesFromDiskWithBundleRegoVersion(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 + managerRegoVersion ast.RegoVersion + bundleRegoVersion *ast.RegoVersion + module string + expErrs []string + }{ + { + note: "v0.x manager, no bundle rego version", + managerRegoVersion: ast.RegoV0, + module: `package foo +corge[1] { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +corge[1] { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV1), + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, compiler err (shadowed import)", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: pointTo(ast.RegoV1), + 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", + }, + }, + { + note: "v1.0 manager, no bundle rego version", + managerRegoVersion: ast.RegoV1, + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, no bundle rego version, compiler err (shadowed import)", + managerRegoVersion: ast.RegoV1, + 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", + }, + }, + { + note: "v1.0 manager, v0.x bundle", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV0), + module: `package foo +corge[1] { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: pointTo(ast.RegoV1), + module: `package foo +corge contains 1 if { + input.x == 2 +}`, + }, + } + + for _, tc := range tests { + t.Run(tc.note, func(t *testing.T) { + ctx := context.Background() + managerPopts := ast.ParserOptions{RegoVersion: tc.managerRegoVersion} + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), + plugins.WithParserOptions(managerPopts)) + 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 + m := bundle.Manifest{Revision: "quickbrownfaux"} + var bundlePopts ast.ParserOptions + if tc.bundleRegoVersion != nil { + m.SetRegoVersion(*tc.bundleRegoVersion) + bundlePopts = ast.ParserOptions{RegoVersion: *tc.bundleRegoVersion} + } else { + bundlePopts = managerPopts + } + b := bundle.Bundle{ + Manifest: m, + 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, bundlePopts), + 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{}) + regoVersionStr := "" + if tc.bundleRegoVersion != nil { + regoVersionStr = fmt.Sprintf(`, "rego_version": %d`, bundleRegoVersion(*tc.bundleRegoVersion)) + } + expData := util.MustUnmarshalJSON([]byte(fmt.Sprintf(`{"foo": {"bar": 1, "baz": "qux"}, "system": {"bundles": {"test-bundle": {"etag": "", "manifest": {"revision": "quickbrownfaux"%s, "roots": [""]}}}}}`, + regoVersionStr))) + 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 pointTo[T any](v T) *T { + return &v +} + +func bundleRegoVersion(v ast.RegoVersion) int { + switch v { + case ast.RegoV0: + return 0 + case ast.RegoV0CompatV1: + return 0 + case ast.RegoV1: + return 1 + } + panic("unknown ast.RegoVersion") +} + func TestLoadAndActivateDepBundlesFromDisk(t *testing.T) { ctx := context.Background() manager := getTestManager() @@ -3655,6 +4423,321 @@ p contains 7 if { } } +func TestPluginUsingFileLoaderWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + managerRegoVersion ast.RegoVersion + bundleRegoVersion ast.RegoVersion + module string + expErrs []string + }{ + { + note: "v0.x manager, v0.x bundle, keywords not used", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +p[7] { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, shadowed import", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, keywords not imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + 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 manager, v0.x bundle, keywords imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, rego.v1 imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + // parse-time error + { + note: "v0.x manager, v1.0 bundle, keywords not used", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + 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: "v0.x manager, v1.0 bundle, shadowed import", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + 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: "v0.x manager, v1.0 bundle, keywords not imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + module: `package test +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, keywords imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, rego.ve imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + + { + note: "v1.0 manager, v0.x bundle, keywords not used", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +p[7] { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, shadowed import", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, keywords not imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + 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: "v1.0 manager, v0.x bundle, keywords imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, rego.v1 imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + // parse-time error + { + note: "v1.0 manager, v1.0 bundle, keywords not used", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + 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 manager, v1.0 bundle, shadowed import", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + 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 manager, v1.0 bundle, keywords not imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + module: `package test +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle, keywords imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle, rego.ve imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + 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) { + test.WithTempFS(map[string]string{}, func(dir string) { + + manifest := bundle.Manifest{} + manifest.SetRegoVersion(tc.bundleRegoVersion) + b := bundle.Bundle{ + Manifest: manifest, + 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() + + managerPopts := ast.ParserOptions{RegoVersion: tc.managerRegoVersion} + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), + plugins.WithParserOptions(managerPopts)) + 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 @@ -3856,6 +4939,298 @@ p contains 7 if { } } +func TestPluginUsingDirectoryLoaderWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + managerRegoVersion ast.RegoVersion + bundleRegoVersion ast.RegoVersion + module string + expErrs []string + }{ + { + note: "v0.x manager, v0.x bundle, keywords not used", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +p[7] { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, shadowed import", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, keywords not imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + 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 manager, v0.x bundle, keywords imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v0.x bundle, rego.v1 imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV0, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + // parse-time error + { + note: "v0.x manager, v1.0 bundle, keywords not used", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + 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: "v0.x manager, v1.0 bundle, shadowed import", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + 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: "v0.x manager, v1.0 bundle, keywords not imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + module: `package test +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, keywords imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v0.x manager, v1.0 bundle, rego.ve imported", + managerRegoVersion: ast.RegoV0, + bundleRegoVersion: ast.RegoV1, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + + { + note: "v1.0 manager, v0.x bundle, keywords not used", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +p[7] { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, shadowed import", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +import data.foo +import data.bar as foo +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, keywords not imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + 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: "v1.0 manager, v0.x bundle, keywords imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v0.x bundle, rego.v1 imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV0, + module: `package test +import rego.v1 +p contains 7 if { + input.x == 2 +}`, + }, + // parse-time error + { + note: "v1.0 manager, v1.0 bundle, keywords not used", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + 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 manager, v1.0 bundle, shadowed import", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + 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 manager, v1.0 bundle, keywords not imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + module: `package test +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle, keywords imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + module: `package test +import future.keywords +p contains 7 if { + input.x == 2 +}`, + }, + { + note: "v1.0 manager, v1.0 bundle, rego.ve imported", + managerRegoVersion: ast.RegoV1, + bundleRegoVersion: ast.RegoV1, + 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) { + test.WithTempFS(map[string]string{ + "test.rego": tc.module, + ".manifest": fmt.Sprintf(`{"rego_version": %d}`, bundleRegoVersion(tc.bundleRegoVersion)), + }, func(dir string) { + + managerPopts := ast.ParserOptions{RegoVersion: tc.managerRegoVersion} + manager, err := plugins.New(nil, "test-instance-id", inmem.New(), + plugins.WithParserOptions(managerPopts)) + 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_test.go b/plugins/discovery/discovery_test.go index 21d3758ee5..37e717168f 100644 --- a/plugins/discovery/discovery_test.go +++ b/plugins/discovery/discovery_test.go @@ -15,6 +15,7 @@ import ( "net/http" "net/http/httptest" "os" + "path" "path/filepath" "reflect" "strings" @@ -1064,6 +1065,176 @@ bundles.authz.service := v if { } } +func TestLoadAndActivateBundleFromDiskWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + bundleRegoVersion int + modules map[string]versionedModule + }{ + { + note: "v0 bundle", + bundleRegoVersion: 0, + modules: map[string]versionedModule{ + "policy.rego": {-1, `package config + +labels.x := "label value changed" +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v { + v := {"a": "b"} +} + +services.acmecorp.url := v { + v := "http://localhost:8181" +} + +bundles.authz.service := v { + v := "localhost" +}`}, + }, + }, + { + note: "v0 bundle, v1 per-file override", + bundleRegoVersion: 0, + modules: map[string]versionedModule{ + "policy1.rego": {-1, `package config + +labels.x := "label value changed" +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v { + v := {"a": "b"} +}`}, + "policy2.rego": {1, `package config + +services.acmecorp.url := v if { + v := "http://localhost:8181" +} + +bundles.authz.service := v if { + v := "localhost" +}`}, + }, + }, + { + note: "v1 bundle", + bundleRegoVersion: 1, + // no future.keywords import + modules: map[string]versionedModule{ + "policy.rego": {-1, `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" +}`}, + }, + }, + { + note: "v1 bundle, v0 per-file override", + bundleRegoVersion: 1, + modules: map[string]versionedModule{ + "policy1.rego": {0, `package config + +labels.x := "label value changed" +default_decision := "bar/baz" +default_authorization_decision := "baz/qux" +plugins.test_plugin := v { + v := {"a": "b"} +}`}, + "policy2.rego": {-1, `package config + +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) { + 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()) + 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 := makeBundleWithRegoVersion(1, tc.bundleRegoVersion, tc.modules) + + 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() @@ -1379,6 +1550,147 @@ plugins.test_plugin := v if { 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) } + + regoV0Bundle := makeModuleBundleWithRegoVersion(2, `package config +labels := v { + v := {"a": "zero"} +} +default_decision := "bar/baz" +default_authorization_decision := "baz/qux"`, 0) + + disco.oneShot(ctx, download.Update{Bundle: regoV0Bundle}) + + 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) + } + + expLabel := "zero" + actLabel := manager.Labels()["a"] + if actLabel != expLabel { + t.Errorf(`Expected label "a" to be: %v, got: %v`, expLabel, actLabel) + } + + regoV1Bundle := makeModuleBundleWithRegoVersion(2, `package config +labels := v if { + v := {"a": "one"} +} +default_decision := "bar/baz" +default_authorization_decision := "baz/qux"`, 1) + + disco.oneShot(ctx, download.Update{Bundle: regoV1Bundle}) + + 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) + } + + expLabel = "one" + actLabel = manager.Labels()["a"] + if actLabel != expLabel { + t.Errorf(`Expected label "a" to be: %v, got: %v`, expLabel, actLabel) + } +} + +func TestReconfigureWithBundleRegoVersion(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 := makeModuleBundleWithRegoVersion(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"} +}`, 1) + + 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 := makeModuleBundleWithRegoVersion(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"} +}`, 1) + + 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) { @@ -2281,6 +2593,65 @@ func makeModuleBundle(n int, s string, popts ast.ParserOptions) *bundleApi.Bundl } } +func makeModuleBundleWithRegoVersion(revision int, bundle string, regoVersion int) *bundleApi.Bundle { + popts := ast.ParserOptions{} + if regoVersion == 0 { + popts.RegoVersion = ast.RegoV0 + } else { + popts.RegoVersion = ast.RegoV1 + } + return &bundleApi.Bundle{ + Manifest: bundleApi.Manifest{ + Revision: fmt.Sprintf("test-revision-%v", revision), + RegoVersion: ®oVersion, + }, + Modules: []bundleApi.ModuleFile{ + { + URL: `policy.rego`, + Path: `/policy.rego`, + Raw: []byte(bundle), + Parsed: ast.MustParseModuleWithOpts(bundle, popts), + }, + }, + Data: map[string]interface{}{}, + } +} + +type versionedModule struct { + version int + module string +} + +func makeBundleWithRegoVersion(revision int, bundleRegoVersion int, modules map[string]versionedModule) *bundleApi.Bundle { + b := bundleApi.Bundle{ + Manifest: bundleApi.Manifest{ + Revision: fmt.Sprintf("test-revision-%v", revision), + RegoVersion: &bundleRegoVersion, + FileRegoVersions: map[string]int{}, + }, + Data: map[string]interface{}{}, + } + + for k, v := range modules { + p := path.Join("/", k) + popts := ast.ParserOptions{} + if v.version >= 0 { + b.Manifest.FileRegoVersions[p] = v.version + popts.RegoVersion = ast.RegoVersionFromInt(v.version) + } else { + popts.RegoVersion = ast.RegoVersionFromInt(bundleRegoVersion) + } + b.Modules = append(b.Modules, bundleApi.ModuleFile{ + URL: k, + Path: p, + Raw: []byte(v.module), + Parsed: ast.MustParseModuleWithOpts(v.module, popts), + }) + } + + return &b +} + func getTestManager(t *testing.T, conf string) *plugins.Manager { t.Helper() store := inmem.New() diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index ba68fe4ca6..6c2179a364 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/open-policy-agent/opa/internal/file/archive" "github.com/open-policy-agent/opa/loader" "github.com/open-policy-agent/opa/internal/report" @@ -914,6 +915,326 @@ func TestServerInitializedWithRegoV1(t *testing.T) { } } +func TestServerInitializedWithBundleRegoVersion(t *testing.T) { + tests := []struct { + note string + files map[string]string + expErr string + }{ + { + note: "v0.x bundle, keywords not imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + p if { + input.x == 1 + } + `, + }, + expErr: "rego_parse_error: var cannot be used for rule name", + }, + { + note: "v0.x bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + import rego.v1 + p if { + input.x == 1 + } + `, + }, + }, + { + note: "v0.x bundle, future.keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + import future.keywords.if + p if { + input.x == 1 + } + `, + }, + }, + { + note: "v0.x bundle, no keywords used", + files: map[string]string{ + ".manifest": `{"rego_version": 0}`, + "policy.rego": `package test + p { + input.x == 1 + } + `, + }, + }, + { + note: "v0 bundle, v1 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } + }`, + "policy1.rego": `package test + p[1] { + input.x == 1 + } + `, + "policy2.rego": `package test + q contains 2 if { + input.x == 1 + } + `, + }, + }, + { + note: "v0 bundle, v1 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/bar/*.rego": 1 + } + }`, + "foo/policy1.rego": `package test + p[1] { + input.x == 1 + } + `, + "bar/policy2.rego": `package test + q contains 2 if { + input.x == 1 + } + `, + }, + }, + { + note: "v0 bundle, v1 per-file override, incompatible", + files: map[string]string{ + ".manifest": `{ + "rego_version": 0, + "file_rego_versions": { + "/policy2.rego": 1 + } + }`, + "policy1.rego": `package test + p[1] { + input.x == 1 + } + `, + "policy2.rego": `package test + q[2] { + input.x == 1 + } + `, + }, + expErr: "rego_parse_error", + }, + + { + note: "v1.0 bundle, keywords not imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + p if { + input.x == 1 + } + `, + }, + }, + { + note: "v1.0 bundle, rego.v1 imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + import rego.v1 + p if { + input.x == 1 + } + `, + }, + }, + { + note: "v1.0 bundle, future.keywords imported", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + import future.keywords.if + p if { + input.x == 1 + } + `, + }, + }, + { + note: "v1.0 bundle, no keywords used", + files: map[string]string{ + ".manifest": `{"rego_version": 1}`, + "policy.rego": `package test + p { + input.x == 1 + } + `, + }, + expErr: "rego_parse_error: `if` keyword is required before rule body", + }, + { + note: "v1 bundle, v0 per-file override", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } + }`, + "policy1.rego": `package test + p[1] { + input.x == 1 + } + `, + "policy2.rego": `package test + q contains 2 if { + input.x == 1 + } + `, + }, + }, + { + note: "v1 bundle, v0 per-file override (glob)", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/foo/*.rego": 0 + } + }`, + "foo/policy1.rego": `package test + p[1] { + input.x == 1 + } + `, + "bar/policy2.rego": `package test + q contains 2 if { + input.x == 1 + } + `, + }, + }, + { + note: "v1 bundle, v0 per-file override, incompatible", + files: map[string]string{ + ".manifest": `{ + "rego_version": 1, + "file_rego_versions": { + "/policy1.rego": 0 + } + }`, + "policy1.rego": `package test + p contains 1 if { + input.x == 1 + } + `, + "policy2.rego": `package test + q contains 2 if { + input.x == 1 + } + `, + }, + expErr: "rego_parse_error", + }, + } + + bundleTypeCases := []struct { + note string + tar bool + }{ + { + "bundle dir", false, + }, + { + "bundle tar", true, + }, + } + + for _, bundleType := range bundleTypeCases { + for _, tc := range tests { + t.Run(fmt.Sprintf("%s, %s", bundleType.note, tc.note), func(t *testing.T) { + files := map[string]string{} + if bundleType.tar { + files["bundle.tar.gz"] = "" + } else { + for k, v := range tc.files { + files[k] = v + } + } + + test.WithTempFS(files, func(root string) { + p := root + if bundleType.tar { + p = filepath.Join(root, "bundle.tar.gz") + files := make([][2]string, 0, len(tc.files)) + for k, v := range tc.files { + files = append(files, [2]string{k, v}) + } + buf := archive.MustWriteTarGz(files) + bf, err := os.Create(p) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + _, err = bf.Write(buf.Bytes()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Millisecond) + defer cancel() + var output bytes.Buffer + + params := NewParams() + params.Output = &output + params.Paths = []string{p} + params.BundleMode = true + params.Addrs = &[]string{"localhost:0"} + params.GracefulShutdownPeriod = 1 + params.Logger = logging.NewNoOpLogger() + + 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"}