Skip to content
Permalink
Browse files

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 #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... #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
```
  • Loading branch information...
Kubernetes Submit Queue
Kubernetes Submit Queue committed Sep 4, 2018
2 parents 7548764 + 0334a34 commit f3b98a08b05257fbc3c19b52ced70ea67c546b1e
@@ -17,13 +17,15 @@ go_library(
"//pkg/scheduler/apis/config:go_default_library",
"//pkg/scheduler/apis/config/scheme:go_default_library",
"//pkg/scheduler/apis/config/v1alpha1:go_default_library",
"//pkg/scheduler/apis/config/validation:go_default_library",
"//pkg/scheduler/factory:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/config:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/server/options:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/client-go/informers:go_default_library",
@@ -58,6 +60,7 @@ filegroup(
go_test(
name = "go_default_test",
srcs = [
"deprecated_test.go",
"insecure_serving_test.go",
"options_test.go",
],
@@ -20,6 +20,7 @@ import (
"fmt"
"github.com/spf13/pflag"

"k8s.io/apimachinery/pkg/util/validation/field"
kubeschedulerconfig "k8s.io/kubernetes/pkg/scheduler/apis/config"
"k8s.io/kubernetes/pkg/scheduler/factory"
)
@@ -43,7 +44,6 @@ func (o *DeprecatedOptions) AddFlags(fs *pflag.FlagSet, cfg *kubeschedulerconfig
}

// TODO: unify deprecation mechanism, string prefix or MarkDeprecated (the latter hides the flag. We also don't want that).

fs.StringVar(&o.AlgorithmProvider, "algorithm-provider", o.AlgorithmProvider, "DEPRECATED: the scheduling algorithm provider to use, one of: "+factory.ListAlgorithmProviders())
fs.StringVar(&o.PolicyConfigFile, "policy-config-file", o.PolicyConfigFile, "DEPRECATED: file with scheduler policy configuration. This file is used if policy ConfigMap is not provided or --use-legacy-policy-config=true")
usage := fmt.Sprintf("DEPRECATED: name of the ConfigMap object that contains scheduler's policy configuration. It must exist in the system namespace before scheduler initialization if --use-legacy-policy-config=false. The config must be provided as the value of an element in 'Data' map with the key='%v'", kubeschedulerconfig.SchedulerPolicyConfigMapKey)
@@ -63,18 +63,20 @@ func (o *DeprecatedOptions) AddFlags(fs *pflag.FlagSet, cfg *kubeschedulerconfig

fs.Int32Var(&cfg.HardPodAffinitySymmetricWeight, "hard-pod-affinity-symmetric-weight", cfg.HardPodAffinitySymmetricWeight,
"RequiredDuringScheduling affinity is not symmetric, but there is an implicit PreferredDuringScheduling affinity rule corresponding "+
"to every RequiredDuringScheduling affinity rule. --hard-pod-affinity-symmetric-weight represents the weight of implicit PreferredDuringScheduling affinity rule.")
"to every RequiredDuringScheduling affinity rule. --hard-pod-affinity-symmetric-weight represents the weight of implicit PreferredDuringScheduling affinity rule. Must be in the range 0-100.")
fs.MarkDeprecated("hard-pod-affinity-symmetric-weight", "This option was moved to the policy configuration file")
fs.StringVar(&cfg.FailureDomains, "failure-domains", cfg.FailureDomains, "Indicate the \"all topologies\" set for an empty topologyKey when it's used for PreferredDuringScheduling pod anti-affinity.")
fs.MarkDeprecated("failure-domains", "Doesn't have any effect. Will be removed in future version.")
}

// Validate validates the deprecated scheduler options.
func (o *DeprecatedOptions) Validate() []error {
if o == nil {
return nil
var errs []error

if o.UseLegacyPolicyConfig && len(o.PolicyConfigFile) == 0 {
errs = append(errs, field.Required(field.NewPath("policyConfigFile"), "required when --use-legacy-policy-config is true"))
}
return nil
return errs
}

// ApplyTo sets cfg.AlgorithmSource from flags passed on the command line in the following precedence order:
@@ -0,0 +1,55 @@
/*
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 options

import (
"testing"
)

func TestValidateDeprecatedKubeSchedulerConfiguration(t *testing.T) {
scenarios := map[string]struct {
expectedToFail bool
config *DeprecatedOptions
}{
"good": {
expectedToFail: false,
config: &DeprecatedOptions{
PolicyConfigFile: "/some/file",
UseLegacyPolicyConfig: true,
AlgorithmProvider: "",
},
},
"bad-policy-config-file-null": {
expectedToFail: true,
config: &DeprecatedOptions{
PolicyConfigFile: "",
UseLegacyPolicyConfig: true,
AlgorithmProvider: "",
},
},
}

for name, scenario := range scenarios {
errs := scenario.config.Validate()
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)
}
}
}
@@ -47,6 +47,7 @@ import (
"k8s.io/kubernetes/pkg/client/leaderelectionconfig"
kubeschedulerconfig "k8s.io/kubernetes/pkg/scheduler/apis/config"
kubeschedulerscheme "k8s.io/kubernetes/pkg/scheduler/apis/config/scheme"
"k8s.io/kubernetes/pkg/scheduler/apis/config/validation"
"k8s.io/kubernetes/pkg/scheduler/factory"
)

@@ -185,6 +186,9 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error {
func (o *Options) Validate() []error {
var errs []error

if err := validation.ValidateKubeSchedulerConfiguration(&o.ComponentConfig).ToAggregate(); err != nil {
errs = append(errs, err.Errors()...)
}
errs = append(errs, o.SecureServing.Validate()...)
errs = append(errs, o.CombinedInsecureServing.Validate()...)
errs = append(errs, o.Authentication.Validate()...)
@@ -32,6 +32,7 @@ filegroup(
":package-srcs",
"//pkg/scheduler/apis/config/scheme:all-srcs",
"//pkg/scheduler/apis/config/v1alpha1:all-srcs",
"//pkg/scheduler/apis/config/validation:all-srcs",
],
tags = ["automanaged"],
visibility = ["//visibility:public"],
@@ -0,0 +1,41 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = ["validation.go"],
importpath = "k8s.io/kubernetes/pkg/scheduler/apis/config/validation",
visibility = ["//visibility:public"],
deps = [
"//pkg/scheduler/apis/config:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/config/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/apis/config/validation:go_default_library",
],
)

go_test(
name = "go_default_test",
srcs = ["validation_test.go"],
embed = [":go_default_library"],
deps = [
"//pkg/scheduler/apis/config:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/config:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1: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,61 @@
/*
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 (
apimachinery "k8s.io/apimachinery/pkg/apis/config/validation"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
apiserver "k8s.io/apiserver/pkg/apis/config/validation"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
)

// ValidateKubeSchedulerConfiguration ensures validation of the KubeSchedulerConfiguration struct
func ValidateKubeSchedulerConfiguration(cc *config.KubeSchedulerConfiguration) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, apimachinery.ValidateClientConnectionConfiguration(&cc.ClientConnection, field.NewPath("clientConnection"))...)
allErrs = append(allErrs, ValidateKubeSchedulerLeaderElectionConfiguration(&cc.LeaderElection, field.NewPath("leaderElection"))...)
if len(cc.SchedulerName) == 0 {
allErrs = append(allErrs, field.Required(field.NewPath("schedulerName"), ""))
}
for _, msg := range validation.IsValidSocketAddr(cc.HealthzBindAddress) {
allErrs = append(allErrs, field.Invalid(field.NewPath("healthzBindAddress"), cc.HealthzBindAddress, msg))
}
for _, msg := range validation.IsValidSocketAddr(cc.MetricsBindAddress) {
allErrs = append(allErrs, field.Invalid(field.NewPath("metricsBindAddress"), cc.MetricsBindAddress, msg))
}
if cc.HardPodAffinitySymmetricWeight < 0 || cc.HardPodAffinitySymmetricWeight > 100 {
allErrs = append(allErrs, field.Invalid(field.NewPath("hardPodAffinitySymmetricWeight"), cc.HardPodAffinitySymmetricWeight, "not in valid range 0-100"))
}
return allErrs
}

// ValidateKubeSchedulerLeaderElectionConfiguration ensures validation of the KubeSchedulerLeaderElectionConfiguration struct
func ValidateKubeSchedulerLeaderElectionConfiguration(cc *config.KubeSchedulerLeaderElectionConfiguration, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if !cc.LeaderElectionConfiguration.LeaderElect {
return allErrs
}
allErrs = append(allErrs, apiserver.ValidateLeaderElectionConfiguration(&cc.LeaderElectionConfiguration, field.NewPath("leaderElectionConfiguration"))...)
if len(cc.LockObjectNamespace) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("lockObjectNamespace"), ""))
}
if len(cc.LockObjectName) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("lockObjectName"), ""))
}
return allErrs
}
@@ -0,0 +1,140 @@
/*
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 (
apimachinery "k8s.io/apimachinery/pkg/apis/config"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apiserver "k8s.io/apiserver/pkg/apis/config"
"k8s.io/kubernetes/pkg/scheduler/apis/config"
"testing"
"time"
)

func TestValidateKubeSchedulerConfiguration(t *testing.T) {
validConfig := &config.KubeSchedulerConfiguration{
SchedulerName: "me",
HealthzBindAddress: "0.0.0.0:10254",
MetricsBindAddress: "0.0.0.0:10254",
HardPodAffinitySymmetricWeight: 80,
ClientConnection: apimachinery.ClientConnectionConfiguration{
AcceptContentTypes: "application/json",
ContentType: "application/json",
QPS: 10,
Burst: 10,
},
AlgorithmSource: config.SchedulerAlgorithmSource{
Policy: &config.SchedulerPolicySource{
ConfigMap: &config.SchedulerPolicyConfigMapSource{
Namespace: "name",
Name: "name",
},
},
},
LeaderElection: config.KubeSchedulerLeaderElectionConfiguration{
LockObjectNamespace: "name",
LockObjectName: "name",
LeaderElectionConfiguration: apiserver.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},
},
},
}

HardPodAffinitySymmetricWeightGt100 := validConfig.DeepCopy()
HardPodAffinitySymmetricWeightGt100.HardPodAffinitySymmetricWeight = 120

HardPodAffinitySymmetricWeightLt0 := validConfig.DeepCopy()
HardPodAffinitySymmetricWeightLt0.HardPodAffinitySymmetricWeight = -1

lockObjectNameNotSet := validConfig.DeepCopy()
lockObjectNameNotSet.LeaderElection.LockObjectName = ""

lockObjectNamespaceNotSet := validConfig.DeepCopy()
lockObjectNamespaceNotSet.LeaderElection.LockObjectNamespace = ""

metricsBindAddrHostInvalid := validConfig.DeepCopy()
metricsBindAddrHostInvalid.MetricsBindAddress = "0.0.0.0.0:9090"

metricsBindAddrPortInvalid := validConfig.DeepCopy()
metricsBindAddrPortInvalid.MetricsBindAddress = "0.0.0.0:909090"

healthzBindAddrHostInvalid := validConfig.DeepCopy()
healthzBindAddrHostInvalid.HealthzBindAddress = "0.0.0.0.0:9090"

healthzBindAddrPortInvalid := validConfig.DeepCopy()
healthzBindAddrPortInvalid.HealthzBindAddress = "0.0.0.0:909090"

enableContentProfilingSetWithoutEnableProfiling := validConfig.DeepCopy()
enableContentProfilingSetWithoutEnableProfiling.EnableProfiling = false
enableContentProfilingSetWithoutEnableProfiling.EnableContentionProfiling = true

scenarios := map[string]struct {
expectedToFail bool
config *config.KubeSchedulerConfiguration
}{
"good": {
expectedToFail: false,
config: validConfig,
},
"bad-lock-object-names-not-set": {
expectedToFail: true,
config: lockObjectNameNotSet,
},
"bad-lock-object-namespace-not-set": {
expectedToFail: true,
config: lockObjectNamespaceNotSet,
},
"bad-healthz-port-invalid": {
expectedToFail: true,
config: healthzBindAddrPortInvalid,
},
"bad-healthz-host-invalid": {
expectedToFail: true,
config: healthzBindAddrHostInvalid,
},
"bad-metrics-port-invalid": {
expectedToFail: true,
config: metricsBindAddrPortInvalid,
},
"bad-metrics-host-invalid": {
expectedToFail: true,
config: metricsBindAddrHostInvalid,
},
"bad-hard-pod-affinity-symmetric-weight-lt-0": {
expectedToFail: true,
config: HardPodAffinitySymmetricWeightGt100,
},
"bad-hard-pod-affinity-symmetric-weight-gt-100": {
expectedToFail: true,
config: HardPodAffinitySymmetricWeightLt0,
},
}

for name, scenario := range scenarios {
errs := ValidateKubeSchedulerConfiguration(scenario.config)
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)
}
}
}
@@ -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 comments on commit f3b98a0

Please sign in to comment.
You can’t perform that action at this time.