Skip to content

Commit 88eb82f

Browse files
behzad-mirCopilot
andauthored
fix: restrict host routes to InfraNIC in stateless CNI SwiftV2 (#4404)
* fix: restrict host routes and iptables to InfraNIC in stateless CNI SwiftV2 Guard handleCommonOptions to skip RoutesKey and IPTablesKey for non-InfraNIC types (DelegatedVMNIC, NodeNetworkInterfaceFrontendNIC). Host gateway routes are not reachable from secondary NICs on different subnets. Empty NICType is treated as InfraNIC for legacy compatibility. Add unit tests verifying routes and iptables are skipped for delegated NICs and applied for InfraNIC. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: remove unrelated sort test from ENETUNREACH fix The EndpointCreate sort test belongs with the separate sort PR (fix/sort-epinfos-infra-first), not this PR which focuses solely on preventing host routes from being applied to delegated NICs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor: move host options guard from handleCommonOptions to createEpInfo Move the NICType check from handleCommonOptions (network_linux.go) to createEpInfo (cni/network/network.go) so that non-infra NICs never receive RoutesKey/IPTablesKey/SNATIPKey options in the first place. This is cleaner than guarding downstream — the bad data is prevented at the source rather than being copied and then ignored. Only InfraNIC or legacy (empty NICType) endpoints get the options from shallowCopyIpamAddConfigOptions(). handleCommonOptions is reverted to its original unconditional behavior since it will never be called with infra-only options for non-infra NICs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f46287c commit 88eb82f

4 files changed

Lines changed: 117 additions & 5 deletions

File tree

cni/network/network.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,15 @@ func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointIn
736736
endpointID = plugin.nm.GetEndpointIDByNicType(opt.args.ContainerID, ifName, opt.ifInfo.NICType)
737737
}
738738

739+
// Only copy options (RoutesKey, IPTablesKey, SNATIPKey) for InfraNIC or legacy (empty NICType).
740+
// These options are infra-only host-namespace constructs set by setHostOptions and are not
741+
// applicable to delegated/secondary NICs on either Linux or Windows.
742+
// For non-infra NICs, options remains nil so handleCommonOptions becomes a no-op.
743+
var options map[string]interface{}
744+
if opt.ifInfo.NICType == cns.InfraNIC || opt.ifInfo.NICType == "" {
745+
options = opt.ipamAddConfig.shallowCopyIpamAddConfigOptions()
746+
}
747+
739748
endpointInfo := network.EndpointInfo{
740749
NetworkID: opt.networkID,
741750
Mode: opt.ipamAddConfig.nwCfg.Mode,
@@ -744,7 +753,7 @@ func (plugin *NetPlugin) createEpInfo(opt *createEpInfoOpt) (*network.EndpointIn
744753
BridgeName: opt.ipamAddConfig.nwCfg.Bridge,
745754
NetworkPolicies: networkPolicies, // nw and ep policies separated to avoid possible conflicts
746755
NetNs: opt.ipamAddConfig.args.Netns,
747-
Options: opt.ipamAddConfig.shallowCopyIpamAddConfigOptions(),
756+
Options: options,
748757
DisableHairpinOnHostInterface: opt.ipamAddConfig.nwCfg.DisableHairpinOnHostInterface,
749758
IsIPv6Enabled: opt.ipv6Enabled, // present infra only
750759

cni/network/network_linux_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -472,9 +472,7 @@ func TestPluginLinuxAdd(t *testing.T) {
472472
},
473473
Suffix: "myDomain",
474474
},
475-
Options: map[string]interface{}{
476-
"testflag": "copy",
477-
},
475+
Options: nil,
478476
// matches with cns ip configuration
479477
IPAddresses: []net.IPNet{
480478
{

network/endpoint.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ type EndpointInfo struct {
107107
Subnets []SubnetInfo
108108
BridgeName string
109109
NetNs string // used in windows
110-
Options map[string]interface{}
110+
Options map[string]interface{} // populated only for InfraNIC; nil for non-infra NICs
111111
DisableHairpinOnHostInterface bool
112112
IsIPv6Enabled bool
113113
HostSubnetPrefix string // can be used later to add an external interface

network/network_linux_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
package network
2+
3+
import (
4+
"net"
5+
"testing"
6+
7+
"github.com/Azure/azure-container-networking/iptables"
8+
"github.com/Azure/azure-container-networking/netio"
9+
"github.com/Azure/azure-container-networking/netlink"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// mockIPTablesClientWithRunCmd extends mock to track RunCmd calls.
15+
type mockIPTablesClientWithRunCmd struct {
16+
mockIPTablesClient
17+
runCmdCalls []string
18+
}
19+
20+
func (c *mockIPTablesClientWithRunCmd) RunCmd(version, params string) error {
21+
c.runCmdCalls = append(c.runCmdCalls, version+" "+params)
22+
return nil
23+
}
24+
25+
func TestHandleCommonOptions_AppliesRoutes(t *testing.T) {
26+
routeCalled := false
27+
nl := netlink.NewMockNetlink(false, "")
28+
nl.SetAddRouteValidationFn(func(_ *netlink.Route) error {
29+
routeCalled = true
30+
return nil
31+
})
32+
33+
nm := &networkManager{
34+
netlink: nl,
35+
netio: netio.NewMockNetIO(false, 0),
36+
iptablesClient: &mockIPTablesClient{},
37+
}
38+
39+
nwInfo := &EndpointInfo{
40+
Options: map[string]interface{}{
41+
RoutesKey: []RouteInfo{
42+
{
43+
Dst: net.IPNet{IP: net.ParseIP("10.1.0.0"), Mask: net.CIDRMask(16, 32)},
44+
Gw: net.ParseIP("10.0.0.1"),
45+
},
46+
},
47+
},
48+
}
49+
50+
err := nm.handleCommonOptions("eth0", nwInfo)
51+
require.NoError(t, err)
52+
assert.True(t, routeCalled, "expected route to be added when RoutesKey is present in options")
53+
}
54+
55+
func TestHandleCommonOptions_AppliesIPTables(t *testing.T) {
56+
iptc := &mockIPTablesClientWithRunCmd{}
57+
58+
nm := &networkManager{
59+
netlink: netlink.NewMockNetlink(false, ""),
60+
netio: netio.NewMockNetIO(false, 0),
61+
iptablesClient: iptc,
62+
}
63+
64+
nwInfo := &EndpointInfo{
65+
Options: map[string]interface{}{
66+
IPTablesKey: []iptables.IPTableEntry{
67+
{Version: iptables.V4, Params: "-A FORWARD -j ACCEPT"},
68+
},
69+
},
70+
}
71+
72+
err := nm.handleCommonOptions("eth0", nwInfo)
73+
require.NoError(t, err)
74+
assert.NotEmpty(t, iptc.runCmdCalls, "expected iptables RunCmd to be called when IPTablesKey is present")
75+
}
76+
77+
func TestHandleCommonOptions_NoOptionsNoError(t *testing.T) {
78+
nm := &networkManager{
79+
netlink: netlink.NewMockNetlink(false, ""),
80+
netio: netio.NewMockNetIO(false, 0),
81+
iptablesClient: &mockIPTablesClient{},
82+
}
83+
84+
nwInfo := &EndpointInfo{
85+
Options: map[string]interface{}{},
86+
}
87+
88+
err := nm.handleCommonOptions("eth0", nwInfo)
89+
require.NoError(t, err)
90+
}
91+
92+
func TestHandleCommonOptions_NilOptionsNoError(t *testing.T) {
93+
nm := &networkManager{
94+
netlink: netlink.NewMockNetlink(false, ""),
95+
netio: netio.NewMockNetIO(false, 0),
96+
iptablesClient: &mockIPTablesClient{},
97+
}
98+
99+
nwInfo := &EndpointInfo{
100+
Options: nil,
101+
}
102+
103+
err := nm.handleCommonOptions("eth0", nwInfo)
104+
require.NoError(t, err)
105+
}

0 commit comments

Comments
 (0)