Dependency Extraction Data Source Driver #5094
Conversation
puerco
force-pushed
the
deps-datasource
branch
2 times, most recently
from
b8401d4 to
a7015f7
Compare
puerco
changed the title
puerco
force-pushed
the
deps-datasource
branch
9 times, most recently
from
5b1774c to
e166c37
Compare
puerco
changed the title
This commit adds the deps data source types to the minder proto definitions. Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
This commit wires the deps data source through the service machinery. Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
puerco
force-pushed
the
deps-datasource
branch
from
e166c37 to
21b82c5
Compare
This adds the dependency data source driver logic to the minder codebase Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
Signed-off-by: Adolfo García Veytia (Puerco) <puerco@stacklok.com>
puerco
force-pushed
the
deps-datasource
branch
from
21b82c5 to
50413b6
Compare
| // DepsDataSource is the dependency data source driver | ||
| message DepsDataSource { | ||
| message Def { | ||
| // Path where the dependency extractor will look for dependency data | ||
| string path = 1; | ||
|
|
||
| // List of ecosystems to check, defaults all supported by the extractor | ||
| repeated string ecosystems = 2; | ||
| } | ||
|
|
||
| map<string, Def> def = 1; | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: is there some form of validation that we can add here?
Looks like this can be almost copy-pasted.
| @@ -56,6 +57,7 @@ func buildFromDataSource( | |||
| } | |||
|
|
|||
| if err := dsf.ValidateArgs(jsonObj); err != nil { | |||
| log.Error().Interface("dsfunction", k).Err(err).Msg("error validating datasource function arguments") | |||
There was a problem hiding this comment.
nit: I would avoid logging so deep in the stack, it would be much better to raise a proper error and log in upper layers instead.
Given that Data Sources are meant to be user-defined, I don't think there's much value to log an error, as the user would not have access to it. Besides, we have no idea how often this function is going to be called by REGO code in the rule type, this might be spammy.
There was a problem hiding this comment.
This is mostly for us to track the rego eval engine, it's hard to observe what it's doing without them. Would it be better if I set the level to Debug?
| @@ -69,6 +71,7 @@ func buildFromDataSource( | |||
| ) | |||
| ret, err := dsf.Call(ctx, jsonObj) | |||
| if err != nil { | |||
| log.Error().Interface("dsfunction", k).Err(err).Msgf("function call returned error") | |||
| case *minderv1.DataSource_Deps: | ||
| for name, def := range drv.Deps.GetDef() { | ||
| defBytes, err := protojson.Marshal(def) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to marshal REST definition: %w", err) | ||
| } | ||
|
|
||
| if _, err := tx.AddDataSourceFunction(ctx, db.AddDataSourceFunctionParams{ | ||
| DataSourceID: dsID, | ||
| ProjectID: projectID, | ||
| Name: name, | ||
| Type: v1datasources.DataSourceDriverDeps, | ||
| Definition: defBytes, | ||
| }); err != nil { | ||
| return fmt.Errorf("failed to create data source function: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
question: is there a way to share this code path regardless of the type?
| return errs | ||
| } | ||
|
|
||
| func (_ *depsDataSourceHandler) ValidateUpdate(_ any) error { return nil } |
There was a problem hiding this comment.
question: I guess we could verify something here, like the cardinality of the functions, but might be impractical. Any ideas?
|
I'll prioritize #5165 because we need that data source sooner, and then come back to this one. |
Summary
This PR introduces the dependency extraction data source.
Not ready for review, checking it in early and needs other PRs and a rebase to work.
This needs
#5091#5092#5093Fixes #5090
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: