From 879559dc2d7a25e505bfe86c95b111d1b3aa936c Mon Sep 17 00:00:00 2001
From: Alex Kristiansen <alex.kristiansen@elastic.co>
Date: Thu, 9 Jul 2020 12:25:24 -0700
Subject: [PATCH 1/2] fix bug with empty filter values returning no results

---
 metricbeat/module/system/service/dbus.go      | 40 ++++++++++++-------
 .../module/system/service/service_test.go     | 26 +++++++++++-
 2 files changed, 49 insertions(+), 17 deletions(-)

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)
+}

From 4a15c8e813387d31c3da776cd3aaae47b8c4c51b Mon Sep 17 00:00:00 2001
From: Alex Kristiansen <alex.kristiansen@elastic.co>
Date: Tue, 14 Jul 2020 07:20:35 -0700
Subject: [PATCH 2/2] add changelog entry

---
 CHANGELOG.next.asciidoc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
index 21c144a0655..406c6885657 100644
--- a/CHANGELOG.next.asciidoc
+++ b/CHANGELOG.next.asciidoc
@@ -204,6 +204,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*