Skip to content

Commit

Permalink
chore: use sysctl package vs exec calls
Browse files Browse the repository at this point in the history
  • Loading branch information
JGAntunes committed Nov 7, 2024
1 parent 4a7172c commit 22b7d74
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 41 deletions.
4 changes: 4 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)
Expand Down
7 changes: 7 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand All @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/troubleshoot/v1beta2/hostcollector_shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/troubleshoot/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/collect/host_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
28 changes: 17 additions & 11 deletions pkg/collect/host_sysctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@ 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"
)

// 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
Expand All @@ -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
}
63 changes: 33 additions & 30 deletions pkg/collect/host_sysctl_test.go
Original file line number Diff line number Diff line change
@@ -1,57 +1,57 @@
package collect

import (
"io"
"os/exec"
"encoding/json"
"fmt"
"os"
"testing"

troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
"github.com/replicatedhq/troubleshoot/pkg/multitype"
"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)

Check failure on line 48 in pkg/collect/host_sysctl_test.go

View workflow job for this annotation

GitHub Actions / test

G306: Expect WriteFile permissions to be 0600 or less (gosec)
req.NoError(err)
err = os.WriteFile(fmt.Sprintf("%s/arp_filter", ipv4All), []byte("1"), 0777)

Check failure on line 50 in pkg/collect/host_sysctl_test.go

View workflow job for this annotation

GitHub Actions / test

G306: Expect WriteFile permissions to be 0600 or less (gosec)
req.NoError(err)
err = os.WriteFile(fmt.Sprintf("%s/arp_evict_nocarrier", ipv4All), []byte("1"), 0777)

Check failure on line 52 in pkg/collect/host_sysctl_test.go

View workflow job for this annotation

GitHub Actions / test

G306: Expect WriteFile permissions to be 0600 or less (gosec)
req.NoError(err)

c := &CollectHostSysctl{
BundlePath: tmpDir,
}
Expand All @@ -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(&parameters)
req.NoError(err)
req.Equal(string(actualOut), cmdOut)

req.Equal(parameters, expectedOut)
}

func TestCollectHostSysctl_Title(t *testing.T) {
Expand Down

0 comments on commit 22b7d74

Please sign in to comment.