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

General refs/docs #6244

Merged
merged 3 commits into from Sep 28, 2023
Merged

Conversation

johanfylling
Copy link
Contributor

@johanfylling johanfylling commented Sep 25, 2023

Fixes: #5996

Updating documentation with information about general refs in rule heads.

🚧 ⚠️ These changes are contingent on #6235 getting merged; and should not be merged before those changes.

Note: we use live code blocks to demonstrate general ref rule heads, and these will be broken until the Rego Playground is updated to use the latest OPA version.

To build the docs, and view the live code blocks, build the playground with main-branch OPA and run it locally; then point the docs to that instance by updating PLAYGROUND in docs/website/scripts/live-blocks/src/constants.js.

```

In the above example, rule `R2` overlaps with the dynamic portion of rule `R1`'s reference (`[x].r`), which is allowed at compile-time, as these rules aren't guaranteed to produce conflicting output.
However, if `R1` defines `x` as `"q"` and `y` as, e.g. `0`, a conflict will be reported at evaluation-time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're saying the above rule will report a conflict at eval time, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 👌 .

Copy link
Contributor Author

@johanfylling johanfylling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deployment fails because the playground doesn't support the new syntax yet.


```json
```live:general_ref_head:input
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 00 23


```rego
```live:general_ref_head:module
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 05 55


```live:general_ref_head:output
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 06 50


The first variable declared in a rule head's reference divides the reference in a leading constant portion and a trailing dynamic portion. Other rules are allowed to overlap with the dynamic portion (dynamic extent) without causing a compile-time conflict.

```live:general_ref_head_conflict:module
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 07 27

}
}
}
```live:general_ref_head_conflict:output:expect_eval_error
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 08 22


Error:

```live:general_ref_head_conflict2:output:expect_rego_error
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 09 53


Rules are not allowed to overlap with object values of other rules.

```live:general_ref_head_conflict3:module
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 10 57


Error:

```live:general_ref_head_conflict3:output:expect_eval_error
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 11 29

In the above example, `R1` is within the dynamic extent of `R2` and a conflict cannot be detected at compile-time. However, at evaluation-time `R2` will attempt to inject a value under key `t` in an object value defined by `R1`. This is a conflict, as rules are not allowed to modify or replace values defined by other rules.
We won't get a conflict if we update the policy to the following:

```live:general_ref_head_conflict4:module
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 12 02


As `R1` is now instead defining a value within the dynamic extent of `R2`'s reference, which is allowed:

```live:general_ref_head_conflict4:output
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-27 at 15 12 40

@netlify
Copy link

netlify bot commented Sep 27, 2023

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit ac1ac4d
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/65142fc37cb9fa00097222eb
😎 Deploy Preview https://deploy-preview-6244--openpolicyagent.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@ashutosh-narkar ashutosh-narkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Fixes: open-policy-agent#5996

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@johanfylling johanfylling merged commit c36a605 into open-policy-agent:main Sep 28, 2023
24 checks passed
@johanfylling johanfylling deleted the general_refs/docs branch September 28, 2023 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

general ref heads: documentation
2 participants