Skip to content

Commit bd76293

Browse files
deads2kdamemi
authored andcommitted
add validation for metav1 conditions
1 parent 5afc42d commit bd76293

File tree

2 files changed

+209
-0
lines changed

2 files changed

+209
-0
lines changed

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package validation
1818

1919
import (
2020
"fmt"
21+
"regexp"
2122
"unicode"
2223

2324
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -184,3 +185,78 @@ func ValidateManagedFields(fieldsList []metav1.ManagedFieldsEntry, fldPath *fiel
184185
}
185186
return allErrs
186187
}
188+
189+
func ValidateConditions(conditions []metav1.Condition, fldPath *field.Path) field.ErrorList {
190+
var allErrs field.ErrorList
191+
192+
conditionTypeToFirstIndex := map[string]int{}
193+
for i, condition := range conditions {
194+
if _, ok := conditionTypeToFirstIndex[condition.Type]; ok {
195+
allErrs = append(allErrs, field.Duplicate(fldPath.Index(i).Child("type"), condition.Type))
196+
} else {
197+
conditionTypeToFirstIndex[condition.Type] = i
198+
}
199+
200+
allErrs = append(allErrs, ValidateCondition(condition, fldPath.Index(i))...)
201+
}
202+
203+
return allErrs
204+
}
205+
206+
// validConditionStatuses is used internally to check validity and provide a good message
207+
var validConditionStatuses = sets.NewString(string(metav1.ConditionTrue), string(metav1.ConditionFalse), string(metav1.ConditionUnknown))
208+
209+
const (
210+
maxReasonLen = 1 * 1024
211+
maxMessageLen = 32 * 1024
212+
)
213+
214+
func ValidateCondition(condition metav1.Condition, fldPath *field.Path) field.ErrorList {
215+
var allErrs field.ErrorList
216+
217+
// type is set and is a valid format
218+
allErrs = append(allErrs, ValidateLabelName(condition.Type, fldPath.Child("type"))...)
219+
220+
// status is set and is an accepted value
221+
if !validConditionStatuses.Has(string(condition.Status)) {
222+
allErrs = append(allErrs, field.NotSupported(fldPath.Child("status"), condition.Status, validConditionStatuses.List()))
223+
}
224+
225+
if condition.ObservedGeneration < 0 {
226+
allErrs = append(allErrs, field.Invalid(fldPath.Child("observedGeneration"), condition.ObservedGeneration, "must be greater than or equal to zero"))
227+
}
228+
229+
if condition.LastTransitionTime.IsZero() {
230+
allErrs = append(allErrs, field.Required(fldPath.Child("lastTransitionTime"), "must be set"))
231+
}
232+
233+
if len(condition.Reason) == 0 {
234+
allErrs = append(allErrs, field.Required(fldPath.Child("reason"), "must be set"))
235+
} else {
236+
for _, currErr := range isValidConditionReason(condition.Reason) {
237+
allErrs = append(allErrs, field.Invalid(fldPath.Child("reason"), condition.Reason, currErr))
238+
}
239+
if len(condition.Reason) > maxReasonLen {
240+
allErrs = append(allErrs, field.TooLong(fldPath.Child("reason"), condition.Reason, maxReasonLen))
241+
}
242+
}
243+
244+
if len(condition.Message) > maxMessageLen {
245+
allErrs = append(allErrs, field.TooLong(fldPath.Child("message"), condition.Message, maxMessageLen))
246+
}
247+
248+
return allErrs
249+
}
250+
251+
const conditionReasonFmt string = "[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?"
252+
const conditionReasonErrMsg string = "a condition reason must start with alphabetic character, optionally followed by a string of alphanumeric characters or '_,:', and must end with an alphanumeric character or '_'"
253+
254+
var conditionReasonRegexp = regexp.MustCompile("^" + conditionReasonFmt + "$")
255+
256+
// isValidConditionReason tests for a string that conforms to rules for condition reasons. This checks the format, but not the length.
257+
func isValidConditionReason(value string) []string {
258+
if !conditionReasonRegexp.MatchString(value) {
259+
return []string{validation.RegexError(conditionReasonErrMsg, conditionReasonFmt, "my_name", "MY_NAME", "MyName", "ReasonA,ReasonB", "ReasonA:ReasonB")}
260+
}
261+
return nil
262+
}

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation/validation_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,3 +293,136 @@ func TestValidateMangedFieldsValid(t *testing.T) {
293293
})
294294
}
295295
}
296+
297+
func TestValidateConditions(t *testing.T) {
298+
tests := []struct {
299+
name string
300+
conditions []metav1.Condition
301+
validateErrs func(t *testing.T, errs field.ErrorList)
302+
}{
303+
{
304+
name: "bunch-of-invalid-fields",
305+
conditions: []metav1.Condition{{
306+
Type: ":invalid",
307+
Status: "unknown",
308+
ObservedGeneration: -1,
309+
LastTransitionTime: metav1.Time{},
310+
Reason: "invalid;val",
311+
Message: "",
312+
}},
313+
validateErrs: func(t *testing.T, errs field.ErrorList) {
314+
needle := `status.conditions[0].type: Invalid value: ":invalid": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`
315+
if !hasError(errs, needle) {
316+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
317+
}
318+
needle = `status.conditions[0].status: Unsupported value: "unknown": supported values: "False", "True", "Unknown"`
319+
if !hasError(errs, needle) {
320+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
321+
}
322+
needle = `status.conditions[0].observedGeneration: Invalid value: -1: must be greater than or equal to zero`
323+
if !hasError(errs, needle) {
324+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
325+
}
326+
needle = `status.conditions[0].lastTransitionTime: Required value: must be set`
327+
if !hasError(errs, needle) {
328+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
329+
}
330+
needle = `status.conditions[0].reason: Invalid value: "invalid;val": a condition reason must start with alphabetic character, optionally followed by a string of alphanumeric characters or '_,:', and must end with an alphanumeric character or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', or 'ReasonA,ReasonB', or 'ReasonA:ReasonB', regex used for validation is '[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?')`
331+
if !hasError(errs, needle) {
332+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
333+
}
334+
},
335+
},
336+
{
337+
name: "duplicates",
338+
conditions: []metav1.Condition{{
339+
Type: "First",
340+
},
341+
{
342+
Type: "Second",
343+
},
344+
{
345+
Type: "First",
346+
},
347+
},
348+
validateErrs: func(t *testing.T, errs field.ErrorList) {
349+
needle := `status.conditions[2].type: Duplicate value: "First"`
350+
if !hasError(errs, needle) {
351+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
352+
}
353+
},
354+
},
355+
{
356+
name: "colon-allowed-in-reason",
357+
conditions: []metav1.Condition{{
358+
Type: "First",
359+
Reason: "valid:val",
360+
}},
361+
validateErrs: func(t *testing.T, errs field.ErrorList) {
362+
needle := `status.conditions[0].reason`
363+
if hasPrefixError(errs, needle) {
364+
t.Errorf("has %q in\n%v", needle, errorsAsString(errs))
365+
}
366+
},
367+
},
368+
{
369+
name: "comma-allowed-in-reason",
370+
conditions: []metav1.Condition{{
371+
Type: "First",
372+
Reason: "valid,val",
373+
}},
374+
validateErrs: func(t *testing.T, errs field.ErrorList) {
375+
needle := `status.conditions[0].reason`
376+
if hasPrefixError(errs, needle) {
377+
t.Errorf("has %q in\n%v", needle, errorsAsString(errs))
378+
}
379+
},
380+
},
381+
{
382+
name: "reason-does-not-end-in-delimiter",
383+
conditions: []metav1.Condition{{
384+
Type: "First",
385+
Reason: "valid,val:",
386+
}},
387+
validateErrs: func(t *testing.T, errs field.ErrorList) {
388+
needle := `status.conditions[0].reason: Invalid value: "valid,val:": a condition reason must start with alphabetic character, optionally followed by a string of alphanumeric characters or '_,:', and must end with an alphanumeric character or '_' (e.g. 'my_name', or 'MY_NAME', or 'MyName', or 'ReasonA,ReasonB', or 'ReasonA:ReasonB', regex used for validation is '[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?')`
389+
if !hasError(errs, needle) {
390+
t.Errorf("missing %q in\n%v", needle, errorsAsString(errs))
391+
}
392+
},
393+
},
394+
}
395+
396+
for _, test := range tests {
397+
t.Run(test.name, func(t *testing.T) {
398+
errs := ValidateConditions(test.conditions, field.NewPath("status").Child("conditions"))
399+
test.validateErrs(t, errs)
400+
})
401+
}
402+
}
403+
404+
func hasError(errs field.ErrorList, needle string) bool {
405+
for _, curr := range errs {
406+
if curr.Error() == needle {
407+
return true
408+
}
409+
}
410+
return false
411+
}
412+
413+
func hasPrefixError(errs field.ErrorList, prefix string) bool {
414+
for _, curr := range errs {
415+
if strings.HasPrefix(curr.Error(), prefix) {
416+
return true
417+
}
418+
}
419+
return false
420+
}
421+
422+
func errorsAsString(errs field.ErrorList) string {
423+
messages := []string{}
424+
for _, curr := range errs {
425+
messages = append(messages, curr.Error())
426+
}
427+
return strings.Join(messages, "\n")
428+
}

0 commit comments

Comments
 (0)