diff --git a/ovh/configuration.go b/ovh/configuration.go index 51713bb..c0bf795 100644 --- a/ovh/configuration.go +++ b/ovh/configuration.go @@ -4,20 +4,21 @@ import ( "fmt" "os" "os/user" - "path/filepath" "strings" "gopkg.in/ini.v1" ) -// Use variables for easier test overload -var ( - systemConfigPath = "/etc/ovh.conf" - userConfigPath = "/.ovh.conf" // prefixed with homeDir - localConfigPath = "./ovh.conf" -) +var configPaths = []string{ + // System wide configuration + "/etc/ovh.com", + // Configuration in user's home + "~/.ovh.conf", + // Configuration in local folder + "./ovh.conf", +} -// currentUserHome attempts to get current user's home directory +// currentUserHome attempts to get current user's home directory. func currentUserHome() (string, error) { usr, err := user.Current() if err != nil { @@ -31,14 +32,43 @@ func currentUserHome() (string, error) { return usr.HomeDir, nil } -// appendConfigurationFile only if it exists. We need to do this because -// ini package will fail to load configuration at all if a configuration -// file is missing. This is racy, but better than always failing. -func appendConfigurationFile(cfg *ini.File, path string) { - if file, err := os.Open(path); err == nil { - defer file.Close() - _ = cfg.Append(path) +// configPaths returns configPaths, with ~/ prefix expanded. +func expandConfigPaths() []interface{} { + paths := []interface{}{} + + // Will be initialized on first use + var home string + var homeErr error + + for _, path := range configPaths { + if strings.HasPrefix(path, "~/") { + // Find home if needed + if home == "" && homeErr == nil { + home, homeErr = currentUserHome() + } + // Ignore file in HOME if we cannot find it + if homeErr != nil { + continue + } + + path = home + path[1:] + } + + paths = append(paths, path) + } + + return paths +} + +// loadINI builds a ini.File from the configuration paths provided in configPaths. +// It's a helper for loadConfig. +func loadINI() (*ini.File, error) { + paths := expandConfigPaths() + if len(paths) == 0 { + return ini.Empty(), nil } + + return ini.LooseLoad(paths[0], paths[1:]...) } // loadConfig loads client configuration from params, environments or configuration @@ -58,13 +88,10 @@ func appendConfigurationFile(cfg *ini.File, path string) { func (c *Client) loadConfig(endpointName string) error { // Load configuration files by order of increasing priority. All configuration // files are optional. Only load file from user home if home could be resolve - cfg := ini.Empty() - appendConfigurationFile(cfg, systemConfigPath) - if home, err := currentUserHome(); err == nil { - userConfigFullPath := filepath.Join(home, userConfigPath) - appendConfigurationFile(cfg, userConfigFullPath) + cfg, err := loadINI() + if err != nil { + return fmt.Errorf("cannot load configuration: %w", err) } - appendConfigurationFile(cfg, localConfigPath) // Canonicalize configuration if endpointName == "" { diff --git a/ovh/configuration_test.go b/ovh/configuration_test.go index 3f23bdf..392d3cf 100644 --- a/ovh/configuration_test.go +++ b/ovh/configuration_test.go @@ -1,75 +1,35 @@ package ovh import ( - "io/ioutil" - "os" "testing" "github.com/maxatome/go-testdeep/td" ) -// -// Utils -// - -var home string - -func setup() { - systemConfigPath = "./ovh.unittest.global.conf" - userConfigPath = "/.ovh.unittest.user.conf" - localConfigPath = "./ovh.unittest.local.conf" - home, _ = currentUserHome() -} +const ( + systemConf = "testdata/system.ini" + userPartialConf = "testdata/userPartial.ini" + userConf = "testdata/user.ini" + localPartialConf = "testdata/localPartial.ini" + localWithURLConf = "testdata/localWithURL.ini" + doesNotExistConf = "testdata/doesNotExist.ini" + invalidINIConf = "testdata/invalid.ini" + errorConf = "testdata" +) -func teardown() { - os.Remove(systemConfigPath) - os.Remove(home + userConfigPath) - os.Remove(localConfigPath) +func setConfigPaths(t testing.TB, paths ...string) { + old := configPaths + configPaths = paths + t.Cleanup(func() { configPaths = old }) } -// -// Tests -// - func TestConfigFromFiles(t *testing.T) { - assert, require := td.AssertRequire(t) - - // Write each parameter to one different configuration file - // This is a simple way to test precedence - - // Prepare - err := ioutil.WriteFile(systemConfigPath, []byte(` -[ovh-eu] -application_key=system -application_secret=system -consumer_key=system -`), 0660) - require.CmpNoError(err) - - err = ioutil.WriteFile(home+userConfigPath, []byte(` -[ovh-eu] -application_secret=user -consumer_key=user -`), 0660) - require.CmpNoError(err) - - err = ioutil.WriteFile(localConfigPath, []byte(` -[ovh-eu] -consumer_key=local -`), 0660) - require.CmpNoError(err) - - // Clear - t.Cleanup(func() { - _ = ioutil.WriteFile(systemConfigPath, []byte(``), 0660) - _ = ioutil.WriteFile(home+userConfigPath, []byte(``), 0660) - _ = ioutil.WriteFile(localConfigPath, []byte(``), 0660) - }) + setConfigPaths(t, systemConf, userPartialConf, localPartialConf) client := Client{} - err = client.loadConfig("ovh-eu") - require.CmpNoError(err) - assert.Cmp(client, td.Struct(Client{ + err := client.loadConfig("ovh-eu") + td.Require(t).CmpNoError(err) + td.Cmp(t, client, td.Struct(Client{ AppKey: "system", AppSecret: "user", ConsumerKey: "local", @@ -77,63 +37,54 @@ consumer_key=local } func TestConfigFromOnlyOneFile(t *testing.T) { - assert, require := td.AssertRequire(t) - - // ini package has a bug causing it to ignore all subsequent configuration - // files if any could not be loaded. Make sure that workaround... works. - - // Prepare - err := os.Remove(systemConfigPath) - require.CmpNoError(err) - - err = ioutil.WriteFile(home+userConfigPath, []byte(` -[ovh-eu] -application_key=user -application_secret=user -consumer_key=user -`), 0660) - require.CmpNoError(err) - - // Clear - t.Cleanup(func() { - _ = ioutil.WriteFile(home+userConfigPath, []byte(``), 0660) - }) + setConfigPaths(t, userConf) client := Client{} - err = client.loadConfig("ovh-eu") - require.CmpNoError(err) - assert.Cmp(client, td.Struct(Client{ + err := client.loadConfig("ovh-eu") + td.Require(t).CmpNoError(err) + td.Cmp(t, client, td.Struct(Client{ AppKey: "user", AppSecret: "user", ConsumerKey: "user", })) } -func TestConfigFromEnv(t *testing.T) { - assert, require := td.AssertRequire(t) +func TestConfigFromNonExistingFile(t *testing.T) { + setConfigPaths(t, doesNotExistConf) - err := ioutil.WriteFile(systemConfigPath, []byte(` -[ovh-eu] -application_key=fail -application_secret=fail -consumer_key=fail -`), 0660) - require.CmpNoError(err) + client := Client{} + err := client.loadConfig("ovh-eu") + td.CmpString(t, err, `missing application key, please check your configuration or consult the documentation to create one`) +} - t.Cleanup(func() { - _ = ioutil.WriteFile(systemConfigPath, []byte(``), 0660) - }) +func TestConfigFromInvalidINIFile(t *testing.T) { + setConfigPaths(t, invalidINIConf) + + client := Client{} + err := client.loadConfig("ovh-eu") + td.CmpString(t, err, "cannot load configuration: unclosed section: [ovh\n") +} + +func TestConfigFromInvalidFile(t *testing.T) { + setConfigPaths(t, errorConf) + + client := Client{} + err := client.loadConfig("ovh-eu") + td.CmpString(t, err, "cannot load configuration: BOM: read testdata: is a directory") +} + +func TestConfigFromEnv(t *testing.T) { + setConfigPaths(t, userConf) t.Setenv("OVH_ENDPOINT", "ovh-eu") t.Setenv("OVH_APPLICATION_KEY", "env") t.Setenv("OVH_APPLICATION_SECRET", "env") t.Setenv("OVH_CONSUMER_KEY", "env") - // Test client := Client{} - err = client.loadConfig("") - require.CmpNoError(err) - assert.Cmp(client, td.Struct(Client{ + err := client.loadConfig("") + td.Require(t).CmpNoError(err) + td.Cmp(t, client, td.Struct(Client{ AppKey: "env", AppSecret: "env", ConsumerKey: "env", @@ -142,12 +93,12 @@ consumer_key=fail } func TestConfigFromArgs(t *testing.T) { - assert, require := td.AssertRequire(t) + setConfigPaths(t, userConf) client := Client{AppKey: "param", AppSecret: "param", ConsumerKey: "param"} err := client.loadConfig("ovh-eu") - require.CmpNoError(err) - assert.Cmp(client, td.Struct(Client{ + td.Require(t).CmpNoError(err) + td.Cmp(t, client, td.Struct(Client{ AppKey: "param", AppSecret: "param", ConsumerKey: "param", @@ -158,27 +109,11 @@ func TestConfigFromArgs(t *testing.T) { func TestEndpoint(t *testing.T) { assert, require := td.AssertRequire(t) - err := ioutil.WriteFile(systemConfigPath, []byte(` -[ovh-eu] -application_key=ovh -application_secret=ovh -consumer_key=ovh - -[https://api.example.com:4242] -application_key=example.com -application_secret=example.com -consumer_key=example.com -`), 0660) - require.CmpNoError(err) - - // Clear - t.Cleanup(func() { - _ = ioutil.WriteFile(systemConfigPath, []byte(``), 0660) - }) + setConfigPaths(t, localWithURLConf) // Test: by name client := Client{} - err = client.loadConfig("ovh-eu") + err := client.loadConfig("ovh-eu") require.CmpNoError(err) assert.Cmp(client, td.Struct(Client{ AppKey: "ovh", @@ -210,16 +145,15 @@ func TestMissingParam(t *testing.T) { td.CmpString(t, err, `missing application secret, please check your configuration or consult the documentation to create one`) } -// -// Main -// - -// TestMain changes the location of configuration files. We need -// this to avoid any interference with existing configuration -// and non-root users -func TestMain(m *testing.M) { - setup() - code := m.Run() - teardown() - os.Exit(code) +func TestConfigPaths(t *testing.T) { + home, err := currentUserHome() + td.Require(t).CmpNoError(err) + + setConfigPaths(t, "", "file", "file.ini", "dir/file.ini", "~/file.ini", "~typo.ini") + + td.Cmp(t, home, td.Not(td.HasSuffix("/"))) + td.Cmp(t, + expandConfigPaths(), + []interface{}{"", "file", "file.ini", "dir/file.ini", home + "/file.ini", "~typo.ini"}, + ) } diff --git a/ovh/testdata/invalid.ini b/ovh/testdata/invalid.ini new file mode 100644 index 0000000..8164c04 --- /dev/null +++ b/ovh/testdata/invalid.ini @@ -0,0 +1,2 @@ +[ovh +consumer_key=local diff --git a/ovh/testdata/localPartial.ini b/ovh/testdata/localPartial.ini new file mode 100644 index 0000000..3a16145 --- /dev/null +++ b/ovh/testdata/localPartial.ini @@ -0,0 +1,2 @@ +[ovh-eu] +consumer_key=local diff --git a/ovh/testdata/localWithURL.ini b/ovh/testdata/localWithURL.ini new file mode 100644 index 0000000..71fe11c --- /dev/null +++ b/ovh/testdata/localWithURL.ini @@ -0,0 +1,9 @@ +[ovh-eu] +application_key=ovh +application_secret=ovh +consumer_key=ovh + +[https://api.example.com:4242] +application_key=example.com +application_secret=example.com +consumer_key=example.com diff --git a/ovh/testdata/system.ini b/ovh/testdata/system.ini new file mode 100644 index 0000000..763f511 --- /dev/null +++ b/ovh/testdata/system.ini @@ -0,0 +1,4 @@ +[ovh-eu] +application_key=system +application_secret=system +consumer_key=system diff --git a/ovh/testdata/user.ini b/ovh/testdata/user.ini new file mode 100644 index 0000000..0b85c84 --- /dev/null +++ b/ovh/testdata/user.ini @@ -0,0 +1,4 @@ +[ovh-eu] +application_key=user +application_secret=user +consumer_key=user diff --git a/ovh/testdata/userPartial.ini b/ovh/testdata/userPartial.ini new file mode 100644 index 0000000..88bc4b4 --- /dev/null +++ b/ovh/testdata/userPartial.ini @@ -0,0 +1,3 @@ +[ovh-eu] +application_secret=user +consumer_key=user