Merge pull request moby#50686 from thaJeztah/libnet_less_copyto

daemon/libnetwork: refactor, modernize various `CopyTo` functions, remove redundant utilities

Author: thaJeztah

daemon/libnetwork/drivers/bridge/bridge_linux.go

@@ -1399,19 +1399,14 @@ func (d *driver) EndpointOperInfo(nid, eid string) (map[string]any, error) {
 	m := make(map[string]any)
 
 	if ep.extConnConfig != nil && ep.extConnConfig.ExposedPorts != nil {
-		// Return a copy of the config data
-		epc := make([]types.TransportPort, 0, len(ep.extConnConfig.ExposedPorts))
-		for _, tp := range ep.extConnConfig.ExposedPorts {
-			epc = append(epc, tp.GetCopy())
-		}
-		m[netlabel.ExposedPorts] = epc
+		m[netlabel.ExposedPorts] = slices.Clone(ep.extConnConfig.ExposedPorts)
 	}
 
 	if ep.portMapping != nil {
 		// Return a copy of the operational data
 		pmc := make([]types.PortBinding, 0, len(ep.portMapping))
 		for _, pm := range ep.portMapping {
-			pmc = append(pmc, pm.PortBinding.GetCopy())
+			pmc = append(pmc, pm.PortBinding.Copy())
 		}
 		m[netlabel.PortMap] = pmc
 	}
@@ -1643,7 +1638,7 @@ func (ep *bridgeEndpoint) trimPortBindings(ctx context.Context, n *bridgeNetwork
 	undo := func() []portmapperapi.PortBinding {
 		pbReq := make([]portmapperapi.PortBindingReq, 0, len(toDrop))
 		for _, pb := range toDrop {
-			pbReq = append(pbReq, portmapperapi.PortBindingReq{PortBinding: pb.GetCopy()})
+			pbReq = append(pbReq, portmapperapi.PortBindingReq{PortBinding: pb.Copy()})
 		}
 		pbs, err := n.addPortMappings(ctx, ep, pbReq, n.config.DefaultBindingIP, ep.portBindingState)
 		if err != nil {
@@ -1895,7 +1890,7 @@ func parseConnectivityOptions(cOptions map[string]any) (*connectivityConfigurati
 		if pbs, ok := opt.([]types.PortBinding); ok {
 			cc.PortBindings = sliceutil.Map(pbs, func(pb types.PortBinding) portmapperapi.PortBindingReq {
 				return portmapperapi.PortBindingReq{
-					PortBinding: pb.GetCopy(),
+					PortBinding: pb.Copy(),
 				}
 			})
 		} else {

daemon/libnetwork/drivers/bridge/bridge_linux_test.go

@@ -158,15 +158,9 @@ func compareConnConfig(a, b *connectivityConfiguration) bool {
 	if a == nil || b == nil {
 		return false
 	}
-	if len(a.ExposedPorts) != len(b.ExposedPorts) ||
-		len(a.PortBindings) != len(b.PortBindings) {
+	if !slices.Equal(a.ExposedPorts, b.ExposedPorts) {
 		return false
 	}
-	for i := 0; i < len(a.ExposedPorts); i++ {
-		if !a.ExposedPorts[i].Equal(&b.ExposedPorts[i]) {
-			return false
-		}
-	}
 	for i := 0; i < len(a.PortBindings); i++ {
 		if !comparePortBinding(&a.PortBindings[i].PortBinding, &b.PortBindings[i].PortBinding) {
 			return false
@@ -718,7 +712,7 @@ func (i *testInterface) SetMacAddress(mac net.HardwareAddr) error {
 	if mac == nil {
 		return types.InvalidParameterErrorf("tried to set nil MAC address to endpoint interface")
 	}
-	i.mac = types.GetMacCopy(mac)
+	i.mac = slices.Clone(mac)
 	return nil
 }
 
@@ -1181,10 +1175,10 @@ func TestSetDefaultGw(t *testing.T) {
 	}
 
 	ipam4 := getIPv4Data(t)
-	gw4 := types.GetIPCopy(ipam4[0].Pool.IP).To4()
+	gw4 := slices.Clone(ipam4[0].Pool.IP).To4()
 	gw4[3] = 254
 	ipam6 := getIPv6Data(t)
-	gw6 := types.GetIPCopy(ipam6[0].Pool.IP)
+	gw6 := slices.Clone(ipam6[0].Pool.IP)
 	gw6[15] = 0x42
 
 	option := map[string]any{

daemon/libnetwork/drivers/bridge/internal/firewaller/stub.go

@@ -109,16 +109,14 @@ func (nw *StubFirewallerNetwork) AddPorts(_ context.Context, pbs []types.PortBin
 		if nw.PortExists(pb) {
 			return nil
 		}
-		nw.Ports = append(nw.Ports, pb.GetCopy())
+		nw.Ports = append(nw.Ports, pb.Copy())
 	}
 	return nil
 }
 
 func (nw *StubFirewallerNetwork) DelPorts(_ context.Context, pbs []types.PortBinding) error {
 	for _, pb := range pbs {
-		nw.Ports = slices.DeleteFunc(nw.Ports, func(p types.PortBinding) bool {
-			return p.Equal(&pb)
-		})
+		nw.Ports = slices.DeleteFunc(nw.Ports, pb.Equal)
 	}
 	return nil
 }
@@ -130,13 +128,7 @@ func (nw *StubFirewallerNetwork) AddLink(_ context.Context, parentIP, childIP ne
 	nw.Links = append(nw.Links, stubFirewallerLink{
 		parentIP: parentIP,
 		childIP:  childIP,
-		ports: func() []types.TransportPort {
-			res := make([]types.TransportPort, 0, len(ports))
-			for _, p := range ports {
-				res = append(res, p.GetCopy())
-			}
-			return res
-		}(),
+		ports:    slices.Clone(ports),
 	})
 	return nil
 }
@@ -148,9 +140,7 @@ func (nw *StubFirewallerNetwork) DelLink(_ context.Context, parentIP, childIP ne
 }
 
 func (nw *StubFirewallerNetwork) PortExists(pb types.PortBinding) bool {
-	return slices.ContainsFunc(nw.Ports, func(p types.PortBinding) bool {
-		return p.Equal(&pb)
-	})
+	return slices.ContainsFunc(nw.Ports, pb.Equal)
 }
 
 func (nw *StubFirewallerNetwork) LinkExists(parentIP, childIP netip.Addr, ports []types.TransportPort) bool {
@@ -164,7 +154,7 @@ func matchLink(l stubFirewallerLink, parentIP, childIP netip.Addr, ports []types
 		return false
 	}
 	for i, p := range l.ports {
-		if !p.Equal(&ports[i]) {
+		if p != ports[i] {
 			return false
 		}
 	}

daemon/libnetwork/drivers/bridge/port_mapping_linux_test.go

@@ -1009,8 +1009,10 @@ func (pm *stubPortMapper) MapPorts(_ context.Context, reqs []portmapperapi.PortB
 
 func (pm *stubPortMapper) UnmapPorts(_ context.Context, reqs []portmapperapi.PortBinding, _ portmapperapi.Firewaller) error {
 	for _, req := range reqs {
+		// We're only checking for the PortBinding here, not any other
+		// property of [portmapperapi.PortBinding].
 		idx := slices.IndexFunc(pm.mapped, func(pb portmapperapi.PortBinding) bool {
-			return pb.Equal(&req.PortBinding)
+			return pb.Equal(req.PortBinding)
 		})
 		if idx == -1 {
 			return fmt.Errorf("stubPortMapper.UnmapPorts: pb doesn't exist %v", req)

daemon/libnetwork/drivers/windows/overlay/ov_endpoint_windows.go

@@ -262,7 +262,7 @@ func (d *driver) EndpointOperInfo(nid, eid string) (map[string]any, error) {
 		// Return a copy of the operational data
 		pmc := make([]types.PortBinding, 0, len(ep.portMapping))
 		for _, pm := range ep.portMapping {
-			pmc = append(pmc, pm.GetCopy())
+			pmc = append(pmc, pm.Copy())
 		}
 		data[netlabel.PortMap] = pmc
 	}

daemon/libnetwork/drivers/windows/port_mapping.go

@@ -21,7 +21,7 @@ const maxAllocatePortAttempts = 10
 func AllocatePorts(portMapper *portmapper.PortMapper, bindings []types.PortBinding, containerIP net.IP) ([]types.PortBinding, error) {
 	bs := make([]types.PortBinding, 0, len(bindings))
 	for _, c := range bindings {
-		b := c.GetCopy()
+		b := c.Copy()
 		if err := allocatePort(portMapper, &b, containerIP); err != nil {
 			// On allocation failure, release previously allocated ports. On cleanup error, just log a warning message
 			if cuErr := ReleasePorts(portMapper, bs); cuErr != nil {

daemon/libnetwork/drivers/windows/windows.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -863,19 +864,14 @@ func (d *driver) EndpointOperInfo(nid, eid string) (map[string]any, error) {
 
 	data["hnsid"] = ep.profileID
 	if ep.epConnectivity.ExposedPorts != nil {
-		// Return a copy of the config data
-		epc := make([]types.TransportPort, 0, len(ep.epConnectivity.ExposedPorts))
-		for _, tp := range ep.epConnectivity.ExposedPorts {
-			epc = append(epc, tp.GetCopy())
-		}
-		data[netlabel.ExposedPorts] = epc
+		data[netlabel.ExposedPorts] = slices.Clone(ep.epConnectivity.ExposedPorts)
 	}
 
 	if ep.portMapping != nil {
 		// Return a copy of the operational data
 		pmc := make([]types.PortBinding, 0, len(ep.portMapping))
 		for _, pm := range ep.portMapping {
-			pmc = append(pmc, pm.GetCopy())
+			pmc = append(pmc, pm.Copy())
 		}
 		data[netlabel.PortMap] = pmc
 	}

daemon/libnetwork/endpoint.go

@@ -5,8 +5,10 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"maps"
 	"net"
 	"net/netip"
+	"slices"
 	"strings"
 	"sync"
 
@@ -18,7 +20,6 @@ import (
 	"github.com/moby/moby/v2/daemon/libnetwork/driverapi"
 	"github.com/moby/moby/v2/daemon/libnetwork/ipamapi"
 	"github.com/moby/moby/v2/daemon/libnetwork/netlabel"
-	"github.com/moby/moby/v2/daemon/libnetwork/options"
 	"github.com/moby/moby/v2/daemon/libnetwork/scope"
 	"github.com/moby/moby/v2/daemon/libnetwork/types"
 	"github.com/moby/moby/v2/errdefs"
@@ -272,10 +273,13 @@ func (ep *Endpoint) New() datastore.KVObject {
 	return &Endpoint{network: ep.getNetwork()}
 }
 
+var _ datastore.KVObject = (*Endpoint)(nil)
+
 func (ep *Endpoint) CopyTo(o datastore.KVObject) error {
 	ep.mu.Lock()
 	defer ep.mu.Unlock()
 
+	// TODO(thaJeztah): should dstEp be locked during this copy? (ideally we'd not have a mutex at all in this struct).
 	dstEp := o.(*Endpoint)
 	dstEp.name = ep.name
 	dstEp.id = ep.id
@@ -286,40 +290,15 @@ func (ep *Endpoint) CopyTo(o datastore.KVObject) error {
 	dstEp.disableIPv6 = ep.disableIPv6
 	dstEp.svcName = ep.svcName
 	dstEp.svcID = ep.svcID
-	dstEp.virtualIP = ep.virtualIP
+	dstEp.virtualIP = ep.virtualIP // TODO(thaJeztah): should this be cloned? (net.IP)?
 	dstEp.loadBalancer = ep.loadBalancer
-
-	dstEp.svcAliases = make([]string, len(ep.svcAliases))
-	copy(dstEp.svcAliases, ep.svcAliases)
-
-	dstEp.ingressPorts = make([]*PortConfig, len(ep.ingressPorts))
-	copy(dstEp.ingressPorts, ep.ingressPorts)
-
-	if ep.iface != nil {
-		dstEp.iface = &EndpointInterface{}
-		if err := ep.iface.CopyTo(dstEp.iface); err != nil {
-			return err
-		}
-	}
-
-	if ep.joinInfo != nil {
-		dstEp.joinInfo = &endpointJoinInfo{}
-		if err := ep.joinInfo.CopyTo(dstEp.joinInfo); err != nil {
-			return err
-		}
-	}
-
-	dstEp.exposedPorts = make([]types.TransportPort, len(ep.exposedPorts))
-	copy(dstEp.exposedPorts, ep.exposedPorts)
-
-	dstEp.dnsNames = make([]string, len(ep.dnsNames))
-	copy(dstEp.dnsNames, ep.dnsNames)
-
-	dstEp.generic = options.Generic{}
-	for k, v := range ep.generic {
-		dstEp.generic[k] = v
-	}
-
+	dstEp.svcAliases = slices.Clone(ep.svcAliases)
+	dstEp.ingressPorts = slices.Clone(ep.ingressPorts) // TODO(thaJeztah): should this be copied in depth? ([]*PortConfig)
+	dstEp.iface = ep.iface.Copy()
+	dstEp.joinInfo = ep.joinInfo.Copy()
+	dstEp.exposedPorts = slices.Clone(ep.exposedPorts)
+	dstEp.dnsNames = slices.Clone(ep.dnsNames)
+	dstEp.generic = maps.Clone(ep.generic)
 	return nil
 }