Merge pull request #2553 from L3n41c/fix_gvrfromtype

fix: panic in `util.GVRFromType` for structured types
kubernetes · Jan 29, 2025 · c7caee6 · c7caee6
2 parents b6305cb + c3b1afe
commit c7caee6
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 10 deletions.
diff --git a/internal/discovery/discovery.go b/internal/discovery/discovery.go
@@ -209,7 +209,16 @@ 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, "failed to update custom resource stores")
+			}
+			var gvrString string
+			if gvr != nil {
+				gvrString = gvr.String()
+			} else {
+				gvrString = factory.Name()
+			}
 			enabledCustomResources = append(enabledCustomResources, gvrString)
 		}
 		// Create clients for discovered factories.

diff --git a/internal/store/builder.go b/internal/store/builder.go
@@ -195,7 +195,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, "Failed to get GVR from type", "resourceName", f.Name(), "expectedType", f.ExpectedType())
+		}
 		var gvrString string
 		if gvr != nil {
 			gvrString = gvr.String()
@@ -551,7 +554,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, "Failed to get GVR from type", "resourceName", resourceName, "expectedType", expectedType)
+	}
 	var gvrString string
 	if gvr != nil {
 		gvrString = gvr.String()

diff --git a/pkg/customresourcestate/config.go b/pkg/customresourcestate/config.go
@@ -205,7 +205,16 @@ 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 {
+				return nil, fmt.Errorf("failed to create GVR for %s: %w", resource.GroupVersionKind, err)
+			}
+			var gvrString string
+			if gvr != nil {
+				gvrString = gvr.String()
+			} else {
+				gvrString = factory.Name()
+			}
 			if _, ok := factoriesIndex[gvrString]; ok {
 				klog.InfoS("reloaded factory", "GVR", gvrString)
 			}

diff --git a/pkg/util/utils.go b/pkg/util/utils.go
@@ -21,7 +21,7 @@ import (
 	"runtime"
 	"strings"
 
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/discovery"
 	clientset "k8s.io/client-go/kubernetes"
@@ -95,7 +95,16 @@ 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 {
+			return nil, err
+		}
+		var gvrString string
+		if gvr != nil {
+			gvrString = gvr.String()
+		} else {
+			gvrString = f.Name()
+		}
 		customResourceClients[gvrString] = customResourceClient
 	}
 	return customResourceClients, nil
@@ -119,12 +128,16 @@ func CreateDiscoveryClient(apiserver string, kubeconfig string) (*discovery.Disc
 }
 
 // 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)
+	t, err := meta.TypeAccessor(expectedType)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to get type accessor for %T: %w", expectedType, err)
+	}
+	apiVersion := t.GetAPIVersion()
 	g, v, found := strings.Cut(apiVersion, "/")
 	if !found {
 		g = "core"
@@ -135,7 +148,7 @@ func GVRFromType(resourceName string, expectedType interface{}) *schema.GroupVer
 		Group:    g,
 		Version:  v,
 		Resource: r,
-	}
+	}, nil
 }
 
 // GatherAndCount gathers all metrics from the provided Gatherer and counts