New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
topdown: Add built-in HMAC functions #4100
Merged
johanfylling
merged 1 commit into
open-policy-agent:main
from
johanfylling:feature/1740/hmac_builtin
Dec 9, 2021
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
--- | ||
title: Adding Built-in Functions | ||
kind: contrib | ||
weight: 5 | ||
--- | ||
|
||
[Built-in Functions](../policy-reference/#built-in-functions) | ||
can be added inside the `topdown` package. | ||
|
||
Built-in functions may be upstreamed if they are generally useful and provide functionality that would be | ||
impractical to implement natively in Rego (e.g., CIDR arithmetic). Implementations should avoid thirdparty | ||
dependencies. If absolutely necessary, consider importing the code manually into the `internal` package. | ||
|
||
{{< info >}} | ||
Read more about extending OPA with custom built-in functions in go [here](../extensions#custom-built-in-functions-in-go). | ||
{{< /info >}} | ||
|
||
Adding a new built-in function involves the following steps: | ||
|
||
1. [Declare and register](#declare-and-register) the function | ||
2. [Implementation](#implement) the function | ||
3. [Test](#test) the function | ||
4. [Document](#document) the function | ||
|
||
## Example | ||
|
||
The following example adds a simple built-in function, `repeat(string, int)`, that returns a given string repeated a given number of times. | ||
|
||
### Declare and Register | ||
|
||
In `ast/builtins.go`, we declare the structure of our built-in function with a `Builtin` struct instance: | ||
|
||
```go | ||
// Repeat returns, as a string, the given string repeated the given number of times. | ||
var Repeat = &Builtin{ | ||
Name: "repeat", // The name of the function | ||
Decl: types.NewFunction( | ||
types.Args( // The built-in takes two arguments, where .. | ||
types.S, // .. the first is a string, and .. | ||
types.N, // .. the second is a number. | ||
), | ||
types.S, // The return type is a string. | ||
), | ||
} | ||
``` | ||
|
||
To register the new built-in function, we locate the `DefaultBuiltins` array in `ast/builtins.go`, and add the `Builtin` instance to it: | ||
|
||
```go | ||
var DefaultBuiltins = [...]*Builtin{ | ||
... | ||
Repeat, | ||
... | ||
} | ||
``` | ||
|
||
### Implement | ||
|
||
In the `topdown` package, we locate a suitable source file for our new built-in function, or add a new file, as appropriate. | ||
|
||
In this example, we introduce a new source file, `topdown/repeat.go`: | ||
|
||
```go | ||
package topdown | ||
|
||
import ( | ||
"fmt" | ||
"strings" | ||
|
||
"github.com/open-policy-agent/opa/ast" | ||
"github.com/open-policy-agent/opa/topdown/builtins" | ||
) | ||
|
||
// implements topdown.BuiltinFunc | ||
func builtinRepeat(_ BuiltinContext, operands []*ast.Term, iter func(*ast.Term) error) error { | ||
// Get the first argument as a string, returning an error if it's not the correct type. | ||
str, err := builtins.StringOperand(operands[0].Value, 1) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Get the first argument as an int, returning an error if it's not the correct type or not a positive value. | ||
count, err := builtins.IntOperand(operands[1].Value, 2) | ||
if err != nil { | ||
return err | ||
} else if count < 0 { | ||
// Defensive check, strings.Repeat(...) will panic for count<0 | ||
return fmt.Errorf("count must be a positive integer") | ||
} | ||
|
||
// Return a string by invoking the given iterator function | ||
return iter(ast.StringTerm(strings.Repeat(string(str), count))) | ||
} | ||
|
||
func init() { | ||
RegisterBuiltinFunc(ast.Repeat.Name, builtinRepeat) | ||
} | ||
``` | ||
|
||
In the above code, `builtinRepeat` implements the `topdown.BuiltinFunc` function type. | ||
The call to `RegisterBuiltinFunc(...)` in `init()` adds the built-in function to the evaluation engine; binding the implementation to `ast.Repeat` that was registered in [an earlier step](#register-the-function). | ||
|
||
### Test | ||
|
||
All built-in function implementations must include a test suite. | ||
Test cases for built-in functions are written in YAML and located under `test/cases/testdata`. | ||
|
||
We create two new test cases (one positive, expecting a string output; and one negative, expecting an error) for our built-in function: | ||
|
||
```yaml | ||
cases: | ||
- note: repeat/positive | ||
query: data.test.p = x | ||
modules: | ||
- | | ||
package test | ||
|
||
p = repeated { | ||
repeated := repeat(input.str, input.count) | ||
} | ||
input: {"str": "Foo", "count": 3} | ||
want_result: | ||
- x: FooFooFoo | ||
- note: repeat/negative | ||
query: data.test.p = x | ||
modules: | ||
- | | ||
package test | ||
|
||
p = repeated { | ||
repeated := repeat(input.str, input.count) | ||
} | ||
input: { "str": "Foo", "count": -3 } | ||
strict_error: true | ||
want_error_code: eval_builtin_error | ||
want_error: 'repeat: count must be a positive integer' | ||
``` | ||
|
||
The above test cases can be run separate from all other tests through: `go test ./topdown -v -run 'TestRego/repeat'` | ||
|
||
See [test/cases/testdata/helloworld](https://github.com/open-policy-agent/opa/blob/main/test/cases/testdata/helloworld) | ||
for a more detailed example of how to implement tests for your built-in functions. | ||
|
||
> Note: We can manually test our new built-in function by [building](../contrib-development#getting-started) | ||
> and running the `eval` command. E.g.: `$./opa_<OS>_<ARCH> eval 'repeat("Foo", 3)'` | ||
|
||
### Document | ||
|
||
All built-in functions must be documented in `docs/content/policy-reference.md` under an appropriate subsection. | ||
|
||
For this example, we add an entry for our new function under the `Strings` section: | ||
|
||
```markdown | ||
### Strings | ||
|
||
| Built-in | Description | Wasm Support | | ||
| ------- |-------------|---------------| | ||
... | ||
| <span class="opa-keep-it-together">``output := repeat(string, count)``</span> | ``output`` is ``string`` repeated ``count``times | ``SDK-dependent`` | | ||
... | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
cases: | ||
- note: cryptohmacmd5/crypto.hmac.md5 | ||
query: data.test.p = x | ||
modules: | ||
- | | ||
package test | ||
|
||
p[mac] { | ||
mac := crypto.hmac.md5(input.message, input.key) | ||
} | ||
input: {"message": "foo", "key": "bar"} | ||
want_result: | ||
- x: | ||
- 31b6db9e5eb4addb42f1a6ca07367adc | ||
- note: cryptohmacmd5/crypto.hmac.md5_unicode | ||
query: data.test.p = x | ||
modules: | ||
- | | ||
package test | ||
|
||
p[mac] { | ||
mac := crypto.hmac.md5(input.message, input.key) | ||
} | ||
input: {"message": "åäöçß🥲♙Ω", "key": "秘密の"} | ||
want_result: | ||
- x: | ||
- 20a8743c2157ac60b7e8b79c83651b8d | ||
strict_error: true |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized there is more in-depth documentation for built-in:s at
extensions/#custom-built-in-functions-in-go
. Should probably link to there from here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a little different, though: it's about extending OPA, so you'd build your own binary with the extra built-in func included. This document describes how to contribute to OPA, so the new built-in function would become part of "core OPA" so to speak. There certainly is overlap, but it's a different twist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is different for sure, but I think @johanfylling is right that it would be good to reference here.. at least in the sense that one step that's currently not mentioned is to first have some form of approval for the built-in. Without that—i.e. for someone who wants to integrate Kafka/Redis/DynamoDB/Whatever—the custom function is probably a better alternative.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a related note, this feels a bit like these are two changes: a docs addition and new built-in functions.
It would be nice if we untangled that a bit, we could massage it into two commits -- two PRs would be another option, but since it's here already, it's OK I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I agree; I should have gone that route.