From 286c04397c53501d23f71f72ab69a5e329f9cbe3 Mon Sep 17 00:00:00 2001
From: "Robin H. Johnson" <rjohnson@coreweave.com>
Date: Fri, 28 Jun 2024 13:55:52 -0700
Subject: [PATCH] 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.

Reference: https://github.com/kubernetes/kube-state-metrics/pull/1851
Reference: https://github.com/kubernetes/kube-state-metrics/issues/2434
Signed-off-by: Robin H. Johnson <rjohnson@coreweave.com>
---
 internal/discovery/discovery.go               |  4 +-
 internal/discovery/discovery_test.go          | 22 +++++++
 .../custom_resource_metrics_test.go           | 63 +++++++++++++++++++
 tests/e2e/discovery_test.go                   | 15 +++++
 4 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go
index bd764ab701..81a35ef826 100644
--- a/internal/discovery/discovery.go
+++ b/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 != "*"
diff --git a/internal/discovery/discovery_test.go b/internal/discovery/discovery_test.go
index 39d81aebe2..870d9b023f 100644
--- a/internal/discovery/discovery_test.go
+++ b/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)
diff --git a/pkg/customresourcestate/custom_resource_metrics_test.go b/pkg/customresourcestate/custom_resource_metrics_test.go
index 31121de75c..6db8618b49 100644
--- a/pkg/customresourcestate/custom_resource_metrics_test.go
+++ b/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{
diff --git a/tests/e2e/discovery_test.go b/tests/e2e/discovery_test.go
index d34c932daf..c041776ecc 100644
--- a/tests/e2e/discovery_test.go
+++ b/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]
 `
 }