-
Notifications
You must be signed in to change notification settings - Fork 103
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
[KATC] Update config schema #1770
Conversation
@@ -17,7 +17,7 @@ import ( | |||
// that performs the query against the source. | |||
type katcSourceType struct { | |||
name string | |||
dataFunc func(ctx context.Context, slogger *slog.Logger, sourcePattern string, query string, sourceConstraints *table.ConstraintList) ([]sourceData, error) | |||
dataFunc func(ctx context.Context, slogger *slog.Logger, sourcePaths []string, query string, pathConstraints *table.ConstraintList) ([]sourceData, error) |
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'm a little unsure about how pathConstraints
will be used. Can you provide an example? (In this comment is fine, doesn't need to be docs)
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 see what the code is doing, but I'm not sure it adds a lot over a pushing sourcePath
through `filepath.Glob(...)
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.
Added documentation with an example
"err", err, | ||
) | ||
continue | ||
} | ||
|
||
if cfg.Platform != runtime.GOOS { | ||
// For now, the filter is simply the OS |
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 think we probably want to future proof this a little.
Brainstorming... We could define a rich DSL with boolean logic. Or maybe we can just have a series of strings? Or maybe a series of K:V pairs? Early use might be goos:darwin
or something?
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.
What about making filter
into filters
, a map[string]string? Simple to unmarshal and therefore parse, and flexible for future additions?
{
"name": "kolide_indexeddb_leveldb_test",
"source_type": "indexeddb_leveldb",
"filters": {
"goos": "darwin"
},
"columns": ["data"],
"source_paths": ["/some/path/to/db.indexeddb.leveldb"],
"source_query": "db.store",
"row_transform_steps": ["deserialize_chrome"]
}
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.
filters
makes sense to me. I don't know if that should be an array of strings, or a map. Array might be little more flexible, I think either can work.
I think we should ponder whether that's an AND
or an OR
. If we go the way of overlays, this should be an AND
, because the overlays handle OR
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've updated to filters
in the other PR #1772 -- if we decide to go with this approach I'll make the update here as well.
479571c
Closing in favor of #1772 |
This PR makes the following config updates:
source
column topath
to be more consistent with our e.g. dataflatten tables, which also return apath
columnsource
tosource_paths
for clarity; additionally, make it an array rather than a single string to allow for matching against multiple patternsquery
tosource_query
to disambiguate from thequery
column we use elsewhereplatform
tofilter
to clarify that we may have other types of filters in the futureThese changes were made in roughly this order; it is possible to review commit-by-commit if that's easier.