fix: merge two conversion functions into a single (#52773)

- this fixes the missing handling of `extended` in the
    `ConvertLimitToResourceList`.

  - it also brings over a fix to the mapping from rancher field names to kube
    resource names. one of the improperly mapped fields was `limitsCpu`.  This
    is now properly mapped to `limits.cpu`.

    this fix exposed a bug in the integration tests, where the test was written
    to adhere to the original bogus behaviour. With `limitsCpu` written and
    `limits.cpu` read by the test the actual data to expect was not seen, and 0
    read. And expected for.

    the test in question is now corrected.

note: tech debt against r/webhook

pkg/controllers/managementuser/resourcequota/resource_quota_calculate_used.go

@@ -81,7 +81,7 @@ func (c *calculateLimitController) calculateProjectResourceQuota(projectID strin
 		}
 		nsResourceList, err := validate.ConvertLimitToResourceList(nsLimit)
 		if err != nil {
-			return err
+			return fmt.Errorf("parsing namespace quota limits: %w", err)
 		}
 		nssResourceList = quota.Add(nssResourceList, nsResourceList)
 	}

pkg/controllers/managementuser/resourcequota/resource_quota_common.go

@@ -2,18 +2,18 @@ package resourcequota
 
 import (
 	"encoding/json"
-	"fmt"
 
 	"github.com/rancher/norman/types/convert"
 	apiv3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
 	wmgmtv3 "github.com/rancher/rancher/pkg/generated/controllers/management.cattle.io/v3"
 	"github.com/rancher/rancher/pkg/ref"
+	"github.com/rancher/rancher/pkg/resourcequota"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 )
 
-const extendedKey = "extended"
+const extendedKey = resourcequota.ExtendedKey
 
 func convertResourceListToLimit(rList corev1.ResourceList) (*apiv3.ResourceQuotaLimit, error) {
 	converted, err := convert.EncodeToMap(rList)
@@ -54,50 +54,7 @@ func convertResourceLimitResourceQuotaSpec(limit *apiv3.ResourceQuotaLimit) (*co
 
 // convertProjectResourceLimitToResourceList tries to convert a Rancher-defined resource quota limit to its native Kubernetes notation.
 func convertProjectResourceLimitToResourceList(limit *apiv3.ResourceQuotaLimit) (corev1.ResourceList, error) {
-	in, err := json.Marshal(limit)
-	if err != nil {
-		return nil, err
-	}
-	limitsMap := map[string]any{}
-	err = json.Unmarshal(in, &limitsMap)
-	if err != nil {
-		return nil, err
-	}
-
-	limits := corev1.ResourceList{}
-
-	// convert the arbitrary set first, ...
-	if extended, ok := limitsMap[extendedKey]; ok {
-		delete(limitsMap, extendedKey)
-		for key, value := range extended.(map[string]any) {
-			resourceName := corev1.ResourceName(key)
-			resourceQuantity, err := resource.ParseQuantity(value.(string))
-			if err != nil {
-				return nil, fmt.Errorf("failed to parse value for key %q: %w", key, err)
-			}
-
-			limits[resourceName] = resourceQuantity
-		}
-	}
-
-	// then place the fixed data. this order ensures that in case of
-	// conflicts between arbitrary and fixed data the fixed data wins.
-	for key, value := range limitsMap {
-		var resourceName corev1.ResourceName
-		if val, ok := resourceQuotaConversion[key]; ok {
-			resourceName = corev1.ResourceName(val)
-		} else {
-			resourceName = corev1.ResourceName(key)
-		}
-
-		resourceQuantity, err := resource.ParseQuantity(value.(string))
-		if err != nil {
-			return nil, err
-		}
-
-		limits[resourceName] = resourceQuantity
-	}
-	return limits, nil
+	return resourcequota.ConvertLimitToResourceList(limit)
 }
 
 func convertContainerResourceLimitToResourceList(limit *apiv3.ContainerResourceLimit) (corev1.ResourceList, corev1.ResourceList, error) {
@@ -154,19 +111,7 @@ var limitRangerLimitConversion = map[string]string{
 	"limitsMemory": "memory",
 }
 
-var resourceQuotaConversion = map[string]string{
-	"replicationControllers": "replicationcontrollers",
-	"configMaps":             "configmaps",
-	"persistentVolumeClaims": "persistentvolumeclaims",
-	"servicesNodePorts":      "services.nodeports",
-	"servicesLoadBalancers":  "services.loadbalancers",
-	"requestsCpu":            "requests.cpu",
-	"requestsMemory":         "requests.memory",
-	"requestsStorage":        "requests.storage",
-	"limitsCpu":              "limits.cpu",
-	"limitsMemory":           "limits.memory",
-}
-
+// Also see resourcequota.resourceQuotaConversion for the table governing the forward conversion
 var resourceQuotaReturnConversion = map[string]string{
 	"configmaps":             "configMaps",
 	"limits.cpu":             "limitsCpu",

pkg/resourcequota/quota_validate.go

@@ -1,6 +1,7 @@
 package resourcequota
 
 import (
+	"fmt"
 	"sync"
 	"time"
 
@@ -12,8 +13,22 @@ import (
 	quota "k8s.io/apiserver/pkg/quota/v1"
 )
 
+const ExtendedKey = "extended"
+
 var (
-	projectLockCache = cache.NewLRUExpireCache(1000)
+	projectLockCache        = cache.NewLRUExpireCache(1000)
+	resourceQuotaConversion = map[string]string{
+		"replicationControllers": "replicationcontrollers",
+		"configMaps":             "configmaps",
+		"persistentVolumeClaims": "persistentvolumeclaims",
+		"servicesNodePorts":      "services.nodeports",
+		"servicesLoadBalancers":  "services.loadbalancers",
+		"requestsCpu":            "requests.cpu",
+		"requestsMemory":         "requests.memory",
+		"requestsStorage":        "requests.storage",
+		"limitsCpu":              "limits.cpu",
+		"limitsMemory":           "limits.memory",
+	}
 )
 
 func GetProjectLock(projectID string) *sync.Mutex {
@@ -30,21 +45,21 @@ func IsQuotaFit(nsLimit *v32.ResourceQuotaLimit, nsLimits []*v32.ResourceQuotaLi
 	nssResourceList := api.ResourceList{}
 	nsResourceList, err := ConvertLimitToResourceList(nsLimit)
 	if err != nil {
-		return false, nil, err
+		return false, nil, fmt.Errorf("checking quota fit: %w", err)
 	}
 	nssResourceList = quota.Add(nssResourceList, nsResourceList)
 
 	for _, nsLimit := range nsLimits {
 		nsResourceList, err := ConvertLimitToResourceList(nsLimit)
 		if err != nil {
-			return false, nil, err
+			return false, nil, fmt.Errorf("checking namespace limits: %w", err)
 		}
 		nssResourceList = quota.Add(nssResourceList, nsResourceList)
 	}
 
 	projectResourceList, err := ConvertLimitToResourceList(projectLimit)
 	if err != nil {
-		return false, nil, err
+		return false, nil, fmt.Errorf("checking project limits: %w", err)
 	}
 
 	_, exceeded := quota.LessThanOrEqual(nssResourceList, projectResourceList)
@@ -58,17 +73,44 @@ func IsQuotaFit(nsLimit *v32.ResourceQuotaLimit, nsLimits []*v32.ResourceQuotaLi
 }
 
 func ConvertLimitToResourceList(limit *v32.ResourceQuotaLimit) (api.ResourceList, error) {
+	// TECH DEBT: Any change here has to be reflected in rancher/webhook
+	//   pkg/resources/management.cattle.io/v3/project/quota_validate.go
+	// until such time as both places are unified in a single function shared between r/r and r/w
+
 	toReturn := api.ResourceList{}
 	converted, err := convert.EncodeToMap(limit)
 	if err != nil {
 		return nil, err
 	}
-	for key, value := range converted {
-		q, err := resource.ParseQuantity(convert.ToString(value))
-		if err != nil {
-			return nil, err
+
+	// convert the extended set first, ...
+	if extended, ok := converted[ExtendedKey]; ok {
+		delete(converted, ExtendedKey)
+		for key, value := range extended.(map[string]any) {
+			resourceName := api.ResourceName(key)
+			resourceQuantity, err := resource.ParseQuantity(value.(string))
+			if err != nil {
+				return nil, fmt.Errorf("failed to parse value for key %q: %w", key, err)
+			}
+
+			toReturn[resourceName] = resourceQuantity
 		}
-		toReturn[api.ResourceName(key)] = q
+	}
+
+	// then place the fixed data. this order ensures that in case of
+	// conflicts between arbitrary and fixed data the fixed data wins.
+	for key, value := range converted {
+		var resourceName api.ResourceName
+		if val, ok := resourceQuotaConversion[key]; ok {
+			resourceName = api.ResourceName(val)
+		} else {
+			resourceName = api.ResourceName(key)
+		}
+		resourceQuantity, err := resource.ParseQuantity(convert.ToString(value))
+		if err != nil {
+			return nil, fmt.Errorf("parsing quantity %q: %w", key, err)
+		}
+		toReturn[resourceName] = resourceQuantity
 	}
 	return toReturn, nil
 }

tests/v2/integration/projects/resource_quota_test.go

@@ -149,7 +149,7 @@ func (s *ResourceQuotaSuite) TestCreateNamespaceWithOverriddenQuotaInProject() {
 
 	resourceList = quotas.Items[0].Spec.Hard
 	want = v1.ResourceList{
-		v1.ResourceLimitsCPU: resource.MustParse("0"),
+		v1.ResourceLimitsCPU: resource.MustParse("400m"),
 	}
 	s.Require().Equal(want, resourceList)
 	s.Require().NoError(err)