[meraki] improve defensive checks to prevent panics (#47950) (#47952)

* check if device exists in the map

* add fragment

* add more safety checks

* Fix more potential nil panic(s)

* Fix more potential nil panic(s)

* update fragment

---------


(cherry picked from commit 1c8215c)

Co-authored-by: Gabriel Pop <94497545+gpop63@users.noreply.github.com>
Co-authored-by: subham sarkar <subham.sarkar@elastic.co>

Author: 3 people

changelog/fragments/1765018492-improve-meraki-defensive-checks.yaml

@@ -0,0 +1,45 @@
+# REQUIRED
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# REQUIRED for all kinds
+# Change summary; a 80ish characters long description of the change.
+summary: Improve defensive checks to prevent panics in meraki module
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# description:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# impact:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# action:
+
+# REQUIRED for all kinds
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: metricbeat
+
+# AUTOMATED
+# OPTIONAL to manually add other PR URLs
+# PR URL: A link the PR that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+# pr: https://github.com/owner/repo/1234
+
+# AUTOMATED
+# OPTIONAL to manually add other issue URLs
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+# issue: https://github.com/owner/repo/1234

x-pack/metricbeat/module/meraki/device_health/device_health_test.go

@@ -42,16 +42,19 @@ func TestGetDeviceChannelUtilization(t *testing.T) {
 				},
 			},
 			validate: func(t *testing.T, devices map[Serial]*Device) {
-				assert.NotNil(t, devices["ABC123"].bandUtilization)
+				device, ok := devices["ABC123"]
+				require.True(t, ok, "device ABC123 should exist in the map")
+				require.NotNil(t, device, "device ABC123 should not be nil")
+				assert.NotNil(t, device.bandUtilization)
 
-				band1, ok := devices["ABC123"].bandUtilization["2.4"]
+				band1, ok := device.bandUtilization["2.4"]
 				assert.NotNil(t, band1)
 				assert.True(t, ok)
 				assert.Equal(t, 45.0, *band1.Wifi.Percentage)
 				assert.Equal(t, 10.0, *band1.NonWifi.Percentage)
 				assert.Equal(t, 55.0, *band1.Total.Percentage)
 
-				band2, ok := devices["ABC123"].bandUtilization["5"]
+				band2, ok := device.bandUtilization["5"]
 				assert.NotNil(t, band2)
 				assert.True(t, ok)
 				assert.Equal(t, 10.0, *band2.Wifi.Percentage)

x-pack/metricbeat/module/meraki/device_health/devices.go

@@ -336,11 +336,20 @@ func reportDeviceMetrics(reporter mb.ReporterV2, organizationID string, devices
 
 		if device.bandUtilization != nil {
 			for band, v := range device.bandUtilization {
+				if v == nil {
+					continue
+				}
 				// Avoid nested object mappings
 				metricBand := strings.ReplaceAll(band, ".", "_")
-				metric[fmt.Sprintf("device.channel_utilization.%s.utilization_80211", metricBand)] = v.Wifi.Percentage
-				metric[fmt.Sprintf("device.channel_utilization.%s.utilization_non_80211", metricBand)] = v.NonWifi.Percentage
-				metric[fmt.Sprintf("device.channel_utilization.%s.utilization_total", metricBand)] = v.Total.Percentage
+				if v.Wifi != nil {
+					metric[fmt.Sprintf("device.channel_utilization.%s.utilization_80211", metricBand)] = v.Wifi.Percentage
+				}
+				if v.NonWifi != nil {
+					metric[fmt.Sprintf("device.channel_utilization.%s.utilization_non_80211", metricBand)] = v.NonWifi.Percentage
+				}
+				if v.Total != nil {
+					metric[fmt.Sprintf("device.channel_utilization.%s.utilization_total", metricBand)] = v.Total.Percentage
+				}
 			}
 		}
 

x-pack/metricbeat/module/meraki/device_health/switchports.go

@@ -76,7 +76,9 @@ func getDeviceSwitchports(client *sdk.Client, organizationID string, devices map
 				}
 			}
 
-			devices[Serial(device.Serial)].switchports = switchports
+			if d, ok := devices[Serial(device.Serial)]; ok && d != nil {
+				d.switchports = switchports
+			}
 		}
 
 		return nil

x-pack/metricbeat/module/meraki/device_health/uplinks.go

@@ -72,8 +72,12 @@ func getDeviceUplinks(client *sdk.Client, organizationID string, devices map[Ser
 
 		for _, device := range *applicanceUplinks {
 			deviceObj, ok := devices[Serial(device.Serial)]
-			if device.HighAvailability != nil && ok && deviceObj != nil {
-				devices[Serial(device.Serial)].haStatus = device.HighAvailability
+			if !ok || deviceObj == nil {
+				continue
+			}
+
+			if device.HighAvailability != nil {
+				deviceObj.haStatus = device.HighAvailability
 			}
 
 			if device.Uplinks != nil {
@@ -96,9 +100,7 @@ func getDeviceUplinks(client *sdk.Client, organizationID string, devices map[Ser
 					uplinks = append(uplinks, uplink)
 				}
 
-				if ok && deviceObj != nil {
-					devices[Serial(device.Serial)].uplinks = uplinks
-				}
+				deviceObj.uplinks = uplinks
 			}
 		}
 
@@ -161,9 +163,8 @@ func getDeviceUplinks(client *sdk.Client, organizationID string, devices map[Ser
 				uplinks = append(uplinks, uplink)
 			}
 
-			deviceObj, ok := devices[Serial(device.Serial)]
-			if ok && deviceObj != nil {
-				devices[Serial(device.Serial)].uplinks = uplinks
+			if deviceObj, ok := devices[Serial(device.Serial)]; ok && deviceObj != nil {
+				deviceObj.uplinks = uplinks
 			}
 		}
 
@@ -192,7 +193,7 @@ func reportUplinkMetrics(reporter mb.ReporterV2, organizationID string, devices
 			if uplink == nil {
 				continue
 			}
-			if uplink.lossAndLatency != nil {
+			if uplink.lossAndLatency != nil && uplink.lossAndLatency.TimeSeries != nil {
 				// each loss and latency metric can have multiple values per collection.
 				// we report each value as it's own (smaller) metric event, containing
 				// the identifying device/uplink fields.