Make iptables use semver lib

lavalamp · Aug 18, 2015 · 3d30970 · 3d30970
1 parent 3a5c23d
commit 3d30970
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 103 deletions.
diff --git 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) {
-	vstring, err := GetIptablesVersionString(exec)
+	minVersion, err := semver.NewVersion(MinCheckVersion)
 	if err != nil {
 		return false, err
 	}
-
-	v1, v2, v3, err := extractIptablesVersion(vstring)
+	// Returns "vX.Y.Z".
+	vstring, err := GetIptablesVersionString(exec)
 	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)
-	}
-
-	v1, err := strconv.Atoi(result[1])
+	// Make a semver of the part after the v in "vX.X.X".
+	version, err := semver.NewVersion(vstring[1:])
 	if err != nil {
-		return 0, 0, 0, err
-	}
-
-	v2, err := strconv.Atoi(result[2])
-	if err != nil {
-		return 0, 0, 0, err
+		return false, err
 	}
-
-	v3, err := strconv.Atoi(result[3])
-	if err != nil {
-		return 0, 0, 0, err
+	if version.LessThan(*minVersion) {
+		return false, nil
 	}
-
-	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
@@ -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