From 141e98ed050d8642c718cbdcb33bfe4eec317e6a Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 21 Feb 2025 14:15:44 -0800 Subject: [PATCH 1/2] Add comments to FunctionGen Now we can emit comments which stick to functions instead of coming before or after the functions when emitting code. For followup: I think we can simplify FunctionGen and ValidationGen --- .../cmd/validation-gen/validation.go | 12 +++++++- .../validation-gen/validators/validators.go | 29 ++++++++++++++++++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go index 54e5c6c9820..3f2b37043a1 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go @@ -1174,6 +1174,9 @@ func emitCallsToValidators(c *generator.Context, validations []validators.Functi } } + for _, comment := range v.Comments() { + sw.Do("// $.$\n", comment) + } if isShortCircuit { sw.Do("if e := ", nil) emitCall() @@ -1214,11 +1217,15 @@ func (g *genValidations) emitValidationVariables(c *generator.Context, t *types. return cmp.Compare(a.Var().Name, b.Var().Name) }) for _, variable := range variables { - supportInitFn, supportInitArgs := variable.Init().SignatureAndArgs() + fn := variable.Init() + supportInitFn, supportInitArgs := fn.SignatureAndArgs() targs := generator.Args{ "varName": c.Universe.Type(types.Name(variable.Var())), "initFn": c.Universe.Type(supportInitFn), } + for _, comment := range fn.Comments() { + sw.Do("// $.$\n", comment) + } sw.Do("var $.varName|private$ = $.initFn|raw$", targs) typeArgs := variable.Init().TypeArgs() if len(typeArgs) > 0 { @@ -1277,6 +1284,9 @@ func toGolangSourceDataLiteral(sw *generator.SnippetWriter, c *generator.Context targs := generator.Args{ "funcName": c.Universe.Type(fn), } + for _, comment := range v.Function.Comments() { + sw.Do("// $.$\n", comment) + } sw.Do("$.funcName|raw$", targs) } else { // If the function to be wrapped has additional arguments, we need diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go index 4ad16b903f5..81f54f80e7b 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go @@ -302,6 +302,10 @@ type FunctionGen interface { // Conditions returns the conditions that must true for a resource to be // validated by this function. Conditions() Conditions + + // Comments returns optional comments that should be added to the generated + // code (without the leading "//"). + Comments() []string } // Conditions defines what conditions must be true for a resource to be validated. @@ -356,14 +360,34 @@ func GenericFunction(tagName string, flags FunctionFlags, function types.Name, t return &functionGen{tagName: tagName, flags: flags, function: function, extraArgs: anyArgs, typeArgs: typeArgs} } +// WithCondition adds a condition to a FunctionGen. func WithCondition(fn FunctionGen, conditions Conditions) FunctionGen { name, args := fn.SignatureAndArgs() return &functionGen{ - tagName: fn.TagName(), flags: fn.Flags(), function: name, extraArgs: args, typeArgs: fn.TypeArgs(), + tagName: fn.TagName(), + flags: fn.Flags(), + function: name, + extraArgs: args, + typeArgs: fn.TypeArgs(), + comments: fn.Comments(), conditions: conditions, } } +// WithComment adds a comment to a FunctionGen. +func WithComment(fn FunctionGen, comment string) FunctionGen { + name, args := fn.SignatureAndArgs() + return &functionGen{ + tagName: fn.TagName(), + flags: fn.Flags(), + function: name, + extraArgs: args, + typeArgs: fn.TypeArgs(), + comments: append(fn.Comments(), comment), + conditions: fn.Conditions(), + } +} + type functionGen struct { tagName string function types.Name @@ -371,6 +395,7 @@ type functionGen struct { typeArgs []types.Name flags FunctionFlags conditions Conditions + comments []string } func (v *functionGen) TagName() string { @@ -389,6 +414,8 @@ func (v *functionGen) Flags() FunctionFlags { func (v *functionGen) Conditions() Conditions { return v.conditions } +func (v *functionGen) Comments() []string { return v.comments } + // Variable creates a VariableGen for a given function name and extraArgs. func Variable(variable PrivateVar, init FunctionGen) VariableGen { return &variableGen{ From 92aeb63a5be90bdcc044aa820d5cc014b4cbb65b Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 22 Feb 2025 18:04:33 -0800 Subject: [PATCH 2/2] Handle optional value-types with defaults --- .../tags/optional/nonzero_defaults/doc.go | 53 ++++++++ .../optional/nonzero_defaults/doc_test.go | 47 +++++++ .../zz_generated.validations.go | 119 ++++++++++++++++++ .../tags/optional/zero_defaults/doc.go | 53 ++++++++ .../tags/optional/zero_defaults/doc_test.go | 47 +++++++ .../zero_defaults/zz_generated.validations.go | 107 ++++++++++++++++ .../cmd/validation-gen/validators/required.go | 117 ++++++++++++++++- .../validation-gen/validators/validators.go | 5 +- 8 files changed, 543 insertions(+), 5 deletions(-) create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc.go create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc_test.go create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/zz_generated.validations.go create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc.go create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc_test.go create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/zz_generated.validations.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc.go new file mode 100644 index 00000000000..d5206da9733 --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc.go @@ -0,0 +1,53 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// +k8s:validation-gen=TypeMeta +// +k8s:validation-gen-scheme-registry=k8s.io/code-generator/cmd/validation-gen/testscheme.Scheme + +// This is a test package. +package nonzerodefaults + +import "k8s.io/code-generator/cmd/validation-gen/testscheme" + +var localSchemeBuilder = testscheme.New() + +type Struct struct { + TypeMeta int + + // +k8s:optional + // +default="foobar" + StringField string `json:"stringField"` + + // +k8s:optional + // +default="foobar" + StringPtrField *string `json:"stringPtrField"` + + // +k8s:optional + // +default=123 + IntField int `json:"intField"` + + // +k8s:optional + // +default=123 + IntPtrField *int `json:"intPtrField"` + + // +k8s:optional + // +default=true + BoolField bool `json:"boolField"` + + // +k8s:optional + // +default=true + BoolPtrField *bool `json:"boolPtrField"` +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc_test.go new file mode 100644 index 00000000000..18841fe4f7f --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/doc_test.go @@ -0,0 +1,47 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nonzerodefaults + +import ( + "testing" + + "k8s.io/utils/ptr" +) + +func Test(t *testing.T) { + st := localSchemeBuilder.Test(t) + + st.Value(&Struct{ + // All zero-values. + }).ExpectRegexpsByPath(map[string][]string{ + "stringField": {"Required value"}, + "stringPtrField": {"Required value"}, + "intField": {"Required value"}, + "intPtrField": {"Required value"}, + "boolField": {"Required value"}, + "boolPtrField": {"Required value"}, + }) + + st.Value(&Struct{ + StringField: "abc", + StringPtrField: ptr.To(""), + IntField: 123, + IntPtrField: ptr.To(0), + BoolField: true, + BoolPtrField: ptr.To(false), + }).ExpectValid() +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/zz_generated.validations.go new file mode 100644 index 00000000000..c710c8deb71 --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/nonzero_defaults/zz_generated.validations.go @@ -0,0 +1,119 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by validation-gen. DO NOT EDIT. + +package nonzerodefaults + +import ( + context "context" + fmt "fmt" + + operation "k8s.io/apimachinery/pkg/api/operation" + safe "k8s.io/apimachinery/pkg/api/safe" + validate "k8s.io/apimachinery/pkg/api/validate" + field "k8s.io/apimachinery/pkg/util/validation/field" + testscheme "k8s.io/code-generator/cmd/validation-gen/testscheme" +) + +func init() { localSchemeBuilder.Register(RegisterValidations) } + +// RegisterValidations adds validation functions to the given scheme. +// Public to allow building arbitrary schemes. +func RegisterValidations(scheme *testscheme.Scheme) error { + scheme.AddValidationFunc((*Struct)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}, subresources ...string) field.ErrorList { + if len(subresources) == 0 { + return Validate_Struct(ctx, op, nil /* fldPath */, obj.(*Struct), safe.Cast[*Struct](oldObj)) + } + return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresources: %v", obj, subresources))} + }) + return nil +} + +func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *Struct) (errs field.ErrorList) { + // field Struct.TypeMeta has no validation + + // field Struct.StringField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredValue(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("stringField"), &obj.StringField, safe.Field(oldObj, func(oldObj *Struct) *string { return &oldObj.StringField }))...) + + // field Struct.StringPtrField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("stringPtrField"), obj.StringPtrField, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.StringPtrField }))...) + + // field Struct.IntField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredValue(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("intField"), &obj.IntField, safe.Field(oldObj, func(oldObj *Struct) *int { return &oldObj.IntField }))...) + + // field Struct.IntPtrField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("intPtrField"), obj.IntPtrField, safe.Field(oldObj, func(oldObj *Struct) *int { return oldObj.IntPtrField }))...) + + // field Struct.BoolField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *bool) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredValue(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("boolField"), &obj.BoolField, safe.Field(oldObj, func(oldObj *Struct) *bool { return &oldObj.BoolField }))...) + + // field Struct.BoolPtrField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *bool) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("boolPtrField"), obj.BoolPtrField, safe.Field(oldObj, func(oldObj *Struct) *bool { return oldObj.BoolPtrField }))...) + + return errs +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc.go new file mode 100644 index 00000000000..71d0a5e7c1d --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc.go @@ -0,0 +1,53 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// +k8s:validation-gen=TypeMeta +// +k8s:validation-gen-scheme-registry=k8s.io/code-generator/cmd/validation-gen/testscheme.Scheme + +// This is a test package. +package zerodefaults + +import "k8s.io/code-generator/cmd/validation-gen/testscheme" + +var localSchemeBuilder = testscheme.New() + +type Struct struct { + TypeMeta int + + // +k8s:optional + // +default="" + StringField string `json:"stringField"` + + // +k8s:optional + // +default="" + StringPtrField *string `json:"stringPtrField"` + + // +k8s:optional + // +default=0 + IntField int `json:"intField"` + + // +k8s:optional + // +default=0 + IntPtrField *int `json:"intPtrField"` + + // +k8s:optional + // +default=false + BoolField bool `json:"boolField"` + + // +k8s:optional + // +default=false + BoolPtrField *bool `json:"boolPtrField"` +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc_test.go new file mode 100644 index 00000000000..1a09a9418e0 --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/doc_test.go @@ -0,0 +1,47 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package zerodefaults + +import ( + "testing" + + "k8s.io/utils/ptr" +) + +func Test(t *testing.T) { + st := localSchemeBuilder.Test(t) + + st.Value(&Struct{ + // All zero-values. + }).ExpectRegexpsByPath(map[string][]string{ + // "stringField": optional value fields with zero defaults are just docs + // "intField": optional value fields with zero defaults are just docs + // "boolField": optional value fields with zero defaults are just docs + "stringPtrField": {"Required value"}, + "intPtrField": {"Required value"}, + "boolPtrField": {"Required value"}, + }) + + st.Value(&Struct{ + StringField: "abc", + StringPtrField: ptr.To(""), + IntField: 123, + IntPtrField: ptr.To(0), + BoolField: true, + BoolPtrField: ptr.To(false), + }).ExpectValid() +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/zz_generated.validations.go new file mode 100644 index 00000000000..54dbc72560c --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/optional/zero_defaults/zz_generated.validations.go @@ -0,0 +1,107 @@ +//go:build !ignore_autogenerated +// +build !ignore_autogenerated + +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Code generated by validation-gen. DO NOT EDIT. + +package zerodefaults + +import ( + context "context" + fmt "fmt" + + operation "k8s.io/apimachinery/pkg/api/operation" + safe "k8s.io/apimachinery/pkg/api/safe" + validate "k8s.io/apimachinery/pkg/api/validate" + field "k8s.io/apimachinery/pkg/util/validation/field" + testscheme "k8s.io/code-generator/cmd/validation-gen/testscheme" +) + +func init() { localSchemeBuilder.Register(RegisterValidations) } + +// RegisterValidations adds validation functions to the given scheme. +// Public to allow building arbitrary schemes. +func RegisterValidations(scheme *testscheme.Scheme) error { + scheme.AddValidationFunc((*Struct)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}, subresources ...string) field.ErrorList { + if len(subresources) == 0 { + return Validate_Struct(ctx, op, nil /* fldPath */, obj.(*Struct), safe.Cast[*Struct](oldObj)) + } + return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresources: %v", obj, subresources))} + }) + return nil +} + +func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *Struct) (errs field.ErrorList) { + // field Struct.TypeMeta has no validation + + // field Struct.StringField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + // optional value-type fields with zero-value defaults are purely documentation + return + }(fldPath.Child("stringField"), &obj.StringField, safe.Field(oldObj, func(oldObj *Struct) *string { return &oldObj.StringField }))...) + + // field Struct.StringPtrField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("stringPtrField"), obj.StringPtrField, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.StringPtrField }))...) + + // field Struct.IntField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + // optional value-type fields with zero-value defaults are purely documentation + return + }(fldPath.Child("intField"), &obj.IntField, safe.Field(oldObj, func(oldObj *Struct) *int { return &oldObj.IntField }))...) + + // field Struct.IntPtrField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("intPtrField"), obj.IntPtrField, safe.Field(oldObj, func(oldObj *Struct) *int { return oldObj.IntPtrField }))...) + + // field Struct.BoolField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *bool) (errs field.ErrorList) { + // optional value-type fields with zero-value defaults are purely documentation + return + }(fldPath.Child("boolField"), &obj.BoolField, safe.Field(oldObj, func(oldObj *Struct) *bool { return &oldObj.BoolField }))...) + + // field Struct.BoolPtrField + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *bool) (errs field.ErrorList) { + // optional fields with default values are effectively required + if e := validate.RequiredPointer(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } + return + }(fldPath.Child("boolPtrField"), obj.BoolPtrField, safe.Field(oldObj, func(oldObj *Struct) *bool { return oldObj.BoolPtrField }))...) + + return errs +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go index 866960c0901..54d7fd44a05 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/required.go @@ -17,16 +17,21 @@ limitations under the License. package validators import ( + "encoding/json" "fmt" + "reflect" + + "k8s.io/gengo/v2" + "k8s.io/gengo/v2/types" "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/gengo/v2/types" ) const ( requiredTagName = "k8s:required" optionalTagName = "k8s:optional" forbiddenTagName = "k8s:forbidden" + defaultTagName = "default" // TODO: this should evenually be +k8s:default ) func init() { @@ -84,7 +89,7 @@ var ( // TODO: It might be valuable to have a string payload for when requiredness is // conditional (e.g. required when is specified). -func (requirednessTagValidator) doRequired(context Context) (Validations, error) { +func (rtv requirednessTagValidator) doRequired(context Context) (Validations, error) { // Most validators don't care whether the value they are validating was // originally defined as a value-type or a pointer-type in the API. This // one does. Since Go doesn't do partial specialization of templates, we @@ -114,7 +119,48 @@ var ( optionalMapValidator = types.Name{Package: libValidationPkg, Name: "OptionalMap"} ) -func (requirednessTagValidator) doOptional(context Context) (Validations, error) { +func (rtv requirednessTagValidator) doOptional(context Context) (Validations, error) { + // All of our tags are expressed from the perspective of a client of the + // API, but the code we generate is for the server. Optional is tricky. + // + // A field which is marked as optional and does not have a default is + // strictly optional. A client is allowed to not set it and the server will + // not give it a default value. Code which consumes it must handle that it + // might not have any value at all. + // + // A field which is marked as optional but has a default is optional to + // clients, but required to the server. A client is allowed to not set it + // but the server will give it a default value. Code which consumes it can + // assume that it always has a value. + // + // One special case must be handled: optional non-pointer fields with + // default values. If the default is not the zero value for the type, then + // the zero value is used to decide whether to assign the default value, + // and so must be out of bounds; we can proceed as above. + // + // But if the default is the zero value, then the zero value is obviously + // valid, and the fact that the field is optional is meaningless - there is + // no way to tell the difference between a client not setting it (yielding + // the zero value) and a client setting it to the zero value. + // + // TODO: handle default=ref(...) + // TODO: handle manual defaulting + if hasDefault, zeroDefault, err := rtv.hasZeroDefault(context); err != nil { + return Validations{}, err + } else if hasDefault { + if !isNilableType(context.Type) && zeroDefault { + return Validations{Comments: []string{"optional value-type fields with zero-value defaults are purely documentation"}}, nil + } + validations, err := rtv.doRequired(context) + if err != nil { + return Validations{}, err + } + for i, fn := range validations.Functions { + validations.Functions[i] = WithComment(fn, "optional fields with default values are effectively required") + } + return validations, nil + } + // Most validators don't care whether the value they are validating was // originally defined as a value-type or a pointer-type in the API. This // one does. Since Go doesn't do partial specialization of templates, we @@ -137,6 +183,71 @@ func (requirednessTagValidator) doOptional(context Context) (Validations, error) return Validations{Functions: []FunctionGen{Function(optionalTagName, ShortCircuit|NonError, optionalValueValidator)}}, nil } +// hasZeroDefault returns whether the field has a default value and whether +// that default value is the zero value for the field's type. +func (rtv requirednessTagValidator) hasZeroDefault(context Context) (bool, bool, error) { + t := realType(context.Type) + // This validator only applies to fields, so Member must be valid. + tagsByName, err := gengo.ExtractFunctionStyleCommentTags("+", []string{defaultTagName}, context.Member.CommentLines) + if err != nil { + return false, false, fmt.Errorf("failed to read tags: %w", err) + } + + tags, hasDefault := tagsByName[defaultTagName] + if !hasDefault { + return false, false, nil + } + if len(tags) == 0 { + return false, false, fmt.Errorf("+default tag with no value") + } + if len(tags) > 1 { + return false, false, fmt.Errorf("+default tag with multiple values: %q", tags) + } + + payload := tags[0].Value + var defaultValue any + if err := json.Unmarshal([]byte(payload), &defaultValue); err != nil { + return false, false, fmt.Errorf("failed to parse default value %q: %w", payload, err) + } + if defaultValue == nil { + return false, false, fmt.Errorf("failed to parse default value %q: unmarshalled to nil", payload) + } + + zero, found := typeZeroValue[t.String()] + if !found { + return false, false, fmt.Errorf("unknown zero-value for type %s", t.String()) + } + + return true, reflect.DeepEqual(defaultValue, zero), nil +} + +// This is copied from defaulter-gen. +// TODO: move this back to gengo as Type.ZeroValue()? +var typeZeroValue = map[string]any{ + "uint": 0., + "uint8": 0., + "uint16": 0., + "uint32": 0., + "uint64": 0., + "int": 0., + "int8": 0., + "int16": 0., + "int32": 0., + "int64": 0., + "byte": 0., + "float64": 0., + "float32": 0., + "bool": false, + "time.Time": "", + "string": "", + "integer": 0., + "number": 0., + "boolean": false, + "[]byte": "", // base64 encoded characters + "interface{}": interface{}(nil), + "any": interface{}(nil), +} + var ( forbiddenValueValidator = types.Name{Package: libValidationPkg, Name: "ForbiddenValue"} forbiddenPointerValidator = types.Name{Package: libValidationPkg, Name: "ForbiddenPointer"} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go index 81f54f80e7b..5d460ea5f6b 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/validators.go @@ -17,10 +17,11 @@ limitations under the License. package validators import ( - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/gengo/v2/generator" "k8s.io/gengo/v2/types" + + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" ) // TagValidator describes a single validation tag and how to use it.