Add missing test cases for Override

The add with override did not have any test coverage.

Signed-off-by: Nino Kodabande <nkodabande@suse.com>

cmd/rancher-desktop-guestagent/main.go

@@ -57,19 +57,14 @@ var (
 		"address to bind Kubernetes services to on the host, valid options are 0.0.0.0 or 127.0.0.1")
 )
 
-//nolint:lll
-// Flags can only be enable in the following combination:
-// +======================+=======================+===========================+=========================+=============================+
-// |                      | Default Network Admin | Default Network Non-Admin | Namespace Network Admin | Namespace Network Non-Admin |
-// +======================+=======================+===========================+=========================+=============================+
-// | privilegedService    | enable                | disable                   | disable                 | disable                     |
-// +----------------------+-----------------------+---------------------------+-------------------------+-----------------------------+
-// | docker Or containerd | enable                | disable                   | enable                  | enable                      |
-// +----------------------+-----------------------+---------------------------+-------------------------+-----------------------------+
-// | iptables             | disable               | enable                    | disable                 | disable                     |
-// +----------------------+-----------------------+---------------------------+-------------------------+-----------------------------+
-// | Kubernetes           | enable                | enable                    | enable                  | enable                      |
-// +----------------------+-----------------------+---------------------------+-------------------------+-----------------------------+
+// Flags can only be enabled in the following combination:
+// +===========+=======================================+====================+
+// |           |            Default Network            | Namespaced Network |
+// +===========+=======================================+====================+
+// | Admin     | privilegedService + docker/containerd | docker/containerd  |
+// +-----------+---------------------------------------+--------------------+
+// | Non-Admin | iptables                              | docker/containerd  |
+// +-----------+---------------------------------------+--------------------+
 
 const (
 	wslInfName             = "eth0"
@@ -111,10 +106,6 @@ func main() {
 		cancel()
 	}()
 
-	// Only one of the containerd, docker or iptables should be enabled
-	// at any give run. The containerd or docker is enabled depending
-	// on the chosen backend engine when privileged service is enabled
-	// and port mappings are sent over the VTunnel. However, when
 	if !*enableContainerd &&
 		!*enableDocker &&
 		!*enableIptables {

pkg/tracker/apitracker_test.go

@@ -71,6 +71,82 @@ func TestBasicAdd(t *testing.T) {
 	assert.Equal(t, portMapping, actualPortMapping)
 }
 
+func TestAddOverride(t *testing.T) {
+	t.Parallel()
+
+	var expectedExposeReq []*types.ExposeRequest
+
+	mux := http.NewServeMux()
+
+	mux.HandleFunc("/services/forwarder/expose", func(w http.ResponseWriter, r *http.Request) {
+		var tmpReq *types.ExposeRequest
+		err := json.NewDecoder(r.Body).Decode(&tmpReq)
+		assert.NoError(t, err)
+		expectedExposeReq = append(expectedExposeReq, tmpReq)
+	})
+
+	testSrv := httptest.NewServer(mux)
+	defer testSrv.Close()
+
+	apiTracker := tracker.NewAPITracker(testSrv.URL)
+	portMapping := nat.PortMap{
+		"80/tcp": []nat.PortBinding{
+			{
+				HostIP:   hostIP,
+				HostPort: hostPort,
+			},
+		},
+		"443/tcp": []nat.PortBinding{
+			{
+				HostIP:   hostIP2,
+				HostPort: hostPort2,
+			},
+		},
+	}
+	err := apiTracker.Add(containerID, portMapping)
+	assert.NoError(t, err)
+
+	firstEntryIndex := 0
+	assert.Equal(t, expectedExposeReq[firstEntryIndex].Local, ipPortBuilder(hostIP, hostPort))
+	assert.Equal(t, expectedExposeReq[firstEntryIndex].Remote, ipPortBuilder(hostSwitchIP, hostPort))
+
+	secondEntryIndex := 1
+	assert.Equal(t, expectedExposeReq[secondEntryIndex].Local, ipPortBuilder(hostIP2, hostPort2))
+	assert.Equal(t, expectedExposeReq[secondEntryIndex].Remote, ipPortBuilder(hostSwitchIP, hostPort2))
+
+	actualPortMapping := apiTracker.Get(containerID)
+	assert.Equal(t, portMapping, actualPortMapping)
+
+	// reset the exposeReq slice
+	expectedExposeReq = nil
+
+	portMapping2 := nat.PortMap{
+		"80/tcp": []nat.PortBinding{
+			{
+				HostIP:   hostIP,
+				HostPort: hostPort,
+			},
+		},
+		"8080/tcp": []nat.PortBinding{
+			{
+				HostIP:   hostIP2,
+				HostPort: "8080",
+			},
+		},
+	}
+	err = apiTracker.Add(containerID, portMapping2)
+	assert.NoError(t, err)
+
+	assert.Equal(t, expectedExposeReq[firstEntryIndex].Local, ipPortBuilder(hostIP, hostPort))
+	assert.Equal(t, expectedExposeReq[firstEntryIndex].Remote, ipPortBuilder(hostSwitchIP, hostPort))
+
+	assert.Equal(t, expectedExposeReq[secondEntryIndex].Local, ipPortBuilder(hostIP2, "8080"))
+	assert.Equal(t, expectedExposeReq[secondEntryIndex].Remote, ipPortBuilder(hostSwitchIP, "8080"))
+
+	actualPortMapping = apiTracker.Get(containerID)
+	assert.Equal(t, portMapping2, actualPortMapping)
+}
+
 func TestAddWithError(t *testing.T) {
 	t.Parallel()
 

pkg/tracker/portstorage.go

@@ -70,12 +70,22 @@ func (p *portStorage) getAll() map[string]nat.PortMap {
 	portMappings := make(map[string]nat.PortMap, len(p.portmap))
 
 	for k, v := range p.portmap {
-		portMappings[k] = v
+		portMappings[k] = copyPortMap(v)
 	}
 
 	return portMappings
 }
 
+func copyPortMap(m nat.PortMap) nat.PortMap {
+	portMap := make(nat.PortMap, len(m))
+
+	for k, v := range m {
+		portMap[k] = v
+	}
+
+	return portMap
+}
+
 func (p *portStorage) remove(containerID string) {
 	p.mutex.Lock()
 	defer p.mutex.Unlock()

pkg/tracker/vtunneltracker.go

@@ -16,11 +16,16 @@ limitations under the License.
 package tracker
 
 import (
+	"errors"
+	"fmt"
+
 	"github.com/docker/go-connections/nat"
 	"github.com/rancher-sandbox/rancher-desktop-agent/pkg/forwarder"
 	"github.com/rancher-sandbox/rancher-desktop-agent/pkg/types"
 )
 
+var ErrRemoveAll = errors.New("failed to remove all portMappings")
+
 // VTunnelTracker keeps track of port mappings and forwards
 // them to the privileged service on the host over AF_VSOCK
 // tunnel (vtunnel).
@@ -41,8 +46,8 @@ func NewVTunnelTracker(vtunnelForwarder forwarder.Forwarder, wslAddrs []types.Co
 	}
 }
 
-// Add adds a container ID and port mapping to the tracker and calls the
-// vtunnle forwarder to send the port mappings to privileged service.
+// Add a container ID and port mapping to the tracker and calls the
+// vtunnel forwarder to send the port mappings to privileged service.
 func (p *VTunnelTracker) Add(containerID string, portMap nat.PortMap) error {
 	if len(portMap) == 0 {
 		return nil
@@ -68,7 +73,7 @@ func (p *VTunnelTracker) Get(containerID string) nat.PortMap {
 }
 
 // Remove deletes a container ID and port mapping from the tracker and calls the
-// vtunnle forwarder to send the port mappings to privileged service.
+// vtunnel forwarder to send the port mappings to privileged service.
 func (p *VTunnelTracker) Remove(containerID string) error {
 	portMap := p.portStorage.get(containerID)
 	if len(portMap) != 0 {
@@ -89,7 +94,26 @@ func (p *VTunnelTracker) Remove(containerID string) error {
 
 // RemoveAll removes all the port bindings from the tracker.
 func (p *VTunnelTracker) RemoveAll() error {
-	p.portStorage.removeAll()
+	defer p.portStorage.removeAll()
+
+	allPortMappings := p.portStorage.getAll()
+
+	var errs []error
+
+	for _, portMap := range allPortMappings {
+		err := p.vtunnelForwarder.Send(types.PortMapping{
+			Remove:       true,
+			Ports:        portMap,
+			ConnectAddrs: p.wslAddrs,
+		})
+		if err != nil {
+			errs = append(errs, err)
+		}
+	}
+
+	if len(errs) != 0 {
+		return fmt.Errorf("%w: %+v", ErrRemoveAll, errs)
+	}
 
 	return nil
 }