From bcf82eb40572d6b62c97e4fbe4077e0153148210 Mon Sep 17 00:00:00 2001 From: Johan Fylling Date: Wed, 15 Nov 2023 12:16:19 +0100 Subject: [PATCH] ast: Adding capability feature for the `rego.v1` import (#6375) Resolves: #6366 Signed-off-by: Johan Fylling --- ast/capabilities.go | 11 +++++++++++ ast/capabilities_test.go | 7 +++++++ ast/compile.go | 21 ++++++++++++--------- ast/parser.go | 5 +++++ ast/version_index.json | 7 +++++++ capabilities.json | 3 ++- capabilities/v0.59.0.json | 3 ++- cmd/check_test.go | 29 +++++++++++++++++++++++++++++ docs/content/deployments.md | 6 ++++-- docs/content/policy-language.md | 2 +- 10 files changed, 80 insertions(+), 14 deletions(-) diff --git a/ast/capabilities.go b/ast/capabilities.go index 4c9b7a1650..3b95d79e57 100644 --- a/ast/capabilities.go +++ b/ast/capabilities.go @@ -52,6 +52,7 @@ var minVersionIndex = func() VersionIndex { // heads, they wouldn't be able to parse them. const FeatureRefHeadStringPrefixes = "rule_head_ref_string_prefixes" const FeatureRefHeads = "rule_head_refs" +const FeatureRegoV1Import = "rego_v1_import" // Capabilities defines a structure containing data that describes the capabilities // or features supported by a particular version of OPA. @@ -104,6 +105,7 @@ func CapabilitiesForThisVersion() *Capabilities { f.Features = []string{ FeatureRefHeadStringPrefixes, FeatureRefHeads, + FeatureRegoV1Import, } return f @@ -205,6 +207,15 @@ func (c *Capabilities) MinimumCompatibleVersion() (string, bool) { return maxVersion.String(), true } +func (c *Capabilities) ContainsFeature(feature string) bool { + for _, f := range c.Features { + if f == feature { + return true + } + } + return false +} + // addBuiltinSorted inserts a built-in into c in sorted order. An existing built-in with the same name // will be overwritten. func (c *Capabilities) addBuiltinSorted(bi *Builtin) { diff --git a/ast/capabilities_test.go b/ast/capabilities_test.go index 78e3b2aeb1..84f2cbea48 100644 --- a/ast/capabilities_test.go +++ b/ast/capabilities_test.go @@ -208,6 +208,13 @@ func TestCapabilitiesMinimumCompatibleVersion(t *testing.T) { `, version: "0.59.0", }, + { + note: "rego.v1 import", + module: ` + package x + import rego.v1`, + version: "0.59.0", + }, } for _, tc := range tests { diff --git a/ast/compile.go b/ast/compile.go index d81c91e1fd..683a448e73 100644 --- a/ast/compile.go +++ b/ast/compile.go @@ -940,21 +940,25 @@ func (c *Compiler) buildComprehensionIndices() { // checker. func (c *Compiler) buildRequiredCapabilities() { + features := map[string]struct{}{} + // extract required keywords from modules keywords := map[string]struct{}{} futureKeywordsPrefix := Ref{FutureRootDocument, StringTerm("keywords")} for _, name := range c.sorted { for _, imp := range c.imports[name] { path := imp.Path.Value.(Ref) - if !path.HasPrefix(futureKeywordsPrefix) { - continue - } - if len(path) == 2 { - for kw := range futureKeywords { - keywords[kw] = struct{}{} + switch { + case path.Equal(regoV1CompatibleRef): + features[FeatureRegoV1Import] = struct{}{} + case path.HasPrefix(futureKeywordsPrefix): + if len(path) == 2 { + for kw := range futureKeywords { + keywords[kw] = struct{}{} + } + } else { + keywords[string(path[2].Value.(String))] = struct{}{} } - } else { - keywords[string(path[2].Value.(String))] = struct{}{} } } } @@ -962,7 +966,6 @@ func (c *Compiler) buildRequiredCapabilities() { c.Required.FutureKeywords = stringMapToSortedSlice(keywords) // extract required features from modules - features := map[string]struct{}{} for _, name := range c.sorted { for _, rule := range c.Modules[name].Rules { diff --git a/ast/parser.go b/ast/parser.go index e9389c51c7..09a9b9583a 100644 --- a/ast/parser.go +++ b/ast/parser.go @@ -2561,6 +2561,11 @@ func (p *Parser) futureImport(imp *Import, allowedFutureKeywords map[string]toke } func (p *Parser) regoV1Import(imp *Import) { + if !p.po.Capabilities.ContainsFeature(FeatureRegoV1Import) { + p.errorf(imp.Path.Location, "invalid import, `%s` is not supported by current capabilities", regoV1CompatibleRef) + return + } + path := imp.Path.Value.(Ref) if len(path) == 1 || !path[1].Equal(regoV1CompatibleRef[1]) || len(path) > 2 { diff --git a/ast/version_index.json b/ast/version_index.json index 642769ba34..d1dfeb3252 100644 --- a/ast/version_index.json +++ b/ast/version_index.json @@ -1367,6 +1367,13 @@ } }, "features": { + "rego_v1_import": { + "Major": 0, + "Minor": 59, + "Patch": 0, + "PreRelease": "", + "Metadata": "" + }, "rule_head_ref_string_prefixes": { "Major": 0, "Minor": 46, diff --git a/capabilities.json b/capabilities.json index c035ecdf56..268fbc34f8 100644 --- a/capabilities.json +++ b/capabilities.json @@ -4706,6 +4706,7 @@ ], "features": [ "rule_head_ref_string_prefixes", - "rule_head_refs" + "rule_head_refs", + "rego_v1_import" ] } diff --git a/capabilities/v0.59.0.json b/capabilities/v0.59.0.json index 8556050f31..0b0b7a790b 100644 --- a/capabilities/v0.59.0.json +++ b/capabilities/v0.59.0.json @@ -4696,6 +4696,7 @@ ], "features": [ "rule_head_ref_string_prefixes", - "rule_head_refs" + "rule_head_refs", + "rego_v1_import" ] } diff --git a/cmd/check_test.go b/cmd/check_test.go index 105128c32b..7d2b3a7a98 100644 --- a/cmd/check_test.go +++ b/cmd/check_test.go @@ -78,6 +78,35 @@ import future.keywords.if import future.keywords.in p if "opa" in input.tools`, }, + { + note: "rego.v1 imported but NOT defined in capabilities", + caps: func() string { + c := ast.CapabilitiesForThisVersion() + c.Features = []string{} + j, err := json.Marshal(c) + if err != nil { + panic(err) + } + return string(j) + }(), + policy: `package test +import rego.v1`, + err: "rego_parse_error: invalid import, `rego.v1` is not supported by current capabilities", + }, + { + note: "rego.v1 imported AND defined in capabilities", + caps: func() string { + c := ast.CapabilitiesForThisVersion() + c.Features = []string{ast.FeatureRegoV1Import} + j, err := json.Marshal(c) + if err != nil { + panic(err) + } + return string(j) + }(), + policy: `package test +import rego.v1`, + }, } // add same tests for bundle-mode == true: diff --git a/docs/content/deployments.md b/docs/content/deployments.md index aadebeee88..941b7264ee 100644 --- a/docs/content/deployments.md +++ b/docs/content/deployments.md @@ -457,7 +457,8 @@ Some features of OPA can be toggled on and off through the `features` list: { "features": [ "rule_head_ref_string_prefixes", - "rule_head_refs" + "rule_head_refs", + "rego_v1_import" ] } ``` @@ -465,7 +466,8 @@ Some features of OPA can be toggled on and off through the `features` list: Features present in the list are enabled, while features not present are disabled. The following features are available: * `rule_head_ref_string_prefixes`: Enables the use of a [reference in place of name](../policy-language/#rule-heads-containing-references) in the head of rules. This is a subset of `rule_head_refs`, and only covers references where all terms are primitive types, or where only the last element of the ref (the key in the generated object or set) is allowed to be a variable. -* `rule_head_refs`: Enables general support for [references in rule heads](../policy-language/#rule-heads-containing-references), including [variables at arbitrary locations](../policy-language/#variables-in-rule-head-references). This feature also covers the functionality of `rule_head_ref_string_prefixes`. +* `rule_head_refs`: Enables general support for [references in rule heads](../policy-language/#rule-heads-containing-references), including [variables at arbitrary locations](../policy-language/#variables-in-rule-head-references). This feature also covers the functionality of `rule_head_ref_string_prefixes`. +* `rego_v1_import`: enables use of the `rego.v1` import. ### Future keywords diff --git a/docs/content/policy-language.md b/docs/content/policy-language.md index 3a96211b07..307fa92ab9 100644 --- a/docs/content/policy-language.md +++ b/docs/content/policy-language.md @@ -3808,7 +3808,7 @@ Unused imports | Unused [imports](../policy-language/#imports) are prohibited. Use of deprecated built-ins | Use of deprecated functions is prohibited, and these will be removed in OPA 1.0. Deprecated built-in functions: `any`, `all`, `re_match`, `net.cidr_overlap`, `set_diff`, `cast_array`, `cast_set`, `cast_string`, `cast_boolean`, `cast_null`, `cast_object` | 1.0 {{< info >}} -If the `rego.v1` import is present in a module, all strict mode checks documented above expect the unused local assignment and unused imports checks are enforced on the module. +If the `rego.v1` import is present in a module, all strict mode checks documented above except the unused local assignment and unused imports checks are enforced on the module. Additionally the `rego.v1` import also requires the usage of `if` and `contains` keywords when declaring certain rules. The `if` keyword is required before a rule body and the `contains` keyword is required for partial set rules. {{< /info >}}