Implement data source interfaces & registration in rego engine #4997
Conversation
JAORMX
force-pushed
the
datasource-reg
branch
3 times, most recently
from
bf93fe0 to
cf465e9
Compare
JAORMX
force-pushed
the
datasource-reg
branch
from
cf465e9 to
1903f42
Compare
This implements the basic interfaces that data sources must fulfil in order to be called and used within Minder. This is: * Listing the functions they provide * Validating updates Each function itself must provide a unique key for the engine to use, as well as argument validation and the data source call itself. Finally, this implements an engine option that allows data sources to be dynamically registered into the rego engine. Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
JAORMX
force-pushed
the
datasource-reg
branch
from
1903f42 to
616856d
Compare
Signed-off-by: Juan Antonio Osorio <[email protected]>
JAORMX
force-pushed
the
datasource-reg
branch
from
6aebd3b to
a3b1c4b
Compare
Co-authored-by: Eleftheria Stein-Kousathana <[email protected]>
| // RegisterDataSources implements the Eval interface. | ||
| func (e *Evaluator) RegisterDataSources(dsr *v1datasources.DataSourceRegistry) { | ||
| for key, dsf := range dsr.GetFuncs() { | ||
| fmt.Printf("Registering data source %s\n", key) |
There was a problem hiding this comment.
Did you mean to leave in a Printf?
| func (e *Evaluator) RegisterDataSources(dsr *v1datasources.DataSourceRegistry) { | ||
| for key, dsf := range dsr.GetFuncs() { | ||
| fmt.Printf("Registering data source %s\n", key) | ||
| e.regoOpts = append(e.regoOpts, buildFromDataSource(key, dsf)) |
There was a problem hiding this comment.
Question: does it make sense for dsf from dsr.GetFuncs() to actually have a func(*rego.Rego) method (i.e. to move the buildFromDataSource into a function on DataSourceFuncDef)?
It pushes the Rego types into a slighly larger area of the code, but it helps avoid having a big ball of code in eval/rego (and if we name the method Rego or RegoOpt, we would have a pattern for any other future evaluation engines).
There was a problem hiding this comment.
I wanted to have data sources to be quite independent from the eval engine. The eval engine would then be in charge of interpreting the data source and making sense of it. It brings a lot more responsibility into the eval engine, but it allows for us to include more engines with ease in the future. e.g. if we'd introduce a WASM engine, we wouldn't need to mess around with rego structures.
| if err := dsf.ValidateArgs(obj); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| // Call the data source function | ||
| ret, err := dsf.Call(jsonObj) |
There was a problem hiding this comment.
If you move buildFromDataSource to DataSourceFuncDef, you might not need to expose ValidateArgs or Call in the interface.
| err = e.Eval(context.Background(), emptyPol, nil, &interfaces.Result{ | ||
| Object: map[string]any{ | ||
| "data": "bar", | ||
| }, | ||
| }) | ||
| require.ErrorIs(t, err, engerrors.ErrEvaluationFailed, "should have failed the evaluation") |
There was a problem hiding this comment.
I'm wondering if it would make sense to use a real registry and DataSource here, to provide an example for future users (and to verify the lifecycle more deeply than Mocks).
(You'll also get a bit more test coverage of registry.go for "free")
There was a problem hiding this comment.
You have a good point, I'll add a testing implementation
| return nil | ||
| } | ||
|
|
||
| // GetFuncs returns all functions that data sources provide globally. |
There was a problem hiding this comment.
This could eventually pull from the database to get project-specific data sources, right?
There was a problem hiding this comment.
That's right, the idea is that we'd do an initial parsing from the db, turn this into in-memory objects, and then pass these around.
Summary
This implements the basic interfaces that data sources must fulfil in
order to be called and used within Minder. This is:
Each function itself must provide a unique key for the engine to use, as
well as argument validation and the data source call itself.
Finally, this implements an engine option that allows data sources to be
dynamically registered into the rego engine.
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: