Skip to content

Commit 3460b22

Browse files
committed
Disallow optional/required on non-pointer structs
1 parent dcbfe67 commit 3460b22

File tree

4 files changed

+15
-27
lines changed

4 files changed

+15
-27
lines changed

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/doc.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ type Struct struct {
3535
// +k8s:validateFalse="field Struct.StringPtrField"
3636
StringPtrField *string `json:"stringPtrField"`
3737

38-
// +k8s:optional
39-
// +k8s:validateFalse="field Struct.OtherStructField"
40-
OtherStructField OtherStruct `json:"otherStructField"`
38+
// non-pointer struct fields cannot be optional
4139

4240
// +k8s:optional
4341
// +k8s:validateFalse="field Struct.OtherStructPtrField"

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/doc_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,17 @@ func Test(t *testing.T) {
2727

2828
st.Value(&Struct{
2929
// All zero-values.
30-
}).ExpectValidateFalseByPath(map[string][]string{
31-
"otherStructField": {"type OtherStruct", "field Struct.OtherStructField"}, // optional for structs is just documentation
32-
})
30+
}).ExpectValid()
3331

3432
st.Value(&Struct{
3533
StringField: "abc",
3634
StringPtrField: ptr.To("xyz"),
37-
OtherStructField: OtherStruct{},
3835
OtherStructPtrField: &OtherStruct{},
3936
SliceField: []string{"a", "b"},
4037
MapField: map[string]string{"a": "b", "c": "d"},
4138
}).ExpectValidateFalseByPath(map[string][]string{
4239
"stringField": {"field Struct.StringField"},
4340
"stringPtrField": {"field Struct.StringPtrField"},
44-
"otherStructField": {"type OtherStruct", "field Struct.OtherStructField"},
4541
"otherStructPtrField": {"type OtherStruct", "field Struct.OtherStructPtrField"},
4642
"sliceField": {"field Struct.SliceField"},
4743
"mapField": {"field Struct.MapField"},

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zz_generated.validations.go

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,12 @@ func (requirednessTagValidator) doRequired(context Context) (Validations, error)
9797
case types.Pointer:
9898
return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredPointerValidator)}}, nil
9999
case types.Struct:
100-
// The +required tag on a non-pointer struct is only for documentation.
101-
// We don't perform validation here and defer the validation to
102-
// the struct's fields.
103-
return Validations{Comments: []string{"required non-pointer structs are purely documentation"}}, nil
100+
// The +k8s:required tag on a non-pointer struct is not supported.
101+
// If you encounter this error and believe you have a valid use case
102+
// for forbiddening a non-pointer struct, please let us know! We need
103+
// to understand your scenario to determine if we need to adjust
104+
// this behavior or provide alternative validation mechanisms.
105+
return Validations{}, fmt.Errorf("non-pointer structs cannot use the %q tag", requiredTagName)
104106
}
105107
return Validations{Functions: []FunctionGen{Function(requiredTagName, ShortCircuit, requiredValueValidator)}}, nil
106108
}
@@ -125,11 +127,12 @@ func (requirednessTagValidator) doOptional(context Context) (Validations, error)
125127
case types.Pointer:
126128
return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalPointerValidator)}}, nil
127129
case types.Struct:
128-
// Specifying that a non-pointer struct is optional doesn't actually
129-
// make sense technically almost ever, and is better described as a
130-
// union inside the struct. It does, however, make sense as
131-
// documentation.
132-
return Validations{Comments: []string{"optional non-pointer structs are purely documentation"}}, nil
130+
// The +k8s:optional tag on a non-pointer struct is not supported.
131+
// If you encounter this error and believe you have a valid use case
132+
// for forbiddening a non-pointer struct, please let us know! We need
133+
// to understand your scenario to determine if we need to adjust
134+
// this behavior or provide alternative validation mechanisms.
135+
return Validations{}, fmt.Errorf("non-pointer structs cannot use the %q tag", optionalTagName)
133136
}
134137
return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalValueValidator)}}, nil
135138
}
@@ -174,7 +177,7 @@ func (requirednessTagValidator) doForbidden(context Context) (Validations, error
174177
},
175178
}, nil
176179
case types.Struct:
177-
// The +forbidden tag on a non-pointer struct is not supported.
180+
// The +k8s:forbidden tag on a non-pointer struct is not supported.
178181
// If you encounter this error and believe you have a valid use case
179182
// for forbiddening a non-pointer struct, please let us know! We need
180183
// to understand your scenario to determine if we need to adjust

0 commit comments

Comments
 (0)