Merge pull request #62726 from xiangpengzhao/kubeadm-json-name

Automatic merge from submit-queue (batch tested with PRs 62726, 60085, 62583). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

 Refactor kubeadm api validation.

**What this PR does / why we need it**:
This PR refactor kubeadm api validation to use field json name. When users get a validation error, they can easily know which field is invalid in their config files.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:
@fabriziopandini I remember we mentioned this refactoring in some comment. I didn't see this change was done, so I send this PR to address this.

**Release note**:

```release-note
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2018-04-19 11:00:13 -07:00 committed by GitHub
commit 500b63aed6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 56 additions and 54 deletions

View File

@ -18,6 +18,7 @@ go_library(
"//pkg/kubelet/apis/kubeletconfig/validation:go_default_library",
"//pkg/proxy/apis/kubeproxyconfig:go_default_library",
"//pkg/proxy/apis/kubeproxyconfig/scheme:go_default_library",
"//pkg/proxy/apis/kubeproxyconfig/v1alpha1:go_default_library",
"//pkg/proxy/apis/kubeproxyconfig/validation:go_default_library",
"//pkg/registry/core/service/ipallocator:go_default_library",
"//pkg/util/node:go_default_library",

View File

@ -43,6 +43,7 @@ import (
kubeletvalidation "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/validation"
"k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig"
kubeproxyscheme "k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig/scheme"
kubeproxyconfigv1alpha1 "k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig/v1alpha1"
proxyvalidation "k8s.io/kubernetes/pkg/proxy/apis/kubeproxyconfig/validation"
"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
"k8s.io/kubernetes/pkg/util/node"
@ -71,20 +72,20 @@ var requiredAuthzModes = []string{
// ValidateMasterConfiguration validates master configuration and collects all encountered errors
func ValidateMasterConfiguration(c *kubeadm.MasterConfiguration) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidateCloudProvider(c.CloudProvider, field.NewPath("cloudprovider"))...)
allErrs = append(allErrs, ValidateAuthorizationModes(c.AuthorizationModes, field.NewPath("authorization-modes"))...)
allErrs = append(allErrs, ValidateCloudProvider(c.CloudProvider, field.NewPath("cloudProvider"))...)
allErrs = append(allErrs, ValidateAuthorizationModes(c.AuthorizationModes, field.NewPath("authorizationModes"))...)
allErrs = append(allErrs, ValidateNetworking(&c.Networking, field.NewPath("networking"))...)
allErrs = append(allErrs, ValidateCertSANs(c.APIServerCertSANs, field.NewPath("api-server-cert-altnames"))...)
allErrs = append(allErrs, ValidateCertSANs(c.Etcd.ServerCertSANs, field.NewPath("etcd-server-cert-altnames"))...)
allErrs = append(allErrs, ValidateCertSANs(c.Etcd.PeerCertSANs, field.NewPath("etcd-peer-cert-altnames"))...)
allErrs = append(allErrs, ValidateAbsolutePath(c.CertificatesDir, field.NewPath("certificates-dir"))...)
allErrs = append(allErrs, ValidateNodeName(c.NodeName, field.NewPath("node-name"))...)
allErrs = append(allErrs, ValidateCertSANs(c.APIServerCertSANs, field.NewPath("apiServerCertSANs"))...)
allErrs = append(allErrs, ValidateCertSANs(c.Etcd.ServerCertSANs, field.NewPath("etcd").Child("serverCertSANs"))...)
allErrs = append(allErrs, ValidateCertSANs(c.Etcd.PeerCertSANs, field.NewPath("etcd").Child("peerCertSANs"))...)
allErrs = append(allErrs, ValidateAbsolutePath(c.CertificatesDir, field.NewPath("certificatesDir"))...)
allErrs = append(allErrs, ValidateNodeName(c.NodeName, field.NewPath("nodeName"))...)
allErrs = append(allErrs, ValidateToken(c.Token, field.NewPath("token"))...)
allErrs = append(allErrs, ValidateTokenUsages(c.TokenUsages, field.NewPath("tokenUsages"))...)
allErrs = append(allErrs, ValidateTokenGroups(c.TokenUsages, c.TokenGroups, field.NewPath("tokenGroups"))...)
allErrs = append(allErrs, ValidateFeatureGates(c.FeatureGates, field.NewPath("feature-gates"))...)
allErrs = append(allErrs, ValidateAPIEndpoint(c, field.NewPath("api-endpoint"))...)
allErrs = append(allErrs, ValidateProxy(c, field.NewPath("kube-proxy"))...)
allErrs = append(allErrs, ValidateFeatureGates(c.FeatureGates, field.NewPath("featureGates"))...)
allErrs = append(allErrs, ValidateAPIEndpoint(&c.API, field.NewPath("api"))...)
allErrs = append(allErrs, ValidateProxy(c.KubeProxy.Config, field.NewPath("kubeProxy").Child("config"))...)
if features.Enabled(c.FeatureGates, features.DynamicKubeletConfig) {
allErrs = append(allErrs, ValidateKubeletConfiguration(&c.KubeletConfiguration, field.NewPath("kubeletConfiguration"))...)
}
@ -92,14 +93,14 @@ func ValidateMasterConfiguration(c *kubeadm.MasterConfiguration) field.ErrorList
}
// ValidateProxy validates proxy configuration and collects all encountered errors
func ValidateProxy(c *kubeadm.MasterConfiguration, fldPath *field.Path) field.ErrorList {
func ValidateProxy(c *kubeproxyconfigv1alpha1.KubeProxyConfiguration, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
// Convert to the internal version
internalcfg := &kubeproxyconfig.KubeProxyConfiguration{}
err := kubeproxyscheme.Scheme.Convert(c.KubeProxy.Config, internalcfg, nil)
err := kubeproxyscheme.Scheme.Convert(c, internalcfg, nil)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, "KubeProxy.Config", err.Error()))
allErrs = append(allErrs, field.Invalid(fldPath, "", err.Error()))
return allErrs
}
return proxyvalidation.Validate(internalcfg)
@ -108,10 +109,10 @@ func ValidateProxy(c *kubeadm.MasterConfiguration, fldPath *field.Path) field.Er
// ValidateNodeConfiguration validates node configuration and collects all encountered errors
func ValidateNodeConfiguration(c *kubeadm.NodeConfiguration) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidateDiscovery(c, field.NewPath("discovery"))...)
allErrs = append(allErrs, ValidateDiscovery(c)...)
if !filepath.IsAbs(c.CACertPath) || !strings.HasSuffix(c.CACertPath, ".crt") {
allErrs = append(allErrs, field.Invalid(field.NewPath("ca-cert-path"), c.CACertPath, "the ca certificate path must be an absolute path"))
allErrs = append(allErrs, field.Invalid(field.NewPath("caCertPath"), c.CACertPath, "the ca certificate path must be an absolute path"))
}
return allErrs
}
@ -140,17 +141,17 @@ func ValidateAuthorizationModes(authzModes []string, fldPath *field.Path) field.
}
// ValidateDiscovery validates discovery related configuration and collects all encountered errors
func ValidateDiscovery(c *kubeadm.NodeConfiguration, fldPath *field.Path) field.ErrorList {
func ValidateDiscovery(c *kubeadm.NodeConfiguration) field.ErrorList {
allErrs := field.ErrorList{}
if len(c.DiscoveryToken) != 0 {
allErrs = append(allErrs, ValidateToken(c.DiscoveryToken, fldPath)...)
allErrs = append(allErrs, ValidateToken(c.DiscoveryToken, field.NewPath("discoveryToken"))...)
}
if len(c.DiscoveryFile) != 0 {
allErrs = append(allErrs, ValidateDiscoveryFile(c.DiscoveryFile, fldPath)...)
allErrs = append(allErrs, ValidateDiscoveryFile(c.DiscoveryFile, field.NewPath("discoveryFile"))...)
}
allErrs = append(allErrs, ValidateArgSelection(c, fldPath)...)
allErrs = append(allErrs, ValidateToken(c.TLSBootstrapToken, fldPath)...)
allErrs = append(allErrs, ValidateJoinDiscoveryTokenAPIServer(c, fldPath)...)
allErrs = append(allErrs, ValidateArgSelection(c, field.NewPath("discovery"))...)
allErrs = append(allErrs, ValidateToken(c.TLSBootstrapToken, field.NewPath("tlsBootstrapToken"))...)
allErrs = append(allErrs, ValidateJoinDiscoveryTokenAPIServer(c.DiscoveryTokenAPIServers, field.NewPath("discoveryTokenAPIServers"))...)
return allErrs
}
@ -159,22 +160,22 @@ func ValidateDiscovery(c *kubeadm.NodeConfiguration, fldPath *field.Path) field.
func ValidateArgSelection(cfg *kubeadm.NodeConfiguration, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
if len(cfg.DiscoveryToken) != 0 && len(cfg.DiscoveryFile) != 0 {
allErrs = append(allErrs, field.Invalid(fldPath, "", "DiscoveryToken and DiscoveryFile cannot both be set"))
allErrs = append(allErrs, field.Invalid(fldPath, "", "discoveryToken and discoveryFile cannot both be set"))
}
if len(cfg.DiscoveryToken) == 0 && len(cfg.DiscoveryFile) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath, "", "DiscoveryToken or DiscoveryFile must be set"))
allErrs = append(allErrs, field.Invalid(fldPath, "", "discoveryToken or discoveryFile must be set"))
}
if len(cfg.DiscoveryTokenAPIServers) < 1 && len(cfg.DiscoveryToken) != 0 {
allErrs = append(allErrs, field.Required(fldPath, "DiscoveryTokenAPIServers not set"))
allErrs = append(allErrs, field.Required(fldPath, "discoveryTokenAPIServers not set"))
}
if len(cfg.DiscoveryFile) != 0 && len(cfg.DiscoveryTokenCACertHashes) != 0 {
allErrs = append(allErrs, field.Invalid(fldPath, "", "DiscoveryTokenCACertHashes cannot be used with DiscoveryFile"))
allErrs = append(allErrs, field.Invalid(fldPath, "", "discoveryTokenCACertHashes cannot be used with discoveryFile"))
}
if len(cfg.DiscoveryFile) == 0 && len(cfg.DiscoveryToken) != 0 &&
len(cfg.DiscoveryTokenCACertHashes) == 0 && !cfg.DiscoveryTokenUnsafeSkipCAVerification {
allErrs = append(allErrs, field.Invalid(fldPath, "", "using token-based discovery without DiscoveryTokenCACertHashes can be unsafe. set --discovery-token-unsafe-skip-ca-verification to continue"))
allErrs = append(allErrs, field.Invalid(fldPath, "", "using token-based discovery without discoveryTokenCACertHashes can be unsafe. set --discovery-token-unsafe-skip-ca-verification to continue"))
}
// TODO remove once we support multiple api servers
@ -185,9 +186,9 @@ func ValidateArgSelection(cfg *kubeadm.NodeConfiguration, fldPath *field.Path) f
}
// ValidateJoinDiscoveryTokenAPIServer validates discovery token for API server
func ValidateJoinDiscoveryTokenAPIServer(c *kubeadm.NodeConfiguration, fldPath *field.Path) field.ErrorList {
func ValidateJoinDiscoveryTokenAPIServer(apiServers []string, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
for _, m := range c.DiscoveryTokenAPIServers {
for _, m := range apiServers {
_, _, err := net.SplitHostPort(m)
if err != nil {
allErrs = append(allErrs, field.Invalid(fldPath, m, err.Error()))
@ -305,10 +306,10 @@ func ValidateIPNetFromString(subnet string, minAddrs int64, fldPath *field.Path)
// ValidateNetworking validates networking configuration
func ValidateNetworking(c *kubeadm.Networking, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, apivalidation.ValidateDNS1123Subdomain(c.DNSDomain, field.NewPath("dns-domain"))...)
allErrs = append(allErrs, ValidateIPNetFromString(c.ServiceSubnet, constants.MinimumAddressesInServiceSubnet, field.NewPath("service-subnet"))...)
allErrs = append(allErrs, apivalidation.ValidateDNS1123Subdomain(c.DNSDomain, field.NewPath("dnsDomain"))...)
allErrs = append(allErrs, ValidateIPNetFromString(c.ServiceSubnet, constants.MinimumAddressesInServiceSubnet, field.NewPath("serviceSubnet"))...)
if len(c.PodSubnet) != 0 {
allErrs = append(allErrs, ValidateIPNetFromString(c.PodSubnet, constants.MinimumAddressesInServiceSubnet, field.NewPath("pod-subnet"))...)
allErrs = append(allErrs, ValidateIPNetFromString(c.PodSubnet, constants.MinimumAddressesInServiceSubnet, field.NewPath("podSubnet"))...)
}
return allErrs
}
@ -385,7 +386,7 @@ func ValidateFeatureGates(featureGates map[string]bool, fldPath *field.Path) fie
}
// ValidateAPIEndpoint validates API server's endpoint
func ValidateAPIEndpoint(c *kubeadm.MasterConfiguration, fldPath *field.Path) field.ErrorList {
func ValidateAPIEndpoint(c *kubeadm.API, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
endpoint, err := kubeadmutil.GetMasterEndpoint(c)

View File

@ -316,7 +316,7 @@ func TestValidateAPIEndpoint(t *testing.T) {
},
}
for _, rt := range tests {
actual := ValidateAPIEndpoint(rt.s, nil)
actual := ValidateAPIEndpoint(&rt.s.API, nil)
if (len(actual) == 0) != rt.expected {
t.Errorf(
"%s test case failed:\n\texpected: %t\n\t actual: %t",
@ -705,7 +705,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) {
}
for _, successCase := range successCases {
if errs := ValidateProxy(&successCase, nil); len(errs) != 0 {
if errs := ValidateProxy(successCase.KubeProxy.Config, nil); len(errs) != 0 {
t.Errorf("failed ValidateProxy: expect no errors but got %v", errs)
}
}
@ -909,7 +909,7 @@ func TestValidateKubeProxyConfiguration(t *testing.T) {
}
for i, errorCase := range errorCases {
if errs := ValidateProxy(&errorCase.masterConfig, nil); len(errs) == 0 {
if errs := ValidateProxy(errorCase.masterConfig.KubeProxy.Config, nil); len(errs) == 0 {
t.Errorf("%d failed ValidateProxy: expected error for %s, but got no error", i, errorCase.msg)
} else if !strings.Contains(errs[0].Error(), errorCase.msg) {
t.Errorf("%d failed ValidateProxy: unexpected error: %v, expected: %s", i, errs[0], errorCase.msg)
@ -1040,7 +1040,7 @@ func TestValidateJoinDiscoveryTokenAPIServer(t *testing.T) {
},
}
for _, rt := range tests {
actual := ValidateJoinDiscoveryTokenAPIServer(rt.s, nil)
actual := ValidateJoinDiscoveryTokenAPIServer(rt.s.DiscoveryTokenAPIServers, nil)
if (len(actual) == 0) != rt.expected {
t.Errorf(
"failed ValidateJoinDiscoveryTokenAPIServer:\n\texpected: %t\n\t actual: %t",

View File

@ -53,7 +53,7 @@ func EnsureProxyAddon(cfg *kubeadmapi.MasterConfiguration, client clientset.Inte
}
// Generate Master Enpoint kubeconfig file
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(cfg)
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(&cfg.API)
if err != nil {
return err
}

View File

@ -142,7 +142,7 @@ func getKubeConfigSpecs(cfg *kubeadmapi.MasterConfiguration) (map[string]*kubeCo
return nil, fmt.Errorf("couldn't create a kubeconfig; the CA files couldn't be loaded: %v", err)
}
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(cfg)
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(&cfg.API)
if err != nil {
return nil, err
}
@ -279,7 +279,7 @@ func WriteKubeConfigWithClientCert(out io.Writer, cfg *kubeadmapi.MasterConfigur
return fmt.Errorf("couldn't create a kubeconfig; the CA files couldn't be loaded: %v", err)
}
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(cfg)
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(&cfg.API)
if err != nil {
return err
}
@ -306,7 +306,7 @@ func WriteKubeConfigWithToken(out io.Writer, cfg *kubeadmapi.MasterConfiguration
return fmt.Errorf("couldn't create a kubeconfig; the CA files couldn't be loaded: %v", err)
}
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(cfg)
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(&cfg.API)
if err != nil {
return err
}

View File

@ -146,7 +146,7 @@ func TestGetKubeConfigSpecs(t *testing.T) {
}
// Asserts MasterConfiguration values injected into spec
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(cfg)
masterEndpoint, err := kubeadmutil.GetMasterEndpoint(&cfg.API)
if err != nil {
t.Error(err)
}

View File

@ -28,9 +28,9 @@ import (
// GetMasterEndpoint returns a properly formatted Master Endpoint
// or passes the error from GetMasterHostPort.
func GetMasterEndpoint(cfg *kubeadmapi.MasterConfiguration) (string, error) {
func GetMasterEndpoint(api *kubeadmapi.API) (string, error) {
hostPort, err := GetMasterHostPort(cfg)
hostPort, err := GetMasterHostPort(api)
if err != nil {
return "", err
}
@ -39,27 +39,27 @@ func GetMasterEndpoint(cfg *kubeadmapi.MasterConfiguration) (string, error) {
// GetMasterHostPort returns a properly formatted Master hostname or IP and port pair, or error
// if the hostname or IP address can not be parsed or port is outside the valid TCP range.
func GetMasterHostPort(cfg *kubeadmapi.MasterConfiguration) (string, error) {
func GetMasterHostPort(api *kubeadmapi.API) (string, error) {
var masterIP string
var portStr string
if len(cfg.API.ControlPlaneEndpoint) > 0 {
if strings.Contains(cfg.API.ControlPlaneEndpoint, ":") {
if len(api.ControlPlaneEndpoint) > 0 {
if strings.Contains(api.ControlPlaneEndpoint, ":") {
var err error
masterIP, portStr, err = net.SplitHostPort(cfg.API.ControlPlaneEndpoint)
masterIP, portStr, err = net.SplitHostPort(api.ControlPlaneEndpoint)
if err != nil {
return "", fmt.Errorf("invalid value `%s` given for `ControlPlaneEndpoint`: %s", cfg.API.ControlPlaneEndpoint, err)
return "", fmt.Errorf("invalid value `%s` given for `ControlPlaneEndpoint`: %s", api.ControlPlaneEndpoint, err)
}
} else {
masterIP = cfg.API.ControlPlaneEndpoint
masterIP = api.ControlPlaneEndpoint
}
errs := validation.IsDNS1123Subdomain(masterIP)
if len(errs) > 0 {
return "", fmt.Errorf("error parsing `ControlPlaneEndpoint` to valid dns subdomain with errors: %s", errs)
}
} else {
ip := net.ParseIP(cfg.API.AdvertiseAddress)
ip := net.ParseIP(api.AdvertiseAddress)
if ip == nil {
return "", fmt.Errorf("error parsing address %s", cfg.API.AdvertiseAddress)
return "", fmt.Errorf("error parsing address %s", api.AdvertiseAddress)
}
masterIP = ip.String()
}
@ -73,7 +73,7 @@ func GetMasterHostPort(cfg *kubeadmapi.MasterConfiguration) (string, error) {
port = int32(portInt)
fmt.Println("[endpoint] WARNING: specifying a port for `ControlPlaneEndpoint` overrides `BindPort`")
} else {
port = cfg.API.BindPort
port = api.BindPort
}
if port < 0 || port > 65535 {

View File

@ -198,7 +198,7 @@ func TestGetMasterEndpoint(t *testing.T) {
},
}
for _, rt := range tests {
actual, err := GetMasterEndpoint(rt.cfg)
actual, err := GetMasterEndpoint(&rt.cfg.API)
if err != nil && rt.expected {
t.Error(err)
}
@ -308,7 +308,7 @@ func TestGetMasterHostPort(t *testing.T) {
},
}
for _, rt := range tests {
actual, err := GetMasterHostPort(rt.cfg)
actual, err := GetMasterHostPort(&rt.cfg.API)
if err != nil && rt.expected {
t.Error(err)
}