Skip to content

Commit a0b29e6

Browse files
authored
Merge pull request #1211 from viccuad/fix-timeouts
feat: Check timeouts for mininum value
2 parents b55af9f + 980ffcd commit a0b29e6

10 files changed

+119
-74
lines changed

api/policies/v1/policy_types.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -148,17 +148,21 @@ type PolicySpec struct {
148148
// TimeoutSeconds specifies the timeout for the policy webhook. After the timeout passes,
149149
// the webhook call will be ignored or the API call will fail based on the
150150
// failure policy.
151-
// The timeout value must be between 1 and 30 seconds.
151+
// The timeout value must be between 2 and 30 seconds.
152152
// Default to 10 seconds.
153153
// +optional
154154
// +kubebuilder:default:=10
155+
// +kubebuilder:validation:Minimum=2
156+
// +kubebuilder:validation:Maximum=30
155157
TimeoutSeconds *int32 `json:"timeoutSeconds,omitempty"`
156158

157159
// TimeoutEvalSeconds specifies the timeout for the policy evaluation. After
158160
// the timeout passes, the policy evaluation call will fail based on the
159161
// failure policy.
160-
// The timeout value must be between 1 and 30 seconds.
162+
// The timeout value must be between 2 and 30 seconds.
161163
// +optional
164+
// +kubebuilder:validation:Minimum=2
165+
// +kubebuilder:validation:Maximum=30
162166
TimeoutEvalSeconds *int32 `json:"timeoutEvalSeconds,omitempty"`
163167

164168
// Message overrides the rejection message of the policy.
@@ -192,8 +196,10 @@ type PolicyGroupMember struct {
192196
// TimeoutEvalSeconds specifies the timeout for the policy evaluation. After
193197
// the timeout passes, the policy evaluation call will fail based on the
194198
// failure policy.
195-
// The timeout value must be between 1 and 30 seconds.
199+
// The timeout value must be between 2 and 30 seconds.
196200
// +optional
201+
// +kubebuilder:validation:Minimum=2
202+
// +kubebuilder:validation:Maximum=30
197203
TimeoutEvalSeconds *int32 `json:"timeoutEvalSeconds,omitempty"`
198204
}
199205

@@ -308,10 +314,12 @@ type GroupSpec struct {
308314
// TimeoutSeconds specifies the timeout for this webhook. After the timeout passes,
309315
// the webhook call will be ignored or the API call will fail based on the
310316
// failure policy.
311-
// The timeout value must be between 1 and 30 seconds.
317+
// The timeout value must be between 2 and 30 seconds.
312318
// Default to 10 seconds.
313319
// +optional
314320
// +kubebuilder:default:=10
321+
// +kubebuilder:validation:Minimum=2
322+
// +kubebuilder:validation:Maximum=30
315323
TimeoutSeconds *int32 `json:"timeoutSeconds,omitempty"`
316324

317325
// Expression is the evaluation expression to accept or reject the

api/policies/v1/policy_validation.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -309,30 +309,13 @@ func validateMatchConditionsExpression(expressionStr string, fldPath *field.Path
309309
}
310310

311311
// validateTimeoutSeconds checks the timeouts so that:
312-
// - the policy's timeoutSeconds is not greater than the Kubernetes webhook max timeout (30s).
313-
// - the policy's timeoutEvalSeconds is not greater than the Kubernetes webhook max timeout (30s).
314312
// - the policy's timeoutEvalSeconds is not greater than the policy's timeoutSeconds.
315313
func validateTimeoutSeconds(policy Policy) field.ErrorList {
316314
var allErrors field.ErrorList
317315
timeoutSeconds := policy.GetTimeoutSeconds()
318316
timeoutEvalSeconds := policy.GetTimeoutEvalSeconds()
319-
fldTimeoutSeconds := field.NewPath("spec").Child("timeoutSeconds")
320317
fldTimeoutEvalSeconds := field.NewPath("spec").Child("timeoutEvalSeconds")
321318

322-
if timeoutSeconds != nil && *timeoutSeconds > maxWebhookTimeoutSeconds {
323-
allErrors = append(allErrors, field.Invalid(
324-
fldTimeoutSeconds,
325-
*timeoutSeconds,
326-
fmt.Sprintf("timeoutSeconds cannot be greater than %d (Kubernetes webhook max timeout)", maxWebhookTimeoutSeconds),
327-
))
328-
}
329-
if timeoutEvalSeconds != nil && *timeoutEvalSeconds > maxWebhookTimeoutSeconds {
330-
allErrors = append(allErrors, field.Invalid(
331-
fldTimeoutEvalSeconds,
332-
*timeoutEvalSeconds,
333-
fmt.Sprintf("timeoutEvalSeconds cannot be greater than %d (Kubernetes webhook max timeout)", maxWebhookTimeoutSeconds),
334-
))
335-
}
336319
if timeoutSeconds != nil && timeoutEvalSeconds != nil {
337320
if *timeoutEvalSeconds > *timeoutSeconds {
338321
allErrors = append(allErrors, field.Invalid(

api/policies/v1/policy_validation_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,6 @@ func TestValidatePolicyModeField(t *testing.T) {
600600

601601
func TestValidateTimeoutSeconds(t *testing.T) {
602602
maxTimeout := int32(30)
603-
overTimeout := int32(31)
604603
underTimeout := int32(10)
605604

606605
type testCase struct {
@@ -617,18 +616,6 @@ func TestValidateTimeoutSeconds(t *testing.T) {
617616
timeoutEvalSeconds: nil,
618617
expectedErrors: nil,
619618
},
620-
{
621-
name: "timeoutSeconds over max",
622-
timeoutSeconds: &overTimeout,
623-
timeoutEvalSeconds: nil,
624-
expectedErrors: []string{"timeoutSeconds cannot be greater than"},
625-
},
626-
{
627-
name: "timeoutEvalSeconds over max",
628-
timeoutSeconds: &maxTimeout,
629-
timeoutEvalSeconds: &overTimeout,
630-
expectedErrors: []string{"timeoutEvalSeconds cannot be greater than"},
631-
},
632619
{
633620
name: "timeoutEvalSeconds > timeoutSeconds",
634621
timeoutSeconds: &underTimeout,

api/policies/v1/policygroup_validation.go

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -142,25 +142,15 @@ func validatePolicyGroupExpressionField(policyGroup PolicyGroup) *field.Error {
142142
}
143143

144144
// validatePolicyGroupMembersTimeouts checks the timeouts so that:
145-
// - the group's timeoutSeconds is not greater than the Kubernetes webhook max timeout (30s).
146145
// - each member's timeoutEvalSeconds is not greater than the group's
147-
// timeoutSeconds nor the Kubernetes webhook max timeout (30s)
146+
// timeoutSeconds
148147
// - the sum of each members' timeoutEvalSeconds is not greater than the group's
149148
// timeoutSeconds nor the Kubernetes webhook max timeout (30s)
150149
func validatePolicyGroupMembersTimeouts(policyGroup PolicyGroup) field.ErrorList {
151150
var allErrors field.ErrorList
152151
groupTimeout := policyGroup.GetTimeoutSeconds()
153-
fldGroupTimeout := field.NewPath("spec").Child("timeoutSeconds")
154152
fldMembers := field.NewPath("spec").Child("policies")
155153

156-
if groupTimeout != nil && *groupTimeout > maxWebhookTimeoutSeconds {
157-
allErrors = append(allErrors, field.Invalid(
158-
fldGroupTimeout,
159-
*groupTimeout,
160-
fmt.Sprintf("timeoutSeconds cannot be greater than %d (Kubernetes webhook max timeout)", maxWebhookTimeoutSeconds),
161-
))
162-
}
163-
164154
sumMemberTimeoutEval := int32(0)
165155
for memberName, member := range policyGroup.GetPolicyGroupMembersWithContext() {
166156
memberTimeoutEval := member.TimeoutEvalSeconds
@@ -170,14 +160,12 @@ func validatePolicyGroupMembersTimeouts(policyGroup PolicyGroup) field.ErrorList
170160
sumMemberTimeoutEval += *memberTimeoutEval
171161

172162
if *memberTimeoutEval > maxWebhookTimeoutSeconds {
173-
sumMemberTimeoutEval += *memberTimeoutEval
174163
allErrors = append(allErrors, field.Invalid(
175164
fldMembers.Key(memberName).Child("timeoutEvalSeconds"),
176165
*memberTimeoutEval,
177166
fmt.Sprintf("timeoutEvalSeconds cannot be greater than %d (Kubernetes webhook max timeout)", maxWebhookTimeoutSeconds),
178167
))
179168
}
180-
181169
if groupTimeout != nil && *memberTimeoutEval > *groupTimeout {
182170
allErrors = append(allErrors, field.Invalid(
183171
fldMembers.Key(memberName).Child("timeoutEvalSeconds"),

api/policies/v1/policygroup_validation_test.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,6 @@ func TestValidatePolicyGroupMembers(t *testing.T) {
344344

345345
func TestValidatePolicyGroupTimeoutSeconds(t *testing.T) {
346346
maxTimeout := int32(30)
347-
overTimeout := int32(31)
348347
underTimeout := int32(10)
349348
overMinusUnderTimeout := int32(21)
350349

@@ -364,20 +363,6 @@ func TestValidatePolicyGroupTimeoutSeconds(t *testing.T) {
364363
timeoutEvalSeconds2: nil,
365364
expectedErrors: nil,
366365
},
367-
{
368-
name: "timeoutSeconds over max",
369-
timeoutSeconds: &overTimeout,
370-
timeoutEvalSeconds: nil,
371-
timeoutEvalSeconds2: nil,
372-
expectedErrors: []string{"timeoutSeconds cannot be greater than"},
373-
},
374-
{
375-
name: "timeoutEvalSeconds over max",
376-
timeoutSeconds: &maxTimeout,
377-
timeoutEvalSeconds: &overTimeout,
378-
timeoutEvalSeconds2: nil,
379-
expectedErrors: []string{"timeoutEvalSeconds cannot be greater than"},
380-
},
381366
{
382367
name: "timeoutEvalSeconds > timeoutSeconds",
383368
timeoutSeconds: &underTimeout,

config/crd/bases/policies.kubewarden.io_admissionpolicies.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,18 +356,22 @@ spec:
356356
TimeoutEvalSeconds specifies the timeout for the policy evaluation. After
357357
the timeout passes, the policy evaluation call will fail based on the
358358
failure policy.
359-
The timeout value must be between 1 and 30 seconds.
359+
The timeout value must be between 2 and 30 seconds.
360360
format: int32
361+
maximum: 30
362+
minimum: 2
361363
type: integer
362364
timeoutSeconds:
363365
default: 10
364366
description: |-
365367
TimeoutSeconds specifies the timeout for the policy webhook. After the timeout passes,
366368
the webhook call will be ignored or the API call will fail based on the
367369
failure policy.
368-
The timeout value must be between 1 and 30 seconds.
370+
The timeout value must be between 2 and 30 seconds.
369371
Default to 10 seconds.
370372
format: int32
373+
maximum: 30
374+
minimum: 2
371375
type: integer
372376
required:
373377
- module

config/crd/bases/policies.kubewarden.io_admissionpolicygroups.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,10 @@ spec:
283283
TimeoutEvalSeconds specifies the timeout for the policy evaluation. After
284284
the timeout passes, the policy evaluation call will fail based on the
285285
failure policy.
286-
The timeout value must be between 1 and 30 seconds.
286+
The timeout value must be between 2 and 30 seconds.
287287
format: int32
288+
maximum: 30
289+
minimum: 2
288290
type: integer
289291
required:
290292
- module
@@ -385,9 +387,11 @@ spec:
385387
TimeoutSeconds specifies the timeout for this webhook. After the timeout passes,
386388
the webhook call will be ignored or the API call will fail based on the
387389
failure policy.
388-
The timeout value must be between 1 and 30 seconds.
390+
The timeout value must be between 2 and 30 seconds.
389391
Default to 10 seconds.
390392
format: int32
393+
maximum: 30
394+
minimum: 2
391395
type: integer
392396
required:
393397
- expression

config/crd/bases/policies.kubewarden.io_clusteradmissionpolicies.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,18 +468,22 @@ spec:
468468
TimeoutEvalSeconds specifies the timeout for the policy evaluation. After
469469
the timeout passes, the policy evaluation call will fail based on the
470470
failure policy.
471-
The timeout value must be between 1 and 30 seconds.
471+
The timeout value must be between 2 and 30 seconds.
472472
format: int32
473+
maximum: 30
474+
minimum: 2
473475
type: integer
474476
timeoutSeconds:
475477
default: 10
476478
description: |-
477479
TimeoutSeconds specifies the timeout for the policy webhook. After the timeout passes,
478480
the webhook call will be ignored or the API call will fail based on the
479481
failure policy.
480-
The timeout value must be between 1 and 30 seconds.
482+
The timeout value must be between 2 and 30 seconds.
481483
Default to 10 seconds.
482484
format: int32
485+
maximum: 30
486+
minimum: 2
483487
type: integer
484488
required:
485489
- module

config/crd/bases/policies.kubewarden.io_clusteradmissionpolicygroups.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,8 +396,10 @@ spec:
396396
TimeoutEvalSeconds specifies the timeout for the policy evaluation. After
397397
the timeout passes, the policy evaluation call will fail based on the
398398
failure policy.
399-
The timeout value must be between 1 and 30 seconds.
399+
The timeout value must be between 2 and 30 seconds.
400400
format: int32
401+
maximum: 30
402+
minimum: 2
401403
type: integer
402404
required:
403405
- module
@@ -498,9 +500,11 @@ spec:
498500
TimeoutSeconds specifies the timeout for this webhook. After the timeout passes,
499501
the webhook call will be ignored or the API call will fail based on the
500502
failure policy.
501-
The timeout value must be between 1 and 30 seconds.
503+
The timeout value must be between 2 and 30 seconds.
502504
Default to 10 seconds.
503505
format: int32
506+
maximum: 30
507+
minimum: 2
504508
type: integer
505509
required:
506510
- expression

config/crd/bases/policies.kubewarden.io_policyservers.yaml

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -619,8 +619,8 @@ spec:
619619
most preferred is the one with the greatest sum of weights, i.e.
620620
for each node that meets all of the scheduling requirements (resource
621621
request, requiredDuringScheduling anti-affinity expressions, etc.),
622-
compute a sum by iterating through the elements of this field and adding
623-
"weight" to the sum if the node has pods which matches the corresponding podAffinityTerm; the
622+
compute a sum by iterating through the elements of this field and subtracting
623+
"weight" from the sum if the node has pods which matches the corresponding podAffinityTerm; the
624624
node(s) with the highest sum are the most preferred.
625625
items:
626626
description: The weights of all of the matched WeightedPodAffinityTerm
@@ -978,7 +978,9 @@ spec:
978978
a Container.
979979
properties:
980980
name:
981-
description: Name of the environment variable. Must be a C_IDENTIFIER.
981+
description: |-
982+
Name of the environment variable.
983+
May consist of any printable ASCII characters except '='.
982984
type: string
983985
value:
984986
description: |-
@@ -1036,6 +1038,43 @@ spec:
10361038
- fieldPath
10371039
type: object
10381040
x-kubernetes-map-type: atomic
1041+
fileKeyRef:
1042+
description: |-
1043+
FileKeyRef selects a key of the env file.
1044+
Requires the EnvFiles feature gate to be enabled.
1045+
properties:
1046+
key:
1047+
description: |-
1048+
The key within the env file. An invalid key will prevent the pod from starting.
1049+
The keys defined within a source may consist of any printable ASCII characters except '='.
1050+
During Alpha stage of the EnvFiles feature gate, the key size is limited to 128 characters.
1051+
type: string
1052+
optional:
1053+
default: false
1054+
description: |-
1055+
Specify whether the file or its key must be defined. If the file or key
1056+
does not exist, then the env var is not published.
1057+
If optional is set to true and the specified key does not exist,
1058+
the environment variable will not be set in the Pod's containers.
1059+
1060+
If optional is set to false and the specified key does not exist,
1061+
an error will be returned during Pod creation.
1062+
type: boolean
1063+
path:
1064+
description: |-
1065+
The path within the volume from which to select the file.
1066+
Must be relative and may not contain the '..' path or start with '..'.
1067+
type: string
1068+
volumeName:
1069+
description: The name of the volume mount containing
1070+
the env file.
1071+
type: string
1072+
required:
1073+
- key
1074+
- path
1075+
- volumeName
1076+
type: object
1077+
x-kubernetes-map-type: atomic
10391078
resourceFieldRef:
10401079
description: |-
10411080
Selects a resource of the container: only resources limits and requests
@@ -1791,7 +1830,9 @@ spec:
17911830
a Container.
17921831
properties:
17931832
name:
1794-
description: Name of the environment variable. Must be a C_IDENTIFIER.
1833+
description: |-
1834+
Name of the environment variable.
1835+
May consist of any printable ASCII characters except '='.
17951836
type: string
17961837
value:
17971838
description: |-
@@ -1849,6 +1890,43 @@ spec:
18491890
- fieldPath
18501891
type: object
18511892
x-kubernetes-map-type: atomic
1893+
fileKeyRef:
1894+
description: |-
1895+
FileKeyRef selects a key of the env file.
1896+
Requires the EnvFiles feature gate to be enabled.
1897+
properties:
1898+
key:
1899+
description: |-
1900+
The key within the env file. An invalid key will prevent the pod from starting.
1901+
The keys defined within a source may consist of any printable ASCII characters except '='.
1902+
During Alpha stage of the EnvFiles feature gate, the key size is limited to 128 characters.
1903+
type: string
1904+
optional:
1905+
default: false
1906+
description: |-
1907+
Specify whether the file or its key must be defined. If the file or key
1908+
does not exist, then the env var is not published.
1909+
If optional is set to true and the specified key does not exist,
1910+
the environment variable will not be set in the Pod's containers.
1911+
1912+
If optional is set to false and the specified key does not exist,
1913+
an error will be returned during Pod creation.
1914+
type: boolean
1915+
path:
1916+
description: |-
1917+
The path within the volume from which to select the file.
1918+
Must be relative and may not contain the '..' path or start with '..'.
1919+
type: string
1920+
volumeName:
1921+
description: The name of the volume mount containing
1922+
the env file.
1923+
type: string
1924+
required:
1925+
- key
1926+
- path
1927+
- volumeName
1928+
type: object
1929+
x-kubernetes-map-type: atomic
18521930
resourceFieldRef:
18531931
description: |-
18541932
Selects a resource of the container: only resources limits and requests

0 commit comments

Comments
 (0)