Merge pull request #66799 from noqcks/master
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md. Add validation for kube-scheduler configuration options **What this PR does / why we need it**: This adds validation to the kube-scheduler so that we're not accepting bogus values to the kube-scheduler. As requested by @bsalamat in issue https://github.com/kubernetes/kubernetes/issues/66743 **Which issue(s) this PR fixes**: Fixes #66743 **Special notes for your reviewer**: - Not sure if this validation is too heavy handed. Would love some feedback. - I started working on this before I realized @islinwb was also working on this same problem... https://github.com/kubernetes/kubernetes/pull/66787 I put this PR up anyways since I'm sure good code exists in both. I wasn't aware of the /assign command so didn't assign myself before starting work. - I didn't have time to work on adding validation to deprecated cli options. If the rest of this looks ok, I can finish that up. - I hope the location of IsValidSocketAddr is correct. Lmk if it isn't. **Release note**: ```release-note Adding validation to kube-scheduler at the API level ```
This commit is contained in:
@@ -24,6 +24,7 @@ filegroup(
|
||||
srcs = [
|
||||
":package-srcs",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/apis/config/v1alpha1:all-srcs",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/apis/config/validation:all-srcs",
|
||||
],
|
||||
tags = ["automanaged"],
|
||||
visibility = ["//visibility:public"],
|
||||
|
@@ -0,0 +1,37 @@
|
||||
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
|
||||
|
||||
go_library(
|
||||
name = "go_default_library",
|
||||
srcs = ["validation.go"],
|
||||
importmap = "k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/apis/config/validation",
|
||||
importpath = "k8s.io/apimachinery/pkg/apis/config/validation",
|
||||
visibility = ["//visibility:public"],
|
||||
deps = [
|
||||
"//staging/src/k8s.io/apimachinery/pkg/apis/config:go_default_library",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
|
||||
],
|
||||
)
|
||||
|
||||
go_test(
|
||||
name = "go_default_test",
|
||||
srcs = ["validation_test.go"],
|
||||
embed = [":go_default_library"],
|
||||
deps = [
|
||||
"//staging/src/k8s.io/apimachinery/pkg/apis/config:go_default_library",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
|
||||
],
|
||||
)
|
||||
|
||||
filegroup(
|
||||
name = "package-srcs",
|
||||
srcs = glob(["**"]),
|
||||
tags = ["automanaged"],
|
||||
visibility = ["//visibility:private"],
|
||||
)
|
||||
|
||||
filegroup(
|
||||
name = "all-srcs",
|
||||
srcs = [":package-srcs"],
|
||||
tags = ["automanaged"],
|
||||
visibility = ["//visibility:public"],
|
||||
)
|
@@ -0,0 +1,31 @@
|
||||
/*
|
||||
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/apis/config"
|
||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||
)
|
||||
|
||||
// ValidateClientConnectionConfiguration ensures validation of the ClientConnectionConfiguration struct
|
||||
func ValidateClientConnectionConfiguration(cc *config.ClientConnectionConfiguration, fldPath *field.Path) field.ErrorList {
|
||||
allErrs := field.ErrorList{}
|
||||
if cc.Burst < 0 {
|
||||
allErrs = append(allErrs, field.Invalid(fldPath.Child("burst"), cc.Burst, "must be non-negative"))
|
||||
}
|
||||
return allErrs
|
||||
}
|
@@ -0,0 +1,66 @@
|
||||
/*
|
||||
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/apis/config"
|
||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestValidateClientConnectionConfiguration(t *testing.T) {
|
||||
validConfig := &config.ClientConnectionConfiguration{
|
||||
AcceptContentTypes: "application/json",
|
||||
ContentType: "application/json",
|
||||
QPS: 10,
|
||||
Burst: 10,
|
||||
}
|
||||
|
||||
qpsLessThanZero := validConfig.DeepCopy()
|
||||
qpsLessThanZero.QPS = -1
|
||||
|
||||
burstLessThanZero := validConfig.DeepCopy()
|
||||
burstLessThanZero.Burst = -1
|
||||
|
||||
scenarios := map[string]struct {
|
||||
expectedToFail bool
|
||||
config *config.ClientConnectionConfiguration
|
||||
}{
|
||||
"good": {
|
||||
expectedToFail: false,
|
||||
config: validConfig,
|
||||
},
|
||||
"good-qps-less-than-zero": {
|
||||
expectedToFail: false,
|
||||
config: qpsLessThanZero,
|
||||
},
|
||||
"bad-burst-less-then-zero": {
|
||||
expectedToFail: true,
|
||||
config: burstLessThanZero,
|
||||
},
|
||||
}
|
||||
|
||||
for name, scenario := range scenarios {
|
||||
errs := ValidateClientConnectionConfiguration(scenario.config, field.NewPath("clientConnectionConfiguration"))
|
||||
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)
|
||||
}
|
||||
}
|
||||
}
|
@@ -21,6 +21,7 @@ import (
|
||||
"math"
|
||||
"net"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||
@@ -389,3 +390,18 @@ func hasChDirPrefix(value string) []string {
|
||||
}
|
||||
return errs
|
||||
}
|
||||
|
||||
// IsSocketAddr checks that a string conforms is a valid socket address
|
||||
// as defined in RFC 789. (e.g 0.0.0.0:10254 or [::]:10254))
|
||||
func IsValidSocketAddr(value string) []string {
|
||||
var errs []string
|
||||
ip, port, err := net.SplitHostPort(value)
|
||||
if err != nil {
|
||||
return append(errs, "must be a valid socket address format, (e.g. 0.0.0.0:10254 or [::]:10254)")
|
||||
return errs
|
||||
}
|
||||
portInt, _ := strconv.Atoi(port)
|
||||
errs = append(errs, IsValidPortNum(portInt)...)
|
||||
errs = append(errs, IsValidIP(ip)...)
|
||||
return errs
|
||||
}
|
||||
|
@@ -511,3 +511,31 @@ func TestIsFullyQualifiedName(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsValidSocketAddr(t *testing.T) {
|
||||
goodValues := []string{
|
||||
"0.0.0.0:10254",
|
||||
"127.0.0.1:8888",
|
||||
"[2001:db8:1f70::999:de8:7648:6e8]:10254",
|
||||
"[::]:10254",
|
||||
}
|
||||
for _, val := range goodValues {
|
||||
if errs := IsValidSocketAddr(val); len(errs) != 0 {
|
||||
t.Errorf("expected no errors for %q: %v", val, errs)
|
||||
}
|
||||
}
|
||||
|
||||
badValues := []string{
|
||||
"0.0.0.0.0:2020",
|
||||
"0.0.0.0",
|
||||
"6.6.6.6:909090",
|
||||
"2001:db8:1f70::999:de8:7648:6e8:87567:102545",
|
||||
"",
|
||||
"*",
|
||||
}
|
||||
for _, val := range badValues {
|
||||
if errs := IsValidSocketAddr(val); len(errs) == 0 {
|
||||
t.Errorf("expected errors for %q", val)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@@ -25,6 +25,7 @@ filegroup(
|
||||
srcs = [
|
||||
":package-srcs",
|
||||
"//staging/src/k8s.io/apiserver/pkg/apis/config/v1alpha1:all-srcs",
|
||||
"//staging/src/k8s.io/apiserver/pkg/apis/config/validation:all-srcs",
|
||||
],
|
||||
tags = ["automanaged"],
|
||||
visibility = ["//visibility:public"],
|
||||
|
@@ -0,0 +1,38 @@
|
||||
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
|
||||
|
||||
go_library(
|
||||
name = "go_default_library",
|
||||
srcs = ["validation.go"],
|
||||
importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/apis/config/validation",
|
||||
importpath = "k8s.io/apiserver/pkg/apis/config/validation",
|
||||
visibility = ["//visibility:public"],
|
||||
deps = [
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
|
||||
"//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library",
|
||||
],
|
||||
)
|
||||
|
||||
go_test(
|
||||
name = "go_default_test",
|
||||
srcs = ["validation_test.go"],
|
||||
embed = [":go_default_library"],
|
||||
deps = [
|
||||
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
|
||||
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
|
||||
"//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library",
|
||||
],
|
||||
)
|
||||
|
||||
filegroup(
|
||||
name = "package-srcs",
|
||||
srcs = glob(["**"]),
|
||||
tags = ["automanaged"],
|
||||
visibility = ["//visibility:private"],
|
||||
)
|
||||
|
||||
filegroup(
|
||||
name = "all-srcs",
|
||||
srcs = [":package-srcs"],
|
||||
tags = ["automanaged"],
|
||||
visibility = ["//visibility:public"],
|
||||
)
|
@@ -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
|
||||
}
|
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
Reference in New Issue
Block a user