Entrypoints Changes in 0.34

[christophwille]

See christophwille/dotnet-opa-wasm#21

I debugged into it and the change seems to be that entrypoints now adds apostrophes which it didn't before:
{"'example'":0,"'example/one'":1}

So, question: intentional change (I presume yes), but about the calling convention for an SDK: do you expect people passing an entry point to include the apostrophes (part of the entrypoint name) or without?

[srenatus]

😓 TBH this is a surprise for me, too. I'll have a look. Thanks for raising this.

[christophwille]

Also, as I pointed out in my repo's issue, all unit tests are broken for https://github.com/christophwille/dotnet-opa-wasm/blob/master/src/Opa.Wasm.UnitTests/MultiEntryPointTests.cs which is suprising as they are using the exact same rego as the npm-opa-wasm multi-entry point test sample app. 0.34 compiles a different wasm than 0.33, because entrypoints are no longer there (line 20), and basically all my outputs are [].

[srenatus]

So from playing with npm-opa-wasm's entrypoint tests, nothing seems to have changed. The plain JSON string returned from here is

jsonAsText: '{"example/one":1,"example/one/myCompositeRule":2,"example":0}'

This is tested using opa from main, which is not significantly different from 0.34.0.

Could this be related to how the test data modules are built?

[srenatus]

Could you try playing with the ' and " here? https://github.com/christophwille/dotnet-opa-wasm/blob/6320fd21bd0ea0dedebd604efff5f3b9fdd51f6c/sample-policies/build-on-windows.bat#L15 Maybe it'll work fine without the single quotes. I'm not knowledgable enough about the quoting in batch files.

[christophwille]

Hunh, interesting. Great catch, I wouldn't have searched in the batch file because it worked fine with 0.33. Yes, replacing ' with " makes the wasm files work again. I copied the cmd line from the JS sample, and it definitely worked until 0.34. So some cmd line parsing in the Windows OPA exec most likely changed.

[srenatus]

So some cmd line parsing in the Windows OPA exec most likely changed.

Yeah that would also be a bit of a surprise. 🤔

But anyhow, the curious thing is that your single-quotes entrypoint strings make it that far and haven't been caught earlier. This makes me suspect that there's some golang-string-to-ast-string (or vice-versa) conversion happening that shouldn't happen. x/y is entrypoint for data.x.y, not 'x/y'.

[christophwille]

Well, one more thing could be at play here - I think I previously always used cmd.exe, but last month I reinstalled my machine from zero, and now the default is Windows Terminal with PS Core. So maybe .bat in Terminal is the thing that broke the ' handling.

[srenatus]

That sounds like a good lead. Would you be able to verify that somehow?

[christophwille]

I reverted to the old bat with ' and ran in cmd.exe. No, this was not the reason, same unit test failures as when I ran that .bat in PS.

[srenatus]

Have you ensured that the multi.wasm is properly rebuilt and copied into the right place? If you want me to have a look and double check, please share it.

[christophwille]

I double-checked in terms of Clean + directory delete

[srenatus]

So I've tried to see what happens when an entrypoint is provided with extra single quotes: it will not work 😄 So the entrypoint will get recorded in the wasm file, and it's usable, but it won't miraculously compile and call the right data functions, but yield an immediate undefined. (I've changed the npm-opa-wasm module's multi-ep tests to pass "'example/one'" instead of "example/one", and a dump of the wasm file shows me that the entrypoint is present, but there's not a single data function for the data.example.one.* rules.)

[srenatus]

@christophwille can you point me to the multi.wasm you've produced there? I'd be happy to have a look.

[christophwille]

multi-epapo.zip

This one is built using

.\opa_windows_amd64 build -t wasm -e 'example' -e 'example/one' example-one.rego
tar -xzf bundle.tar.gz /policy.wasm
copy policy.wasm multi-epapo.wasm
del policy.wasm

[srenatus]

That wasm file was basically no content, it's an example of the case I've described above: the entrypoint doesn't match anything -- there's no data["'example'"] or data["'example/one'"] in the rego -- and thus there's no data functions in the wasm file.

#3953 (comment)

Could you go with " for now? I'm not sure what we can do here, there hasn't been a change to either the windows binary or the wasm entrypoint handling between 0.33.0 and 0.34.0 🤔

[christophwille]

Yes, I am going with " because it is consistent with the other cmd lines in that batch file anyways.

Given that it doesn't match anything - don't you think this warrants an error? (User adding -e invalidentrypoint to the cmd line) Is there a case for not doing anything silently?

[srenatus]

Sorry, I was imprecise: it doesn't match any rules -- we don't know the data, though. You should be able to query data.foo.bar via entrypoint foo/bar, I think.

[tsandall]

I tend to agree that if opa build silently builds when -e refers to a missing/undefined rule that it will be confusing... I'm also willing to assume that most people will refer to entrypoints that are defined by rules... given this we could (i) update the compile package to check entrypoints refer to rules and (ii) provide an option to disable this check. Although this is backwards incompatible, I'm guessing few people would be relying on -e to refer to data.

[srenatus]

Yeah that sounds like a good compromise! I'll create a new issue later today.

[srenatus]

Let's track this in #3957. Thanks for reporting this! 😃