Skip to content

Commit

Permalink
Fix case with different and-filter on same key
Browse files Browse the repository at this point in the history
  • Loading branch information
jotak committed Mar 30, 2022
1 parent 99551dd commit 7b1265b
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 97 deletions.
12 changes: 7 additions & 5 deletions pkg/handler/flows.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,27 @@ type errorWithCode struct {
code int
}

type keyValues = []string

// Example of raw filters (url-encoded):
// foo=a,b&bar=c|baz=d
func parseFilters(raw string) ([]map[string]string, error) {
var parsed []map[string]string
func parseFilters(raw string) ([][]keyValues, error) {
var parsed [][]keyValues
decoded, err := url.QueryUnescape(raw)
if err != nil {
return nil, err
}
groups := strings.Split(decoded, "|")
for _, group := range groups {
m := make(map[string]string)
parsed = append(parsed, m)
var andFilters []keyValues
filters := strings.Split(group, "&")
for _, filter := range filters {
pair := strings.Split(filter, "=")
if len(pair) == 2 {
m[pair[0]] = pair[1]
andFilters = append(andFilters, pair)
}
}
parsed = append(parsed, andFilters)
}
return parsed, nil
}
Expand Down
44 changes: 28 additions & 16 deletions pkg/handler/flows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,38 +14,50 @@ func TestParseFilters(t *testing.T) {
require.NoError(t, err)

assert.Len(t, groups, 2)
assert.Equal(t, map[string]string{
"foo": "a,b",
"bar": "c",
assert.Equal(t, [][]string{
{"foo", "a,b"},
{"bar", "c"},
}, groups[0])
assert.Equal(t, map[string]string{
"baz": "d",
assert.Equal(t, [][]string{
{"baz", "d"},
}, groups[1])

// Resource path + port, match all
groups, err = parseFilters(url.QueryEscape(`SrcK8S_Type="Pod"&SrcK8S_Namespace="default"&SrcK8S_Name="test"&SrcPort=8080`))
require.NoError(t, err)

assert.Len(t, groups, 1)
assert.Equal(t, map[string]string{
"SrcK8S_Type": `"Pod"`,
"SrcK8S_Namespace": `"default"`,
"SrcK8S_Name": `"test"`,
"SrcPort": `8080`,
assert.Equal(t, [][]string{
{"SrcK8S_Type", `"Pod"`},
{"SrcK8S_Namespace", `"default"`},
{"SrcK8S_Name", `"test"`},
{"SrcPort", `8080`},
}, groups[0])

// Resource path + port, match any
groups, err = parseFilters(url.QueryEscape(`SrcK8S_Type="Pod"&SrcK8S_Namespace="default"&SrcK8S_Name="test"|SrcPort=8080`))
require.NoError(t, err)

assert.Len(t, groups, 2)
assert.Equal(t, map[string]string{
"SrcK8S_Type": `"Pod"`,
"SrcK8S_Namespace": `"default"`,
"SrcK8S_Name": `"test"`,
assert.Equal(t, [][]string{
{"SrcK8S_Type", `"Pod"`},
{"SrcK8S_Namespace", `"default"`},
{"SrcK8S_Name", `"test"`},
}, groups[0])

assert.Equal(t, map[string]string{
"SrcPort": `8080`,
assert.Equal(t, [][]string{
{"SrcPort", `8080`},
}, groups[1])

// Resource path + name, match all
groups, err = parseFilters(url.QueryEscape(`SrcK8S_Type="Pod"&SrcK8S_Namespace="default"&SrcK8S_Name="test"&SrcK8S_Name="nomatch"`))
require.NoError(t, err)

assert.Len(t, groups, 1)
assert.Equal(t, [][]string{
{"SrcK8S_Type", `"Pod"`},
{"SrcK8S_Namespace", `"default"`},
{"SrcK8S_Name", `"test"`},
{"SrcK8S_Name", `"nomatch"`},
}, groups[0])
}
10 changes: 5 additions & 5 deletions pkg/handler/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,15 @@ func GetNames(cfg loki.Config) func(w http.ResponseWriter, r *http.Request) {
}

func getNamesForPrefix(cfg loki.Config, lokiClient httpclient.HTTPClient, prefix, kind, namespace string) ([]string, int, error) {
lokiParams := map[string]string{
prefix + fields.Namespace: exact(namespace),
}
lokiParams := [][]string{{
prefix + fields.Namespace, exact(namespace),
}}
var fieldToExtract string
if utils.IsOwnerKind(kind) {
lokiParams[prefix+fields.OwnerType] = exact(kind)
lokiParams = append(lokiParams, []string{prefix + fields.OwnerType, exact(kind)})
fieldToExtract = prefix + fields.OwnerName
} else {
lokiParams[prefix+fields.Type] = exact(kind)
lokiParams = append(lokiParams, []string{prefix + fields.Type, exact(kind)})
fieldToExtract = prefix + fields.Name
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/handler/topology.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func getTopologyFlows(cfg loki.Config, client httpclient.HTTPClient, params url.
rawJSON = res
} else {
// else, run all at once
var filters map[string]string
var filters [][]string
if len(filterGroups) > 0 {
filters = filterGroups[0]
}
Expand All @@ -76,7 +76,7 @@ func getTopologyFlows(cfg loki.Config, client httpclient.HTTPClient, params url.
return rawJSON, http.StatusOK, nil
}

func buildTopologyQuery(cfg *loki.Config, filters map[string]string, start, end, limit, reporter string) (string, int, error) {
func buildTopologyQuery(cfg *loki.Config, filters [][]string, start, end, limit, reporter string) (string, int, error) {
qb, err := loki.NewTopologyQuery(cfg, start, end, limit, reporter)
if err != nil {
return "", http.StatusBadRequest, err
Expand Down
6 changes: 3 additions & 3 deletions pkg/loki/flow_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ func NewFlowQueryBuilderWithDefaults(cfg *Config) *FlowQueryBuilder {
return NewFlowQueryBuilder(cfg, "", "", "", constants.ReporterBoth)
}

func (q *FlowQueryBuilder) Filters(filters map[string]string) error {
for key, values := range filters {
if err := q.AddFilter(key, values); err != nil {
func (q *FlowQueryBuilder) Filters(filters [][]string) error {
for _, filter := range filters {
if err := q.AddFilter(filter[0], filter[1]); err != nil {
return err
}
}
Expand Down
48 changes: 16 additions & 32 deletions web/src/model/flow-query.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { FieldMapping, Filter, FilterValue } from './filters';
import { Filter } from './filters';

export type Reporter = 'source' | 'destination' | 'both';
export type Match = 'all' | 'any';
Expand All @@ -12,8 +12,8 @@ export interface FlowQuery {
limit: number;
}

// All filters in AND-group (ie. usually for "match all") are set in a map
type AndGroup = Map<string, string[]>;
// All filters in AND-group (ie. usually for "match all") are set in a list of [key-values]
type AndGroup = { key: string; values: string[] }[];
// All filters in OR-group (ie. usually for "match any") are set as elements of AndGroup array
type OrGroup = AndGroup[];

Expand All @@ -28,65 +28,49 @@ type OrGroup = AndGroup[];
// Match all: put all filters in a single AndGroup, except if there's a Src/Dst group split found
// in which case there will be Src-AndGroup OR Dst-AndGroup
export const groupFiltersMatchAll = (filters: Filter[]): string => {
const srcMatch: AndGroup = new Map();
const dstMatch: AndGroup = new Map();
const srcMatch: AndGroup = [];
const dstMatch: AndGroup = [];
let needSrcDstSplit = false;
filters.forEach(f => {
if (f.def.fieldMatching.always) {
// Filters here are always applied, regardless Src/Dst group split
f.def.fieldMatching.always(f.values).forEach(filter => {
addToAndGroup(srcMatch, filter.key, filter.values);
addToAndGroup(dstMatch, filter.key, filter.values);
srcMatch.push(filter);
dstMatch.push(filter);
});
} else {
needSrcDstSplit = true;
// Filters here are applied for their Src/Dst group split
f.def.fieldMatching.ifSrc!(f.values).forEach(filter => {
addToAndGroup(srcMatch, filter.key, filter.values);
srcMatch.push(filter);
});
f.def.fieldMatching.ifDst!(f.values).forEach(filter => {
addToAndGroup(dstMatch, filter.key, filter.values);
dstMatch.push(filter);
});
}
});
return encodeFilters(needSrcDstSplit ? [srcMatch, dstMatch] : [srcMatch]);
};

const addToAndGroup = (group: AndGroup, key: string, values: string[]) => {
const currentValues = group.get(key) || [];
group.set(key, [...currentValues, ...values]);
};

export const groupFiltersMatchAny = (filters: Filter[]): string => {
const orGroup: OrGroup = [];
filters.forEach(f => {
if (f.def.fieldMatching.always) {
orGroup.push(newGroup(f.def.fieldMatching.always, f.values));
orGroup.push(f.def.fieldMatching.always(f.values));
} else {
orGroup.push(newGroup(f.def.fieldMatching.ifSrc!, f.values));
orGroup.push(newGroup(f.def.fieldMatching.ifDst!, f.values));
orGroup.push(f.def.fieldMatching.ifSrc!(f.values));
orGroup.push(f.def.fieldMatching.ifDst!(f.values));
}
});
return encodeFilters(orGroup);
};

const newGroup = (mapping: FieldMapping, values: FilterValue[]): AndGroup => {
const filterFields = mapping(values).map(filter => {
return [filter.key, filter.values] as [string, string[]];
});
return new Map(filterFields);
};

const encodeFilters = (filters: Map<string, string[]>[]): string => {
const encodeFilters = (filters: OrGroup): string => {
// Example of output: foo=a,b&bar=c|baz=d (url-encoded)
const str = filters
.map(group => {
return Array.from(group.entries())
.map(pair => {
return `${pair[0]}=${pair[1].join(',')}`;
})
.join('&');
})
.map(group => group
.map(filter => `${filter.key}=${filter.values.join(',')}`)
.join('&'))
.join('|');
return encodeURIComponent(str);
};
34 changes: 0 additions & 34 deletions web/src/utils/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,6 @@ export const flowdirToReporter: { [flowdir: string]: Reporter } = {
'': 'both'
};

// export const reporterToFlowdir = _.invert(flowdirToReporter);

// export const buildURLParams = (
// filters: Filter[],
// range: number | TimeRange,
// match: Match,
// opts: QueryOptions
// ): URLParams => {
// const params: URLParams = {};
// params[URLParam.Filters] = filtersToURL(filters);
// if (range) {
// if (typeof range === 'number') {
// params[URLParam.TimeRange] = range;
// } else if (typeof range === 'object') {
// params[URLParam.StartTime] = range.from.toString();
// params[URLParam.EndTime] = range.to.toString();
// }
// }
// if (opts.reporter !== 'both') {
// params[URLParam.Reporter] = reporterToFlowdir[opts.reporter];
// }
// params[URLParam.Limit] = opts.limit;
// params[URLParam.Match] = match;
// return params;
// };

// export const getURLParams = (qa: URLParams) => {
// const urlParams = new URLSearchParams();
// _.each(qa, (v, k) => {
// urlParams.set(k, String(v));
// });
// return urlParams;
// };

export const getRangeFromURL = (): number | TimeRange => {
const timeRange = getURLParamAsNumber(URLParam.TimeRange);
const startTime = getURLParamAsNumber(URLParam.StartTime);
Expand Down

0 comments on commit 7b1265b

Please sign in to comment.