Renaming --rego-v1 cmd flag to --v0-v1
#7225
Conversation
Affects `opa fmt` and `opa check`. `opa fmt` command is updated to expect incoming rego to be v0. Signed-off-by: Johan Fylling <[email protected]>
| @@ -153,8 +153,12 @@ func addStrictFlag(fs *pflag.FlagSet, strict *bool, value bool) { | |||
| fs.BoolVarP(strict, "strict", "S", value, "enable compiler strict mode") | |||
| } | |||
|
|
|||
| func addRegoV1FlagWithDescription(fs *pflag.FlagSet, regoV1 *bool, value bool, description string) { | |||
| func addRegoV0V1FlagWithDescription(fs *pflag.FlagSet, regoV1 *bool, value bool, description string) { | |||
| fs.BoolVar(regoV1, "v0-v1", value, description) | |||
There was a problem hiding this comment.
Not sure if --v0-v1 is the best name, but I can't come up with anything better.
Could call it --v0-v1-compatible to be more similar to --v0-compatible; but that command covers a slightly different use-case, so could be more confusing than helpful.
Suggestions are welcome :).
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -138,6 +138,10 @@ func formatFile(params *fmtCommandParams, out io.Writer, filename string, info o | |||
| DropV0Imports: params.dropV0Imports, | |||
| } | |||
|
|
|||
| if params.regoV1 { | |||
| opts.ParserOptions = &ast.ParserOptions{RegoVersion: ast.RegoV0} | |||
There was a problem hiding this comment.
Since we expect incoming policies to be v0, we'll error if the relevant imports aren't present; which isn't ideal.
If we drop this, then we'll default to v1, and the user can override to with --v0-compatible (now they can actually override to v1 with the hidden --v1-compatible flag).
Alternatively, we could try parsing with one version, and if it fails, parse with the other; but that's hard to make fool-proof.
There was a problem hiding this comment.
To clarify the current behaviour:
opa fmt:- v1 rego is formatted to v1
rego.v1/future.keywordsimports are NOT removedrego.v1/future.keywordsimports are NOT added if missing- v0 rego is rejected
opa fmt --v0-compatible:- v0 rego is formatted to v0
- v1 rego is rejected
opa fmt --v0-v1:- v0 rego is formatted to be compatible with v0 AND v1
- v1 rego is rejected
opa fmt --v0-v1 --v1-compatible:- v1 rego is formatted to be compatible with v0 AND v1
- v0 rego is rejected
--v1-compatibleflag is hidden inopa fmt --help
A mix of v0/v1 modules is NOT supported.
In the above, a v0 module can have rego.v1 and future.keywords imports and be v1 compliant.
Signed-off-by: Johan Fylling <[email protected]>
| p contains x if { | ||
| some x in ["a", "b", "c"] | ||
| } | ||
| `, |
There was a problem hiding this comment.
This test case illustrates a scenario that can be solved with setting the --v1-compatible flag. This flag is however hidden, so not ideal.
converting v0->v1 is likely the more common use case, but we can't guarantee this won't happen; and will probably become more likely as time pass ..
| } | ||
|
|
||
| for _, tc := range tests { | ||
| t.Run(tc.note, func(t *testing.T) { | ||
| params := fmtCommandParams{ | ||
| // Locking rego-version to v0, as it's only then the --rego-v1 flag is relevant | ||
| v0Compatible: true, |
There was a problem hiding this comment.
This is why the tests didn't fail on main when we moved to v1 as default.
Signed-off-by: Johan Fylling <[email protected]>
Follows open-policy-agent#7225 Signed-off-by: Charlie Egan <[email protected]>
Follows open-policy-agent#7225 Signed-off-by: Charlie Egan <[email protected]>
Follows open-policy-agent#7225 Signed-off-by: Charlie Egan <[email protected]>
Follows open-policy-agent#7225 Signed-off-by: Charlie Egan <[email protected]>
Follows open-policy-agent#7225 Signed-off-by: Charlie Egan <[email protected]>
Affects
opa fmtandopa check.opa fmtcommand is updated to expect incoming rego to be v0.