Merge pull request #2983 from fabriziopandini/fix-clusterctl-read-config

🐛Fix clusterctl error when reading default config file

Author: k8s-ci-robot

Diff for: cmd/clusterctl/client/config/reader_viper.go

@@ -64,39 +64,41 @@ func newViperReader(opts ...viperReaderOption) Reader {
 func (v *viperReader) Init(path string) error {
 	log := logf.Log
 
+	// Configure viper for reading environment variables as well, and more specifically:
+	// AutomaticEnv force viper to check for an environment variable any time a viper.Get request is made.
+	// It will check for a environment variable with a name matching the key uppercased; in case name use the - delimiter,
+	// the SetEnvKeyReplacer forces matching to name use the _ delimiter instead (- is not allowed in linux env variable names).
+	replacer := strings.NewReplacer("-", "_")
+	viper.SetEnvKeyReplacer(replacer)
+	viper.AllowEmptyEnv(true)
+	viper.AutomaticEnv()
+
+	// Reads the clusterctl config file
 	if path != "" {
 		if _, err := os.Stat(path); err != nil {
 			return err
 		}
 		// Use path file from the flag.
 		viper.SetConfigFile(path)
 	} else {
-		// Configure for searching .cluster-api/clusterctl{.extension} in home directory
-		viper.SetConfigName(ConfigName)
-		for _, p := range v.configPaths {
-			viper.AddConfigPath(p)
-		}
+		// Checks if there is a default .cluster-api/clusterctl{.extension} file in home directory
 		if !v.checkDefaultConfig() {
 			// since there is no default config to read from, just skip
 			// reading in config
 			log.V(5).Info("No default config file available")
 			return nil
 		}
+		// Configure viper for reading .cluster-api/clusterctl{.extension} in home directory
+		viper.SetConfigName(ConfigName)
+		for _, p := range v.configPaths {
+			viper.AddConfigPath(p)
+		}
 	}
 
-	// Configure for reading environment variables as well, and more specifically:
-	// AutomaticEnv force viper to check for an environment variable any time a viper.Get request is made.
-	// It will check for a environment variable with a name matching the key uppercased; in case name use the - delimiter,
-	// the SetEnvKeyReplacer forces matching to name use the _ delimiter instead (- is not allowed in linux env variable names).
-	replacer := strings.NewReplacer("-", "_")
-	viper.SetEnvKeyReplacer(replacer)
-	viper.AllowEmptyEnv(true)
-	viper.AutomaticEnv()
-
 	if err := viper.ReadInConfig(); err != nil {
 		return err
 	}
-	log.V(5).Info("Reading configuration", "File", viper.ConfigFileUsed())
+	log.V(5).Info("Using configuration", "File", viper.ConfigFileUsed())
 	return nil
 }
 
@@ -121,7 +123,7 @@ func (v *viperReader) UnmarshalKey(key string, rawval interface{}) error {
 func (v *viperReader) checkDefaultConfig() bool {
 	for _, path := range v.configPaths {
 		for _, ext := range viper.SupportedExts {
-			f := fmt.Sprintf("%s%s.%s", path, ConfigName, ext)
+			f := filepath.Join(path, fmt.Sprintf("%s.%s", ConfigName, ext))
 			_, err := os.Stat(f)
 			if err == nil {
 				return true

Diff for: cmd/clusterctl/client/config/reader_viper_test.go

@@ -17,9 +17,11 @@ limitations under the License.
 package config
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -38,10 +40,10 @@ func Test_viperReader_Init(t *testing.T) {
 	g.Expect(err).NotTo(HaveOccurred())
 	defer os.RemoveAll(dir)
 
-	configFile := filepath.Join(dir, ".clusterctl.yaml")
+	configFile := filepath.Join(dir, "clusterctl.yaml")
 	g.Expect(ioutil.WriteFile(configFile, []byte("bar: bar"), 0640)).To(Succeed())
 
-	configFileBadContents := filepath.Join(dir, ".clusterctl-bad.yaml")
+	configFileBadContents := filepath.Join(dir, "clusterctl-bad.yaml")
 	g.Expect(ioutil.WriteFile(configFileBadContents, []byte("bad-contents"), 0640)).To(Succeed())
 
 	tests := []struct {
@@ -99,7 +101,7 @@ func Test_viperReader_Get(t *testing.T) {
 
 	os.Setenv("FOO", "foo")
 
-	configFile := filepath.Join(dir, ".clusterctl.yaml")
+	configFile := filepath.Join(dir, "clusterctl.yaml")
 	g.Expect(ioutil.WriteFile(configFile, []byte("bar: bar"), 0640)).To(Succeed())
 
 	type args struct {
@@ -140,7 +142,7 @@ func Test_viperReader_Get(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			gs := NewWithT(t)
 
-			v := &viperReader{}
+			v := newViperReader(InjectConfigPaths([]string{dir}))
 
 			gs.Expect(v.Init(configFile)).To(Succeed())
 
@@ -156,6 +158,22 @@ func Test_viperReader_Get(t *testing.T) {
 	}
 }
 
+func Test_viperReader_GetWithoutDefaultConfig(t *testing.T) {
+	g := NewWithT(t)
+	dir, err := ioutil.TempDir("", "clusterctl")
+	g.Expect(err).NotTo(HaveOccurred())
+	defer os.RemoveAll(dir)
+
+	os.Setenv("FOO_FOO", "bar")
+
+	v := newViperReader(InjectConfigPaths([]string{dir}))
+	g.Expect(v.Init("")).To(Succeed())
+
+	got, err := v.Get("FOO_FOO")
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(got).To(Equal("bar"))
+}
+
 func Test_viperReader_Set(t *testing.T) {
 	g := NewWithT(t)
 
@@ -165,7 +183,7 @@ func Test_viperReader_Set(t *testing.T) {
 
 	os.Setenv("FOO", "foo")
 
-	configFile := filepath.Join(dir, ".clusterctl.yaml")
+	configFile := filepath.Join(dir, "clusterctl.yaml")
 
 	g.Expect(ioutil.WriteFile(configFile, []byte("bar: bar"), 0640)).To(Succeed())
 
@@ -203,3 +221,48 @@ func Test_viperReader_Set(t *testing.T) {
 		})
 	}
 }
+
+func Test_viperReader_checkDefaultConfig(t *testing.T) {
+	g := NewWithT(t)
+	dir, err := ioutil.TempDir("", "clusterctl")
+	g.Expect(err).NotTo(HaveOccurred())
+	defer os.RemoveAll(dir)
+	dir = strings.TrimSuffix(dir, "/")
+
+	configFile := filepath.Join(dir, "clusterctl.yaml")
+	g.Expect(ioutil.WriteFile(configFile, []byte("bar: bar"), 0640)).To(Succeed())
+
+	type fields struct {
+		configPaths []string
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		want   bool
+	}{
+		{
+			name: "tmp path without final /",
+			fields: fields{
+				configPaths: []string{dir},
+			},
+			want: true,
+		},
+		{
+			name: "tmp path with final /",
+			fields: fields{
+				configPaths: []string{fmt.Sprintf("%s/", dir)},
+			},
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gs := NewWithT(t)
+
+			v := &viperReader{
+				configPaths: tt.fields.configPaths,
+			}
+			gs.Expect(v.checkDefaultConfig()).To(Equal(tt.want))
+		})
+	}
+}