fix(go.d/ethtool): use ndsudo for module info (#19429)
Ilya Mashchenko committed
Jan 17, 2025 at 16:07 UTC
27f7fd8bcd4d5bbfd0a7d5a5a2ebe2f4fe3db4e2
8 files changed
+30
-56
src/go/plugin/go.d/collector/ethtool/collector.go
+1
-3
@@ -30,8 +30,7 @@ func init() {
30
func New() *Collector {
31
return &Collector{
32
Config: Config{
33
- BinaryPath: "/usr/sbin/ethtool",
34
- Timeout: confopt.Duration(time.Second * 2),
33
+ Timeout: confopt.Duration(time.Second * 2),
34
},
35
charts: &module.Charts{},
36
seenOpticIfaces: make(map[string]bool),
@@ -42,7 +41,6 @@ func New() *Collector {
41
type Config struct {
42
UpdateEvery int `yaml:"update_every,omitempty" json:"update_every"`
43
Timeout confopt.Duration `yaml:"timeout,omitempty" json:"timeout"`
45
- BinaryPath string `yaml:"binary_path,omitempty" json:"binary_path"`
44
OpticInterfaces string `yaml:"optical_interfaces,omitempty" json:"optical_interfaces"`
45
}
46
src/go/plugin/go.d/collector/ethtool/collector_test.go
+2
-10
@@ -46,17 +46,9 @@ func TestCollector_Init(t *testing.T) {
46
config Config
47
wantFail bool
48
}{
49
- "fails if 'binary_path' is not set": {
49
+ "fails if failed to locate ndsudo": {
50
wantFail: true,
51
- config: Config{
52
- BinaryPath: "",
53
- },
54
- },
55
- "fails if failed to find binary": {
56
- wantFail: true,
57
- config: Config{
58
- BinaryPath: "ethtool!!!",
59
- },
51
+ config: New().Config,
52
},
53
}
54
src/go/plugin/go.d/collector/ethtool/config_schema.json
+1
-7
@@ -11,12 +11,6 @@
11
"minimum": 1,
12
"default": 10
13
},
14
- "binary_path": {
15
- "title": "Binary path",
16
- "description": "Path to the `ethtool` binary.",
17
- "type": "string",
18
- "default": "/usr/sbin/ethtool"
19
- },
14
"timeout": {
15
"title": "Timeout",
16
"description": "Timeout for executing the binary, specified in seconds.",
@@ -30,7 +24,7 @@
24
"type": "string"
25
},
26
"required": [
33
- "binary_path"
27
+ "optical_interfaces"
28
]
29
}
30
},
src/go/plugin/go.d/collector/ethtool/exec.go
+13
-9
@@ -6,7 +6,6 @@ import (
6
"context"
7
"fmt"
8
"os/exec"
9
- "strings"
9
"time"
10
11
"github.com/netdata/netdata/go/plugins/logger"
@@ -16,31 +15,36 @@ type ethtoolCli interface {
15
moduleEeprom(iface string) ([]byte, error)
16
}
17
19
-func newEthtoolExec(binPath string, timeout time.Duration) *ethtoolCLIExec {
18
+func newEthtoolExec(ndsudoPath string, timeout time.Duration, logger *logger.Logger) *ethtoolCLIExec {
19
return ðtoolCLIExec{
21
- binPath: binPath,
22
- timeout: timeout,
20
+ Logger: logger,
21
+ ndsudoPath: ndsudoPath,
22
+ timeout: timeout,
23
}
24
}
25
26
type ethtoolCLIExec struct {
27
*logger.Logger
28
29
- binPath string
30
- timeout time.Duration
29
+ ndsudoPath string
30
+ timeout time.Duration
31
}
32
33
func (e *ethtoolCLIExec) moduleEeprom(iface string) ([]byte, error) {
34
ctx, cancel := context.WithTimeout(context.Background(), e.timeout)
35
defer cancel()
36
37
- cmd := exec.CommandContext(ctx, e.binPath, "-m", iface)
37
+ cmd := exec.CommandContext(ctx,
38
+ e.ndsudoPath,
39
+ "ethtool-module-info",
40
+ "--devname",
41
+ iface,
42
+ )
43
e.Debugf("executing '%s'", cmd)
44
45
bs, err := cmd.Output()
46
if err != nil {
42
- out := strings.ReplaceAll(string(bs), "\n", " ")
43
- return nil, fmt.Errorf("error on '%s': %v (%s)", cmd, err, out)
47
+ return nil, fmt.Errorf("error on '%s': %v", cmd, err)
48
}
49
50
return bs, nil
src/go/plugin/go.d/collector/ethtool/init.go
+8
-18
@@ -4,38 +4,28 @@ package ethtool
4
5
import (
6
"errors"
7
+ "fmt"
8
"os"
8
- "os/exec"
9
- "strings"
9
+ "path/filepath"
10
+
11
+ "github.com/netdata/netdata/go/plugins/pkg/executable"
12
)
13
14
func (c *Collector) validateConfig() error {
15
if c.OpticInterfaces == "" {
16
return errors.New("no optic interfaces specified")
17
}
16
- if c.BinaryPath == "" {
17
- return errors.New("no ethtool binary path specified")
18
- }
18
return nil
19
}
20
21
func (c *Collector) initEthtoolCli() (ethtoolCli, error) {
23
- binPath := c.BinaryPath
24
-
25
- if !strings.HasPrefix(binPath, "/") {
26
- path, err := exec.LookPath(binPath)
27
- if err != nil {
28
- return nil, err
29
- }
30
- binPath = path
31
- }
22
+ ndsudoPath := filepath.Join(executable.Directory, "ndsudo")
23
+ if _, err := os.Stat(ndsudoPath); err != nil {
24
+ return nil, fmt.Errorf("ndsudo executable not found: %v", err)
25
33
- if _, err := os.Stat(binPath); err != nil {
34
- return nil, err
26
}
27
37
- et := newEthtoolExec(binPath, c.Timeout.Duration())
38
- et.Logger = c.Logger
28
+ et := newEthtoolExec(ndsudoPath, c.Timeout.Duration(), c.Logger)
29
30
return et, nil
31
}
src/go/plugin/go.d/collector/ethtool/metadata.yaml
+5
-7
@@ -5,7 +5,7 @@ modules:
5
plugin_name: go.d.plugin
6
module_name: ethtool
7
monitored_instance:
8
- name: Network interfaces (hardware)
8
+ name: Optical modules
9
link: ""
10
icon_filename: network-wired.svg
11
categories:
@@ -25,8 +25,10 @@ modules:
25
metrics_description: >
26
This collector monitors optical transceiver modules' diagnostic parameters
27
(temperature, voltage, laser bias current, transmit/receive power levels) from network interfaces
28
- equipped with modules that support Digital Diagnostic Monitoring (DDM)
29
- using the command line tool [ethtool](https://man7.org/linux/man-pages/man8/ethtool.8.html).
28
+ equipped with modules that support Digital Diagnostic Monitoring (DDM).
29
+ It relies on the [`ethtool`](https://man7.org/linux/man-pages/man8/ethtool.8.html) CLI tool but avoids directly executing the binary.
30
+ Instead, it utilizes `ndsudo`, a Netdata helper specifically designed to run privileged commands securely within the Netdata environment.
31
+ This approach eliminates the need to use `sudo`, improving security and potentially simplifying permission management.
32
method_description: ""
33
supported_platforms:
34
include: [Linux]
@@ -58,10 +60,6 @@ modules:
60
description: Data collection frequency.
61
default_value: 10
62
required: false
61
- - name: binary_path
62
- description: Path to the `ethtool` binary. If an absolute path is provided, the collector will use it directly; otherwise, it will search for the binary in directories specified in the PATH environment variable.
63
- default_value: /usr/sbin/ethtool
64
- required: true
63
- name: timeout
64
description: Timeout for executing the binary, specified in seconds.
65
default_value: 2
src/go/plugin/go.d/collector/ethtool/testdata/config.json
-1
@@ -1,6 +1,5 @@
1
{
2
"update_every": 123,
3
"timeout": 123.123,
4
- "binary_path": "ok",
4
"optical_interfaces": "ok"
5
}
src/go/plugin/go.d/collector/ethtool/testdata/config.yaml
-1
@@ -1,4 +1,3 @@
1
update_every: 123
2
timeout: 123.123
3
-binary_path: "ok"
3
optical_interfaces: "ok"