add_process_metadata: Support different integer types for pid field (#…

…26829) Fixes a bug with the add_process_metadata processor where sometimes the lookup PID specified in match_pids cannot be parsed even if it's a valid integer. This is caused by the processor expecting the field to be of int type, while depending on how the field is populated, it can be other types, usually an int64 if the source is a json document. Fixes #26830 (cherry picked from commit 7be6e5e)
elastic · Jul 12, 2021 · d4b9990 · d4b9990
1 parent 9c7c360
commit d4b9990
Show file tree

Hide file tree

Showing 3 changed files with 212 additions and 12 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -110,6 +110,32 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - Add service resource in k8s cluster role. {pull}20546[20546]
 - [Metricbeat][Kubernetes] Change cluster_ip field from ip to keyword. {pull}20571[20571]
 - The `o365input` and `o365` module now recover from an authentication problem or other fatal errors, instead of terminating. {pull}21258[21258]
+- Rename cloud.provider `az` value to `azure` inside the add_cloud_metadata processor. {pull}20689[20689]
+- Add missing country_name geo field in `add_host_metadata` and `add_observer_metadata` processors. {issue}20796[20796] {pull}20811[20811]
+- [Autodiscover] Handle input-not-finished errors in config reload. {pull}20915[20915]
+- Explicitly detect missing variables in autodiscover configuration, log them at the debug level. {issue}20568[20568] {pull}20898[20898]
+- Fix `libbeat.output.write.bytes` and `libbeat.output.read.bytes` metrics of the Elasticsearch output. {issue}20752[20752] {pull}21197[21197]
+- Orderly close processors when processing pipelines are not needed anymore to release their resources. {pull}16349[16349]
+- Fix memory leak and events duplication in docker autodiscover and add_docker_metadata. {pull}21851[21851]
+- Fixed documentation for commands in beats dev guide {pull}22194[22194]
+- Fix parsing of expired licences. {issue}21112[21112] {pull}22180[22180]
+- Fix duplicated pod events in kubernetes autodiscover for pods with init or ephemeral containers. {pull}22438[22438]
+- Fix FileVersion contained in Windows exe files. {pull}22581[22581]
+- Fix index template loading when the new index format is selected. {issue}22482[22482] {pull}22682[22682]
+- Periodic metrics in logs will now report `libbeat.output.events.active` and `beat.memstats.rss`
+  as gauges (rather than counters). {pull}22877[22877]
+- Use PROGRAMDATA environment variable instead of C:\ProgramData for windows install service {pull}22874[22874]
+- Fix reporting of cgroup metrics when running under Docker {pull}22879[22879]
+- Fix typo in config docs {pull}23185[23185]
+- Add FAQ entry for madvdontneed variable {pull}23429[23429]
+- Fix panic due to unhandled DeletedFinalStateUnknown in k8s OnDelete {pull}23419[23419]
+- Fix error loop with runaway CPU use when the Kafka output encounters some connection errors {pull}23484[23484]
+- Fix issue discovering docker containers and metadata after reconnections {pull}24318[24318]
+- Fix ILM alias creation when write alias exists and initial index does not exist {pull}26143[26143]
+- Omit full index template from errors that occur while loading the template. {pull}25743[25743]
+- In the script processor, the `decode_xml` and `decode_xml_wineventlog` processors are now available as `DecodeXML` and `DecodeXMLWineventlog` respectively.
+- Fix encoding errors when using the disk queue on nested data with multi-byte characters {pull}26484[26484]
+- Fix `add_process_metadata` processor complaining about valid pid fields not being valid integers. {pull}26829[26829] {issue}26830[26830]
 
 *Auditbeat*
 

diff --git a/libbeat/processors/add_process_metadata/add_process_metadata.go b/libbeat/processors/add_process_metadata/add_process_metadata.go
@@ -19,6 +19,7 @@ package add_process_metadata
 
 import (
 	"fmt"
+	"reflect"
 	"strconv"
 	"time"
 
@@ -182,23 +183,40 @@ func (p *addProcessMetadata) Run(event *beat.Event) (*beat.Event, error) {
 	return event, ErrNoMatch
 }
 
-func (p *addProcessMetadata) enrich(event common.MapStr, pidField string) (result common.MapStr, err error) {
-	pidIf, err := event.GetValue(pidField)
-	if err != nil {
-		return nil, err
-	}
-
-	var pid int
-	switch v := pidIf.(type) {
+func pidToInt(value interface{}) (pid int, err error) {
+	switch v := value.(type) {
 	case string:
 		pid, err = strconv.Atoi(v)
 		if err != nil {
-			return nil, errors.Wrapf(err, "cannot convert string field '%s' to an integer", pidField)
+			return 0, errors.Wrap(err, "error converting string to integer")
 		}
 	case int:
 		pid = v
+	case int8, int16, int32, int64:
+		pid64 := reflect.ValueOf(v).Int()
+		if pid = int(pid64); int64(pid) != pid64 {
+			return 0, errors.Errorf("integer out of range: %d", pid64)
+		}
+	case uint, uintptr, uint8, uint16, uint32, uint64:
+		pidu64 := reflect.ValueOf(v).Uint()
+		if pid = int(pidu64); pid < 0 || uint64(pid) != pidu64 {
+			return 0, errors.Errorf("integer out of range: %d", pidu64)
+		}
 	default:
-		return nil, errors.Errorf("cannot parse field '%s' (not an integer or string)", pidField)
+		return 0, errors.Errorf("not an integer or string, but %T", v)
+	}
+	return pid, nil
+}
+
+func (p *addProcessMetadata) enrich(event common.MapStr, pidField string) (result common.MapStr, err error) {
+	pidIf, err := event.GetValue(pidField)
+	if err != nil {
+		return nil, err
+	}
+
+	pid, err := pidToInt(pidIf)
+	if err != nil {
+		return nil, errors.Wrapf(err, "cannot parse pid field '%s'", pidField)
 	}
 
 	var meta common.MapStr

diff --git a/libbeat/processors/add_process_metadata/add_process_metadata_test.go b/libbeat/processors/add_process_metadata/add_process_metadata_test.go
@@ -18,9 +18,11 @@
 package add_process_metadata
 
 import (
+	"math"
 	"os"
 	"testing"
 	"time"
+	"unsafe"
 
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
@@ -403,7 +405,7 @@ func TestAddProcessMetadata(t *testing.T) {
 			expected: common.MapStr{
 				"ppid": "a",
 			},
-			err: errors.New("error applying add_process_metadata processor: cannot convert string field 'ppid' to an integer: strconv.Atoi: parsing \"a\": invalid syntax"),
+			err: errors.New("error applying add_process_metadata processor: cannot parse pid field 'ppid': error converting string to integer: strconv.Atoi: parsing \"a\": invalid syntax"),
 		},
 		{
 			description: "bad PID field type",
@@ -416,7 +418,7 @@ func TestAddProcessMetadata(t *testing.T) {
 			expected: common.MapStr{
 				"ppid": false,
 			},
-			err: errors.New("error applying add_process_metadata processor: cannot parse field 'ppid' (not an integer or string)"),
+			err: errors.New("error applying add_process_metadata processor: cannot parse pid field 'ppid': not an integer or string, but bool"),
 		},
 		{
 			description: "process not found",
@@ -824,3 +826,157 @@ func TestBadProcess(t *testing.T) {
 	assert.NotNil(t, result.Fields)
 	assert.Equal(t, ev.Fields, result.Fields)
 }
+
+func TestPIDToInt(t *testing.T) {
+	const intIs64bit = unsafe.Sizeof(int(0)) == unsafe.Sizeof(int64(0))
+	for _, test := range []struct {
+		name string
+		pid  interface{}
+		fail bool
+	}{
+		{
+			name: "numeric string",
+			pid:  "1234",
+		},
+		{
+			name: "numeric string ignore octal",
+			pid:  "008",
+		},
+		{
+			name: "numeric string invalid hex",
+			pid:  "0x10",
+			fail: true,
+		},
+		{
+			name: "non-numeric string",
+			pid:  "abcd",
+			fail: true,
+		},
+		{
+			name: "int",
+			pid:  0,
+		},
+		{
+			name: "int min",
+			pid:  math.MaxInt32,
+		},
+		{
+			name: "int max",
+			pid:  math.MaxInt32,
+		},
+		{
+			name: "uint min",
+			pid:  uint(0),
+		},
+		{
+			name: "uint max",
+			pid:  uint(math.MaxUint32),
+			fail: !intIs64bit,
+		},
+		{
+			name: "int8",
+			pid:  int8(0),
+		},
+		{
+			name: "int8 min",
+			pid:  int8(math.MinInt8),
+		},
+		{
+			name: "int8 max",
+			pid:  int8(math.MaxInt8),
+		},
+		{
+			name: "uint8 min",
+			pid:  uint8(0),
+		},
+		{
+			name: "uint8 max",
+			pid:  uint8(math.MaxUint8),
+		},
+		{
+			name: "int16",
+			pid:  int16(0),
+		},
+		{
+			name: "int16 min",
+			pid:  int16(math.MinInt16),
+		},
+		{
+			name: "int16 max",
+			pid:  int16(math.MaxInt16),
+		},
+		{
+			name: "uint16 min",
+			pid:  uint16(0),
+		},
+		{
+			name: "uint16 max",
+			pid:  uint16(math.MaxUint16),
+		},
+		{
+			name: "int32",
+			pid:  int32(0),
+		},
+		{
+			name: "int32 min",
+			pid:  int32(math.MinInt32),
+		},
+		{
+			name: "int32 max",
+			pid:  int32(math.MaxInt32),
+		},
+		{
+			name: "uint32 min",
+			pid:  uint32(0),
+		},
+		{
+			name: "uint32 max",
+			pid:  uint32(math.MaxUint32),
+			fail: !intIs64bit,
+		},
+		{
+			name: "int64",
+			pid:  int64(0),
+			fail: false,
+		},
+		{
+			name: "int64 min",
+			pid:  int64(math.MinInt64),
+			fail: !intIs64bit,
+		},
+		{
+			name: "int64 max",
+			pid:  int64(math.MaxInt64),
+			fail: !intIs64bit,
+		},
+		{
+			name: "uint64 min",
+			pid:  uint64(0),
+			fail: false,
+		},
+		{
+			name: "uint64 max",
+			pid:  uint64(math.MaxUint64),
+			fail: true,
+		},
+		{
+			name: "uintptr",
+			pid:  uintptr(0),
+			fail: false,
+		},
+		{
+			name: "boolean",
+			pid:  false,
+			fail: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			_, err := pidToInt(test.pid)
+			if test.fail {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}