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

OPA 1.0 docs #6455

Merged

Conversation

johanfylling
Copy link
Contributor

No description provided.

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

netlify bot commented Dec 5, 2023

Deploy Preview for openpolicyagent ready!

Name Link
🔨 Latest commit 9be6d1b
🔍 Latest deploy log https://app.netlify.com/sites/openpolicyagent/deploys/6572dc349664a3000870df69
😎 Deploy Preview https://deploy-preview-6455--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.

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@@ -0,0 +1,201 @@
---
title: OPA 1.0
kind: misc
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be inclined to file this under a more prominent section, even the core docs for the next few releases. We're going to promote this, but it'd be nice to have it be eye catching in the sidebar too.

weight: 4
---

OPA v1.0 will introduce breaking changes to the Rego language.
Copy link
Contributor

Choose a reason for hiding this comment

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

There are also server changes such as Binding the OPA server to the localhost interface by default.

I know of at least one issue to the OPA server labelled as v1: #2137 - there might be others too. Does it make sense to talk more generally about breaking changes with this in mind?

OPA’s Go SDK and Go API will be equipped with similar functionality, as well.
In addition to this, the bundle manifest will also be updated to include a bit that indicates whether pre-OPA v1.0 behavior is desired.
OPA tooling such as the `opa build` command will be equipped with a new flag that will allow users to control the relevant bit in the manifest.
This should be useful for cases when pre-OPA v1.0 functionality is desired but the users themselves have no control over the OPA deployment and hence cannot set the CLI flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this applies to

In addition to this, the bundle manifest will also be updated to include a bit that indicates whether pre-OPA v1.0 behavior is desired.

rather than

OPA tooling such as the opa build command will be equipped with a new flag that will allow users to control the relevant bit in the manifest.

| p if { true } | {"p": true} | {"p": true} |
| p.a if { true } | {"p":{"a": true}} | {"p":{"a": true}} |
| p.a.b if { true } | {"p": {"a": {"b": true}} | {"p": {"a": {"b": true}} |
| p contains “a” | {"p": {"a"}} | {"p": {"a"}} |
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be helpful to provide a column for the best practice form from v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Best practice would be anything in the output in v1.0 column that isn't a compile error.
Should we add a table to the end where we highlight what's the 1.0-compatible version of deprecated forms?

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, on review, I think this table is great as it is.

If the Rego language was changed so that all rules were single-value by default, unless the `contains` keyword was used to make them multi-value, then the outcome of a rule like `p.a { true }` would change between today and v1.0 without generating an error.
Generating errors in this case is preferable to changing the semantics of existing rules. Whereby, use of the `if` keyword will be a requirement in OPA v1.0, as this is also backwards compatible with older versions of OPA.

In OPA v1.0, the `if` keyword is only required for rules with a declared body. Constants, rules that only consist of a value assignment, do not require `if`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can make this clearer by saying, "so all of the following forms are unchanged" or similar.

Other option is to remove the output in v1.0 col since it's the same?


import rego.v1 # Implies future.keywords.if and future.keywords.contains

a contains b if { b := 1}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
a contains b if { b := 1}
a contains b if { b := 1 }

anderseknert
anderseknert previously approved these changes Dec 6, 2023
Copy link
Member

@anderseknert anderseknert left a comment

Choose a reason for hiding this comment

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

Yay!


1. A new `rego.v1` import has been introduced that, when used, makes OPA apply all restrictions that will eventually be enforced by default in OPA v1.0.
If a Rego module imports `rego.v1`, it means applicable `future.keywords` imports are implied. It is illegal to import both `rego.v1` and `future.keywords` in the same module.
2. The `--rego-v1` flag on the `opa fmt` command will rewrite existing modules to use the `rego.v1` import instead of `future.keywords` imports.
Copy link
Member

Choose a reason for hiding this comment

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

Should we mention somewhere when these flags were introduced? I.e. v0.59.0 (or at least when this became stable)

The Rego compiler supports [strict mode](../policy-language/#strict-mode), which enforces additional constraints and safety checks during compilation.
One of these checks is to ensure that `input` and `data` are reserved keywords and may not be used as names for rules and variable assignments.

The `input` document holds the user-provided input, while the data pushed into OPA and rule evaluation results are nested under the `data` document.
Copy link
Member

Choose a reason for hiding this comment

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

In some of the repos where I've helped teams enforce strict mode, they have not understood that with input as { .. } doesn't constitute shadowing. Perhaps it would be good to say something to that end here.

@johanfylling johanfylling marked this pull request as ready for review December 6, 2023 11:34
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Fixes: open-policy-agent#6453
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@johanfylling johanfylling linked an issue Dec 6, 2023 that may be closed by this pull request
requested by @anderseknert

Signed-off-by: Johan Fylling <johan.dev@fylling.se>
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
default allow := false

# 'if' is part of default v1.0 syntax, and doesn't need import.
# 'if' is required before role body.
Copy link
Member

Choose a reason for hiding this comment

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

role -> rule

anderseknert
anderseknert previously approved these changes Dec 8, 2023
Signed-off-by: Johan Fylling <johan.dev@fylling.se>
@johanfylling johanfylling merged commit 4eb95c5 into open-policy-agent:main Dec 8, 2023
24 checks passed
@johanfylling johanfylling deleted the rego-v1/docs_description branch December 8, 2023 09:18
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.

Add docs on import rego.v1
3 participants