Merge pull request #44868 from neersighted/libnetwork_netutils

libnetwork: minor cleanup related to GenerateRandomName

libnetwork/drivers/bridge/bridge.go

@@ -30,7 +30,7 @@ import (
 const (
 	networkType                = "bridge"
 	vethPrefix                 = "veth"
-	vethLen                    = 7
+	vethLen                    = len(vethPrefix) + 7
 	defaultContainerVethPrefix = "eth"
 	maxAllocatePortAttempts    = 10
 )

libnetwork/drivers/ipvlan/ipvlan.go

@@ -14,9 +14,9 @@ import (
 )
 
 const (
-	vethLen             = 7
 	containerVethPrefix = "eth"
 	vethPrefix          = "veth"
+	vethLen             = len(vethPrefix) + 7
 
 	driverName    = "ipvlan"      // driver type name
 	parentOpt     = "parent"      // parent interface -o parent

libnetwork/drivers/macvlan/macvlan.go

@@ -14,9 +14,9 @@ import (
 )
 
 const (
-	vethLen             = 7
 	containerVethPrefix = "eth"
 	vethPrefix          = "veth"
+	vethLen             = len(vethPrefix) + 7
 	driverName          = "macvlan"      // driver type name
 	modePrivate         = "private"      // macvlan mode private
 	modeVepa            = "vepa"         // macvlan mode vepa

libnetwork/drivers/overlay/overlay.go

@@ -25,7 +25,7 @@ import (
 const (
 	networkType  = "overlay"
 	vethPrefix   = "veth"
-	vethLen      = 7
+	vethLen      = len(vethPrefix) + 7
 	vxlanIDStart = 256
 	vxlanIDEnd   = (1 << 24) - 1
 	vxlanEncap   = 50

libnetwork/drivers/windows/overlay/overlay_windows.go

@@ -17,10 +17,7 @@ import (
 )
 
 const (
-	networkType  = "overlay"
-	vethPrefix   = "veth"
-	vethLen      = 7
-	secureOption = "encrypted"
+	networkType = "overlay"
 )
 
 type driver struct {

libnetwork/netutils/utils.go

@@ -121,14 +121,21 @@ func GenerateMACFromIP(ip net.IP) net.HardwareAddr {
 	return genMAC(ip)
 }
 
-// GenerateRandomName returns a new name joined with a prefix.  This size
-// specified is used to truncate the randomly generated value
-func GenerateRandomName(prefix string, size int) (string, error) {
-	id := make([]byte, 32)
-	if _, err := io.ReadFull(rand.Reader, id); err != nil {
+// GenerateRandomName returns a string of the specified length, created by joining the prefix to random hex characters.
+// The length must be strictly larger than len(prefix), or an error will be returned.
+func GenerateRandomName(prefix string, length int) (string, error) {
+	if length <= len(prefix) {
+		return "", fmt.Errorf("invalid length %d for prefix %s", length, prefix)
+	}
+
+	// We add 1 here as integer division will round down, and we want to round up.
+	b := make([]byte, (length-len(prefix)+1)/2)
+	if _, err := io.ReadFull(rand.Reader, b); err != nil {
 		return "", err
 	}
-	return prefix + hex.EncodeToString(id)[:size], nil
+
+	// By taking a slice here, we ensure that the string is always the correct length.
+	return (prefix + hex.EncodeToString(b))[:length], nil
 }
 
 // ReverseIP accepts a V4 or V6 IP string in the canonical form and returns a reversed IP in

libnetwork/netutils/utils_linux.go

@@ -8,7 +8,6 @@ package netutils
 import (
 	"net"
 	"os"
-	"strings"
 
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/libnetwork/ns"
@@ -49,11 +48,11 @@ func GenerateIfaceName(nlh *netlink.Handle, prefix string, len int) (string, err
 	for i := 0; i < 3; i++ {
 		name, err := GenerateRandomName(prefix, len)
 		if err != nil {
-			continue
+			return "", err
 		}
 		_, err = linkByName(name)
 		if err != nil {
-			if strings.Contains(err.Error(), "not found") {
+			if errors.As(err, &netlink.LinkNotFoundError{}) {
 				return name, nil
 			}
 			return "", err

libnetwork/netutils/utils_linux_test.go

@@ -2,14 +2,18 @@ package netutils
 
 import (
 	"bytes"
+	"fmt"
 	"net"
 	"sort"
+	"strings"
 	"testing"
 
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/libnetwork/testutils"
 	"github.com/docker/docker/libnetwork/types"
 	"github.com/vishvananda/netlink"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
 )
 
 func TestNonOverlappingNameservers(t *testing.T) {
@@ -186,21 +190,46 @@ func TestNetworkRange(t *testing.T) {
 
 // Test veth name generation "veth"+rand (e.g.veth0f60e2c)
 func TestGenerateRandomName(t *testing.T) {
-	name1, err := GenerateRandomName("veth", 7)
-	if err != nil {
-		t.Fatal(err)
+	const vethPrefix = "veth"
+	const vethLen = len(vethPrefix) + 7
+
+	testCases := []struct {
+		prefix string
+		length int
+		error  bool
+	}{
+		{vethPrefix, -1, true},
+		{vethPrefix, 0, true},
+		{vethPrefix, len(vethPrefix) - 1, true},
+		{vethPrefix, len(vethPrefix), true},
+		{vethPrefix, len(vethPrefix) + 1, false},
+		{vethPrefix, 255, false},
 	}
-	// veth plus generated append equals a len of 11
-	if len(name1) != 11 {
-		t.Fatalf("Expected 11 characters, instead received %d characters", len(name1))
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("prefix=%s/length=%d", tc.prefix, tc.length), func(t *testing.T) {
+			name, err := GenerateRandomName(tc.prefix, tc.length)
+			if tc.error {
+				assert.Check(t, is.ErrorContains(err, "invalid length"))
+			} else {
+				assert.NilError(t, err)
+				assert.Check(t, strings.HasPrefix(name, tc.prefix), "Expected name to start with %s", tc.prefix)
+				assert.Check(t, is.Equal(len(name), tc.length), "Expected %d characters, instead received %d characters", tc.length, len(name))
+			}
+		})
 	}
-	name2, err := GenerateRandomName("veth", 7)
-	if err != nil {
-		t.Fatal(err)
-	}
-	// Fail if the random generated names equal one another
-	if name1 == name2 {
-		t.Fatalf("Expected differing values but received %s and %s", name1, name2)
+
+	var randomNames [16]string
+	for i := range randomNames {
+		randomName, err := GenerateRandomName(vethPrefix, vethLen)
+		assert.NilError(t, err)
+
+		for _, oldName := range randomNames {
+			if randomName == oldName {
+				t.Fatalf("Duplicate random name generated: %s", randomName)
+			}
+		}
+
+		randomNames[i] = randomName
 	}
 }