From 8ae7171041bb1a24d9436e3f576fced4aa604b79 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Fri, 23 May 2025 19:30:09 -0400 Subject: [PATCH] Simplify subresource matching --- .../pkg/api/operation/operation.go | 26 +----- .../pkg/api/operation/operation_test.go | 92 ------------------- .../supported_resources/issubresource/doc.go | 0 .../issubresource/doc_test.go | 0 .../issubresource/zz_generated.validations.go | 58 ++++++++++++ .../supported_resources/root/doc.go | 0 .../supported_resources/root/doc_test.go | 0 .../root/zz_generated.validations.go | 58 ++++++++++++ .../supported_resources/subresource/doc.go | 0 .../subresource/doc_test.go | 0 .../subresource/zz_generated.validations.go | 58 ++++++++++++ .../cmd/validation-gen/validation.go | 30 +++--- 12 files changed, 194 insertions(+), 128 deletions(-) delete mode 100644 staging/src/k8s.io/apimachinery/pkg/api/operation/operation_test.go rename staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/{ => tags}/supported_resources/issubresource/doc.go (100%) rename staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/{ => tags}/supported_resources/issubresource/doc_test.go (100%) create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/zz_generated.validations.go rename staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/{ => tags}/supported_resources/root/doc.go (100%) rename staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/{ => tags}/supported_resources/root/doc_test.go (100%) create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/zz_generated.validations.go rename staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/{ => tags}/supported_resources/subresource/doc.go (100%) rename staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/{ => tags}/supported_resources/subresource/doc_test.go (100%) create mode 100644 staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/zz_generated.validations.go diff --git a/staging/src/k8s.io/apimachinery/pkg/api/operation/operation.go b/staging/src/k8s.io/apimachinery/pkg/api/operation/operation.go index b54b0fc2f1a..1195b42110c 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/operation/operation.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/operation/operation.go @@ -17,7 +17,6 @@ limitations under the License. package operation import ( - "slices" "strings" "k8s.io/apimachinery/pkg/util/sets" @@ -78,31 +77,12 @@ type Request struct { Subresources []string } -// MatchesSubresource returns true if the request is for the given subresource path. -// The subresource path is a slash-separated list of subresource names. For -// example, `/status`, `/resize`, or `/x/y/z`. -// -// `/` identifies the root resource. That is, MatchesSubresource returns true -// subresourcePath is `/` and Subresources is an empty list. -func (r Request) MatchesSubresource(subresourcePath string) bool { - if len(r.Subresources) == 0 && subresourcePath == "/" { - return true - } - return r.SubresourcePath() == subresourcePath -} - -// SubresourceIn returns true if the request is for a subresource in the given list. -// The subresource path is a slash-separated list of subresource names. For example, -// `/status`, `/resize`, or `/x/y/z`. -// `/` identifies the root resource. That is, SubresourceIn returns true -// subresourcePaths contains `/` and Subresources is an empty list. -func (r Request) SubresourceIn(subresourcePaths []string) bool { - return slices.ContainsFunc(subresourcePaths, r.MatchesSubresource) -} - // SubresourcePath returns the path is a slash-separated list of subresource // names. For example, `/status`, `/resize`, or `/x/y/z`. func (r Request) SubresourcePath() string { + if len(r.Subresources) == 0 { + return "/" + } return "/" + strings.Join(r.Subresources, "/") } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/operation/operation_test.go b/staging/src/k8s.io/apimachinery/pkg/api/operation/operation_test.go deleted file mode 100644 index 77565ce78b7..00000000000 --- a/staging/src/k8s.io/apimachinery/pkg/api/operation/operation_test.go +++ /dev/null @@ -1,92 +0,0 @@ -/* -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 operation - -import "testing" - -func TestRequest_SubresourceIn(t *testing.T) { - tests := []struct { - name string - requestSubresources []string - matchSubresources []string - want bool - }{ - { - name: "subresource match", - requestSubresources: []string{"x"}, - matchSubresources: []string{"/x"}, - want: true, - }, - { - name: "subresource no match", - requestSubresources: []string{"x"}, - matchSubresources: []string{"/y"}, - want: false, - }, - { - name: "root match", - requestSubresources: []string{}, - matchSubresources: []string{"/"}, - want: true, - }, - { - name: "subresource does not match root", - requestSubresources: []string{"x"}, - matchSubresources: []string{"/"}, - want: false, - }, - { - name: "root does not match subresource", - requestSubresources: []string{}, - matchSubresources: []string{"/x"}, - want: false, - }, - { - name: "root matches root and subresource", - requestSubresources: []string{}, - matchSubresources: []string{"/", "/x"}, - want: true, - }, - { - name: "subresource matches root and subresource", - requestSubresources: []string{"x"}, - matchSubresources: []string{"/", "/x"}, - want: true, - }, - { - name: "subresource matches multiple", - requestSubresources: []string{"y"}, - matchSubresources: []string{"/x", "/y"}, - want: true, - }, - { - name: "nested subresource match", - requestSubresources: []string{"x", "y", "z"}, - matchSubresources: []string{"/x/y/z"}, - want: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - r := Request{Subresources: tt.requestSubresources} - if got := r.SubresourceIn(tt.matchSubresources); got != tt.want { - t.Errorf("Request.SubresourceIn() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/issubresource/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/doc.go similarity index 100% rename from staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/issubresource/doc.go rename to staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/doc.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/issubresource/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/doc_test.go similarity index 100% rename from staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/issubresource/doc_test.go rename to staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/doc_test.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/zz_generated.validations.go new file mode 100644 index 00000000000..970340222b7 --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/issubresource/zz_generated.validations.go @@ -0,0 +1,58 @@ +//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 issubresource + +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((*T1)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList { + if op.Request.MatchesSubresource("/scale") { + return Validate_T1(ctx, op, nil /* fldPath */, obj.(*T1), safe.Cast[*T1](oldObj)) + } + return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath()))} + }) + return nil +} + +func Validate_T1(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *T1) (errs field.ErrorList) { + // field T1.S + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, true, "field T1.S")...) + return + }(fldPath.Child("s"), &obj.S, safe.Field(oldObj, func(oldObj *T1) *string { return &oldObj.S }))...) + + return errs +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/root/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/doc.go similarity index 100% rename from staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/root/doc.go rename to staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/doc.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/root/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/doc_test.go similarity index 100% rename from staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/root/doc_test.go rename to staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/doc_test.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/zz_generated.validations.go new file mode 100644 index 00000000000..9d17c10967d --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/root/zz_generated.validations.go @@ -0,0 +1,58 @@ +//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 root + +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((*T1)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList { + if op.Request.MatchesSubresource("/") { + return Validate_T1(ctx, op, nil /* fldPath */, obj.(*T1), safe.Cast[*T1](oldObj)) + } + return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath()))} + }) + return nil +} + +func Validate_T1(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *T1) (errs field.ErrorList) { + // field T1.S + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, true, "field T1.S")...) + return + }(fldPath.Child("s"), &obj.S, safe.Field(oldObj, func(oldObj *T1) *string { return &oldObj.S }))...) + + return errs +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/subresource/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/doc.go similarity index 100% rename from staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/subresource/doc.go rename to staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/doc.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/subresource/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/doc_test.go similarity index 100% rename from staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/supported_resources/subresource/doc_test.go rename to staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/doc_test.go diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/zz_generated.validations.go new file mode 100644 index 00000000000..84d8f683240 --- /dev/null +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/supported_resources/subresource/zz_generated.validations.go @@ -0,0 +1,58 @@ +//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 subresource + +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((*T1)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList { + if op.Request.MatchesSubresource("/") || op.Request.MatchesSubresource("/scale") || op.Request.MatchesSubresource("/status") || op.Request.MatchesSubresource("/x/y") { + return Validate_T1(ctx, op, nil /* fldPath */, obj.(*T1), safe.Cast[*T1](oldObj)) + } + return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath()))} + }) + return nil +} + +func Validate_T1(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *T1) (errs field.ErrorList) { + // field T1.S + errs = append(errs, + func(fldPath *field.Path, obj, oldObj *string) (errs field.ErrorList) { + errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, true, "field T1.S")...) + return + }(fldPath.Child("s"), &obj.S, safe.Field(oldObj, func(oldObj *T1) *string { return &oldObj.S }))...) + + return errs +} 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 0a5a2c3fa60..76828ecaa21 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 @@ -816,30 +816,34 @@ func (g *genValidations) emitRegisterFunction(c *generator.Context, schemeRegist panic(fmt.Sprintf("found nil node for root-type %v", rootType)) } - supportedResources := g.toResourceList(rootType) - targs := generator.Args{ - "rootType": rootType, - "typePfx": "", - "field": mkSymbolArgs(c, fieldPkgSymbols), - "fmt": mkSymbolArgs(c, fmtPkgSymbols), - "operation": mkSymbolArgs(c, operationPkgSymbols), - "safe": mkSymbolArgs(c, safePkgSymbols), - "context": mkSymbolArgs(c, contextPkgSymbols), - "supportsResources": strings.Join(supportedResources, ", "), + "rootType": rootType, + "typePfx": "", + "field": mkSymbolArgs(c, fieldPkgSymbols), + "fmt": mkSymbolArgs(c, fmtPkgSymbols), + "operation": mkSymbolArgs(c, operationPkgSymbols), + "safe": mkSymbolArgs(c, safePkgSymbols), + "context": mkSymbolArgs(c, contextPkgSymbols), } if !isNilableType(rootType) { targs["typePfx"] = "*" } - // TODO: Remove special-casing for `/` and `/scale` resources once ratcheting is introduced. // This uses a typed nil pointer, rather than a real instance because // we need the type information, but not an instance of the type. - sw.Do("$.rootType|private$SupportedResources := []string{$.supportsResources$}\n", targs) sw.Do("scheme.AddValidationFunc(", targs) sw.Do(" ($.typePfx$$.rootType|raw$)(nil), ", targs) sw.Do(" func(ctx $.context.Context$, op $.operation.Operation|raw$, obj, oldObj interface{}) $.field.ErrorList|raw$ {\n", targs) - sw.Do(" if op.Request.SubresourceIn($.rootType|private$SupportedResources) {\n", targs) + + sw.Do("switch op.Request.SubresourcePath() {\n", nil) + sw.Do("case ", nil) + for i, s := range g.toResourceList(rootType) { + if i > 0 { + sw.Do(", ", nil) + } + sw.Do("$.$", s) + } + sw.Do(":\n", nil) sw.Do(" return $.rootType|objectvalidationfn$(", targs) sw.Do(" ctx, ", targs) sw.Do(" op, ", targs)