diff --git a/src/code.cloudfoundry.org/cni-wrapper-plugin/fakes/rule_converter.go b/src/code.cloudfoundry.org/cni-wrapper-plugin/fakes/rule_converter.go index aff7c1df8..58084d3c8 100644 --- a/src/code.cloudfoundry.org/cni-wrapper-plugin/fakes/rule_converter.go +++ b/src/code.cloudfoundry.org/cni-wrapper-plugin/fakes/rule_converter.go @@ -35,6 +35,17 @@ type RuleConverter struct { convertReturnsOnCall map[int]struct { result1 []rules.IPTablesRule } + DeduplicateRulesStub func([]rules.IPTablesRule) []rules.IPTablesRule + deduplicateRulesMutex sync.RWMutex + deduplicateRulesArgsForCall []struct { + arg1 []rules.IPTablesRule + } + deduplicateRulesReturns struct { + result1 []rules.IPTablesRule + } + deduplicateRulesReturnsOnCall map[int]struct { + result1 []rules.IPTablesRule + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -170,6 +181,72 @@ func (fake *RuleConverter) ConvertReturnsOnCall(i int, result1 []rules.IPTablesR }{result1} } +func (fake *RuleConverter) DeduplicateRules(arg1 []rules.IPTablesRule) []rules.IPTablesRule { + var arg1Copy []rules.IPTablesRule + if arg1 != nil { + arg1Copy = make([]rules.IPTablesRule, len(arg1)) + copy(arg1Copy, arg1) + } + fake.deduplicateRulesMutex.Lock() + ret, specificReturn := fake.deduplicateRulesReturnsOnCall[len(fake.deduplicateRulesArgsForCall)] + fake.deduplicateRulesArgsForCall = append(fake.deduplicateRulesArgsForCall, struct { + arg1 []rules.IPTablesRule + }{arg1Copy}) + stub := fake.DeduplicateRulesStub + fakeReturns := fake.deduplicateRulesReturns + fake.recordInvocation("DeduplicateRules", []interface{}{arg1Copy}) + fake.deduplicateRulesMutex.Unlock() + if stub != nil { + return stub(arg1) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *RuleConverter) DeduplicateRulesCallCount() int { + fake.deduplicateRulesMutex.RLock() + defer fake.deduplicateRulesMutex.RUnlock() + return len(fake.deduplicateRulesArgsForCall) +} + +func (fake *RuleConverter) DeduplicateRulesCalls(stub func([]rules.IPTablesRule) []rules.IPTablesRule) { + fake.deduplicateRulesMutex.Lock() + defer fake.deduplicateRulesMutex.Unlock() + fake.DeduplicateRulesStub = stub +} + +func (fake *RuleConverter) DeduplicateRulesArgsForCall(i int) []rules.IPTablesRule { + fake.deduplicateRulesMutex.RLock() + defer fake.deduplicateRulesMutex.RUnlock() + argsForCall := fake.deduplicateRulesArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *RuleConverter) DeduplicateRulesReturns(result1 []rules.IPTablesRule) { + fake.deduplicateRulesMutex.Lock() + defer fake.deduplicateRulesMutex.Unlock() + fake.DeduplicateRulesStub = nil + fake.deduplicateRulesReturns = struct { + result1 []rules.IPTablesRule + }{result1} +} + +func (fake *RuleConverter) DeduplicateRulesReturnsOnCall(i int, result1 []rules.IPTablesRule) { + fake.deduplicateRulesMutex.Lock() + defer fake.deduplicateRulesMutex.Unlock() + fake.DeduplicateRulesStub = nil + if fake.deduplicateRulesReturnsOnCall == nil { + fake.deduplicateRulesReturnsOnCall = make(map[int]struct { + result1 []rules.IPTablesRule + }) + } + fake.deduplicateRulesReturnsOnCall[i] = struct { + result1 []rules.IPTablesRule + }{result1} +} + func (fake *RuleConverter) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() @@ -177,6 +254,8 @@ func (fake *RuleConverter) Invocations() map[string][][]interface{} { defer fake.bulkConvertMutex.RUnlock() fake.convertMutex.RLock() defer fake.convertMutex.RUnlock() + fake.deduplicateRulesMutex.RLock() + defer fake.deduplicateRulesMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout.go b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout.go index 9983447e9..2a7e757f2 100644 --- a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout.go +++ b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout.go @@ -19,6 +19,7 @@ const secondInMillis = 1000 type ruleConverter interface { Convert(Rule, string, bool) []rules.IPTablesRule BulkConvert([]Rule, string, bool) []rules.IPTablesRule + DeduplicateRules([]rules.IPTablesRule) []rules.IPTablesRule } type OutConn struct { diff --git a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain.go b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain.go index 98a6ee668..37097d3ab 100644 --- a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain.go +++ b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain.go @@ -62,6 +62,8 @@ func (c *NetOutChain) IPTablesRules(containerHandle string, containerWorkload st } iptablesRules := c.Converter.BulkConvert(ruleSpec, logChain, c.ASGLogging) + iptablesRules = c.Converter.DeduplicateRules(iptablesRules) + iptablesRules = append(iptablesRules, c.denyNetworksRules(containerWorkload)...) if c.Conn.Limit || c.Conn.DryRun { diff --git a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain_test.go b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain_test.go index 6b199c1a5..a36f4cb16 100644 --- a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain_test.go +++ b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/netout_chain_test.go @@ -77,7 +77,7 @@ var _ = Describe("NetOutChain", func() { } converter.BulkConvertReturns(genericRules) - + converter.DeduplicateRulesReturns(genericRules) }) It("prepends allow rules to the container's netout chain", func() { diff --git a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter.go b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter.go index 38e21710f..2e3fbba20 100644 --- a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter.go +++ b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter.go @@ -59,6 +59,22 @@ func (c *RuleConverter) BulkConvert(ruleSpec []Rule, logChainName string, global return iptablesRules } +func (c *RuleConverter) DeduplicateRules(iptablesRules []rules.IPTablesRule) []rules.IPTablesRule { + keys := make(map[string]bool) + dedupedRules := []rules.IPTablesRule{} + + for _, rule := range iptablesRules { + key := strings.Join(rule, " ") + if !keys[key] { + + keys[key] = true + dedupedRules = append(dedupedRules, rule) + } + } + + return dedupedRules +} + func (c *RuleConverter) Convert(rule Rule, logChainName string, globalLogging bool) []rules.IPTablesRule { ruleSpec := []rules.IPTablesRule{} for _, network := range rule.Networks() { diff --git a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter_test.go b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter_test.go index 1d3b271f4..fb18470dd 100644 --- a/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter_test.go +++ b/src/code.cloudfoundry.org/cni-wrapper-plugin/netrules/rule_converter_test.go @@ -426,4 +426,62 @@ var _ = Describe("RuleConverter", func() { }) }) + Describe("DeduplicateRules", func() { + var unfilteredRules []rules.IPTablesRule + Context("when there are duplicate iptables rules", func() { + BeforeEach(func() { + unfilteredRules = []rules.IPTablesRule{ + { + "-m", "iprange", "-p", "tcp", + "--dst-range", "1.1.1.1-2.2.2.2", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT", + }, + { + "-m", "iprange", "-p", "tcp", + "--dst-range", "1.1.1.1-2.2.2.2", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT", + }, + { + "-m", "iprange", "-p", "tcp", + "--dst-range", "1.1.1.1-2.2.2.2", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT", + }, + { + "-m", "iprange", "-p", "tcp", + "--dst-range", "3.3.3.3-4.4.4.4", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT", + }, + { + "-m", "iprange", "-p", "tcp", + "--dst-range", "4.4.4.4-5.5.5.5", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT", + }, + } + }) + + It("removes the duplicate iptables rules", func() { + dedupedRules := converter.DeduplicateRules(unfilteredRules) + + Expect(dedupedRules).To(ConsistOf( + rules.IPTablesRule{"-m", "iprange", "-p", "tcp", + "--dst-range", "1.1.1.1-2.2.2.2", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT"}, + rules.IPTablesRule{"-m", "iprange", "-p", "tcp", + "--dst-range", "4.4.4.4-5.5.5.5", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT"}, + rules.IPTablesRule{"-m", "iprange", "-p", "tcp", + "--dst-range", "3.3.3.3-4.4.4.4", + "-m", "tcp", "--destination-port", "9000:9999", + "--jump", "ACCEPT"}, + )) + }) + }) + }) })