diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go index 05606c7e4b5..65ffdd4f16d 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -18,6 +18,7 @@ package rest import ( "context" + "errors" "fmt" "slices" "strings" @@ -296,23 +297,26 @@ func panicSafeValidateFunc( } } -func metricIdentifier(ctx context.Context, obj runtime.Object, opType operation.Type) (string, error) { +func metricIdentifier(ctx context.Context, scheme *runtime.Scheme, obj runtime.Object, opType operation.Type) (string, error) { + var errs error var identifier string - var err error identifier = "unknown_resource" // Use kind for identifier. - if obj != nil { - gvk := obj.GetObjectKind().GroupVersionKind() - if gvk.Kind != "" { - identifier = strings.ToLower(gvk.Kind) + if obj != nil && scheme != nil { + gvks, _, err := scheme.ObjectKinds(obj) + if err != nil { + errs = errors.Join(errs, err) + } + if len(gvks) > 0 { + identifier = strings.ToLower(gvks[0].Kind) } } // Use requestInfo for subresource. requestInfo, found := genericapirequest.RequestInfoFrom(ctx) if !found { - err = fmt.Errorf("could not find requestInfo in context") + errs = errors.Join(errs, fmt.Errorf("could not find requestInfo in context")) } else if len(requestInfo.Subresource) > 0 { // subresource can be a path, so replace '/' with '_' identifier += "_" + strings.ReplaceAll(requestInfo.Subresource, "/", "_") @@ -324,12 +328,10 @@ func metricIdentifier(ctx context.Context, obj runtime.Object, opType operation. case operation.Update: identifier += "_update" default: - if err == nil { - err = fmt.Errorf("unknown operation type: %v", opType) - } + errs = errors.Join(errs, fmt.Errorf("unknown operation type: %v", opType)) identifier += "_unknown_op" } - return identifier, err + return identifier, errs } // ValidateDeclarativelyWithMigrationChecks is a helper function that encapsulates the logic for running declarative validation. @@ -342,7 +344,7 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti takeover := utilfeature.DefaultFeatureGate.Enabled(features.DeclarativeValidationTakeover) - validationIdentifier, err := metricIdentifier(ctx, obj, opType) + validationIdentifier, err := metricIdentifier(ctx, scheme, obj, opType) if err != nil { // Log the error, but continue with the best-effort identifier. klog.FromContext(ctx).Error(err, "failed to generate complete validation identifier for declarative validation") diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go index 81a1786abf3..b1218e01918 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go @@ -125,7 +125,7 @@ func TestValidateDeclaratively(t *testing.T) { scheme.AddKnownTypes(internalGV, &Pod{}) scheme.AddKnownTypes(v1GV, &v1.Pod{}) - scheme.AddValidationFunc(&v1.Pod{}, func(ctx context.Context, op operation.Operation, object, oldObject interface{}) field.ErrorList { + scheme.AddValidationFunc(&v1.Pod{}, func(ctx context.Context, op operation.Operation, object, oldObject any) field.ErrorList { results := field.ErrorList{} if op.HasOption("option1") { results = append(results, invalidIfOptionErr) @@ -735,10 +735,14 @@ func equalErrorLists(a, b field.ErrorList) bool { } func TestMetricIdentifier(t *testing.T) { + scheme := runtime.NewScheme() + scheme.AddKnownTypes(schema.GroupVersion{Version: "v1"}, &v1.Pod{}) + testCases := []struct { name string opType operation.Type obj runtime.Object + scheme *runtime.Scheme subresource string expected string expectErr bool @@ -747,6 +751,7 @@ func TestMetricIdentifier(t *testing.T) { name: "with subresource", opType: operation.Create, obj: &v1.Pod{TypeMeta: metav1.TypeMeta{Kind: "Pod"}}, + scheme: scheme, subresource: "status", expected: "pod_status_create", expectErr: false, @@ -755,6 +760,7 @@ func TestMetricIdentifier(t *testing.T) { name: "without subresource", opType: operation.Update, obj: &v1.Pod{TypeMeta: metav1.TypeMeta{Kind: "Pod"}}, + scheme: scheme, expected: "pod_update", expectErr: false, }, @@ -762,6 +768,7 @@ func TestMetricIdentifier(t *testing.T) { name: "unknown operation", opType: 3, // not a valid operation.Type obj: &v1.Pod{TypeMeta: metav1.TypeMeta{Kind: "Pod"}}, + scheme: scheme, expected: "pod_unknown_op", expectErr: true, }, @@ -772,6 +779,29 @@ func TestMetricIdentifier(t *testing.T) { expected: "unknown_resource_create", expectErr: true, }, + { + name: "known type without kind", + opType: operation.Update, + obj: &v1.Pod{}, + scheme: scheme, + expected: "pod_update", + expectErr: false, + }, + { + name: "unknown type with scheme", + opType: operation.Create, + obj: &runtime.Unknown{}, // Not registered in the scheme + scheme: scheme, + expected: "unknown_resource_create", + expectErr: true, + }, + { + name: "unknown type without scheme", + opType: operation.Type(4), + obj: &runtime.Unknown{}, // Not registered in the scheme + expected: "unknown_resource_unknown_op", + expectErr: true, + }, } for _, tc := range testCases { @@ -783,7 +813,7 @@ func TestMetricIdentifier(t *testing.T) { }) } - result, err := metricIdentifier(ctx, tc.obj, tc.opType) + result, err := metricIdentifier(ctx, tc.scheme, tc.obj, tc.opType) if (err != nil) != tc.expectErr { t.Errorf("expected error: %v, got: %v", tc.expectErr, err) }