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

ast: Location Text attribute wrong for multivalue rules with generated body #7128

Closed
anderseknert opened this issue Oct 21, 2024 · 1 comment · Fixed by #7129
Closed

ast: Location Text attribute wrong for multivalue rules with generated body #7128

anderseknert opened this issue Oct 21, 2024 · 1 comment · Fixed by #7129
Labels

Comments

@anderseknert
Copy link
Member

anderseknert commented Oct 21, 2024

package system.log
	
import rego.v1
	
mask contains "foo"

Parsing this policy and inspecting the Text attribute of the mask rule's Location, one would expect to see mask contains "foo", as any other type of rule would work. Multivalue rules with generated bodies however don't get the full Text, but only the name of the rule, like mask in this case.

This leads to some issues in Regal wrt these rules, as reported by @drewcorlin1

(since the contains keyword is not part of the AST, Regal has to scan the Text attribute in order to know if it's used)

anderseknert added a commit to anderseknert/opa that referenced this issue Oct 21, 2024
Since there is no body, the location of the head is a better option
than simply using the fist scanned token for location.

Fixes open-policy-agent#7128

Signed-off-by: Anders Eknert <anders@styra.com>
anderseknert added a commit that referenced this issue Oct 21, 2024
Since there is no body, the location of the head is a better option
than simply using the fist scanned token for location.

Fixes #7128

Signed-off-by: Anders Eknert <anders@styra.com>
@drewcorlin1
Copy link

thanks for the quick fix as usual @anderseknert !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants