From 22b7d741cc9cc1260d35bd4bab5076c46ae9ca70 Mon Sep 17 00:00:00 2001 From: JGAntunes Date: Thu, 7 Nov 2024 11:12:14 +0000 Subject: [PATCH] chore: use sysctl package vs exec calls --- go.mod | 4 ++ go.sum | 7 +++ .../v1beta2/hostcollector_shared.go | 1 + .../v1beta2/zz_generated.deepcopy.go | 5 ++ pkg/collect/host_collector.go | 2 + pkg/collect/host_sysctl.go | 28 +++++---- pkg/collect/host_sysctl_test.go | 63 ++++++++++--------- 7 files changed, 69 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index d6814fc30..dcb37abdb 100644 --- a/go.mod +++ b/go.mod @@ -99,6 +99,7 @@ require ( github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lib/pq v1.10.9 // indirect + github.com/lorenzosaino/go-sysctl v0.3.1 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/mistifyio/go-zfs/v3 v3.0.1 // indirect github.com/mitchellh/copystructure v1.2.0 // indirect @@ -128,10 +129,13 @@ require ( go.opentelemetry.io/otel/metric v1.31.0 // indirect go.opentelemetry.io/otel/trace v1.31.0 // indirect go.uber.org/multierr v1.11.0 // indirect + golang.org/x/exp/typeparams v0.0.0-20220613132600-b0d781184e0d // indirect + golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect golang.org/x/tools v0.22.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect + honnef.co/go/tools v0.3.2 // indirect k8s.io/component-base v0.31.2 // indirect k8s.io/kubectl v0.31.1 // indirect ) diff --git a/go.sum b/go.sum index 0dc63c576..775a1d184 100644 --- a/go.sum +++ b/go.sum @@ -651,6 +651,8 @@ github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9 github.com/longhorn/go-iscsi-helper v0.0.0-20210330030558-49a327fb024e h1:hz4quJkaJWDo+xW+G6wTF6d6/95QvJ+o2D0+bB/tJ1U= github.com/longhorn/go-iscsi-helper v0.0.0-20210330030558-49a327fb024e/go.mod h1:9z/y9glKmWEdV50tjlUPxFwi1goQfIrrsoZbnMyIZbY= github.com/longhorn/nsfilelock v0.0.0-20200723175406-fa7c83ad0003/go.mod h1:0CLeXlf59Lg6C0kjLSDf47ft73Dh37CwymYRKWwAn04= +github.com/lorenzosaino/go-sysctl v0.3.1 h1:3phX80tdITw2fJjZlwbXQnDWs4S30beNcMbw0cn0HtY= +github.com/lorenzosaino/go-sysctl v0.3.1/go.mod h1:5grcsBRpspKknNS1qzt1eIeRDLrhpKZAtz8Fcuvs1Rc= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 h1:6E+4a0GO5zZEnZ81pIr0yLvtUWk2if982qA3F3QD6H4= github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0/go.mod h1:zJYVVT2jmtg6P3p1VtQj7WsuWi/y4VnjVBn7F8KPB3I= github.com/magiconair/properties v1.8.7 h1:IeQXZAiQcpL9mgcAe1Nu6cX9LLw6ExEHKjN0VQdvPDY= @@ -988,6 +990,8 @@ golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EH golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8 h1:yixxcjnhBmY0nkL253HFVIm0JsFHwrHdT3Yh6szTnfY= golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8/go.mod h1:jj3sYF3dwk5D+ghuXyeI3r5MFf+NT2An6/9dOA95KSI= +golang.org/x/exp/typeparams v0.0.0-20220613132600-b0d781184e0d h1:+W8Qf4iJtMGKkyAygcKohjxTk4JPsL9DpzApJ22m5Ic= +golang.org/x/exp/typeparams v0.0.0-20220613132600-b0d781184e0d/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -1001,6 +1005,7 @@ golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f/go.mod h1:5qLYkcX4OjUUV8bRu golang.org/x/lint v0.0.0-20200130185559-910be7a94367/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/lint v0.0.0-20201208152925-83fdc39ff7b5/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= +golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 h1:VLliZ0d+/avPrXXH+OakdXhpJuEoBZuwh1m2j7U6Iug= golang.org/x/lint v0.0.0-20210508222113-6edffad5e616/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6/go.mod h1:z+o9i4GpDbdi3rU15maQ/Ox0txvL9dWGYEHz965HBQE= golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028/go.mod h1:E/iHnbuqvinMTCcRqshq8CkpyQDoeVncDDYHnLhea+o= @@ -1543,6 +1548,8 @@ honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWh honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg= honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= +honnef.co/go/tools v0.3.2 h1:ytYb4rOqyp1TSa2EPvNVwtPQJctSELKaMyLfqNP4+34= +honnef.co/go/tools v0.3.2/go.mod h1:jzwdWgg7Jdq75wlfblQxO4neNaFFSvgc1tD5Wv8U0Yw= k8s.io/api v0.31.2 h1:3wLBbL5Uom/8Zy98GRPXpJ254nEFpl+hwndmk9RwmL0= k8s.io/api v0.31.2/go.mod h1:bWmGvrGPssSK1ljmLzd3pwCQ9MgoTsRCuK35u6SygUk= k8s.io/apiextensions-apiserver v0.31.2 h1:W8EwUb8+WXBLu56ser5IudT2cOho0gAKeTOnywBLxd0= diff --git a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go index e2205c0c1..383d6102a 100644 --- a/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go +++ b/pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go @@ -264,6 +264,7 @@ type HostCollect struct { HostCGroups *HostCGroups `json:"cgroups,omitempty" yaml:"cgroups,omitempty"` HostDNS *HostDNS `json:"dns,omitempty" yaml:"dns,omitempty"` NetworkNamespaceConnectivity *HostNetworkNamespaceConnectivity `json:"networkNamespaceConnectivity,omitempty" yaml:"networkNamespaceConnectivity,omitempty"` + HostSysctl *HostSysctl `json:"sysctl,omitempty" yaml:"sysctl,omitempty"` } // GetName gets the name of the collector diff --git a/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go b/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go index c26134768..74d0ddb92 100644 --- a/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go +++ b/pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go @@ -2160,6 +2160,11 @@ func (in *HostCollect) DeepCopyInto(out *HostCollect) { *out = new(HostNetworkNamespaceConnectivity) (*in).DeepCopyInto(*out) } + if in.HostSysctl != nil { + in, out := &in.HostSysctl, &out.HostSysctl + *out = new(HostSysctl) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostCollect. diff --git a/pkg/collect/host_collector.go b/pkg/collect/host_collector.go index ff93e6655..aa73a2f65 100644 --- a/pkg/collect/host_collector.go +++ b/pkg/collect/host_collector.go @@ -101,6 +101,8 @@ func GetHostCollector(collector *troubleshootv1beta2.HostCollect, bundlePath str return &CollectHostDNS{collector.HostDNS, bundlePath}, true case collector.NetworkNamespaceConnectivity != nil: return &CollectHostNetworkNamespaceConnectivity{collector.NetworkNamespaceConnectivity, bundlePath}, true + case collector.HostSysctl != nil: + return &CollectHostSysctl{collector.HostSysctl, bundlePath}, true default: return nil, false } diff --git a/pkg/collect/host_sysctl.go b/pkg/collect/host_sysctl.go index 8c2ef9d7b..ae55d8978 100644 --- a/pkg/collect/host_sysctl.go +++ b/pkg/collect/host_sysctl.go @@ -2,8 +2,9 @@ package collect import ( "bytes" - "os/exec" + "encoding/json" + "github.com/lorenzosaino/go-sysctl" "github.com/pkg/errors" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" ) @@ -11,9 +12,10 @@ import ( // Ensure `CollectHostSysctl` implements `HostCollector` interface at compile time. var _ HostCollector = (*CollectHostSysctl)(nil) -var execCommand = exec.Command +// Path to the kernel virtual files, defaults to /proc/sys +var sysctlVirtualFiles = sysctl.DefaultPath -const HostSysctlPath = `host-collectors/system/sysctl.txt` +const HostSysctlPath = `host-collectors/system/sysctl.json` type CollectHostSysctl struct { hostCollector *troubleshootv1beta2.HostSysctl @@ -29,18 +31,22 @@ func (c *CollectHostSysctl) IsExcluded() (bool, error) { } func (c *CollectHostSysctl) Collect(progressChan chan<- interface{}) (map[string][]byte, error) { + client, err := sysctl.NewClient(sysctlVirtualFiles) + if err != nil { + return nil, errors.Wrap(err, "failed to initialize sysctl client") + } + + values, err := client.GetAll() + if err != nil { + return nil, errors.Wrap(err, "failed to run sysctl client") + } - cmd := execCommand("sysctl", "-a") - out, err := cmd.Output() + payload, err := json.Marshal(values) if err != nil { - if exitErr, ok := err.(*exec.ExitError); ok { - return nil, errors.Wrapf(err, "failed to run sysctl exit-code=%d stderr=%s", exitErr.ExitCode(), exitErr.Stderr) - } else { - return nil, errors.Wrap(err, "failed to run sysctl") - } + return nil, errors.Wrap(err, "failed to marshal data to json") } output := NewResult() - output.SaveResult(c.BundlePath, HostSysctlPath, bytes.NewBuffer(out)) + output.SaveResult(c.BundlePath, HostSysctlPath, bytes.NewBuffer(payload)) return output, nil } diff --git a/pkg/collect/host_sysctl_test.go b/pkg/collect/host_sysctl_test.go index af8b1164c..bef182c84 100644 --- a/pkg/collect/host_sysctl_test.go +++ b/pkg/collect/host_sysctl_test.go @@ -1,8 +1,9 @@ package collect import ( - "io" - "os/exec" + "encoding/json" + "fmt" + "os" "testing" troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2" @@ -10,48 +11,47 @@ import ( "github.com/stretchr/testify/require" ) -type execStub struct { - cmd *exec.Cmd - name string - args []string -} - -func (s *execStub) testExecCommand(name string, args ...string) *exec.Cmd { - s.name = name - s.args = args - return s.cmd -} - -func setExecStub(c *exec.Cmd) { - e := &execStub{ - cmd: c, - } - execCommand = e.testExecCommand +func setKernelVirtualFilesPath(path string) { + sysctlVirtualFiles = path } func TestCollectHostSysctl_Error(t *testing.T) { req := require.New(t) - setExecStub(exec.Command("sh", "-c", "exit 1")) - tmpDir := t.TempDir() + + setKernelVirtualFilesPath(fmt.Sprintf("%s/does/not/exist", tmpDir)) + c := &CollectHostSysctl{ BundlePath: tmpDir, } _, err := c.Collect(nil) - req.ErrorContains(err, "failed to run sysctl exit-code=1") + req.ErrorContains(err, "failed to initialize sysctl client") } func TestCollectHostSysctl(t *testing.T) { req := require.New(t) - cmdOut := ` - net.ipv4.conf.all.arp_evict_nocarrier = 1 - net.ipv4.conf.all.arp_filter = 0 - net.ipv4.conf.all.arp_ignore = 0 - ` - setExecStub(exec.Command("echo", "-n", cmdOut)) + expectedOut := map[string]string{ + "net.ipv4.conf.all.arp_ignore": "0", + "net.ipv4.conf.all.arp_filter": "1", + "net.ipv4.conf.all.arp_evict_nocarrier": "1", + } tmpDir := t.TempDir() + virtualFilesPath := fmt.Sprintf("%s/proc/sys/", tmpDir) + ipv4All := fmt.Sprintf("%s/net/ipv4/conf/all", virtualFilesPath) + + setKernelVirtualFilesPath(virtualFilesPath) + err := os.MkdirAll(ipv4All, 0777) + req.NoError(err) + + err = os.WriteFile(fmt.Sprintf("%s/arp_ignore", ipv4All), []byte("0"), 0777) + req.NoError(err) + err = os.WriteFile(fmt.Sprintf("%s/arp_filter", ipv4All), []byte("1"), 0777) + req.NoError(err) + err = os.WriteFile(fmt.Sprintf("%s/arp_evict_nocarrier", ipv4All), []byte("1"), 0777) + req.NoError(err) + c := &CollectHostSysctl{ BundlePath: tmpDir, } @@ -61,9 +61,12 @@ func TestCollectHostSysctl(t *testing.T) { res := CollectorResult(out) reader, err := res.GetReader(tmpDir, HostSysctlPath) req.NoError(err) - actualOut, err := io.ReadAll(reader) + + parameters := map[string]string{} + err = json.NewDecoder(reader).Decode(¶meters) req.NoError(err) - req.Equal(string(actualOut), cmdOut) + + req.Equal(parameters, expectedOut) } func TestCollectHostSysctl_Title(t *testing.T) {