refactored the networkutils and windows watcher package

Presently, we have setter methods present in the networkutils and watcher package instead of having mock methods for the same. This is inconsistent with rest of the agent package and goes against Go guidelines.

agent/eni/networkutils/utils_windows.go

@@ -25,22 +25,23 @@ import (
 	"time"
 	"unsafe"
 
-	"golang.org/x/sys/windows"
-
 	apierrors "github.com/aws/amazon-ecs-agent/agent/api/errors"
 	"github.com/aws/amazon-ecs-agent/agent/eni/netwrapper"
 	"github.com/aws/amazon-ecs-agent/agent/utils/retry"
+
 	"github.com/cihub/seelog"
 	"github.com/pkg/errors"
+	"golang.org/x/sys/windows"
 )
 
+//go:generate mockgen -destination=mocks/$GOFILE -copyright_file=../../../scripts/copyright_file github.com/aws/amazon-ecs-agent/agent/eni/networkutils NetworkUtils
+
 // NetworkUtils is the interface used for accessing network related functionality on Windows.
 // The methods declared in this package may or may not add any additional logic over the actual networking api calls.
 type NetworkUtils interface {
 	GetInterfaceMACByIndex(int, context.Context, time.Duration) (string, error)
 	GetAllNetworkInterfaces() ([]net.Interface, error)
 	GetDNSServerAddressList(macAddress string) ([]string, error)
-	SetNetWrapper(netWrapper netwrapper.NetWrapper)
 }
 
 type networkUtils struct {
@@ -118,11 +119,6 @@ func (utils *networkUtils) GetAllNetworkInterfaces() ([]net.Interface, error) {
 	return utils.netWrapper.GetAllNetworkInterfaces()
 }
 
-// SetNetWrapper is used to inject netWrapper instance. This will be handy while testing to inject mocks.
-func (utils *networkUtils) SetNetWrapper(netWrapper netwrapper.NetWrapper) {
-	utils.netWrapper = netWrapper
-}
-
 // GetDNSServerAddressList returns the DNS server addresses of the queried interface.
 func (utils *networkUtils) GetDNSServerAddressList(macAddress string) ([]string, error) {
 	addresses, err := funcGetAdapterAddresses()
@@ -141,7 +137,7 @@ func (utils *networkUtils) GetDNSServerAddressList(macAddress string) ([]string,
 
 	dnsServerAddressList := make([]string, 0)
 	for firstDnsNode != nil {
-		dnsServerAddressList = append(dnsServerAddressList, utils.parseSocketAddress(firstDnsNode.Address))
+		dnsServerAddressList = append(dnsServerAddressList, firstDnsNode.Address.IP().String())
 		firstDnsNode = firstDnsNode.Next
 	}
 
@@ -158,18 +154,6 @@ func (utils *networkUtils) parseMACAddress(adapterAddress *windows.IpAdapterAddr
 	return hardwareAddr
 }
 
-// parseSocketAddress parses the SocketAddress into its string representation.
-// This method needs to be deprecated in favour of IP() method of SocketAdress introduced in Go 1.13+.
-// The method details have been taken from https://github.com/golang/sys/blob/release-branch.go1.13/windows/types_windows.go
-func (utils *networkUtils) parseSocketAddress(addr windows.SocketAddress) string {
-	var ipAddr string
-	if uintptr(addr.SockaddrLength) >= unsafe.Sizeof(syscall.RawSockaddrInet4{}) && addr.Sockaddr.Addr.Family == syscall.AF_INET {
-		ip := net.IP((*syscall.RawSockaddrInet4)(unsafe.Pointer(addr.Sockaddr)).Addr[:])
-		ipAddr = ip.String()
-	}
-	return ipAddr
-}
-
 // getAdapterAddresses returns a list of IP adapter and address
 // structures. The structure contains an IP adapter and flattened
 // multiple IP addresses including unicast, anycast and multicast

agent/eni/networkutils/utils_windows_test.go

@@ -44,8 +44,7 @@ func TestGetInterfaceMACByIndex(t *testing.T) {
 
 	ctx := context.TODO()
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 	hardwareAddr, err := net.ParseMAC(macAddress)
 
 	mocknetwrapper.EXPECT().FindInterfaceByIndex(interfaceIndex).Return(
@@ -68,8 +67,7 @@ func TestGetInterfaceMACByIndexEmptyAddress(t *testing.T) {
 
 	ctx := context.TODO()
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 
 	mocknetwrapper.EXPECT().FindInterfaceByIndex(interfaceIndex).Return(
 		&net.Interface{
@@ -91,8 +89,7 @@ func TestGetInterfaceMACByIndexRetries(t *testing.T) {
 
 	ctx := context.TODO()
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 	hardwareAddr, err := net.ParseMAC(macAddress)
 	emptyaddr := make([]byte, 0)
 
@@ -123,8 +120,7 @@ func TestGetInterfaceMACByIndexContextTimeout(t *testing.T) {
 
 	ctx := context.TODO()
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 
 	mocknetwrapper.EXPECT().FindInterfaceByIndex(interfaceIndex).Return(
 		&net.Interface{
@@ -146,8 +142,7 @@ func TestGetInterfaceMACByIndexWithGolangNetError(t *testing.T) {
 
 	ctx := context.TODO()
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 
 	mocknetwrapper.EXPECT().FindInterfaceByIndex(interfaceIndex).Return(
 		nil, errors.New("unable to retrieve interface"))
@@ -164,8 +159,7 @@ func TestGetAllNetworkInterfaces(t *testing.T) {
 	defer mockCtrl.Finish()
 
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 
 	expectedIface := make([]net.Interface, 1)
 
@@ -191,8 +185,7 @@ func TestGetAllNetworkInterfacesError(t *testing.T) {
 	defer mockCtrl.Finish()
 
 	mocknetwrapper := mock_netwrapper.NewMockNetWrapper(mockCtrl)
-	netUtils := New()
-	netUtils.SetNetWrapper(mocknetwrapper)
+	netUtils := &networkUtils{netWrapper: mocknetwrapper}
 
 	mocknetwrapper.EXPECT().GetAllNetworkInterfaces().Return(
 		nil, errors.New("error occurred while fetching interfaces"),

agent/eni/watcher/watcher_windows.go

@@ -59,8 +59,6 @@ func newWatcher(ctx context.Context,
 	state dockerstate.TaskEngineState,
 	stateChangeEvents chan<- statechange.Event) (*ENIWatcher, error) {
 
-	derivedContext, cancel := context.WithCancel(ctx)
-
 	eniMonitor := iphelperwrapper.NewMonitor()
 	notificationChannel := make(chan int)
 	err := eniMonitor.Start(notificationChannel)
@@ -69,6 +67,7 @@ func newWatcher(ctx context.Context,
 	}
 	log.Info("windows eni watcher has been initialized")
 
+	derivedContext, cancel := context.WithCancel(ctx)
 	return &ENIWatcher{
 		ctx:              derivedContext,
 		cancel:           cancel,
@@ -172,9 +171,3 @@ func (eniWatcher *ENIWatcher) getAllInterfaces() (state map[string]int, err erro
 	}
 	return state, nil
 }
-
-// SetNetworkUtils is used for injecting NetworkUtils instance in eniWatcher
-// This will be handy while testing to inject mock objects
-func (eniWatcher *ENIWatcher) SetNetworkUtils(utils networkutils.NetworkUtils) {
-	eniWatcher.netutils = utils
-}