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

Dotted packages pass build in WASM but don't work #4304

Closed
willhausman opened this issue Jan 29, 2022 · 11 comments
Closed

Dotted packages pass build in WASM but don't work #4304

willhausman opened this issue Jan 29, 2022 · 11 comments

Comments

@willhausman
Copy link

Short description

Using this sample, https://github.com/open-policy-agent/npm-opa-wasm/tree/main/examples/nodejs-app, with OPA 0.35 installed on my machine.

I changed the package from example to example.dotted, and could no longer retrieve a result from evaluation.

Steps To Reproduce

package example.dotted

default hello = false

hello {
    x := input.message
    x == data.world
}

I ran these commands for compiling and running the wasm:

opa build -t wasm -e 'example.dotted' ./example.rego
tar -xzf ./bundle.tar.gz /policy.wasm
node app.js '{"message": "world"}' 

The output is an empty array. [].

Expected behavior

I expect dotted package names to work in wasms the same as a server, or at least some documentation saying it isn't supported.
https://play.openpolicyagent.org/p/3gdkg5nd8q

Additional context

I found my way to this use case because I was making a wasm to run in C# with Wasmtime and wanted to provide a builtin to call some C# code. I never got a result out of the wasm, and noticed that my call to the exported builtins wasm function did not return any mapped functions for me to connect with. I eventually removed the dot from my package name simply because I was desperate to match samples as closely as possible, and then my builtins started showing up.

@srenatus
Copy link
Contributor

Excuse my brevity, but please try example/dotted as entrypoint. That's the format and we should probably call that out somewhere better.

@willhausman
Copy link
Author

Excuse my brevity, but please try example/dotted as entrypoint. That's the format and we should probably call that out somewhere better.

Ahh, that makes sense for how paths happen from a REST endpoint as well. Thanks!

@srenatus
Copy link
Contributor

Your issue is further evidence that #3957 is a thing we should do.

@anderseknert
Copy link
Member

anderseknert commented Jan 30, 2022

This has bitten me too in the past, even if not in the context of Wasm. Failing on non-existing paths would be good, but I wonder if we can't just accept both forms whenever a path like this is expected. Postel's Law and all that :) Meaning that if there is an example/dotted path/entrypoint (or path in the SDK, or whatever). These would all be accepted (possibly with a info/debug message about the preferred form):

example/dotted
example.dotted
data.example.dotted

@srenatus
Copy link
Contributor

That's a good idea. We'll only need to figure out what to do when there's both

package foo.example.dotted

and

package foo["example.dotted"]

@anderseknert
Copy link
Member

You mean both of those packages would exist inside of the same OPA? I'd honestly be fine with keeping the current behavor for that edge case... it's like they'd want problems, so let them have them 😆

@stale
Copy link

stale bot commented Mar 2, 2022

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

@stale stale bot added the inactive label Mar 2, 2022
@ashutosh-narkar
Copy link
Member

Should we update the docs and the opa build instructions with the entrypoint format to close out this issue ? cc @srenatus

@srenatus
Copy link
Contributor

srenatus commented Mar 9, 2022

@ashutosh-narkar Yeah. I would prefer @anderseknert's "let's just do the right thing when we can figure out what it is" approach, but documentation is the low-hanging fruit here.

@stale
Copy link

stale bot commented Apr 8, 2022

This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.

@stale stale bot added the inactive label Apr 8, 2022
@srenatus
Copy link
Contributor

#3957

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

4 participants