Refactor stateManager parameter to use value type instead of pointer in multiple functions

This commit is contained in:
Hakan Sariman
2025-03-18 20:34:44 +08:00
parent 01d01ac16f
commit a01e5abfee
38 changed files with 629 additions and 85 deletions
+2 -2
View File
@@ -22,7 +22,7 @@ var (
}
)
type repairConfFn func([]string, string, *resolvConf, *statemanager.Manager) error
type repairConfFn func([]string, string, *resolvConf, statemanager.Manager) error
type repair struct {
operationFile string
@@ -42,7 +42,7 @@ func newRepair(operationFile string, updateFn repairConfFn) *repair {
}
}
func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP string, stateManager *statemanager.Manager) {
func (f *repair) watchFileChanges(nbSearchDomains []string, nbNameserverIP string, stateManager statemanager.Manager) {
if f.inotify != nil {
return
}
+2 -2
View File
@@ -105,7 +105,7 @@ nameserver 8.8.8.8`,
var changed bool
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
updateFn := func([]string, string, *resolvConf, *statemanager.Manager) error {
updateFn := func([]string, string, *resolvConf, statemanager.Manager) error {
changed = true
cancel()
return nil
@@ -152,7 +152,7 @@ searchdomain netbird.cloud something`
var changed bool
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
updateFn := func([]string, string, *resolvConf, *statemanager.Manager) error {
updateFn := func([]string, string, *resolvConf, statemanager.Manager) error {
changed = true
cancel()
return nil
+2 -2
View File
@@ -48,7 +48,7 @@ func (f *fileConfigurator) supportCustomPort() bool {
return false
}
func (f *fileConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (f *fileConfigurator) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
backupFileExist := f.isBackupFileExist()
if !config.RouteAll {
if backupFileExist {
@@ -86,7 +86,7 @@ func (f *fileConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *st
return nil
}
func (f *fileConfigurator) updateConfig(nbSearchDomains []string, nbNameserverIP string, cfg *resolvConf, stateManager *statemanager.Manager) error {
func (f *fileConfigurator) updateConfig(nbSearchDomains []string, nbNameserverIP string, cfg *resolvConf, stateManager statemanager.Manager) error {
searchDomainList := mergeSearchDomains(nbSearchDomains, cfg.searchDomains)
nameServers := generateNsList(nbNameserverIP, cfg)
+5 -5
View File
@@ -17,7 +17,7 @@ const (
)
type hostManager interface {
applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error
applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error
restoreHostDNS() error
supportCustomPort() bool
string() string
@@ -43,14 +43,14 @@ type DomainConfig struct {
}
type mockHostConfigurator struct {
applyDNSConfigFunc func(config HostDNSConfig, stateManager *statemanager.Manager) error
applyDNSConfigFunc func(config HostDNSConfig, stateManager statemanager.Manager) error
restoreHostDNSFunc func() error
supportCustomPortFunc func() bool
restoreUncleanShutdownDNSFunc func(*netip.Addr) error
stringFunc func() string
}
func (m *mockHostConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (m *mockHostConfigurator) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
if m.applyDNSConfigFunc != nil {
return m.applyDNSConfigFunc(config, stateManager)
}
@@ -80,7 +80,7 @@ func (m *mockHostConfigurator) string() string {
func newNoopHostMocker() hostManager {
return &mockHostConfigurator{
applyDNSConfigFunc: func(config HostDNSConfig, stateManager *statemanager.Manager) error { return nil },
applyDNSConfigFunc: func(config HostDNSConfig, stateManager statemanager.Manager) error { return nil },
restoreHostDNSFunc: func() error { return nil },
supportCustomPortFunc: func() bool { return true },
restoreUncleanShutdownDNSFunc: func(*netip.Addr) error { return nil },
@@ -122,7 +122,7 @@ func dnsConfigToHostDNSConfig(dnsConfig nbdns.Config, ip string, port int) HostD
type noopHostConfigurator struct{}
func (n noopHostConfigurator) applyDNSConfig(HostDNSConfig, *statemanager.Manager) error {
func (n noopHostConfigurator) applyDNSConfig(HostDNSConfig, statemanager.Manager) error {
return nil
}
+1 -1
View File
@@ -11,7 +11,7 @@ func newHostManager() (*androidHostManager, error) {
return &androidHostManager{}, nil
}
func (a androidHostManager) applyDNSConfig(HostDNSConfig, *statemanager.Manager) error {
func (a androidHostManager) applyDNSConfig(HostDNSConfig, statemanager.Manager) error {
return nil
}
+7 -2
View File
@@ -5,6 +5,7 @@ package dns
import (
"bufio"
"bytes"
"errors"
"fmt"
"io"
"net"
@@ -49,7 +50,7 @@ func (s *systemConfigurator) supportCustomPort() bool {
return true
}
func (s *systemConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (s *systemConfigurator) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
var err error
if err := stateManager.UpdateState(&ShutdownState{}); err != nil {
@@ -200,8 +201,12 @@ func (s *systemConfigurator) recordSystemDNSSettings(force bool) error {
func (s *systemConfigurator) getSystemDNSSettings() (SystemDNSSettings, error) {
primaryServiceKey, _, err := s.getPrimaryService()
if err != nil || primaryServiceKey == "" {
if err == nil {
err = errors.New("primary service key not found")
}
return SystemDNSSettings{}, fmt.Errorf("couldn't find the primary service key: %w", err)
}
dnsServiceKey := getKeyWithInput(primaryServiceStateKeyFormat, primaryServiceKey)
line := buildCommandLine("show", dnsServiceKey, "")
stdinCommands := wrapCommand(line)
@@ -379,7 +384,7 @@ func buildWriteStateOperation(operation, state, commands string) string {
return fmt.Sprintf("d.init\n%s %s\n%s\nset %s\n", operation, state, commands, state)
}
func runSystemConfigCommand(command string) ([]byte, error) {
var runSystemConfigCommand = func(command string) ([]byte, error) {
cmd := exec.Command(scutilPath)
cmd.Stdin = strings.NewReader(command)
out, err := cmd.Output()
+210
View File
@@ -0,0 +1,210 @@
package dns
import (
"errors"
"os/exec"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"go.uber.org/mock/gomock"
"github.com/netbirdio/netbird/client/internal/statemanager/mocks"
)
// MockCommander to mock exec.Command
type MockCommander struct {
mock.Mock
}
func (m *MockCommander) Command(name string, arg ...string) *exec.Cmd {
args := m.Called(name, arg)
return args.Get(0).(*exec.Cmd)
}
func TestNewHostManager(t *testing.T) {
tests := []struct {
name string
wantErr bool
}{
{
name: "successful creation",
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := newHostManager()
if tt.wantErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
assert.NotNil(t, got)
assert.NotNil(t, got.createdKeys)
})
}
}
func TestApplyDNSConfig(t *testing.T) {
type mockSetup struct {
stateManagerError error
commandOutput []byte
commandError error
}
tests := []struct {
name string
config HostDNSConfig
mockSetup mockSetup
wantErr bool
}{
{
name: "successful apply with search domains",
config: HostDNSConfig{
RouteAll: true,
Domains: []DomainConfig{
{Domain: "example.com", MatchOnly: false},
{Domain: "test.com", MatchOnly: true},
},
ServerIP: "1.1.1.1",
ServerPort: 53,
},
mockSetup: mockSetup{
stateManagerError: nil,
commandOutput: []byte(`
PrimaryService : ABC123
Router : 192.168.1.1
DomainName : example.com
SearchDomains : <array> {
0 : test.com
}
ServerAddresses : <array> {
0 : 1.1.1.1
}
`),
commandError: nil,
},
wantErr: false,
},
{
name: "state manager error",
config: HostDNSConfig{
ServerIP: "1.1.1.1",
},
mockSetup: mockSetup{
stateManagerError: errors.New("state error"),
},
wantErr: false, // Function does not return an error, it only logs it.
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Setup mocks
s := &systemConfigurator{
createdKeys: make(map[string]struct{}),
}
ctrl := gomock.NewController(t)
defer ctrl.Finish() // Ensures all expectations are met
mockState := mocks.NewMockManager(ctrl)
mockCmd := new(MockCommander)
// Mock UpdateState
mockState.EXPECT().UpdateState(gomock.Any()).Return(tt.mockSetup.stateManagerError).AnyTimes()
// Mock all expected command executions
// mockCmd.On("Command", dscacheutilPath, "-flushcache").Return(&exec.Cmd{}).Once()
// mockCmd.On("Command", "killall", "-HUP", "mDNSResponder").Return(&exec.Cmd{}).Once()
// mockCmd.On("Command", scutilPath).Return(&exec.Cmd{}).Once() // For runSystemConfigCommand
// Mock `runSystemConfigCommand`
originalRunCommand := runSystemConfigCommand
runSystemConfigCommand = func(command string) ([]byte, error) {
return tt.mockSetup.commandOutput, tt.mockSetup.commandError
}
defer func() { runSystemConfigCommand = originalRunCommand }()
err := s.applyDNSConfig(tt.config, mockState)
if tt.wantErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
mockCmd.AssertExpectations(t) // Ensure Command() is called
})
}
}
func TestGetSystemDNSSettings(t *testing.T) {
tests := []struct {
name string
commandOutput []byte
commandError error
wantSettings SystemDNSSettings
wantErr bool
}{
{
name: "successful retrieval",
commandOutput: []byte(`
PrimaryService : ABC123
Router : 192.168.1.1
---
DomainName : example.com
SearchDomains : <array> {
0 : test.com
}
ServerAddresses : <array> {
0 : 1.1.1.1
}
`),
wantSettings: SystemDNSSettings{
Domains: []string{"example.com", "test.com"},
ServerIP: "1.1.1.1",
ServerPort: 53,
},
wantErr: false,
},
{
name: "command error",
commandError: errors.New("command failed"),
wantErr: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := &systemConfigurator{
createdKeys: make(map[string]struct{}),
}
originalRunCommand := runSystemConfigCommand
runSystemConfigCommand = func(command string) ([]byte, error) {
return tt.commandOutput, tt.commandError
}
defer func() { runSystemConfigCommand = originalRunCommand }()
got, err := s.getSystemDNSSettings()
if tt.wantErr {
assert.Error(t, err)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wantSettings, got)
})
}
}
func TestSupportCustomPort(t *testing.T) {
s := &systemConfigurator{}
assert.True(t, s.supportCustomPort())
}
func TestString(t *testing.T) {
s := &systemConfigurator{}
assert.Equal(t, "scutil", s.string())
}
+1 -1
View File
@@ -20,7 +20,7 @@ func newHostManager(dnsManager IosDnsManager) (*iosHostManager, error) {
}, nil
}
func (a iosHostManager) applyDNSConfig(config HostDNSConfig, _ *statemanager.Manager) error {
func (a iosHostManager) applyDNSConfig(config HostDNSConfig, _ statemanager.Manager) error {
jsonData, err := json.Marshal(config)
if err != nil {
return fmt.Errorf("marshal: %w", err)
+1 -1
View File
@@ -74,7 +74,7 @@ func (r *registryConfigurator) supportCustomPort() bool {
return false
}
func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
if config.RouteAll {
if err := r.addDNSSetupForAll(config.ServerIP); err != nil {
return fmt.Errorf("add dns setup: %w", err)
+1 -1
View File
@@ -103,7 +103,7 @@ func (n *networkManagerDbusConfigurator) supportCustomPort() bool {
return false
}
func (n *networkManagerDbusConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (n *networkManagerDbusConfigurator) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
connSettings, configVersion, err := n.getAppliedConnectionSettings()
if err != nil {
return fmt.Errorf("retrieving the applied connection settings, error: %w", err)
+1 -1
View File
@@ -84,7 +84,7 @@ func (r *resolvconf) supportCustomPort() bool {
return false
}
func (r *resolvconf) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (r *resolvconf) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
var err error
if !config.RouteAll {
err = r.restoreHostDNS()
+3 -3
View File
@@ -75,7 +75,7 @@ type DefaultServer struct {
iosDnsManager IosDnsManager
statusRecorder *peer.Status
stateManager *statemanager.Manager
stateManager statemanager.Manager
}
type handlerWithStop interface {
@@ -99,7 +99,7 @@ func NewDefaultServer(
wgInterface WGIface,
customAddress string,
statusRecorder *peer.Status,
stateManager *statemanager.Manager,
stateManager statemanager.Manager,
disableSys bool,
) (*DefaultServer, error) {
var addrPort *netip.AddrPort
@@ -161,7 +161,7 @@ func newDefaultServer(
wgInterface WGIface,
dnsService service,
statusRecorder *peer.Status,
stateManager *statemanager.Manager,
stateManager statemanager.Manager,
disableSys bool,
) *DefaultServer {
ctx, stop := context.WithCancel(ctx)
+1 -1
View File
@@ -647,7 +647,7 @@ func TestDNSServerUpstreamDeactivateCallback(t *testing.T) {
}
var domainsUpdate string
hostManager.applyDNSConfigFunc = func(config HostDNSConfig, statemanager *statemanager.Manager) error {
hostManager.applyDNSConfigFunc = func(config HostDNSConfig, statemanager statemanager.Manager) error {
domains := []string{}
for _, item := range config.Domains {
if item.Disabled {
+1 -1
View File
@@ -87,7 +87,7 @@ func (s *systemdDbusConfigurator) supportCustomPort() bool {
return true
}
func (s *systemdDbusConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error {
func (s *systemdDbusConfigurator) applyDNSConfig(config HostDNSConfig, stateManager statemanager.Manager) error {
parsedIP, err := netip.ParseAddr(config.ServerIP)
if err != nil {
return fmt.Errorf("unable to parse ip address, error: %w", err)
+1 -1
View File
@@ -35,7 +35,7 @@ func (s *ShutdownState) Cleanup() error {
}
// TODO: move file contents to state manager
func createUncleanShutdownIndicator(sourcePath string, dnsAddressStr string, stateManager *statemanager.Manager) error {
func createUncleanShutdownIndicator(sourcePath string, dnsAddressStr string, stateManager statemanager.Manager) error {
dnsAddress, err := netip.ParseAddr(dnsAddressStr)
if err != nil {
return fmt.Errorf("parse dns address %s: %w", dnsAddressStr, err)