Skip to content

Commit 8f8346f

Browse files
kartik-579pghildiyal
kartik-579
and
pghildiyal
authored
Deployment Template validation bug fix (#868)
* fixed template validation bug * wip * review changes * review changes * Added test cases for cpu and memory and fixed bug * review changes Co-authored-by: Prashant Ghildiyal <[email protected]>
1 parent 9a5098f commit 8f8346f

File tree

3 files changed

+213
-124
lines changed

3 files changed

+213
-124
lines changed

‎util/ValidatorHelper.go‎

Lines changed: 59 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,34 @@ type resourceParser struct {
2020
var memoryParser *resourceParser
2121
var cpuParser *resourceParser
2222

23+
func getResourcesLimitsKeys(envoyProxy bool) []string {
24+
if envoyProxy {
25+
return []string{"envoyproxy", "resources", "limits"}
26+
} else {
27+
return []string{"resources", "limits"}
28+
}
29+
}
30+
func getResourcesRequestsKeys(envoyProxy bool) []string {
31+
if envoyProxy {
32+
return []string{"envoyproxy", "resources", "requests"}
33+
} else {
34+
return []string{"resources", "requests"}
35+
}
36+
}
37+
38+
func validateAndBuildResourcesAssignment(dat map[string]interface{}, validationKeys []string) (validatedMap map[string]interface{}) {
39+
var test map[string]interface{}
40+
test = dat
41+
for _, validationKey := range validationKeys {
42+
if test[validationKey] != nil {
43+
test = test[validationKey].(map[string]interface{})
44+
} else {
45+
return map[string]interface{}{"cpu": "0", "memory": "0"}
46+
}
47+
}
48+
return test
49+
}
50+
2351
func MemoryToNumber(memory string) (float64, error) {
2452
if memoryParser == nil {
2553
pattern := "(\\d*e?\\d*)(Ei?|Pi?|Ti?|Gi?|Mi?|Ki?|$)"
@@ -119,14 +147,11 @@ func ParseFloat(str string) (float64, error) {
119147
}
120148

121149
func CompareLimitsRequests(dat map[string]interface{}) (bool, error) {
122-
limit, ok := dat["resources"].(map[string]interface{})["limits"].(map[string]interface{})
123-
if !ok {
124-
return false, errors.New("resources.limits is required")
125-
}
126-
envoproxyLimit, ok := dat["envoyproxy"].(map[string]interface{})["resources"].(map[string]interface{})["limits"].(map[string]interface{})
127-
if !ok {
128-
return false, errors.New("envoproxy.resources.limits is required")
150+
if dat == nil {
151+
return true, nil
129152
}
153+
limit := validateAndBuildResourcesAssignment(dat, getResourcesLimitsKeys(false))
154+
envoproxyLimit := validateAndBuildResourcesAssignment(dat, getResourcesLimitsKeys(true))
130155
checkCPUlimit, ok := limit["cpu"]
131156
if !ok {
132157
return false, errors.New("resources.limits.cpu is required")
@@ -143,14 +168,8 @@ func CompareLimitsRequests(dat map[string]interface{}) (bool, error) {
143168
if !ok {
144169
return false, errors.New("envoyproxy.resources.limits.memory is required")
145170
}
146-
request, ok := dat["resources"].(map[string]interface{})["requests"].(map[string]interface{})
147-
if !ok {
148-
return true, nil
149-
}
150-
envoproxyRequest, ok := dat["envoyproxy"].(map[string]interface{})["resources"].(map[string]interface{})["requests"].(map[string]interface{})
151-
if !ok {
152-
return true, nil
153-
}
171+
request := validateAndBuildResourcesAssignment(dat, getResourcesRequestsKeys(false))
172+
envoproxyRequest := validateAndBuildResourcesAssignment(dat, getResourcesRequestsKeys(true))
154173
checkCPURequests, ok := request["cpu"]
155174
if !ok {
156175
return true, nil
@@ -201,125 +220,43 @@ func CompareLimitsRequests(dat map[string]interface{}) (bool, error) {
201220
if err != nil {
202221
return false, err
203222
}
204-
205-
if (envoproxyCPULimit < envoproxyCPURequest){
223+
if envoproxyCPULimit < envoproxyCPURequest && envoproxyCPULimit != 0 {
206224
return false, errors.New("envoyproxy.resources.limits.cpu must be greater than or equal to envoyproxy.resources.requests.cpu")
207-
}else if (envoproxyMemoryLimit < envoproxyMemoryRequest){
225+
} else if envoproxyMemoryLimit < envoproxyMemoryRequest && envoproxyMemoryLimit != 0 {
208226
return false, errors.New("envoyproxy.resources.limits.memory must be greater than or equal to envoyproxy.resources.requests.memory")
209-
}else if (cpuLimit < cpuRequest) {
227+
} else if cpuLimit < cpuRequest && cpuLimit != 0 {
210228
return false, errors.New("resources.limits.cpu must be greater than or equal to resources.requests.cpu")
211-
}else if (memoryLimit < memoryRequest) {
229+
} else if memoryLimit < memoryRequest && memoryLimit != 0 {
212230
return false, errors.New("resources.limits.memory must be greater than or equal to resources.requests.memory")
213231
}
214232
return true, nil
215233

216234
}
217235

218236
func AutoScale(dat map[string]interface{}) (bool, error) {
219-
autoscaleEnabled, ok := dat["autoscaling"].(map[string]interface{})["enabled"]
220-
if !ok {
237+
if dat == nil {
221238
return true, nil
222239
}
223-
if autoscaleEnabled.(bool) {
224-
limit, ok := dat["resources"].(map[string]interface{})["limits"].(map[string]interface{})
225-
if !ok {
226-
return false, errors.New("resources.limits is required")
227-
}
228-
envoproxyLimit, ok := dat["envoyproxy"].(map[string]interface{})["resources"].(map[string]interface{})["limits"].(map[string]interface{})
229-
if !ok {
230-
return false, errors.New("envoproxy.resources.limits is required")
231-
}
232-
checkCPUlimit, ok := limit["cpu"]
233-
if !ok {
234-
return false, errors.New("resources.limits.cpu is required")
235-
}
236-
checkMemorylimit, ok := limit["memory"]
237-
if !ok {
238-
return false, errors.New("resources.limits.memory is required")
239-
}
240-
checkEnvoproxyCPUlimit, ok := envoproxyLimit["cpu"]
241-
if !ok {
242-
return false, errors.New("envoyproxy.resources.limits.cpu is required")
243-
}
244-
checkEnvoproxyMemorylimit, ok := envoproxyLimit["memory"]
240+
if dat["autoscaling"]!=nil {
241+
autoScaleEnabled, ok := dat["autoscaling"].(map[string]interface{})["enabled"]
245242
if !ok {
246-
return false, errors.New("envoyproxy.resources.limits.memory is required")
247-
}
248-
request, ok := dat["resources"].(map[string]interface{})["requests"].(map[string]interface{})
249-
if !ok {
250-
return false, errors.New("resources.requests is required")
251-
}
252-
envoproxyRequest, ok := dat["envoyproxy"].(map[string]interface{})["resources"].(map[string]interface{})["requests"].(map[string]interface{})
253-
if !ok {
254-
return false, errors.New("envoyproxy.resources.requests is required")
255-
}
256-
checkCPURequests, ok := request["cpu"]
257-
if !ok {
258-
return false, errors.New("resources.requests.cpu is required")
259-
}
260-
checkMemoryRequests, ok := request["memory"]
261-
if !ok {
262-
return false, errors.New("resources.requests.memory is required")
263-
}
264-
checkEnvoproxyCPURequests, ok := envoproxyRequest["cpu"]
265-
if !ok {
266-
return false, errors.New("envoyproxy.resources.requests.cpu is required")
267-
}
268-
checkEnvoproxyMemoryRequests, ok := envoproxyRequest["memory"]
269-
if !ok {
270-
return false, errors.New("envoyproxy.resources.requests.memory is required")
271-
}
272-
273-
cpuLimit, err := CpuToNumber(checkCPUlimit.(string))
274-
if err != nil {
275-
return false, err
276-
}
277-
memoryLimit, err := MemoryToNumber(checkMemorylimit.(string))
278-
if err != nil {
279-
return false, err
280-
}
281-
cpuRequest, err := CpuToNumber(checkCPURequests.(string))
282-
if err != nil {
283-
return false, err
284-
}
285-
memoryRequest, err := MemoryToNumber(checkMemoryRequests.(string))
286-
if err != nil {
287-
return false, err
288-
}
289-
290-
envoproxyCPULimit, err := CpuToNumber(checkEnvoproxyCPUlimit.(string))
291-
if err != nil {
292-
return false, err
293-
}
294-
envoproxyMemoryLimit, err := MemoryToNumber(checkEnvoproxyMemorylimit.(string))
295-
if err != nil {
296-
return false, err
297-
}
298-
envoproxyCPURequest, err := CpuToNumber(checkEnvoproxyCPURequests.(string))
299-
if err != nil {
300-
return false, err
301-
}
302-
envoproxyMemoryRequest, err := MemoryToNumber(checkEnvoproxyMemoryRequests.(string))
303-
if err != nil {
304-
return false, err
305-
}
306-
307-
if (envoproxyCPULimit < envoproxyCPURequest){
308-
return false, errors.New("envoyproxy.resources.limits.cpu must be greater than or equal to envoyproxy.resources.requests.cpu")
309-
}else if (envoproxyMemoryLimit < envoproxyMemoryRequest){
310-
return false, errors.New("envoyproxy.resources.limits.memory must be greater than or equal to envoyproxy.resources.requests.memory")
311-
}else if (cpuLimit < cpuRequest) {
312-
return false, errors.New("resources.limits.cpu must be greater than or equal to resources.requests.cpu")
313-
}else if (memoryLimit < memoryRequest) {
314-
return false, errors.New("resources.limits.memory must be greater than or equal to resources.requests.memory")
315-
}else {
316243
return true, nil
317244
}
318-
} else {
319-
return true, nil
245+
if autoScaleEnabled.(bool) {
246+
minReplicas, okMin := dat["autoscaling"].(map[string]interface{})["MinReplicas"]
247+
maxReplicas, okMax := dat["autoscaling"].(map[string]interface{})["MaxReplicas"]
248+
if !okMin || !okMax{
249+
return false, errors.New("autoscaling.MinReplicas and autoscaling.MaxReplicas are mandatory fields")
250+
}
251+
if minReplicas.(int) > maxReplicas.(int){
252+
return false, errors.New("autoscaling.MinReplicas can not be greater than autoscaling.MaxReplicas")
253+
}
254+
}
320255
}
256+
return true,nil
321257
}
322258

259+
323260
var (
324261
CpuUnitChecker, _ = regexp.Compile("^([0-9.]+)m$")
325262
NoCpuUnitChecker, _ = regexp.Compile("^([0-9.]+)$")
@@ -331,6 +268,9 @@ var (
331268
)
332269

333270
func (f CpuChecker) IsFormat(input interface{}) bool {
271+
if input == nil {
272+
return false
273+
}
334274
asString, ok := input.(string)
335275
if !ok {
336276
return false
@@ -346,6 +286,9 @@ func (f CpuChecker) IsFormat(input interface{}) bool {
346286
}
347287

348288
func (f MemoryChecker) IsFormat(input interface{}) bool {
289+
if input == nil {
290+
return false
291+
}
349292
asString, ok := input.(string)
350293
if !ok {
351294
return false

‎util/ValidatorHelper_test.go‎

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
package util
2+
3+
import "testing"
4+
5+
func TestCompareLimitsRequests(t *testing.T) {
6+
requests := "requests"
7+
limits := "limits"
8+
resources := "resources"
9+
cpu := "cpu"
10+
memory := "memory"
11+
type args struct {
12+
dat map[string]interface{}
13+
}
14+
tests := []struct {
15+
name string
16+
args args
17+
want bool
18+
wantErr bool
19+
}{
20+
{
21+
name: "empty base object",
22+
args: args{dat: nil},
23+
want: true,
24+
wantErr: false,
25+
},
26+
{
27+
name: "empty resources object",
28+
args: args{dat: map[string]interface{}{resources: nil}},
29+
want: true,
30+
wantErr: false,
31+
},
32+
{
33+
name: "empty resources requests limits object",
34+
args: args{dat: map[string]interface{}{resources: map[string]interface{}{limits: nil, requests: nil}}},
35+
want: true,
36+
wantErr: false,
37+
},
38+
{
39+
name: "non-empty resources limits object",
40+
args: args{dat: map[string]interface{}{resources: map[string]interface{}{limits: map[string]interface{}{cpu: "10Gi", memory: ".1" }, requests: nil}}},
41+
want: true,
42+
wantErr: false,
43+
},
44+
{
45+
name: "non-empty resources requests object",
46+
args: args{dat: map[string]interface{}{resources: map[string]interface{}{limits: nil, requests: map[string]interface{}{cpu: "10Gi", memory: ".1" }}}},
47+
want: true,
48+
wantErr: false,
49+
},
50+
{
51+
name: "non-empty and equal resources limits and requests object",
52+
args: args{dat: map[string]interface{}{resources: map[string]interface{}{limits: map[string]interface{}{cpu: "10Gi", memory: ".1" }, requests: map[string]interface{}{cpu: "10Gi", memory: ".1" }}}},
53+
want: true,
54+
wantErr: false,
55+
},
56+
{
57+
name: "negative: non-empty and not equal resources limits and requests object",
58+
args: args{dat: map[string]interface{}{resources: map[string]interface{}{limits: map[string]interface{}{cpu: "10Gi", memory: ".1" }, requests: map[string]interface{}{cpu: "11Gi", memory: ".15" }}}},
59+
want: false,
60+
wantErr: true,
61+
},
62+
}
63+
for _, tt := range tests {
64+
t.Run(tt.name, func(t *testing.T) {
65+
got, err := CompareLimitsRequests(tt.args.dat)
66+
if (err != nil) != tt.wantErr {
67+
t.Errorf("CompareLimitsRequests() error = %v, wantErr %v", err, tt.wantErr)
68+
return
69+
}
70+
if got != tt.want {
71+
t.Errorf("CompareLimitsRequests() got = %v, want %v", got, tt.want)
72+
}
73+
})
74+
}
75+
}
76+
77+
func TestAutoScale(t *testing.T) {
78+
autoScaling := "autoscaling"
79+
minReplicas := "MinReplicas"
80+
maxReplicas := "MaxReplicas"
81+
enabled := "enabled"
82+
type args struct {
83+
dat map[string]interface{}
84+
}
85+
tests := []struct {
86+
name string
87+
args args
88+
want bool
89+
wantErr bool
90+
}{
91+
{
92+
name: "empty base object",
93+
args: args{dat: nil},
94+
want: true,
95+
wantErr: false,
96+
},
97+
{
98+
name: "empty autoscaling object",
99+
args: args{dat: map[string]interface{}{autoScaling: nil}},
100+
want: true,
101+
wantErr: false,
102+
},
103+
{
104+
name: "negative : non-empty autoscaling empty enabled minReplicas maxReplicas object",
105+
args: args{dat: map[string]interface{}{autoScaling: map[string]interface{}{}}},
106+
want: true,
107+
wantErr: false,
108+
},
109+
{
110+
name: "non-empty autoscaling enabled minReplicas maxReplicas object",
111+
args: args{dat: map[string]interface{}{autoScaling: map[string]interface{}{enabled: false,minReplicas: 10, maxReplicas:11}}},
112+
want: true,
113+
wantErr: false,
114+
},
115+
{
116+
name: "non-empty autoscaling enabled minReplicas empty maxReplicas object",
117+
args: args{dat: map[string]interface{}{autoScaling: map[string]interface{}{enabled: false,minReplicas: 11}}},
118+
want: true,
119+
wantErr: false,
120+
},
121+
{
122+
name: "non-empty autoscaling minReplicas maxReplicas object empty enabled",
123+
args: args{dat: map[string]interface{}{autoScaling: map[string]interface{}{minReplicas: 10, maxReplicas:11}}},
124+
want: true,
125+
wantErr: false,
126+
},
127+
{
128+
name: "negative: non-empty and greater minReplicas than maxReplicas object",
129+
args: args{dat: map[string]interface{}{autoScaling: map[string]interface{}{enabled: true, minReplicas: 10, maxReplicas:9}}},
130+
want: false,
131+
wantErr: true,
132+
},
133+
}
134+
for _, tt := range tests {
135+
t.Run(tt.name, func(t *testing.T) {
136+
got, err := AutoScale(tt.args.dat)
137+
if (err != nil) != tt.wantErr {
138+
t.Errorf("AutoScale() error = %v, wantErr %v", err, tt.wantErr)
139+
return
140+
}
141+
if got != tt.want {
142+
t.Errorf("AutoScale() got = %v, want %v", got, tt.want)
143+
}
144+
})
145+
}
146+
}

0 commit comments

Comments
 (0)