From 1e531b1e802ea03356983c32fef95216e259b86a Mon Sep 17 00:00:00 2001
From: Alex K <8418476+fearful-symmetry@users.noreply.github.com>
Date: Tue, 14 Jul 2020 10:53:45 -0700
Subject: [PATCH] [Bug] fix bug with empty filter values returning no results
 in system/service (#19812)

* fix bug with empty filter values returning no results

* add changelog entry
---
 CHANGELOG.next.asciidoc                       |  1 +
 metricbeat/module/system/service/dbus.go      | 40 ++++++++++++-------
 .../module/system/service/service_test.go     | 26 +++++++++++-
 3 files changed, 50 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index dd08552554e..49a3a79eece 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -208,6 +208,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Fix memory leak in tcp and unix input sources. {pull}19459[19459]
 - Add missing `default_field: false` to aws filesets fields.yml. {pull}19568[19568]
 - Fix tls mapping in suricata module {issue}19492[19492] {pull}19494[19494]
+- Fix bug with empty filter values in system/service {pull}19812[19812]
 
 *Heartbeat*
 
diff --git a/metricbeat/module/system/service/dbus.go b/metricbeat/module/system/service/dbus.go
index 62112922136..c3c5bf1dc41 100644
--- a/metricbeat/module/system/service/dbus.go
+++ b/metricbeat/module/system/service/dbus.go
@@ -139,33 +139,43 @@ func listUnitsWrapper(conn *dbus.Conn, states, patterns []string) ([]dbus.UnitSt
 	if err != nil {
 		return nil, errors.Wrap(err, "ListUnits error")
 	}
-	if len(patterns) > 0 {
-		units, err = matchUnitPatterns(patterns, units)
-		if err != nil {
-			return nil, errors.Wrap(err, "error matching unit patterns")
-		}
+
+	units, err = matchUnitPatterns(patterns, units)
+	if err != nil {
+		return nil, errors.Wrap(err, "error matching unit patterns")
 	}
 
-	if len(states) > 0 {
-		var finalUnits []dbus.UnitStatus
-		for _, unit := range units {
-			for _, state := range states {
-				if unit.LoadState == state || unit.ActiveState == state || unit.SubState == state {
-					finalUnits = append(finalUnits, unit)
-					break
-				}
+	finalUnits := matchUnitState(states, units)
+
+	return finalUnits, nil
+}
+
+// matchUnitState returns a list of units that match the pattern list
+// This checks the LoadState, ActiveState, and SubState for a matching status string
+func matchUnitState(states []string, units []dbus.UnitStatus) []dbus.UnitStatus {
+	if len(states) == 0 {
+		return units
+	}
+	var finalUnits []dbus.UnitStatus
+	for _, unit := range units {
+		for _, state := range states {
+			if unit.LoadState == state || unit.ActiveState == state || unit.SubState == state {
+				finalUnits = append(finalUnits, unit)
+				break
 			}
 		}
-		return finalUnits, nil
 	}
+	return finalUnits
 
-	return units, nil
 }
 
 // matchUnitPatterns returns a list of units that match the pattern list.
 // This algo, including filepath.Match, is designed to (somewhat) emulate the behavior of ListUnitsByPatterns, which uses `fnmatch`.
 func matchUnitPatterns(patterns []string, units []dbus.UnitStatus) ([]dbus.UnitStatus, error) {
 	var matchUnits []dbus.UnitStatus
+	if len(patterns) == 0 {
+		return units, nil
+	}
 	for _, unit := range units {
 		for _, pattern := range patterns {
 			match, err := filepath.Match(pattern, unit.Name)
diff --git a/metricbeat/module/system/service/service_test.go b/metricbeat/module/system/service/service_test.go
index 10549f81c74..87581ff45f8 100644
--- a/metricbeat/module/system/service/service_test.go
+++ b/metricbeat/module/system/service/service_test.go
@@ -31,10 +31,12 @@ import (
 
 var exampleUnits = []dbus.UnitStatus{
 	dbus.UnitStatus{
-		Name: "sshd.service",
+		Name:      "sshd.service",
+		LoadState: "active",
 	},
 	dbus.UnitStatus{
-		Name: "metricbeat.service",
+		Name:      "metricbeat.service",
+		LoadState: "active",
 	},
 	dbus.UnitStatus{
 		Name: "filebeat.service",
@@ -104,3 +106,23 @@ func TestFilterMatches(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Len(t, shouldMatch, 1)
 }
+
+func TestNoFilter(t *testing.T) {
+	shouldReturnResults, err := matchUnitPatterns([]string{}, exampleUnits)
+	assert.NoError(t, err)
+	assert.Len(t, shouldReturnResults, 3)
+}
+
+func TestUnitStateFilter(t *testing.T) {
+	stateFilter := []string{
+		"active",
+	}
+	shouldReturnResults := matchUnitState(stateFilter, exampleUnits)
+	assert.Len(t, shouldReturnResults, 2)
+
+}
+
+func TestUnitStateNoFilter(t *testing.T) {
+	shouldReturnResults := matchUnitState([]string{}, exampleUnits)
+	assert.Len(t, shouldReturnResults, 3)
+}