From 05ab443cef45e1ecb04cdebe99cf21b572693448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Bedi?= Date: Mon, 26 Jan 2026 09:50:49 +0100 Subject: [PATCH] fix(zabbix): prevent silent removal of itemTagFilter when no tags match regex (#2248) Added a test to ensure that when a regex itemTagFilter does not match any tags, the GetItems function returns an empty list instead of all items. This addresses a bug where an empty tag filter would lead to unintended behavior by removing the filter silently. Fixes #2247 --- pkg/zabbix/methods.go | 8 +++++ pkg/zabbix/methods_test.go | 70 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/pkg/zabbix/methods.go b/pkg/zabbix/methods.go index 372471c..524b581 100644 --- a/pkg/zabbix/methods.go +++ b/pkg/zabbix/methods.go @@ -105,16 +105,24 @@ func (ds *Zabbix) GetItems( } if isRegex(itemTagFilter) { + ds.logger.Debug("Processing regex item tag filter", "filterPattern", itemTagFilter, "hostCount", len(hostids)) tags, err := ds.GetItemTags(ctx, groupFilter, hostFilter, itemTagFilter) if err != nil { return nil, err } + ds.logger.Debug("GetItemTags completed", "matchedTagCount", len(tags), "filterPattern", itemTagFilter) + // If regex filter doesn't match any tags, return empty items list + // to prevent silently removing the filter and returning all items + if len(tags) == 0 { + return []*Item{}, nil + } var tagStrs []string for _, t := range tags { tagStrs = append(tagStrs, itemTagToString(t)) } itemTagFilter = strings.Join(tagStrs, ",") } + ds.logger.Debug("Fetching items with filters", "hostCount", len(hostids), "itemType", itemType, "showDisabled", showDisabled, "hasItemTagFilter", len(itemTagFilter) > 0) allItems, err = ds.GetAllItems(ctx, hostids, nil, itemType, showDisabled, itemTagFilter) if err != nil { return nil, err diff --git a/pkg/zabbix/methods_test.go b/pkg/zabbix/methods_test.go index 3c1383b..7b49caa 100644 --- a/pkg/zabbix/methods_test.go +++ b/pkg/zabbix/methods_test.go @@ -122,6 +122,76 @@ func TestGetItems(t *testing.T) { } } +func TestGetItemsWithRegexItemTagFilterNoMatch(t *testing.T) { + // Test that when a regex itemTagFilter doesn't match any tags, + // GetItems returns an empty list instead of all items. + // + // This test verifies the fix for the bug where an empty tag filter result + // would cause itemTagFilter to become empty string, silently removing the filter. + // + // Bug scenario (without fix): + // 1. Regex itemTagFilter "/^NonExistentTag/" doesn't match any tags + // 2. GetItemTags returns empty list + // 3. itemTagFilter becomes "" (empty string) + // 4. GetAllItems is called with empty itemTagFilter → returns ALL items (no tag filtering) + // 5. filterItemsByQuery filters by itemFilter "/.*/" → returns all 2 items + // + // Fixed scenario (with fix): + // 1. Regex itemTagFilter "/^NonExistentTag/" doesn't match any tags + // 2. GetItemTags returns empty list + // 3. Early return with empty items list → returns 0 items + itemGetCalls := []map[string]interface{}{} + client := NewZabbixClientWithHandler(t, func(payload ApiRequestPayload) string { + if payload.Method == "item.get" { + // Track all item.get calls to verify behavior + paramsCopy := make(map[string]interface{}) + for k, v := range payload.Params { + paramsCopy[k] = v + } + itemGetCalls = append(itemGetCalls, paramsCopy) + + // Return items that would be returned if no tag filter is applied + return `{ + "result":[ + {"itemid":"100","name":"CPU usage","tags":[{"tag":"Env","value":"prod"}]}, + {"itemid":"200","name":"Memory usage","tags":[{"tag":"Application","value":"api"}]} + ] + }` + } + if payload.Method == "hostgroup.get" { + return `{"result":[{"groupid":"1","name":"Servers"}]}` + } + if payload.Method == "host.get" { + return `{"result":[{"hostid":"10","name":"web01"}]}` + } + if payload.Method == "apiinfo.version" { + return `{"result":"6.4.0"}` + } + return `{"result":null}` + }) + + // Use a regex itemTagFilter that won't match any tags + // Use itemFilter "/.*/" which matches all items, so we can detect if bug returns all items + items, err := client.GetItems(context.Background(), "Servers", "web01", "/^NonExistentTag/", "/.*/", "num", false) + require.NoError(t, err) + + // With fix: Should return empty list (0 items) because no tags matched + // Without fix: Would return 2 items because itemTagFilter becomes empty and GetAllItems returns all items + if len(items) != 0 { + t.Errorf("Expected empty list when regex itemTagFilter matches no tags, but got %d items. This indicates the bug: itemTagFilter was silently removed and all items were returned.", len(items)) + t.Logf("Item.get was called %d times", len(itemGetCalls)) + for i, call := range itemGetCalls { + tags, hasTags := call["tags"] + if hasTags { + t.Logf("Call %d: has tags filter: %v", i+1, tags) + } else { + t.Logf("Call %d: NO tags filter (this is the bug - should not call GetAllItems with empty filter)", i+1) + } + } + } + assert.Len(t, items, 0, "Expected empty list when regex itemTagFilter matches no tags. Without fix, this would return 2 items.") +} + func TestGetItemsBefore54(t *testing.T) { client := NewZabbixClientWithResponses(t, map[string]string{ "hostgroup.get": `{"result":[{"groupid":"1","name":"Servers"}]}`,