From 7ad89e19971771bbed6ceffbe7f826690866ef58 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Sat, 12 Jul 2025 04:06:55 +0000 Subject: [PATCH] feat(validation-gen): add FieldValidator - Added FieldValidator interface to allow validation on field definitions. - Implemented registration and initialization for field validators in the registry. - Updated validation extraction logic to process field validators after tag and type validators. - Improved error handling and validation checks in the validation generation process. Co-authored-by: Tim Hockin --- .../cmd/validation-gen/validation.go | 9 ++- .../cmd/validation-gen/validators/registry.go | 50 ++++++++++++-- .../validation-gen/validators/validators.go | 65 ++++++++++++++++++- 3 files changed, 113 insertions(+), 11 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 e796ddc6e22..837eb7dcd05 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 @@ -392,6 +392,9 @@ func (td *typeDiscoverer) discoverType(t *types.Type, fldPath *field.Path) (*typ // are called in emitted code, just how we evaluate what to emit. switch t.Kind { case types.Alias, types.Struct: + if fldPath.String() != t.String() { + panic(fmt.Sprintf("path for type != the type name: %s, %s", t.String(), fldPath.String())) + } context := validators.Context{ Scope: validators.ScopeType, Type: t, @@ -452,7 +455,7 @@ func (td *typeDiscoverer) discoverType(t *types.Type, fldPath *field.Path) (*typ // // Note: the first argument to Function() is really // only for debugging. - v, err := validators.ForEachVal(fldPath, underlying.childType, + v, err := validators.ForEachVal(fldPath, thisNode.valueType, validators.Function("iterateListValues", validators.DefaultFlags, funcName)) if err != nil { return nil, fmt.Errorf("generating list iteration: %w", err) @@ -576,11 +579,11 @@ func (td *typeDiscoverer) discoverStruct(thisNode *typeNode, fldPath *field.Path Path: childPath, } - Tags, err := td.validator.ExtractTags(context, memb.CommentLines) + tags, err := td.validator.ExtractTags(context, memb.CommentLines) if err != nil { return fmt.Errorf("field %s: %w", childPath.String(), err) } - if validations, err := td.validator.ExtractValidations(context, Tags...); err != nil { + if validations, err := td.validator.ExtractValidations(context, tags...); err != nil { return fmt.Errorf("field %s: %w", childPath.String(), err) } else if validations.Empty() { klog.V(6).InfoS("no field-attached validations", "field", childPath) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/registry.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/registry.go index 228e3b3174e..779ad82eb4f 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/registry.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/registry.go @@ -42,7 +42,8 @@ type registry struct { tagValidators map[string]TagValidator // keyed by tagname tagIndex []string // all tag names - typeValidators []TypeValidator + typeValidators []TypeValidator + fieldValidators []FieldValidator } func (reg *registry) addTagValidator(tv TagValidator) { @@ -71,6 +72,17 @@ func (reg *registry) addTypeValidator(tv TypeValidator) { globalRegistry.typeValidators = append(globalRegistry.typeValidators, tv) } +func (reg *registry) addFieldValidator(fv FieldValidator) { + if reg.initialized.Load() { + panic("registry was modified after init") + } + + reg.lock.Lock() + defer reg.lock.Unlock() + + globalRegistry.fieldValidators = append(globalRegistry.fieldValidators, fv) +} + func (reg *registry) init(c *generator.Context) { if reg.initialized.Load() { panic("registry.init() was called twice") @@ -97,6 +109,13 @@ func (reg *registry) init(c *generator.Context) { return cmp.Compare(a.Name(), b.Name()) }) + for _, fv := range reg.fieldValidators { + fv.Init(cfg) + } + slices.SortFunc(reg.fieldValidators, func(a, b FieldValidator) int { + return cmp.Compare(a.Name(), b.Name()) + }) + reg.initialized.Store(true) } @@ -127,6 +146,8 @@ func (reg *registry) ExtractValidations(context Context, tags ...codetags.Tag) ( panic("registry.init() was not called") } validations := Validations{} + + // Run tag-validators first. phases := reg.sortTagsIntoPhases(tags) for _, tags := range phases { for _, tag := range tags { @@ -144,6 +165,7 @@ func (reg *registry) ExtractValidations(context Context, tags ...codetags.Tag) ( } } } + // Run type-validators after tag validators are done. if context.Scope == ScopeType { // Run all type-validators. @@ -156,6 +178,18 @@ func (reg *registry) ExtractValidations(context Context, tags ...codetags.Tag) ( } } + // Run field-validators after tag and type validators are done. + if context.Scope == ScopeField { + // Run all field-validators. + for _, fv := range reg.fieldValidators { + if theseValidations, err := fv.GetValidations(context); err != nil { + return Validations{}, fmt.Errorf("field validator %q: %w", fv.Name(), err) + } else { + validations.Add(theseValidations) + } + } + } + return validations, nil } @@ -213,18 +247,24 @@ func (reg *registry) Docs() []TagDoc { return result } -// RegisterTagValidator must be called by any validator which wants to run when -// a specific tag is found. +// RegisterTagValidator must be called for TagValidator to be used by +// validation-gen. See TagValidator for more information. func RegisterTagValidator(tv TagValidator) { globalRegistry.addTagValidator(tv) } -// RegisterTypeValidator must be called by any validator which wants to run -// against every type definition. +// RegisterTypeValidator must be called for a TypeValidator to be used by +// validation-gen. See TypeValidator for more information. func RegisterTypeValidator(tv TypeValidator) { globalRegistry.addTypeValidator(tv) } +// RegisterFieldValidator must be called for a FieldValidator to be used by +// validation-gen. See FieldValidator for more information. +func RegisterFieldValidator(fv FieldValidator) { + globalRegistry.addFieldValidator(fv) +} + // Validator represents an aggregation of validator plugins. type Validator interface { 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 012926ec178..60bed17b17a 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 @@ -26,7 +26,21 @@ import ( "k8s.io/gengo/v2/types" ) -// TagValidator describes a single validation tag and how to use it. +// TagValidator describes a single validation tag and how to use it. To be +// findable by validation-gen, a TagValidator must be registered - see +// RegisterTagValidator. +// +// TagValidators are always evaluated before TypeValidators and +// FieldValidators. In general, TagValidators should not depend on other +// TagValidators having been run already because users might specify tags in +// the any order. The one exception to this rule is that some TagValidators may +// be designated as "late" validators (see LateTagValidator), which means they +// will be run after all non-late TagValidators. +// +// No other guarantees are made about the order of execution of TagValidators +// or LateTagValidators. Instead of relying on tag ordering, TagValidators can +// accumulate information internally and use a TypeValidator and/or +// FieldValidator to finish the work. type TagValidator interface { // Init initializes the implementation. This will be called exactly once. Init(cfg Config) @@ -53,6 +67,16 @@ type LateTagValidator interface { } // TypeValidator describes a validator which runs on every type definition. +// To be findable by validation-gen, a TypeValidator must be registered - see +// RegisterTypeValidator. +// +// TypeValidators are always processed after TagValidators, and after the type +// has been fully processed (including all child fields and their types). This +// means that they can "finish" work with data that was collected by +// TagValidators. +// +// TypeValidators MUST NOT depend on other TypeValidators having been run +// already. type TypeValidator interface { // Init initializes the implementation. This will be called exactly once. Init(cfg Config) @@ -73,6 +97,37 @@ type TypeValidator interface { GetValidations(context Context) (Validations, error) } +// FieldValidator describes a validator which runs on every field definition. +// To be findable by validation-gen, a FieldValidator must be registered - see +// RegisterFieldValidator. +// +// FieldValidators are always processed after TagValidators and TypeValidators, +// and after the field has been fully processed (including all child fields). +// This means that they can "finish" work with data that was collected by +// TagValidators. +// +// FieldValidators MUST NOT depend on other FieldValidators having been run +// already. +type FieldValidator interface { + // Init initializes the implementation. This will be called exactly once. + Init(cfg Config) + + // Name returns a unique name for this validator. This is used for sorting + // and logging. + Name() string + + // GetValidations returns any validations imposed by this validator for the + // given context. + // + // The way gengo handles type definitions varies between structs and other + // types. For struct definitions (e.g. `type Foo struct {}`), the realType + // is the struct itself (the Kind field will be `types.Struct`) and the + // parentType will be nil. For other types (e.g. `type Bar string`), the + // realType will be the underlying type and the parentType will be the + // newly defined type (the Kind field will be `types.Alias`). + GetValidations(context Context) (Validations, error) +} + // Config carries optional configuration information for use by validators. type Config struct { // GengoContext provides gengo's generator Context. This allows validators @@ -182,6 +237,8 @@ type TagDoc struct { PayloadsType codetags.ValueType // PayloadsRequired is true if a payload is required. PayloadsRequired bool + // AcceptsUnknownArgs is true if unknown args are accepted + AcceptsUnknownArgs bool } func (td TagDoc) Arg(name string) (TagArgDoc, bool) { @@ -476,14 +533,16 @@ func typeCheck(tag codetags.Tag, doc TagDoc) error { for _, tagArg := range tag.Args { if _, ok := doc.Arg(tagArg.Name); !ok { - return fmt.Errorf("unrecognized named argument %q", tagArg) + if !doc.AcceptsUnknownArgs { + return fmt.Errorf("unrecognized named argument %q", tagArg) + } } } if tag.ValueType == codetags.ValueTypeNone { if doc.PayloadsRequired { return fmt.Errorf("missing required tag value of type %s", doc.PayloadsType) } - } else if tag.ValueType != doc.PayloadsType { + } else if doc.PayloadsType != codetags.ValueTypeRaw && tag.ValueType != doc.PayloadsType { return fmt.Errorf("tag value has wrong type: got %s, want %s", tag.ValueType, doc.PayloadsType) } return nil