Add resource validation

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go

@@ -19,7 +19,10 @@ package vpa
 import (
 	"encoding/json"
 	"fmt"
+
 	v1 "k8s.io/api/admission/v1"
+	corev1 "k8s.io/api/core/v1"
+	apires "k8s.io/apimachinery/pkg/api/resource"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource"
@@ -133,11 +136,20 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
 				}
 			}
 			for resource, min := range policy.MinAllowed {
+				if err := validateResourceResolution(resource, min); err != nil {
+					return fmt.Errorf("MinAllowed: %v", err)
+				}
 				max, found := policy.MaxAllowed[resource]
 				if found && max.Cmp(min) < 0 {
 					return fmt.Errorf("max resource for %v is lower than min", resource)
 				}
 			}
+
+			for resource, max := range policy.MaxAllowed {
+				if err := validateResourceResolution(resource, max); err != nil {
+					return fmt.Errorf("MaxAllowed: %v", err)
+				}
+			}
 			ControlledValues := policy.ControlledValues
 			if mode != nil && ControlledValues != nil {
 				if *mode == vpa_types.ContainerScalingModeOff && *ControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits {
@@ -157,3 +169,27 @@ func validateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
 
 	return nil
 }
+
+func validateResourceResolution(name corev1.ResourceName, val apires.Quantity) error {
+	switch name {
+	case corev1.ResourceCPU:
+		return validateCPUResolution(val)
+	case corev1.ResourceMemory:
+		return validateMemoryResolution(val)
+	}
+	return nil
+}
+
+func validateCPUResolution(val apires.Quantity) error {
+	if _, precissionPreserved := val.AsScale(apires.Milli); !precissionPreserved {
+		return fmt.Errorf("CPU [%s] must be a whole number of milli CPUs", val.String())
+	}
+	return nil
+}
+
+func validateMemoryResolution(val apires.Quantity) error {
+	if _, precissionPreserved := val.AsScale(0); !precissionPreserved {
+		return fmt.Errorf("Memory [%v] must be a whole number of bytes", val)
+	}
+	return nil
+}

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go

@@ -27,7 +27,8 @@ import (
 )
 
 const (
-	cpu = apiv1.ResourceCPU
+	cpu    = apiv1.ResourceCPU
+	memory = apiv1.ResourceMemory
 )
 
 func TestValidateVPA(t *testing.T) {
@@ -36,6 +37,7 @@ func TestValidateVPA(t *testing.T) {
 	badMinReplicas := int32(0)
 	validMinReplicas := int32(1)
 	badScalingMode := vpa_types.ContainerScalingMode("bad")
+	badCPUResource := resource.MustParse("187500u")
 	validScalingMode := vpa_types.ContainerScalingModeAuto
 	scalingModeOff := vpa_types.ContainerScalingModeOff
 	controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits
@@ -150,6 +152,90 @@ func TestValidateVPA(t *testing.T) {
 			},
 			expectError: fmt.Errorf("max resource for cpu is lower than min"),
 		},
+		{
+			name: "bad minAllowed cpu value",
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					ResourcePolicy: &vpa_types.PodResourcePolicy{
+						ContainerPolicies: []vpa_types.ContainerResourcePolicy{
+							{
+								ContainerName: "loot box",
+								MinAllowed: apiv1.ResourceList{
+									cpu: resource.MustParse("187500u"),
+								},
+								MaxAllowed: apiv1.ResourceList{
+									cpu: resource.MustParse("275m"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expectError: fmt.Errorf("MinAllowed: CPU [%v] must be a whole number of milli CPUs", badCPUResource.String()),
+		},
+		{
+			name: "bad minAllowed memory value",
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					ResourcePolicy: &vpa_types.PodResourcePolicy{
+						ContainerPolicies: []vpa_types.ContainerResourcePolicy{
+							{
+								ContainerName: "loot box",
+								MinAllowed: apiv1.ResourceList{
+									cpu:    resource.MustParse("1m"),
+									memory: resource.MustParse("100m"),
+								},
+								MaxAllowed: apiv1.ResourceList{
+									cpu:    resource.MustParse("275m"),
+									memory: resource.MustParse("500M"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expectError: fmt.Errorf("MinAllowed: Memory [%v] must be a whole number of bytes", resource.MustParse("100m")),
+		},
+		{
+			name: "bad maxAllowed cpu value",
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					ResourcePolicy: &vpa_types.PodResourcePolicy{
+						ContainerPolicies: []vpa_types.ContainerResourcePolicy{
+							{
+								ContainerName: "loot box",
+								MinAllowed:    apiv1.ResourceList{},
+								MaxAllowed: apiv1.ResourceList{
+									cpu: resource.MustParse("187500u"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expectError: fmt.Errorf("MaxAllowed: CPU [%s] must be a whole number of milli CPUs", badCPUResource.String()),
+		},
+		{
+			name: "bad maxAllowed memory value",
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					ResourcePolicy: &vpa_types.PodResourcePolicy{
+						ContainerPolicies: []vpa_types.ContainerResourcePolicy{
+							{
+								ContainerName: "loot box",
+								MinAllowed: apiv1.ResourceList{
+									cpu: resource.MustParse("1m")},
+								MaxAllowed: apiv1.ResourceList{
+									cpu:    resource.MustParse("275m"),
+									memory: resource.MustParse("500m"),
+								},
+							},
+						},
+					},
+				},
+			},
+			expectError: fmt.Errorf("MaxAllowed: Memory [%v] must be a whole number of bytes", resource.MustParse("500m")),
+		},
 		{
 			name: "scaling off with controlled values requests and limits",
 			vpa: vpa_types.VerticalPodAutoscaler{