Skip to content

Commit 07169cf

Browse files
furykerryfurykerry
authored
fix(client): avoid panic in ShouldUpdateResourceByResize on GKE/EKS version strings (#2502)
coreos/go-semver.New panics on strings that are not in dotted-tri format. The K8s server version returned by /version is reported by some cloud providers with a trailing '+' or build metadata (e.g. v1.28+gke.1, v1.30+eks.1), which would previously crash the controller on startup. Changes: - Strip the leading 'v' prefix from curVersion.GitVersion. - Clean each dotted component with a new digitsOnly helper so cloud build metadata is dropped before parsing (1.32+gke.1 -> 1.32.1). - Add a deferred recover() to return false on any remaining format exception, preserving the pre-1.32 update path as a safe default. - Add unit tests covering exact, above, below, GKE/EKS-cleaned, empty, garbage, and missing-segment inputs.
1 parent 2ba7196 commit 07169cf

2 files changed

Lines changed: 173 additions & 2 deletions

File tree

‎pkg/client/registry.go‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package client
22

33
import (
44
"fmt"
5+
"strings"
56

67
"github.com/coreos/go-semver/semver"
78
"k8s.io/apimachinery/pkg/version"
@@ -54,6 +55,42 @@ func GetCurrentServerVersion() *version.Info {
5455

5556
// ShouldUpdateResourceByResize returns whether should update resource by resize
5657
// The resize sub-resource was introduced in version 1.32, https://github.com/kubernetes/kubernetes/pull/128266
57-
func ShouldUpdateResourceByResize() bool {
58-
return semver.New(fmt.Sprintf("%s.%s.0", curVersion.Major, curVersion.Minor)).Compare(*semver.New("1.32.0")) >= 0
58+
func ShouldUpdateResourceByResize() (result bool) {
59+
defer func() {
60+
if r := recover(); r != nil {
61+
// semver.New panics on strings that are not in dotted-tri format
62+
// (e.g. unexpected non-numeric content from custom build metadata).
63+
// Treat that as "not >= 1.32" so the caller falls back to the
64+
// pre-1.32 update path.
65+
_ = r
66+
result = false
67+
}
68+
}()
69+
// coreos/go-semver does not strip the leading 'v' that k8s prefixes on
70+
// GitVersion (e.g. "v1.32.0"). Remove it before parsing.
71+
v := strings.TrimPrefix(curVersion.GitVersion, "v")
72+
// digitsOnly cleans each dotted component so cloud-provider build metadata
73+
// (e.g. "1.32+gke.1" → "1.32.1") does not cause a parse panic. SplitN
74+
// keeps at most three parts (major/minor/patch); any extra segments are
75+
// folded into the last part and then trimmed by digitsOnly. Any
76+
// remaining format exception is caught by the deferred recover above.
77+
parts := strings.SplitN(v, ".", 3)
78+
for i, p := range parts {
79+
parts[i] = digitsOnly(p)
80+
}
81+
cur := semver.New(strings.Join(parts, "."))
82+
return cur.Compare(*semver.New("1.32.0")) >= 0
83+
}
84+
85+
// digitsOnly returns the leading numeric prefix of a version component.
86+
// This handles GKE/EKS-style build metadata embedded in a component
87+
// (e.g. "32+gke" → "32"). If the input has no leading digit, an empty
88+
// string is returned.
89+
func digitsOnly(s string) string {
90+
for i, r := range s {
91+
if r < '0' || r > '9' {
92+
return s[:i]
93+
}
94+
}
95+
return s
5996
}

‎pkg/client/registry_test.go‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
/*
2+
Copyright 2026 The Kruise Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package client
18+
19+
import (
20+
"testing"
21+
22+
"k8s.io/apimachinery/pkg/version"
23+
)
24+
25+
func TestDigitsOnly(t *testing.T) {
26+
cases := []struct {
27+
name string
28+
in string
29+
want string
30+
}{
31+
{name: "empty string", in: "", want: ""},
32+
{name: "plain digits", in: "28", want: "28"},
33+
{name: "gke build metadata suffix", in: "32+gke", want: "32"},
34+
{name: "eks build metadata suffix", in: "30+eks", want: "30"},
35+
{name: "trailing plus", in: "28+", want: "28"},
36+
{name: "major with plus", in: "1+", want: "1"},
37+
{name: "all non-numeric", in: "+++", want: ""},
38+
{name: "letters only", in: "abc", want: ""},
39+
{name: "leading digits then letters", in: "1a2b3c", want: "1"},
40+
{name: "zero", in: "0", want: "0"},
41+
{name: "large number", in: "132", want: "132"},
42+
}
43+
for _, tc := range cases {
44+
t.Run(tc.name, func(t *testing.T) {
45+
if got := digitsOnly(tc.in); got != tc.want {
46+
t.Errorf("digitsOnly(%q) = %q, want %q", tc.in, got, tc.want)
47+
}
48+
})
49+
}
50+
}
51+
52+
func TestShouldUpdateResourceByResize(t *testing.T) {
53+
original := curVersion
54+
t.Cleanup(func() {
55+
curVersion = original
56+
})
57+
58+
cases := []struct {
59+
name string
60+
version *version.Info
61+
want bool
62+
}{
63+
{
64+
name: "exactly 1.32.0 returns true",
65+
version: &version.Info{Major: "1", Minor: "32", GitVersion: "v1.32.0"},
66+
want: true,
67+
},
68+
{
69+
name: "above 1.32 returns true",
70+
version: &version.Info{Major: "1", Minor: "33", GitVersion: "v1.33.0"},
71+
want: true,
72+
},
73+
{
74+
name: "below 1.32 returns false",
75+
version: &version.Info{Major: "1", Minor: "31", GitVersion: "v1.31.0"},
76+
want: false,
77+
},
78+
{
79+
name: "gke-style 1.28+gke.1 is cleaned to 1.28.1 and returns false",
80+
version: &version.Info{Major: "1", Minor: "28+", GitVersion: "v1.28+gke.1"},
81+
want: false,
82+
},
83+
{
84+
name: "gke-style 1.32+gke.1 is cleaned to 1.32.1 and returns true",
85+
version: &version.Info{Major: "1", Minor: "32+", GitVersion: "v1.32+gke.1"},
86+
want: true,
87+
},
88+
{
89+
name: "eks-style 1.30+eks.1 is cleaned to 1.30.1 and returns false",
90+
version: &version.Info{Major: "1", Minor: "30+", GitVersion: "v1.30+eks.1"},
91+
want: false,
92+
},
93+
{
94+
name: "1.28+.1 is cleaned to 1.28.1 and returns false",
95+
version: &version.Info{Major: "1", Minor: "28+", GitVersion: "v1.28+.1"},
96+
want: false,
97+
},
98+
{
99+
name: "extra segment after patch is folded into patch and cleaned to 1.32.0",
100+
version: &version.Info{Major: "1", Minor: "32", GitVersion: "v1.32.0.extra"},
101+
want: true,
102+
},
103+
{
104+
name: "empty GitVersion recovers from panic and returns false",
105+
version: &version.Info{Major: "1", Minor: ""},
106+
want: false,
107+
},
108+
{
109+
name: "non-semver garbage recovers from panic and returns false",
110+
version: &version.Info{Major: "1", Minor: "32", GitVersion: "garbage"},
111+
want: false,
112+
},
113+
{
114+
name: "missing patch segment recovers from panic and returns false",
115+
version: &version.Info{Major: "1", Minor: "32", GitVersion: "1.32"},
116+
want: false,
117+
},
118+
}
119+
for _, tc := range cases {
120+
t.Run(tc.name, func(t *testing.T) {
121+
curVersion = tc.version
122+
defer func() {
123+
if r := recover(); r != nil {
124+
t.Fatalf("ShouldUpdateResourceByResize leaked a panic for GitVersion=%q: %v",
125+
tc.version.GitVersion, r)
126+
}
127+
}()
128+
if got := ShouldUpdateResourceByResize(); got != tc.want {
129+
t.Errorf("ShouldUpdateResourceByResize() for GitVersion=%q = %v, want %v",
130+
tc.version.GitVersion, got, tc.want)
131+
}
132+
})
133+
}
134+
}

0 commit comments

Comments
 (0)