From 387011cfd9aec80150504559572b2cf42feaa163 Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Thu, 6 Feb 2020 18:22:29 +0000 Subject: [PATCH 1/3] The value of minor was incorrectly assumed to be e.g. 14.8-hotfix.20191113 instead of 14+ --- npm/util/util.go | 12 ++++++++++-- npm/util/util_test.go | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/npm/util/util.go b/npm/util/util.go index 4c42ca8140..dbf34f333d 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -6,6 +6,7 @@ import ( "fmt" "hash/fnv" "os" + "regexp" "strings" "sort" @@ -16,6 +17,9 @@ import ( // IsNewNwPolicyVerFlag indicates if the current kubernetes version is newer than 1.11 or not var IsNewNwPolicyVerFlag = false +// regex to get minor version +var re = regexp.MustCompile("[0-9]+") + // Exists reports whether the named file or directory exists. func Exists(filePath string) bool { if _, err := os.Stat(filePath); err == nil { @@ -98,11 +102,15 @@ func GetHashedName(name string) string { // returns -1, 0, 1 if firstVer smaller, equals, bigger than secondVer respectively. // returns -2 for error. func CompareK8sVer(firstVer *version.Info, secondVer *version.Info) int { - v1, err := semver.NewVersion(firstVer.Major+firstVer.Minor) + minorVers := re.FindAllString(firstVer.Minor, -1) + if len(minorVers) < 1 { + return -2 + } + v1, err := semver.NewVersion(firstVer.Major+"."+minorVers[0]) if err != nil { return -2 } - v2, err := semver.NewVersion(secondVer.Major+secondVer.Minor) + v2, err := semver.NewVersion(secondVer.Major+"."+secondVer.Minor) if err != nil { return -2 } diff --git a/npm/util/util_test.go b/npm/util/util_test.go index a4aa61d076..ad22484b8a 100644 --- a/npm/util/util_test.go +++ b/npm/util/util_test.go @@ -111,6 +111,20 @@ func TestCompareK8sVer(t *testing.T) { if res := CompareK8sVer(firstVer, secondVer); res != 1 { t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ hotfix tag/pre-release") } + + firstVer = &version.Info{ + Major: "1", + Minor: "14+", + } + + secondVer = &version.Info{ + Major: "1", + Minor: "11", + } + + if res := CompareK8sVer(firstVer, secondVer); res != 1 { + t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ minor+ release") + } } func TestIsNewNwPolicyVer(t *testing.T) { From 10386cf6db3a0cce9d357a11eb2640ab58d16637 Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Thu, 6 Feb 2020 19:14:36 +0000 Subject: [PATCH 2/3] adding Jonathan Chauncey's test --- npm/util/util_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/npm/util/util_test.go b/npm/util/util_test.go index ad22484b8a..d0c2274c84 100644 --- a/npm/util/util_test.go +++ b/npm/util/util_test.go @@ -125,6 +125,20 @@ func TestCompareK8sVer(t *testing.T) { if res := CompareK8sVer(firstVer, secondVer); res != 1 { t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ minor+ release") } + + firstVer = &version.Info{ + Major: "2", + Minor: "1", + } + + secondVer = &version.Info{ + Major: "1", + Minor: "11", + } + + if res := CompareK8sVer(firstVer, secondVer); res != 1 { + t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ major version upgrade") + } } func TestIsNewNwPolicyVer(t *testing.T) { From 15318ccb945c614468f1d4b13070b499bcd7f5cc Mon Sep 17 00:00:00 2001 From: Jaeryn Date: Thu, 6 Feb 2020 20:01:39 +0000 Subject: [PATCH 3/3] addressing Robbie's comments --- npm/util/util.go | 12 ++++++++---- npm/util/util_test.go | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/npm/util/util.go b/npm/util/util.go index dbf34f333d..eb77b951ae 100644 --- a/npm/util/util.go +++ b/npm/util/util.go @@ -102,15 +102,19 @@ func GetHashedName(name string) string { // returns -1, 0, 1 if firstVer smaller, equals, bigger than secondVer respectively. // returns -2 for error. func CompareK8sVer(firstVer *version.Info, secondVer *version.Info) int { - minorVers := re.FindAllString(firstVer.Minor, -1) - if len(minorVers) < 1 { + v1Minor := re.FindAllString(firstVer.Minor, -1) + if len(v1Minor) < 1 { return -2 } - v1, err := semver.NewVersion(firstVer.Major+"."+minorVers[0]) + v1, err := semver.NewVersion(firstVer.Major+"."+v1Minor[0]) if err != nil { return -2 } - v2, err := semver.NewVersion(secondVer.Major+"."+secondVer.Minor) + v2Minor := re.FindAllString(secondVer.Minor, -1) + if len(v2Minor) < 1 { + return -2 + } + v2, err := semver.NewVersion(secondVer.Major+"."+v2Minor[0]) if err != nil { return -2 } diff --git a/npm/util/util_test.go b/npm/util/util_test.go index d0c2274c84..f388ee030e 100644 --- a/npm/util/util_test.go +++ b/npm/util/util_test.go @@ -139,6 +139,20 @@ func TestCompareK8sVer(t *testing.T) { if res := CompareK8sVer(firstVer, secondVer); res != 1 { t.Errorf("TestCompareK8sVer failed @ firstVer > secondVer w/ major version upgrade") } + + firstVer = &version.Info{ + Major: "1", + Minor: "11", + } + + secondVer = &version.Info{ + Major: "2", + Minor: "1", + } + + if res := CompareK8sVer(firstVer, secondVer); res != -1 { + t.Errorf("TestCompareK8sVer failed @ firstVer < secondVer w/ major version upgrade") + } } func TestIsNewNwPolicyVer(t *testing.T) {