From 3d309700d04572f8b81c48b7d4c2ea22065e5086 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 15 Aug 2015 21:10:12 -0700 Subject: [PATCH] Make iptables use semver lib --- pkg/util/iptables/iptables.go | 60 ++++++++---------------------- pkg/util/iptables/iptables_test.go | 59 ----------------------------- 2 files changed, 15 insertions(+), 104 deletions(-) diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go index 5e686d71db..35f4d84297 100644 --- a/pkg/util/iptables/iptables.go +++ b/pkg/util/iptables/iptables.go @@ -21,10 +21,10 @@ import ( "io/ioutil" "os" "regexp" - "strconv" "strings" "sync" + "github.com/coreos/go-semver/semver" "github.com/golang/glog" "k8s.io/kubernetes/pkg/util" utilexec "k8s.io/kubernetes/pkg/util/exec" @@ -106,6 +106,10 @@ type FlushFlag bool const FlushTables FlushFlag = true const NoFlushTables FlushFlag = false +// Versions of iptables less than this do not support the -C / --check flag +// (test whether a rule exists). +const MinCheckVersion = "1.4.11" + // runner implements Interface in terms of exec("iptables"). type runner struct { mu sync.Mutex @@ -399,44 +403,24 @@ func makeFullArgs(table Table, chain Chain, args ...string) []string { // Checks if iptables has the "-C" flag func getIptablesHasCheckCommand(exec utilexec.Interface) (bool, error) { + minVersion, err := semver.NewVersion(MinCheckVersion) + if err != nil { + return false, err + } + // Returns "vX.Y.Z". vstring, err := GetIptablesVersionString(exec) if err != nil { return false, err } - - v1, v2, v3, err := extractIptablesVersion(vstring) + // Make a semver of the part after the v in "vX.X.X". + version, err := semver.NewVersion(vstring[1:]) if err != nil { return false, err } - - return iptablesHasCheckCommand(v1, v2, v3), nil -} - -// extractIptablesVersion returns the first three components of the iptables version. -// e.g. "iptables v1.3.66" would return (1, 3, 66, nil) -func extractIptablesVersion(str string) (int, int, int, error) { - versionMatcher := regexp.MustCompile("v([0-9]+)\\.([0-9]+)\\.([0-9]+)") - result := versionMatcher.FindStringSubmatch(str) - if result == nil { - return 0, 0, 0, fmt.Errorf("no iptables version found in string: %s", str) + if version.LessThan(*minVersion) { + return false, nil } - - v1, err := strconv.Atoi(result[1]) - if err != nil { - return 0, 0, 0, err - } - - v2, err := strconv.Atoi(result[2]) - if err != nil { - return 0, 0, 0, err - } - - v3, err := strconv.Atoi(result[3]) - if err != nil { - return 0, 0, 0, err - } - - return v1, v2, v3, nil + return true, nil } // GetIptablesVersionString runs "iptables --version" to get the version string, @@ -455,17 +439,3 @@ func GetIptablesVersionString(exec utilexec.Interface) (string, error) { } return match[0], nil } - -// Checks if an iptables version is after 1.4.11, when --check was added -func iptablesHasCheckCommand(v1 int, v2 int, v3 int) bool { - if v1 > 1 { - return true - } - if v1 == 1 && v2 > 4 { - return true - } - if v1 == 1 && v2 == 4 && v3 >= 11 { - return true - } - return false -} diff --git a/pkg/util/iptables/iptables_test.go b/pkg/util/iptables/iptables_test.go index 8327481469..b8a0e45a78 100644 --- a/pkg/util/iptables/iptables_test.go +++ b/pkg/util/iptables/iptables_test.go @@ -438,65 +438,6 @@ func TestGetIptablesHasCheckCommand(t *testing.T) { } } -func TestExtractIptablesVersion(t *testing.T) { - testCases := []struct { - Version string - V1 int - V2 int - V3 int - Err bool - }{ - {"iptables v1.4.7", 1, 4, 7, false}, - {"iptables v1.4.11", 1, 4, 11, false}, - {"iptables v0.2.5", 0, 2, 5, false}, - {"iptables v1.2.3.4.5.6", 1, 2, 3, false}, - {"iptables v1.4", 0, 0, 0, true}, - {"iptables v12345.12345.12345.12344", 12345, 12345, 12345, false}, - {"total junk", 0, 0, 0, true}, - } - - for _, testCase := range testCases { - v1, v2, v3, err := extractIptablesVersion(testCase.Version) - if (err != nil) != testCase.Err { - t.Errorf("Expected error: %v, Got error: %v", testCase.Err, err) - } - if err == nil { - if v1 != testCase.V1 { - t.Errorf("First version number incorrect for string \"%s\", got %d, expected %d", testCase.Version, v1, testCase.V1) - } - if v2 != testCase.V2 { - t.Errorf("Second version number incorrect for string \"%s\", got %d, expected %d", testCase.Version, v2, testCase.V2) - } - if v3 != testCase.V3 { - t.Errorf("Third version number incorrect for string \"%s\", got %d, expected %d", testCase.Version, v3, testCase.V3) - } - } - } -} - -func TestIptablesHasCheckCommand(t *testing.T) { - testCases := []struct { - V1 int - V2 int - V3 int - Result bool - }{ - {0, 55, 55, false}, - {1, 0, 55, false}, - {1, 4, 10, false}, - {1, 4, 11, true}, - {1, 4, 19, true}, - {1, 5, 0, true}, - {2, 0, 0, true}, - } - - for _, testCase := range testCases { - if result := iptablesHasCheckCommand(testCase.V1, testCase.V2, testCase.V3); result != testCase.Result { - t.Errorf("For %d.%d.%d expected %v got %v", testCase.V1, testCase.V2, testCase.V3, testCase.Result, result) - } - } -} - func TestCheckRuleWithoutCheckPresent(t *testing.T) { iptables_save_output := `# Generated by iptables-save v1.4.7 on Wed Oct 29 14:56:01 2014 *nat