From 383e6cc9a38511db2ac5468d61fe343f254cbe87 Mon Sep 17 00:00:00 2001 From: noqcks Date: Mon, 30 Jul 2018 16:22:22 -0400 Subject: [PATCH] Add validation for kube-scheduler adding validation for componentconfig adding validation to cmd kube-scheduler Add support for ipv6 in IsValidSocketAddr function updating copyright date in componentconfig/validation/validation.go updating copyright date in componentconfig/validation/validation_test.go adding validation for cli options adding BUILD files updating validate function to return []errors in cmd/kube-scheduler ok, really returning []error this time adding comments for exported componentconfig Validation functions silly me, not checking structs along the way :'( refactor to avoid else statement moving policy nil check up one function rejigging some deprecated cmd validations stumbling my way around validation slowly but surely updating according to review from @bsalamat - not validating leader election config unless leader election is enabled - leader election time values cannot be zero - removing validation for KubeConfigFile - removing validation for scheduler policy leader elect options should be non-negative adding test cases for renewDeadline and leaseDuration being zero fixing logic in componentconfig validation :sweat_smile: removing KubeConfigFile reference from tests as it was removed in master https://github.com/kubernetes/kubernetes/commit/2ff9bd6699ccdfb1f46c4aeda01518990ff71eda removing bogus space after var assignment adding more tests for componentconfig based on feedback making updates to validation because types were moved on master update bazel build adding validation for staging/apimachinery adding validation for staging/apiserver adding fieldPaths for staging validations moving staging validations out of componentconfig updating test case scenario for staging/apimachinery ./hack/update-bazel.sh moving kube-scheduler validations from componentconfig ./hack/update-bazel.sh removing non-negative check for QPS resourceLock required adding HardPodAffinitySymmetricWeight 0-100 range to cmd flag help section Kubernetes-commit: 0334a34e4af9b56ffa4d8fe17514c931c69db84b --- pkg/apis/config/validation/validation.go | 46 +++++++ pkg/apis/config/validation/validation_test.go | 112 ++++++++++++++++++ 2 files changed, 158 insertions(+) create mode 100644 pkg/apis/config/validation/validation.go create mode 100644 pkg/apis/config/validation/validation_test.go diff --git a/pkg/apis/config/validation/validation.go b/pkg/apis/config/validation/validation.go new file mode 100644 index 000000000..00cadf101 --- /dev/null +++ b/pkg/apis/config/validation/validation.go @@ -0,0 +1,46 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/apis/config" +) + +// ValidateLeaderElectionConfiguration ensures validation of the LeaderElectionConfiguration struct +func ValidateLeaderElectionConfiguration(cc *config.LeaderElectionConfiguration, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if !cc.LeaderElect { + return allErrs + } + if cc.LeaseDuration.Duration <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("leaseDuration"), cc.LeaseDuration, "must be greater than zero")) + } + if cc.RenewDeadline.Duration <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("renewDeadline"), cc.LeaseDuration, "must be greater than zero")) + } + if cc.RetryPeriod.Duration <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("retryPeriod"), cc.RetryPeriod, "must be greater than zero")) + } + if cc.LeaseDuration.Duration < cc.RenewDeadline.Duration { + allErrs = append(allErrs, field.Invalid(fldPath.Child("leaseDuration"), cc.RenewDeadline, "LeaseDuration must be greater than RenewDeadline")) + } + if len(cc.ResourceLock) == 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("resourceLock"), cc.RenewDeadline, "resourceLock is required")) + } + return allErrs +} diff --git a/pkg/apis/config/validation/validation_test.go b/pkg/apis/config/validation/validation_test.go new file mode 100644 index 000000000..b55c9fb1b --- /dev/null +++ b/pkg/apis/config/validation/validation_test.go @@ -0,0 +1,112 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package validation + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/apis/config" + "testing" + "time" +) + +func TestValidateLeaderElectionConfiguration(t *testing.T) { + validConfig := &config.LeaderElectionConfiguration{ + ResourceLock: "configmap", + LeaderElect: true, + LeaseDuration: metav1.Duration{Duration: 30 * time.Second}, + RenewDeadline: metav1.Duration{Duration: 15 * time.Second}, + RetryPeriod: metav1.Duration{Duration: 5 * time.Second}, + } + + renewDeadlineExceedsLeaseDuration := validConfig.DeepCopy() + renewDeadlineExceedsLeaseDuration.RenewDeadline = metav1.Duration{Duration: 45 * time.Second} + + renewDeadlineZero := validConfig.DeepCopy() + renewDeadlineZero.RenewDeadline = metav1.Duration{Duration: 0 * time.Second} + + leaseDurationZero := validConfig.DeepCopy() + leaseDurationZero.LeaseDuration = metav1.Duration{Duration: 0 * time.Second} + + negativeValForRetryPeriod := validConfig.DeepCopy() + negativeValForRetryPeriod.RetryPeriod = metav1.Duration{Duration: -45 * time.Second} + + negativeValForLeaseDuration := validConfig.DeepCopy() + negativeValForLeaseDuration.LeaseDuration = metav1.Duration{Duration: -45 * time.Second} + + negativeValForRenewDeadline := validConfig.DeepCopy() + negativeValForRenewDeadline.RenewDeadline = metav1.Duration{Duration: -45 * time.Second} + + LeaderElectButLeaderElectNotEnabled := validConfig.DeepCopy() + LeaderElectButLeaderElectNotEnabled.LeaderElect = false + LeaderElectButLeaderElectNotEnabled.LeaseDuration = metav1.Duration{Duration: -45 * time.Second} + + resourceLockNotDefined := validConfig.DeepCopy() + resourceLockNotDefined.ResourceLock = "" + + scenarios := map[string]struct { + expectedToFail bool + config *config.LeaderElectionConfiguration + }{ + "good": { + expectedToFail: false, + config: validConfig, + }, + "good-dont-check-leader-config-if-not-enabled": { + expectedToFail: false, + config: LeaderElectButLeaderElectNotEnabled, + }, + "bad-renew-deadline-exceeds-lease-duration": { + expectedToFail: true, + config: renewDeadlineExceedsLeaseDuration, + }, + "bad-negative-value-for-retry-period": { + expectedToFail: true, + config: negativeValForRetryPeriod, + }, + "bad-negative-value-for-lease-duration": { + expectedToFail: true, + config: negativeValForLeaseDuration, + }, + "bad-negative-value-for-renew-deadline": { + expectedToFail: true, + config: negativeValForRenewDeadline, + }, + "bad-renew-deadline-zero": { + expectedToFail: true, + config: renewDeadlineZero, + }, + "bad-lease-duration-zero": { + expectedToFail: true, + config: leaseDurationZero, + }, + "bad-resource-lock-not-defined": { + expectedToFail: true, + config: resourceLockNotDefined, + }, + } + + for name, scenario := range scenarios { + errs := ValidateLeaderElectionConfiguration(scenario.config, field.NewPath("leaderElectionConfiguration")) + if len(errs) == 0 && scenario.expectedToFail { + t.Errorf("Unexpected success for scenario: %s", name) + } + if len(errs) > 0 && !scenario.expectedToFail { + t.Errorf("Unexpected failure for scenario: %s - %+v", name, errs) + } + } +}