diff --git a/pkg/client/clientcmd/loader.go b/pkg/client/clientcmd/loader.go index 9f0f04f457..6d1c9d93ba 100644 --- a/pkg/client/clientcmd/loader.go +++ b/pkg/client/clientcmd/loader.go @@ -17,6 +17,7 @@ limitations under the License. package clientcmd import ( + "fmt" "io/ioutil" "os" "path/filepath" @@ -26,6 +27,7 @@ import ( clientcmdapi "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api" clientcmdlatest "github.com/GoogleCloudPlatform/kubernetes/pkg/client/clientcmd/api/latest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/util/errors" ) const ( @@ -55,7 +57,8 @@ func NewClientConfigLoadingRules() *ClientConfigLoadingRules { // 2. EnvVarPath // 3. CurrentDirectoryPath // 4. HomeDirectoryPath -// Empty filenames are ignored. Files with non-deserializable content produced errors. +// A missing CommandLinePath file produces an error. Empty filenames or other missing files are ignored. +// Read errors or files with non-deserializable content produce errors. // The first file to set a particular map key wins and map key's value is never changed. // BUT, if you set a struct value that is NOT contained inside of map, the value WILL be changed. // This results in some odd looking logic to merge in one direction, merge in the other, and then merge the two. @@ -65,16 +68,30 @@ func NewClientConfigLoadingRules() *ClientConfigLoadingRules { // and only absolute file paths are returned. func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { + errlist := []error{} + + // Make sure a file we were explicitly told to use exists + if len(rules.CommandLinePath) > 0 { + if _, err := os.Stat(rules.CommandLinePath); os.IsNotExist(err) { + errlist = append(errlist, fmt.Errorf("The config file %v does not exist", rules.CommandLinePath)) + } + } + kubeConfigFiles := []string{rules.CommandLinePath, rules.EnvVarPath, rules.CurrentDirectoryPath, rules.HomeDirectoryPath} // first merge all of our maps mapConfig := clientcmdapi.NewConfig() for _, file := range kubeConfigFiles { - mergeConfigWithFile(mapConfig, file) - resolveLocalPaths(file, mapConfig) + if err := mergeConfigWithFile(mapConfig, file); err != nil { + errlist = append(errlist, err) + } + if err := resolveLocalPaths(file, mapConfig); err != nil { + errlist = append(errlist, err) + } } // merge all of the struct values in the reverse order so that priority is given correctly + // errors are not added to the list the second time nonMapConfig := clientcmdapi.NewConfig() for i := len(kubeConfigFiles) - 1; i >= 0; i-- { file := kubeConfigFiles[i] @@ -88,7 +105,7 @@ func (rules *ClientConfigLoadingRules) Load() (*clientcmdapi.Config, error) { mergo.Merge(config, mapConfig) mergo.Merge(config, nonMapConfig) - return config, nil + return config, errors.NewAggregate(errlist) } func mergeConfigWithFile(startingConfig *clientcmdapi.Config, filename string) error { @@ -98,8 +115,11 @@ func mergeConfigWithFile(startingConfig *clientcmdapi.Config, filename string) e } config, err := LoadFromFile(filename) + if os.IsNotExist(err) { + return nil + } if err != nil { - return err + return fmt.Errorf("Error loading config file \"%s\": %v", filename, err) } mergo.Merge(startingConfig, config) @@ -117,7 +137,7 @@ func resolveLocalPaths(filename string, config *clientcmdapi.Config) error { configDir, err := filepath.Abs(filepath.Dir(filename)) if err != nil { - return err + return fmt.Errorf("Could not determine the absolute path of config file %s: %v", filename, err) } resolvedClusters := make(map[string]clientcmdapi.Cluster) diff --git a/pkg/client/clientcmd/loader_test.go b/pkg/client/clientcmd/loader_test.go index c0169dcc64..17e91c75b5 100644 --- a/pkg/client/clientcmd/loader_test.go +++ b/pkg/client/clientcmd/loader_test.go @@ -22,6 +22,7 @@ import ( "os" "path" "path/filepath" + "strings" "testing" "github.com/ghodss/yaml" @@ -75,6 +76,74 @@ var ( } ) +func TestNonExistentCommandLineFile(t *testing.T) { + loadingRules := ClientConfigLoadingRules{ + CommandLinePath: "bogus_file", + } + + _, err := loadingRules.Load() + if err == nil { + t.Fatalf("Expected error for missing command-line file, got none") + } + if !strings.Contains(err.Error(), "bogus_file") { + t.Fatalf("Expected error about 'bogus_file', got %s", err.Error()) + } +} + +func TestToleratingMissingFiles(t *testing.T) { + loadingRules := ClientConfigLoadingRules{ + EnvVarPath: "bogus1", + CurrentDirectoryPath: "bogus2", + HomeDirectoryPath: "bogus3", + } + + _, err := loadingRules.Load() + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } +} + +func TestErrorReadingFile(t *testing.T) { + commandLineFile, _ := ioutil.TempFile("", "") + defer os.Remove(commandLineFile.Name()) + + if err := ioutil.WriteFile(commandLineFile.Name(), []byte("bogus value"), 0644); err != nil { + t.Fatalf("Error creating tempfile: %v", err) + } + + loadingRules := ClientConfigLoadingRules{ + CommandLinePath: commandLineFile.Name(), + } + + _, err := loadingRules.Load() + if err == nil { + t.Fatalf("Expected error for unloadable file, got none") + } + if !strings.Contains(err.Error(), commandLineFile.Name()) { + t.Fatalf("Expected error about '%s', got %s", commandLineFile.Name(), err.Error()) + } +} + +func TestErrorReadingNonFile(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Couldn't create tmpdir") + } + defer os.Remove(tmpdir) + + loadingRules := ClientConfigLoadingRules{ + CommandLinePath: tmpdir, + } + + _, err = loadingRules.Load() + if err == nil { + t.Fatalf("Expected error for non-file, got none") + } + if !strings.Contains(err.Error(), tmpdir) { + t.Fatalf("Expected error about '%s', got %s", tmpdir, err.Error()) + } +} + func TestConflictingCurrentContext(t *testing.T) { commandLineFile, _ := ioutil.TempFile("", "") defer os.Remove(commandLineFile.Name())