Skip to content

Commit

Permalink
refactor - create egressContext new constructor for add/del
Browse files Browse the repository at this point in the history
  • Loading branch information
@wanyufe
wanyufe committed Apr 24, 2023
1 parent ea56b92 commit b78ddb0
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 27 deletions.
101 changes: 96 additions & 5 deletions cmd/egress-cni-plugin/egressContext.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,100 @@ type EgressContext struct {
TmpResult *current.Result
Log logger.Logger

Mtu int
Chain string
Comment string
Mtu int
// SnatChain is the chain name for iptables rules
SnatChain string
// SnatComment is the comment for iptables rules
SnatComment string
}

// EgressContextAddOption has all necessary fields for container egress setup
type EgressContextAddOption struct {
Procsys procsyswrapper.ProcSys
Ipam hostipamwrapper.HostIpam
Link netlinkwrapper.NetLink
Ns nswrapper.NS
NsPath string
ArgsIfName string
Veth vethwrapper.Veth
IpTablesIface iptableswrapper.IPTablesIface
IptCreator func(iptables.Protocol) (iptableswrapper.IPTablesIface, error)
}

// EgressContextDelOption has all necessary fields for container egress removal
type EgressContextDelOption struct {
Ipam hostipamwrapper.HostIpam
Link netlinkwrapper.NetLink
Ns nswrapper.NS
NsPath string
IpTablesIface iptableswrapper.IPTablesIface
IptCreator func(iptables.Protocol) (iptableswrapper.IPTablesIface, error)
}

// NewEgressAddContext create a context for container egress traffic
func NewEgressAddContext(option EgressContextAddOption) (*EgressContext, error) {
if option.Procsys == nil {
return nil, fmt.Errorf("prosys is nil in EgressContextAddOption")
}
if option.Ipam == nil {
return nil, fmt.Errorf("ipam is nil in EgressContextAddOption")
}
if option.Link == nil {
return nil, fmt.Errorf("link is nil in EgressContextAddOption")
}
if option.Ns == nil {
return nil, fmt.Errorf("ns is nil in EgressContextAddOption")
}
if option.NsPath == "" {
return nil, fmt.Errorf("nsPath is empty in EgressContextAddOption")
}
if option.ArgsIfName == "" {
return nil, fmt.Errorf("argsIfName is empty in EgressContextAddOption")
}
if option.Veth == nil {
return nil, fmt.Errorf("veth is nil in EgressContextAddOption")
}
if option.IpTablesIface == nil && option.IptCreator == nil {
return nil, fmt.Errorf("both ipTablesIface and iptCreator are nil in EgressContextAddOption")
}
return &EgressContext{
Procsys: option.Procsys,
Ipam: option.Ipam,
Link: option.Link,
Ns: option.Ns,
NsPath: option.NsPath,
ArgsIfName: option.ArgsIfName,
Veth: option.Veth,
IpTablesIface: option.IpTablesIface,
IptCreator: option.IptCreator,
}, nil
}

// NewEgressDelContext create a context for container egress traffic
func NewEgressDelContext(option EgressContextDelOption) (*EgressContext, error) {
if option.Ipam == nil {
return nil, fmt.Errorf("ipam is nil in EgressContextAddOption")
}
if option.Link == nil {
return nil, fmt.Errorf("link is nil in EgressContextAddOption")
}
if option.Ns == nil {
return nil, fmt.Errorf("ns is nil in EgressContextAddOption")
}
if option.NsPath == "" {
return nil, fmt.Errorf("nsPath is empty in EgressContextAddOption")
}
if option.IpTablesIface == nil && option.IptCreator == nil {
return nil, fmt.Errorf("both ipTablesIface and iptCreator are nil in EgressContextAddOption")
}
return &EgressContext{
Ipam: option.Ipam,
Link: option.Link,
Ns: option.Ns,
NsPath: option.NsPath,
IpTablesIface: option.IpTablesIface,
IptCreator: option.IptCreator,
}, nil
}

func (c *EgressContext) SetupContainerVeth() (*current.Interface, *current.Interface, error) {
Expand Down Expand Up @@ -225,7 +316,7 @@ func (c *EgressContext) CmdAddEgressV4() error {
for _, ipc := range c.TmpResult.IPs {
if ipc.Address.IP.To4() != nil {
// add SNAT chain/rules necessary for the container IPv6 egress traffic
if err = snat.Add(c.IpTablesIface, c.NetConf.NodeIP, ipc.Address.IP, ipv4MulticastRange, c.Chain, c.Comment, c.NetConf.RandomizeSNAT); err != nil {
if err = snat.Add(c.IpTablesIface, c.NetConf.NodeIP, ipc.Address.IP, ipv4MulticastRange, c.SnatChain, c.SnatComment, c.NetConf.RandomizeSNAT); err != nil {
return err
}
}
Expand Down Expand Up @@ -292,7 +383,7 @@ func (c *EgressContext) CmdDelEgressV4() error {
if c.NetConf.NodeIP != nil {
c.Log.Debugf("DEL: SNAT setup, let's clean them up. Size of ipnets: %d", len(ipnets))
for _, ipn := range ipnets {
if err := snat.Del(c.IpTablesIface, ipn.IP, c.Chain, c.Comment); err != nil {
if err := snat.Del(c.IpTablesIface, ipn.IP, c.SnatChain, c.SnatComment); err != nil {
return err
}
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/egress-cni-plugin/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ func _cmdAdd(args *skel.CmdArgs, c *EgressContext) (err error) {
}
}

c.Chain = utils.MustFormatChainNameWithPrefix(c.NetConf.Name, args.ContainerID, "E4-")
c.Comment = utils.FormatComment(c.NetConf.Name, args.ContainerID)
c.SnatChain = utils.MustFormatChainNameWithPrefix(c.NetConf.Name, args.ContainerID, "E4-")
c.SnatComment = utils.FormatComment(c.NetConf.Name, args.ContainerID)

// Invoke ipam del if err to avoid ip leak
defer func() {
Expand Down Expand Up @@ -174,8 +174,8 @@ func _cmdDel(args *skel.CmdArgs, c *EgressContext) (err error) {
return fmt.Errorf("running IPAM plugin failed: %v", err)
}

c.Chain = utils.MustFormatChainNameWithPrefix(c.NetConf.Name, args.ContainerID, "E4-")
c.Comment = utils.FormatComment(c.NetConf.Name, args.ContainerID)
c.SnatChain = utils.MustFormatChainNameWithPrefix(c.NetConf.Name, args.ContainerID, "E4-")
c.SnatComment = utils.FormatComment(c.NetConf.Name, args.ContainerID)

// IPv4 egress
c.NetConf.IfName = EgressIPv4InterfaceName
Expand Down
33 changes: 17 additions & 16 deletions cmd/egress-cni-plugin/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
)

const (
chainV4 = "CNI-E4-5740307710ebfd5fb24f5"
snatChainV4 = "CNI-E4-5740307710ebfd5fb24f5"
)

func TestCmdAddV4(t *testing.T) {
Expand Down Expand Up @@ -66,7 +66,7 @@ func TestCmdAddV4(t *testing.T) {
}`),
}

c := &EgressContext{
addOption := EgressContextAddOption{
Procsys: mock_procsyswrapper.NewMockProcSys(ctrl),
Ns: mock_nswrapper.NewMockNS(ctrl),
NsPath: "/var/run/netns/cni-xxxx",
Expand All @@ -75,20 +75,22 @@ func TestCmdAddV4(t *testing.T) {
Ipam: mock_ipamwrapper.NewMockHostIpam(ctrl),
Link: mock_netlinkwrapper.NewMockNetLink(ctrl),
Veth: mock_veth.NewMockVeth(ctrl),
Mtu: 9001,
}

c, err := NewEgressAddContext(addOption)
assert.Nil(t, err)

var actualIptablesRules, actualRouteAdd, actualRouteDel []string
err := SetupAddExpectV4(*c, chainV4, &actualIptablesRules, &actualRouteAdd, &actualRouteDel)
err = SetupAddExpectV4(*c, snatChainV4, &actualIptablesRules, &actualRouteAdd, &actualRouteDel)
assert.Nil(t, err)

err = _cmdAdd(args, c)
assert.Nil(t, err)

expectIptablesRules := []string{
fmt.Sprintf("nat %s -d 224.0.0.0/4 -j ACCEPT -m comment --comment name: \"aws-cni\" id: \"containerId-123\"", chainV4),
fmt.Sprintf("nat %s -j SNAT --to-source 192.168.1.123 -m comment --comment name: \"aws-cni\" id: \"containerId-123\" --random-fully", chainV4),
fmt.Sprintf("nat POSTROUTING -s 169.254.172.10 -j %s -m comment --comment name: \"aws-cni\" id: \"containerId-123\"", chainV4)}
fmt.Sprintf("nat %s -d 224.0.0.0/4 -j ACCEPT -m comment --comment name: \"aws-cni\" id: \"containerId-123\"", snatChainV4),
fmt.Sprintf("nat %s -j SNAT --to-source 192.168.1.123 -m comment --comment name: \"aws-cni\" id: \"containerId-123\" --random-fully", snatChainV4),
fmt.Sprintf("nat POSTROUTING -s 169.254.172.10 -j %s -m comment --comment name: \"aws-cni\" id: \"containerId-123\"", snatChainV4)}
assert.EqualValues(t, expectIptablesRules, actualIptablesRules)

expectRouteDel := []string{"route del: {Ifindex: 2 Dst: 169.254.172.0/22 Src: <nil> Gw: <nil> Flags: [] Table: 0}"}
Expand Down Expand Up @@ -137,20 +139,19 @@ func TestCmdDelV4(t *testing.T) {
"vethPrefix":"eni"
}`),
}
c := &EgressContext{
Procsys: mock_procsyswrapper.NewMockProcSys(ctrl),
delOption := EgressContextDelOption{
Ns: mock_nswrapper.NewMockNS(ctrl),
NsPath: "/var/run/netns/cni-xxxx",
ArgsIfName: args.IfName,
IpTablesIface: mock_iptables.NewMockIPTablesIface(ctrl),
Ipam: mock_ipamwrapper.NewMockHostIpam(ctrl),
Link: mock_netlinkwrapper.NewMockNetLink(ctrl),
Veth: mock_veth.NewMockVeth(ctrl),
Mtu: 9001,
}

c, err := NewEgressDelContext(delOption)
assert.Nil(t, err)

var actualLinkDel, actualIptablesDel []string
err := SetupDelExpectV4(*c, &actualLinkDel, &actualIptablesDel)
err = SetupDelExpectV4(*c, &actualLinkDel, &actualIptablesDel)
assert.Nil(t, err)

err = _cmdDel(args, c)
Expand All @@ -160,8 +161,8 @@ func TestCmdDelV4(t *testing.T) {
assert.EqualValues(t, expectLinkDel, actualLinkDel)

expectIptablesDel := []string{
fmt.Sprintf("nat POSTROUTING -s 169.254.172.10 -j %s -m comment --comment name: \"aws-cni\" id: \"containerId-123\"", chainV4),
fmt.Sprintf("clear chain nat %s", chainV4),
fmt.Sprintf("del chain nat %s", chainV4)}
fmt.Sprintf("nat POSTROUTING -s 169.254.172.10 -j %s -m comment --comment name: \"aws-cni\" id: \"containerId-123\"", snatChainV4),
fmt.Sprintf("clear chain nat %s", snatChainV4),
fmt.Sprintf("del chain nat %s", snatChainV4)}
assert.EqualValues(t, expectIptablesDel, actualIptablesDel)
}
2 changes: 1 addition & 1 deletion cmd/egress-cni-plugin/test_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func SetupAddExpectV4(c EgressContext, chain string, actualIptablesRules, actual
f(nsParent)
}).Return(nil)

c.Veth.(*mock_veth.MockVeth).EXPECT().Setup(EgressIPv4InterfaceName, c.Mtu, gomock.Any()).Return(
c.Veth.(*mock_veth.MockVeth).EXPECT().Setup(EgressIPv4InterfaceName, 9001, gomock.Any()).Return(
net.Interface{
Name: HostIfName,
HardwareAddr: macHost[:],
Expand Down
2 changes: 1 addition & 1 deletion pkg/networkutils/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ func computeStaleIptablesRules(ipt iptableswrapper.IPTablesIface, table, chainPr
}

func containChainExistErr(err error) bool {
return strings.Contains(err.Error(), "Chain already exists")
return strings.Contains(err.Error(), "SnatChain already exists")
}

type iptablesRule struct {
Expand Down

0 comments on commit b78ddb0

Please sign in to comment.