diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go index bd764ab701..e5bf7e15f5 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 != "*" @@ -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. 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/internal/store/builder.go b/internal/store/builder.go index 5c38857d7b..f4dfaf35ae 100644 --- a/internal/store/builder.go +++ b/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() diff --git a/pkg/customresourcestate/config.go b/pkg/customresourcestate/config.go index 873591372f..a123f22ccc 100644 --- a/pkg/customresourcestate/config.go +++ b/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) } 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/pkg/util/utils.go b/pkg/util/utils.go index 63e5aba650..285a8aa8c4 100644 --- a/pkg/util/utils.go +++ b/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 } 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] ` }