Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --schema flag to opa test #5923

Closed
anderseknert opened this issue May 16, 2023 · 3 comments · Fixed by #5957
Closed

Add --schema flag to opa test #5923

anderseknert opened this issue May 16, 2023 · 3 comments · Fixed by #5957
Assignees

Comments

@anderseknert
Copy link
Member

Should work identically to opa eval, but having to run a single command for e.g. TDD makes for less friction.

@renatosc
Copy link
Contributor

I can take care of this

@renatosc
Copy link
Contributor

Hi @anderseknert

I added the --schema flag to opa test and tested again my "merge error" repo with success.

Before the fix:

opa test regos/policy2.rego tests/policy2_test.rego --schema schemas/policy2_schema.json 
Error: unknown flag: --schema

After the fix:

opa_darwin_amd64 test regos/policy2.rego tests/policy2_test.rego --schema schemas/policy2_schema.json 
1 error occurred: regos/policy2.rego:11: rego_type_error: undefined ref: input.dept
	input.dept
	      ^
	      have: "dept"
	      want (one of): ["department"]

I didn't submit the PR yet because I still need to check where to add the proper test case for that new flag.

Meanwhile, since it is my first PR, do you mind checking my fix commit so I can adjust it if needed before I submit the PR?

Also, a few extra things that I noticed:

  1. I still get the merge error if I try to run opa test . --schema schemas but I noticed that I also get the same merge error when trying to do opa eval data.policy1.allow --data . --schema schemas. As you pointed out, this is due to opa merging all files together. I was thinking here, maybe opa should automatically ignore the schema files/path for both eval and test? What do you think? I could work on this if you think it makes sense.

  2. I noticed that the opa test is not reading the Metadata annotation from the rego files. Can you confirm that? If you want to open a new issue for it I can add Annotation support for opa test so it can read the schema info from the metadata (which is important when passing a schema directory instead of a schema file to opa test).

Thanks

@anderseknert
Copy link
Member Author

Awesome! The commit looks fine to me, but it would be good if some tests were added just to assert this works as expected.

renatosc pushed a commit to renatosc/opa that referenced this issue May 30, 2023
…a reading

runner: added annotation/metadata reading for opa test

Fixes: open-policy-agent#5923
Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>
renatosc pushed a commit to renatosc/opa that referenced this issue May 30, 2023
…a reading

runner: added annotation/metadata reading for opa test

Fixes: open-policy-agent#5923
Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>
johanfylling pushed a commit that referenced this issue Jun 2, 2023
* test: Add --schema flag to opa test
* runner: added annotation/metadata reading for opa test

Fixes: #5923
Signed-off-by: Renato Cordeiro <renato@renatocordeiro.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants