Skip to content

Commit dcc0a66

Browse files
author
Joseph Chen
committed
reapply aws#2697 from release-1.16 branch
1 parent 870ead7 commit dcc0a66

File tree

11 files changed

+304
-179
lines changed

11 files changed

+304
-179
lines changed

‎pkg/ipamd/ipamd.go

+5
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,11 @@ func (c *IPAMContext) nodeInit() error {
395395
if err != nil {
396396
return errors.Wrap(err, "ipamd init: failed to set up host network")
397397
}
398+
err = c.networkClient.CleanUpStaleAWSChains(c.enableIPv4, c.enableIPv6)
399+
if err != nil {
400+
// We should not error if clean up fails since these chains don't affect the rules
401+
log.Debugf("Failed to clean up stale AWS chains: %v", err)
402+
}
398403

399404
metadataResult, err := c.awsClient.DescribeAllENIs()
400405
if err != nil {

‎pkg/ipamd/ipamd_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ func TestNodeInit(t *testing.T) {
150150
m.awsutils.EXPECT().GetVPCIPv4CIDRs().AnyTimes().Return(cidrs, nil)
151151
m.awsutils.EXPECT().GetPrimaryENImac().Return("")
152152
m.network.EXPECT().SetupHostNetwork(cidrs, "", &primaryIP, false, true, false).Return(nil)
153+
m.network.EXPECT().CleanUpStaleAWSChains(true, false).Return(nil)
153154
m.awsutils.EXPECT().GetPrimaryENI().AnyTimes().Return(primaryENIid)
154155
m.awsutils.EXPECT().RefreshSGIDs(gomock.Any()).AnyTimes().Return(nil)
155156

@@ -234,6 +235,7 @@ func TestNodeInitwithPDenabledIPv4Mode(t *testing.T) {
234235
m.awsutils.EXPECT().GetVPCIPv4CIDRs().AnyTimes().Return(cidrs, nil)
235236
m.awsutils.EXPECT().GetPrimaryENImac().Return("")
236237
m.network.EXPECT().SetupHostNetwork(cidrs, "", &primaryIP, false, true, false).Return(nil)
238+
m.network.EXPECT().CleanUpStaleAWSChains(true, false).Return(nil)
237239
m.awsutils.EXPECT().GetPrimaryENI().AnyTimes().Return(primaryENIid)
238240
m.awsutils.EXPECT().RefreshSGIDs(gomock.Any()).AnyTimes().Return(nil)
239241

@@ -308,6 +310,7 @@ func TestNodeInitwithPDenabledIPv6Mode(t *testing.T) {
308310

309311
primaryIP := net.ParseIP(ipaddr01)
310312
m.network.EXPECT().SetupHostNetwork(cidrs, eni1.MAC, &primaryIP, false, false, true).Return(nil)
313+
m.network.EXPECT().CleanUpStaleAWSChains(false, true).Return(nil)
311314
m.awsutils.EXPECT().GetIPv6PrefixesFromEC2(eni1.ENIID).AnyTimes().Return(eni1.IPv6Prefixes, nil)
312315
m.awsutils.EXPECT().GetPrimaryENI().AnyTimes().Return(primaryENIid)
313316
m.awsutils.EXPECT().GetPrimaryENImac().Return(eni1.MAC)

‎pkg/iptableswrapper/iptables.go

+6
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type IPTablesIface interface {
2929
ClearChain(table, chain string) error
3030
DeleteChain(table, chain string) error
3131
ListChains(table string) ([]string, error)
32+
ChainExists(table, chain string) (bool, error)
3233
HasRandomFully() bool
3334
}
3435

@@ -98,6 +99,11 @@ func (i ipTables) ListChains(table string) ([]string, error) {
9899
return i.ipt.ListChains(table)
99100
}
100101

102+
// ChainExists implements IPTablesIface interface by calling iptables package
103+
func (i ipTables) ChainExists(table, chain string) (bool, error) {
104+
return i.ipt.ChainExists(table, chain)
105+
}
106+
101107
// HasRandomFully implements IPTablesIface interface by calling iptables package
102108
func (i ipTables) HasRandomFully() bool {
103109
return i.ipt.HasRandomFully()

‎pkg/iptableswrapper/mocks/iptables_maps.go

+8
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,14 @@ func (ipt *MockIptables) ListChains(table string) ([]string, error) {
124124
return chains, nil
125125
}
126126

127+
func (ipt *MockIptables) ChainExists(table, chain string) (bool, error) {
128+
_, ok := ipt.DataplaneState[table][chain]
129+
if ok {
130+
return true, nil
131+
}
132+
return false, nil
133+
}
134+
127135
func (ipt *MockIptables) HasRandomFully() bool {
128136
// TODO: Work out how to write a test case for this
129137
return true

‎pkg/iptableswrapper/mocks/iptables_mocks.go

+15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/networkutils/mocks/network_mocks.go

+14
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/networkutils/network.go

+102-58
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ type NetworkAPIs interface {
152152
SetupENINetwork(eniIP string, mac string, deviceNumber int, subnetCIDR string) error
153153
// UpdateHostIptablesRules updates the nat table iptables rules on the host
154154
UpdateHostIptablesRules(vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP, v4Enabled bool, v6Enabled bool) error
155+
CleanUpStaleAWSChains(v4Enabled, v6Enabled bool) error
155156
UseExternalSNAT() bool
156157
GetExcludeSNATCIDRs() []string
157158
GetExternalServiceCIDRs() []string
@@ -375,6 +376,51 @@ func (n *linuxNetwork) UpdateHostIptablesRules(vpcCIDRs []string, primaryMAC str
375376
return n.updateHostIptablesRules(vpcCIDRs, primaryMAC, primaryAddr, v4Enabled, v6Enabled)
376377
}
377378

379+
func (n *linuxNetwork) CleanUpStaleAWSChains(v4Enabled, v6Enabled bool) error {
380+
ipProtocol := iptables.ProtocolIPv4
381+
if v6Enabled {
382+
ipProtocol = iptables.ProtocolIPv6
383+
}
384+
385+
ipt, err := n.newIptables(ipProtocol)
386+
if err != nil {
387+
return errors.Wrap(err, "stale chain cleanup: failed to create iptables")
388+
}
389+
390+
exists, err := ipt.ChainExists("nat", "AWS-SNAT-CHAIN-1")
391+
if err != nil {
392+
return errors.Wrap(err, "stale chain cleanup: failed to check if AWS-SNAT-CHAIN-1 exists")
393+
}
394+
395+
if exists {
396+
existingChains, err := ipt.ListChains("nat")
397+
if err != nil {
398+
return errors.Wrap(err, "stale chain cleanup: failed to list iptables nat chains")
399+
}
400+
401+
for _, chain := range existingChains {
402+
if !strings.HasPrefix(chain, "AWS-CONNMARK-CHAIN") && !strings.HasPrefix(chain, "AWS-SNAT-CHAIN") {
403+
continue
404+
}
405+
parsedChain := strings.Split(chain, "-")
406+
chainNum, err := strconv.Atoi(parsedChain[len(parsedChain)-1])
407+
if err != nil {
408+
return errors.Wrap(err, "stale chain cleanup: failed to convert string to int")
409+
}
410+
// Chains 1 --> x (0 indexed) will be stale
411+
if chainNum > 0 {
412+
// No need to clear the chain since computeStaleIptablesRules cleans up all rules already
413+
log.Infof("Deleting stale chain: %s", chain)
414+
err := ipt.DeleteChain("nat", chain)
415+
if err != nil {
416+
return errors.Wrapf(err, "stale chain cleanup: failed to delete chain %s", chain)
417+
}
418+
}
419+
}
420+
}
421+
return nil
422+
}
423+
378424
func (n *linuxNetwork) updateHostIptablesRules(vpcCIDRs []string, primaryMAC string, primaryAddr *net.IP, v4Enabled bool,
379425
v6Enabled bool) error {
380426
primaryIntf, err := findPrimaryInterfaceName(primaryMAC)
@@ -434,15 +480,13 @@ func (n *linuxNetwork) buildIptablesSNATRules(vpcCIDRs []string, primaryAddr *ne
434480
log.Debugf("Total CIDRs to program - %d", len(allCIDRs))
435481
// build IPTABLES chain for SNAT of non-VPC outbound traffic and excluded CIDRs
436482
var chains []string
437-
for i := 0; i <= len(allCIDRs); i++ {
438-
chain := fmt.Sprintf("AWS-SNAT-CHAIN-%d", i)
439-
log.Debugf("Setup Host Network: iptables -N %s -t nat", chain)
440-
if err := ipt.NewChain("nat", chain); err != nil && !containChainExistErr(err) {
441-
log.Errorf("ipt.NewChain error for chain [%s]: %v", chain, err)
442-
return []iptablesRule{}, errors.Wrapf(err, "host network setup: failed to add chain")
443-
}
444-
chains = append(chains, chain)
483+
chain := "AWS-SNAT-CHAIN-0"
484+
log.Debugf("Setup Host Network: iptables -N %s -t nat", chain)
485+
if err := ipt.NewChain("nat", chain); err != nil && !containChainExistErr(err) {
486+
log.Errorf("ipt.NewChain error for chain [%s]: %v", chain, err)
487+
return []iptablesRule{}, errors.Wrapf(err, "host network setup: failed to add chain")
445488
}
489+
chains = append(chains, chain)
446490

447491
// build SNAT rules for outbound non-VPC traffic
448492
var iptableRules []iptablesRule
@@ -456,23 +500,20 @@ func (n *linuxNetwork) buildIptablesSNATRules(vpcCIDRs []string, primaryAddr *ne
456500
"-m", "comment", "--comment", "AWS SNAT CHAIN", "-j", "AWS-SNAT-CHAIN-0",
457501
}})
458502

459-
for i, cidr := range allCIDRs {
460-
curChain := chains[i]
461-
curName := fmt.Sprintf("[%d] AWS-SNAT-CHAIN", i)
462-
nextChain := chains[i+1]
503+
for _, cidr := range allCIDRs {
463504
comment := "AWS SNAT CHAIN"
464505
if cidr.isExclusion {
465506
comment += " EXCLUSION"
466507
}
467-
log.Debugf("Setup Host Network: iptables -A %s ! -d %s -t nat -j %s", curChain, cidr, nextChain)
508+
log.Debugf("Setup Host Network: iptables -A %s -d %s -t nat -j %s", chain, cidr, "RETURN")
468509

469510
iptableRules = append(iptableRules, iptablesRule{
470-
name: curName,
511+
name: chain,
471512
shouldExist: !n.useExternalSNAT,
472513
table: "nat",
473-
chain: curChain,
514+
chain: chain,
474515
rule: []string{
475-
"!", "-d", cidr.cidr, "-m", "comment", "--comment", comment, "-j", nextChain,
516+
"-d", cidr.cidr, "-m", "comment", "--comment", comment, "-j", "RETURN",
476517
}})
477518
}
478519

@@ -494,22 +535,21 @@ func (n *linuxNetwork) buildIptablesSNATRules(vpcCIDRs []string, primaryAddr *ne
494535
}
495536
}
496537

497-
lastChain := chains[len(chains)-1]
498-
iptableRules = append(iptableRules, iptablesRule{
499-
name: "last SNAT rule for non-VPC outbound traffic",
500-
shouldExist: !n.useExternalSNAT,
501-
table: "nat",
502-
chain: lastChain,
503-
rule: snatRule,
504-
})
505-
506538
snatStaleRules, err := computeStaleIptablesRules(ipt, "nat", "AWS-SNAT-CHAIN", iptableRules, chains)
507539
if err != nil {
508540
return []iptablesRule{}, err
509541
}
510542

511543
iptableRules = append(iptableRules, snatStaleRules...)
512544

545+
iptableRules = append(iptableRules, iptablesRule{
546+
name: "last SNAT rule for non-VPC outbound traffic",
547+
shouldExist: !n.useExternalSNAT,
548+
table: "nat",
549+
chain: chain,
550+
rule: snatRule,
551+
})
552+
513553
iptableRules = append(iptableRules, iptablesRule{
514554
name: "connmark for primary ENI",
515555
shouldExist: n.nodePortSupportEnabled,
@@ -556,16 +596,15 @@ func (n *linuxNetwork) buildIptablesConnmarkRules(vpcCIDRs []string, ipt iptable
556596
excludeCIDRs := sets.NewString(n.excludeSNATCIDRs...)
557597

558598
log.Debugf("Total CIDRs to exempt from connmark rules - %d", len(allCIDRs))
599+
559600
var chains []string
560-
for i := 0; i <= len(allCIDRs); i++ {
561-
chain := fmt.Sprintf("AWS-CONNMARK-CHAIN-%d", i)
562-
log.Debugf("Setup Host Network: iptables -N %s -t nat", chain)
563-
if err := ipt.NewChain("nat", chain); err != nil && !containChainExistErr(err) {
564-
log.Errorf("ipt.NewChain error for chain [%s]: %v", chain, err)
565-
return []iptablesRule{}, errors.Wrapf(err, "host network setup: failed to add chain")
566-
}
567-
chains = append(chains, chain)
601+
chain := "AWS-CONNMARK-CHAIN-0"
602+
log.Debugf("Setup Host Network: iptables -N %s -t nat", chain)
603+
if err := ipt.NewChain("nat", chain); err != nil && !containChainExistErr(err) {
604+
log.Errorf("ipt.NewChain error for chain [%s]: %v", chain, err)
605+
return []iptablesRule{}, errors.Wrapf(err, "host network setup: failed to add chain")
568606
}
607+
chains = append(chains, chain)
569608

570609
var iptableRules []iptablesRule
571610
log.Debugf("Setup Host Network: iptables -t nat -A PREROUTING -i %s+ -m comment --comment \"AWS, outbound connections\" -j AWS-CONNMARK-CHAIN-0", n.vethPrefix)
@@ -590,37 +629,23 @@ func (n *linuxNetwork) buildIptablesConnmarkRules(vpcCIDRs []string, ipt iptable
590629
"-j", "AWS-CONNMARK-CHAIN-0",
591630
}})
592631

593-
for i, cidr := range allCIDRs {
594-
curChain := chains[i]
595-
curName := fmt.Sprintf("[%d] AWS-SNAT-CHAIN", i)
596-
nextChain := chains[i+1]
632+
for _, cidr := range allCIDRs {
597633
comment := "AWS CONNMARK CHAIN, VPC CIDR"
598634
if excludeCIDRs.Has(cidr) {
599635
comment = "AWS CONNMARK CHAIN, EXCLUDED CIDR"
600636
}
601-
log.Debugf("Setup Host Network: iptables -A %s ! -d %s -t nat -j %s", curChain, cidr, nextChain)
637+
log.Debugf("Setup Host Network: iptables -A %s -d %s -t nat -j %s", chain, cidr, "RETURN")
602638

603639
iptableRules = append(iptableRules, iptablesRule{
604-
name: curName,
640+
name: chain,
605641
shouldExist: !n.useExternalSNAT,
606642
table: "nat",
607-
chain: curChain,
643+
chain: chain,
608644
rule: []string{
609-
"!", "-d", cidr, "-m", "comment", "--comment", comment, "-j", nextChain,
645+
"-d", cidr, "-m", "comment", "--comment", comment, "-j", "RETURN",
610646
}})
611647
}
612648

613-
iptableRules = append(iptableRules, iptablesRule{
614-
name: "connmark rule for external outbound traffic",
615-
shouldExist: !n.useExternalSNAT,
616-
table: "nat",
617-
chain: chains[len(chains)-1],
618-
rule: []string{
619-
"-m", "comment", "--comment", "AWS, CONNMARK", "-j", "CONNMARK",
620-
"--set-xmark", fmt.Sprintf("%#x/%#x", n.mainENIMark, n.mainENIMark),
621-
},
622-
})
623-
624649
// Force delete existing restore mark rule so that the subsequent rule gets added to the end
625650
iptableRules = append(iptableRules, iptablesRule{
626651
name: "connmark to fwmark copy",
@@ -652,14 +677,24 @@ func (n *linuxNetwork) buildIptablesConnmarkRules(vpcCIDRs []string, ipt iptable
652677
}
653678
iptableRules = append(iptableRules, connmarkStaleRules...)
654679

680+
iptableRules = append(iptableRules, iptablesRule{
681+
name: "connmark rule for external outbound traffic",
682+
shouldExist: !n.useExternalSNAT,
683+
table: "nat",
684+
chain: chain,
685+
rule: []string{
686+
"-m", "comment", "--comment", "AWS, CONNMARK", "-j", "CONNMARK",
687+
"--set-xmark", fmt.Sprintf("%#x/%#x", n.mainENIMark, n.mainENIMark),
688+
},
689+
})
690+
655691
log.Debugf("iptableRules: %v", iptableRules)
656692
return iptableRules, nil
657693
}
658694

659695
func (n *linuxNetwork) updateIptablesRules(iptableRules []iptablesRule, ipt iptableswrapper.IPTablesIface) error {
660696
for _, rule := range iptableRules {
661697
log.Debugf("execute iptable rule : %s", rule.name)
662-
663698
exists, err := ipt.Exists(rule.table, rule.chain, rule.rule...)
664699
log.Debugf("rule %v exists %v, err %v", rule, exists, err)
665700
if err != nil {
@@ -668,10 +703,19 @@ func (n *linuxNetwork) updateIptablesRules(iptableRules []iptablesRule, ipt ipta
668703
}
669704

670705
if !exists && rule.shouldExist {
671-
err = ipt.Append(rule.table, rule.chain, rule.rule...)
672-
if err != nil {
673-
log.Errorf("host network setup: failed to add %v, %v", rule, err)
674-
return errors.Wrapf(err, "host network setup: failed to add %v", rule)
706+
if rule.name == "AWS-CONNMARK-CHAIN-0" || rule.name == "AWS-SNAT-CHAIN-0" {
707+
// All CIDR rules must go before the SNAT/Mark rule
708+
err = ipt.Insert(rule.table, rule.chain, 1, rule.rule...)
709+
if err != nil {
710+
log.Errorf("host network setup: failed to insert %v, %v", rule, err)
711+
return errors.Wrapf(err, "host network setup: failed to add %v", rule)
712+
}
713+
} else {
714+
err = ipt.Append(rule.table, rule.chain, rule.rule...)
715+
if err != nil {
716+
log.Errorf("host network setup: failed to add %v, %v", rule, err)
717+
return errors.Wrapf(err, "host network setup: failed to add %v", rule)
718+
}
675719
}
676720
} else if exists && !rule.shouldExist {
677721
err = ipt.Delete(rule.table, rule.chain, rule.rule...)
@@ -726,7 +770,7 @@ func computeStaleIptablesRules(ipt iptableswrapper.IPTablesIface, table, chainPr
726770
return []iptablesRule{}, errors.Wrapf(err, "host network setup: failed to list rules from table %s with chain prefix %s", table, chainPrefix)
727771
}
728772
activeChains := sets.NewString(chains...)
729-
log.Debugf("Setup Host Network: computing stale iptables rules for %s table with chain prefix %s")
773+
log.Debugf("Setup Host Network: computing stale iptables rules for %s table with chain prefix %s", table, chainPrefix)
730774
for _, staleRule := range existingRules {
731775
if len(staleRule.rule) == 0 && activeChains.Has(staleRule.chain) {
732776
log.Debugf("Setup Host Network: active chain found: %s", staleRule.chain)

0 commit comments

Comments
 (0)