Config consumers should be able to recognize an empty config

For UX, it would be better if we presented an error for validation that
is "this config is empty" rather than the inaccurate "no server name" or
"no context name" errors. Returns a typed error.
pull/6/head
Clayton Coleman 2015-11-05 14:59:22 -05:00
parent 2041297ade
commit 2dc2db5d02
6 changed files with 67 additions and 7 deletions

View File

@ -31,6 +31,14 @@ func init() {
redactedBytes = []byte(string(sDec)) redactedBytes = []byte(string(sDec))
} }
// IsConfigEmpty returns true if the config is empty.
func IsConfigEmpty(config *Config) bool {
return len(config.AuthInfos) == 0 && len(config.Clusters) == 0 && len(config.Contexts) == 0 &&
len(config.CurrentContext) == 0 &&
len(config.Preferences.Extensions) == 0 && !config.Preferences.Colors &&
len(config.Extensions) == 0
}
// MinifyConfig read the current context and uses that to keep only the relevant pieces of config // MinifyConfig read the current context and uses that to keep only the relevant pieces of config
// This is useful for making secrets based on kubeconfig files // This is useful for making secrets based on kubeconfig files
func MinifyConfig(config *Config) error { func MinifyConfig(config *Config) error {

View File

@ -24,6 +24,7 @@ import (
// Top level config objects and all values required for proper functioning are not "omitempty". Any truly optional piece of config is allowed to be omitted. // Top level config objects and all values required for proper functioning are not "omitempty". Any truly optional piece of config is allowed to be omitted.
// Config holds the information needed to build connect to remote kubernetes clusters as a given user // Config holds the information needed to build connect to remote kubernetes clusters as a given user
// IMPORTANT if you add fields to this struct, please update IsConfigEmpty()
type Config struct { type Config struct {
// Legacy field from pkg/api/types.go TypeMeta. // Legacy field from pkg/api/types.go TypeMeta.
// TODO(jlowdermilk): remove this after eliminating downstream dependencies. // TODO(jlowdermilk): remove this after eliminating downstream dependencies.
@ -44,6 +45,7 @@ type Config struct {
Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"` Extensions map[string]*runtime.EmbeddedObject `json:"extensions,omitempty"`
} }
// IMPORTANT if you add fields to this struct, please update IsConfigEmpty()
type Preferences struct { type Preferences struct {
Colors bool `json:"colors,omitempty"` Colors bool `json:"colors,omitempty"`
// Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields // Extensions holds additional information. This is useful for extenders so that reads and writes don't clobber unknown fields

View File

@ -233,7 +233,11 @@ func (config DirectClientConfig) ConfirmUsable() error {
validationErrors := make([]error, 0) validationErrors := make([]error, 0)
validationErrors = append(validationErrors, validateAuthInfo(config.getAuthInfoName(), config.getAuthInfo())...) validationErrors = append(validationErrors, validateAuthInfo(config.getAuthInfoName(), config.getAuthInfo())...)
validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...) validationErrors = append(validationErrors, validateClusterInfo(config.getClusterName(), config.getCluster())...)
// when direct client config is specified, and our only error is that no server is defined, we should
// return a standard "no config" error
if len(validationErrors) == 1 && validationErrors[0] == ErrEmptyCluster {
return newErrConfigurationInvalid([]error{ErrEmptyConfig})
}
return newErrConfigurationInvalid(validationErrors) return newErrConfigurationInvalid(validationErrors)
} }

View File

@ -20,6 +20,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"os" "os"
"reflect"
"strings" "strings"
clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api" clientcmdapi "k8s.io/kubernetes/pkg/client/unversioned/clientcmd/api"
@ -27,7 +28,12 @@ import (
"k8s.io/kubernetes/pkg/util/validation" "k8s.io/kubernetes/pkg/util/validation"
) )
var ErrNoContext = errors.New("no context chosen") var (
ErrNoContext = errors.New("no context chosen")
ErrEmptyConfig = errors.New("no configuration has been provided")
// message is for consistency with old behavior
ErrEmptyCluster = errors.New("cluster has no server defined")
)
type errContextNotFound struct { type errContextNotFound struct {
ContextName string ContextName string
@ -49,6 +55,16 @@ func IsContextNotFound(err error) bool {
return strings.Contains(err.Error(), "context was not found for specified context") return strings.Contains(err.Error(), "context was not found for specified context")
} }
// IsEmptyConfig returns true if the provided error indicates the provided configuration
// is empty.
func IsEmptyConfig(err error) bool {
switch t := err.(type) {
case errConfigurationInvalid:
return len(t) == 1 && t[0] == ErrEmptyConfig
}
return err == ErrEmptyConfig
}
// errConfigurationInvalid is a set of errors indicating the configuration is invalid. // errConfigurationInvalid is a set of errors indicating the configuration is invalid.
type errConfigurationInvalid []error type errConfigurationInvalid []error
@ -88,6 +104,10 @@ func IsConfigurationInvalid(err error) bool {
func Validate(config clientcmdapi.Config) error { func Validate(config clientcmdapi.Config) error {
validationErrors := make([]error, 0) validationErrors := make([]error, 0)
if clientcmdapi.IsConfigEmpty(&config) {
return newErrConfigurationInvalid([]error{ErrEmptyConfig})
}
if len(config.CurrentContext) != 0 { if len(config.CurrentContext) != 0 {
if _, exists := config.Contexts[config.CurrentContext]; !exists { if _, exists := config.Contexts[config.CurrentContext]; !exists {
validationErrors = append(validationErrors, &errContextNotFound{config.CurrentContext}) validationErrors = append(validationErrors, &errContextNotFound{config.CurrentContext})
@ -114,6 +134,10 @@ func Validate(config clientcmdapi.Config) error {
func ConfirmUsable(config clientcmdapi.Config, passedContextName string) error { func ConfirmUsable(config clientcmdapi.Config, passedContextName string) error {
validationErrors := make([]error, 0) validationErrors := make([]error, 0)
if clientcmdapi.IsConfigEmpty(&config) {
return newErrConfigurationInvalid([]error{ErrEmptyConfig})
}
var contextName string var contextName string
if len(passedContextName) != 0 { if len(passedContextName) != 0 {
contextName = passedContextName contextName = passedContextName
@ -143,6 +167,10 @@ func ConfirmUsable(config clientcmdapi.Config, passedContextName string) error {
func validateClusterInfo(clusterName string, clusterInfo clientcmdapi.Cluster) []error { func validateClusterInfo(clusterName string, clusterInfo clientcmdapi.Cluster) []error {
validationErrors := make([]error, 0) validationErrors := make([]error, 0)
if reflect.DeepEqual(clientcmdapi.Cluster{}, clusterInfo) {
return []error{ErrEmptyCluster}
}
if len(clusterInfo.Server) == 0 { if len(clusterInfo.Server) == 0 {
if len(clusterName) == 0 { if len(clusterName) == 0 {
validationErrors = append(validationErrors, fmt.Errorf("default cluster has no server defined")) validationErrors = append(validationErrors, fmt.Errorf("default cluster has no server defined"))

View File

@ -87,7 +87,7 @@ func TestConfirmUsableEmptyConfig(t *testing.T) {
config := clientcmdapi.NewConfig() config := clientcmdapi.NewConfig()
test := configValidationTest{ test := configValidationTest{
config: config, config: config,
expectedErrorSubstring: []string{"no context chosen"}, expectedErrorSubstring: []string{"invalid configuration: no configuration has been provided"},
} }
test.testConfirmUsable("", t) test.testConfirmUsable("", t)
@ -96,7 +96,7 @@ func TestConfirmUsableMissingConfig(t *testing.T) {
config := clientcmdapi.NewConfig() config := clientcmdapi.NewConfig()
test := configValidationTest{ test := configValidationTest{
config: config, config: config,
expectedErrorSubstring: []string{"context was not found for"}, expectedErrorSubstring: []string{"invalid configuration: no configuration has been provided"},
} }
test.testConfirmUsable("not-here", t) test.testConfirmUsable("not-here", t)
@ -105,6 +105,7 @@ func TestValidateEmptyConfig(t *testing.T) {
config := clientcmdapi.NewConfig() config := clientcmdapi.NewConfig()
test := configValidationTest{ test := configValidationTest{
config: config, config: config,
expectedErrorSubstring: []string{"invalid configuration: no configuration has been provided"},
} }
test.testConfig(t) test.testConfig(t)
@ -132,6 +133,18 @@ func TestIsContextNotFound(t *testing.T) {
} }
} }
func TestIsEmptyConfig(t *testing.T) {
config := clientcmdapi.NewConfig()
err := Validate(*config)
if !IsEmptyConfig(err) {
t.Errorf("Expected context not found, but got %v", err)
}
if !IsConfigurationInvalid(err) {
t.Errorf("Expected configuration invalid, but got %v", err)
}
}
func TestIsConfigurationInvalid(t *testing.T) { func TestIsConfigurationInvalid(t *testing.T) {
if newErrConfigurationInvalid([]error{}) != nil { if newErrConfigurationInvalid([]error{}) != nil {
t.Errorf("unexpected error") t.Errorf("unexpected error")
@ -177,7 +190,7 @@ func TestValidateEmptyClusterInfo(t *testing.T) {
config.Clusters["empty"] = &clientcmdapi.Cluster{} config.Clusters["empty"] = &clientcmdapi.Cluster{}
test := configValidationTest{ test := configValidationTest{
config: config, config: config,
expectedErrorSubstring: []string{"no server found for"}, expectedErrorSubstring: []string{"cluster has no server defined"},
} }
test.testCluster("empty", t) test.testCluster("empty", t)

View File

@ -327,6 +327,11 @@ func NewOrDie(c *Config) *Client {
// running inside a pod running on kuberenetes. It will return an error if // running inside a pod running on kuberenetes. It will return an error if
// called from a process not running in a kubernetes environment. // called from a process not running in a kubernetes environment.
func InClusterConfig() (*Config, error) { func InClusterConfig() (*Config, error) {
host, port := os.Getenv("KUBERNETES_SERVICE_HOST"), os.Getenv("KUBERNETES_SERVICE_PORT")
if len(host) == 0 || len(port) == 0 {
return nil, fmt.Errorf("unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined")
}
token, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/" + api.ServiceAccountTokenKey) token, err := ioutil.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/" + api.ServiceAccountTokenKey)
if err != nil { if err != nil {
return nil, err return nil, err
@ -341,7 +346,7 @@ func InClusterConfig() (*Config, error) {
return &Config{ return &Config{
// TODO: switch to using cluster DNS. // TODO: switch to using cluster DNS.
Host: "https://" + net.JoinHostPort(os.Getenv("KUBERNETES_SERVICE_HOST"), os.Getenv("KUBERNETES_SERVICE_PORT")), Host: "https://" + net.JoinHostPort(host, port),
BearerToken: string(token), BearerToken: string(token),
TLSClientConfig: tlsClientConfig, TLSClientConfig: tlsClientConfig,
}, nil }, nil