test: Add schema flag to test (abandoned) #5951

renatosc · 2023-05-25T00:19:38Z

Why the changes in this PR are needed?

To enable schema validation on opa test the same way that is supported today by opa eval

What are the changes in this PR?

schema flags reading: schema: &schemaFlags{}
schema files reading: schemaSet, err = loader.Schemas(testParams.schema.path)
schema validation: ast.NewCompiler().WithSchemas(schemaSet)

Notes to assist PR review:

Fixes: #5923

Signed-off-by: Renato Cordeiro renato@renatocordeiro.com

UPDATE
This PR has been replaced by PR 5957

anderseknert

LGTM! Have you tested all the options manually? i.e. without schema, schema on file and schemas on directory.

johanfylling

Looking good 👍.
There's just a few changes required to drive this across the goal line 🙂 .

johanfylling · 2023-05-25T12:59:54Z

cmd/test.go

 	compiler := ast.NewCompiler().
 		SetErrorLimit(testParams.errLimit).
 		WithPathConflictsCheck(storage.NonEmpty(ctx, store, txn)).
 		WithEnablePrintStatements(!testParams.benchmark).
-		WithCapabilities(capabilities)
+		WithCapabilities(capabilities).
+		WithSchemas(schemaSet)


I think you also need to set WithUseTypeCheckAnnotations to make the compiler respect the schemas metadata annotation.
See e.g.

opa/cmd/check.go

Line 102 in dfe947a

WithUseTypeCheckAnnotations(true)

Likewise, to make the parser pick up those annotations, WithProcessAnnotation must be set on the file loaders at

opa/tester/runner.go

Line 591 in dfe947a

loaded, err := loader.NewFileLoader().Filtered(args, filter)

and

opa/tester/runner.go

Line 619 in dfe947a

b, err := loader.NewFileLoader().WithSkipBundleVerification(true).AsBundle(bundleDir)

Hi @johanfylling, thanks for the comments.

Yes, I identified the lack of annotation support and I mentioned that on the issue 5923 and was wondering if that should be another issue or not (since technically they are different features). What do you think? Work on that on a different issue or on this one?

If one wants to be pedantic, we could set WithUseTypeCheckAnnotations and WithProcessAnnotation to true only if we have a non-empty schema set. But looking at both the check command and the eval command, which uses the rego SDK, we hard-code to true there, so it's probably fine to do so here too.

Ok, I will update the PR with that.

Sorry @renatosc , I missed your first comment. If I'm to express an opinion here, I think that to not break user expectations with this feature, it's probably a good idea to have test work similarly to eval and check when using the --schema flag.

johanfylling · 2023-05-25T13:34:49Z

We should add tests for this in test_test.go.
The already existing

opa/cmd/test_test.go

Line 234 in dfe947a

func TestSchemasAnnotation(t *testing.T) {

is a good start, but looking at it now, it looks like it's currently doing the wrong thing 😬 !
It's using the rego API:

opa/cmd/test_test.go

Line 227 in dfe947a

).Eval(context.Background())

but I think it should probably be calling

opa/cmd/test.go

Line 151 in dfe947a

func opaTest(args []string) int {

similar to what eval_test.go is doing:

opa/cmd/eval_test.go

Line 472 in dfe947a

defined, err = eval([]string{query}, params, &buf)

Fixes: #5923 Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Prior to this change, the `fmt` command produced invalid code when applied to the rego files presented in #5537. With these changes, the `fmt` command is now able to handle the edge cases that produce invalid code in output. Fixes: #5537 Signed-off-by: Gianluca Oldani <oldanigianluca@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Signed-off-by: pheianox <77569421+pheianox@users.noreply.github.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

We can use alternative functions/methods to avoid unnecessary byte/string conversion calls. Signed-off-by: Eng Zer Jun <engzerjun@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

crypto.x509.parse_keypair(cert, key) takes PEM/DER (base64) encoded input. if key pair is valid, returns the tls.certificate(https://pkg.go.dev/crypto/tls#Certificate) as an object. if key pair is invalid, returns an error. Fixes #5853 Signed-off-by: Emil Volckmar Ry <emilvry@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

This patch removes ordered block storage in fixed-sized block freelists in the OPA WASM memory allocator. Variable-sized block allocation still orders blocks so that free() can coalesce them back into larger sized blocks. This greatly reduces the runtime of opa_free() for fixed-size blocks as it turns it from an O(N) operation to an O(1) operation. This comes at the cost that reducing the heap_ptr implicitly on opa_free() becomes impractical since reduction will stop at the first fixed-sied block regardless of whether it is allocated or not. In practice, what this means is that the allocator can never combine fixed-size and variable-sized blocks. However, it was rarely able to do so previously: only when the two blocks happened to be free at the same time and line up with the heap_ptr. This patch also adds support for a new function called opa_free_bulk() that enables releasing memory objects always in O(1) time per object and O(N log N) worst case for releasing N objects. The patch works by freeing variable-sized objects (which would normally take O(N) time per free) to a temporary holding list and setting a flag indicating that the next variable-sized allocation needs to merge said holding list. When releasing the holding list, the memory allocator first merge-sorts in address-order the released blocks and then merges and coalesces them into the variable-sized block list in address order. This takes at most O(max(M+N, N log N)) time where M is the number of blocks on the variable freelist and N is the number of blocks bulk freed. The patch also updates the __opa_value_free() function to take a new parameter named 'bulk' which directs the function passes to its various type-specific subroutines. Every time one of the type-sepcific subroutines goes to free an object it invokes either opa_free() or opa_free_bulk() depending upon the 'bulk' parameter. (This is abstracted by a function __opa_free_maybe_bulk() in value.c) Calls to opa_value_free() or opa_value_free_shallow(), will set the the 'bulk' parameter to false preserving the existing behavior. However, the opa_value_add_path() and opa_value_remove_path() functions will invoke the function with 'bulk' set to true to ensure that the cascaded free operations on objects each take only O(1) time. Finally, the patch re-enables the RESTAuthzAllow100Paths benchmark. Fixes: #5901 Signed-off-by: Chris Telfer <chris.telfer@sophos.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

This extension to the loader package allows experimenting with data formats that are not JSON, but pretend to be. Signed-off-by: Stephan Renatus <stephan@styra.com> Co-authored-by: kevinstyra <83973046+kevinstyra@users.noreply.github.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

The handoff point for plugins right now is the IR: that, the store, and the open transaction is passed to `PrepareForEval`. Rego target plugins aren't plugged in through the plugin manager machinery, and they don't need to be enabled via configs. They are more akin to custom builtins, which also become available right away after having been registered with the ast package. In the future, another option would be to pass the Wasm bytes: we could then have a wasmtime-based plugin, and a wazero-based one. * rego.EvalContext: expose a few fields * ()*Rego).Compile() is only used by our wasm tests, and doesn't even make sense for the "rego" target. Not added to the plugin interface. * In rego.New, we now only gather plugins once, and have the plugins decide if a target is for them or not. This untangles the plugin name from the target matching; a plugin can have any name it wants now. Signed-off-by: Stephan Renatus <stephan@styra.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Signed-off-by: guoguangwu <guoguangwu@magic-shield.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Fixes: #5947 Signed-off-by: Johan Fylling <johan.dev@fylling.se> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

renatosc · 2023-05-30T04:58:59Z

I forgot to sign-off on last commit and thus it was failing the DCO task. I tried to follow the steps to fix it via rebase but I think something got messed up. I will abandon this PR and recreate everything from scratch.

anderseknert · 2023-05-30T07:18:19Z

Alright. Looking forward to a new PR @renatosc 👍

anderseknert previously approved these changes May 25, 2023

View reviewed changes

johanfylling requested changes May 25, 2023

View reviewed changes

renatosc dismissed anderseknert’s stale review via 022157c May 30, 2023 04:19

Renato Cordeiro and others added 14 commits May 29, 2023 23:56

test: Add --schema flag to opa test

4d3841b

Fixes: #5923 Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Add PITS Global Data Recovery Services to the list of adopters

80723b0

Signed-off-by: pheianox <77569421+pheianox@users.noreply.github.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

perf: avoid unnecessary byte/string conversion

0372467

We can use alternative functions/methods to avoid unnecessary byte/string conversion calls. Signed-off-by: Eng Zer Jun <engzerjun@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

build(deps): bump oras.land/oras-go/v2 from 2.1.0 to 2.2.0 (#5952)

af83c2c

Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

fix: CVE-2022-3517 (#5941)

c3e5dce

Signed-off-by: guoguangwu <guoguangwu@magic-shield.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Aborting query-eval if the compiler has errors (#5949)

8a0507c

Fixes: #5947 Signed-off-by: Johan Fylling <johan.dev@fylling.se> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Prepare v0.53.0 release

a7d33ca

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

Prepare v0.54.0 development

928a2d6

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com> Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

adding annotation/metadata reading for opa test

51e23d0

Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>

renatosc changed the title ~~test: Add schema flag to test~~ test: Add schema flag to test (abandoned) May 30, 2023

renatosc closed this May 30, 2023

renatosc mentioned this pull request May 30, 2023

Add schema flag to test #5957

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Add schema flag to test (abandoned) #5951

test: Add schema flag to test (abandoned) #5951

renatosc commented May 25, 2023 •

edited

anderseknert left a comment •

edited

johanfylling left a comment

johanfylling May 25, 2023

johanfylling May 25, 2023

renatosc May 25, 2023

johanfylling May 25, 2023

renatosc May 25, 2023

johanfylling May 25, 2023

johanfylling commented May 25, 2023

renatosc commented May 30, 2023 •

edited

anderseknert commented May 30, 2023

test: Add schema flag to test (abandoned) #5951

test: Add schema flag to test (abandoned) #5951

Conversation

renatosc commented May 25, 2023 • edited

Why the changes in this PR are needed?

What are the changes in this PR?

Notes to assist PR review:

anderseknert left a comment • edited

Choose a reason for hiding this comment

johanfylling left a comment

Choose a reason for hiding this comment

johanfylling May 25, 2023

Choose a reason for hiding this comment

johanfylling May 25, 2023

Choose a reason for hiding this comment

renatosc May 25, 2023

Choose a reason for hiding this comment

johanfylling May 25, 2023

Choose a reason for hiding this comment

renatosc May 25, 2023

Choose a reason for hiding this comment

johanfylling May 25, 2023

Choose a reason for hiding this comment

johanfylling commented May 25, 2023

renatosc commented May 30, 2023 • edited

anderseknert commented May 30, 2023

renatosc commented May 25, 2023 •

edited

anderseknert left a comment •

edited

renatosc commented May 30, 2023 •

edited