Skip to content

Commit

Permalink
ensure iptables chain creation is idempotent
Browse files Browse the repository at this point in the history
Concurrent use of the `portmap` and `firewall` plugins can result in
errors during iptables chain creation:

- The `portmap` plugin has a time-of-check-time-of-use race where it
  checks for existence of the chain but the operation isn't atomic.
- The `firewall` plugin doesn't check for existing chains and just
  returns an error.

This commit makes both operations idempotent by creating the chain and
then discarding the error if it's caused by the chain already existing.

Signed-off-by: Tim Gross <[email protected]>
  • Loading branch information
tgross committed Nov 6, 2019
1 parent a162329 commit 3d0ddef
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 4 deletions.
7 changes: 7 additions & 0 deletions plugins/meta/firewall/firewall_iptables_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,13 @@ var _ = Describe("firewall plugin iptables backend", func() {
Expect(err).NotTo(HaveOccurred())

validateFullRuleset(fullConf)

// ensure creation is idempotent
_, _, err = testutils.CmdAdd(targetNS.Path(), args.ContainerID, IFNAME, fullConf, func() error {
return cmdAdd(args)
})
Expect(err).NotTo(HaveOccurred())

return nil
})
Expect(err).NotTo(HaveOccurred())
Expand Down
9 changes: 8 additions & 1 deletion plugins/meta/firewall/iptables.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,14 @@ func ensureChain(ipt *iptables.IPTables, table, chain string) error {
}
}

return ipt.NewChain(table, chain)
err = ipt.NewChain(table, chain)
if err != nil {
eerr, eok := err.(*iptables.Error)
if eok && eerr.ExitStatus() != 1 {
return err
}
}
return nil
}

func generateFilterRule(privChainName string) []string {
Expand Down
11 changes: 8 additions & 3 deletions plugins/meta/portmap/chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,19 @@ type chain struct {

// setup idempotently creates the chain. It will not error if the chain exists.
func (c *chain) setup(ipt *iptables.IPTables) error {
// create the chain

exists, err := chainExists(ipt, c.table, c.name)
if err != nil {
return err
}

if !exists {
if err := ipt.NewChain(c.table, c.name); err != nil {
return err
err := ipt.NewChain(c.table, c.name)
if err != nil {
eerr, eok := err.(*iptables.Error)
if eok && eerr.ExitStatus() != 1 {
return err
}
}
}

Expand Down

0 comments on commit 3d0ddef

Please sign in to comment.