From 0ea50fa3b553a32a2266e44b025198ff87c15e17 Mon Sep 17 00:00:00 2001 From: Tomas Barton Date: Tue, 7 Feb 2023 16:55:28 +0100 Subject: [PATCH] fix(inputs.conntrack): Resolve segfault when setting collect field (#12603) --- plugins/inputs/conntrack/conntrack.go | 34 ++++--- plugins/inputs/conntrack/conntrack_test.go | 108 +++++++++++++++++++-- 2 files changed, 118 insertions(+), 24 deletions(-) diff --git a/plugins/inputs/conntrack/conntrack.go b/plugins/inputs/conntrack/conntrack.go index 97ae9b1f0..0e08d269b 100644 --- a/plugins/inputs/conntrack/conntrack.go +++ b/plugins/inputs/conntrack/conntrack.go @@ -12,6 +12,7 @@ import ( "strings" "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/internal/choice" "github.com/influxdata/telegraf/plugins/inputs" "github.com/influxdata/telegraf/plugins/inputs/system" ) @@ -57,9 +58,17 @@ func (*Conntrack) SampleConfig() string { return sampleConfig } -func (c *Conntrack) Gather(acc telegraf.Accumulator) error { +func (c *Conntrack) Init() error { c.setDefaults() + if err := choice.CheckSlice(c.Collect, []string{"all", "percpu"}); err != nil { + return fmt.Errorf("config option 'collect': %w", err) + } + + return nil +} + +func (c *Conntrack) Gather(acc telegraf.Accumulator) error { var metricKey string fields := make(map[string]interface{}) @@ -93,19 +102,8 @@ func (c *Conntrack) Gather(acc telegraf.Accumulator) error { } } - var all bool - var perCPU bool - - for _, collect := range c.Collect { - if collect == "all" { - all = true - } - if collect == "percpu" { - perCPU = true - } - } - - if all || perCPU { + for _, metric := range c.Collect { + perCPU := metric == "percpu" stats, err := c.ps.NetConntrack(perCPU) if err != nil { acc.AddError(fmt.Errorf("failed to retrieve conntrack statistics: %v", err)) @@ -115,8 +113,8 @@ func (c *Conntrack) Gather(acc telegraf.Accumulator) error { acc.AddError(fmt.Errorf("conntrack input failed to collect stats")) } + cpuTag := "all" for i, sts := range stats { - cpuTag := "all" if perCPU { cpuTag = fmt.Sprintf("cpu%d", i) } @@ -157,5 +155,9 @@ func (c *Conntrack) Gather(acc telegraf.Accumulator) error { } func init() { - inputs.Add(inputName, func() telegraf.Input { return &Conntrack{} }) + inputs.Add(inputName, func() telegraf.Input { + return &Conntrack{ + ps: system.NewSystemPS(), + } + }) } diff --git a/plugins/inputs/conntrack/conntrack_test.go b/plugins/inputs/conntrack/conntrack_test.go index 265867e2c..6ecc80ba3 100644 --- a/plugins/inputs/conntrack/conntrack_test.go +++ b/plugins/inputs/conntrack/conntrack_test.go @@ -27,6 +27,7 @@ func TestNoFilesFound(t *testing.T) { dfltFiles = []string{"baz.txt"} dfltDirs = []string{"./foo/bar"} c := &Conntrack{} + require.NoError(t, c.Init()) acc := &testutil.Accumulator{} err := c.Gather(acc) @@ -51,6 +52,7 @@ func TestDefaultsUsed(t *testing.T) { count := 1234321 require.NoError(t, os.WriteFile(tmpFile.Name(), []byte(strconv.Itoa(count)), 0660)) c := &Conntrack{} + require.NoError(t, c.Init()) acc := &testutil.Accumulator{} require.NoError(t, c.Gather(acc)) @@ -81,6 +83,7 @@ func TestConfigsUsed(t *testing.T) { require.NoError(t, os.WriteFile(cntFile.Name(), []byte(strconv.Itoa(count)), 0660)) require.NoError(t, os.WriteFile(maxFile.Name(), []byte(strconv.Itoa(max)), 0660)) c := &Conntrack{} + require.NoError(t, c.Init()) acc := &testutil.Accumulator{} require.NoError(t, c.Gather(acc)) @@ -123,9 +126,10 @@ func TestCollectStats(t *testing.T) { mps.On("NetConntrack", false).Return([]net.ConntrackStat{sts}, nil) cs := &Conntrack{ - ps: &mps, + ps: &mps, + Collect: []string{"all"}, } - cs.Collect = []string{"all"} + require.NoError(t, cs.Init()) err := cs.Gather(&acc) if err != nil && strings.Contains(err.Error(), "Is the conntrack kernel module loaded?") { @@ -211,10 +215,35 @@ func TestCollectStatsPerCpu(t *testing.T) { mps.On("NetConntrack", true).Return(sts, nil) - cs := &Conntrack{ - ps: &mps, + allSts := []net.ConntrackStat{ + { + Entries: 129, + Searched: 20, + Found: 2, + New: 10, + Invalid: 86, + Ignore: 26, + Delete: 6, + DeleteList: 10, + Insert: 18, + InsertFailed: 40, + Drop: 98, + EarlyDrop: 17, + IcmpError: 42, + ExpectNew: 24, + ExpectCreate: 88, + ExpectDelete: 106, + SearchRestart: 62, + }, } - cs.Collect = []string{"all", "percpu"} + + mps.On("NetConntrack", false).Return(allSts, nil) + + cs := &Conntrack{ + ps: &mps, + Collect: []string{"all", "percpu"}, + } + require.NoError(t, cs.Init()) err := cs.Gather(&acc) if err != nil && strings.Contains(err.Error(), "Is the conntrack kernel module loaded?") { @@ -243,13 +272,76 @@ func TestCollectStatsPerCpu(t *testing.T) { "search_restart": uint32(31), } - acc.AssertContainsFields(t, inputName, expectedFields) acc.AssertContainsTaggedFields(t, inputName, expectedFields, map[string]string{ "cpu": "cpu0", }) - //TODO: check cpu1 fields + //cpu1 + expectedFields1 := map[string]interface{}{ + "entries": uint32(79), + "searched": uint32(10), + "found": uint32(1), + "new": uint32(5), + "invalid": uint32(43), + "ignore": uint32(13), + "delete": uint32(3), + "delete_list": uint32(5), + "insert": uint32(9), + "insert_failed": uint32(10), + "drop": uint32(49), + "early_drop": uint32(7), + "icmp_error": uint32(21), + "expect_new": uint32(12), + "expect_create": uint32(44), + "expect_delete": uint32(53), + "search_restart": uint32(31), + } - require.Equal(t, 36, acc.NFields()) + acc.AssertContainsTaggedFields(t, inputName, expectedFields1, + map[string]string{ + "cpu": "cpu1", + }) + + allFields := map[string]interface{}{ + "entries": uint32(129), + "searched": uint32(20), + "found": uint32(2), + "new": uint32(10), + "invalid": uint32(86), + "ignore": uint32(26), + "delete": uint32(6), + "delete_list": uint32(10), + "insert": uint32(18), + "insert_failed": uint32(40), + "drop": uint32(98), + "early_drop": uint32(17), + "icmp_error": uint32(42), + "expect_new": uint32(24), + "expect_create": uint32(88), + "expect_delete": uint32(106), + "search_restart": uint32(62), + } + + acc.AssertContainsTaggedFields(t, inputName, allFields, + map[string]string{ + "cpu": "all", + }) + + require.Equal(t, 53, acc.NFields()) +} + +func TestCollectPsSystemInit(t *testing.T) { + var acc testutil.Accumulator + cs := &Conntrack{ + ps: system.NewSystemPS(), + Collect: []string{"all"}, + } + require.NoError(t, cs.Init()) + err := cs.Gather(&acc) + if err != nil && strings.Contains(err.Error(), "Is the conntrack kernel module loaded?") { + t.Skip("Conntrack kernel module not loaded.") + } + //make sure Conntrack.ps gets initialized without mocking + require.NoError(t, err) }