Browse code

libnetwork/netutils: refactor GenerateRandomName

GenerateRandomName now uses length to represent the overall length of
the string; this will help future users avoid creating interface names
that are too long for the kernel to accept by mistake. The test coverage
is increased and cleaned up using gotest.tools.

Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>

Bjorn Neergaard authored on 2023/01/22 08:04:20
Showing 6 changed files
... ...
@@ -30,7 +30,7 @@ import (
30 30
 const (
31 31
 	networkType                = "bridge"
32 32
 	vethPrefix                 = "veth"
33
-	vethLen                    = 7
33
+	vethLen                    = len(vethPrefix) + 7
34 34
 	defaultContainerVethPrefix = "eth"
35 35
 	maxAllocatePortAttempts    = 10
36 36
 )
... ...
@@ -14,9 +14,9 @@ import (
14 14
 )
15 15
 
16 16
 const (
17
-	vethLen             = 7
18 17
 	containerVethPrefix = "eth"
19 18
 	vethPrefix          = "veth"
19
+	vethLen             = len(vethPrefix) + 7
20 20
 
21 21
 	driverName    = "ipvlan"      // driver type name
22 22
 	parentOpt     = "parent"      // parent interface -o parent
... ...
@@ -14,9 +14,9 @@ import (
14 14
 )
15 15
 
16 16
 const (
17
-	vethLen             = 7
18 17
 	containerVethPrefix = "eth"
19 18
 	vethPrefix          = "veth"
19
+	vethLen             = len(vethPrefix) + 7
20 20
 	driverName          = "macvlan"      // driver type name
21 21
 	modePrivate         = "private"      // macvlan mode private
22 22
 	modeVepa            = "vepa"         // macvlan mode vepa
... ...
@@ -25,7 +25,7 @@ import (
25 25
 const (
26 26
 	networkType  = "overlay"
27 27
 	vethPrefix   = "veth"
28
-	vethLen      = 7
28
+	vethLen      = len(vethPrefix) + 7
29 29
 	vxlanIDStart = 256
30 30
 	vxlanIDEnd   = (1 << 24) - 1
31 31
 	vxlanEncap   = 50
... ...
@@ -121,14 +121,21 @@ func GenerateMACFromIP(ip net.IP) net.HardwareAddr {
121 121
 	return genMAC(ip)
122 122
 }
123 123
 
124
-// GenerateRandomName returns a new name joined with a prefix.  This size
125
-// specified is used to truncate the randomly generated value
126
-func GenerateRandomName(prefix string, size int) (string, error) {
127
-	id := make([]byte, 32)
128
-	if _, err := io.ReadFull(rand.Reader, id); err != nil {
124
+// GenerateRandomName returns a string of the specified length, created by joining the prefix to random hex characters.
125
+// The length must be strictly larger than len(prefix), or an error will be returned.
126
+func GenerateRandomName(prefix string, length int) (string, error) {
127
+	if length <= len(prefix) {
128
+		return "", fmt.Errorf("invalid length %d for prefix %s", length, prefix)
129
+	}
130
+
131
+	// We add 1 here as integer division will round down, and we want to round up.
132
+	b := make([]byte, (length-len(prefix)+1)/2)
133
+	if _, err := io.ReadFull(rand.Reader, b); err != nil {
129 134
 		return "", err
130 135
 	}
131
-	return prefix + hex.EncodeToString(id)[:size], nil
136
+
137
+	// By taking a slice here, we ensure that the string is always the correct length.
138
+	return (prefix + hex.EncodeToString(b))[:length], nil
132 139
 }
133 140
 
134 141
 // ReverseIP accepts a V4 or V6 IP string in the canonical form and returns a reversed IP in
... ...
@@ -2,14 +2,18 @@ package netutils
2 2
 
3 3
 import (
4 4
 	"bytes"
5
+	"fmt"
5 6
 	"net"
6 7
 	"sort"
8
+	"strings"
7 9
 	"testing"
8 10
 
9 11
 	"github.com/docker/docker/libnetwork/ipamutils"
10 12
 	"github.com/docker/docker/libnetwork/testutils"
11 13
 	"github.com/docker/docker/libnetwork/types"
12 14
 	"github.com/vishvananda/netlink"
15
+	"gotest.tools/v3/assert"
16
+	is "gotest.tools/v3/assert/cmp"
13 17
 )
14 18
 
15 19
 func TestNonOverlappingNameservers(t *testing.T) {
... ...
@@ -186,21 +190,46 @@ func TestNetworkRange(t *testing.T) {
186 186
 
187 187
 // Test veth name generation "veth"+rand (e.g.veth0f60e2c)
188 188
 func TestGenerateRandomName(t *testing.T) {
189
-	name1, err := GenerateRandomName("veth", 7)
190
-	if err != nil {
191
-		t.Fatal(err)
192
-	}
193
-	// veth plus generated append equals a len of 11
194
-	if len(name1) != 11 {
195
-		t.Fatalf("Expected 11 characters, instead received %d characters", len(name1))
196
-	}
197
-	name2, err := GenerateRandomName("veth", 7)
198
-	if err != nil {
199
-		t.Fatal(err)
200
-	}
201
-	// Fail if the random generated names equal one another
202
-	if name1 == name2 {
203
-		t.Fatalf("Expected differing values but received %s and %s", name1, name2)
189
+	const vethPrefix = "veth"
190
+	const vethLen = len(vethPrefix) + 7
191
+
192
+	testCases := []struct {
193
+		prefix string
194
+		length int
195
+		error  bool
196
+	}{
197
+		{vethPrefix, -1, true},
198
+		{vethPrefix, 0, true},
199
+		{vethPrefix, len(vethPrefix) - 1, true},
200
+		{vethPrefix, len(vethPrefix), true},
201
+		{vethPrefix, len(vethPrefix) + 1, false},
202
+		{vethPrefix, 255, false},
203
+	}
204
+	for _, tc := range testCases {
205
+		t.Run(fmt.Sprintf("prefix=%s/length=%d", tc.prefix, tc.length), func(t *testing.T) {
206
+			name, err := GenerateRandomName(tc.prefix, tc.length)
207
+			if tc.error {
208
+				assert.Check(t, is.ErrorContains(err, "invalid length"))
209
+			} else {
210
+				assert.NilError(t, err)
211
+				assert.Check(t, strings.HasPrefix(name, tc.prefix), "Expected name to start with %s", tc.prefix)
212
+				assert.Check(t, is.Equal(len(name), tc.length), "Expected %d characters, instead received %d characters", tc.length, len(name))
213
+			}
214
+		})
215
+	}
216
+
217
+	var randomNames [16]string
218
+	for i := range randomNames {
219
+		randomName, err := GenerateRandomName(vethPrefix, vethLen)
220
+		assert.NilError(t, err)
221
+
222
+		for _, oldName := range randomNames {
223
+			if randomName == oldName {
224
+				t.Fatalf("Duplicate random name generated: %s", randomName)
225
+			}
226
+		}
227
+
228
+		randomNames[i] = randomName
204 229
 	}
205 230
 }
206 231