Skip to content

Commit

Permalink
Migrate Admission Controller Validation to CEL
Browse files Browse the repository at this point in the history
Signed-off-by: Omer Aplatony <[email protected]>
  • Loading branch information
omerap12 committed Jan 14, 2025
1 parent 5cd491a commit 45785a4
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 32 deletions.
13 changes: 13 additions & 0 deletions vertical-pod-autoscaler/deploy/vpa-v1-crd-gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ spec:
required:
- name
type: object
maxItems: 1
type: array
resourcePolicy:
description: |-
Expand All @@ -324,7 +325,11 @@ spec:
Name of the container or DefaultContainerResourcePolicy, in which
case the policy is used by the containers that don't have their own
policy specified.
pattern: ^[a-zA-Z0-9-_]+$
type: string
x-kubernetes-validations:
- message: ContainerName cannot be empty
rule: size(self) > 0
controlledResources:
description: |-
Specifies the type of recommendations that will be computed
Expand Down Expand Up @@ -366,13 +371,20 @@ spec:
for the container. The default is no minimum.
type: object
mode:
default: Auto
description: Whether autoscaler is enabled for the container.
The default is "Auto".
enum:
- Auto
- "Off"
type: string
type: object
x-kubernetes-validations:
- message: ControlledValues shouldn't be specified if container
scaling mode is off
rule: '!has(self.mode) || !has(self.controlledValues) || self.mode
!= ''Off'' || self.controlledValues != ''RequestsAndLimits'''
maxItems: 100
type: array
type: object
targetRef:
Expand Down Expand Up @@ -449,6 +461,7 @@ spec:
pod eviction (pending other checks like PDB). Only positive values are
allowed. Overrides global '--min-replicas' flag.
format: int32
minimum: 1
type: integer
updateMode:
description: |-
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,25 +112,8 @@ func parseVPA(raw []byte) (*vpa_types.VerticalPodAutoscaler, error) {

// ValidateVPA checks the correctness of VPA Spec and returns an error if there is a problem.
func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
if vpa.Spec.UpdatePolicy != nil {
mode := vpa.Spec.UpdatePolicy.UpdateMode
if mode == nil {
return fmt.Errorf("UpdateMode is required if UpdatePolicy is used")
}
if _, found := possibleUpdateModes[*mode]; !found {
return fmt.Errorf("unexpected UpdateMode value %s", *mode)
}

if minReplicas := vpa.Spec.UpdatePolicy.MinReplicas; minReplicas != nil && *minReplicas <= 0 {
return fmt.Errorf("MinReplicas has to be positive, got %v", *minReplicas)
}
}

if vpa.Spec.ResourcePolicy != nil {
for _, policy := range vpa.Spec.ResourcePolicy.ContainerPolicies {
if policy.ContainerName == "" {
return fmt.Errorf("ContainerPolicies.ContainerName is required")
}
mode := policy.Mode
if mode != nil {
if _, found := possibleScalingModes[*mode]; !found {
Expand All @@ -152,23 +135,8 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
return fmt.Errorf("MaxAllowed: %v", err)
}
}
ControlledValues := policy.ControlledValues
if mode != nil && ControlledValues != nil {
if *mode == vpa_types.ContainerScalingModeOff && *ControlledValues == vpa_types.ContainerControlledValuesRequestsAndLimits {
return fmt.Errorf("ControlledValues shouldn't be specified if container scaling mode is off.")
}
}
}
}

if isCreate && vpa.Spec.TargetRef == nil {
return fmt.Errorf("TargetRef is required. If you're using v1beta1 version of the API, please migrate to v1")
}

if len(vpa.Spec.Recommenders) > 1 {
return fmt.Errorf("The current version of VPA object shouldn't specify more than one recommenders.")
}

return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ type VerticalPodAutoscalerSpec struct {
// Recommender responsible for generating recommendation for this object.
// List should be empty (then the default recommender will generate the
// recommendation) or contain exactly one recommender.
// +kubebuilder:validation:MaxItems=1
// +optional
Recommenders []*VerticalPodAutoscalerRecommenderSelector `json:"recommenders,omitempty" protobuf:"bytes,4,opt,name=recommenders"`
}
Expand Down Expand Up @@ -142,6 +143,7 @@ type PodUpdatePolicy struct {
// pod eviction (pending other checks like PDB). Only positive values are
// allowed. Overrides global '--min-replicas' flag.
// +optional
// +kubebuilder:validation:Minimum=1
MinReplicas *int32 `json:"minReplicas,omitempty" protobuf:"varint,2,opt,name=minReplicas"`

// EvictionRequirements is a list of EvictionRequirements that need to
Expand Down Expand Up @@ -183,18 +185,23 @@ type PodResourcePolicy struct {
// +optional
// +patchMergeKey=containerName
// +patchStrategy=merge
// +kubebuilder:validation:MaxItems=100
ContainerPolicies []ContainerResourcePolicy `json:"containerPolicies,omitempty" patchStrategy:"merge" patchMergeKey:"containerName" protobuf:"bytes,1,rep,name=containerPolicies"`
}

// ContainerResourcePolicy controls how autoscaler computes the recommended
// resources for a specific container.
// +kubebuilder:validation:XValidation:rule="!has(self.mode) || !has(self.controlledValues) || self.mode != 'Off' || self.controlledValues != 'RequestsAndLimits'",message="ControlledValues shouldn't be specified if container scaling mode is off"
type ContainerResourcePolicy struct {
// Name of the container or DefaultContainerResourcePolicy, in which
// case the policy is used by the containers that don't have their own
// policy specified.
// +kubebuilder:validation:Pattern=`^[a-zA-Z0-9-_]+$`
// +kubebuilder:validation:XValidation:rule="size(self) > 0",message="ContainerName cannot be empty"
ContainerName string `json:"containerName,omitempty" protobuf:"bytes,1,opt,name=containerName"`
// Whether autoscaler is enabled for the container. The default is "Auto".
// +optional
// +kubebuilder:default="Auto"
Mode *ContainerScalingMode `json:"mode,omitempty" protobuf:"bytes,2,opt,name=mode"`
// Specifies the minimal amount of resources that will be recommended
// for the container. The default is no minimum.
Expand Down

0 comments on commit 45785a4

Please sign in to comment.