opa icon indicating copy to clipboard operation
opa copied to clipboard

Optimized bundle build doesn't respect `rego.v1` and `future.keywords` imports

Open johanfylling opened this issue 2 years ago • 1 comments

When building a bundle from the following module with opa build -O1:

package test

import rego.v1

# METADATA
# entrypoint: true
p if {
    input.x == 1
}

the resulting optimized support module will drop the rego.v1 import and if keyword:

package test

p {
    input.x = 1
}

The same issue occurs when the original module imports future.keywords instead of rego.v1.

johanfylling avatar Dec 01 '23 10:12 johanfylling

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[bot] avatar Jan 01 '24 20:01 stale[bot]

The build command respects the --v1-compatible flag, and opa build -O1 --v1-compatible will output:

package test

p if {
	input.x = 1
}

Given this fact, I don't think it's crucial to solve this issue.

johanfylling avatar Apr 05 '24 08:04 johanfylling

@johanfylling regarding this issue, I've come to realize that the SDK during bundle plugin activation does not respect the V1 compatibility flag provided.

This is because the bundle.ActivateOpts struct is instantiated without forwarding the plugin manager parser options provided during SDK initialization.

So, if a build with the v1 flag is performed, the SDK might not load it because of the missing import rego.v1 .

xico42 avatar Apr 09 '24 19:04 xico42

I am not totally sure that the problem I faced is really related to this issue. But anyway, I have provided a failing test case with the fix at #6689.

Also, it seems to only happen when there are multiple bundles being loaded. Also, by taking a look at the logs it seems that OPA is mixing up the bundles together, because the syntax error logged is for a source code of a bundle other than the one the status is logged for.

xico42 avatar Apr 09 '24 21:04 xico42

Your find in #6689 is unrelated to this issue. Nevertheless, great find and fix! 👍 😃

johanfylling avatar Apr 11 '24 09:04 johanfylling