From 579602bba22c6ef3718cf0eb2a2af16a0e49914f Mon Sep 17 00:00:00 2001 From: xiangpengzhao Date: Tue, 17 Apr 2018 19:58:54 +0800 Subject: [PATCH 1/2] Refactor kubeadm api validation. --- .../app/apis/kubeadm/validation/validation.go | 65 ++++++++++--------- .../kubeadm/validation/validation_test.go | 8 +-- cmd/kubeadm/app/phases/addons/proxy/proxy.go | 2 +- .../app/phases/kubeconfig/kubeconfig.go | 6 +- .../app/phases/kubeconfig/kubeconfig_test.go | 2 +- cmd/kubeadm/app/util/endpoint.go | 22 +++---- cmd/kubeadm/app/util/endpoint_test.go | 4 +- 7 files changed, 55 insertions(+), 54 deletions(-) diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go index 7925070b989..6419bd59198 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go @@ -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) diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index a1aca11cbb6..87d82d1fda8 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go @@ -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", diff --git a/cmd/kubeadm/app/phases/addons/proxy/proxy.go b/cmd/kubeadm/app/phases/addons/proxy/proxy.go index a5e3dd2d1a4..500d9f500f5 100644 --- a/cmd/kubeadm/app/phases/addons/proxy/proxy.go +++ b/cmd/kubeadm/app/phases/addons/proxy/proxy.go @@ -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 } diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go index a3e2060204e..02194885a97 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go @@ -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 } diff --git a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go index cbb74be7b28..9b6fccd5cb7 100644 --- a/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go +++ b/cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go @@ -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) } diff --git a/cmd/kubeadm/app/util/endpoint.go b/cmd/kubeadm/app/util/endpoint.go index 7326d321713..ed2c2eb7fb9 100644 --- a/cmd/kubeadm/app/util/endpoint.go +++ b/cmd/kubeadm/app/util/endpoint.go @@ -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 { diff --git a/cmd/kubeadm/app/util/endpoint_test.go b/cmd/kubeadm/app/util/endpoint_test.go index 1fae35f0d29..ba37d1d1989 100644 --- a/cmd/kubeadm/app/util/endpoint_test.go +++ b/cmd/kubeadm/app/util/endpoint_test.go @@ -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) } From 7fa3b981648455b9cd6a75aafe745f969ebe1b0d Mon Sep 17 00:00:00 2001 From: xiangpengzhao Date: Tue, 17 Apr 2018 19:59:14 +0800 Subject: [PATCH 2/2] Auto generated BUILD files. --- cmd/kubeadm/app/apis/kubeadm/validation/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/BUILD b/cmd/kubeadm/app/apis/kubeadm/validation/BUILD index 022e9c7a34c..5611e972b00 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/BUILD +++ b/cmd/kubeadm/app/apis/kubeadm/validation/BUILD @@ -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",