diff --git a/docs.md b/docs.md index f432453a6..0e9541da9 100644 --- a/docs.md +++ b/docs.md @@ -115,6 +115,26 @@ If `field.cattle.io/no-creator-rbac` annotation is set, `field.cattle.io/creator - If version management is determined to be disabled, and the `.spec.rke2Config` or `.spec.k3sConfig` field exists in the new cluster object with a value different from the old one, the webhook will permit the update with a warning indicating that these changes will not take effect until version management is enabled for the cluster. - If version management is determined to be disabled, and the `.spec.rke2Config` or `.spec.k3sConfig` field is missing, the webhook will permit the request to allow users to remove the unused fields via API or Terraform. + +##### Feature: Cluster Agent Scheduling Customization + +The `SchedulingCustomization` subfield of the `DeploymentCustomization` field defines the properties of a Pod Disruption Budget and Priority Class which will be automatically deployed by Rancher for the cattle-cluster-agent. + +The `schedulingCustomization.PriorityClass` field contains two attributes + ++ `value`: This must be an integer value equal to or between negative 1 billion and 1 billion. ++ `preemption`: This must be a string value which indicates the desired preemption behavior, its value can be either `PreemptLowerPriority` or `Never`. Any other value must be rejected. + +The `schedulingCustomization.PodDisruptionBudget` field contains two attributes + ++ `minAvailable`: This is a string value that indicates the minimum number of agent replicas that must be running at a given time. ++ `maxUnavailable`: This is a string value that indicates the maximum number of agent replicas that can be unavailable at a given time. + +Both `minAvailable` and `maxUnavailable` must be a string which represents a non-negative whole number, or a whole number percentage greater than or equal to `0%` and less than or equal to `100%`. Only one of the two fields can have a non-zero or empty value at a given time. These fields use the following regex when assessing if a given percentage value is valid: +```regex +^([0-9]|[1-9][0-9]|100)%$ +``` + ## ClusterProxyConfig ### Validation Checks @@ -407,6 +427,12 @@ When settings are updated, the following additional checks take place: have a status condition `AgentTlsStrictCheck` set to `True`, unless the new setting has an overriding annotation `cattle.io/force=true`. + +- `cluster-agent-default-priority-class` must contain a valid JSON object which matches the format of a `v1.PriorityClassSpec` object. The Value field must be greater than or equal to negative 1 billion and less than or equal to 1 billion. The Preemption field must be a string value set to either `PreemptLowerPriority` or `Never`. + + +- `cluster-agent-default-pod-disruption-budget` must contain a valid JSON object which matches the format of a `v1.PodDisruptionBudgetSpec` object. The `minAvailable` and `maxUnavailable` fields must have a string value that is either a non-negative whole number, or a non-negative whole number percentage value less than or equal to `100%`. + ## Token ### Validation Checks @@ -499,6 +525,25 @@ A `Toleration` is matched to a regex which is provided by upstream [apimachinery For the `Affinity` based rules, the `podAffinity`/`podAntiAffinity` are validated via label selectors via [this apimachinery function](https://github.com/kubernetes/apimachinery/blob/02a41040d88da08de6765573ae2b1a51f424e1ca/pkg/apis/meta/v1/validation/validation.go#L56) whereas the `nodeAffinity` `nodeSelectorTerms` are validated via the same `Toleration` function. +#### cluster.spec.clusterAgentDeploymentCustomization.schedulingCustomization + +The `SchedulingCustomization` subfield of the `DeploymentCustomization` field defines the properties of a Pod Disruption Budget and Priority Class which will be automatically deployed by Rancher for the cattle-cluster-agent. + +The `schedulingCustomization.PriorityClass` field contains two attributes + ++ `value`: This must be an integer value equal to or between negative 1 billion and 1 billion. ++ `preemption`: This must be a string value which indicates the desired preemption behavior, its value can be either `PreemptLowerPriority` or `Never`. Any other value must be rejected. + +The `schedulingCustomization.PodDisruptionBudget` field contains two attributes + ++ `minAvailable`: This is a string value that indicates the minimum number of agent replicas that must be running at a given time. ++ `maxUnavailable`: This is a string value that indicates the maximum number of agent replicas that can be unavailable at a given time. + +Both `minAvailable` and `maxUnavailable` must be a string which represents a non-negative whole number, or a whole number percentage greater than or equal to `0%` and less than or equal to `100%`. Only one of the two fields can have a non-zero or empty value at a given time. These fields use the following regex when assessing if a given percentage value is valid: +```regex +^([0-9]|[1-9][0-9]|100)%$ +``` + ### Mutation Checks #### On Create diff --git a/go.mod b/go.mod index fb735108c..9fc69ad38 100644 --- a/go.mod +++ b/go.mod @@ -43,7 +43,7 @@ require ( github.com/gorilla/mux v1.8.1 github.com/rancher/dynamiclistener v0.6.1 github.com/rancher/lasso v0.2.1 - github.com/rancher/rancher/pkg/apis v0.0.0-20250213173112-3d729db8a848 + github.com/rancher/rancher/pkg/apis v0.0.0-20250220153925-3abb578f42fe github.com/rancher/rke v1.8.0-rc.1 github.com/rancher/wrangler/v3 v3.2.0-rc.3 github.com/robfig/cron v1.2.0 diff --git a/go.sum b/go.sum index 39534077d..4846a64d3 100644 --- a/go.sum +++ b/go.sum @@ -157,8 +157,8 @@ github.com/rancher/lasso v0.2.1 h1:SZTqMVQn8cAOqvwGBd1/EYOIJ/MGN+UfJrOWvHd4jHU= github.com/rancher/lasso v0.2.1/go.mod h1:KSV3jBXfdXqdCuMm2uC8kKB9q/wuDYb3h0eHZoRjShM= github.com/rancher/norman v0.5.1 h1:jbp49IcX2Hn+N2QA3MHdIXeUG0VgCSIjJs4xnqG+j90= github.com/rancher/norman v0.5.1/go.mod h1:qX/OG/4wY27xSAcSdRilUBxBumV6Ey2CWpAeaKnBQDs= -github.com/rancher/rancher/pkg/apis v0.0.0-20250213173112-3d729db8a848 h1:0mNj9JwUmMtn5lGfPoE1AiCXMRuCRwMbhnmFVqktswM= -github.com/rancher/rancher/pkg/apis v0.0.0-20250213173112-3d729db8a848/go.mod h1:FfFL3Pw7ds9aaaA0JvZ3m8kJXTg6DNknxLBC0vODpuI= +github.com/rancher/rancher/pkg/apis v0.0.0-20250220153925-3abb578f42fe h1:DNGD4RCs1k5PxAHUc1zA9FiEfowcejQQcGAItwUIDh4= +github.com/rancher/rancher/pkg/apis v0.0.0-20250220153925-3abb578f42fe/go.mod h1:0JtLfvgj4YiwddyHEvhF3yEK9k5c22CWs55DppqdP5o= github.com/rancher/rke v1.7.2 h1:+2fcl0gCjRHzf1ev9C9ptQ1pjYbDngC1Qv8V/0ki/dk= github.com/rancher/rke v1.7.2/go.mod h1:+x++Mvl0A3jIzNLiu8nkraqZXiHg6VPWv0Xl4iQCg+A= github.com/rancher/wrangler/v3 v3.2.0-rc.3 h1:MySHWLxLLrGrM2sq5YYp7Ol1kQqYt9lvIzjGR50UZ+c= diff --git a/pkg/resources/common/common.go b/pkg/resources/common/common.go index d7aac2827..dfb500dc6 100644 --- a/pkg/resources/common/common.go +++ b/pkg/resources/common/common.go @@ -1,6 +1,8 @@ package common import ( + "regexp" + "github.com/rancher/webhook/pkg/admission" "github.com/rancher/webhook/pkg/auth" "github.com/sirupsen/logrus" @@ -19,8 +21,15 @@ const ( CreatorPrincipalNameAnn = "field.cattle.io/creator-principal-name" // NoCreatorRBACAnn is an annotation key to indicate that a cluster doesn't need NoCreatorRBACAnn = "field.cattle.io/no-creator-rbac" + // SchedulingCustomizationFeatureName is the feature name for enabling customization of PDBs and PCs for the + // cattle-cluster-agent + SchedulingCustomizationFeatureName = "cluster-agent-scheduling-customization" ) +// PdbPercentageRegex ensures that a given string is a properly formatted percentage value +// between 0% and 100% so that it can be used in a Pod Disruption Budget +var PdbPercentageRegex = regexp.MustCompile("^([0-9]|[1-9][0-9]|100)%$") + // ConvertAuthnExtras converts authnv1 type extras to authzv1 extras. Technically these are both // type alias to string, so the conversion is straightforward func ConvertAuthnExtras(extra map[string]authnv1.ExtraValue) map[string]authzv1.ExtraValue { diff --git a/pkg/resources/management.cattle.io/v3/cluster/Cluster.md b/pkg/resources/management.cattle.io/v3/cluster/Cluster.md index 0394a25ad..079aabd69 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/Cluster.md +++ b/pkg/resources/management.cattle.io/v3/cluster/Cluster.md @@ -23,3 +23,23 @@ If `field.cattle.io/no-creator-rbac` annotation is set, `field.cattle.io/creator - If the cluster represents other types of clusters and the annotation is present, the webhook will permit the request with a warning that the annotation is intended for imported RKE2/k3s clusters and will not take effect on this cluster. - If version management is determined to be disabled, and the `.spec.rke2Config` or `.spec.k3sConfig` field exists in the new cluster object with a value different from the old one, the webhook will permit the update with a warning indicating that these changes will not take effect until version management is enabled for the cluster. - If version management is determined to be disabled, and the `.spec.rke2Config` or `.spec.k3sConfig` field is missing, the webhook will permit the request to allow users to remove the unused fields via API or Terraform. + + +#### Feature: Cluster Agent Scheduling Customization + +The `SchedulingCustomization` subfield of the `DeploymentCustomization` field defines the properties of a Pod Disruption Budget and Priority Class which will be automatically deployed by Rancher for the cattle-cluster-agent. + +The `schedulingCustomization.PriorityClass` field contains two attributes + ++ `value`: This must be an integer value equal to or between negative 1 billion and 1 billion. ++ `preemption`: This must be a string value which indicates the desired preemption behavior, its value can be either `PreemptLowerPriority` or `Never`. Any other value must be rejected. + +The `schedulingCustomization.PodDisruptionBudget` field contains two attributes + ++ `minAvailable`: This is a string value that indicates the minimum number of agent replicas that must be running at a given time. ++ `maxUnavailable`: This is a string value that indicates the maximum number of agent replicas that can be unavailable at a given time. + +Both `minAvailable` and `maxUnavailable` must be a string which represents a non-negative whole number, or a whole number percentage greater than or equal to `0%` and less than or equal to `100%`. Only one of the two fields can have a non-zero or empty value at a given time. These fields use the following regex when assessing if a given percentage value is valid: +```regex +^([0-9]|[1-9][0-9]|100)%$ +``` diff --git a/pkg/resources/management.cattle.io/v3/cluster/validator.go b/pkg/resources/management.cattle.io/v3/cluster/validator.go index e1e9774c5..4e4a6dde4 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "reflect" + "strconv" "github.com/blang/semver" apisv3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" @@ -17,6 +18,7 @@ import ( admissionregistrationv1 "k8s.io/api/admissionregistration/v1" authenticationv1 "k8s.io/api/authentication/v1" v1 "k8s.io/api/authorization/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,13 +38,15 @@ func NewValidator( sar authorizationv1.SubjectAccessReviewInterface, cache v3.PodSecurityAdmissionConfigurationTemplateCache, userCache v3.UserCache, + featureCache v3.FeatureCache, settingCache v3.SettingCache, ) *Validator { return &Validator{ admitter: admitter{ sar: sar, psact: cache, - userCache: userCache, // userCache is nil for downstream clusters. + userCache: userCache, // userCache is nil for downstream clusters. + featureCache: featureCache, settingCache: settingCache, // settingCache is nil for downstream clusters }, } @@ -79,6 +83,7 @@ type admitter struct { sar authorizationv1.SubjectAccessReviewInterface psact v3.PodSecurityAdmissionConfigurationTemplateCache userCache v3.UserCache + featureCache v3.FeatureCache settingCache v3.SettingCache } @@ -117,6 +122,14 @@ func (a *admitter) Admit(request *admission.Request) (*admissionv1.AdmissionResp } } + if response, err = a.validatePodDisruptionBudget(oldCluster, newCluster, request.Operation); err != nil || !response.Allowed { + return response, err + } + + if response, err = a.validatePriorityClass(oldCluster, newCluster, request.Operation); err != nil || !response.Allowed { + return response, err + } + response, err = a.validatePSACT(oldCluster, newCluster, request.Operation) if err != nil { return nil, fmt.Errorf("failed to validate PodSecurityAdmissionConfigurationTemplate(PSACT): %w", err) @@ -265,6 +278,155 @@ func (a *admitter) validatePSACT(oldCluster, newCluster *apisv3.Cluster, op admi return admission.ResponseAllowed(), nil } +// validatePriorityClass validates that the Priority Class defined in the cluster SchedulingCustomization field is properly +// configured. The cluster-agent-scheduling-customization feature must be enabled to configure a Priority Class, however an existing +// Priority Class may be deleted even if the feature is disabled. +func (a *admitter) validatePriorityClass(oldCluster, newCluster *apisv3.Cluster, op admissionv1.Operation) (*admissionv1.AdmissionResponse, error) { + if op != admissionv1.Create && op != admissionv1.Update { + return admission.ResponseAllowed(), nil + } + + newClusterScheduling := getSchedulingCustomization(newCluster) + oldClusterScheduling := getSchedulingCustomization(oldCluster) + + var newPC, oldPC *apisv3.PriorityClassSpec + if newClusterScheduling != nil { + newPC = newClusterScheduling.PriorityClass + } + + if oldClusterScheduling != nil { + oldPC = oldClusterScheduling.PriorityClass + } + + if newPC == nil { + return admission.ResponseAllowed(), nil + } + + featuredEnabled, err := a.featureCache.Get(common.SchedulingCustomizationFeatureName) + if err != nil { + return nil, fmt.Errorf("failed to determine status of '%s' feature", common.SchedulingCustomizationFeatureName) + } + + enabled := featuredEnabled.Status.Default + if featuredEnabled.Spec.Value != nil { + enabled = *featuredEnabled.Spec.Value + } + + // if the feature is disabled then we should not permit any changes between the old and new clusters other than deletion + if !enabled && oldPC != nil { + if reflect.DeepEqual(*oldPC, *newPC) { + return admission.ResponseAllowed(), nil + } + + return admission.ResponseBadRequest(fmt.Sprintf("'%s' feature is disabled, will only permit removal of Scheduling Customization fields until reenabled", common.SchedulingCustomizationFeatureName)), nil + } + + if !enabled && oldPC == nil { + return admission.ResponseBadRequest(fmt.Sprintf("the '%s' feature must be enabled in order to configure a Priority Class or Pod Disruption Budget", common.SchedulingCustomizationFeatureName)), nil + } + + if newPC.Preemption != nil && *newPC.Preemption != corev1.PreemptNever && *newPC.Preemption != corev1.PreemptLowerPriority && *newPC.Preemption != "" { + return admission.ResponseBadRequest("Priority Class Preemption value must be 'Never', 'PreemptLowerPriority', or empty"), nil + } + + if newPC.Value > 1000000000 { + return admission.ResponseBadRequest("Priority Class value cannot be greater than 1 billion"), nil + } + + if newPC.Value < -1000000000 { + return admission.ResponseBadRequest("Priority Class value cannot be less than negative 1 billion"), nil + } + + return admission.ResponseAllowed(), nil +} + +// validatePodDisruptionBudget validates that the Pod Disruption Budget defined in the cluster SchedulingCustomization field is properly +// configured. The cluster-agent-scheduling-customization feature must be enabled to configure a Pod Disruption Budget, however an existing +// Pod Disruption Budget may be deleted even if the feature is disabled. +func (a *admitter) validatePodDisruptionBudget(oldCluster, newCluster *apisv3.Cluster, op admissionv1.Operation) (*admissionv1.AdmissionResponse, error) { + if op != admissionv1.Create && op != admissionv1.Update { + return admission.ResponseAllowed(), nil + } + newClusterScheduling := getSchedulingCustomization(newCluster) + oldClusterScheduling := getSchedulingCustomization(oldCluster) + + var newPDB, oldPDB *apisv3.PodDisruptionBudgetSpec + if newClusterScheduling != nil { + newPDB = newClusterScheduling.PodDisruptionBudget + } + + if oldClusterScheduling != nil { + oldPDB = oldClusterScheduling.PodDisruptionBudget + } + + if newPDB == nil { + return admission.ResponseAllowed(), nil + } + + featuredEnabled, err := a.featureCache.Get(common.SchedulingCustomizationFeatureName) + if err != nil { + return nil, fmt.Errorf("failed to determine status of '%s' feature", common.SchedulingCustomizationFeatureName) + } + + enabled := featuredEnabled.Status.Default + if featuredEnabled.Spec.Value != nil { + enabled = *featuredEnabled.Spec.Value + } + + // if the feature is disabled then we should not permit any changes between the old and new clusters other than deletion + if !enabled && oldPDB != nil { + if reflect.DeepEqual(*oldPDB, *newPDB) { + return admission.ResponseAllowed(), nil + } + + return admission.ResponseBadRequest(fmt.Sprintf("'%s' feature is disabled, will only permit removal of Scheduling Customization fields until reenabled", common.SchedulingCustomizationFeatureName)), nil + } + + if !enabled && oldPDB == nil { + return admission.ResponseBadRequest(fmt.Sprintf("the '%s' feature must be enabled in order to configure a Priority Class or Pod Disruption Budget", common.SchedulingCustomizationFeatureName)), nil + } + + minAvailStr := newPDB.MinAvailable + maxUnavailStr := newPDB.MaxUnavailable + + if (minAvailStr == "" && maxUnavailStr == "") || + (minAvailStr == "0" && maxUnavailStr == "0") || + (minAvailStr != "" && minAvailStr != "0") && (maxUnavailStr != "" && maxUnavailStr != "0") { + return admission.ResponseBadRequest("both minAvailable and maxUnavailable cannot be set to a non zero value, at least one must be omitted or set to zero"), nil + } + + minAvailIsString := false + maxUnavailIsString := false + + minAvailInt, err := strconv.Atoi(minAvailStr) + if err != nil { + minAvailIsString = minAvailStr != "" + } + + maxUnavailInt, err := strconv.Atoi(maxUnavailStr) + if err != nil { + maxUnavailIsString = maxUnavailStr != "" + } + + if !minAvailIsString && minAvailInt < 0 { + return admission.ResponseBadRequest("minAvailable cannot be set to a negative integer"), nil + } + + if !maxUnavailIsString && maxUnavailInt < 0 { + return admission.ResponseBadRequest("maxUnavailable cannot be set to a negative integer"), nil + } + + if minAvailIsString && !common.PdbPercentageRegex.Match([]byte(minAvailStr)) { + return admission.ResponseBadRequest(fmt.Sprintf("minAvailable must be a non-negative whole integer or a percentage value between 0 and 100, regex used is '%s'", common.PdbPercentageRegex.String())), nil + } + + if maxUnavailIsString && maxUnavailStr != "" && !common.PdbPercentageRegex.Match([]byte(maxUnavailStr)) { + return admission.ResponseBadRequest(fmt.Sprintf("minAvailable must be a non-negative whole integer or a percentage value between 0 and 100, regex used is '%s'", common.PdbPercentageRegex.String())), nil + } + + return admission.ResponseAllowed(), nil +} + // checkPSAConfigOnCluster validates the cluster spec when DefaultPodSecurityAdmissionConfigurationTemplateName is set. func (a *admitter) checkPSAConfigOnCluster(cluster *apisv3.Cluster) (*admissionv1.AdmissionResponse, error) { // validate that extra_args.admission-control-config-file is not set at the same time @@ -310,6 +472,22 @@ func (a *admitter) checkPSAConfigOnCluster(cluster *apisv3.Cluster) (*admissionv return admission.ResponseAllowed(), nil } +func getSchedulingCustomization(cluster *apisv3.Cluster) *apisv3.AgentSchedulingCustomization { + if cluster == nil { + return nil + } + + if cluster.Spec.ClusterAgentDeploymentCustomization == nil { + return nil + } + + if cluster.Spec.ClusterAgentDeploymentCustomization.SchedulingCustomization == nil { + return nil + } + + return cluster.Spec.ClusterAgentDeploymentCustomization.SchedulingCustomization +} + // validateVersionManagementFeature validates the annotation for the version management feature is set with valid value on the imported RKE2/K3s cluster, // additionally, it permits but include a warning to the response if either of the following is true: // - the annotation is found on a cluster rather than imported RKE2/K3s cluster; diff --git a/pkg/resources/management.cattle.io/v3/cluster/validator_test.go b/pkg/resources/management.cattle.io/v3/cluster/validator_test.go index 7e777b8cd..94b98b252 100644 --- a/pkg/resources/management.cattle.io/v3/cluster/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/cluster/validator_test.go @@ -13,6 +13,7 @@ import ( "go.uber.org/mock/gomock" admissionv1 "k8s.io/api/admission/v1" authorizationv1 "k8s.io/api/authorization/v1" + k8sv1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -613,3 +614,720 @@ func Test_versionManagementEnabled(t *testing.T) { }) } } + +func Test_validateAgentSchedulingCustomizationPodDisruptionBudget(t *testing.T) { + tests := []struct { + name string + cluster *v3.Cluster + oldCluster *v3.Cluster + featureEnabled bool + shouldSucceed bool + }{ + { + name: "no scheduling customization - feature enabled", + cluster: &v3.Cluster{}, + shouldSucceed: true, + featureEnabled: true, + }, + { + name: "no scheduling customization - feature disabled", + cluster: &v3.Cluster{}, + shouldSucceed: true, + featureEnabled: false, + }, + { + name: "invalid PDB configuration - negative min available integer", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "-1", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - negative max unavailable integer", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MaxUnavailable: "-1", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - both fields set", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MaxUnavailable: "1", + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string passed to max unavailable", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MaxUnavailable: "five", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string passed to min available", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "five", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string with invalid percentage number set for min available", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "5.5%", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string with invalid percentage number set for max unavailable", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MaxUnavailable: "5.5%", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - both set to zero", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "0", + MaxUnavailable: "0", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - max unavailable set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MaxUnavailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - min available set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - min available set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "1", + MaxUnavailable: "0", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - max unavailable set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "0", + MaxUnavailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - max unavailable set to percentage", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MaxUnavailable: "50%", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - min available set to percentage", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - updating from percentage to int", + shouldSucceed: true, + featureEnabled: true, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB reconfiguration - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB creation - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{}, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + }, + { + name: "valid PDB reconfiguration - field is removed while feature is disabled", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{}, + }, + }, + { + name: "valid update - field is unchanged while feature is disabled", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PodDisruptionBudget: &v3.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + }, + } + + t.Parallel() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + a := admitter{ + featureCache: createMockFeatureCache(ctrl, common.SchedulingCustomizationFeatureName, tt.featureEnabled), + } + + response, err := a.validatePodDisruptionBudget(tt.oldCluster, tt.cluster, admissionv1.Create) + assert.Equal(t, tt.shouldSucceed, response.Allowed) + assert.NoError(t, err) + + response, err = a.validatePodDisruptionBudget(tt.oldCluster, tt.cluster, admissionv1.Update) + assert.Equal(t, tt.shouldSucceed, response.Allowed) + assert.Nil(t, err) + assert.NoError(t, err) + }) + } +} + +func Test_validateAgentSchedulingCustomizationPriorityClass(t *testing.T) { + preemptionNever := k8sv1.PreemptionPolicy("Never") + preemptionInvalid := k8sv1.PreemptionPolicy("rancher") + + tests := []struct { + name string + cluster *v3.Cluster + oldCluster *v3.Cluster + featureEnabled bool + shouldSucceed bool + }{ + { + name: "empty priority class - feature enabled", + cluster: &v3.Cluster{}, + shouldSucceed: true, + featureEnabled: true, + }, + { + name: "empty priority class - feature disabled", + cluster: &v3.Cluster{}, + shouldSucceed: true, + featureEnabled: true, + }, + { + name: "valid priority class with default preemption", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 123456, + }, + }, + }, + }, + }, + }, + }, + { + name: "valid priority class with custom preemption", + shouldSucceed: true, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 123456, + Preemption: &preemptionNever, + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - value too large", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 1234567891234567890, + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - value too small", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: -1234567891234567890, + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - preemption value invalid", + shouldSucceed: false, + featureEnabled: true, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 24321, + Preemption: &preemptionInvalid, + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 24321, + Preemption: &preemptionInvalid, + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid update attempt - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 4321, + }, + }, + }, + }, + }, + }, + }, + { + name: "valid update attempt - feature is enabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 4321, + }, + }, + }, + }, + }, + }, + }, + { + name: "valid update attempt - feature is disabled, but fields are unchanged", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + }, + }, + { + name: "valid update attempt - field is removed while feature is disabled", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v3.Cluster{ + Spec: v3.ClusterSpec{ + ClusterSpecBase: v3.ClusterSpecBase{ + ClusterAgentDeploymentCustomization: &v3.AgentDeploymentCustomization{ + SchedulingCustomization: &v3.AgentSchedulingCustomization{ + PriorityClass: &v3.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + }, + cluster: &v3.Cluster{ + Spec: v3.ClusterSpec{}, + }, + }, + } + + t.Parallel() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + a := admitter{ + featureCache: createMockFeatureCache(ctrl, common.SchedulingCustomizationFeatureName, tt.featureEnabled), + } + + response, err := a.validatePriorityClass(tt.oldCluster, tt.cluster, admissionv1.Create) + assert.Equal(t, tt.shouldSucceed, response.Allowed) + assert.NoError(t, err) + + response, err = a.validatePriorityClass(tt.oldCluster, tt.cluster, admissionv1.Update) + assert.Equal(t, tt.shouldSucceed, response.Allowed) + assert.NoError(t, err) + + }) + } +} + +func createMockFeatureCache(ctrl *gomock.Controller, featureName string, enabled bool) *fake.MockNonNamespacedCacheInterface[*v3.Feature] { + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + featureCache.EXPECT().Get(featureName).DoAndReturn(func(string) (*v3.Feature, error) { + return &v3.Feature{ + Spec: v3.FeatureSpec{ + Value: &enabled, + }, + }, nil + }).AnyTimes() + return featureCache +} diff --git a/pkg/resources/management.cattle.io/v3/setting/Setting.md b/pkg/resources/management.cattle.io/v3/setting/Setting.md index 95ab66c10..935017162 100644 --- a/pkg/resources/management.cattle.io/v3/setting/Setting.md +++ b/pkg/resources/management.cattle.io/v3/setting/Setting.md @@ -17,3 +17,9 @@ When settings are updated, the following additional checks take place: - If `agent-tls-mode` has `default` or `value` updated from `system-store` to `strict`, then all non-local clusters must have a status condition `AgentTlsStrictCheck` set to `True`, unless the new setting has an overriding annotation `cattle.io/force=true`. + + +- `cluster-agent-default-priority-class` must contain a valid JSON object which matches the format of a `v1.PriorityClassSpec` object. The Value field must be greater than or equal to negative 1 billion and less than or equal to 1 billion. The Preemption field must be a string value set to either `PreemptLowerPriority` or `Never`. + + +- `cluster-agent-default-pod-disruption-budget` must contain a valid JSON object which matches the format of a `v1.PodDisruptionBudgetSpec` object. The `minAvailable` and `maxUnavailable` fields must have a string value that is either a non-negative whole number, or a non-negative whole number percentage value less than or equal to `100%`. diff --git a/pkg/resources/management.cattle.io/v3/setting/validator.go b/pkg/resources/management.cattle.io/v3/setting/validator.go index 478bb73eb..7b5bded7e 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator.go @@ -1,20 +1,23 @@ package setting import ( + "encoding/json" "errors" "fmt" "strconv" "time" v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" + provv1 "github.com/rancher/rancher/pkg/apis/provisioning.cattle.io/v1" "github.com/rancher/webhook/pkg/admission" controllerv3 "github.com/rancher/webhook/pkg/generated/controllers/management.cattle.io/v3" objectsv3 "github.com/rancher/webhook/pkg/generated/objects/management.cattle.io/v3" + "github.com/rancher/webhook/pkg/resources/common" "github.com/robfig/cron" "github.com/sirupsen/logrus" admissionv1 "k8s.io/api/admission/v1" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" @@ -22,12 +25,14 @@ import ( ) const ( - DeleteInactiveUserAfter = "delete-inactive-user-after" - DisableInactiveUserAfter = "disable-inactive-user-after" - AuthUserSessionTTLMinutes = "auth-user-session-ttl-minutes" - UserLastLoginDefault = "user-last-login-default" - UserRetentionCron = "user-retention-cron" - AgentTLSMode = "agent-tls-mode" + DeleteInactiveUserAfter = "delete-inactive-user-after" + DisableInactiveUserAfter = "disable-inactive-user-after" + AuthUserSessionTTLMinutes = "auth-user-session-ttl-minutes" + UserLastLoginDefault = "user-last-login-default" + UserRetentionCron = "user-retention-cron" + AgentTLSMode = "agent-tls-mode" + CattleClusterAgentPriorityClass = "cluster-agent-default-priority-class" + CattleClusterAgentPodDisruptionBudget = "cluster-agent-default-pod-disruption-budget" ) // MinDeleteInactiveUserAfter is the minimum duration for delete-inactive-user-after setting. @@ -115,6 +120,10 @@ func (a *admitter) admitUpdate(oldSetting, newSetting *v3.Setting) (*admissionv1 switch newSetting.Name { case AgentTLSMode: err = a.validateAgentTLSMode(oldSetting, newSetting) + case CattleClusterAgentPriorityClass: + err = a.validateClusterAgentPriorityClass(newSetting) + case CattleClusterAgentPodDisruptionBudget: + err = a.validateClusterAgentPodDisruptionBudget(newSetting) default: } @@ -340,7 +349,78 @@ func (a *admitter) validateAgentTLSMode(oldSetting, newSetting *v3.Setting) erro return nil } -func clusterConditionMatches(cluster *v3.Cluster, t v3.ClusterConditionType, status v1.ConditionStatus) bool { +func (a *admitter) validateClusterAgentPriorityClass(newSetting *v3.Setting) error { + pc := provv1.PriorityClassSpec{} + + err := json.Unmarshal([]byte(newSetting.Value), &pc) + if err != nil { + return err + } + + if pc.Value > 1000000000 { + return fmt.Errorf("value must be less than 1 billion and greater than negative 1 billion") + } + + if pc.Value < -1000000000 { + return fmt.Errorf("value must be less than 1 billion and greater than negative 1 billion") + } + + if pc.Preemption != nil && *pc.Preemption != corev1.PreemptNever && *pc.Preemption != corev1.PreemptLowerPriority && *pc.Preemption != "" { + return fmt.Errorf("preemption policy must be set to either 'Never' or 'PreemptLowerPriority'") + } + + return nil +} + +func (a *admitter) validateClusterAgentPodDisruptionBudget(newSetting *v3.Setting) error { + pdb := provv1.PodDisruptionBudgetSpec{} + + err := json.Unmarshal([]byte(newSetting.Value), &pdb) + if err != nil { + return err + } + + minAvailIsString := false + maxUnavailIsString := false + minAvailStr := pdb.MinAvailable + maxUnavailStr := pdb.MaxUnavailable + + minAvailInt, err := strconv.Atoi(pdb.MinAvailable) + if err != nil { + minAvailIsString = true + } + + maxUnavailInt, err := strconv.Atoi(pdb.MaxUnavailable) + if err != nil { + maxUnavailIsString = true + } + + // can't set a non-zero value on both fields at the same time + if (minAvailStr == "" && maxUnavailStr == "") || + (minAvailStr != "" && minAvailStr != "0") && (maxUnavailStr != "" && maxUnavailStr != "0") { + return fmt.Errorf("both minAvailable and maxUnavailable cannot be set to a non zero value, at least one must be set to zero") + } + + if minAvailInt < 0 { + return fmt.Errorf("minAvailable cannot be set to a negative integer") + } + + if maxUnavailInt < 0 { + return fmt.Errorf("maxUnavailable cannot be set to a negative integer") + } + + if minAvailIsString && !common.PdbPercentageRegex.Match([]byte(minAvailStr)) { + return fmt.Errorf("minAvailable must be a non-negative whole integer or a percentage value between 0 and 100, regex used is '%s'", common.PdbPercentageRegex.String()) + } + + if maxUnavailIsString && !common.PdbPercentageRegex.Match([]byte(maxUnavailStr)) { + return fmt.Errorf("minAvailable must be a non-negative whole integer or a percentage value between 0 and 100, regex used is '%s'", common.PdbPercentageRegex.String()) + } + + return nil +} + +func clusterConditionMatches(cluster *v3.Cluster, t v3.ClusterConditionType, status corev1.ConditionStatus) bool { for _, cond := range cluster.Status.Conditions { if cond.Type == t && cond.Status == status { return true diff --git a/pkg/resources/management.cattle.io/v3/setting/validator_test.go b/pkg/resources/management.cattle.io/v3/setting/validator_test.go index 6753f662f..cfb5fe12f 100644 --- a/pkg/resources/management.cattle.io/v3/setting/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/setting/validator_test.go @@ -349,6 +349,211 @@ func (s *SettingSuite) validateUserLastLoginDefault(op v1.Operation) { } } +func (s *SettingSuite) TestValidateClusterAgentSchedulingPriorityClass() { + tests := []struct { + name string + newValue string + allowed bool + }{ + { + name: "valid update to PC - value", + allowed: true, + newValue: ` +{ + "value": 1356, + "preemption": "PreemptLowerPriority" +} +`, + }, + { + name: "valid update to PC - preemption", + allowed: true, + newValue: ` +{ + "value": 10000000, + "preemption": "Never" +} +`, + }, + { + name: "valid update to PC - both", + allowed: true, + newValue: ` +{ + "value": 1000, + "preemption": "Never" +} +`, + }, + { + name: "invalid update to PC - value lower than 1 billion", + allowed: false, + newValue: ` +{ + "value": -1000000001, + "preemption": "PreemptLowerPriority" +} +`, + }, + { + name: "invalid update to PC - value greater than 1 billion", + allowed: false, + newValue: ` +{ + "value": 1000000001, + "preemption": "PreemptLowerPriority" +} +`, + }, + { + name: "invalid update to PC - invalid preemption string", + allowed: false, + newValue: ` +{ + "value": 100000000, + "preemption": "invalid" +} +`, + }, + { + name: "invalid update to PC - invalid object", + allowed: false, + newValue: ` +{ + "invalid": 100000000, +} +`, + }, + } + + for _, test := range tests { + test := test + s.T().Run(test.name, func(t *testing.T) { + t.Parallel() + + validator := setting.NewValidator(nil, nil) + s.testAdmit(t, validator, nil, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.CattleClusterAgentPriorityClass, + }, + Value: test.newValue, + }, v1.Update, test.allowed) + }) + } +} + +func (s *SettingSuite) TestValidateClusterAgentSchedulingPodDisruptionBudget() { + tests := []struct { + name string + newValue string + allowed bool + }{ + { + name: "valid update to PDB - integer", + allowed: true, + newValue: ` +{ + "minAvailable": "0", + "maxUnavailable": "1" +}`, + }, + { + name: "valid update to PDB - percent", + allowed: true, + newValue: ` +{ + "minAvailable": "0", + "maxUnavailable": "50%" +}`, + }, + { + name: "valid update to PDB - both set to zero", + allowed: true, + newValue: ` +{ + "minAvailable": "0", + "maxUnavailable": "0" +}`, + }, + { + name: "invalid update to PDB - both fields set", + allowed: false, + newValue: ` +{ + "minAvailable": "1", + "maxUnavailable": "1" +}`, + }, + { + name: "invalid update to PDB - field set to negative value", + allowed: false, + newValue: ` +{ + "minAvailable": "-1", + "maxUnavailable": "0" +}`, + }, + { + name: "invalid update to PDB - field set to negative value", + allowed: false, + newValue: ` +{ + "minAvailable": "0", + "maxUnavailable": "-1" +}`, + }, + { + name: "invalid update to PDB - field set to invalid percentage value", + allowed: false, + newValue: ` +{ + "minAvailable": "50.5%", + "maxUnavailable": "0" +}`, + }, + { + name: "invalid update to PDB - field set to non-number string", + allowed: false, + newValue: ` +{ + "minAvailable": "five", + "maxUnavailable": "0" +}`, + }, + { + name: "invalid update to PDB - field set to non-number string", + allowed: false, + newValue: ` +{ + "minAvailable": "0", + "maxUnavailable": "five" +}`, + }, + { + name: "invalid update to PDB - bad object", + allowed: false, + newValue: ` +{ + "fake": "0", +}`, + }, + } + for _, test := range tests { + test := test + s.T().Run(test.name, func(t *testing.T) { + t.Parallel() + + validator := setting.NewValidator(nil, nil) + s.testAdmit(t, validator, nil, &v3.Setting{ + ObjectMeta: metav1.ObjectMeta{ + Name: setting.CattleClusterAgentPodDisruptionBudget, + }, + Value: test.newValue, + }, v1.Update, test.allowed) + }) + } +} + func (s *SettingSuite) TestValidateAuthUserSessionTTLMinutesOnUpdate() { s.validateAuthUserSessionTTLMinutes(v1.Update) } diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md b/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md index f566e0c67..ea114aa22 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/Cluster.md @@ -50,6 +50,25 @@ A `Toleration` is matched to a regex which is provided by upstream [apimachinery For the `Affinity` based rules, the `podAffinity`/`podAntiAffinity` are validated via label selectors via [this apimachinery function](https://github.com/kubernetes/apimachinery/blob/02a41040d88da08de6765573ae2b1a51f424e1ca/pkg/apis/meta/v1/validation/validation.go#L56) whereas the `nodeAffinity` `nodeSelectorTerms` are validated via the same `Toleration` function. +### cluster.spec.clusterAgentDeploymentCustomization.schedulingCustomization + +The `SchedulingCustomization` subfield of the `DeploymentCustomization` field defines the properties of a Pod Disruption Budget and Priority Class which will be automatically deployed by Rancher for the cattle-cluster-agent. + +The `schedulingCustomization.PriorityClass` field contains two attributes + ++ `value`: This must be an integer value equal to or between negative 1 billion and 1 billion. ++ `preemption`: This must be a string value which indicates the desired preemption behavior, its value can be either `PreemptLowerPriority` or `Never`. Any other value must be rejected. + +The `schedulingCustomization.PodDisruptionBudget` field contains two attributes + ++ `minAvailable`: This is a string value that indicates the minimum number of agent replicas that must be running at a given time. ++ `maxUnavailable`: This is a string value that indicates the maximum number of agent replicas that can be unavailable at a given time. + +Both `minAvailable` and `maxUnavailable` must be a string which represents a non-negative whole number, or a whole number percentage greater than or equal to `0%` and less than or equal to `100%`. Only one of the two fields can have a non-zero or empty value at a given time. These fields use the following regex when assessing if a given percentage value is valid: +```regex +^([0-9]|[1-9][0-9]|100)%$ +``` + ## Mutation Checks ### On Create diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go index 270de0df5..08cf2d989 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator.go @@ -6,8 +6,10 @@ import ( "fmt" "net/http" "path/filepath" + "reflect" "regexp" "slices" + "strconv" "strings" v1 "github.com/rancher/rancher/pkg/apis/provisioning.cattle.io/v1" @@ -52,6 +54,7 @@ func NewProvisioningClusterValidator(client *clients.Clients) *ProvisioningClust mgmtClusterClient: client.Management.Cluster(), secretCache: client.Core.Secret().Cache(), psactCache: client.Management.PodSecurityAdmissionConfigurationTemplate().Cache(), + featureCache: client.Management.Feature().Cache(), }, } } @@ -85,6 +88,7 @@ type provisioningAdmitter struct { mgmtClusterClient v3.ClusterClient secretCache corev1controller.SecretCache psactCache v3.PodSecurityAdmissionConfigurationTemplateCache + featureCache v3.FeatureCache } // Admit handles the webhook admission request sent to this webhook. @@ -132,6 +136,14 @@ func (p *provisioningAdmitter) Admit(request *admission.Request) (*admissionv1.A if response = p.validateDataDirectories(request, oldCluster, cluster); !response.Allowed { return response, err } + + if response, err = p.validatePodDisruptionBudget(oldCluster, cluster); err != nil || !response.Allowed { + return response, err + } + + if response, err = p.validatePriorityClass(oldCluster, cluster); err != nil || !response.Allowed { + return response, err + } } if err := p.validatePSACT(request, response, cluster); err != nil || response.Result != nil { @@ -504,6 +516,148 @@ func (p *provisioningAdmitter) validatePSACT(request *admission.Request, respons return nil } +// validatePriorityClass validates that the Priority Class defined in the cluster SchedulingCustomization field is properly +// configured. The cluster-agent-scheduling-customization feature must be enabled to configure a Priority Class, however an existing +// Priority Class may be deleted even if the feature is disabled. +func (p *provisioningAdmitter) validatePriorityClass(oldCluster, cluster *v1.Cluster) (*admissionv1.AdmissionResponse, error) { + newClusterScheduling := getSchedulingCustomization(cluster) + oldClusterScheduling := getSchedulingCustomization(oldCluster) + + var newPC, oldPC *v1.PriorityClassSpec + if newClusterScheduling != nil { + newPC = newClusterScheduling.PriorityClass + } + + if oldClusterScheduling != nil { + oldPC = oldClusterScheduling.PriorityClass + } + + if newPC == nil { + return admission.ResponseAllowed(), nil + } + + featuredEnabled, err := p.featureCache.Get(common.SchedulingCustomizationFeatureName) + if err != nil { + return nil, fmt.Errorf("failed to determine status of '%s' feature", common.SchedulingCustomizationFeatureName) + } + + enabled := featuredEnabled.Status.Default + if featuredEnabled.Spec.Value != nil { + enabled = *featuredEnabled.Spec.Value + } + + // if the feature is disabled then we should not permit any changes between the old and new clusters other than deletion + if !enabled && oldPC != nil { + if reflect.DeepEqual(*oldPC, *newPC) { + return admission.ResponseAllowed(), nil + } + return admission.ResponseBadRequest(fmt.Sprintf("'%s' feature is disabled, will only permit removal of Scheduling Customization fields until reenabled", common.SchedulingCustomizationFeatureName)), nil + } + + if !enabled && oldPC == nil { + return admission.ResponseBadRequest(fmt.Sprintf("the '%s' feature must be enabled in order to configure a Priority Class or Pod Disruption Budget", common.SchedulingCustomizationFeatureName)), nil + } + + if newPC.Preemption != nil && *newPC.Preemption != k8sv1.PreemptNever && *newPC.Preemption != k8sv1.PreemptLowerPriority && *newPC.Preemption != "" { + return admission.ResponseBadRequest("Priority Class Preemption value must be 'Never', 'PreemptLowerPriority', or omitted"), nil + } + + if newPC.Value > 1000000000 { + return admission.ResponseBadRequest("Priority Class value cannot be greater than 1 billion"), nil + } + + if newPC.Value < -1000000000 { + return admission.ResponseBadRequest("Priority Class value cannot be less than negative 1 billion"), nil + } + + return admission.ResponseAllowed(), nil +} + +// validatePodDisruptionBudget validates that the Pod Disruption Budget defined in the cluster SchedulingCustomization field is properly +// configured. The cluster-agent-scheduling-customization feature must be enabled to configure a Pod Disruption Budget, however an existing +// Pod Disruption Budget may be deleted even if the feature is disabled. +func (p *provisioningAdmitter) validatePodDisruptionBudget(oldCluster, cluster *v1.Cluster) (*admissionv1.AdmissionResponse, error) { + newClusterScheduling := getSchedulingCustomization(cluster) + oldClusterScheduling := getSchedulingCustomization(oldCluster) + + var newPDB, oldPDB *v1.PodDisruptionBudgetSpec + if newClusterScheduling != nil { + newPDB = newClusterScheduling.PodDisruptionBudget + } + + if oldClusterScheduling != nil { + oldPDB = oldClusterScheduling.PodDisruptionBudget + } + + if newPDB == nil { + return admission.ResponseAllowed(), nil + } + + featuredEnabled, err := p.featureCache.Get(common.SchedulingCustomizationFeatureName) + if err != nil { + return nil, fmt.Errorf("failed to determine status of '%s' feature", common.SchedulingCustomizationFeatureName) + } + + enabled := featuredEnabled.Status.Default + if featuredEnabled.Spec.Value != nil { + enabled = *featuredEnabled.Spec.Value + } + + // if the feature is disabled then we should not permit any changes between the old and new clusters other than deletion + if !enabled && oldPDB != nil { + if reflect.DeepEqual(*oldPDB, *newPDB) { + return admission.ResponseAllowed(), nil + } + + return admission.ResponseBadRequest(fmt.Sprintf("'%s' feature is disabled, will only permit removal of Scheduling Customization fields until reenabled", common.SchedulingCustomizationFeatureName)), nil + } + + if !enabled && oldPDB == nil { + return admission.ResponseBadRequest(fmt.Sprintf("the '%s' feature must be enabled in order to configure a Priority Class or Pod Disruption Budget", common.SchedulingCustomizationFeatureName)), nil + } + + minAvailStr := newPDB.MinAvailable + maxUnavailStr := newPDB.MaxUnavailable + + if (minAvailStr == "" && maxUnavailStr == "") || + (minAvailStr == "0" && maxUnavailStr == "0") || + (minAvailStr != "" && minAvailStr != "0") && (maxUnavailStr != "" && maxUnavailStr != "0") { + + return admission.ResponseBadRequest("both minAvailable and maxUnavailable cannot be set to a non zero value, at least one must be omitted or set to zero"), nil + } + + minAvailIsString := false + maxUnavailIsString := false + + minAvailInt, err := strconv.Atoi(minAvailStr) + if err != nil { + minAvailIsString = minAvailStr != "" + } + + maxUnavailInt, err := strconv.Atoi(maxUnavailStr) + if err != nil { + maxUnavailIsString = maxUnavailStr != "" + } + + if !minAvailIsString && minAvailInt < 0 { + return admission.ResponseBadRequest("minAvailable cannot be set to a negative integer"), nil + } + + if !maxUnavailIsString && maxUnavailInt < 0 { + return admission.ResponseBadRequest("maxUnavailable cannot be set to a negative integer"), nil + } + + if minAvailIsString && !common.PdbPercentageRegex.Match([]byte(minAvailStr)) { + return admission.ResponseBadRequest("minAvailable must be a whole integer or a percentage value between 1 and 100, regex used is '^([1-9]|[1-9][0-9]|100)%$'"), nil + } + + if maxUnavailIsString && maxUnavailStr != "" && !common.PdbPercentageRegex.Match([]byte(maxUnavailStr)) { + return admission.ResponseBadRequest("minAvailable must be a whole integer or a percentage value between 1 and 100, regex used is '^([1-9]|[1-9][0-9]|100)%$'"), nil + } + + return admission.ResponseAllowed(), nil +} + func validateAgentDeploymentCustomization(customization *v1.AgentDeploymentCustomization, path *field.Path) field.ErrorList { if customization == nil { return nil @@ -551,6 +705,22 @@ func validateAffinity(overrideAffinity *k8sv1.Affinity, path *field.Path) field. return errList } +func getSchedulingCustomization(cluster *v1.Cluster) *v1.AgentSchedulingCustomization { + if cluster == nil { + return nil + } + + if cluster.Spec.ClusterAgentDeploymentCustomization == nil { + return nil + } + + if cluster.Spec.ClusterAgentDeploymentCustomization.SchedulingCustomization == nil { + return nil + } + + return cluster.Spec.ClusterAgentDeploymentCustomization.SchedulingCustomization +} + func validatePodAffinityTerms(terms []k8sv1.PodAffinityTerm, path *field.Path) field.ErrorList { var errList field.ErrorList diff --git a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go index d9b57a1a0..c0463ab0c 100644 --- a/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go +++ b/pkg/resources/provisioning.cattle.io/v1/cluster/validator_test.go @@ -5,10 +5,14 @@ import ( "strings" "testing" + v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" v1 "github.com/rancher/rancher/pkg/apis/provisioning.cattle.io/v1" rkev1 "github.com/rancher/rancher/pkg/apis/rke.cattle.io/v1" "github.com/rancher/webhook/pkg/admission" + "github.com/rancher/webhook/pkg/resources/common" + "github.com/rancher/wrangler/v3/pkg/generic/fake" "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" admissionv1 "k8s.io/api/admission/v1" k8sv1 "k8s.io/api/core/v1" v12 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1505,3 +1509,640 @@ func Test_validateAgentDeploymentCustomization(t *testing.T) { }) } } + +func Test_validateAgentSchedulingCustomizationPriorityClass(t *testing.T) { + preemptionNever := k8sv1.PreemptionPolicy("Never") + preemptionInvalid := k8sv1.PreemptionPolicy("rancher") + + tests := []struct { + name string + cluster *v1.Cluster + oldCluster *v1.Cluster + featureEnabled bool + shouldSucceed bool + }{ + { + name: "empty priority class - feature enabled", + cluster: &v1.Cluster{}, + shouldSucceed: true, + featureEnabled: true, + }, + { + name: "empty priority class - feature disabled", + cluster: &v1.Cluster{}, + shouldSucceed: true, + featureEnabled: true, + }, + { + name: "valid priority class with default preemption", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 123456, + }, + }, + }, + }, + }, + }, + { + name: "valid priority class with custom preemption", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 123456, + Preemption: &preemptionNever, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - value too large", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 1234567891234567890, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - value too small", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: -1234567891234567890, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - preemption value invalid", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 24321, + Preemption: &preemptionInvalid, + }, + }, + }, + }, + }, + }, + { + name: "invalid priority class - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 24321, + Preemption: &preemptionInvalid, + }, + }, + }, + }, + }, + }, + { + name: "invalid update attempt - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 4321, + }, + }, + }, + }, + }, + }, + { + name: "valid update attempt - feature is enabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 4321, + }, + }, + }, + }, + }, + }, + { + name: "valid update attempt - feature is disabled, but fields are unchanged", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + }, + { + name: "valid update attempt - field is removed while feature is disabled", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PriorityClass: &v1.PriorityClassSpec{ + Value: 1234, + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{}, + }, + }, + } + + t.Parallel() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + a := provisioningAdmitter{ + featureCache: createMockFeatureCache(ctrl, common.SchedulingCustomizationFeatureName, tt.featureEnabled), + } + + response, err := a.validatePriorityClass(tt.oldCluster, tt.cluster) + assert.Equal(t, tt.shouldSucceed, response.Allowed) + assert.NoError(t, err) + }) + } +} + +func Test_validateAgentSchedulingCustomizationPodDisruptionBudget(t *testing.T) { + tests := []struct { + name string + cluster *v1.Cluster + oldCluster *v1.Cluster + featureEnabled bool + shouldSucceed bool + }{ + { + name: "no scheduling customization - feature enabled", + cluster: &v1.Cluster{}, + shouldSucceed: true, + featureEnabled: true, + }, + { + name: "no scheduling customization - feature disabled", + cluster: &v1.Cluster{}, + shouldSucceed: true, + featureEnabled: false, + }, + { + name: "invalid PDB configuration - negative min available integer", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "-1", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - negative max unavailable integer", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MaxUnavailable: "-1", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - both fields set", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MaxUnavailable: "1", + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string passed to max unavailable", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MaxUnavailable: "five", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string passed to min available", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "five", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string with invalid percentage number set for min available", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "5.5%", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - string with invalid percentage number set for max unavailable", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MaxUnavailable: "5.5%", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB configuration - both set to zero", + shouldSucceed: false, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "0", + MaxUnavailable: "0", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - max unavailable set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MaxUnavailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - min available set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - min available set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "1", + MaxUnavailable: "0", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - max unavailable set to integer", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "0", + MaxUnavailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - max unavailable set to percentage", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MaxUnavailable: "50%", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - min available set to percentage", + shouldSucceed: true, + featureEnabled: true, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB configuration - updating from percentage to int", + shouldSucceed: true, + featureEnabled: true, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB reconfiguration - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "invalid PDB creation - feature is disabled", + shouldSucceed: false, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{}, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "1", + }, + }, + }, + }, + }, + }, + { + name: "valid PDB reconfiguration - field is removed while feature is disabled", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{}, + }, + }, + { + name: "valid update - field is unchanged while feature is disabled", + shouldSucceed: true, + featureEnabled: false, + oldCluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + cluster: &v1.Cluster{ + Spec: v1.ClusterSpec{ + ClusterAgentDeploymentCustomization: &v1.AgentDeploymentCustomization{ + SchedulingCustomization: &v1.AgentSchedulingCustomization{ + PodDisruptionBudget: &v1.PodDisruptionBudgetSpec{ + MinAvailable: "50%", + }, + }, + }, + }, + }, + }, + } + + t.Parallel() + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + a := provisioningAdmitter{ + featureCache: createMockFeatureCache(ctrl, common.SchedulingCustomizationFeatureName, tt.featureEnabled), + } + + response, err := a.validatePodDisruptionBudget(tt.oldCluster, tt.cluster) + assert.Equal(t, tt.shouldSucceed, response.Allowed) + assert.NoError(t, err) + }) + } +} + +func createMockFeatureCache(ctrl *gomock.Controller, featureName string, enabled bool) *fake.MockNonNamespacedCacheInterface[*v3.Feature] { + featureCache := fake.NewMockNonNamespacedCacheInterface[*v3.Feature](ctrl) + featureCache.EXPECT().Get(featureName).DoAndReturn(func(string) (*v3.Feature, error) { + return &v3.Feature{ + Spec: v3.FeatureSpec{ + Value: &enabled, + }, + }, nil + }).AnyTimes() + return featureCache +} diff --git a/pkg/server/handlers.go b/pkg/server/handlers.go index da9143b85..06c72f737 100644 --- a/pkg/server/handlers.go +++ b/pkg/server/handlers.go @@ -45,6 +45,7 @@ func Validation(clients *clients.Clients) ([]admission.ValidatingAdmissionHandle clients.K8s.AuthorizationV1().SubjectAccessReviews(), clients.Management.PodSecurityAdmissionConfigurationTemplate().Cache(), userCache, + clients.Management.Feature().Cache(), settingCache, )