From 034ce0affb8044610b7ceb36dd8a2ec6a8d350d5 Mon Sep 17 00:00:00 2001 From: Stanislav Chzhen Date: Tue, 12 Aug 2025 16:25:32 +0300 Subject: [PATCH] all: imp code --- internal/agh/agh.go | 127 +++++++++ internal/aghnet/net.go | 50 ++-- internal/aghnet/net_darwin.go | 50 +++- internal/aghnet/net_darwin_internal_test.go | 279 +++++++++---------- internal/aghnet/net_freebsd.go | 10 +- internal/aghnet/net_freebsd_internal_test.go | 5 +- internal/aghnet/net_internal_test.go | 86 ++---- internal/aghnet/net_linux.go | 16 +- internal/aghnet/net_linux_internal_test.go | 4 +- internal/aghnet/net_openbsd.go | 10 +- internal/aghnet/net_openbsd_internal_test.go | 5 +- internal/aghnet/net_windows.go | 10 +- internal/aghos/os.go | 41 +-- internal/arpdb/arpdb.go | 35 ++- internal/arpdb/arpdb_bsd.go | 8 +- internal/arpdb/arpdb_internal_test.go | 60 +--- internal/arpdb/arpdb_linux.go | 23 +- internal/arpdb/arpdb_linux_internal_test.go | 25 +- internal/arpdb/arpdb_openbsd.go | 8 +- internal/arpdb/arpdb_windows.go | 8 +- internal/dhcpd/http_unix.go | 18 +- internal/home/controlinstall.go | 82 ++---- internal/home/home.go | 26 +- internal/home/service.go | 18 +- internal/home/service_linux.go | 8 +- internal/home/service_openbsd.go | 13 +- internal/updater/check_test.go | 35 +-- internal/updater/updater.go | 11 +- internal/updater/updater_internal_test.go | 16 +- internal/updater/updater_test.go | 20 +- scripts/translations/main.go | 53 +++- 31 files changed, 672 insertions(+), 488 deletions(-) diff --git a/internal/agh/agh.go b/internal/agh/agh.go index 8fbd740f..adffa75f 100644 --- a/internal/agh/agh.go +++ b/internal/agh/agh.go @@ -3,6 +3,12 @@ package agh import ( "context" + "fmt" + "strings" + + "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/osutil/executil" + "github.com/AdguardTeam/golibs/testutil/fakeos/fakeexec" ) // ConfigModifier defines an interface for updating the global configuration. @@ -20,3 +26,124 @@ var _ ConfigModifier = EmptyConfigModifier{} // Apply implements the [ConfigModifier] for EmptyConfigModifier. func (em EmptyConfigModifier) Apply(ctx context.Context) {} + +// TODO(s.chzhen): !! Is there another way? +// +// TODO(s.chzhen): !! Docs, naming. +// +// TODO(s.chzhen): Move to aghtest once the import cycle is resolved. +type exitErr struct { + code osutil.ExitCode +} + +// type check +var _ executil.ExitCodeError = exitErr{} + +func (e exitErr) Error() (s string) { + return fmt.Sprintf("exit code %d", e.code) +} + +func (e exitErr) ExitCode() (code osutil.ExitCode) { + return e.code +} + +type ExternalCommand struct { + Err error + Cmd string + Out string + Code int +} + +func keyCommand(path string, args []string) (k string) { + return path + " " + strings.Join(args, " ") +} + +func parseCommand(s string) (path string, args []string) { + f := strings.Fields(s) + if len(f) == 0 { + return "", nil + } + + return f[0], f[1:] +} + +// NewMultipleCommandConstructor is a helper function that returns a mock +// [executil.CommandConstructor] for tests. +func NewMultipleCommandConstructor(cmds ...ExternalCommand) (cs executil.CommandConstructor) { + table := make(map[string]ExternalCommand, len(cmds)) + for _, ec := range cmds { + p, a := parseCommand(ec.Cmd) + table[keyCommand(p, a)] = ec + } + + return &fakeexec.CommandConstructor{ + OnNew: func( + _ context.Context, + conf *executil.CommandConfig, + ) (c executil.Command, err error) { + ec := table[keyCommand(conf.Path, conf.Args)] + + cmd := fakeexec.NewCommand() + cmd.OnStart = func(_ context.Context) (err error) { + if ec.Out != "" { + _, _ = conf.Stdout.Write([]byte(ec.Out)) + } + + return nil + } + + cmd.OnWait = func(_ context.Context) (err error) { + if ec.Err != nil { + return ec.Err + } + + if ec.Code != 0 { + return exitErr{code: ec.Code} + } + + return nil + } + + return cmd, nil + }, + } +} + +// NewCommandConstructor is a helper function that returns a mock +// [executil.CommandConstructor] for tests. +func NewCommandConstructor( + _ string, + code int, + stdout string, + cmdErr error, +) (cs executil.CommandConstructor) { + return &fakeexec.CommandConstructor{ + OnNew: func( + _ context.Context, + conf *executil.CommandConfig, + ) (c executil.Command, err error) { + cmd := fakeexec.NewCommand() + cmd.OnStart = func(_ context.Context) (err error) { + if conf.Stdout != nil { + _, _ = conf.Stdout.Write([]byte(stdout)) + } + + return nil + } + + cmd.OnWait = func(_ context.Context) (err error) { + if cmdErr != nil { + return cmdErr + } + + if code != 0 { + return exitErr{code: code} + } + + return nil + } + + return cmd, nil + }, + } +} diff --git a/internal/aghnet/net.go b/internal/aghnet/net.go index 71e81b02..cb9def86 100644 --- a/internal/aghnet/net.go +++ b/internal/aghnet/net.go @@ -13,11 +13,11 @@ import ( "strings" "syscall" - "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/dnsproxy/upstream" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) // DialContextFunc is the semantic alias for dialing functions, such as @@ -26,9 +26,6 @@ type DialContextFunc = func(ctx context.Context, network, addr string) (conn net // Variables and functions to substitute in tests. var ( - // aghosRunCommand is the function to run shell commands. - aghosRunCommand = aghos.RunCommand - // netInterfaces is the function to get the available network interfaces. netInterfaceAddrs = net.InterfaceAddrs @@ -43,32 +40,53 @@ const ErrNoStaticIPInfo errors.Error = "no information about static ip" // IfaceHasStaticIP checks if interface is configured to have static IP address. // If it can't give a definitive answer, it returns false and an error for which // errors.Is(err, ErrNoStaticIPInfo) is true. -func IfaceHasStaticIP(ifaceName string) (has bool, err error) { - return ifaceHasStaticIP(ifaceName) +func IfaceHasStaticIP( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (has bool, err error) { + return ifaceHasStaticIP(ctx, cmdCons, ifaceName) } // IfaceSetStaticIP sets static IP address for network interface. -func IfaceSetStaticIP(ifaceName string) (err error) { - return ifaceSetStaticIP(ifaceName) +func IfaceSetStaticIP( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (err error) { + return ifaceSetStaticIP(ctx, cmdCons, ifaceName) } // GatewayIP returns IP address of interface's gateway. // // TODO(e.burkov): Investigate if the gateway address may be fetched in another // way since not every machine has the software installed. -func GatewayIP(ifaceName string) (ip netip.Addr) { - code, out, err := aghosRunCommand("ip", "route", "show", "dev", ifaceName) +func GatewayIP( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (ip netip.Addr) { + stdout := bytes.Buffer{} + err := executil.Run( + ctx, + cmdCons, + &executil.CommandConfig{ + Path: "ip", + Args: []string{"route", "show", "dev", ifaceName}, + Stdout: &stdout, + }, + ) if err != nil { - log.Debug("%s", err) - - return netip.Addr{} - } else if code != 0 { - log.Debug("fetching gateway ip: unexpected exit code: %d", code) + if code, ok := executil.ExitCodeFromError(err); ok { + log.Debug("fetching gateway ip: unexpected exit code: %d", code) + } else { + log.Debug("%s", err) + } return netip.Addr{} } - fields := bytes.Fields(out) + fields := bytes.Fields(stdout.Bytes()) // The meaningful "ip route" command output should contain the word // "default" at first field and default gateway IP address at third field. if len(fields) < 3 || string(fields[0]) != "default" { diff --git a/internal/aghnet/net_darwin.go b/internal/aghnet/net_darwin.go index bdf04729..5a974202 100644 --- a/internal/aghnet/net_darwin.go +++ b/internal/aghnet/net_darwin.go @@ -5,12 +5,14 @@ package aghnet import ( "bufio" "bytes" + "context" "fmt" "io" "regexp" "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/osutil/executil" ) // hardwarePortInfo contains information about the current state of the internet @@ -23,8 +25,12 @@ type hardwarePortInfo struct { static bool } -func ifaceHasStaticIP(ifaceName string) (ok bool, err error) { - portInfo, err := getCurrentHardwarePortInfo(ifaceName) +func ifaceHasStaticIP( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (ok bool, err error) { + portInfo, err := getCurrentHardwarePortInfo(ctx, cmdCons, ifaceName) if err != nil { return false, err } @@ -34,15 +40,19 @@ func ifaceHasStaticIP(ifaceName string) (ok bool, err error) { // getCurrentHardwarePortInfo gets information for the specified network // interface. -func getCurrentHardwarePortInfo(ifaceName string) (hardwarePortInfo, error) { +func getCurrentHardwarePortInfo( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (hardwarePortInfo, error) { // First of all we should find hardware port name. - m := getNetworkSetupHardwareReports() + m := getNetworkSetupHardwareReports(ctx, cmdCons) hardwarePort, ok := m[ifaceName] if !ok { return hardwarePortInfo{}, fmt.Errorf("could not find hardware port for %s", ifaceName) } - return getHardwarePortInfo(hardwarePort) + return getHardwarePortInfo(ctx, cmdCons, hardwarePort) } // hardwareReportsReg is the regular expression matching the lines of @@ -57,8 +67,12 @@ var hardwareReportsReg = regexp.MustCompile("Hardware Port: (.*?)\nDevice: (.*?) // TODO(e.burkov): There should be more proper approach than parsing the // command output. For example, see // https://developer.apple.com/documentation/systemconfiguration. -func getNetworkSetupHardwareReports() (reports map[string]string) { - _, out, err := aghosRunCommand("networksetup", "-listallhardwareports") +func getNetworkSetupHardwareReports( + ctx context.Context, + cmdCons executil.CommandConstructor, +) (reports map[string]string) { + // TODO(s.chzhen): Pass context. + _, out, err := aghos.RunCommand(ctx, cmdCons, "networksetup", "-listallhardwareports") if err != nil { return nil } @@ -77,8 +91,12 @@ func getNetworkSetupHardwareReports() (reports map[string]string) { // command output lines containing the port information. var hardwarePortReg = regexp.MustCompile("IP address: (.*?)\nSubnet mask: (.*?)\nRouter: (.*?)\n") -func getHardwarePortInfo(hardwarePort string) (h hardwarePortInfo, err error) { - _, out, err := aghosRunCommand("networksetup", "-getinfo", hardwarePort) +func getHardwarePortInfo( + ctx context.Context, + cmdCons executil.CommandConstructor, + hardwarePort string, +) (h hardwarePortInfo, err error) { + _, out, err := aghos.RunCommand(ctx, cmdCons, "networksetup", "-getinfo", hardwarePort) if err != nil { return h, err } @@ -97,8 +115,12 @@ func getHardwarePortInfo(hardwarePort string) (h hardwarePortInfo, err error) { }, nil } -func ifaceSetStaticIP(ifaceName string) (err error) { - portInfo, err := getCurrentHardwarePortInfo(ifaceName) +func ifaceSetStaticIP( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (err error) { + portInfo, err := getCurrentHardwarePortInfo(ctx, cmdCons, ifaceName) if err != nil { return err } @@ -115,7 +137,7 @@ func ifaceSetStaticIP(ifaceName string) (err error) { args := append([]string{"-setdnsservers", portInfo.name}, dnsAddrs...) // Setting DNS servers is necessary when configuring a static IP - code, _, err := aghosRunCommand("networksetup", args...) + code, _, err := aghos.RunCommand(ctx, cmdCons, "networksetup", args...) if err != nil { return err } else if code != 0 { @@ -123,7 +145,9 @@ func ifaceSetStaticIP(ifaceName string) (err error) { } // Actually configures hardware port to have static IP - code, _, err = aghosRunCommand( + code, _, err = aghos.RunCommand( + ctx, + cmdCons, "networksetup", "-setmanual", portInfo.name, diff --git a/internal/aghnet/net_darwin_internal_test.go b/internal/aghnet/net_darwin_internal_test.go index 7593a969..41304018 100644 --- a/internal/aghnet/net_darwin_internal_test.go +++ b/internal/aghnet/net_darwin_internal_test.go @@ -7,7 +7,9 @@ import ( "testing" "testing/fstest" + "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/testutil" "github.com/AdguardTeam/golibs/testutil/fakefs" "github.com/stretchr/testify/assert" @@ -16,48 +18,46 @@ import ( func TestIfaceHasStaticIP(t *testing.T) { testCases := []struct { name string - shell mapShell + cmdCons executil.CommandConstructor ifaceName string wantHas assert.BoolAssertionFunc wantErrMsg string }{{ name: "success", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, + }), ifaceName: "en0", wantHas: assert.False, wantErrMsg: ``, }, { name: "success_static", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "Manual Configuration\nIP address: 1.2.3.4\n" + - "Subnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "Manual Configuration\nIP address: 1.2.3.4\n" + + "Subnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, + }), ifaceName: "en0", wantHas: assert.True, wantErrMsg: ``, }, { name: "reports_error", - shell: theOnlyCmd( + cmdCons: agh.NewCommandConstructor( "networksetup -listallhardwareports", 0, "", @@ -68,35 +68,33 @@ func TestIfaceHasStaticIP(t *testing.T) { wantErrMsg: `could not find hardware port for en0`, }, { name: "port_error", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: errors.Error("can't get"), - out: ``, - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: errors.Error("can't get"), + Out: ``, + Code: 0, + }), ifaceName: "en0", wantHas: assert.False, wantErrMsg: `can't get`, }, { name: "port_bad_output", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "nothing meaningful", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "nothing meaningful", + Code: 0, + }), ifaceName: "en0", wantHas: assert.False, wantErrMsg: `could not find hardware port info`, @@ -104,9 +102,8 @@ func TestIfaceHasStaticIP(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - substShell(t, tc.shell.RunCmd) - - has, err := IfaceHasStaticIP(tc.ifaceName) + ctx := testutil.ContextWithTimeout(t, testTimeout) + has, err := IfaceHasStaticIP(ctx, tc.cmdCons, tc.ifaceName) testutil.AssertErrorMsg(t, tc.wantErrMsg, err) tc.wantHas(t, has) @@ -126,55 +123,53 @@ func TestIfaceSetStaticIP(t *testing.T) { testCases := []struct { name string - shell mapShell + cmdCons executil.CommandConstructor fsys fs.FS wantErrMsg string }{{ name: "success", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, - "networksetup -setdnsservers hwport 1.1.1.1": { - err: nil, - out: "", - code: 0, - }, - "networksetup -setmanual hwport 1.2.3.4 255.255.255.0 1.2.3.1": { - err: nil, - out: "", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -setdnsservers hwport 1.1.1.1", + Err: nil, + Out: "", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -setmanual hwport 1.2.3.4 255.255.255.0 1.2.3.1", + Err: nil, + Out: "", + Code: 0, + }), fsys: succFsys, wantErrMsg: ``, }, { name: "static_already", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "Manual Configuration\nIP address: 1.2.3.4\n" + - "Subnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "Manual Configuration\nIP address: 1.2.3.4\n" + + "Subnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, + }), fsys: panicFsys, wantErrMsg: `ip address is already static`, }, { name: "reports_error", - shell: theOnlyCmd( + cmdCons: agh.NewCommandConstructor( "networksetup -listallhardwareports", 0, "", @@ -184,18 +179,18 @@ func TestIfaceSetStaticIP(t *testing.T) { wantErrMsg: `could not find hardware port for en0`, }, { name: "resolv_conf_error", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, }, + ), fsys: fstest.MapFS{ "etc/resolv.conf": &fstest.MapFile{ Data: []byte("this resolv.conf is invalid"), @@ -204,59 +199,57 @@ func TestIfaceSetStaticIP(t *testing.T) { wantErrMsg: `found no dns servers in etc/resolv.conf`, }, { name: "set_dns_error", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, - "networksetup -setdnsservers hwport 1.1.1.1": { - err: errors.Error("can't set"), - out: "", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -setdnsservers hwport 1.1.1.1", + Err: errors.Error("can't set"), + Out: "", + Code: 0, + }), fsys: succFsys, wantErrMsg: `can't set`, }, { name: "set_manual_error", - shell: mapShell{ - "networksetup -listallhardwareports": { - err: nil, - out: "Hardware Port: hwport\nDevice: en0\n", - code: 0, - }, - "networksetup -getinfo hwport": { - err: nil, - out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", - code: 0, - }, - "networksetup -setdnsservers hwport 1.1.1.1": { - err: nil, - out: "", - code: 0, - }, - "networksetup -setmanual hwport 1.2.3.4 255.255.255.0 1.2.3.1": { - err: errors.Error("can't set"), - out: "", - code: 0, - }, - }, + cmdCons: agh.NewMultipleCommandConstructor(agh.ExternalCommand{ + Cmd: "networksetup -listallhardwareports", + Err: nil, + Out: "Hardware Port: hwport\nDevice: en0\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -getinfo hwport", + Err: nil, + Out: "IP address: 1.2.3.4\nSubnet mask: 255.255.255.0\nRouter: 1.2.3.1\n", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -setdnsservers hwport 1.1.1.1", + Err: nil, + Out: "", + Code: 0, + }, agh.ExternalCommand{ + Cmd: "networksetup -setmanual hwport 1.2.3.4 255.255.255.0 1.2.3.1", + Err: errors.Error("can't set"), + Out: "", + Code: 0, + }), fsys: succFsys, wantErrMsg: `can't set`, }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - substShell(t, tc.shell.RunCmd) substRootDirFS(t, tc.fsys) - err := IfaceSetStaticIP("en0") + ctx := testutil.ContextWithTimeout(t, testTimeout) + err := IfaceSetStaticIP(ctx, tc.cmdCons, "en0") testutil.AssertErrorMsg(t, tc.wantErrMsg, err) }) } diff --git a/internal/aghnet/net_freebsd.go b/internal/aghnet/net_freebsd.go index 6b205639..d646b80f 100644 --- a/internal/aghnet/net_freebsd.go +++ b/internal/aghnet/net_freebsd.go @@ -4,15 +4,21 @@ package aghnet import ( "bufio" + "context" "fmt" "io" "strings" "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/netutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) -func ifaceHasStaticIP(ifaceName string) (ok bool, err error) { +func ifaceHasStaticIP( + _ context.Context, + _ executil.CommandConstructor, + ifaceName string, +) (ok bool, err error) { const rcConfFilename = "etc/rc.conf" walker := aghos.FileWalker(interfaceName(ifaceName).rcConfStaticConfig) @@ -52,6 +58,6 @@ func (n interfaceName) rcConfStaticConfig(r io.Reader) (_ []string, cont bool, e return nil, true, s.Err() } -func ifaceSetStaticIP(string) (err error) { +func ifaceSetStaticIP(_ context.Context, _ executil.CommandConstructor, _ string) (err error) { return aghos.Unsupported("setting static ip") } diff --git a/internal/aghnet/net_freebsd_internal_test.go b/internal/aghnet/net_freebsd_internal_test.go index 8e23092c..14ffda4f 100644 --- a/internal/aghnet/net_freebsd_internal_test.go +++ b/internal/aghnet/net_freebsd_internal_test.go @@ -7,6 +7,8 @@ import ( "testing" "testing/fstest" + "github.com/AdguardTeam/golibs/osutil/executil" + "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -67,7 +69,8 @@ func TestIfaceHasStaticIP(t *testing.T) { t.Run(tc.name, func(t *testing.T) { substRootDirFS(t, tc.rootFsys) - has, err := IfaceHasStaticIP(ifaceName) + ctx := testutil.ContextWithTimeout(t, testTimeout) + has, err := IfaceHasStaticIP(ctx, executil.EmptyCommandConstructor{}, ifaceName) require.NoError(t, err) tc.wantHas(t, has) diff --git a/internal/aghnet/net_internal_test.go b/internal/aghnet/net_internal_test.go index 6b5dc392..aec10055 100644 --- a/internal/aghnet/net_internal_test.go +++ b/internal/aghnet/net_internal_test.go @@ -3,20 +3,24 @@ package aghnet import ( "bytes" "encoding/json" - "fmt" "io/fs" "net" "net/netip" - "strings" "testing" + "time" + "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/netutil" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// testTimeout is the common timeout for tests. +const testTimeout = 1 * time.Second + // substRootDirFS replaces the aghos.RootDirFS function used throughout the // package with fsys for tests ran under t. func substRootDirFS(t testing.TB, fsys fs.FS) { @@ -30,43 +34,6 @@ func substRootDirFS(t testing.TB, fsys fs.FS) { // RunCmdFunc is the signature of aghos.RunCommand function. type RunCmdFunc func(cmd string, args ...string) (code int, out []byte, err error) -// substShell replaces the the aghos.RunCommand function used throughout the -// package with rc for tests ran under t. -func substShell(t testing.TB, rc RunCmdFunc) { - t.Helper() - - prev := aghosRunCommand - t.Cleanup(func() { aghosRunCommand = prev }) - aghosRunCommand = rc -} - -// mapShell is a substitution of aghos.RunCommand that maps the command to it's -// execution result. It's only needed to simplify testing. -// -// TODO(e.burkov): Perhaps put all the shell interactions behind an interface. -type mapShell map[string]struct { - err error - out string - code int -} - -// theOnlyCmd returns mapShell that only handles a single command and arguments -// combination from cmd. -func theOnlyCmd(cmd string, code int, out string, err error) (s mapShell) { - return mapShell{cmd: {code: code, out: out, err: err}} -} - -// RunCmd is a RunCmdFunc handled by s. -func (s mapShell) RunCmd(cmd string, args ...string) (code int, out []byte, err error) { - key := strings.Join(append([]string{cmd}, args...), " ") - ret, ok := s[key] - if !ok { - return 0, nil, fmt.Errorf("unexpected shell command %q", key) - } - - return ret.code, []byte(ret.out), ret.err -} - // ifaceAddrsFunc is the signature of net.InterfaceAddrs function. type ifaceAddrsFunc func() (ifaces []net.Addr, err error) @@ -85,36 +52,35 @@ func TestGatewayIP(t *testing.T) { const cmd = "ip route show dev " + ifaceName testCases := []struct { - shell mapShell - want netip.Addr - name string + cmdCons executil.CommandConstructor + want netip.Addr + name string }{{ - shell: theOnlyCmd(cmd, 0, `default via 1.2.3.4 onlink`, nil), - want: netip.MustParseAddr("1.2.3.4"), - name: "success_v4", + cmdCons: agh.NewCommandConstructor(cmd, 0, `default via 1.2.3.4 onlink`, nil), + want: netip.MustParseAddr("1.2.3.4"), + name: "success_v4", }, { - shell: theOnlyCmd(cmd, 0, `default via ::ffff onlink`, nil), - want: netip.MustParseAddr("::ffff"), - name: "success_v6", + cmdCons: agh.NewCommandConstructor(cmd, 0, `default via ::ffff onlink`, nil), + want: netip.MustParseAddr("::ffff"), + name: "success_v6", }, { - shell: theOnlyCmd(cmd, 0, `non-default via 1.2.3.4 onlink`, nil), - want: netip.Addr{}, - name: "bad_output", + cmdCons: agh.NewCommandConstructor(cmd, 0, `non-default via 1.2.3.4 onlink`, nil), + want: netip.Addr{}, + name: "bad_output", }, { - shell: theOnlyCmd(cmd, 0, "", errors.Error("can't run command")), - want: netip.Addr{}, - name: "err_runcmd", + cmdCons: agh.NewCommandConstructor(cmd, 0, "", errors.Error("can't run command")), + want: netip.Addr{}, + name: "err_runcmd", }, { - shell: theOnlyCmd(cmd, 1, "", nil), - want: netip.Addr{}, - name: "bad_code", + cmdCons: agh.NewCommandConstructor(cmd, 1, "", nil), + want: netip.Addr{}, + name: "bad_code", }} for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - substShell(t, tc.shell.RunCmd) - - assert.Equal(t, tc.want, GatewayIP(ifaceName)) + ctx := testutil.ContextWithTimeout(t, testTimeout) + assert.Equal(t, tc.want, GatewayIP(ctx, tc.cmdCons, ifaceName)) }) } } diff --git a/internal/aghnet/net_linux.go b/internal/aghnet/net_linux.go index 0c0784c6..87baa961 100644 --- a/internal/aghnet/net_linux.go +++ b/internal/aghnet/net_linux.go @@ -4,6 +4,7 @@ package aghnet import ( "bufio" + "context" "fmt" "io" "net/netip" @@ -13,6 +14,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/stringutil" "github.com/google/renameio/v2/maybe" "golang.org/x/sys/unix" @@ -104,7 +106,11 @@ func (n interfaceName) ifacesStaticConfig(r io.Reader) (sub []string, cont bool, return sub, true, s.Err() } -func ifaceHasStaticIP(ifaceName string) (has bool, err error) { +func ifaceHasStaticIP( + _ context.Context, + _ executil.CommandConstructor, + ifaceName string, +) (has bool, err error) { // TODO(a.garipov): Currently, this function returns the first definitive // result. So if /etc/dhcpcd.conf has and /etc/network/interfaces has no // static IP configuration, it will return true. Perhaps this is not the @@ -149,7 +155,11 @@ func findIfaceLine(s *bufio.Scanner, name string) (ok bool) { // ifaceSetStaticIP configures the system to retain its current IP on the // interface through dhcpcd.conf. -func ifaceSetStaticIP(ifaceName string) (err error) { +func ifaceSetStaticIP( + ctx context.Context, + cmdCons executil.CommandConstructor, + ifaceName string, +) (err error) { ipNet := GetSubnet(ifaceName) if !ipNet.Addr().IsValid() { return errors.Error("can't get IP address") @@ -160,7 +170,7 @@ func ifaceSetStaticIP(ifaceName string) (err error) { return err } - gatewayIP := GatewayIP(ifaceName) + gatewayIP := GatewayIP(ctx, cmdCons, ifaceName) add := dhcpcdConfIface(ifaceName, ipNet, gatewayIP) body = append(body, []byte(add)...) diff --git a/internal/aghnet/net_linux_internal_test.go b/internal/aghnet/net_linux_internal_test.go index 89058312..4785183b 100644 --- a/internal/aghnet/net_linux_internal_test.go +++ b/internal/aghnet/net_linux_internal_test.go @@ -7,6 +7,7 @@ import ( "testing" "testing/fstest" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" ) @@ -117,7 +118,8 @@ func TestHasStaticIP(t *testing.T) { t.Run(tc.name, func(t *testing.T) { substRootDirFS(t, tc.rootFsys) - has, err := IfaceHasStaticIP(ifaceName) + ctx := testutil.ContextWithTimeout(t, testTimeout) + has, err := IfaceHasStaticIP(ctx, executil.EmptyCommandConstructor{}, ifaceName) testutil.AssertErrorMsg(t, tc.wantErrMsg, err) tc.wantHas(t, has) diff --git a/internal/aghnet/net_openbsd.go b/internal/aghnet/net_openbsd.go index 073ff109..3e405089 100644 --- a/internal/aghnet/net_openbsd.go +++ b/internal/aghnet/net_openbsd.go @@ -4,15 +4,21 @@ package aghnet import ( "bufio" + "context" "fmt" "io" "strings" "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/netutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) -func ifaceHasStaticIP(ifaceName string) (ok bool, err error) { +func ifaceHasStaticIP( + _ context.Context, + _ executil.CommandConstructor, + ifaceName string, +) (ok bool, err error) { filename := fmt.Sprintf("etc/hostname.%s", ifaceName) return aghos.FileWalker(hostnameIfStaticConfig).Walk(rootDirFS, filename) @@ -39,6 +45,6 @@ func hostnameIfStaticConfig(r io.Reader) (_ []string, ok bool, err error) { return nil, true, s.Err() } -func ifaceSetStaticIP(string) (err error) { +func ifaceSetStaticIP(_ context.Context, _ executil.CommandConstructor, _ string) (err error) { return aghos.Unsupported("setting static ip") } diff --git a/internal/aghnet/net_openbsd_internal_test.go b/internal/aghnet/net_openbsd_internal_test.go index 7239f50f..e9f7b925 100644 --- a/internal/aghnet/net_openbsd_internal_test.go +++ b/internal/aghnet/net_openbsd_internal_test.go @@ -8,6 +8,8 @@ import ( "testing" "testing/fstest" + "github.com/AdguardTeam/golibs/osutil/executil" + "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -62,7 +64,8 @@ func TestIfaceHasStaticIP(t *testing.T) { t.Run(tc.name, func(t *testing.T) { substRootDirFS(t, tc.rootFsys) - has, err := IfaceHasStaticIP(ifaceName) + ctx := testutil.ContextWithTimeout(t, testTimeout) + has, err := IfaceHasStaticIP(ctx, executil.EmptyCommandConstructor{}, ifaceName) require.NoError(t, err) tc.wantHas(t, has) diff --git a/internal/aghnet/net_windows.go b/internal/aghnet/net_windows.go index 1a17ab4d..164bb6fc 100644 --- a/internal/aghnet/net_windows.go +++ b/internal/aghnet/net_windows.go @@ -3,12 +3,14 @@ package aghnet import ( + "context" "io" "syscall" "time" "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/osutil/executil" "golang.org/x/sys/windows" ) @@ -16,11 +18,15 @@ func canBindPrivilegedPorts() (can bool, err error) { return true, nil } -func ifaceHasStaticIP(string) (ok bool, err error) { +func ifaceHasStaticIP( + _ context.Context, + _ executil.CommandConstructor, + _ string, +) (ok bool, err error) { return false, aghos.Unsupported("checking static ip") } -func ifaceSetStaticIP(string) (err error) { +func ifaceSetStaticIP(_ context.Context, _ executil.CommandConstructor, _ string) (err error) { return aghos.Unsupported("setting static ip") } diff --git a/internal/aghos/os.go b/internal/aghos/os.go index 023576ad..5de82706 100644 --- a/internal/aghos/os.go +++ b/internal/aghos/os.go @@ -19,6 +19,7 @@ import ( "strings" "github.com/AdguardTeam/golibs/errors" + "github.com/AdguardTeam/golibs/ioutil" "github.com/AdguardTeam/golibs/osutil" "github.com/AdguardTeam/golibs/osutil/executil" ) @@ -52,41 +53,43 @@ func HaveAdminRights() (bool, error) { const MaxCmdOutputSize = 64 * 1024 // RunCommand runs shell command. -func RunCommand(command string, arguments ...string) (code int, output []byte, err error) { - // TODO(s.chzhen): Pass context. - ctx := context.TODO() - +// +// TODO(s.chzhen): Consider removing this after addressing the current behavior +// where a non-zero exit code is returned together with a nil error. +func RunCommand( + ctx context.Context, + cmdCons executil.CommandConstructor, + command string, + arguments ...string, +) (code int, output []byte, err error) { stdoutBuf := bytes.Buffer{} stderrBuf := bytes.Buffer{} err = executil.Run( ctx, - executil.SystemCommandConstructor{}, + cmdCons, &executil.CommandConfig{ Path: command, Args: arguments, - Stdout: &stdoutBuf, + Stdout: ioutil.NewTruncatedWriter(&stdoutBuf, MaxCmdOutputSize), Stderr: &stderrBuf, }, ) - out := stdoutBuf.Bytes() - if len(out) > MaxCmdOutputSize { - out = out[:MaxCmdOutputSize] + if err == nil { + return osutil.ExitCodeSuccess, stdoutBuf.Bytes(), nil } code, ok := executil.ExitCodeFromError(err) - if err != nil { - if ok { - return code, stderrBuf.Bytes(), nil - } - - return osutil.ExitCodeFailure, - nil, - fmt.Errorf("command %q failed: %w: %s", command, err, out) + if ok { + // Mirror the old behavior and return a nil-error on non-zero code + // status. + return code, stderrBuf.Bytes(), nil } - return code, out, nil + return osutil.ExitCodeFailure, + nil, + fmt.Errorf("command %q failed: %w: %s", command, err, stdoutBuf.Bytes()) } // PIDByCommand searches for process named command and returns its PID ignoring @@ -102,6 +105,8 @@ func PIDByCommand( psArgs := []string{"-A", "-o", "pid=", "-o", "comm="} + l.DebugContext(ctx, "executing", "cmd", psCmd, "args", psArgs) + // Don't use -C flag here since it's a feature of linux's ps // implementation. Use POSIX-compatible flags instead. // diff --git a/internal/arpdb/arpdb.go b/internal/arpdb/arpdb.go index 13cabd0c..87385b8b 100644 --- a/internal/arpdb/arpdb.go +++ b/internal/arpdb/arpdb.go @@ -4,6 +4,7 @@ package arpdb import ( "bufio" "bytes" + "context" "fmt" "log/slog" "net" @@ -11,18 +12,15 @@ import ( "slices" "sync" - "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) // Variables and functions to substitute in tests. var ( - // aghosRunCommand is the function to run shell commands. - aghosRunCommand = aghos.RunCommand - // rootDirFS is the filesystem pointing to the root directory. rootDirFS = osutil.RootDirFS() ) @@ -40,7 +38,7 @@ type Interface interface { // New returns the [Interface] properly initialized for the OS. func New(logger *slog.Logger) (arp Interface) { - return newARPDB(logger) + return newARPDB(logger, executil.SystemCommandConstructor{}) } // Empty is the [Interface] implementation that does nothing. @@ -164,11 +162,12 @@ type parseNeighsFunc func(logger *slog.Logger, sc *bufio.Scanner, lenHint int) ( // cmdARPDB is the implementation of the [Interface] that uses command line to // retrieve data. type cmdARPDB struct { - logger *slog.Logger - parse parseNeighsFunc - ns *neighs - cmd string - args []string + logger *slog.Logger + cmdCons executil.CommandConstructor + parse parseNeighsFunc + ns *neighs + cmd string + args []string } // type check @@ -178,14 +177,26 @@ var _ Interface = (*cmdARPDB)(nil) func (arp *cmdARPDB) Refresh() (err error) { defer func() { err = errors.Annotate(err, "cmd arpdb: %w") }() - code, out, err := aghosRunCommand(arp.cmd, arp.args...) + var stdout bytes.Buffer + err = executil.Run( + // TODO(s.chzhen): Pass context. + context.TODO(), + arp.cmdCons, + &executil.CommandConfig{ + Path: arp.cmd, + Args: arp.args, + Stdout: &stdout, + }, + ) + + code, _ := executil.ExitCodeFromError(err) if err != nil { return fmt.Errorf("running command: %w", err) } else if code != 0 { return fmt.Errorf("running command: unexpected exit code %d", code) } - sc := bufio.NewScanner(bytes.NewReader(out)) + sc := bufio.NewScanner(bytes.NewReader(stdout.Bytes())) ns := arp.parse(arp.logger, sc, arp.ns.len()) if err = sc.Err(); err != nil { // TODO(e.burkov): This error seems unreachable. Investigate. diff --git a/internal/arpdb/arpdb_bsd.go b/internal/arpdb/arpdb_bsd.go index e7357603..922b8c99 100644 --- a/internal/arpdb/arpdb_bsd.go +++ b/internal/arpdb/arpdb_bsd.go @@ -9,12 +9,14 @@ import ( "sync" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) -func newARPDB(logger *slog.Logger) (arp *cmdARPDB) { +func newARPDB(logger *slog.Logger, cmdCons executil.CommandConstructor) (arp *cmdARPDB) { return &cmdARPDB{ - logger: logger, - parse: parseArpA, + logger: logger, + cmdCons: cmdCons, + parse: parseArpA, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), diff --git a/internal/arpdb/arpdb_internal_test.go b/internal/arpdb/arpdb_internal_test.go index 50cd6661..0b40ccfa 100644 --- a/internal/arpdb/arpdb_internal_test.go +++ b/internal/arpdb/arpdb_internal_test.go @@ -1,15 +1,14 @@ package arpdb import ( - "fmt" "io/fs" "net" "net/netip" "os" - "strings" "sync" "testing" + "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/testutil" @@ -23,43 +22,6 @@ var testdata fs.FS = os.DirFS("./testdata") // RunCmdFunc is the signature of aghos.RunCommand function. type RunCmdFunc func(cmd string, args ...string) (code int, out []byte, err error) -// substShell replaces the the aghos.RunCommand function used throughout the -// package with rc for tests ran under t. -func substShell(t testing.TB, rc RunCmdFunc) { - t.Helper() - - prev := aghosRunCommand - t.Cleanup(func() { aghosRunCommand = prev }) - aghosRunCommand = rc -} - -// mapShell is a substitution of aghos.RunCommand that maps the command to it's -// execution result. It's only needed to simplify testing. -// -// TODO(e.burkov): Perhaps put all the shell interactions behind an interface. -type mapShell map[string]struct { - err error - out string - code int -} - -// theOnlyCmd returns mapShell that only handles a single command and arguments -// combination from cmd. -func theOnlyCmd(cmd string, code int, out string, err error) (s mapShell) { - return mapShell{cmd: {code: code, out: out, err: err}} -} - -// RunCmd is a RunCmdFunc handled by s. -func (s mapShell) RunCmd(cmd string, args ...string) (code int, out []byte, err error) { - key := strings.Join(append([]string{cmd}, args...), " ") - ret, ok := s[key] - if !ok { - return 0, nil, fmt.Errorf("unexpected shell command %q", key) - } - - return ret.code, []byte(ret.out), ret.err -} - func Test_New(t *testing.T) { var a Interface require.NotPanics(t, func() { a = New(slogutil.NewDiscardLogger()) }) @@ -212,8 +174,7 @@ func TestCmdARPDB_arpa(t *testing.T) { } t.Run("arp_a", func(t *testing.T) { - sh := theOnlyCmd("cmd", 0, arpAOutput, nil) - substShell(t, sh.RunCmd) + a.cmdCons = agh.NewCommandConstructor("cmd", 0, arpAOutput, nil) err := a.Refresh() require.NoError(t, err) @@ -222,24 +183,25 @@ func TestCmdARPDB_arpa(t *testing.T) { }) t.Run("runcmd_error", func(t *testing.T) { - sh := theOnlyCmd("cmd", 0, "", errors.Error("can't run")) - substShell(t, sh.RunCmd) + a.cmdCons = agh.NewCommandConstructor("cmd", 0, "", errors.Error("can't run")) err := a.Refresh() - testutil.AssertErrorMsg(t, "cmd arpdb: running command: can't run", err) + testutil.AssertErrorMsg(t, "cmd arpdb: running command: running: can't run", err) }) t.Run("bad_code", func(t *testing.T) { - sh := theOnlyCmd("cmd", 1, "", nil) - substShell(t, sh.RunCmd) + a.cmdCons = agh.NewCommandConstructor("cmd", 1, "", nil) err := a.Refresh() - testutil.AssertErrorMsg(t, "cmd arpdb: running command: unexpected exit code 1", err) + testutil.AssertErrorMsg( + t, + "cmd arpdb: running command: running: exit code 1", + err, + ) }) t.Run("empty", func(t *testing.T) { - sh := theOnlyCmd("cmd", 0, "", nil) - substShell(t, sh.RunCmd) + a.cmdCons = agh.NewCommandConstructor("cmd", 0, "", nil) err := a.Refresh() require.NoError(t, err) diff --git a/internal/arpdb/arpdb_linux.go b/internal/arpdb/arpdb_linux.go index 364d2ce9..518e7ea4 100644 --- a/internal/arpdb/arpdb_linux.go +++ b/internal/arpdb/arpdb_linux.go @@ -14,10 +14,11 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/stringutil" ) -func newARPDB(logger *slog.Logger) (arp *arpdbs) { +func newARPDB(logger *slog.Logger, cmdCons executil.CommandConstructor) (arp *arpdbs) { // Use the common storage among the implementations. ns := &neighs{ mu: &sync.RWMutex{}, @@ -40,10 +41,11 @@ func newARPDB(logger *slog.Logger) (arp *arpdbs) { }, // Then, try "arp -a -n". &cmdARPDB{ - logger: logger, - parse: parseF, - ns: ns, - cmd: "arp", + logger: logger, + cmdCons: cmdCons, + parse: parseF, + ns: ns, + cmd: "arp", // Use -n flag to avoid resolving the hostnames of the neighbors. // By default ARP attempts to resolve the hostnames via DNS. See // man 8 arp. @@ -53,11 +55,12 @@ func newARPDB(logger *slog.Logger) (arp *arpdbs) { }, // Finally, try "ip neigh". &cmdARPDB{ - logger: logger, - parse: parseIPNeigh, - ns: ns, - cmd: "ip", - args: []string{"neigh"}, + logger: logger, + cmdCons: cmdCons, + parse: parseIPNeigh, + ns: ns, + cmd: "ip", + args: []string{"neigh"}, }, ) } diff --git a/internal/arpdb/arpdb_linux_internal_test.go b/internal/arpdb/arpdb_linux_internal_test.go index cb0cb1d3..b2e2f022 100644 --- a/internal/arpdb/arpdb_linux_internal_test.go +++ b/internal/arpdb/arpdb_linux_internal_test.go @@ -9,6 +9,7 @@ import ( "testing" "testing/fstest" + "github.com/AdguardTeam/AdGuardHome/internal/agh" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -62,18 +63,13 @@ func TestFSysARPDB(t *testing.T) { } func TestCmdARPDB_linux(t *testing.T) { - sh := mapShell{ - "arp -a": {err: nil, out: arpAOutputWrt, code: 0}, - "ip neigh": {err: nil, out: ipNeighOutput, code: 0}, - } - substShell(t, sh.RunCmd) - t.Run("wrt", func(t *testing.T) { a := &cmdARPDB{ - logger: slogutil.NewDiscardLogger(), - parse: parseArpAWrt, - cmd: "arp", - args: []string{"-a"}, + logger: slogutil.NewDiscardLogger(), + cmdCons: agh.NewCommandConstructor("arp -a", 0, arpAOutputWrt, nil), + parse: parseArpAWrt, + cmd: "arp", + args: []string{"-a"}, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), @@ -88,10 +84,11 @@ func TestCmdARPDB_linux(t *testing.T) { t.Run("ip_neigh", func(t *testing.T) { a := &cmdARPDB{ - logger: slogutil.NewDiscardLogger(), - parse: parseIPNeigh, - cmd: "ip", - args: []string{"neigh"}, + logger: slogutil.NewDiscardLogger(), + cmdCons: agh.NewCommandConstructor("ip neigh", 0, ipNeighOutput, nil), + parse: parseIPNeigh, + cmd: "ip", + args: []string{"neigh"}, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), diff --git a/internal/arpdb/arpdb_openbsd.go b/internal/arpdb/arpdb_openbsd.go index 8d6ee657..40b45271 100644 --- a/internal/arpdb/arpdb_openbsd.go +++ b/internal/arpdb/arpdb_openbsd.go @@ -9,12 +9,14 @@ import ( "sync" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) -func newARPDB(logger *slog.Logger) (arp *cmdARPDB) { +func newARPDB(logger *slog.Logger, cmdCons executil.CommandConstructor) (arp *cmdARPDB) { return &cmdARPDB{ - logger: logger, - parse: parseArpA, + logger: logger, + cmdCons: cmdCons, + parse: parseArpA, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), diff --git a/internal/arpdb/arpdb_windows.go b/internal/arpdb/arpdb_windows.go index 878d1d31..f226d65b 100644 --- a/internal/arpdb/arpdb_windows.go +++ b/internal/arpdb/arpdb_windows.go @@ -9,12 +9,14 @@ import ( "sync" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) -func newARPDB(logger *slog.Logger) (arp *cmdARPDB) { +func newARPDB(logger *slog.Logger, cmdCons executil.CommandConstructor) (arp *cmdARPDB) { return &cmdARPDB{ - logger: logger, - parse: parseArpA, + logger: logger, + cmdCons: cmdCons, + parse: parseArpA, ns: &neighs{ mu: &sync.RWMutex{}, ns: make([]Neighbor, 0), diff --git a/internal/dhcpd/http_unix.go b/internal/dhcpd/http_unix.go index 6d6226bd..aaf19f86 100644 --- a/internal/dhcpd/http_unix.go +++ b/internal/dhcpd/http_unix.go @@ -3,6 +3,7 @@ package dhcpd import ( + "context" "encoding/json" "fmt" "io" @@ -20,6 +21,7 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) type v4ServerConfJSON struct { @@ -173,7 +175,10 @@ func (s *server) handleDHCPStatus(w http.ResponseWriter, r *http.Request) { func (s *server) enableDHCP(ifaceName string) (code int, err error) { var hasStaticIP bool - hasStaticIP, err = aghnet.IfaceHasStaticIP(ifaceName) + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + cmdCons := executil.SystemCommandConstructor{} + hasStaticIP, err = aghnet.IfaceHasStaticIP(ctx, cmdCons, ifaceName) if err != nil { if errors.Is(err, os.ErrPermission) { // ErrPermission may happen here on Linux systems where AdGuard Home @@ -202,7 +207,7 @@ func (s *server) enableDHCP(ifaceName string) (code int, err error) { } if !hasStaticIP { - err = aghnet.IfaceSetStaticIP(ifaceName) + err = aghnet.IfaceSetStaticIP(ctx, executil.SystemCommandConstructor{}, ifaceName) if err != nil { err = fmt.Errorf("setting static ip: %w", err) @@ -473,7 +478,9 @@ func newNetInterfaceJSON(iface net.Interface) (out *netInterfaceJSON, err error) return nil, nil } - out.GatewayIP = aghnet.GatewayIP(iface.Name) + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + out.GatewayIP = aghnet.GatewayIP(ctx, executil.SystemCommandConstructor{}, iface.Name) return out, nil } @@ -558,7 +565,10 @@ func (s *server) handleDHCPFindActiveServer(w http.ResponseWriter, r *http.Reque }, } - if isStaticIP, serr := aghnet.IfaceHasStaticIP(ifaceName); serr != nil { + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + cmdCons := executil.SystemCommandConstructor{} + if isStaticIP, serr := aghnet.IfaceHasStaticIP(ctx, cmdCons, ifaceName); serr != nil { result.V4.StaticIP.Static = "error" result.V4.StaticIP.Error = serr.Error() } else if !isStaticIP { diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index f097c065..975ec09b 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -20,6 +20,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghhttp" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/version" + "github.com/AdguardTeam/golibs/container" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/osutil/executil" @@ -202,6 +203,10 @@ func (web *webAPI) handleInstallCheckConfig(w http.ResponseWriter, r *http.Reque // It either checks if we have a static IP // Or if set=true, it tries to set it func handleStaticIP(ip netip.Addr, set bool) staticIPJSON { + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + cmdCons := executil.SystemCommandConstructor{} + resp := staticIPJSON{} interfaceName := aghnet.InterfaceByIP(ip) @@ -215,7 +220,7 @@ func handleStaticIP(ip netip.Addr, set bool) staticIPJSON { if set { // Try to set static IP for the specified interface - err := aghnet.IfaceSetStaticIP(interfaceName) + err := aghnet.IfaceSetStaticIP(ctx, cmdCons, interfaceName) if err != nil { resp.Static = "error" resp.Error = err.Error() @@ -225,7 +230,7 @@ func handleStaticIP(ip netip.Addr, set bool) staticIPJSON { // Fallthrough here even if we set static IP // Check if we have a static IP and return the details - isStaticIP, err := aghnet.IfaceHasStaticIP(interfaceName) + isStaticIP, err := aghnet.IfaceHasStaticIP(ctx, cmdCons, interfaceName) if err != nil { resp.Static = "error" resp.Error = err.Error() @@ -244,65 +249,30 @@ func checkDNSStubListener(ctx context.Context, l *slog.Logger) (ok bool) { return false } - const ( - systemctlCmd = "systemctl" - grepCmd = "grep" - ) + cmds := container.KeyValues[string, []string]{{ + Key: "systemctl", + Value: []string{"is-enabled", "systemd-resolved"}, + }, { + Key: "grep", + Value: []string{"-E", "#?DNSStubListener=yes", "/etc/systemd/resolved.conf"}, + }} - var ( - systemctlArgs = []string{"is-enabled", "systemd-resolved"} - systemctlStdout bytes.Buffer - systemctlStderr bytes.Buffer + for _, cmd := range cmds { + l.DebugContext(ctx, "executing", "cmd", cmd.Key, "args", cmd.Value) - grepArgs = []string{"-E", "#?DNSStubListener=yes", "/etc/systemd/resolved.conf"} - grepStdout bytes.Buffer - grepStderr bytes.Buffer - ) - - l.DebugContext(ctx, "executing", "cmd", systemctlCmd, "args", systemctlArgs) - - err := executil.Run( - ctx, - executil.SystemCommandConstructor{}, - &executil.CommandConfig{ - Path: systemctlCmd, - Args: systemctlArgs, - Stdout: &systemctlStdout, - Stderr: &systemctlStderr, - }, - ) - if err != nil { - l.InfoContext( + err := executil.Run( ctx, - "execution failed", - "cmd", systemctlCmd, - slogutil.KeyError, err, + executil.SystemCommandConstructor{}, + &executil.CommandConfig{ + Path: cmd.Key, + Args: cmd.Value, + }, ) + if err != nil { + l.InfoContext(ctx, "execution failed", "cmd", cmd.Key, slogutil.KeyError, err) - return false - } - - l.DebugContext(ctx, "executing", "cmd", grepCmd, "args", grepArgs) - - err = executil.Run( - ctx, - executil.SystemCommandConstructor{}, - &executil.CommandConfig{ - Path: grepCmd, - Args: grepArgs, - Stdout: &grepStdout, - Stderr: &grepStderr, - }, - ) - if err != nil { - l.InfoContext( - ctx, - "execution failed", - "cmd", grepCmd, - slogutil.KeyError, err, - ) - - return false + return false + } } return true diff --git a/internal/home/home.go b/internal/home/home.go index 9536219d..78a8ae68 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -43,6 +43,7 @@ import ( "github.com/AdguardTeam/golibs/netutil" "github.com/AdguardTeam/golibs/netutil/urlutil" "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) // Global context @@ -820,18 +821,19 @@ func newUpdater( l.DebugContext(ctx, "creating updater", "config_path", confPath) return updater.NewUpdater(&updater.Config{ - Client: conf.Filtering.HTTPClient, - Logger: l, - Version: version.Version(), - Channel: version.Channel(), - GOARCH: runtime.GOARCH, - GOOS: runtime.GOOS, - GOARM: version.GOARM(), - GOMIPS: version.GOMIPS(), - WorkDir: workDir, - ConfName: confPath, - ExecPath: execPath, - VersionCheckURL: versionURL, + Client: conf.Filtering.HTTPClient, + Logger: l, + CommandConstructor: executil.SystemCommandConstructor{}, + Version: version.Version(), + Channel: version.Channel(), + GOARCH: runtime.GOARCH, + GOOS: runtime.GOOS, + GOARM: version.GOARM(), + GOMIPS: version.GOMIPS(), + WorkDir: workDir, + ConfName: confPath, + ExecPath: execPath, + VersionCheckURL: versionURL, }), isCustomURL } diff --git a/internal/home/service.go b/internal/home/service.go index af3d4a70..e63b015f 100644 --- a/internal/home/service.go +++ b/internal/home/service.go @@ -18,6 +18,7 @@ import ( "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/netutil/urlutil" "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/kardianos/service" ) @@ -76,11 +77,11 @@ func (p *program) Stop(_ service.Service) (err error) { // // On OpenWrt, the service utility may not exist. We use our service script // directly in this case. -func svcStatus(s service.Service) (status service.Status, err error) { +func svcStatus(ctx context.Context, s service.Service) (status service.Status, err error) { status, err = s.Status() if err != nil && service.Platform() == "unix-systemv" { var code int - code, err = runInitdCommand("status") + code, err = runInitdCommand(ctx, "status") if err != nil || code != 0 { return service.StatusStopped, nil } @@ -105,7 +106,7 @@ func svcAction(ctx context.Context, l *slog.Logger, s service.Service, action st err = service.Control(s, action) if err != nil && service.Platform() == "unix-systemv" && (action == "start" || action == "stop" || action == "restart") { - _, err = runInitdCommand(action) + _, err = runInitdCommand(ctx, action) } return err @@ -326,7 +327,7 @@ func handleServiceStatusCommand( l *slog.Logger, s service.Service, ) { - status, errSt := svcStatus(s) + status, errSt := svcStatus(ctx, s) if errSt != nil { l.ErrorContext(ctx, "failed to get service status", slogutil.KeyError, errSt) os.Exit(osutil.ExitCodeFailure) @@ -356,7 +357,7 @@ func handleServiceInstallCommand(ctx context.Context, l *slog.Logger, s service. // On OpenWrt it is important to run enable after the service // installation. Otherwise, the service won't start on the system // startup. - _, err = runInitdCommand("enable") + _, err = runInitdCommand(ctx, "enable") if err != nil { l.ErrorContext(ctx, "running init enable", slogutil.KeyError, err) os.Exit(osutil.ExitCodeFailure) @@ -386,7 +387,7 @@ func handleServiceUninstallCommand(ctx context.Context, l *slog.Logger, s servic if aghos.IsOpenWrt() { // On OpenWrt it is important to run disable command first // as it will remove the symlink - _, err := runInitdCommand("disable") + _, err := runInitdCommand(ctx, "disable") if err != nil { l.ErrorContext(ctx, "running init disable", slogutil.KeyError, err) os.Exit(osutil.ExitCodeFailure) @@ -458,10 +459,11 @@ func configureService(c *service.Config) { // runInitdCommand runs init.d service command // returns command code or error if any -func runInitdCommand(action string) (int, error) { +func runInitdCommand(ctx context.Context, action string) (int, error) { confPath := "/etc/init.d/" + serviceName // Pass the script and action as a single string argument. - code, _, err := aghos.RunCommand("sh", "-c", confPath+" "+action) + cmdCons := executil.SystemCommandConstructor{} + code, _, err := aghos.RunCommand(ctx, cmdCons, "sh", "-c", confPath+" "+action) return code, err } diff --git a/internal/home/service_linux.go b/internal/home/service_linux.go index 4efb0e37..6b43630c 100644 --- a/internal/home/service_linux.go +++ b/internal/home/service_linux.go @@ -87,7 +87,9 @@ func (svc *sysvService) Install() (err error) { return err } - _, _, err = aghos.RunCommand("update-rc.d", svc.name, "defaults") + cmdCons := executil.SystemCommandConstructor{} + // TODO(s.chzhen): Pass context. + _, _, err = aghos.RunCommand(context.TODO(), cmdCons, "update-rc.d", svc.name, "defaults") // Don't wrap an error since it's informative enough as is. return err @@ -102,7 +104,9 @@ func (svc *sysvService) Uninstall() (err error) { return err } - _, _, err = aghos.RunCommand("update-rc.d", svc.name, "remove") + cmdCons := executil.SystemCommandConstructor{} + // TODO(s.chzhen): Pass context. + _, _, err = aghos.RunCommand(context.TODO(), cmdCons, "update-rc.d", svc.name, "remove") // Don't wrap an error since it's informative enough as is. return err diff --git a/internal/home/service_openbsd.go b/internal/home/service_openbsd.go index b4f29c7a..db8f1d65 100644 --- a/internal/home/service_openbsd.go +++ b/internal/home/service_openbsd.go @@ -4,6 +4,7 @@ package home import ( "cmp" + "context" "fmt" "os" "os/signal" @@ -15,6 +16,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghos" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/kardianos/service" ) @@ -210,8 +212,11 @@ func (s *openbsdRunComService) configureSysStartup(enable bool) (err error) { cmd = "disable" } + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + cmdCons := executil.SystemCommandConstructor{} var code int - code, _, err = aghos.RunCommand("rcctl", cmd, s.cfg.Name) + code, _, err = aghos.RunCommand(ctx, cmdCons, "rcctl", cmd, s.cfg.Name) if err != nil { return err } else if code != 0 { @@ -312,11 +317,15 @@ func (s *openbsdRunComService) runCom(cmd string) (out string, err error) { return "", err } + // TODO(s.chzhen): Pass context. + ctx := context.TODO() + cmdCons := executil.SystemCommandConstructor{} + // TODO(e.burkov): It's possible that os.ErrNotExist is caused by // something different than the service script's non-existence. Keep it // in mind, when replace the aghos.RunCommand. var outData []byte - _, outData, err = aghos.RunCommand(scriptPath, cmd) + _, outData, err = aghos.RunCommand(ctx, cmdCons, scriptPath, cmd) if errors.Is(err, os.ErrNotExist) { return "", service.ErrNotInstalled } diff --git a/internal/updater/check_test.go b/internal/updater/check_test.go index 4da1c876..a94ed5a2 100644 --- a/internal/updater/check_test.go +++ b/internal/updater/check_test.go @@ -10,6 +10,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghtest" "github.com/AdguardTeam/AdGuardHome/internal/updater" "github.com/AdguardTeam/AdGuardHome/internal/version" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -58,13 +59,14 @@ func TestUpdater_VersionInfo(t *testing.T) { fakeURL := srvURL.JoinPath("adguardhome", version.ChannelBeta, "version.json") u := updater.NewUpdater(&updater.Config{ - Client: srv.Client(), - Logger: testLogger, - Version: "v0.103.0-beta.1", - Channel: version.ChannelBeta, - GOARCH: "arm", - GOOS: "linux", - VersionCheckURL: fakeURL, + Client: srv.Client(), + Logger: testLogger, + CommandConstructor: executil.EmptyCommandConstructor{}, + Version: "v0.103.0-beta.1", + Channel: version.ChannelBeta, + GOARCH: "arm", + GOOS: "linux", + VersionCheckURL: fakeURL, }) ctx := testutil.ContextWithTimeout(t, testTimeout) @@ -132,15 +134,16 @@ func TestUpdater_VersionInfo_others(t *testing.T) { for _, tc := range testCases { u := updater.NewUpdater(&updater.Config{ - Client: fakeClient, - Logger: testLogger, - Version: "v0.103.0-beta.1", - Channel: version.ChannelBeta, - GOOS: "linux", - GOARCH: tc.arch, - GOARM: tc.arm, - GOMIPS: tc.mips, - VersionCheckURL: fakeURL, + Client: fakeClient, + Logger: testLogger, + CommandConstructor: executil.EmptyCommandConstructor{}, + Version: "v0.103.0-beta.1", + Channel: version.ChannelBeta, + GOOS: "linux", + GOARCH: tc.arch, + GOARM: tc.arm, + GOMIPS: tc.mips, + VersionCheckURL: fakeURL, }) ctx := testutil.ContextWithTimeout(t, testTimeout) diff --git a/internal/updater/updater.go b/internal/updater/updater.go index 82fde671..f45581f6 100644 --- a/internal/updater/updater.go +++ b/internal/updater/updater.go @@ -34,6 +34,8 @@ type Updater struct { client *http.Client logger *slog.Logger + cmdCons executil.CommandConstructor + version string channel string goarch string @@ -89,6 +91,9 @@ type Config struct { // be nil, see [DefaultVersionURL]. VersionCheckURL *url.URL + // CommandConstructor is used to run external commands. It must not be nil. + CommandConstructor executil.CommandConstructor + // Version is the current AdGuard Home version. It must not be empty. Version string @@ -130,6 +135,8 @@ func NewUpdater(conf *Config) *Updater { client: conf.Client, logger: conf.Logger, + cmdCons: conf.CommandConstructor, + version: conf.Version, channel: conf.Channel, goarch: conf.GOARCH, @@ -293,9 +300,11 @@ func (u *Updater) check(ctx context.Context) (err error) { stderr bytes.Buffer ) + u.logger.DebugContext(ctx, "executing", "cmd", u.updateExeName, "args", args) + err = executil.Run( ctx, - executil.SystemCommandConstructor{}, + u.cmdCons, &executil.CommandConfig{ Path: u.updateExeName, Args: args, diff --git a/internal/updater/updater_internal_test.go b/internal/updater/updater_internal_test.go index 3d96a8ff..195d86b2 100644 --- a/internal/updater/updater_internal_test.go +++ b/internal/updater/updater_internal_test.go @@ -10,6 +10,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/aghtest" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -58,13 +59,14 @@ func TestUpdater_internal(t *testing.T) { fakeURL = fakeURL.JoinPath(tc.archiveName) u := NewUpdater(&Config{ - Client: fakeClient, - Logger: slogutil.NewDiscardLogger(), - GOOS: tc.os, - Version: "v0.103.0", - ExecPath: exePath, - WorkDir: wd, - ConfName: yamlPath, + Client: fakeClient, + Logger: slogutil.NewDiscardLogger(), + CommandConstructor: executil.EmptyCommandConstructor{}, + GOOS: tc.os, + Version: "v0.103.0", + ExecPath: exePath, + WorkDir: wd, + ConfName: yamlPath, // TODO(e.burkov): Rewrite the test to use a fake version check // URL with a fake URLs for the package files. VersionCheckURL: &url.URL{}, diff --git a/internal/updater/updater_test.go b/internal/updater/updater_test.go index dfef0b10..f52fad8d 100644 --- a/internal/updater/updater_test.go +++ b/internal/updater/updater_test.go @@ -15,6 +15,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/updater" "github.com/AdguardTeam/AdGuardHome/internal/version" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/osutil/executil" "github.com/AdguardTeam/golibs/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -76,15 +77,16 @@ func TestUpdater_Update(t *testing.T) { require.NoError(t, err) u := updater.NewUpdater(&updater.Config{ - Client: srv.Client(), - Logger: testLogger, - GOARCH: "amd64", - GOOS: "linux", - Version: "v0.103.0", - ConfName: yamlPath, - WorkDir: wd, - ExecPath: exePath, - VersionCheckURL: versionCheckURL, + Client: srv.Client(), + Logger: testLogger, + CommandConstructor: executil.EmptyCommandConstructor{}, + GOARCH: "amd64", + GOOS: "linux", + Version: "v0.103.0", + ConfName: yamlPath, + WorkDir: wd, + ExecPath: exePath, + VersionCheckURL: versionCheckURL, }) ctx := testutil.ContextWithTimeout(t, testTimeout) diff --git a/scripts/translations/main.go b/scripts/translations/main.go index cfd3ab9b..82b664d8 100644 --- a/scripts/translations/main.go +++ b/scripts/translations/main.go @@ -22,6 +22,7 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" "github.com/AdguardTeam/golibs/osutil" + "github.com/AdguardTeam/golibs/osutil/executil" ) const ( @@ -95,7 +96,7 @@ func main() { errors.Check(cli.upload()) case "auto-add": - err := autoAdd(conf.LocalizableFiles[0]) + err := autoAdd(ctx, l, conf.LocalizableFiles[0]) errors.Check(err) default: usage("unknown command") @@ -394,10 +395,12 @@ func findUnused(fileNames []string, loc locales) (err error) { // autoAdd adds locales with additions to the git and restores locales with // deletions. -func autoAdd(basePath string) (err error) { +func autoAdd(ctx context.Context, l *slog.Logger, basePath string) (err error) { defer func() { err = errors.Annotate(err, "auto add: %w") }() - adds, dels, err := changedLocales() + cmdCons := executil.SystemCommandConstructor{} + + adds, dels, err := changedLocales(ctx, l, cmdCons) if err != nil { // Don't wrap the error since it's informative enough as is. return err @@ -407,13 +410,13 @@ func autoAdd(basePath string) (err error) { return errors.Error("base locale contains deletions") } - err = handleAdds(adds) + err = handleAdds(ctx, l, cmdCons, adds) if err != nil { // Don't wrap the error since it's informative enough as is. return nil } - err = handleDels(dels) + err = handleDels(ctx, l, cmdCons, dels) if err != nil { // Don't wrap the error since it's informative enough as is. return nil @@ -422,14 +425,24 @@ func autoAdd(basePath string) (err error) { return nil } +// gitCmd is the shell command for Git. +const gitCmd = "git" + // handleAdds adds locales with additions to the git. -func handleAdds(locales []string) (err error) { +func handleAdds( + ctx context.Context, + l *slog.Logger, + cmdCons executil.CommandConstructor, + locales []string, +) (err error) { if len(locales) == 0 { return nil } - args := append([]string{"add"}, locales...) - code, out, err := aghos.RunCommand("git", args...) + gitArgs := append([]string{"add"}, locales...) + l.DebugContext(ctx, "executing", "cmd", gitCmd, "args", gitArgs) + + code, out, err := aghos.RunCommand(ctx, cmdCons, gitCmd, gitArgs...) if err != nil || code != 0 { return fmt.Errorf("git add exited with code %d output %q: %w", code, out, err) @@ -439,13 +452,20 @@ func handleAdds(locales []string) (err error) { } // handleDels restores locales with deletions. -func handleDels(locales []string) (err error) { +func handleDels( + ctx context.Context, + l *slog.Logger, + cmdCons executil.CommandConstructor, + locales []string, +) (err error) { if len(locales) == 0 { return nil } - args := append([]string{"restore"}, locales...) - code, out, err := aghos.RunCommand("git", args...) + gitArgs := append([]string{"restore"}, locales...) + l.DebugContext(ctx, "executing", "cmd", gitCmd, "args", gitArgs) + + code, out, err := aghos.RunCommand(ctx, cmdCons, gitCmd, gitArgs...) if err != nil || code != 0 { return fmt.Errorf("git restore exited with code %d output %q: %w", code, out, err) @@ -457,14 +477,17 @@ func handleDels(locales []string) (err error) { // changedLocales returns cleaned paths of locales with changes or error. adds // is the list of locales with only additions. dels is the list of locales // with only deletions. -func changedLocales() (adds, dels []string, err error) { +func changedLocales( + ctx context.Context, + l *slog.Logger, + cmdCons executil.CommandConstructor, +) (adds, dels []string, err error) { defer func() { err = errors.Annotate(err, "getting changes: %w") }() - const gitCmd = "git" - gitArgs := []string{"diff", "--numstat", localesDir} + l.DebugContext(ctx, "executing", "cmd", gitCmd, "args", gitArgs) - code, out, err := aghos.RunCommand(gitCmd, gitArgs...) + code, out, err := aghos.RunCommand(ctx, cmdCons, gitCmd, gitArgs...) if err != nil || code != 0 { return nil, nil, fmt.Errorf("executing cmd: %w", err) }