Disallow optional/required on non-pointer structs

This commit is contained in:
Tim Hockin 2025-03-05 20:04:56 -08:00
parent dcbfe67b1c
commit 3460b2238e
No known key found for this signature in database
4 changed files with 15 additions and 27 deletions

View File

@ -35,9 +35,7 @@ type Struct struct {
// +k8s:validateFalse="field Struct.StringPtrField" // +k8s:validateFalse="field Struct.StringPtrField"
StringPtrField *string `json:"stringPtrField"` StringPtrField *string `json:"stringPtrField"`
// +k8s:optional // non-pointer struct fields cannot be optional
// +k8s:validateFalse="field Struct.OtherStructField"
OtherStructField OtherStruct `json:"otherStructField"`
// +k8s:optional // +k8s:optional
// +k8s:validateFalse="field Struct.OtherStructPtrField" // +k8s:validateFalse="field Struct.OtherStructPtrField"

View File

@ -27,21 +27,17 @@ func Test(t *testing.T) {
st.Value(&Struct{ st.Value(&Struct{
// All zero-values. // All zero-values.
}).ExpectValidateFalseByPath(map[string][]string{ }).ExpectValid()
"otherStructField": {"type OtherStruct", "field Struct.OtherStructField"}, // optional for structs is just documentation
})
st.Value(&Struct{ st.Value(&Struct{
StringField: "abc", StringField: "abc",
StringPtrField: ptr.To("xyz"), StringPtrField: ptr.To("xyz"),
OtherStructField: OtherStruct{},
OtherStructPtrField: &OtherStruct{}, OtherStructPtrField: &OtherStruct{},
SliceField: []string{"a", "b"}, SliceField: []string{"a", "b"},
MapField: map[string]string{"a": "b", "c": "d"}, MapField: map[string]string{"a": "b", "c": "d"},
}).ExpectValidateFalseByPath(map[string][]string{ }).ExpectValidateFalseByPath(map[string][]string{
"stringField": {"field Struct.StringField"}, "stringField": {"field Struct.StringField"},
"stringPtrField": {"field Struct.StringPtrField"}, "stringPtrField": {"field Struct.StringPtrField"},
"otherStructField": {"type OtherStruct", "field Struct.OtherStructField"},
"otherStructPtrField": {"type OtherStruct", "field Struct.OtherStructPtrField"}, "otherStructPtrField": {"type OtherStruct", "field Struct.OtherStructPtrField"},
"sliceField": {"field Struct.SliceField"}, "sliceField": {"field Struct.SliceField"},
"mapField": {"field Struct.MapField"}, "mapField": {"field Struct.MapField"},

View File

@ -76,15 +76,6 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field
return return
}(fldPath.Child("stringPtrField"), obj.StringPtrField, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.StringPtrField }))...) }(fldPath.Child("stringPtrField"), obj.StringPtrField, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.StringPtrField }))...)
// field Struct.OtherStructField
errs = append(errs,
func(fldPath *field.Path, obj, oldObj *OtherStruct) (errs field.ErrorList) {
errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field Struct.OtherStructField")...)
// optional non-pointer structs are purely documentation
errs = append(errs, Validate_OtherStruct(ctx, op, fldPath, obj, oldObj)...)
return
}(fldPath.Child("otherStructField"), &obj.OtherStructField, safe.Field(oldObj, func(oldObj *Struct) *OtherStruct { return &oldObj.OtherStructField }))...)
// field Struct.OtherStructPtrField // field Struct.OtherStructPtrField
errs = append(errs, errs = append(errs,
func(fldPath *field.Path, obj, oldObj *OtherStruct) (errs field.ErrorList) { func(fldPath *field.Path, obj, oldObj *OtherStruct) (errs field.ErrorList) {

View File

@ -97,10 +97,12 @@ func (requirednessTagValidator) doRequired(context Context) (Validations, error)
case types.Pointer: case types.Pointer:
return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredPointerValidator)}}, nil return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredPointerValidator)}}, nil
case types.Struct: case types.Struct:
// The +required tag on a non-pointer struct is only for documentation. // The +k8s:required tag on a non-pointer struct is not supported.
// We don't perform validation here and defer the validation to // If you encounter this error and believe you have a valid use case
// the struct's fields. // for forbiddening a non-pointer struct, please let us know! We need
return Validations{Comments: []string{"required non-pointer structs are purely documentation"}}, nil // to understand your scenario to determine if we need to adjust
// this behavior or provide alternative validation mechanisms.
return Validations{}, fmt.Errorf("non-pointer structs cannot use the %q tag", requiredTagName)
} }
return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredValueValidator)}}, nil return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredValueValidator)}}, nil
} }
@ -125,11 +127,12 @@ func (requirednessTagValidator) doOptional(context Context) (Validations, error)
case types.Pointer: case types.Pointer:
return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalPointerValidator)}}, nil return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalPointerValidator)}}, nil
case types.Struct: case types.Struct:
// Specifying that a non-pointer struct is optional doesn't actually // The +k8s:optional tag on a non-pointer struct is not supported.
// make sense technically almost ever, and is better described as a // If you encounter this error and believe you have a valid use case
// union inside the struct. It does, however, make sense as // for forbiddening a non-pointer struct, please let us know! We need
// documentation. // to understand your scenario to determine if we need to adjust
return Validations{Comments: []string{"optional non-pointer structs are purely documentation"}}, nil // this behavior or provide alternative validation mechanisms.
return Validations{}, fmt.Errorf("non-pointer structs cannot use the %q tag", optionalTagName)
} }
return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalValueValidator)}}, nil return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalValueValidator)}}, nil
} }
@ -174,7 +177,7 @@ func (requirednessTagValidator) doForbidden(context Context) (Validations, error
}, },
}, nil }, nil
case types.Struct: case types.Struct:
// The +forbidden tag on a non-pointer struct is not supported. // The +k8s:forbidden tag on a non-pointer struct is not supported.
// If you encounter this error and believe you have a valid use case // If you encounter this error and believe you have a valid use case
// for forbiddening a non-pointer struct, please let us know! We need // for forbiddening a non-pointer struct, please let us know! We need
// to understand your scenario to determine if we need to adjust // to understand your scenario to determine if we need to adjust