diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index ae3f86c6756..54593353a92 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -423,7 +423,7 @@ func CleanupLeftovers(ipt utiliptables.Interface) (encounteredError bool) { klog.ErrorS(err, "Failed to execute iptables-save", "table", utiliptables.TableNAT) encounteredError = true } else { - existingNATChains := utiliptables.GetChainLines(utiliptables.TableNAT, iptablesData.Bytes()) + existingNATChains := utiliptables.GetChainsFromTable(iptablesData.Bytes()) natChains := &utilproxy.LineBuffer{} natRules := &utilproxy.LineBuffer{} natChains.Write("*nat") @@ -460,7 +460,7 @@ func CleanupLeftovers(ipt utiliptables.Interface) (encounteredError bool) { klog.ErrorS(err, "Failed to execute iptables-save", "table", utiliptables.TableFilter) encounteredError = true } else { - existingFilterChains := utiliptables.GetChainLines(utiliptables.TableFilter, iptablesData.Bytes()) + existingFilterChains := utiliptables.GetChainsFromTable(iptablesData.Bytes()) filterChains := &utilproxy.LineBuffer{} filterRules := &utilproxy.LineBuffer{} filterChains.Write("*filter") @@ -890,14 +890,13 @@ func (proxier *Proxier) syncProxyRules() { // Get iptables-save output so we can check for existing chains and rules. // This will be a map of chain name to chain with rules as stored in iptables-save/iptables-restore - // IMPORTANT: existingNATChains may share memory with proxier.iptablesData. - existingNATChains := make(map[utiliptables.Chain][]byte) + existingNATChains := make(map[utiliptables.Chain]struct{}) proxier.iptablesData.Reset() err := proxier.iptables.SaveInto(utiliptables.TableNAT, proxier.iptablesData) if err != nil { klog.ErrorS(err, "Failed to execute iptables-save: stale chains will not be deleted") } else { - existingNATChains = utiliptables.GetChainLines(utiliptables.TableNAT, proxier.iptablesData.Bytes()) + existingNATChains = utiliptables.GetChainsFromTable(proxier.iptablesData.Bytes()) } // Reset all buffers used later. diff --git a/pkg/util/iptables/save_restore.go b/pkg/util/iptables/save_restore.go index 7ec11e4f00f..b788beb9113 100644 --- a/pkg/util/iptables/save_restore.go +++ b/pkg/util/iptables/save_restore.go @@ -21,102 +21,32 @@ import ( "fmt" ) -var ( - commitBytes = []byte("COMMIT") - spaceBytes = []byte(" ") -) - // MakeChainLine return an iptables-save/restore formatted chain line given a Chain func MakeChainLine(chain Chain) string { return fmt.Sprintf(":%s - [0:0]", chain) } -// GetChainLines parses a table's iptables-save data to find chains in the table. -// It returns a map of iptables.Chain to []byte where the []byte is the chain line -// from save (with counters etc.). -// Note that to avoid allocations memory is SHARED with save. -func GetChainLines(table Table, save []byte) map[Chain][]byte { - chainsMap := make(map[Chain][]byte) - tablePrefix := []byte("*" + string(table)) - readIndex := 0 - // find beginning of table - for readIndex < len(save) { - line, n := readLine(readIndex, save) - readIndex = n - if bytes.HasPrefix(line, tablePrefix) { +// GetChainsFromTable parses iptables-save data to find the chains that are defined. It +// assumes that save contains a single table's data, and returns a map with keys for every +// chain defined in that table. +func GetChainsFromTable(save []byte) map[Chain]struct{} { + chainsMap := make(map[Chain]struct{}) + + for { + i := bytes.Index(save, []byte("\n:")) + if i == -1 { break } - } - // parse table lines - for readIndex < len(save) { - line, n := readLine(readIndex, save) - readIndex = n - if len(line) == 0 { - continue - } - if bytes.HasPrefix(line, commitBytes) || line[0] == '*' { + start := i + 2 + save = save[start:] + end := bytes.Index(save, []byte(" ")) + if i == -1 { + // shouldn't happen, but... break - } else if line[0] == '#' { - continue - } else if line[0] == ':' && len(line) > 1 { - // We assume that the contains space - chain lines have 3 fields, - // space delimited. If there is no space, this line will panic. - spaceIndex := bytes.Index(line, spaceBytes) - if spaceIndex == -1 { - panic(fmt.Sprintf("Unexpected chain line in iptables-save output: %v", string(line))) - } - chain := Chain(line[1:spaceIndex]) - chainsMap[chain] = line } + chain := Chain(save[:end]) + chainsMap[chain] = struct{}{} + save = save[end:] } return chainsMap } - -func readLine(readIndex int, byteArray []byte) ([]byte, int) { - currentReadIndex := readIndex - - // consume left spaces - for currentReadIndex < len(byteArray) { - if byteArray[currentReadIndex] == ' ' { - currentReadIndex++ - } else { - break - } - } - - // leftTrimIndex stores the left index of the line after the line is left-trimmed - leftTrimIndex := currentReadIndex - - // rightTrimIndex stores the right index of the line after the line is right-trimmed - // it is set to -1 since the correct value has not yet been determined. - rightTrimIndex := -1 - - for ; currentReadIndex < len(byteArray); currentReadIndex++ { - if byteArray[currentReadIndex] == ' ' { - // set rightTrimIndex - if rightTrimIndex == -1 { - rightTrimIndex = currentReadIndex - } - } else if (byteArray[currentReadIndex] == '\n') || (currentReadIndex == (len(byteArray) - 1)) { - // end of line or byte buffer is reached - if currentReadIndex <= leftTrimIndex { - return nil, currentReadIndex + 1 - } - // set the rightTrimIndex - if rightTrimIndex == -1 { - rightTrimIndex = currentReadIndex - if currentReadIndex == (len(byteArray)-1) && (byteArray[currentReadIndex] != '\n') { - // ensure that the last character is part of the returned string, - // unless the last character is '\n' - rightTrimIndex = currentReadIndex + 1 - } - } - // Avoid unnecessary allocation. - return byteArray[leftTrimIndex:rightTrimIndex], currentReadIndex + 1 - } else { - // unset rightTrimIndex - rightTrimIndex = -1 - } - } - return nil, currentReadIndex -} diff --git a/pkg/util/iptables/save_restore_test.go b/pkg/util/iptables/save_restore_test.go index 52b4f420511..2a4c383e12f 100644 --- a/pkg/util/iptables/save_restore_test.go +++ b/pkg/util/iptables/save_restore_test.go @@ -22,73 +22,23 @@ import ( "github.com/lithammer/dedent" ) -func TestReadLinesFromByteBuffer(t *testing.T) { - testFn := func(byteArray []byte, expected []string) { - index := 0 - readIndex := 0 - for ; readIndex < len(byteArray); index++ { - line, n := readLine(readIndex, byteArray) - readIndex = n - if expected[index] != string(line) { - t.Errorf("expected:%q, actual:%q", expected[index], line) - } - } // for - if readIndex < len(byteArray) { - t.Errorf("Byte buffer was only partially read. Buffer length is:%d, readIndex is:%d", len(byteArray), readIndex) - } - if index < len(expected) { - t.Errorf("All expected strings were not compared. expected arr length:%d, matched count:%d", len(expected), index-1) +func checkChains(t *testing.T, save []byte, expected map[Chain]struct{}) { + chains := GetChainsFromTable(save) + for chain := range expected { + if _, exists := chains[chain]; !exists { + t.Errorf("GetChainsFromTable expected chain not present: %s", chain) } } - - byteArray1 := []byte("\n Line 1 \n\n\n L ine4 \nLine 5 \n \n") - expected1 := []string{"", "Line 1", "", "", "L ine4", "Line 5", ""} - testFn(byteArray1, expected1) - - byteArray1 = []byte("") - expected1 = []string{} - testFn(byteArray1, expected1) - - byteArray1 = []byte("\n\n") - expected1 = []string{"", ""} - testFn(byteArray1, expected1) -} - -func checkAllLines(t *testing.T, table Table, save []byte, expectedLines map[Chain]string) { - chainLines := GetChainLines(table, save) - for chain, lineBytes := range chainLines { - line := string(lineBytes) - if expected, exists := expectedLines[chain]; exists { - if expected != line { - t.Errorf("getChainLines expected chain line not present. For chain: %s Expected: %s Got: %s", chain, expected, line) - } - } else { - t.Errorf("getChainLines expected chain not present: %s", chain) + for chain := range chains { + if _, exists := expected[chain]; !exists { + t.Errorf("GetChainsFromTable chain unexpectedly present: %s", chain) } } } -func TestGetChainLines(t *testing.T) { - iptablesSave := dedent.Dedent( - `# Generated by iptables-save v1.4.7 on Wed Oct 29 14:56:01 2014 - *nat - :PREROUTING ACCEPT [2136997:197881818] - :POSTROUTING ACCEPT [4284525:258542680] - :OUTPUT ACCEPT [5901660:357267963] - -A PREROUTING -m addrtype --dst-type LOCAL -j DOCKER - COMMIT - # Completed on Wed Oct 29 14:56:01 2014`) - expected := map[Chain]string{ - ChainPrerouting: ":PREROUTING ACCEPT [2136997:197881818]", - ChainPostrouting: ":POSTROUTING ACCEPT [4284525:258542680]", - ChainOutput: ":OUTPUT ACCEPT [5901660:357267963]", - } - checkAllLines(t, TableNAT, []byte(iptablesSave), expected) -} - -func TestGetChainLinesMultipleTables(t *testing.T) { - iptablesSave := dedent.Dedent( - `# Generated by iptables-save v1.4.21 on Fri Aug 7 14:47:37 2015 +func TestGetChainsFromTable(t *testing.T) { + iptablesSave := dedent.Dedent(` + # Generated by iptables-save v1.4.21 on Fri Aug 7 14:47:37 2015 *nat :PREROUTING ACCEPT [2:138] :INPUT ACCEPT [0:0] @@ -126,35 +76,23 @@ func TestGetChainLinesMultipleTables(t *testing.T) { -A KUBE-SVC-5555555555555555 -m comment --comment "default/kubernetes:" -j KUBE-SVC-4444444444444444 -A KUBE-SVC-6666666666666666 -m comment --comment "kube-system/kube-dns:dns" -j KUBE-SVC-1111111111111111 COMMIT - # Completed on Fri Aug 7 14:47:37 2015 - # Generated by iptables-save v1.4.21 on Fri Aug 7 14:47:37 2015 - *filter - :INPUT ACCEPT [17514:83115836] - :FORWARD ACCEPT [0:0] - :OUTPUT ACCEPT [8909:688225] - :DOCKER - [0:0] - -A FORWARD -o cbr0 -j DOCKER - -A FORWARD -o cbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT - -A FORWARD -i cbr0 ! -o cbr0 -j ACCEPT - -A FORWARD -i cbr0 -o cbr0 -j ACCEPT - COMMIT `) - expected := map[Chain]string{ - ChainPrerouting: ":PREROUTING ACCEPT [2:138]", - Chain("INPUT"): ":INPUT ACCEPT [0:0]", - Chain("OUTPUT"): ":OUTPUT ACCEPT [0:0]", - ChainPostrouting: ":POSTROUTING ACCEPT [0:0]", - Chain("DOCKER"): ":DOCKER - [0:0]", - Chain("KUBE-NODEPORT-CONTAINER"): ":KUBE-NODEPORT-CONTAINER - [0:0]", - Chain("KUBE-NODEPORT-HOST"): ":KUBE-NODEPORT-HOST - [0:0]", - Chain("KUBE-PORTALS-CONTAINER"): ":KUBE-PORTALS-CONTAINER - [0:0]", - Chain("KUBE-PORTALS-HOST"): ":KUBE-PORTALS-HOST - [0:0]", - Chain("KUBE-SVC-1111111111111111"): ":KUBE-SVC-1111111111111111 - [0:0]", - Chain("KUBE-SVC-2222222222222222"): ":KUBE-SVC-2222222222222222 - [0:0]", - Chain("KUBE-SVC-3333333333333333"): ":KUBE-SVC-3333333333333333 - [0:0]", - Chain("KUBE-SVC-4444444444444444"): ":KUBE-SVC-4444444444444444 - [0:0]", - Chain("KUBE-SVC-5555555555555555"): ":KUBE-SVC-5555555555555555 - [0:0]", - Chain("KUBE-SVC-6666666666666666"): ":KUBE-SVC-6666666666666666 - [0:0]", + expected := map[Chain]struct{}{ + ChainPrerouting: {}, + Chain("INPUT"): {}, + Chain("OUTPUT"): {}, + ChainPostrouting: {}, + Chain("DOCKER"): {}, + Chain("KUBE-NODEPORT-CONTAINER"): {}, + Chain("KUBE-NODEPORT-HOST"): {}, + Chain("KUBE-PORTALS-CONTAINER"): {}, + Chain("KUBE-PORTALS-HOST"): {}, + Chain("KUBE-SVC-1111111111111111"): {}, + Chain("KUBE-SVC-2222222222222222"): {}, + Chain("KUBE-SVC-3333333333333333"): {}, + Chain("KUBE-SVC-4444444444444444"): {}, + Chain("KUBE-SVC-5555555555555555"): {}, + Chain("KUBE-SVC-6666666666666666"): {}, } - checkAllLines(t, TableNAT, []byte(iptablesSave), expected) + checkChains(t, []byte(iptablesSave), expected) }