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

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

Closed
johanfylling opened this issue Dec 1, 2023 · 5 comments · Fixed by #6698
Closed

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

johanfylling opened this issue Dec 1, 2023 · 5 comments · Fixed by #6698

Comments

@johanfylling
Copy link
Contributor

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.

Copy link

stale bot commented Jan 1, 2024

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 Jan 1, 2024
@johanfylling johanfylling added this to Backlog in Open Policy Agent via automation Mar 27, 2024
@johanfylling johanfylling moved this from Backlog to Planning - v0.64 in Open Policy Agent Mar 27, 2024
@stale stale bot removed the inactive label Apr 2, 2024
@johanfylling
Copy link
Contributor Author

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.

@xico42
Copy link
Contributor

xico42 commented Apr 9, 2024

@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
Copy link
Contributor

xico42 commented Apr 9, 2024

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.

@johanfylling
Copy link
Contributor Author

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

johanfylling added a commit to johanfylling/opa that referenced this issue Apr 15, 2024
Prioritizing generating v0 Rego with `rego.v1` import when producing support modules for non-`--v1-compatible` optimized builds.

Affects `opa build` when the `-O` flag is used for optimization, and `opa eval` for partial evaluation with the `-p` flag.

Fixes: open-policy-agent#6450
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
ashutosh-narkar pushed a commit to johanfylling/opa that referenced this issue Apr 22, 2024
Prioritizing generating v0 Rego with `rego.v1` import when producing support modules for non-`--v1-compatible` optimized builds.

Affects `opa build` when the `-O` flag is used for optimization, and `opa eval` for partial evaluation with the `-p` flag.

Fixes: open-policy-agent#6450
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
johanfylling added a commit that referenced this issue Apr 24, 2024
Prioritizing generating v0 Rego with `rego.v1` import when producing support modules for non-`--v1-compatible` optimized builds.

Affects `opa build` when the `-O` flag is used for optimization, and `opa eval` for partial evaluation with the `-p` flag.

Fixes: #6450
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Open Policy Agent automation moved this from Planning - v0.64 to Done Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

2 participants