From 0965859e2174f9f031715a05bddde20315fd32cf Mon Sep 17 00:00:00 2001 From: "Nathan A. Ferch" Date: Sun, 8 Oct 2017 20:07:47 +0000 Subject: [PATCH 1/6] Add input plugin for OpenBSD/FreeBSD pf --- plugins/inputs/all/all.go | 1 + plugins/inputs/pf/README.md | 65 ++++++++ plugins/inputs/pf/pf.go | 207 ++++++++++++++++++++++++ plugins/inputs/pf/pf_nocompile.go | 3 + plugins/inputs/pf/pf_test.go | 253 ++++++++++++++++++++++++++++++ 5 files changed, 529 insertions(+) create mode 100644 plugins/inputs/pf/README.md create mode 100644 plugins/inputs/pf/pf.go create mode 100644 plugins/inputs/pf/pf_nocompile.go create mode 100644 plugins/inputs/pf/pf_test.go diff --git a/plugins/inputs/all/all.go b/plugins/inputs/all/all.go index 421fd113c0031..81ee8b15d14e1 100644 --- a/plugins/inputs/all/all.go +++ b/plugins/inputs/all/all.go @@ -61,6 +61,7 @@ import ( _ "github.com/influxdata/telegraf/plugins/inputs/ntpq" _ "github.com/influxdata/telegraf/plugins/inputs/openldap" _ "github.com/influxdata/telegraf/plugins/inputs/passenger" + _ "github.com/influxdata/telegraf/plugins/inputs/pf" _ "github.com/influxdata/telegraf/plugins/inputs/phpfpm" _ "github.com/influxdata/telegraf/plugins/inputs/ping" _ "github.com/influxdata/telegraf/plugins/inputs/postgresql" diff --git a/plugins/inputs/pf/README.md b/plugins/inputs/pf/README.md new file mode 100644 index 0000000000000..691f7e9bc8ec2 --- /dev/null +++ b/plugins/inputs/pf/README.md @@ -0,0 +1,65 @@ +# PF Plugin + +The pf plugin gathers counters from the FreeBSD/OpenBSD pf firewall. + +The pfstat command requires read access to the device file /dev/pf. You have several options to grant telegraf to run pfctl: + +* Run telegraf as root. This is strongly discouraged. +* Change the ownership and permissions for /dev/pf such that the user telegraf runs at can read the /dev/pf device file. This is probably not that good of an idea either, +* Configure sudo to grant telegraf to run pfctl as root. This is the most restrictive option, but require sudo setup. + +### Using sudo + +You may edit your sudo configuration with the following: + +```sudo +telegraf ALL=(root) NOPASSWD: /sbin/pfctl -s info +``` + +### Configuration: + +```toml + # use sudo to run pfctl + use_sudo = false +``` + +### Measurements & Fields: + + +- pf + - entries (integer, count) + +### Example Output: + +``` +> pfctl -s info +Status: Enabled for 0 days 00:26:05 Debug: Urgent + +State Table Total Rate + current entries 2 + searches 11325 7.2/s + inserts 5 0.0/s + removals 3 0.0/s +Counters + match 11226 7.2/s + bad-offset 0 0.0/s + fragment 0 0.0/s + short 0 0.0/s + normalize 0 0.0/s + memory 0 0.0/s + bad-timestamp 0 0.0/s + congestion 0 0.0/s + ip-option 0 0.0/s + proto-cksum 0 0.0/s + state-mismatch 0 0.0/s + state-insert 0 0.0/s + state-limit 0 0.0/s + src-limit 0 0.0/s + synproxy 0 0.0/s +``` + +``` +> ./telegraf --config telegraf.conf --input-filter pf --test +* Plugin: inputs.pf, Collection 1 +> pf,host=columbia entries=2i 1507492593000000000 +``` diff --git a/plugins/inputs/pf/pf.go b/plugins/inputs/pf/pf.go new file mode 100644 index 0000000000000..8fcc3c3a81178 --- /dev/null +++ b/plugins/inputs/pf/pf.go @@ -0,0 +1,207 @@ +// +build freebsd + +package pf + +import ( + "errors" + "fmt" + "os/exec" + "reflect" + "regexp" + "strconv" + "strings" + + "github.com/influxdata/telegraf" + "github.com/influxdata/telegraf/plugins/inputs" +) + +const measurement = "pf" +const pfctlCommand = "pfctl" + +type PF struct { + UseSudo bool + StateTable StateTable + infoFunc func() (string, error) +} + +func (pf *PF) Description() string { + return "Gather counters from PF" +} + +func (pf *PF) SampleConfig() string { + return ` + ## PF require root access on most systems. + ## Setting 'use_sudo' to true will make use of sudo to run pfctl. + ## Users must configure sudo to allow telegraf user to run pfctl with no password. + ## pfctl can be restricted to only list command "pfctl -s info". + use_sudo = false +` +} + +// Gather is the entrypoint for the plugin. +func (pf *PF) Gather(acc telegraf.Accumulator) error { + o, err := pf.infoFunc() + if err != nil { + acc.AddError(err) + return nil + } + + if perr := pf.parsePfctlOutput(o, acc); perr != nil { + acc.AddError(perr) + } + return nil +} + +var errParseHeader = fmt.Errorf("Cannot find header in %s output", pfctlCommand) + +func errMissingData(tag string) error { + return fmt.Errorf("struct data for tag \"%s\" not found in %s output", tag, pfctlCommand) +} + +var stateTableHeaderRE = regexp.MustCompile("^State Table") +var anyTableHeaderRE = regexp.MustCompile("^[A-Z]") + +func (pf *PF) parsePfctlOutput(pfoutput string, acc telegraf.Accumulator) error { + lines := strings.Split(pfoutput, "\n") + stateTableFound := false + for i, line := range lines { + if stateTableHeaderRE.MatchString(line) { + endline := len(lines) + for j, el := range lines[i+1:] { + if anyTableHeaderRE.MatchString(el) { + endline = j + i + 1 + break + } + } + if perr := pf.parseStateTable(lines[i+1:endline], acc); perr != nil { + return perr + } + stateTableFound = true + } + } + if !stateTableFound { + return errParseHeader + } + return nil +} + +var stateTableRE = regexp.MustCompile(`^ (.*?)\s+(\d+)`) + +func (pf *PF) parseStateTable(lines []string, acc telegraf.Accumulator) error { + st := StateTable{} + tags, err := st.getTags() + if err != nil { + return fmt.Errorf("Can't retrieve struct tags: %v", err) + } + fMap := make(map[string]bool) + for i := 0; i < len(tags); i++ { + fMap[tags[i]] = false + } + + for _, v := range lines { + entries := stateTableRE.FindStringSubmatch(v) + if entries != nil { + fs, err := st.setByTag(entries[1], entries[2]) + if err != nil { + return errors.New("can't set statetable field from tag") + } + if fs { + fMap[entries[1]] = true + } + } + } + + for k, v := range fMap { + if !v { + return errMissingData(k) + } + } + + fields := make(map[string]interface{}) + fields["entries"] = st.CurrentEntries + fields["searches"] = st.Searches + fields["inserts"] = st.Inserts + fields["removals"] = st.Removals + acc.AddFields(measurement, fields, make(map[string]string)) + return nil +} + +func (pf *PF) callPfctl() (string, error) { + c, err := pf.buildPfctlCmd() + if err != nil { + return "", fmt.Errorf("Can't construct commandline: %s", err) + } + out, oerr := c.Output() + if oerr != nil { + return string(out), fmt.Errorf("error running %s: %s: %s", pfctlCommand, oerr, oerr.(*exec.ExitError).Stderr) + } + return string(out), oerr +} + +var execLookPath = exec.LookPath +var execCommand = exec.Command + +func (pf *PF) buildPfctlCmd() (*exec.Cmd, error) { + cmd, err := execLookPath(pfctlCommand) + if err != nil { + return nil, fmt.Errorf("can't locate %s: %v", pfctlCommand, err) + } + args := []string{"-s", "info"} + if pf.UseSudo { + args = append([]string{cmd}, args...) + cmd, err = execLookPath("sudo") + if err != nil { + return nil, fmt.Errorf("can't locate sudo: %v", err) + } + } + c := execCommand(cmd, args...) + return c, nil +} + +type StateTable struct { + CurrentEntries uint32 `pfctl:"current entries"` + Searches uint64 `pfctl:"searches"` + Inserts uint64 `pfctl:"inserts"` + Removals uint64 `pfctl:"removals"` +} + +func (pf *StateTable) getTags() ([]string, error) { + tags := []string{} + structVal := reflect.ValueOf(pf).Elem() + for i := 0; i < structVal.NumField(); i++ { + tags = append(tags, structVal.Type().Field(i).Tag.Get(pfctlCommand)) + } + return tags, nil +} + +// setByTag sets val for a struct field given the tag. returns false if tag not found. +func (pf *StateTable) setByTag(tag string, val string) (bool, error) { + structVal := reflect.ValueOf(pf).Elem() + + for i := 0; i < structVal.NumField(); i++ { + tagField := structVal.Type().Field(i).Tag.Get(pfctlCommand) + if tagField == tag { + valueField := structVal.Field(i) + switch valueField.Type().Kind() { + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + iVal, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return false, fmt.Errorf("Error parsing \"%s\" into uint: %s", val, err) + } + valueField.SetUint(iVal) + return true, nil + default: + return false, fmt.Errorf("unhandled struct type %s", valueField.Type()) + } + } + } + return false, nil +} + +func init() { + inputs.Add("pf", func() telegraf.Input { + pf := new(PF) + pf.infoFunc = pf.callPfctl + return pf + }) +} diff --git a/plugins/inputs/pf/pf_nocompile.go b/plugins/inputs/pf/pf_nocompile.go new file mode 100644 index 0000000000000..fbb9f3388461b --- /dev/null +++ b/plugins/inputs/pf/pf_nocompile.go @@ -0,0 +1,3 @@ +// +build !freebsd + +package pf diff --git a/plugins/inputs/pf/pf_test.go b/plugins/inputs/pf/pf_test.go new file mode 100644 index 0000000000000..9914e2254a19d --- /dev/null +++ b/plugins/inputs/pf/pf_test.go @@ -0,0 +1,253 @@ +// +build freebsd + +package pf + +import ( + "log" + "os/exec" + "reflect" + "strconv" + "testing" + + "github.com/influxdata/telegraf/testutil" +) + +type measurementResult struct { + tags map[string]string + fields map[string]interface{} +} + +func fakeexecFunc(i int, t *testing.T, desiredCmd string, desiredArgs ...string) func(string, ...string) *exec.Cmd { + return func(cmd string, args ...string) *exec.Cmd { + if cmd != desiredCmd || !reflect.DeepEqual(args, desiredArgs) { + t.Errorf("%d: not invoked correctly! %s - %#v vs %s - %#v", i, cmd, args, desiredCmd, desiredArgs) + } + return nil + } +} + +func TestPfctlInvocation(t *testing.T) { + type pfctlInvocationTestCase struct { + config PF + cmd string + args []string + } + + var testCases = []pfctlInvocationTestCase{ + // 0: no sudo + pfctlInvocationTestCase{ + config: PF{UseSudo: false}, + cmd: "fakepfctl", + args: []string{"-s", "info"}, + }, + // 1: with sudo + pfctlInvocationTestCase{ + config: PF{UseSudo: true}, + cmd: "fakesudo", + args: []string{"fakepfctl", "-s", "info"}, + }, + } + + for i, tt := range testCases { + execLookPath = func(cmd string) (string, error) { return "fake" + cmd, nil } + t.Run(strconv.Itoa(i), func(t *testing.T) { + log.Printf("running #%d\n", i) + execCommand = fakeexecFunc(i, t, tt.cmd, tt.args...) + _, err := tt.config.buildPfctlCmd() + if err != nil { + t.Fatalf("error when running buildPfctlCmd: %s", err) + } + }) + } +} + +func TestPfMeasurements(t *testing.T) { + type pfTestCase struct { + TestInput string + err error + measurements []measurementResult + } + + testCases := []pfTestCase{ + // 0: nil input should raise an error + pfTestCase{TestInput: "", err: errParseHeader}, + // 1: changes to pfctl output should raise an error + pfTestCase{TestInput: `Status: Enabled for 161 days 21:24:45 Debug: Urgent + +Interface Stats for re1 IPv4 IPv6 + Bytes In 2585823744614 1059233657221 + Bytes Out 1227266932673 3274698578875 + Packets In + Passed 2289953086 1945437219 + Blocked 392835739 48609 + Packets Out + Passed 1649146326 2605569054 + Blocked 107 0 + +State Table Total Rate + Current Entrys 649 + searches 18421725761 1317.0/s + inserts 156762508 11.2/s + removals 156761859 11.2/s +Counters + match 473002784 33.8/s + bad-offset 0 0.0/s + fragment 2729 0.0/s + short 107 0.0/s + normalize 1685 0.0/s + memory 101 0.0/s + bad-timestamp 0 0.0/s + congestion 0 0.0/s + ip-option 152301 0.0/s + proto-cksum 108 0.0/s + state-mismatch 24393 0.0/s + state-insert 92 0.0/s + state-limit 0 0.0/s + src-limit 0 0.0/s + synproxy 0 0.0/s +`, + err: errMissingData("current entries"), + }, + // 2: bad numbers should raise an error + pfTestCase{TestInput: `Status: Enabled for 0 days 00:26:05 Debug: Urgent + +State Table Total Rate + current entries -23 + searches 11325 7.2/s + inserts 5 0.0/s + removals 3 0.0/s +Counters + match 11226 7.2/s + bad-offset 0 0.0/s + fragment 0 0.0/s + short 0 0.0/s + normalize 0 0.0/s + memory 0 0.0/s + bad-timestamp 0 0.0/s + congestion 0 0.0/s + ip-option 0 0.0/s + proto-cksum 0 0.0/s + state-mismatch 0 0.0/s + state-insert 0 0.0/s + state-limit 0 0.0/s + src-limit 0 0.0/s + synproxy 0 0.0/s +`, + err: errMissingData("current entries"), + }, + pfTestCase{TestInput: `Status: Enabled for 0 days 00:26:05 Debug: Urgent + +State Table Total Rate + current entries 2 + searches 11325 7.2/s + inserts 5 0.0/s + removals 3 0.0/s +Counters + match 11226 7.2/s + bad-offset 0 0.0/s + fragment 0 0.0/s + short 0 0.0/s + normalize 0 0.0/s + memory 0 0.0/s + bad-timestamp 0 0.0/s + congestion 0 0.0/s + ip-option 0 0.0/s + proto-cksum 0 0.0/s + state-mismatch 0 0.0/s + state-insert 0 0.0/s + state-limit 0 0.0/s + src-limit 0 0.0/s + synproxy 0 0.0/s +`, + measurements: []measurementResult{ + measurementResult{ + fields: map[string]interface{}{ + "entries": uint32(2), + "searches": uint64(11325), + "inserts": uint64(5), + "removals": uint64(3)}, + tags: map[string]string{}, + }, + }, + }, + pfTestCase{TestInput: `Status: Enabled for 161 days 21:24:45 Debug: Urgent + +Interface Stats for re1 IPv4 IPv6 + Bytes In 2585823744614 1059233657221 + Bytes Out 1227266932673 3274698578875 + Packets In + Passed 2289953086 1945437219 + Blocked 392835739 48609 + Packets Out + Passed 1649146326 2605569054 + Blocked 107 0 + +State Table Total Rate + current entries 649 + searches 18421725761 1317.0/s + inserts 156762508 11.2/s + removals 156761859 11.2/s +Counters + match 473002784 33.8/s + bad-offset 0 0.0/s + fragment 2729 0.0/s + short 107 0.0/s + normalize 1685 0.0/s + memory 101 0.0/s + bad-timestamp 0 0.0/s + congestion 0 0.0/s + ip-option 152301 0.0/s + proto-cksum 108 0.0/s + state-mismatch 24393 0.0/s + state-insert 92 0.0/s + state-limit 0 0.0/s + src-limit 0 0.0/s + synproxy 0 0.0/s +`, + measurements: []measurementResult{ + measurementResult{ + fields: map[string]interface{}{ + "entries": uint32(649), + "searches": uint64(18421725761), + "inserts": uint64(156762508), + "removals": uint64(156761859)}, + tags: map[string]string{}, + }, + }, + }, + } + + for i, tt := range testCases { + t.Run(strconv.Itoa(i), func(t *testing.T) { + log.Printf("running #%d\n", i) + pf := &PF{ + infoFunc: func() (string, error) { + return tt.TestInput, nil + }, + } + acc := new(testutil.Accumulator) + err := acc.GatherError(pf.Gather) + if !reflect.DeepEqual(tt.err, err) { + t.Errorf("%d: expected error '%#v' got '%#v'", i, tt.err, err) + } + n := 0 + for j, v := range tt.measurements { + if len(acc.Metrics) < n+1 { + t.Errorf("%d: expected at least %d values got %d", i, n+1, len(acc.Metrics)) + break + } + m := acc.Metrics[n] + if !reflect.DeepEqual(m.Measurement, measurement) { + t.Errorf("%d %d: expected measurement '%#v' got '%#v'\n", i, j, measurement, m.Measurement) + } + if !reflect.DeepEqual(m.Tags, v.tags) { + t.Errorf("%d %d: expected tags\n%#v got\n%#v\n", i, j, v.tags, m.Tags) + } + if !reflect.DeepEqual(m.Fields, v.fields) { + t.Errorf("%d %d: expected fields\n%#v got\n%#v\n", i, j, v.fields, m.Fields) + } + n++ + } + }) + } +} From 1411eb882c435c53ca0e5679dcf2c776e1148c9f Mon Sep 17 00:00:00 2001 From: "Nathan A. Ferch" Date: Fri, 17 Nov 2017 19:43:04 +0000 Subject: [PATCH 2/6] update documentation --- plugins/inputs/pf/README.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plugins/inputs/pf/README.md b/plugins/inputs/pf/README.md index 691f7e9bc8ec2..ed058671ba558 100644 --- a/plugins/inputs/pf/README.md +++ b/plugins/inputs/pf/README.md @@ -1,12 +1,12 @@ # PF Plugin -The pf plugin gathers counters from the FreeBSD/OpenBSD pf firewall. +The pf plugin gathers information from the FreeBSD/OpenBSD pf firewall. Currently it can retrive information about the state table: the number of current entries in the table, and counters for the number of searches, inserts, and removals to the table. -The pfstat command requires read access to the device file /dev/pf. You have several options to grant telegraf to run pfctl: +The pf plugin retrives this information by invoking the `pfstat` command. The `pfstat` command requires read access to the device file `/dev/pf`. You have several options to permit telegraf to run `pfctl`: * Run telegraf as root. This is strongly discouraged. -* Change the ownership and permissions for /dev/pf such that the user telegraf runs at can read the /dev/pf device file. This is probably not that good of an idea either, -* Configure sudo to grant telegraf to run pfctl as root. This is the most restrictive option, but require sudo setup. +* Change the ownership and permissions for /dev/pf such that the user telegraf runs at can read the /dev/pf device file. This is probably not that good of an idea either. +* Configure sudo to grant telegraf to run `pfctl` as root. This is the most restrictive option, but require sudo setup. ### Using sudo @@ -28,6 +28,9 @@ telegraf ALL=(root) NOPASSWD: /sbin/pfctl -s info - pf - entries (integer, count) + - searches (integer, count) + - inserts (integer, count) + - removals (integer, count) ### Example Output: @@ -61,5 +64,5 @@ Counters ``` > ./telegraf --config telegraf.conf --input-filter pf --test * Plugin: inputs.pf, Collection 1 -> pf,host=columbia entries=2i 1507492593000000000 +> pf,host=columbia entries=3i,searches=2668i,inserts=12i,removals=9i 1510941775000000000 ``` From f3df1bec7d3d6e53879a1f1a98bb38c76e548799 Mon Sep 17 00:00:00 2001 From: "Nathan A. Ferch" Date: Fri, 17 Nov 2017 19:43:27 +0000 Subject: [PATCH 3/6] build for all os --- plugins/inputs/pf/pf.go | 2 -- plugins/inputs/pf/pf_nocompile.go | 3 --- plugins/inputs/pf/pf_test.go | 2 -- 3 files changed, 7 deletions(-) delete mode 100644 plugins/inputs/pf/pf_nocompile.go diff --git a/plugins/inputs/pf/pf.go b/plugins/inputs/pf/pf.go index 8fcc3c3a81178..11d51798b1104 100644 --- a/plugins/inputs/pf/pf.go +++ b/plugins/inputs/pf/pf.go @@ -1,5 +1,3 @@ -// +build freebsd - package pf import ( diff --git a/plugins/inputs/pf/pf_nocompile.go b/plugins/inputs/pf/pf_nocompile.go deleted file mode 100644 index fbb9f3388461b..0000000000000 --- a/plugins/inputs/pf/pf_nocompile.go +++ /dev/null @@ -1,3 +0,0 @@ -// +build !freebsd - -package pf diff --git a/plugins/inputs/pf/pf_test.go b/plugins/inputs/pf/pf_test.go index 9914e2254a19d..e528676fa4e7d 100644 --- a/plugins/inputs/pf/pf_test.go +++ b/plugins/inputs/pf/pf_test.go @@ -1,5 +1,3 @@ -// +build freebsd - package pf import ( From b5e72c928e14807e6f3b2154349f64ebdcd6b74e Mon Sep 17 00:00:00 2001 From: "Nathan A. Ferch" Date: Fri, 17 Nov 2017 19:50:35 +0000 Subject: [PATCH 4/6] only create pfctl cmd/args once --- plugins/inputs/pf/pf.go | 32 +++++++++++++++++++------------- plugins/inputs/pf/pf_test.go | 16 ++++------------ 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/plugins/inputs/pf/pf.go b/plugins/inputs/pf/pf.go index 11d51798b1104..c58478d807c57 100644 --- a/plugins/inputs/pf/pf.go +++ b/plugins/inputs/pf/pf.go @@ -17,9 +17,11 @@ const measurement = "pf" const pfctlCommand = "pfctl" type PF struct { - UseSudo bool - StateTable StateTable - infoFunc func() (string, error) + PfctlCommand string + PfctlArgs []string + UseSudo bool + StateTable []*Entry + infoFunc func() (string, error) } func (pf *PF) Description() string { @@ -38,6 +40,14 @@ func (pf *PF) SampleConfig() string { // Gather is the entrypoint for the plugin. func (pf *PF) Gather(acc telegraf.Accumulator) error { + if pf.PfctlCommand == "" { + var err error + if pf.PfctlCommand, pf.PfctlArgs, err = pf.buildPfctlCmd(); err != nil { + acc.AddError(fmt.Errorf("Can't construct pfctl commandline: %s", err)) + return nil + } + } + o, err := pf.infoFunc() if err != nil { acc.AddError(err) @@ -125,11 +135,8 @@ func (pf *PF) parseStateTable(lines []string, acc telegraf.Accumulator) error { } func (pf *PF) callPfctl() (string, error) { - c, err := pf.buildPfctlCmd() - if err != nil { - return "", fmt.Errorf("Can't construct commandline: %s", err) - } - out, oerr := c.Output() + cmd := execCommand(pf.PfctlCommand, pf.PfctlArgs...) + out, oerr := cmd.Output() if oerr != nil { return string(out), fmt.Errorf("error running %s: %s: %s", pfctlCommand, oerr, oerr.(*exec.ExitError).Stderr) } @@ -139,21 +146,20 @@ func (pf *PF) callPfctl() (string, error) { var execLookPath = exec.LookPath var execCommand = exec.Command -func (pf *PF) buildPfctlCmd() (*exec.Cmd, error) { +func (pf *PF) buildPfctlCmd() (string, []string, error) { cmd, err := execLookPath(pfctlCommand) if err != nil { - return nil, fmt.Errorf("can't locate %s: %v", pfctlCommand, err) + return "", nil, fmt.Errorf("can't locate %s: %v", pfctlCommand, err) } args := []string{"-s", "info"} if pf.UseSudo { args = append([]string{cmd}, args...) cmd, err = execLookPath("sudo") if err != nil { - return nil, fmt.Errorf("can't locate sudo: %v", err) + return "", nil, fmt.Errorf("can't locate sudo: %v", err) } } - c := execCommand(cmd, args...) - return c, nil + return cmd, args, nil } type StateTable struct { diff --git a/plugins/inputs/pf/pf_test.go b/plugins/inputs/pf/pf_test.go index e528676fa4e7d..52d22e5316523 100644 --- a/plugins/inputs/pf/pf_test.go +++ b/plugins/inputs/pf/pf_test.go @@ -2,7 +2,6 @@ package pf import ( "log" - "os/exec" "reflect" "strconv" "testing" @@ -15,15 +14,6 @@ type measurementResult struct { fields map[string]interface{} } -func fakeexecFunc(i int, t *testing.T, desiredCmd string, desiredArgs ...string) func(string, ...string) *exec.Cmd { - return func(cmd string, args ...string) *exec.Cmd { - if cmd != desiredCmd || !reflect.DeepEqual(args, desiredArgs) { - t.Errorf("%d: not invoked correctly! %s - %#v vs %s - %#v", i, cmd, args, desiredCmd, desiredArgs) - } - return nil - } -} - func TestPfctlInvocation(t *testing.T) { type pfctlInvocationTestCase struct { config PF @@ -50,11 +40,13 @@ func TestPfctlInvocation(t *testing.T) { execLookPath = func(cmd string) (string, error) { return "fake" + cmd, nil } t.Run(strconv.Itoa(i), func(t *testing.T) { log.Printf("running #%d\n", i) - execCommand = fakeexecFunc(i, t, tt.cmd, tt.args...) - _, err := tt.config.buildPfctlCmd() + cmd, args, err := tt.config.buildPfctlCmd() if err != nil { t.Fatalf("error when running buildPfctlCmd: %s", err) } + if tt.cmd != cmd || !reflect.DeepEqual(tt.args, args) { + t.Errorf("%d: expected %s - %#v got %s - %#v", tt.cmd, tt.args, cmd, args) + } }) } } From 21827769972761856797488eca8d7b07988a1b41 Mon Sep 17 00:00:00 2001 From: "Nathan A. Ferch" Date: Fri, 17 Nov 2017 19:51:29 +0000 Subject: [PATCH 5/6] guard against type assertion failure --- plugins/inputs/pf/pf.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/inputs/pf/pf.go b/plugins/inputs/pf/pf.go index c58478d807c57..b25593e6aeda7 100644 --- a/plugins/inputs/pf/pf.go +++ b/plugins/inputs/pf/pf.go @@ -138,7 +138,11 @@ func (pf *PF) callPfctl() (string, error) { cmd := execCommand(pf.PfctlCommand, pf.PfctlArgs...) out, oerr := cmd.Output() if oerr != nil { - return string(out), fmt.Errorf("error running %s: %s: %s", pfctlCommand, oerr, oerr.(*exec.ExitError).Stderr) + ee, ok := oerr.(*exec.ExitError) + if !ok { + return string(out), fmt.Errorf("error running %s: %s: (unable to get stderr)", pfctlCommand, oerr) + } + return string(out), fmt.Errorf("error running %s: %s: %s", pfctlCommand, oerr, ee.Stderr) } return string(out), oerr } From f1322aec075a5f36f22f4936be44001a36a1f09b Mon Sep 17 00:00:00 2001 From: "Nathan A. Ferch" Date: Fri, 17 Nov 2017 19:52:29 +0000 Subject: [PATCH 6/6] refactor pfctl output parsing use bufio instead of strings.Split use int64 for everything use list of fields instead of reflection and struct tags --- plugins/inputs/pf/pf.go | 143 +++++++++++++++-------------------- plugins/inputs/pf/pf_test.go | 18 ++--- 2 files changed, 69 insertions(+), 92 deletions(-) diff --git a/plugins/inputs/pf/pf.go b/plugins/inputs/pf/pf.go index b25593e6aeda7..9712ee8a64403 100644 --- a/plugins/inputs/pf/pf.go +++ b/plugins/inputs/pf/pf.go @@ -1,10 +1,9 @@ package pf import ( - "errors" + "bufio" "fmt" "os/exec" - "reflect" "regexp" "strconv" "strings" @@ -66,70 +65,88 @@ func errMissingData(tag string) error { return fmt.Errorf("struct data for tag \"%s\" not found in %s output", tag, pfctlCommand) } -var stateTableHeaderRE = regexp.MustCompile("^State Table") +type pfctlOutputStanza struct { + HeaderRE *regexp.Regexp + ParseFunc func([]string, telegraf.Accumulator) error + Found bool +} + +var pfctlOutputStanzas = []*pfctlOutputStanza{ + &pfctlOutputStanza{ + HeaderRE: regexp.MustCompile("^State Table"), + ParseFunc: parseStateTable, + }, +} + var anyTableHeaderRE = regexp.MustCompile("^[A-Z]") func (pf *PF) parsePfctlOutput(pfoutput string, acc telegraf.Accumulator) error { - lines := strings.Split(pfoutput, "\n") - stateTableFound := false - for i, line := range lines { - if stateTableHeaderRE.MatchString(line) { - endline := len(lines) - for j, el := range lines[i+1:] { - if anyTableHeaderRE.MatchString(el) { - endline = j + i + 1 - break + scanner := bufio.NewScanner(strings.NewReader(pfoutput)) + for scanner.Scan() { + line := scanner.Text() + for _, s := range pfctlOutputStanzas { + if s.HeaderRE.MatchString(line) { + var stanzaLines []string + scanner.Scan() + line = scanner.Text() + for !anyTableHeaderRE.MatchString(line) { + stanzaLines = append(stanzaLines, line) + scanner.Scan() + line = scanner.Text() } + if perr := s.ParseFunc(stanzaLines, acc); perr != nil { + return perr + } + s.Found = true } - if perr := pf.parseStateTable(lines[i+1:endline], acc); perr != nil { - return perr - } - stateTableFound = true } } - if !stateTableFound { - return errParseHeader + for _, s := range pfctlOutputStanzas { + if !s.Found { + return errParseHeader + } } return nil } -var stateTableRE = regexp.MustCompile(`^ (.*?)\s+(\d+)`) +type Entry struct { + Field string + PfctlTitle string + Value int64 +} -func (pf *PF) parseStateTable(lines []string, acc telegraf.Accumulator) error { - st := StateTable{} - tags, err := st.getTags() - if err != nil { - return fmt.Errorf("Can't retrieve struct tags: %v", err) - } - fMap := make(map[string]bool) - for i := 0; i < len(tags); i++ { - fMap[tags[i]] = false - } +var StateTable = []*Entry{ + &Entry{"entries", "current entries", -1}, + &Entry{"searches", "searches", -1}, + &Entry{"inserts", "inserts", -1}, + &Entry{"removals", "removals", -1}, +} + +var stateTableRE = regexp.MustCompile(`^ (.*?)\s+(\d+)`) +func parseStateTable(lines []string, acc telegraf.Accumulator) error { for _, v := range lines { entries := stateTableRE.FindStringSubmatch(v) if entries != nil { - fs, err := st.setByTag(entries[1], entries[2]) - if err != nil { - return errors.New("can't set statetable field from tag") - } - if fs { - fMap[entries[1]] = true + for _, f := range StateTable { + if f.PfctlTitle == entries[1] { + var err error + if f.Value, err = strconv.ParseInt(entries[2], 10, 64); err != nil { + return err + } + } } } } - for k, v := range fMap { - if !v { - return errMissingData(k) + fields := make(map[string]interface{}) + for _, v := range StateTable { + if v.Value == -1 { + return errMissingData(v.PfctlTitle) } + fields[v.Field] = v.Value } - fields := make(map[string]interface{}) - fields["entries"] = st.CurrentEntries - fields["searches"] = st.Searches - fields["inserts"] = st.Inserts - fields["removals"] = st.Removals acc.AddFields(measurement, fields, make(map[string]string)) return nil } @@ -166,46 +183,6 @@ func (pf *PF) buildPfctlCmd() (string, []string, error) { return cmd, args, nil } -type StateTable struct { - CurrentEntries uint32 `pfctl:"current entries"` - Searches uint64 `pfctl:"searches"` - Inserts uint64 `pfctl:"inserts"` - Removals uint64 `pfctl:"removals"` -} - -func (pf *StateTable) getTags() ([]string, error) { - tags := []string{} - structVal := reflect.ValueOf(pf).Elem() - for i := 0; i < structVal.NumField(); i++ { - tags = append(tags, structVal.Type().Field(i).Tag.Get(pfctlCommand)) - } - return tags, nil -} - -// setByTag sets val for a struct field given the tag. returns false if tag not found. -func (pf *StateTable) setByTag(tag string, val string) (bool, error) { - structVal := reflect.ValueOf(pf).Elem() - - for i := 0; i < structVal.NumField(); i++ { - tagField := structVal.Type().Field(i).Tag.Get(pfctlCommand) - if tagField == tag { - valueField := structVal.Field(i) - switch valueField.Type().Kind() { - case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - iVal, err := strconv.ParseUint(val, 10, 64) - if err != nil { - return false, fmt.Errorf("Error parsing \"%s\" into uint: %s", val, err) - } - valueField.SetUint(iVal) - return true, nil - default: - return false, fmt.Errorf("unhandled struct type %s", valueField.Type()) - } - } - } - return false, nil -} - func init() { inputs.Add("pf", func() telegraf.Input { pf := new(PF) diff --git a/plugins/inputs/pf/pf_test.go b/plugins/inputs/pf/pf_test.go index 52d22e5316523..233e72592bdec 100644 --- a/plugins/inputs/pf/pf_test.go +++ b/plugins/inputs/pf/pf_test.go @@ -45,7 +45,7 @@ func TestPfctlInvocation(t *testing.T) { t.Fatalf("error when running buildPfctlCmd: %s", err) } if tt.cmd != cmd || !reflect.DeepEqual(tt.args, args) { - t.Errorf("%d: expected %s - %#v got %s - %#v", tt.cmd, tt.args, cmd, args) + t.Errorf("%d: expected %s - %#v got %s - %#v", i, tt.cmd, tt.args, cmd, args) } }) } @@ -152,10 +152,10 @@ Counters measurements: []measurementResult{ measurementResult{ fields: map[string]interface{}{ - "entries": uint32(2), - "searches": uint64(11325), - "inserts": uint64(5), - "removals": uint64(3)}, + "entries": int64(2), + "searches": int64(11325), + "inserts": int64(5), + "removals": int64(3)}, tags: map[string]string{}, }, }, @@ -197,10 +197,10 @@ Counters measurements: []measurementResult{ measurementResult{ fields: map[string]interface{}{ - "entries": uint32(649), - "searches": uint64(18421725761), - "inserts": uint64(156762508), - "removals": uint64(156761859)}, + "entries": int64(649), + "searches": int64(18421725761), + "inserts": int64(156762508), + "removals": int64(156761859)}, tags: map[string]string{}, }, },