Skip to content

Commit

Permalink
Merge branch 'main' into jf/5685/multi-var-rule-refs
Browse files Browse the repository at this point in the history
  • Loading branch information
johanfylling authored Aug 1, 2023
2 parents 2073c3d + 160d8a8 commit 29c2a2e
Show file tree
Hide file tree
Showing 180 changed files with 27,479 additions and 2,413 deletions.
2 changes: 1 addition & 1 deletion .go-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.20.5
1.20.6
112 changes: 112 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,118 @@ project adheres to [Semantic Versioning](http://semver.org/).

## Unreleased

### Breaking changes

Since its introduction in 0.34.0, the `--exit-zero-on-skipped` option always made the `opa test` command return an exit code 0. When used, it now returns the exit code 0 only if no failed tests were found.

Test runs on existing projects using `--exit-zero-on-skipped` will fail if any failed tests were inhibited by this behavior.

### Tooling, SDK, and Runtime

- `opa test`: Fix `--exit-zero-on-skipped` behavior to make test runs fail with failing test rules ([#6126](https://github.com/open-policy-agent/opa/issues/6126)) reported and authored by @fdaguin

## 0.55.0

> **_NOTES:_**
>
> * All published OPA images now run with a non-root uid/gid. The `uid:gid` is set to `1000:1000` for all images. As a result
> there is no longer a need for the `-rootless` image variant and hence it will be not be published as part of future releases.
> This change is in line with container security best practices. OPA can still be run with root privileges by explicitly setting the user,
> either with the `--user` argument for `docker run`, or by specifying the `securityContext` in the Kubernetes Pod specification.
>
> * The minimum version of Go required to build the OPA module is **1.19**
This release contains a mix of new features, bugfixes and a new builtin function.

### Honor `default` keyword on functions

Previously if a function was defined with a `default` value, OPA would ignore it. Now the `default` function is honored
if all functions with the same name are undefined. For example,

```rego
package example
default clamp_positive(x) := 0
clamp_positive(x) = x {
x > 0
}
```

```
$ opa eval -d example.rego 'data.example.clamp_positive(1)' -f pretty
1
```

```
$ opa eval -d example.rego 'data.example.clamp_positive(-1)' -f pretty
0
```

The value of a `default` function follows the same conditions as that of a `default` rule. In addition, a `default`
function satisfies the following properties:

- same arity as other functions with the same name
- arguments should only be plain variables ie. no composite values
- argument names should not be repeated

> **_NOTE:_**
>
> `default` functions used to be previously ignored. If existing policies contain `default` functions, ensure that they conform
> to the properties mentioned above. Otherwise, those policies will fail to evaluate.
Authored by @ashutosh-narkar.

### New Built-In Function: crypto.parse_private_keys

`crypto.parse_private_keys` returns zero or more private keys from the given encoded string containing DER certificate data.
If the input contains a list of one or more concatenated PEM blocks, then the built-in will output the parsed private keys
represented as objects.

See [the documentation on the new built-in](https://www.openpolicyagent.org/docs/v0.55.0/policy-reference/#builtin-crypto-cryptoparse_private_keys)
for all the details.

Authored by @volck.

### Runtime, Tooling, SDK

- plugins/rest: Add AWS KMS support for OAuth2 Client Credentials JWT authentication ([#5942](https://github.com/open-policy-agent/opa/pull/5942)) authored by @prasanthu
- sdk: Update input object to conform to the format expected by decision log masking ([#6090](https://github.com/open-policy-agent/opa/pull/6090)) authored by @epaulson10
- sdk: Add option for specifying decision ID to SDK. Users can use this to control the ID that gets included in the decision logs ([#6101](https://github.com/open-policy-agent/opa/pull/6101)) authored by @brianchhun-chime
- cmd: Add `discard` output format to `opa eval` which discards the result while still showing the output of eval flags like `--profile` ([#6103](https://github.com/open-policy-agent/opa/pull/6103)) authored by @26tanishabanik
- Make rootless deprecation messages more explicit as all published OPA images now run with non-root uid/gid ([#6091](https://github.com/open-policy-agent/opa/pull/6091)) authored by @charlieegan3
- download/oci: Add support for Docker Registry v2 authentication scheme ([#6045](https://github.com/open-policy-agent/opa/pull/6045)) authored by @gitu and @DerGut
- plugins/discovery: Ensure discovery plugin doesn't erase its own config on the plugin manager ([#6070](https://github.com/open-policy-agent/opa/pull/6070)) authored by @blacksails

### Topdown and Rego

- ast: Add `WithRoots` compiler option that allows callers to set the roots to include in the output bundle manifest ([#6088](https://github.com/open-policy-agent/opa/pull/6088)) authored by @kubaj
- rego: Parse store modules iff modules set on the Rego object. This change assumes that while using the Rego package, the compiler and store are kept in-sync, and thereby attempts to avoid a race during the compilation process ([#6081](https://github.com/open-policy-agent/opa/pull/6081)) authored by @ashutosh-narkar

### Docs

- docs/envoy: Update the standalone Envoy tutorial to use [kind](https://kind.sigs.k8s.io/), updated Envoy version etc. ([#6105](https://github.com/open-policy-agent/opa/pull/6105)) authored by @charlieegan3

### Website + Ecosystem

- Ecosystem:
- Carbonetes BrainIAC ([#6073](https://github.com/open-policy-agent/opa/pull/6073)) authored by @jaysonsantos05

- Website:
- Reorganize relevant doc sections and OPA Ecosystem projects to have a closer integration between them ([#6064](https://github.com/open-policy-agent/opa/issues/6064)) authored by @charlieegan3

### Miscellaneous
- chore: Update comments on some exported functions and clean up instances where the same package was imported multiple times (authored by @testwill)
- Fix issue in the OPA release patch scripts related to `CRLF` line terminations in the patch output ([#6069](https://github.com/open-policy-agent/opa/pull/6069)) authored by @johanfylling
- Dependency bumps, notably:
- golang from 1.20.5 to 1.20.6
- oras.land/oras-go/v2 from 2.2.0 to 2.2.1
- google.golang.org/grpc from 1.56.1 to 1.56.2
- github.com/containerd/containerd from 1.6.19 to 1.7.2
- golang.org/x/net from 0.11.0 to 0.12.0
- go.uber.org/automaxprocs from 1.5.2 to 1.5.3
- go.opentelemetry.io/otel from v1.14.0 to v1.16.0 ([#6062](https://github.com/open-policy-agent/opa/pull/6062)) authored by @srenatus with feedback from @ghaskins and @zregvart

## 0.54.0

This release focuses on bug fixes, but also includes some improvements to the SDK and commandline.
Expand Down
14 changes: 14 additions & 0 deletions ast/builtins.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ var DefaultBuiltins = [...]*Builtin{
CryptoX509ParseCertificateRequest,
CryptoX509ParseRSAPrivateKey,
CryptoX509ParseKeyPair,
CryptoParsePrivateKeys,
CryptoHmacMd5,
CryptoHmacSha1,
CryptoHmacSha256,
Expand Down Expand Up @@ -2312,6 +2313,19 @@ var CryptoX509ParseRSAPrivateKey = &Builtin{
),
}

var CryptoParsePrivateKeys = &Builtin{
Name: "crypto.parse_private_keys",
Description: `Returns zero or more private keys from the given encoded string containing DER certificate data.
If the input is empty, the function will return null. The input string should be a list of one or more concatenated PEM blocks. The whole input of concatenated PEM blocks can optionally be Base64 encoded.`,
Decl: types.NewFunction(
types.Args(
types.Named("keys", types.S).Description("PEM encoded data containing one or more private keys as concatenated blocks. Optionally Base64 encoded."),
),
types.Named("output", types.NewArray(nil, types.NewObject(nil, types.NewDynamicProperty(types.S, types.A)))).Description("parsed private keys represented as objects"),
),
}

var CryptoMd5 = &Builtin{
Name: "crypto.md5",
Description: "Returns a string representing the input string hashed with the MD5 function",
Expand Down
2 changes: 1 addition & 1 deletion ast/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -2289,7 +2289,7 @@ func (c *Compiler) rewriteLocalVars() {
// Report an error for each unused function argument
for arg := range unusedArgs {
if !arg.IsWildcard() {
c.err(NewError(CompileErr, rule.Head.Location, "unused argument %v", arg))
c.err(NewError(CompileErr, rule.Head.Location, "unused argument %v. (hint: use _ (wildcard variable) instead)", arg))
}
}
}
Expand Down
69 changes: 58 additions & 11 deletions ast/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1904,6 +1904,41 @@ p[r] := 2 { r := "foo" }`,
assertCompilerErrorStrings(t, c, expected)
}

func TestCompilerCheckRuleConflictsDefaultFunction(t *testing.T) {
tests := []struct {
note string
modules []*Module
err string
}{
{
note: "conflicting rules",
modules: modules(
`package pkg
default f(_) = 100
f(x, y) = x {
x == y
}`),
err: "rego_type_error: conflicting rules data.pkg.f found",
},
}
for _, tc := range tests {
t.Run(tc.note, func(t *testing.T) {
mods := make(map[string]*Module, len(tc.modules))
for i, m := range tc.modules {
mods[fmt.Sprint(i)] = m
}
c := NewCompiler()
c.Modules = mods
compileStages(c, c.checkRuleConflicts)
if tc.err != "" {
assertCompilerErrorStrings(t, c, []string{tc.err})
} else {
assertCompilerErrorStrings(t, c, []string{})
}
})
}
}

func TestCompilerCheckRuleConflictsDotsInRuleHeads(t *testing.T) {
t.Setenv("OPA_ENABLE_GENERAL_RULE_REFS", "true")

Expand Down Expand Up @@ -5434,7 +5469,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y)"), "", 2, 4),
Message: "unused argument y",
Message: "unused argument y. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand All @@ -5448,7 +5483,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("a.b.c.func(x, y)"), "", 2, 4),
Message: "unused argument y",
Message: "unused argument y. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand All @@ -5463,12 +5498,12 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y)"), "", 2, 4),
Message: "unused argument x",
Message: "unused argument x. (hint: use _ (wildcard variable) instead)",
},
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y)"), "", 2, 4),
Message: "unused argument y",
Message: "unused argument y. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand All @@ -5483,7 +5518,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y)"), "", 2, 4),
Message: "unused argument y",
Message: "unused argument y. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand All @@ -5506,7 +5541,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, _)"), "", 2, 4),
Message: "unused argument x",
Message: "unused argument x. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand Down Expand Up @@ -5549,7 +5584,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x) := { x: v | x := \"foo\"; v := a[x] }"), "", 3, 4),
Message: "unused argument x",
Message: "unused argument x. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand Down Expand Up @@ -5627,7 +5662,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y, z)"), "", 2, 4),
Message: "unused argument x",
Message: "unused argument x. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand All @@ -5645,7 +5680,7 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y, z)"), "", 2, 4),
Message: "unused argument y",
Message: "unused argument y. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand All @@ -5663,7 +5698,19 @@ func TestCheckUnusedFunctionArgVars(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y, z)"), "", 2, 4),
Message: "unused argument z",
Message: "unused argument z. (hint: use _ (wildcard variable) instead)",
},
},
},
{
note: "unused default function argvar",
module: `package test
default func(x) := 0`,
expectedErrors: Errors{
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x) := 0"), "", 2, 12),
Message: "unused argument x. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand Down Expand Up @@ -5695,7 +5742,7 @@ func TestCompileUnusedAssignedVarsErrorLocations(t *testing.T) {
&Error{
Code: CompileErr,
Location: NewLocation([]byte("func(x, y)"), "", 2, 4),
Message: "unused argument y",
Message: "unused argument y. (hint: use _ (wildcard variable) instead)",
},
},
},
Expand Down
38 changes: 38 additions & 0 deletions ast/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,12 @@ func (p *Parser) parseRules() []*Rule {
return nil
}

if len(rule.Head.Args) > 0 {
if !p.validateDefaultRuleArgs(&rule) {
return nil
}
}

rule.Body = NewBody(NewExpr(BooleanTerm(true).SetLocation(rule.Location)).SetLocation(rule.Location))
return []*Rule{&rule}
}
Expand Down Expand Up @@ -2179,6 +2185,38 @@ func (p *Parser) validateDefaultRuleValue(rule *Rule) bool {
return valid
}

func (p *Parser) validateDefaultRuleArgs(rule *Rule) bool {

valid := true
vars := NewVarSet()

vis := NewGenericVisitor(func(x interface{}) bool {
switch x := x.(type) {
case Var:
if vars.Contains(x) {
p.error(rule.Loc(), fmt.Sprintf("illegal default rule (arguments cannot be repeated %v)", x))
valid = false
return true
}
vars.Add(x)

case *Term:
switch v := x.Value.(type) {
case Var: // do nothing
default:
p.error(rule.Loc(), fmt.Sprintf("illegal default rule (arguments cannot contain %v)", TypeName(v)))
valid = false
return true
}
}

return false
})

vis.Walk(rule.Head.Args)
return valid
}

// We explicitly use yaml unmarshalling, to accommodate for the '_' in 'related_resources',
// which isn't handled properly by json for some reason.
type rawAnnotation struct {
Expand Down
8 changes: 8 additions & 0 deletions ast/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1611,6 +1611,14 @@ func TestRule(t *testing.T) {
assertParseErrorContains(t, "default invalid rule head builtin call", `default a = upper("foo")`, "illegal default rule (value cannot contain call)")
assertParseErrorContains(t, "default invalid rule head call", `default a = b`, "illegal default rule (value cannot contain var)")

assertParseErrorContains(t, "default invalid function head ref", `default f(x) = b.c.d`, "illegal default rule (value cannot contain ref)")
assertParseErrorContains(t, "default invalid function head call", `default f(x) = g(x)`, "illegal default rule (value cannot contain call)")
assertParseErrorContains(t, "default invalid function head builtin call", `default f(x) = upper("foo")`, "illegal default rule (value cannot contain call)")
assertParseErrorContains(t, "default invalid function head call", `default f(x) = b`, "illegal default rule (value cannot contain var)")
assertParseErrorContains(t, "default invalid function composite argument", `default f([x]) = 1`, "illegal default rule (arguments cannot contain array)")
assertParseErrorContains(t, "default invalid function number argument", `default f(1) = 1`, "illegal default rule (arguments cannot contain number)")
assertParseErrorContains(t, "default invalid function repeated vars", `default f(x, x) = 1`, "illegal default rule (arguments cannot be repeated x)")

assertParseError(t, "extra braces", `{ a := 1 }`)
assertParseError(t, "invalid rule name hyphen", `a-b = x { x := 1 }`)

Expand Down
Loading

0 comments on commit 29c2a2e

Please sign in to comment.