Skip to content

Commit 9c6d0b8

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#103402 from ilyee/validation-test
Add unit tests for validateStructuralInvariants
2 parents 355bc3d + 8264dbe commit 9c6d0b8

File tree

2 files changed

+154
-21
lines changed

2 files changed

+154
-21
lines changed

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -145,35 +145,45 @@ func validateStructuralInvariants(s *Structural, lvl level, fldPath *field.Path)
145145
allErrs = append(allErrs, field.Invalid(fldPath.Child("properties").Key("apiVersion").Child("type"), apiVersion.Type, "must be string"))
146146
}
147147
}
148-
if metadata, found := s.Properties["metadata"]; found && checkMetadata {
149-
if metadata.Type != "object" {
150-
allErrs = append(allErrs, field.Invalid(fldPath.Child("properties").Key("metadata").Child("type"), metadata.Type, "must be object"))
151-
}
148+
149+
if metadata, found := s.Properties["metadata"]; found {
150+
allErrs = append(allErrs, validateStructuralMetadataInvariants(&metadata, checkMetadata, lvl, fldPath.Child("properties").Key("metadata"))...)
151+
}
152+
153+
if s.XEmbeddedResource && !s.XPreserveUnknownFields && len(s.Properties) == 0 {
154+
allErrs = append(allErrs, field.Required(fldPath.Child("properties"), "must not be empty if x-kubernetes-embedded-resource is true without x-kubernetes-preserve-unknown-fields"))
155+
}
156+
157+
return allErrs
158+
}
159+
160+
func validateStructuralMetadataInvariants(s *Structural, checkMetadata bool, lvl level, fldPath *field.Path) field.ErrorList {
161+
allErrs := field.ErrorList{}
162+
163+
if checkMetadata && s.Type != "object" {
164+
allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), s.Type, "must be object"))
152165
}
153-
if metadata, found := s.Properties["metadata"]; found && lvl == rootLevel {
166+
167+
if lvl == rootLevel {
154168
// metadata is a shallow copy. We can mutate it.
155-
_, foundName := metadata.Properties["name"]
156-
_, foundGenerateName := metadata.Properties["generateName"]
157-
if foundName && foundGenerateName && len(metadata.Properties) == 2 {
158-
metadata.Properties = nil
159-
} else if (foundName || foundGenerateName) && len(metadata.Properties) == 1 {
160-
metadata.Properties = nil
169+
_, foundName := s.Properties["name"]
170+
_, foundGenerateName := s.Properties["generateName"]
171+
if foundName && foundGenerateName && len(s.Properties) == 2 {
172+
s.Properties = nil
173+
} else if (foundName || foundGenerateName) && len(s.Properties) == 1 {
174+
s.Properties = nil
161175
}
162-
metadata.Type = ""
163-
metadata.Default.Object = nil // this is checked in API validation (and also tested)
164-
if metadata.ValueValidation == nil {
165-
metadata.ValueValidation = &ValueValidation{}
176+
s.Type = ""
177+
s.Default.Object = nil // this is checked in API validation (and also tested)
178+
if s.ValueValidation == nil {
179+
s.ValueValidation = &ValueValidation{}
166180
}
167-
if !reflect.DeepEqual(metadata, Structural{ValueValidation: &ValueValidation{}}) {
181+
if !reflect.DeepEqual(*s, Structural{ValueValidation: &ValueValidation{}}) {
168182
// TODO: this is actually a field.Invalid error, but we cannot do JSON serialization of metadata here to get a proper message
169-
allErrs = append(allErrs, field.Forbidden(fldPath.Child("properties").Key("metadata"), "must not specify anything other than name and generateName, but metadata is implicitly specified"))
183+
allErrs = append(allErrs, field.Forbidden(fldPath, "must not specify anything other than name and generateName, but metadata is implicitly specified"))
170184
}
171185
}
172186

173-
if s.XEmbeddedResource && !s.XPreserveUnknownFields && len(s.Properties) == 0 {
174-
allErrs = append(allErrs, field.Required(fldPath.Child("properties"), "must not be empty if x-kubernetes-embedded-resource is true without x-kubernetes-preserve-unknown-fields"))
175-
}
176-
177187
return allErrs
178188
}
179189

‎staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation_test.go‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,132 @@ import (
2020
"reflect"
2121
"testing"
2222

23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
2325
fuzz "github.com/google/gofuzz"
2426
)
2527

28+
func TestValidateStructuralMetadataInvariants(t *testing.T) {
29+
fuzzer := fuzz.New()
30+
fuzzer.Funcs(
31+
func(s *JSON, c fuzz.Continue) {
32+
if c.RandBool() {
33+
s.Object = float64(42.0)
34+
}
35+
},
36+
func(s **StructuralOrBool, c fuzz.Continue) {
37+
if c.RandBool() {
38+
*s = &StructuralOrBool{}
39+
}
40+
},
41+
func(s **Structural, c fuzz.Continue) {
42+
if c.RandBool() {
43+
*s = &Structural{}
44+
}
45+
},
46+
func(s *Structural, c fuzz.Continue) {
47+
if c.RandBool() {
48+
*s = Structural{}
49+
}
50+
},
51+
func(vv **NestedValueValidation, c fuzz.Continue) {
52+
if c.RandBool() {
53+
*vv = &NestedValueValidation{}
54+
}
55+
},
56+
func(vv *NestedValueValidation, c fuzz.Continue) {
57+
if c.RandBool() {
58+
*vv = NestedValueValidation{}
59+
}
60+
},
61+
)
62+
fuzzer.NilChance(0)
63+
64+
// check that type must be object
65+
typeNames := []string{"object", "array", "number", "integer", "boolean", "string"}
66+
for _, typeName := range typeNames {
67+
s := Structural{
68+
Generic: Generic{
69+
Type: typeName,
70+
},
71+
}
72+
73+
errs := validateStructuralMetadataInvariants(&s, true, rootLevel, nil)
74+
if len(errs) != 0 {
75+
t.Logf("errors returned: %v", errs)
76+
}
77+
if len(errs) != 0 && typeName == "object" {
78+
t.Errorf("unexpected forbidden field validation errors for: %#v", s)
79+
}
80+
if len(errs) == 0 && typeName != "object" {
81+
t.Errorf("expected forbidden field validation errors for: %#v", s)
82+
}
83+
}
84+
85+
// check that anything other than name and generateName of ObjectMeta in metadata properties is forbidden
86+
tt := reflect.TypeOf(metav1.ObjectMeta{})
87+
for i := 0; i < tt.NumField(); i++ {
88+
property := tt.Field(i).Name
89+
s := &Structural{
90+
Generic: Generic{
91+
Type: "object",
92+
},
93+
Properties: map[string]Structural{
94+
property: {},
95+
},
96+
}
97+
98+
errs := validateStructuralMetadataInvariants(s, true, rootLevel, nil)
99+
if len(errs) != 0 {
100+
t.Logf("errors returned: %v", errs)
101+
}
102+
if len(errs) != 0 && (property == "name" || property == "generateName") {
103+
t.Errorf("unexpected forbidden field validation errors for: %#v", s)
104+
}
105+
if len(errs) == 0 && property != "name" && property != "generateName" {
106+
t.Errorf("expected forbidden field validation errors for: %#v", s)
107+
}
108+
}
109+
110+
// check that anything other than type and properties in metadata is forbidden
111+
tt = reflect.TypeOf(Structural{})
112+
for i := 0; i < tt.NumField(); i++ {
113+
s := Structural{}
114+
x := reflect.ValueOf(&s).Elem()
115+
fuzzer.Fuzz(x.Field(i).Addr().Interface())
116+
s.Type = "object"
117+
s.Properties = map[string]Structural{
118+
"name": {},
119+
"generateName": {},
120+
}
121+
s.Default.Object = nil // this is checked in API validation, we don't need to test it here
122+
123+
valid := reflect.DeepEqual(s, Structural{
124+
Generic: Generic{
125+
Type: "object",
126+
Default: JSON{
127+
Object: nil,
128+
},
129+
},
130+
Properties: map[string]Structural{
131+
"name": {},
132+
"generateName": {},
133+
},
134+
})
135+
136+
errs := validateStructuralMetadataInvariants(s.DeepCopy(), true, rootLevel, nil)
137+
if len(errs) != 0 {
138+
t.Logf("errors returned: %v", errs)
139+
}
140+
if len(errs) != 0 && valid {
141+
t.Errorf("unexpected forbidden field validation errors for: %#v", s)
142+
}
143+
if len(errs) == 0 && !valid {
144+
t.Errorf("expected forbidden field validation errors for: %#v", s)
145+
}
146+
}
147+
}
148+
26149
func TestValidateNestedValueValidationComplete(t *testing.T) {
27150
fuzzer := fuzz.New()
28151
fuzzer.Funcs(

0 commit comments

Comments
 (0)