diff --git a/.golangci.yml b/.golangci.yml index 370e1c36f..52eb3a545 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -26,5 +26,8 @@ linters-settings: - rangeValCopy - indexAlloc - deprecatedComment + settings: + ifElseChain: + minThreshold: 3 cyclop: max-complexity: 20 diff --git a/pkg/loki/flow_query.go b/pkg/loki/flow_query.go index f61ae8c2c..d20a0d271 100644 --- a/pkg/loki/flow_query.go +++ b/pkg/loki/flow_query.go @@ -156,21 +156,26 @@ func (q *FlowQueryBuilder) addLineFilters(key string, values []string, not bool, // addIPFilters assumes that we are searching for that IP addresses as part // of the log line (not in the stream selector labels) func (q *FlowQueryBuilder) addIPFilters(key string, values []string, not bool) { + if not { + // NOT IP filters means we don't want any of the values, ie. IP!=A AND IP!=B instead of IP!=A OR IP!=B + for _, value := range values { + // empty exact matches should be treated as attribute filters looking for empty IP + if value == emptyMatch { + q.jsonFilters = append(q.jsonFilters, []filters.LabelFilter{filters.NotStringLabelFilter(key, "")}) + } else { + q.jsonFilters = append(q.jsonFilters, []filters.LabelFilter{filters.NotIPLabelFilter(key, value)}) + } + } + return + } + // Positive case filtersPerKey := make([]filters.LabelFilter, 0, len(values)) for _, value := range values { // empty exact matches should be treated as attribute filters looking for empty IP if value == emptyMatch { - if not { - filtersPerKey = append(filtersPerKey, filters.NotStringLabelFilter(key, "")) - } else { - filtersPerKey = append(filtersPerKey, filters.StringEqualLabelFilter(key, "")) - } + filtersPerKey = append(filtersPerKey, filters.StringEqualLabelFilter(key, "")) } else { - if not { - filtersPerKey = append(filtersPerKey, filters.NotIPLabelFilter(key, value)) - } else { - filtersPerKey = append(filtersPerKey, filters.IPLabelFilter(key, value)) - } + filtersPerKey = append(filtersPerKey, filters.IPLabelFilter(key, value)) } } q.jsonFilters = append(q.jsonFilters, filtersPerKey) diff --git a/pkg/model/filters/logql.go b/pkg/model/filters/logql.go index cf83b0126..9f38de1e8 100644 --- a/pkg/model/filters/logql.go +++ b/pkg/model/filters/logql.go @@ -252,6 +252,7 @@ func NotContainsKeyLineFilter(key string) LineFilter { } } +// NumericLineFilter returns a LineFilter and true if it has an empty match func NumericLineFilter(key string, values []string, not, moreThan bool) (LineFilter, bool) { return checkExact( LineFilter{ @@ -271,6 +272,7 @@ func ArrayLineFilter(key string, values []string, not bool) LineFilter { return lf } +// StringLineFilterCheckExact returns a LineFilter and true if it has an empty match func StringLineFilterCheckExact(key string, values []string, not bool) (LineFilter, bool) { return checkExact(LineFilter{key: key, not: not}, values, typeRegexContains) } @@ -364,21 +366,11 @@ func moreThanRegex(sb *strings.Builder, value string) { // under construction (contained in the provided strings.Builder) func (f *LineFilter) WriteInto(sb *strings.Builder) { if f.not { - if !f.allowEmpty { - // the record must contains the field if values are specified - // since FLP skip empty fields / zeros values - if len(f.values) > 0 { - sb.WriteString("|~`\"") - sb.WriteString(f.key) - sb.WriteString("\"`") - } - } - // then we exclude match results - sb.WriteString("!~`") - } else { - sb.WriteString("|~`") + f.writeIntoNot(sb) + return } + sb.WriteString("|~`") if len(f.values) == 0 { // match only the end of KEY if not 'strictKey' // no value will be provided here as we only check if key exists @@ -392,49 +384,85 @@ func (f *LineFilter) WriteInto(sb *strings.Builder) { if i > 0 { sb.WriteByte('|') } + f.writeValueInto(sb, v) + } + } + sb.WriteRune('`') +} - // match only the end of KEY + regex VALUE if not 'strictKey' - // if numeric, KEY":VALUE, - // if string KEY":"VALUE" - // ie 'Port' key will match both 'SrcPort":"XXX"' and 'DstPort":"XXX" - // VALUE can be quoted for exact match or contains * to inject regex any - // For numeric values, exact match is implicit - // (the trick is to match for the ending coma; it works as long as the filtered field - // is not the last one (they're in alphabetic order); a less performant alternative - // but more future-proof/less hacky could be to move that to a json filter, if needed) - if f.strictKey { - sb.WriteByte('"') - } +// WriteInto transforms a LineFilter to its corresponding part of a LogQL query +// under construction (contained in the provided strings.Builder) +func (f *LineFilter) writeIntoNot(sb *strings.Builder) { + if !f.allowEmpty { + // the record must contains the field if values are specified + // since FLP skip empty fields / zeros values + if len(f.values) > 0 { + sb.WriteString("|~`\"") sb.WriteString(f.key) - sb.WriteString(`":`) - switch v.valueType { - case typeNumber, typeRegex: - if f.moreThan { - moreThanRegex(sb, v.value) - } else { - sb.WriteString(v.value) - } - // a number or regex can be followed by } if it's the last property of a JSON document - sb.WriteString("[,}]") - case typeBool: - sb.WriteString(v.value) - case typeString, typeIP: - // exact matches are specified as just strings - sb.WriteByte('"') - sb.WriteString(valueReplacer.Replace(v.value)) - sb.WriteByte('"') - // contains-match are specified as regular expressions - case typeRegexContains: - sb.WriteString(`"(?i)[^"]*`) - sb.WriteString(valueReplacer.Replace(v.value)) - sb.WriteString(`.*"`) - // for array, we ensure it starts by [ and ends by ] - case typeRegexArrayContains: - sb.WriteString(`\[(?i)[^]]*`) - sb.WriteString(valueReplacer.Replace(v.value)) - sb.WriteString(`[^]]*]`) - } + sb.WriteString("\"`") } } - sb.WriteRune('`') + + if len(f.values) == 0 { + // then we exclude match results + sb.WriteString("!~`") + + // match only the end of KEY if not 'strictKey' + // no value will be provided here as we only check if key exists + if f.strictKey { + sb.WriteByte('"') + } + sb.WriteString(f.key) + sb.WriteString("\"`") + } else { + for _, v := range f.values { + sb.WriteString("!~`") + f.writeValueInto(sb, v) + sb.WriteRune('`') + } + } +} + +func (f *LineFilter) writeValueInto(sb *strings.Builder, v lineMatch) { + // match only the end of KEY + regex VALUE if not 'strictKey' + // if numeric, KEY":VALUE, + // if string KEY":"VALUE" + // ie 'Port' key will match both 'SrcPort":"XXX"' and 'DstPort":"XXX" + // VALUE can be quoted for exact match or contains * to inject regex any + // For numeric values, exact match is implicit + // (the trick is to match for the ending coma; it works as long as the filtered field + // is not the last one (they're in alphabetic order); a less performant alternative + // but more future-proof/less hacky could be to move that to a json filter, if needed) + if f.strictKey { + sb.WriteByte('"') + } + sb.WriteString(f.key) + sb.WriteString(`":`) + switch v.valueType { + case typeNumber, typeRegex: + if f.moreThan { + moreThanRegex(sb, v.value) + } else { + sb.WriteString(v.value) + } + // a number or regex can be followed by } if it's the last property of a JSON document + sb.WriteString("[,}]") + case typeBool: + sb.WriteString(v.value) + case typeString, typeIP: + // exact matches are specified as just strings + sb.WriteByte('"') + sb.WriteString(valueReplacer.Replace(v.value)) + sb.WriteByte('"') + // contains-match are specified as regular expressions + case typeRegexContains: + sb.WriteString(`"(?i)[^"]*`) + sb.WriteString(valueReplacer.Replace(v.value)) + sb.WriteString(`.*"`) + // for array, we ensure it starts by [ and ends by ] + case typeRegexArrayContains: + sb.WriteString(`\[(?i)[^]]*`) + sb.WriteString(valueReplacer.Replace(v.value)) + sb.WriteString(`[^]]*]`) + } } diff --git a/pkg/model/filters/logql_test.go b/pkg/model/filters/logql_test.go index d77134924..26ce73149 100644 --- a/pkg/model/filters/logql_test.go +++ b/pkg/model/filters/logql_test.go @@ -74,3 +74,22 @@ func TestWriteInto_7654(t *testing.T) { assert.Equal(t, reg.MatchString(val), i >= 7654, fmt.Sprintf("Value: %d", i)) } } + +func TestMultiStrings(t *testing.T) { + lf, ok := StringLineFilterCheckExact("foo", []string{`"a"`, `"b"`}, false) + assert.False(t, ok) + sb := strings.Builder{} + lf.WriteInto(&sb) + assert.Equal(t, "|~"+backtick(`foo":"a"|foo":"b"`), sb.String()) + + // Repeat with "not" (here we expect foo being neither a nor b) + lf, ok = StringLineFilterCheckExact("foo", []string{`"a"`, `"b"`}, true) + assert.False(t, ok) + sb = strings.Builder{} + lf.WriteInto(&sb) + assert.Equal(t, "|~"+backtick(`"foo"`)+"!~"+backtick(`foo":"a"`)+"!~"+backtick(`foo":"b"`), sb.String()) +} + +func backtick(str string) string { + return "`" + str + "`" +} diff --git a/pkg/server/server_flows_test.go b/pkg/server/server_flows_test.go index a38aea879..9017cf2e9 100644 --- a/pkg/server/server_flows_test.go +++ b/pkg/server/server_flows_test.go @@ -64,6 +64,12 @@ func TestLokiFiltering(t *testing.T) { outputQueries: []string{ "?query={app=\"netobserv-flowcollector\"}|~`SrcK8S_Name\":\"(?i)[^\"]*name1.*\"|SrcK8S_Name\":\"(?i)[^\"]*name2.*\"`", }, + }, { + name: "NOT line filter same key", + inputPath: "?filters=" + url.QueryEscape(`SrcK8S_Name!="name1","name2"`), + outputQueries: []string{ + "?query={app=\"netobserv-flowcollector\"}|~`\"SrcK8S_Name\"`!~`SrcK8S_Name\":\"name1\"`!~`SrcK8S_Name\":\"name2\"`", + }, }, { name: "OR label filter same key", inputPath: "?filters=" + url.QueryEscape("SrcK8S_Namespace=ns1,ns2"), @@ -90,6 +96,12 @@ func TestLokiFiltering(t *testing.T) { outputQueries: []string{ "?query={app=\"netobserv-flowcollector\"}|json|SrcAddr=ip(\"10.128.0.1\")+or+SrcAddr=ip(\"10.128.0.2\")", }, + }, { + name: "NOT IP filters", + inputPath: "?filters=" + url.QueryEscape(`SrcAddr!=10.128.0.1,10.128.0.2`), + outputQueries: []string{ + "?query={app=\"netobserv-flowcollector\"}|json|SrcAddr!=ip(\"10.128.0.1\")|SrcAddr!=ip(\"10.128.0.2\")", + }, }, { name: "Several OR filters", inputPath: "?filters=" + url.QueryEscape("SrcPort=8080|SrcAddr=10.128.0.1|SrcK8S_Namespace=default"), diff --git a/web/src/model/__tests__/filters.spec.ts b/web/src/model/__tests__/filters.spec.ts index a9d52559b..c822b72d3 100644 --- a/web/src/model/__tests__/filters.spec.ts +++ b/web/src/model/__tests__/filters.spec.ts @@ -1,6 +1,7 @@ import { FilterDefinitionSample } from '../../components/__tests-data__/filters'; import { findFilter } from '../../utils/filter-definitions'; import { doesIncludeFilter, Filter, filtersEqual } from '../filters'; +import { filtersToString } from '../flow-query'; describe('doesIncludeFilter', () => { const srcNameFilter = findFilter(FilterDefinitionSample, 'src_name')!; @@ -18,6 +19,11 @@ describe('doesIncludeFilter', () => { } ]; + it('should encode as', () => { + const asString = filtersToString(activeFilters, false); + expect(asString).toEqual(encodeURIComponent('SrcK8S_Name=abc,def&DstK8S_Name!=abc,def')); + }); + it('should not include filter due to different key', () => { const isIncluded = doesIncludeFilter(activeFilters, { def: findFilter(FilterDefinitionSample, 'protocol')! }, [ { v: 'abc' },