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

Testing shared functions between policies #6282

Open
ajith-sub opened this issue Oct 7, 2023 · 4 comments
Open

Testing shared functions between policies #6282

ajith-sub opened this issue Oct 7, 2023 · 4 comments

Comments

@ajith-sub
Copy link
Contributor

What is the underlying problem you're trying to solve?

I have a use case where I need to use a user-defined Rego function from another policy that is part of a policy library. However, I don't have access to the Rego files implementing the library policies themselves. This policy would be registered in a system that allows users to independently develop and register their own policies that can consume common library functions.

I would like to write and run tests for my own policy that mocks the library function using the with keyword and run opa test without having to include the actual library policy implementing the function and possibly other transitive dependencies the library policy requires.

Consider the following example:

lib.rego:

# The user doesn't have access to this module, but the functions defined in this module are documented and this module would be included in the final bundle.
package lib

foo(a) := a

policy.rego:

package policy

import data.lib
import future.keywords.if

allow if {
    "bar" == lib.foo("bar")
}

policy_test.rego:

package policy

import future.keywords.if

mock_foo(_) := "bar"

test_allow if {
    allow with data.lib.foo as mock_foo
}

Running $ opa test policy.rego policy_test.rego results in the following error: 1 error occurred: .\policy.rego:7: rego_type_error: undefined function data.lib.foo

Is there any way to to test policy.rego without having to specify lib.rego in the test command?

Describe the ideal solution

Ideally, testing a policy with a mocked function in Rego would not require the implementation of that function. This way, I could develop policies that consume documented library functions without having to access all the library Rego policies.

Describe a "Good Enough" solution

A switch for opa test to loosen the requirement for user-defined function implementations to be provided for testing if they have been mocked.

Additional Context

There have been some tangential discussions to this issue, particularly around function sharing between bundles:

@anderseknert
Copy link
Member

I'm curious to learn more about your use case, as I don't think this request ever came up before :)
When/why would someone writing a policy not have access to its dependencies?

I guess you could provide a lib skeleton, which would later be replaced when you got access to the real lib and could build the complete bundle?

package lib

foo(_) := "not implemented"
package policy

import future.keywords.if

mock_foo(_) := "bar"

test_allow if {
    allow with data.lib.foo as mock_foo
}

@ajith-sub
Copy link
Contributor Author

The use case is based on users independently developing and registering their policies in a central policy store. In this case, the dependencies are not available at the time policies are tested for coverage, etc. I think stubbing the functions might work, but still involves some boilerplate. Ideally, the functions would be testable without having the implementation especially if they have been mocked.

Also note that this is only an issue for functions, as references to rules in modules that are not specified as part of the test command don't result in an error message. My understanding from previous discussions is that the OPA type checker is more strict about functions, but I would think there's a benefit in loosening this for at least testing policies.

This also brings up a larger issue of dependency management between policies as well. I know the multiple bundle use case is not currently recommended, but I think a model for packaging and sharing policies would be beneficial and may prevent issues such as this as well.

@anderseknert
Copy link
Member

anderseknert commented Oct 11, 2023

Also note that this is only an issue for functions, as references to rules in modules that are not specified as part of the test command don't result in an error message.

Indeed — that's because they aren't references to rules, they are just references. What those actually point to is resolved at runtime. The "OPA document model" from the docs explains well how that works.

Functions however aren't documents, and they cannot be queries as data. That allows the compiler to test conditions like arity, types, etc. Going with what you suggest would entail having that safety/contract removed, and I don't think we'll want to do that, as it would mean errors that are now surfaced at compile time would turn into runtime failures.

The only exception I can think of where it perhaps could be possible, is what the docs describe as "simple cases", where a function is entirely mocked out by a value, i.e.:

test_allow if {
    allow with data.lib.foo as {"foo": "bar"}
}

But... the allow rule you're testing would still need to reference the function as a real one:

allow if {
    foo_data := data.lib.foo("foo", "bar")
    foo_data.foo == "bar"
}

How do you know that the real foo function takes two args and not three? There'd be no contract in place to ensure that. Providing a lib skeleton to your implementors, as previously mentioned, and to use that as a form of "contract" is probably the best thing you can get to given the contstraints you describe... but the better approach would be to try and fix the constraints :)

Copy link

stale bot commented Nov 10, 2023

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days. Although currently inactive, the issue could still be considered and actively worked on in the future. More details about the use-case this issue attempts to address, the value provided by completing it or possible solutions to resolve it would help to prioritize the issue.

@stale stale bot added the inactive label Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants