Skip to content

Commit afdee48

Browse files
behzad-mirbehzad-mir
authored
fix: fixing the CIDR format of getNetworkName() on Windows (#1589)
* fix: fixing the CIDR format of getNetworkName() on Windows plus adding a unti test for the function * fix: fixing the CIDR format of getNetworkName() on Windows. Testcases for failure paths has been added.
1 parent 57304a4 commit afdee48

File tree

2 files changed

+207
-3
lines changed

2 files changed

+207
-3
lines changed

‎cni/network/network_windows.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"encoding/json"
55
"fmt"
66
"net"
7+
"net/netip"
78
"os"
89
"path/filepath"
910
"strconv"
@@ -154,9 +155,14 @@ func (plugin *NetPlugin) getNetworkName(netNs string, ipamAddResult *IPAMAddResu
154155
// This will happen during ADD call
155156
if ipamAddResult != nil && ipamAddResult.ncResponse != nil {
156157
// networkName will look like ~ azure-vlan1-172-28-1-0_24
157-
subnet := ipamAddResult.ipv4Result.IPs[0].Address
158-
networkName := strings.Replace(subnet.String(), ".", "-", -1)
159-
networkName = strings.Replace(networkName, "/", "_", -1)
158+
ipAddrNet := ipamAddResult.ipv4Result.IPs[0].Address
159+
prefix, err := netip.ParsePrefix(ipAddrNet.String())
160+
if err != nil {
161+
log.Printf("Error parsing %s network CIDR: %v.", ipAddrNet.String(), err)
162+
return "", errors.Wrapf(err, "cns returned invalid CIDR %s", ipAddrNet.String())
163+
}
164+
networkName := strings.ReplaceAll(prefix.Masked().String(), ".", "-")
165+
networkName = strings.ReplaceAll(networkName, "/", "_")
160166
networkName = fmt.Sprintf("%s-vlan%v-%v", nwCfg.Name, ipamAddResult.ncResponse.MultiTenancyInfo.ID, networkName)
161167
return networkName, nil
162168
}

‎cni/network/network_windows_test.go‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,3 +301,201 @@ func TestDSRPolciy(t *testing.T) {
301301
})
302302
}
303303
}
304+
305+
func TestGetNetworkNameFromCNS(t *testing.T) {
306+
plugin, _ := cni.NewPlugin("name", "0.3.0")
307+
tests := []struct {
308+
name string
309+
plugin *NetPlugin
310+
netNs string
311+
nwCfg *cni.NetworkConfig
312+
ipamAddResult *IPAMAddResult
313+
want string
314+
wantErr bool
315+
}{
316+
{
317+
name: "Get Network Name from CNS with correct CIDR",
318+
plugin: &NetPlugin{
319+
Plugin: plugin,
320+
nm: network.NewMockNetworkmanager(),
321+
ipamInvoker: NewMockIpamInvoker(false, false, false),
322+
report: &telemetry.CNIReport{},
323+
tb: &telemetry.TelemetryBuffer{},
324+
},
325+
netNs: "net",
326+
nwCfg: &cni.NetworkConfig{
327+
CNIVersion: "0.3.0",
328+
Name: "azure",
329+
MultiTenancy: true,
330+
},
331+
ipamAddResult: &IPAMAddResult{
332+
ncResponse: &cns.GetNetworkContainerResponse{
333+
MultiTenancyInfo: cns.MultiTenancyInfo{
334+
ID: 1,
335+
},
336+
},
337+
ipv4Result: &cniTypesCurr.Result{
338+
IPs: []*cniTypesCurr.IPConfig{
339+
{
340+
Address: net.IPNet{
341+
IP: net.ParseIP("10.240.0.5"),
342+
Mask: net.CIDRMask(24, 32),
343+
},
344+
},
345+
},
346+
},
347+
},
348+
want: "azure-vlan1-10-240-0-0_24",
349+
wantErr: false,
350+
},
351+
{
352+
name: "Get Network Name from CNS with malformed CIDR #1",
353+
plugin: &NetPlugin{
354+
Plugin: plugin,
355+
nm: network.NewMockNetworkmanager(),
356+
ipamInvoker: NewMockIpamInvoker(false, false, false),
357+
report: &telemetry.CNIReport{},
358+
tb: &telemetry.TelemetryBuffer{},
359+
},
360+
netNs: "net",
361+
nwCfg: &cni.NetworkConfig{
362+
CNIVersion: "0.3.0",
363+
Name: "azure",
364+
MultiTenancy: true,
365+
},
366+
ipamAddResult: &IPAMAddResult{
367+
ncResponse: &cns.GetNetworkContainerResponse{
368+
MultiTenancyInfo: cns.MultiTenancyInfo{
369+
ID: 1,
370+
},
371+
},
372+
ipv4Result: &cniTypesCurr.Result{
373+
IPs: []*cniTypesCurr.IPConfig{
374+
{
375+
Address: net.IPNet{
376+
IP: net.ParseIP(""),
377+
Mask: net.CIDRMask(24, 32),
378+
},
379+
},
380+
},
381+
},
382+
},
383+
want: "",
384+
wantErr: true,
385+
},
386+
{
387+
name: "Get Network Name from CNS with malformed CIDR #2",
388+
plugin: &NetPlugin{
389+
Plugin: plugin,
390+
nm: network.NewMockNetworkmanager(),
391+
ipamInvoker: NewMockIpamInvoker(false, false, false),
392+
report: &telemetry.CNIReport{},
393+
tb: &telemetry.TelemetryBuffer{},
394+
},
395+
netNs: "net",
396+
nwCfg: &cni.NetworkConfig{
397+
CNIVersion: "0.3.0",
398+
Name: "azure",
399+
MultiTenancy: true,
400+
},
401+
ipamAddResult: &IPAMAddResult{
402+
ncResponse: &cns.GetNetworkContainerResponse{
403+
MultiTenancyInfo: cns.MultiTenancyInfo{
404+
ID: 1,
405+
},
406+
},
407+
ipv4Result: &cniTypesCurr.Result{
408+
IPs: []*cniTypesCurr.IPConfig{
409+
{
410+
Address: net.IPNet{
411+
IP: net.ParseIP("10.0.00.6"),
412+
Mask: net.CIDRMask(24, 32),
413+
},
414+
},
415+
},
416+
},
417+
},
418+
want: "",
419+
wantErr: true,
420+
},
421+
{
422+
name: "Get Network Name from CNS without NetNS",
423+
plugin: &NetPlugin{
424+
Plugin: plugin,
425+
nm: network.NewMockNetworkmanager(),
426+
ipamInvoker: NewMockIpamInvoker(false, false, false),
427+
report: &telemetry.CNIReport{},
428+
tb: &telemetry.TelemetryBuffer{},
429+
},
430+
netNs: "",
431+
nwCfg: &cni.NetworkConfig{
432+
CNIVersion: "0.3.0",
433+
Name: "azure",
434+
MultiTenancy: true,
435+
},
436+
ipamAddResult: &IPAMAddResult{
437+
ncResponse: &cns.GetNetworkContainerResponse{
438+
MultiTenancyInfo: cns.MultiTenancyInfo{
439+
ID: 1,
440+
},
441+
},
442+
ipv4Result: &cniTypesCurr.Result{
443+
IPs: []*cniTypesCurr.IPConfig{
444+
{
445+
Address: net.IPNet{
446+
IP: net.ParseIP("10.0.0.6"),
447+
Mask: net.CIDRMask(24, 32),
448+
},
449+
},
450+
},
451+
},
452+
},
453+
want: "",
454+
wantErr: true,
455+
},
456+
{
457+
name: "Get Network Name from CNS without multitenancy",
458+
plugin: &NetPlugin{
459+
Plugin: plugin,
460+
nm: network.NewMockNetworkmanager(),
461+
ipamInvoker: NewMockIpamInvoker(false, false, false),
462+
report: &telemetry.CNIReport{},
463+
tb: &telemetry.TelemetryBuffer{},
464+
},
465+
netNs: "azure",
466+
nwCfg: &cni.NetworkConfig{
467+
CNIVersion: "0.3.0",
468+
Name: "azure",
469+
MultiTenancy: false,
470+
},
471+
ipamAddResult: &IPAMAddResult{
472+
ncResponse: &cns.GetNetworkContainerResponse{},
473+
ipv4Result: &cniTypesCurr.Result{
474+
IPs: []*cniTypesCurr.IPConfig{
475+
{
476+
Address: net.IPNet{
477+
IP: net.ParseIP("10.0.0.6"),
478+
Mask: net.CIDRMask(24, 32),
479+
},
480+
},
481+
},
482+
},
483+
},
484+
want: "azure",
485+
wantErr: false,
486+
},
487+
}
488+
489+
for _, tt := range tests {
490+
tt := tt
491+
t.Run(tt.name, func(t *testing.T) {
492+
networkName, err := tt.plugin.getNetworkName(tt.netNs, tt.ipamAddResult, tt.nwCfg)
493+
if tt.wantErr {
494+
require.Error(t, err)
495+
} else {
496+
require.NoError(t, err)
497+
require.Equal(t, tt.want, networkName)
498+
}
499+
})
500+
}
501+
}

0 commit comments

Comments
 (0)