[KATC] Update config schema, including overlays #1772
Conversation
| return errors.New("key value consumer is nil") | ||
| } | ||
|
|
||
| var kvPairs map[string]any |
There was a problem hiding this comment.
Discovered when testing with @jbeker that the existing KV consumer in this package does not work for this config because Golang won't unmarshal the config as a map[string]string since it's a more complex object, which makes sense.
This is a quick fix, but maybe we want something cleverer?
There was a problem hiding this comment.
Thinking out loud....
Hrm. To be honest, I had to pull them down and diff them to understand the difference.
If I read this right, the difference is that in the new case, k2 is sending map[string]any, while the old case is map[string]string.
The thing that seems weird, is that this is then json.Marshal back down to map[string]string I assume because the underlying ~everything expects that.
So I wonder... If we're going to json.Marshal down to strings anyhow, is it cleaner if K2 does that for us? It might not be, it's sorta of nice of the API is just the json, and not marshalled json. But I want to ask...
There was a problem hiding this comment.
Correct, this handles the error that Jeremy and I were seeing: failed to decode key-value json: json: cannot unmarshal object into Go value of type string.
I wasn't sure if it would be cleaner for K2 to update or not.
There was a problem hiding this comment.
It's going to be pretty trivial on the k2 side. And I think it's pretty trivial here.
So I think we get to decide what feels like a cleaner protocol.
Having everything as a string feels clean, but having the KATC definitions marshalled into strings feels kinda weird. 🤷
There was a problem hiding this comment.
I guess I like it this way a little better, then -- it feels tidy that k2 doesn't need to know about the marshalling and unmarshalling to a string; launcher handles that itself for ease of storage in bucket/startup_settings.
There was a problem hiding this comment.
It's step towards a more structured interface and API between k2 and launcher. (vs what we have now, which feels like it requires a very complete knowledge of all the parts)
| return errors.New("key value consumer is nil") | ||
| } | ||
|
|
||
| var kvPairs map[string]any |
There was a problem hiding this comment.
Thinking out loud....
Hrm. To be honest, I had to pull them down and diff them to understand the difference.
If I read this right, the difference is that in the new case, k2 is sending map[string]any, while the old case is map[string]string.
The thing that seems weird, is that this is then json.Marshal back down to map[string]string I assume because the underlying ~everything expects that.
So I wonder... If we're going to json.Marshal down to strings anyhow, is it cleaner if K2 does that for us? It might not be, it's sorta of nice of the API is just the json, and not marshalled json. But I want to ask...
| if sourceConstraintExists { | ||
| return &sourceConstraint | ||
| // getPathConstraint retrieves any constraints against the `path` column | ||
| func getPathConstraint(queryContext table.QueryContext) *table.ConstraintList { |
There was a problem hiding this comment.
FWIW https://github.com/kolide/launcher/blob/main/ee/tables/tablehelpers/getconstraints.go#L51 is similar, but returns []string. This is fine though...
Other caveat, is that I think this all only works if the constraints are =, it'll fail for LIKE or <. But, conveniently, I don't think those are passed in the QueryContext. At least, this assumption is also made in other parts of our code. Mostly noting it as an FYI for the future.
There was a problem hiding this comment.
Oh, I see you're handling LIKE and GLOB as well. I'm not sure osquery passes those, but that's nice completeness.
This PR makes the following config updates:
sourcecolumn topathto be more consistent with our e.g. dataflatten tables, which also return apathcolumnsourcetosource_pathsfor clarity; additionally, make it an array rather than a single string to allow for matching against multiple patternsquerytosource_queryto disambiguate from thequerycolumn we use elsewhereplatformtofiltersto clarify that we may have other types of filters in the futurefiltersinside the overlaysThis is an alternative implementation to #1770.