From 039baab7de0cab152c6e67e295c74b4ccfbd0ce1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paolo=20Chil=C3=A0?= Date: Fri, 6 Dec 2024 14:36:43 +0100 Subject: [PATCH] Add support for failureThreshold passed as float64 from policy (#6230) --- .../application/monitoring/v1_monitor.go | 18 +- .../application/monitoring/v1_monitor_test.go | 168 ++++++++++++++++++ 2 files changed, 184 insertions(+), 2 deletions(-) diff --git a/internal/pkg/agent/application/monitoring/v1_monitor.go b/internal/pkg/agent/application/monitoring/v1_monitor.go index ae51994988c..55e9a4dc3f9 100644 --- a/internal/pkg/agent/application/monitoring/v1_monitor.go +++ b/internal/pkg/agent/application/monitoring/v1_monitor.go @@ -7,6 +7,7 @@ package monitoring import ( "crypto/sha256" "fmt" + "math" "net/url" "os" "path/filepath" @@ -164,12 +165,25 @@ func (b *BeatsMonitor) MonitoringConfig( case uint: failureThreshold = &policyValue case int: + if policyValue < 0 { + return nil, fmt.Errorf("converting policy failure threshold int to uint, value must be non-negative: %v", policyValue) + } unsignedValue := uint(policyValue) failureThreshold = &unsignedValue + case float64: + if policyValue < 0 || policyValue > math.MaxUint { + return nil, fmt.Errorf("converting policy failure threshold float64 to uint, value out of range: %v", policyValue) + } + truncatedUnsignedValue := uint(policyValue) + failureThreshold = &truncatedUnsignedValue case string: - parsedPolicyValue, err := strconv.Atoi(policyValue) + parsedPolicyValue, err := strconv.ParseUint(policyValue, 10, 64) if err != nil { - return nil, fmt.Errorf("failed to convert policy failure threshold string to int: %w", err) + return nil, fmt.Errorf("converting policy failure threshold string to uint: %w", err) + } + if parsedPolicyValue > math.MaxUint { + // this is to catch possible overflow in 32-bit envs, should not happen that often + return nil, fmt.Errorf("converting policy failure threshold from string to uint, value out of range: %v", policyValue) } uintPolicyValue := uint(parsedPolicyValue) failureThreshold = &uintPolicyValue diff --git a/internal/pkg/agent/application/monitoring/v1_monitor_test.go b/internal/pkg/agent/application/monitoring/v1_monitor_test.go index 4cd3e61cf00..f00f92417ee 100644 --- a/internal/pkg/agent/application/monitoring/v1_monitor_test.go +++ b/internal/pkg/agent/application/monitoring/v1_monitor_test.go @@ -409,6 +409,34 @@ func TestMonitoringConfigMetricsFailureThreshold(t *testing.T) { }, expectedThreshold: sampleTenErrorsStreamThreshold, }, + { + name: "policy failure threshold float64", + monitoringCfg: &monitoringConfig{ + C: &monitoringcfg.MonitoringConfig{ + Enabled: true, + MonitorMetrics: true, + HTTP: &monitoringcfg.MonitoringHTTPConfig{ + Enabled: false, + }, + FailureThreshold: &sampleSevenErrorsStreamThreshold, + }, + }, + policy: map[string]any{ + "agent": map[string]any{ + "monitoring": map[string]any{ + "metrics": true, + "http": map[string]any{ + "enabled": false, + }, + failureThresholdKey: float64(10), + }, + }, + "outputs": map[string]any{ + "default": map[string]any{}, + }, + }, + expectedThreshold: sampleTenErrorsStreamThreshold, + }, } for _, tc := range tcs { @@ -462,6 +490,146 @@ func TestMonitoringConfigMetricsFailureThreshold(t *testing.T) { } } +func TestErrorMonitoringConfigMetricsFailureThreshold(t *testing.T) { + + agentInfo, err := info.NewAgentInfo(context.Background(), false) + require.NoError(t, err, "Error creating agent info") + + tcs := []struct { + name string + monitoringCfg *monitoringConfig + policy map[string]any + assertError assert.ErrorAssertionFunc + }{ + { + name: "invalid policy failure threshold float64", + monitoringCfg: &monitoringConfig{ + C: &monitoringcfg.MonitoringConfig{ + Enabled: true, + MonitorMetrics: true, + HTTP: &monitoringcfg.MonitoringHTTPConfig{ + Enabled: false, + }, + FailureThreshold: nil, + }, + }, + policy: map[string]any{ + "agent": map[string]any{ + "monitoring": map[string]any{ + "metrics": true, + "http": map[string]any{ + "enabled": false, + }, + failureThresholdKey: float64(-1), + }, + }, + "outputs": map[string]any{ + "default": map[string]any{}, + }, + }, + assertError: assert.Error, + }, + { + name: "invalid policy failure threshold string", + monitoringCfg: &monitoringConfig{ + C: &monitoringcfg.MonitoringConfig{ + Enabled: true, + MonitorMetrics: true, + HTTP: &monitoringcfg.MonitoringHTTPConfig{ + Enabled: false, + }, + FailureThreshold: nil, + }, + }, + policy: map[string]any{ + "agent": map[string]any{ + "monitoring": map[string]any{ + "metrics": true, + "http": map[string]any{ + "enabled": false, + }, + failureThresholdKey: "foobar", + }, + }, + "outputs": map[string]any{ + "default": map[string]any{}, + }, + }, + assertError: assert.Error, + }, + { + name: "invalid policy failure threshold negative number as string", + monitoringCfg: &monitoringConfig{ + C: &monitoringcfg.MonitoringConfig{ + Enabled: true, + MonitorMetrics: true, + HTTP: &monitoringcfg.MonitoringHTTPConfig{ + Enabled: false, + }, + FailureThreshold: nil, + }, + }, + policy: map[string]any{ + "agent": map[string]any{ + "monitoring": map[string]any{ + "metrics": true, + "http": map[string]any{ + "enabled": false, + }, + failureThresholdKey: "-12", + }, + }, + "outputs": map[string]any{ + "default": map[string]any{}, + }, + }, + assertError: assert.Error, + }, + { + name: "invalid policy failure threshold negative int", + monitoringCfg: &monitoringConfig{ + C: &monitoringcfg.MonitoringConfig{ + Enabled: true, + MonitorMetrics: true, + HTTP: &monitoringcfg.MonitoringHTTPConfig{ + Enabled: false, + }, + FailureThreshold: nil, + }, + }, + policy: map[string]any{ + "agent": map[string]any{ + "monitoring": map[string]any{ + "metrics": true, + "http": map[string]any{ + "enabled": false, + }, + failureThresholdKey: -12, + }, + }, + "outputs": map[string]any{ + "default": map[string]any{}, + }, + }, + assertError: assert.Error, + }, + } + + for _, tc := range tcs { + + t.Run(tc.name, func(t *testing.T) { + b := &BeatsMonitor{ + enabled: true, + config: tc.monitoringCfg, + operatingSystem: runtime.GOOS, + agentInfo: agentInfo, + } + _, err := b.MonitoringConfig(tc.policy, nil, map[string]string{"foobeat": "filebeat"}, map[string]uint64{}) // put a componentID/binary mapping to have something in the beats monitoring input + tc.assertError(t, err) + }) + } +} + func TestMonitoringConfigComponentFields(t *testing.T) { agentInfo, err := info.NewAgentInfo(context.Background(), false) require.NoError(t, err, "Error creating agent info")