opa
opa copied to clipboard
Allow multiple imports on a single line
It would be nice enhancement if there was support for importing multiple rules, or future keywords on a single line. As it currently stands, explicitly importing all the future keywords — which we recommend over the "catch all" import — plus any rules or functions required from other packages, is a bit verbose, and tends to distract from what's important when demonstrating policy authoring. I.e. rather than having to do something like:
package policy
import future.keywords.every
import future.keywords.contains
import future.keywords.if
import data.other.policy.foo
import data.other.policy.bar as b
deny contains "not on my watch!" if {
every x in foo {
x == b
}
}
One would be able to do something like:
package policy
import future.keywords [every, contains, if]
import data.other.policy [foo, bar as b]
deny contains "not on my watch!" if {
every x in foo {
x == b
}
}
Open to suggestions on the syntax — whatever looks best on a single line is fine with me :)
The obvious alternative... but I don't know python so this might or might not be similar enough.
from future.keywords import [every, contains, if]
Yeah, that'd be nice... although it would require the use of a new from
keyword... and it's not clear how that could be imported
from future.keywords import [from]
😆
There's also Haskell's import syntax. They allow qualified, unqualified, and renamed imports, as well as the ability to import all names except a selection from the other module, which is interesting.
Yeah.. another request that's come up a few times is to forbid imports that don't "resolve" to something, like... I guess something close to how PHP differentiates between "import" and "require". Or to be able to exclude something from the stdlib or a keyword, in order to use that for your own custom functions or rules. Both could be useful, but let's keep the scope to multiple imports for this issue 😄
This is not much of a problem in "production" policies, but it would help a lot with providing concise, copy-pasteable snippets, like in the Rego Style Guide, or as shown in presentations and demos.
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
This is quite an interesting one, I might play around with this if its okay and see if I can make something cool :smile:
I'd love to see it @Parsifal-M 😃
Is there consensus on how to do it? I presume we'll go with
import future.keywords [every, contains, if]
import data.other.policy [foo, bar as b]
❓
Is there consensus on how to do it? I presume we'll go with
import future.keywords [every, contains, if] import data.other.policy [foo, bar as b]
question
This is what I had in mind to try out :+1: , LMK if you think otherwise.
It's fine. I just wanted everyone to be on the same page.
I think an interesting question here is capabilities: How can we introduce this change so that the user can take the capabilities file of some OPA version before this feature was introduced, and have it fail in a sensible way?
(Usually, you'll have a myriad of OPA versions running in your installation, and you'd take the oldest running version's capabilities file when bundling policies for your fleet.)
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
This issue has been automatically marked as inactive because it has not had any activity in the last 30 days.
Is anyone working on this issue?
I started understanding parser file under ast folder and token and scaner files from internal, but kind of wondering where to get started with so that the new form of import import future.keywords [every, contains, if]
can be parsed
That's awesome @26tanishabanik 😃 I'd imagine the tokenizer would need to be made aware of the new format, and that the parser would "rewrite" the short form syntax into the longer form. I'd need to look into it, but I'm currently on leave :) So perhaps @srenatus or @johanfylling could help provide some pointers.
One thing that I hadn't really considered is the compatibility problem. Any policy using short form imports would be incompatible with previous version of OPA. I can't really think of any way around that 🤔 I guess we could have tooling that rewrote the short form to it's longer one when e.g. building bundles, or whatnot... but it would obviously not cover all cases. Perhaps we could sneak in support but not make it official / documented until X releases later, as we'd then have at least X number of past versions that supported it 😅
Not sure if we're OK with biting that bullet. Personally, I'm in favor, but I'd appreciate hearing what others think about that.
That's awesome @26tanishabanik 😃 I'd imagine the tokenizer would need to be made aware of the new format, and that the parser would "rewrite" the short form syntax into the longer form. I'd need to look into it, but I'm currently on leave :) So perhaps @srenatus or @johanfylling could help provide some pointers.
One thing that I hadn't really considered is the compatibility problem. Any policy using short form imports would be incompatible with previous version of OPA. I can't really think of any way around that 🤔 I guess we could have tooling that rewrote the short form to it's longer one when e.g. building bundles, or whatnot... but it would obviously not cover all cases. Perhaps we could sneak in support but not make it official / documented until X releases later, as we'd then have at least X number of past versions that supported it 😅
Not sure if we're OK with biting that bullet. Personally, I'm in favor, but I'd appreciate hearing what others think about that.
Thanks a lot @anderseknert for your inputs. I will surely wait for @srenatus and @johanfylling to give their inputs too. This is my first time working with parser so would need some pointers 😬
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.
I'm going to close this one for now, seeing as the future keywords are soon to be made included by default. There's still a case to be made for this and regular imports, I think, but it's not as urgent, and would still require new syntax. If anyone else thinks this is important, let me know.