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.

Sign up for GitHub

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

Jump to bottom

variables in closure of every block are sometimes not recognized #6979

Closed
bluebrown opened this issue Aug 30, 2024 · 1 comment · Fixed by #6980
Closed

variables in closure of every block are sometimes not recognized #6979

bluebrown opened this issue Aug 30, 2024 · 1 comment · Fixed by #6980
Assignees
@srenatus
Labels
bug

Comments

@bluebrown
Copy link

Short description

Sometimes when using the every construct, variables from the outer scope are not recognized.

OPA version = 0.67.0

Steps To Reproduce

Here is one reproduction https://github.com/bluebrown/opa-every-with-issue
See https://openpolicyagent.slack.com/archives/CBR63TK2A/p1725004042414319 for more context and examples
@bluebrown bluebrown added the bug label Aug 30, 2024
@srenatus
Copy link
Contributor 

package test

p {
	x := "foo"
	true with input[x] as 1
}

⬇️

1 error occurred: policy.rego:4: rego_compile_error: assigned var x unused

PG

@srenatus srenatus self-assigned this Aug 30, 2024
srenatus added a commit to srenatus/opa that referenced this issue Aug 30, 2024
@srenatus
ast/compile: also replace declared vars in with's target
eb18297 
It might be uncommon to do this, but it's not wrong to expect this to work.

Fixes open-policy-agent#6979.

Signed-off-by: Stephan Renatus <[email protected]>
@srenatus srenatus mentioned this issue Aug 30, 2024
Merged
srenatus added a commit that referenced this issue Sep 2, 2024
@srenatus
ast/compile: also replace declared vars in with's target (#6980)
caa62a3 
It might be uncommon to do this, but it's not wrong to expect this to work.
One peculiarity is that we've consciously made an exception for
```
input := "whatever"
allow with input as true
```
where the with-target "input" was meant not to be replaced although there's
a local variable shadowing "input". This change has become a bit trickier
because this behaviour is kept intact. Note that it's only possible in non-strict
mode (Rego v0).

Fixes #6979

Small refactoring included:
* ast/compile: remove With from recursive term rewriting

   I could find no term where this actually mattered, so I've simplified the code
   a little bit.

Signed-off-by: Stephan Renatus <[email protected]>
@BrewTestBot BrewTestBot mentioned this issue Sep 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@srenatus srenatus

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ast/compile: also replace declared vars in with's target srenatus/opa
2 participants
@srenatus @bluebrown