fix: support empty groups again, e.g. Node

PR#1851 introduced accidental breakage for custom metrics on resources
with an empty empty, e.g. Node.

Fix it, and add tests to catch the next breakage.

```
demo_timestamp{customresource_group="",customresource_kind="Node",customresource_version="v1",node="kind-control-plane"} 1.719618433e+09
```

Reference: #1851
Reference: #2434
Signed-off-by: Robin H. Johnson <rjohnson@coreweave.com>

internal/discovery/discovery.go

@@ -120,8 +120,8 @@ func (r *CRDiscoverer) ResolveGVKToGVKPs(gvk schema.GroupVersionKind) (resolvedG
 	g := gvk.Group
 	v := gvk.Version
 	k := gvk.Kind
-	if g == "" || g == "*" {
-		return nil, fmt.Errorf("group is required in the defined GVK %v", gvk)
+	if g == "*" {
+		return nil, fmt.Errorf("non-wildcard group is required in the defined GVK %v", gvk)
 	}
 	hasVersion := v != "" && v != "*"
 	hasKind := k != "" && k != "*"
@@ -216,7 +216,11 @@ func (r *CRDiscoverer) PollForCacheUpdates(
 		// Update the list of enabled custom resources.
 		var enabledCustomResources []string
 		for _, factory := range customFactories {
-			gvrString := util.GVRFromType(factory.Name(), factory.ExpectedType()).String()
+			gvr, err := util.GVRFromType(factory.Name(), factory.ExpectedType())
+			if err != nil {
+				klog.ErrorS(err, "invalid type")
+			}
+			gvrString := gvr.String()
 			enabledCustomResources = append(enabledCustomResources, gvrString)
 		}
 		// Create clients for discovered factories.

internal/discovery/discovery_test.go

@@ -164,6 +164,28 @@ func TestGVKMapsResolveGVK(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "fixed version and kind, empty group",
+			gvkmaps: &CRDiscoverer{
+				Map: map[string]map[string][]kindPlural{
+					"": {
+						"v1": {
+							kindPlural{
+								Kind:   "Node",
+								Plural: "nodes",
+							},
+						},
+					},
+				},
+			},
+			gvk: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"},
+			want: []groupVersionKindPlural{
+				{
+					GroupVersionKind: schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"},
+					Plural:           "nodes",
+				},
+			},
+		},
 	}
 	for _, tc := range testcases {
 		got, err := tc.gvkmaps.ResolveGVKToGVKPs(tc.gvk)

internal/store/builder.go

@@ -197,7 +197,10 @@ func (b *Builder) DefaultGenerateCustomResourceStoresFunc() ksmtypes.BuildCustom
 func (b *Builder) WithCustomResourceStoreFactories(fs ...customresource.RegistryFactory) {
 	for i := range fs {
 		f := fs[i]
-		gvr := util.GVRFromType(f.Name(), f.ExpectedType())
+		gvr, err := util.GVRFromType(f.Name(), f.ExpectedType())
+		if err != nil {
+			klog.ErrorS(err, "invalid type")
+		}
 		var gvrString string
 		if gvr != nil {
 			gvrString = gvr.String()
@@ -553,7 +556,10 @@ func (b *Builder) buildCustomResourceStores(resourceName string,
 
 	familyHeaders := generator.ExtractMetricFamilyHeaders(metricFamilies)
 
-	gvr := util.GVRFromType(resourceName, expectedType)
+	gvr, err := util.GVRFromType(resourceName, expectedType)
+	if err != nil {
+		klog.ErrorS(err, "invalid type")
+	}
 	var gvrString string
 	if gvr != nil {
 		gvrString = gvr.String()

pkg/customresourcestate/config.go

@@ -204,7 +204,11 @@ func FromConfig(decoder ConfigDecoder, discovererInstance *discovery.CRDiscovere
 			if err != nil {
 				return nil, fmt.Errorf("failed to create metrics factory for %s: %w", resource.GroupVersionKind, err)
 			}
-			gvrString := util.GVRFromType(factory.Name(), factory.ExpectedType()).String()
+			gvr, err := util.GVRFromType(factory.Name(), factory.ExpectedType())
+			if err != nil {
+				klog.ErrorS(err, "invalid type")
+			}
+			gvrString := gvr.String()
 			if _, ok := factoriesIndex[gvrString]; ok {
 				klog.InfoS("reloaded factory", "GVR", gvrString)
 			}

pkg/customresourcestate/custom_resource_metrics_test.go

@@ -161,6 +161,69 @@ func TestNewCustomResourceMetrics(t *testing.T) {
 				},
 			},
 		},
+		{
+			// https://github.com/kubernetes/kube-state-metrics/issues/2434
+			name: "cr metric with dynamic metric type, empty group",
+			r: Resource{
+				GroupVersionKind: GroupVersionKind{
+					Group:   "",
+					Version: "v1",
+					Kind:    "Node",
+				},
+				Labels: Labels{
+					LabelsFromPath: map[string][]string{
+						"name": {"metadata", "name"},
+					},
+					CommonLabels: map[string]string{
+						"hello": "world",
+					},
+				},
+				Metrics: []Generator{
+					{
+						Name: "test_metrics",
+						Help: "metrics for testing",
+						Each: Metric{
+							Type: metric.Info,
+							Info: &MetricInfo{
+								MetricMeta: MetricMeta{
+									Path: []string{
+										"metadata",
+										"annotations",
+									},
+								},
+								LabelFromKey: "test",
+							},
+						},
+					},
+				},
+			},
+			wantErr: false,
+			wantResult: &customResourceMetrics{
+				MetricNamePrefix: "kube_customresource",
+				GroupVersionKind: schema.GroupVersionKind{
+					Group:   "",
+					Version: "v1",
+					Kind:    "Node",
+				},
+				ResourceName: "nodes",
+				Families: []compiledFamily{
+					{
+						Name: "kube_customresource_test_metrics",
+						Help: "metrics for testing",
+						Each: &compiledInfo{},
+						Labels: map[string]string{
+							"customresource_group":   "",
+							"customresource_kind":    "Node",
+							"customresource_version": "v1",
+							"hello":                  "world",
+						},
+						LabelFromPath: map[string]valuePath{
+							"name": mustCompilePath(t, "metadata", "name"),
+						},
+					},
+				},
+			},
+		},
 		{
 			name: "cr metric with custom prefix - expect error",
 			r: Resource{

pkg/util/utils.go

@@ -17,6 +17,7 @@ limitations under the License.
 package util
 
 import (
+	"errors"
 	"fmt"
 	"runtime"
 	"strings"
@@ -95,7 +96,11 @@ func CreateCustomResourceClients(apiserver string, kubeconfig string, factories
 		if err != nil {
 			return nil, err
 		}
-		gvrString := GVRFromType(f.Name(), f.ExpectedType()).String()
+		gvr, err := GVRFromType(f.Name(), f.ExpectedType())
+		if err != nil {
+			klog.ErrorS(err, "invalid type")
+		}
+		gvrString := gvr.String()
 		customResourceClients[gvrString] = customResourceClient
 	}
 	return customResourceClients, nil
@@ -135,22 +140,32 @@ func CreateDynamicClient(apiserver string, kubeconfig string) (*dynamic.DynamicC
 }
 
 // GVRFromType returns the GroupVersionResource for a given type.
-func GVRFromType(resourceName string, expectedType interface{}) *schema.GroupVersionResource {
+func GVRFromType(resourceName string, expectedType interface{}) (*schema.GroupVersionResource, error) {
 	if _, ok := expectedType.(*testUnstructuredMock.Foo); ok {
 		// testUnstructuredMock.Foo is a mock type for testing
-		return nil
+		return nil, nil
 	}
 	apiVersion := expectedType.(*unstructured.Unstructured).Object["apiVersion"].(string)
 	expectedTypeSlice := strings.Split(apiVersion, "/")
-	g := expectedTypeSlice[0]
-	v := expectedTypeSlice[1]
-	if v == "" /* "" group (core) objects */ {
-		v = expectedTypeSlice[0]
+	var g string
+	var v string
+	switch len(expectedTypeSlice) {
+	case 1: // apiVersion = /v1
+		g = ""
+		v = apiVersion
+	case 2: // apiVersion = apps/v1
+		g = expectedTypeSlice[0]
+		v = expectedTypeSlice[1]
+		if v == "" /* "" group (core) objects */ {
+			v = expectedTypeSlice[0]
+		}
+	default:
+		return nil, errors.New("Expected resourceName to have exactly one slash")
 	}
 	r := resourceName
 	return &schema.GroupVersionResource{
 		Group:    g,
 		Version:  v,
 		Resource: r,
-	}
+	}, nil
 }

tests/e2e/discovery_test.go

@@ -263,5 +263,20 @@ spec:
               path: [metadata]
               labelsFromPath:
                 name: [name]
+    - groupVersionKind:
+        group: ""
+        version: v1
+        kind: Node
+      metricNamePrefix: demo
+      metrics:
+        - name: timestamp
+          help: "example custom metric from non-custom resource"
+          each:
+            type: Gauge
+            gauge:
+              path: [metadata]
+              labelsFromPath:
+                node: [name]
+              valueFrom: [creationTimestamp]
 `
 }