From 5e8ca29a76eb19517044fa54fe8ede28a199e488 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Wed, 14 Dec 2016 10:37:20 +0100 Subject: [PATCH] Clean up apiserver and federation defaulting and validation --- cmd/kube-apiserver/app/options/validation.go | 36 +++++++++------- cmd/kube-apiserver/app/server.go | 29 ++++++------- examples/apiserver/apiserver.go | 17 +++++--- .../app/options/validation.go | 32 ++++++++++++++ .../cmd/federation-apiserver/app/server.go | 17 +++++--- pkg/genericapiserver/config.go | 41 ------------------ .../options/server_run_options.go | 43 ++++++++++++++++++- pkg/genericapiserver/validation/BUILD | 19 -------- 8 files changed, 128 insertions(+), 106 deletions(-) create mode 100644 federation/cmd/federation-apiserver/app/options/validation.go delete mode 100644 pkg/genericapiserver/validation/BUILD diff --git a/cmd/kube-apiserver/app/options/validation.go b/cmd/kube-apiserver/app/options/validation.go index de273a1f442..df4c124ffdb 100644 --- a/cmd/kube-apiserver/app/options/validation.go +++ b/cmd/kube-apiserver/app/options/validation.go @@ -1,5 +1,5 @@ /* -Copyright 2014 The Kubernetes Authors. +Copyright 2016 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. @@ -18,45 +18,49 @@ package options import ( "fmt" - - utilerrors "k8s.io/kubernetes/pkg/util/errors" ) // TODO: Longer term we should read this from some config store, rather than a flag. -func verifyClusterIPFlags(options *ServerRunOptions) []error { +func validateClusterIPFlags(options *ServerRunOptions) []error { errors := []error{} if options.ServiceClusterIPRange.IP == nil { - errors = append(errors, fmt.Errorf("No --service-cluster-ip-range specified")) + errors = append(errors, fmt.Errorf("no --service-cluster-ip-range specified")) } var ones, bits = options.ServiceClusterIPRange.Mask.Size() if bits-ones > 20 { - errors = append(errors, fmt.Errorf("Specified --service-cluster-ip-range is too large")) + errors = append(errors, fmt.Errorf("specified --service-cluster-ip-range is too large")) } return errors } -func verifyServiceNodePort(options *ServerRunOptions) []error { +func validateServiceNodePort(options *ServerRunOptions) []error { errors := []error{} if options.KubernetesServiceNodePort < 0 || options.KubernetesServiceNodePort > 65535 { - errors = append(errors, fmt.Errorf("--kubernetes-service-node-port %v must be between 0 and 65535, inclusive. If 0, the Kubernetes master service will be of type ClusterIP.", options.KubernetesServiceNodePort)) + errors = append(errors, fmt.Errorf("--kubernetes-service-node-port %v must be between 0 and 65535, inclusive. If 0, the Kubernetes master service will be of type ClusterIP", options.KubernetesServiceNodePort)) } if options.KubernetesServiceNodePort > 0 && !options.ServiceNodePortRange.Contains(options.KubernetesServiceNodePort) { - errors = append(errors, fmt.Errorf("Kubernetes service port range %v doesn't contain %v", options.ServiceNodePortRange, (options.KubernetesServiceNodePort))) + errors = append(errors, fmt.Errorf("kubernetes service port range %v doesn't contain %v", options.ServiceNodePortRange, (options.KubernetesServiceNodePort))) } return errors } -func ValidateRunOptions(options *ServerRunOptions) error { - errors := []error{} - if errs := verifyClusterIPFlags(options); len(errs) > 0 { +func (options *ServerRunOptions) Validate() []error { + var errors []error + if errs := options.Etcd.Validate(); len(errs) > 0 { errors = append(errors, errs...) } - if errs := verifyServiceNodePort(options); len(errs) > 0 { + if errs := validateClusterIPFlags(options); len(errs) > 0 { errors = append(errors, errs...) } - if err := utilerrors.NewAggregate(errors); err != nil { - return fmt.Errorf("validate server run options failed: %v", err) + if errs := validateServiceNodePort(options); len(errs) > 0 { + errors = append(errors, errs...) } - return nil + if errs := options.SecureServing.Validate(); len(errs) > 0 { + errors = append(errors, errs...) + } + if errs := options.InsecureServing.Validate("insecure-port"); len(errs) > 0 { + errors = append(errors, errs...) + } + return errors } diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 8c9d7b1c573..dc3115718ce 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -81,33 +81,30 @@ cluster's shared state through which all other components interact.`, // Run runs the specified APIServer. This should never exit. func Run(s *options.ServerRunOptions) error { - if errs := s.Etcd.Validate(); len(errs) > 0 { - return utilerrors.NewAggregate(errs) - } - if err := s.GenericServerRunOptions.DefaultExternalAddress(s.SecureServing, s.InsecureServing); err != nil { + // set defaults + if err := s.GenericServerRunOptions.DefaultAdvertiseAddress(s.SecureServing, s.InsecureServing); err != nil { return err } - serviceIPRange, apiServerServiceIP, err := master.DefaultServiceIPRange(s.ServiceClusterIPRange) if err != nil { return fmt.Errorf("error determining service IP ranges: %v", err) } - if err := s.SecureServing.MaybeDefaultWithSelfSignedCerts(s.GenericServerRunOptions.AdvertiseAddress.String(), apiServerServiceIP); err != nil { return fmt.Errorf("error creating self-signed certificates: %v", err) } - - // TODO(sttts): change signature of DefaultAndValidateRunOptions to aggregate errors - genericapiserver.DefaultAndValidateRunOptions(s.GenericServerRunOptions) - - // TODO(sttts): move all defaulting and validation above into cmd/kube-apiserver/app/options.DefaultAndValidateRunOptions() - if err != options.ValidateRunOptions(s) { - return err + if err := s.GenericServerRunOptions.DefaultExternalHost(); err != nil { + return fmt.Errorf("error setting the external host value: %v", err) } - genericConfig := genericapiserver.NewConfig(). // create the new config - ApplyOptions(s.GenericServerRunOptions). // apply the options selected - ApplyInsecureServingOptions(s.InsecureServing) + // validate options + if errs := s.Validate(); len(errs) != 0 { + return utilerrors.NewAggregate(errs) + } + + // create config from options + genericConfig := genericapiserver.NewConfig(). + ApplyOptions(s.GenericServerRunOptions). + ApplyInsecureServingOptions(s.InsecureServing) if _, err := genericConfig.ApplySecureServingOptions(s.SecureServing); err != nil { return fmt.Errorf("failed to configure https: %s", err) diff --git a/examples/apiserver/apiserver.go b/examples/apiserver/apiserver.go index 873a149998c..61abfca47e7 100644 --- a/examples/apiserver/apiserver.go +++ b/examples/apiserver/apiserver.go @@ -18,7 +18,6 @@ package apiserver import ( "fmt" - "net" "k8s.io/kubernetes/cmd/libs/go2idl/client-gen/test_apis/testgroup/v1" testgroupetcd "k8s.io/kubernetes/examples/apiserver/rest" @@ -28,7 +27,6 @@ import ( "k8s.io/kubernetes/pkg/genericapiserver" "k8s.io/kubernetes/pkg/genericapiserver/authorizer" genericoptions "k8s.io/kubernetes/pkg/genericapiserver/options" - genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation" "k8s.io/kubernetes/pkg/registry/generic" "k8s.io/kubernetes/pkg/runtime/schema" "k8s.io/kubernetes/pkg/storage/storagebackend" @@ -82,8 +80,15 @@ func NewServerRunOptions() *ServerRunOptions { func (serverOptions *ServerRunOptions) Run(stopCh <-chan struct{}) error { serverOptions.Etcd.StorageConfig.ServerList = []string{"http://127.0.0.1:2379"} - // TODO(sttts): unify signature of DefaultAndValidateRunOptions with the others - genericapiserver.DefaultAndValidateRunOptions(serverOptions.GenericServerRunOptions) + // set defaults + if err := serverOptions.GenericServerRunOptions.DefaultExternalHost(); err != nil { + return err + } + if err := serverOptions.SecureServing.MaybeDefaultWithSelfSignedCerts(serverOptions.GenericServerRunOptions.AdvertiseAddress.String()); err != nil { + glog.Fatalf("Error creating self-signed certificates: %v", err) + } + + // validate options if errs := serverOptions.Etcd.Validate(); len(errs) > 0 { return utilerrors.NewAggregate(errs) } @@ -93,10 +98,8 @@ func (serverOptions *ServerRunOptions) Run(stopCh <-chan struct{}) error { if errs := serverOptions.InsecureServing.Validate("insecure-port"); len(errs) > 0 { return utilerrors.NewAggregate(errs) } - if err := serverOptions.SecureServing.MaybeDefaultWithSelfSignedCerts(serverOptions.GenericServerRunOptions.AdvertiseAddress.String()); err != nil { - glog.Fatalf("Error creating self-signed certificates: %v", err) - } + // create config from options config := genericapiserver.NewConfig(). ApplyOptions(serverOptions.GenericServerRunOptions). ApplyInsecureServingOptions(serverOptions.InsecureServing) diff --git a/federation/cmd/federation-apiserver/app/options/validation.go b/federation/cmd/federation-apiserver/app/options/validation.go new file mode 100644 index 00000000000..a2044377e61 --- /dev/null +++ b/federation/cmd/federation-apiserver/app/options/validation.go @@ -0,0 +1,32 @@ +/* +Copyright 2016 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 + +func (options *ServerRunOptions) Validate() []error { + var errors []error + if errs := options.Etcd.Validate(); len(errs) > 0 { + errors = append(errors, errs...) + } + if errs := options.SecureServing.Validate(); len(errs) > 0 { + errors = append(errors, errs...) + } + if errs := options.InsecureServing.Validate("insecure-port"); len(errs) > 0 { + errors = append(errors, errs...) + } + // TODO: add more checks + return errors +} diff --git a/federation/cmd/federation-apiserver/app/server.go b/federation/cmd/federation-apiserver/app/server.go index 213a7f21dc3..955ddfff7ae 100644 --- a/federation/cmd/federation-apiserver/app/server.go +++ b/federation/cmd/federation-apiserver/app/server.go @@ -68,18 +68,23 @@ cluster's shared state through which all other components interact.`, // Run runs the specified APIServer. This should never exit. func Run(s *options.ServerRunOptions) error { - if errs := s.Etcd.Validate(); len(errs) > 0 { - utilerrors.NewAggregate(errs) - } - if err := s.GenericServerRunOptions.DefaultExternalAddress(s.SecureServing, s.InsecureServing); err != nil { + // set defaults + if err := s.GenericServerRunOptions.DefaultAdvertiseAddress(s.SecureServing, s.InsecureServing); err != nil { return err } - if err := s.SecureServing.MaybeDefaultWithSelfSignedCerts(s.GenericServerRunOptions.AdvertiseAddress.String()); err != nil { return fmt.Errorf("error creating self-signed certificates: %v", err) } + if err := s.GenericServerRunOptions.DefaultExternalHost(); err != nil { + return fmt.Errorf("error setting the external host value: %v", err) + } - genericapiserver.DefaultAndValidateRunOptions(s.GenericServerRunOptions) + // validate options + if errs := s.Validate(); len(errs) != 0 { + return utilerrors.NewAggregate(errs) + } + + // create config from options genericConfig := genericapiserver.NewConfig(). // create the new config ApplyOptions(s.GenericServerRunOptions). // apply the options selected ApplyInsecureServingOptions(s.InsecureServing) diff --git a/pkg/genericapiserver/config.go b/pkg/genericapiserver/config.go index bf8333773e6..35d0ad5e4ad 100644 --- a/pkg/genericapiserver/config.go +++ b/pkg/genericapiserver/config.go @@ -25,7 +25,6 @@ import ( "io/ioutil" "net" "net/http" - "os" goruntime "runtime" "sort" "strconv" @@ -34,13 +33,11 @@ import ( "github.com/emicklei/go-restful/swagger" "github.com/go-openapi/spec" - "github.com/golang/glog" "github.com/pborman/uuid" "gopkg.in/natefinch/lumberjack.v2" "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" - "k8s.io/kubernetes/pkg/api/v1" metav1 "k8s.io/kubernetes/pkg/apis/meta/v1" apiserverauthenticator "k8s.io/kubernetes/pkg/apiserver/authenticator" apiserverfilters "k8s.io/kubernetes/pkg/apiserver/filters" @@ -52,7 +49,6 @@ import ( authhandlers "k8s.io/kubernetes/pkg/auth/handlers" "k8s.io/kubernetes/pkg/auth/user" "k8s.io/kubernetes/pkg/client/restclient" - "k8s.io/kubernetes/pkg/cloudprovider" apiserverauthorizer "k8s.io/kubernetes/pkg/genericapiserver/authorizer" genericfilters "k8s.io/kubernetes/pkg/genericapiserver/filters" "k8s.io/kubernetes/pkg/genericapiserver/mux" @@ -629,43 +625,6 @@ func (s *GenericAPIServer) installAPI(c *Config) { s.HandlerContainer.Add(s.DynamicApisDiscovery()) } -func DefaultAndValidateRunOptions(options *options.ServerRunOptions) { - glog.Infof("Will report %v as public IP address.", options.AdvertiseAddress) - - // Set default value for ExternalAddress if not specified. - if len(options.ExternalHost) == 0 { - // TODO: extend for other providers - if options.CloudProvider == "gce" || options.CloudProvider == "aws" { - cloud, err := cloudprovider.InitCloudProvider(options.CloudProvider, options.CloudConfigFile) - if err != nil { - glog.Fatalf("Cloud provider could not be initialized: %v", err) - } - instances, supported := cloud.Instances() - if !supported { - glog.Fatalf("%q cloud provider has no instances. this shouldn't happen. exiting.", options.CloudProvider) - } - hostname, err := os.Hostname() - if err != nil { - glog.Fatalf("Failed to get hostname: %v", err) - } - nodeName, err := instances.CurrentNodeName(hostname) - if err != nil { - glog.Fatalf("Failed to get NodeName: %v", err) - } - addrs, err := instances.NodeAddresses(nodeName) - if err != nil { - glog.Warningf("Unable to obtain external host address from cloud provider: %v", err) - } else { - for _, addr := range addrs { - if addr.Type == v1.NodeExternalIP { - options.ExternalHost = addr.Address - } - } - } - } - } -} - func NewRequestInfoResolver(c *Config) *request.RequestInfoFactory { apiPrefixes := sets.NewString(strings.Trim(APIGroupPrefix, "/")) // all possible API prefixes legacyAPIPrefixes := sets.String{} // APIPrefixes that won't have groups (legacy) diff --git a/pkg/genericapiserver/options/server_run_options.go b/pkg/genericapiserver/options/server_run_options.go index 64308cc0538..65d025d0b15 100644 --- a/pkg/genericapiserver/options/server_run_options.go +++ b/pkg/genericapiserver/options/server_run_options.go @@ -19,11 +19,14 @@ package options import ( "fmt" "net" + "os" "strings" "k8s.io/kubernetes/pkg/admission" "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/apimachinery/registered" + "k8s.io/kubernetes/pkg/cloudprovider" "k8s.io/kubernetes/pkg/runtime/schema" "k8s.io/kubernetes/pkg/util/config" @@ -82,7 +85,7 @@ func NewServerRunOptions() *ServerRunOptions { } } -func (s *ServerRunOptions) DefaultExternalAddress(secure *SecureServingOptions, insecure *ServingOptions) error { +func (s *ServerRunOptions) DefaultAdvertiseAddress(secure *SecureServingOptions, insecure *ServingOptions) error { if s.AdvertiseAddress == nil || s.AdvertiseAddress.IsUnspecified() { switch { case secure != nil: @@ -106,6 +109,44 @@ func (s *ServerRunOptions) DefaultExternalAddress(secure *SecureServingOptions, return nil } +func (options *ServerRunOptions) DefaultExternalHost() error { + if len(options.ExternalHost) != 0 { + return nil + } + + // TODO: extend for other providers + if options.CloudProvider == "gce" || options.CloudProvider == "aws" { + cloud, err := cloudprovider.InitCloudProvider(options.CloudProvider, options.CloudConfigFile) + if err != nil { + return fmt.Errorf("%q cloud provider could not be initialized: %v", options.CloudProvider, err) + } + instances, supported := cloud.Instances() + if !supported { + return fmt.Errorf("%q cloud provider has no instances", options.CloudProvider) + } + hostname, err := os.Hostname() + if err != nil { + return fmt.Errorf("failed to get hostname: %v", err) + } + nodeName, err := instances.CurrentNodeName(hostname) + if err != nil { + return fmt.Errorf("failed to get NodeName from %q cloud provider: %v", options.CloudProvider, err) + } + addrs, err := instances.NodeAddresses(nodeName) + if err != nil { + return fmt.Errorf("failed to get external host address from %q cloud provider: %v", options.CloudProvider, err) + } else { + for _, addr := range addrs { + if addr.Type == v1.NodeExternalIP { + options.ExternalHost = addr.Address + } + } + } + } + + return nil +} + // StorageGroupsToEncodingVersion returns a map from group name to group version, // computed from s.StorageVersions flag. func (s *ServerRunOptions) StorageGroupsToEncodingVersion() (map[string]schema.GroupVersion, error) { diff --git a/pkg/genericapiserver/validation/BUILD b/pkg/genericapiserver/validation/BUILD deleted file mode 100644 index fd2bc5aab00..00000000000 --- a/pkg/genericapiserver/validation/BUILD +++ /dev/null @@ -1,19 +0,0 @@ -package(default_visibility = ["//visibility:public"]) - -licenses(["notice"]) - -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", -) - -go_library( - name = "go_default_library", - srcs = ["universal_validation.go"], - tags = ["automanaged"], - deps = [ - "//pkg/genericapiserver/options:go_default_library", - "//pkg/util/errors:go_default_library", - "//vendor:github.com/golang/glog", - ], -)