Skip to content

Commit

Permalink
parent cb01fba
Browse files Browse the repository at this point in the history
author Itay Shakury <itay@itaysk.com> 1646501263 +0200
committer Damien Burks <damien@damienjburks.com> 1646794189 -0600

docs: add missing title to annotations index (#4406)

Signed-off-by: Itay Shakury <itay@itaysk.com>

Update philosophy.md (#4412)

I am suggesting this change to fix grammatical errors which will increase readability for consumers of the OPA Philosophy document.

---

1. I noticed there is a missing word ("be") in the OPA Document Model section, just above the table that summarizes the different models for loading base documents into OPA. This change adds the omitted word.

2. I've also made a change to the first note at the bottom of the page ([1]). "However" is used as a conjunctive adverb, I've updated the sentence's punctuation to reflect this.

3. Lastly, I've made a punctuation change to note number three at the bottom of the page ([3])  to increase readability.

Signed-off-by: Arthur Jones <arthur.h.jones3@gmail.com>

Co-authored-by: Stephan Renatus <stephan.renatus@gmail.com>

docs: mention who created OPA and who owns it (#4414)

These are two common questions that come up and we do not mention this
anywhere else in the docs.

Signed-off-by: Torin Sandall <torinsandall@gmail.com>

committing latest changes

Signed-off-by: Damien Burks <damien@damienjburks.com>

committing latest changes to unsued imports function

Signed-off-by: Damien Burks <damien@damienjburks.com>

removing print statements and adding more test cases

Signed-off-by: Damien Burks <damien@damienjburks.com>

server: exposing disableInlining option via Compile API (#4378)

This change enables consumers of Compile API to specify packages
those should not be inlined in partial evaluation response.

Fixes: #4357

Signed-off-by: skosunda <skosunda@adobe.com>

docs: Update Envoy primer to use variables instead of objects

Previously, the Envoy primer provided samples of policies for
status-code, body, and headers that explicitly returned an object
in individual rules instead of the more common and recommended
approach of using separate variables for each concern.

This change replaces those sample policies with ones that use
variables for each of allow, status_code, headers, body,
and it adds a section on the output expected by Envoy,
where we include the snippet combining those variables into
the JSON object expected by Envoy.

Signed-off-by: Tim Hinrichs <tim@styra.com>

make: Disable WASM by default on darwin/arm64 (#4382)

Signed-off-by: Anders Eknert <anders@eknert.com>

docs: Update Istio tutorial to expose application to outside traffic (#4384)

Signed-off-by: Ashutosh Narkar <anarkar4387@gmail.com>

fixing linting issues

Signed-off-by: Damien Burks <damien@damienjburks.com>

committing latest changes for PR

Signed-off-by: Damien Burks <damien@damienjburks.com>

adding info to strict.md

adding switch statement for 'in' operator

adding additional case to if statement

finalizing changes for broken test cases

Signed-off-by: Damien Burks <damien@damienjburks.com>
  • Loading branch information
itaysk authored and damienjburks committed Mar 9, 2022
1 parent 1164094 commit f0154d8
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 0 deletions.
58 changes: 58 additions & 0 deletions ast/compile.go
Expand Up @@ -259,6 +259,7 @@ func NewCompiler() *Compiler {
f func()
}{
{"CheckDuplicateImports", "compile_stage_check_duplicate_imports", c.checkDuplicateImports},
{"CheckUnusedImports", "compile_stage_check_unused_imports", c.checkUnusedImports},
{"CheckKeywordOverrides", "compile_stage_check_keyword_overrides", c.checkKeywordOverrides},
// Reference resolution should run first as it may be used to lazily
// load additional modules. If any stages run before resolution, they
Expand Down Expand Up @@ -1321,6 +1322,62 @@ func (c *Compiler) GetAnnotationSet() *AnnotationSet {
return c.annotationSet
}

func (c *Compiler) checkUnusedImports() {
if !c.strict {
return
}

for _, name := range c.sorted {
mod := c.Modules[name]

for _, imp := range mod.Imports {
var impCounter int
importName := imp.Name().String()

WalkExprs(mod, func(expr *Expr) bool {
switch t := expr.Terms.(type) {
case *Every:
_ = t // term is unused
if strings.Contains(expr.String(), importName) {
impCounter++
}
default:
WalkTerms(mod, func(term *Term) bool {
termStr := term.Value.String()
if importName == "in" {
switch {
case termStr == "internal.member_2":
if importName == "in" {
impCounter++
}
case termStr == "internal.member_3":
if importName == "in" {
impCounter++
}
default:
if strings.Contains(expr.String(), termStr) {
impCounter++
}
}
} else {
if importName == termStr {
impCounter++
}
}
return false
})
}

return true
})

if impCounter == 0 {
c.err(NewError(CompileErr, imp.Location, "%v unused", imp.String()))
}
}
}
}

func (c *Compiler) checkDuplicateImports() {
if !c.strict {
return
Expand All @@ -1332,6 +1389,7 @@ func (c *Compiler) checkDuplicateImports() {

for _, imp := range mod.Imports {
name := imp.Name()

if processed, conflict := processedImports[name]; conflict {
c.err(NewError(CompileErr, imp.Location, "import must not shadow %v", processed))
} else {
Expand Down
141 changes: 141 additions & 0 deletions ast/compile_test.go
Expand Up @@ -1500,6 +1500,147 @@ func TestCompilerRewriteExprTerms(t *testing.T) {
}
}

func TestCompilerCheckUnusedImports(t *testing.T) {
cases := []strictnessTestCase{
{
note: "fail_case_1",
module: `package p
import data.foo.bar as bar
r {
input.bar == 11
}
`,
expectedErrors: Errors{
&Error{
Location: NewLocation([]byte("import"), "", 2, 4),
Message: "import data.foo.bar as bar unused",
},
},
},
{
note: "fail_case_2",
module: `package p
import data.foo # unused
r { data.foo == 10 }
`,
expectedErrors: Errors{
&Error{
Location: NewLocation([]byte("import"), "", 2, 4),
Message: "import data.foo unused",
},
},
},
{
note: "fail_case_3",
module: `package p
import data.foo
import data.x.power #unused
r { foo == 10 }
`,
expectedErrors: Errors{
&Error{
Location: NewLocation([]byte("import"), "", 3, 4),
Message: "import data.x.power unused",
},
},
},
{
note: "fail_case_4",
module: `package p
import data.foo
import data.x.power #unused
r { input.unused == 10 }
`,
expectedErrors: Errors{
&Error{
Location: NewLocation([]byte("import"), "", 2, 4),
Message: "import data.foo unused",
},
&Error{
Location: NewLocation([]byte("import"), "", 3, 4),
Message: "import data.x.power unused",
},
},
},
{
note: "pass_case_1",
module: `package p
import data.foo.x
r { x == 10 }
`,
},
{
note: "pass_case_2",
module: `package p
import data.foo.x
r { input.data == x }
`,
},
{
note: "pass_case_3",
module: `package p
import data.foo.x
import data.power.ranger
r { ranger == x }
`,
},
{
note: "pass_case_4",
module: `package p
import data.foo.x
import data.power.ranger
r { ranger == 23 }
t { x == 1 }
`,
},
{
note: "pass_case_5",
module: `package p
import data.foo
r = count(foo) > 1 # only one operand
`,
},
{
note: "pass_case_6",
module: `package p
import data.foo
r = sprintf("%v %d", [foo, 0]) # more complicated operands
`,
},
{
note: "pass_case_7",
module: `package p
import data.foo
r {
foo # not a call, plain term
}
`,
},
{
note: "pass_case_8",
module: `package p
import future.keywords.every
import data.foo
r {
every x in foo { x > 1 } # import in 'every' domain
}
`,
},
{
note: "pass_case_9",
module: `package p
import future.keywords.every
import data.foo
r {
every x in [1,2,3] { x > foo } # import in 'every' body
}
`,
},
}

runStrictnessTestCase(t, cases, true)
}

func TestCompilerCheckDuplicateImports(t *testing.T) {
cases := []strictnessTestCase{
{
Expand Down
1 change: 1 addition & 0 deletions ast/fuzz.go
@@ -1,3 +1,4 @@
//go:build gofuzz
// +build gofuzz

package ast
Expand Down
1 change: 1 addition & 0 deletions docs/content/strict.md
Expand Up @@ -20,5 +20,6 @@ Name | Description | Enforced by default in OPA version
--- | --- | ---
Duplicate imports | Duplicate [imports](../policy-language/#imports), where one import shadows another, are prohibited. | 1.0
Unused local assignments | Unused [assignments](../policy-reference/#assignment-and-equality) local to a rule, function or comprehension are prohibited | 1.0
Unused imports | Unused [imports](../policy-language/#imports) are prohibited. | 1.0
`input` and `data` reserved keywords | `input` and `data` are reserved keywords, and may not be used as names for rules and variable assignment. | 1.0
`any()` and `all()` removed | The `any()` and `all()` built-in functions have been deprecated, and will be removed in OPA 1.0. Use of these functions is prohibited. | 1.0

0 comments on commit f0154d8

Please sign in to comment.