Repository: bpicode/fritzctl
Pull request #123 (2018-01-14 14:34:42)

Issue #110: support for configuration with .yml files
This is "opt-in right now", the app will look for configuration in the
following places:
  - ~/.fritzctl/config.yml
  - ~/.fritzctl/config.json
  - ~/.fritzctl/fritzctl.yml
  - ~/.fritzctl/fritzctl.json
  - ~/.fritzctl.yml
  - ~/.fritzctl.json
  - $PWD/fritzctl.yml
  - $PWD/fritzctl.json
  - $PWD/.fritzctl.yml
  - $PWD/.fritzctl.json
  - /etc/fritzctl/config.yml
  - /etc/fritzctl/config.json
  - /etc/fritzctl/fritzctl.yml
  - /etc/fritzctl/fritzctl.json

I.e. the old default location will still work.
The recommended location is ~/.fritzctl/config.yml
An example of new format is:
```yml
---
net:
  protocol: "https"
  host: "fritz.box"
  port: 443
login:
  url: "/login_sid.lua"
  username:
  password: "xxxxx"
pki:
  skip_tls_verify: true
  certificate_file:
```
API note:
The functions config.FindConfigFile, config DefaultConfigFileAbsolute are now deprecated.


Patch:
diff --git a/cmd/certificate_export.go b/cmd/certificate_export.go
index 43be1bc..5024b85 100644
--- a/cmd/certificate_export.go
+++ b/cmd/certificate_export.go
@@ -41,11 +41,9 @@ func certExport(_ *cobra.Command, _ []string) error {
 }
 
 func mustReadConfig() *config.Config {
-	path, err := config.FindConfigFile()
-	assertNoErr(err, "cannot find configuration file")
-	cfg, err := config.New(path)
-	assertNoErr(err, "cannot parse configuration file")
-	return cfg
+	c, err := cfg(defaultConfigPlaces...)
+	assertNoErr(err, "cannot parse configuration")
+	return c
 }
 
 func mustConnect(cfg *config.Config) *tls.Conn {
diff --git a/cmd/client.go b/cmd/client.go
index bf966f8..8a00a58 100644
--- a/cmd/client.go
+++ b/cmd/client.go
@@ -3,24 +3,41 @@ package cmd
 import (
 	"io/ioutil"
 	"net/url"
+	"os/user"
 
 	"github.com/bpicode/fritzctl/config"
 	"github.com/bpicode/fritzctl/fritz"
 	"github.com/bpicode/fritzctl/logger"
 )
 
+var defaultConfigPlaces = []config.Place{
+	config.InHomeDir(user.Current, ".fritzctl/config.yml", config.YAML()),
+	config.InHomeDir(user.Current, ".fritzctl/config.json", config.JSON()),
+	config.InHomeDir(user.Current, ".fritzctl/fritzctl.yml", config.YAML()),
+	config.InHomeDir(user.Current, ".fritzctl/fritzctl.json", config.JSON()),
+	config.InHomeDir(user.Current, ".fritzctl.yml", config.YAML()),
+	config.InHomeDir(user.Current, ".fritzctl.json", config.JSON()),
+	config.InDir("", "fritzctl.yml", config.YAML()),
+	config.InDir("", "fritzctl.json", config.JSON()),
+	config.InDir("", ".fritzctl.yml", config.YAML()),
+	config.InDir("", ".fritzctl.json", config.JSON()),
+	config.InDir("/etc/fritzctl", "config.yml", config.YAML()),
+	config.InDir("/etc/fritzctl", "config.json", config.JSON()),
+	config.InDir("/etc/fritzctl", "fritzctl.yml", config.YAML()),
+	config.InDir("/etc/fritzctl", "fritzctl.json", config.JSON()),
+}
+
 func clientLogin() *fritz.Client {
-	configFile, err := config.FindConfigFile()
-	assertNoErr(err, "cannot find configuration file")
-	client, err := fritz.NewClient(configFile)
-	assertNoErr(err, "failed to create FRITZ!Box client")
+	conf, err := cfg(defaultConfigPlaces...)
+	assertNoErr(err, "cannot parse configuration")
+	client := fritz.NewClientFromConfig(conf)
 	err = client.Login()
 	assertNoErr(err, "login failed")
 	return client
 }
 
 func homeAutoClient(overrides ...fritz.Option) fritz.HomeAuto {
-	opts := findOptions(config.FindConfigFile)
+	opts := optsFromPlaces(defaultConfigPlaces...)
 	opts = append(opts, overrides...)
 	h := fritz.NewHomeAuto(opts...)
 	err := h.Login()
@@ -28,20 +45,13 @@ func homeAutoClient(overrides ...fritz.Option) fritz.HomeAuto {
 	return h
 }
 
-type cfgFileFinder func() (string, error)
-
-func findOptions(finder cfgFileFinder) []fritz.Option {
-	path, err := finder()
+func optsFromPlaces(places ...config.Place) []fritz.Option {
+	opts := make([]fritz.Option, 0)
+	cfg, err := cfg(places...)
 	if err != nil {
 		logger.Warn("Using default configuration because no config file could be inferred:", err)
 		return make([]fritz.Option, 0)
 	}
-	return fromFile(path)
-}
-
-func fromFile(path string) []fritz.Option {
-	opts := make([]fritz.Option, 0)
-	cfg, err := config.New(path)
 	assertNoErr(err, "cannot apply configuration")
 	opts = networkOptions(opts, cfg.Net)
 	opts = certificateOptions(opts, cfg.Pki)
@@ -72,3 +82,8 @@ func loginOptions(opts []fritz.Option, login *config.Login) []fritz.Option {
 	opts = append(opts, fritz.AuthEndpoint(login.LoginURL))
 	return opts
 }
+
+func cfg(places ...config.Place) (*config.Config, error) {
+	p := config.NewParser(places...)
+	return p.Parse()
+}
diff --git a/config/configfiles.go b/config/configfiles.go
index f21b090..63d1134 100644
--- a/config/configfiles.go
+++ b/config/configfiles.go
@@ -15,16 +15,20 @@ var (
 	// Revision is the hash in VCS (git commit).
 	Revision = "unknown"
 	// Filename defines the filename of the configuration file.
+	// Deprecated: use Place API.
 	Filename = "fritzctl.json"
 	// filenameHidden defines the filename of the configuration file (hidden).
 	filenameHidden = "." + Filename
 	// Dir defines the directory of the configuration file.
+	// Deprecated: use Place API.
 	Dir = "."
 	// DefaultDir is the default directory where the config file resides.
+	// Deprecated: will not be the default in future versions.
 	DefaultDir = "/etc/fritzctl"
 )
 
 // FindConfigFile returns the path to the config file.
+// Deprecated: use Parser.
 func FindConfigFile() (string, error) {
 	return firstWithoutError(
 		curry(fmt.Sprintf("%s/%s", Dir, Filename), accessible),
@@ -34,6 +38,7 @@ func FindConfigFile() (string, error) {
 }
 
 // DefaultConfigFileAbsolute returns the absolute path of the default configuration file.
+// Deprecated: use Place in combination with Parser.
 func DefaultConfigFileAbsolute() string {
 	return fmt.Sprintf("%s/%s", DefaultDir, Filename)
 }
diff --git a/config/fritzclientconfig.go b/config/fritzclientconfig.go
index 72cac83..5973f9d 100644
--- a/config/fritzclientconfig.go
+++ b/config/fritzclientconfig.go
@@ -18,22 +18,22 @@ type Config struct {
 
 // Net wraps the protocol://host:port data to contact the FRITZ!Box.
 type Net struct {
-	Protocol string `json:"protocol"` // The protocol to use when communicating with the FRITZ!Box. "http" or "https".
-	Host     string `json:"host"`     // Host name or ip address of the FRITZ!Box. In most home setups "fritz.box" can be used. Other possible formats: "192.168.2.200".
-	Port     string `json:"port"`     // Port to use for the HTTP interface. Leave empty for default values.
+	Protocol string `json:"protocol" yaml:"protocol"` // The protocol to use when communicating with the FRITZ!Box. "http" or "https".
+	Host     string `json:"host"  yaml:"host"`        // Host name or ip address of the FRITZ!Box. In most home setups "fritz.box" can be used. Other possible formats: "192.168.2.200".
+	Port     string `json:"port" yaml:"port"`         // Port to use for the HTTP interface. Leave empty for default values.
 }
 
 // Login wraps the login data to be used by the client.
 type Login struct {
-	LoginURL string `json:"loginURL"` // The URL for the login negotiation.
-	Username string `json:"username"` // Username to log in. In user-agnostic setups this can be left empty.
-	Password string `json:"password"` // The password corresponding to the Username.
+	LoginURL string `json:"loginURL" yaml:"url"`      // The URL for the login negotiation.
+	Username string `json:"username" yaml:"username"` // Username to log in. In user-agnostic setups this can be left empty.
+	Password string `json:"password" yaml:"password"` // The password corresponding to the Username.
 }
 
 // Pki wraps the client-side certificate handling.
 type Pki struct {
-	SkipTLSVerify   bool   `json:"skipTlsVerify"`   // Skip TLS verification when using https.
-	CertificateFile string `json:"certificateFile"` // Points to a certificate file (in PEM format) to verify the integrity of the FRITZ!Box.
+	SkipTLSVerify   bool   `json:"skipTlsVerify" yaml:"skip_tls_verify"`     // Skip TLS verification when using https.
+	CertificateFile string `json:"certificateFile"  yaml:"certificate_file"` // Points to a certificate file (in PEM format) to verify the integrity of the FRITZ!Box.
 }
 
 // New creates a new Config by reading from a file given by the path.
diff --git a/config/parser.go b/config/parser.go
new file mode 100644
index 0000000..7c271b6
--- /dev/null
+++ b/config/parser.go
@@ -0,0 +1,148 @@
+package config
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"os/user"
+
+	errors2 "github.com/pkg/errors"
+	"gopkg.in/yaml.v2"
+)
+
+// Parser defines one method, Parse, which reads from 3rd party source(s).
+type Parser interface {
+	Parse() (*Config, error)
+}
+
+type parser struct {
+	sources []source
+}
+
+type source struct {
+	openCloser
+	Decode
+}
+
+type openCloser interface {
+	Open() (io.Reader, error)
+	io.Closer
+}
+
+// Decode uses an io.Reader, whose read bytes shall be marshaled into the passed interface. An error is returned if the
+// operation did not work.
+type Decode func(io.Reader, interface{}) error
+
+// Place is a configuration option for a parser.
+type Place func(*parser)
+
+// NewParser constructs a Parser that looks for configuration in the passed Places. If multiple Places would spawn
+// correct configurations, the first-without-error is used.
+func NewParser(places ...Place) Parser {
+	s := &parser{}
+	for _, p := range places {
+		p(s)
+	}
+	return s
+}
+
+type homeDirOpenCloser struct {
+	path string
+	io.Closer
+	user func() (*user.User, error)
+}
+
+func (h *homeDirOpenCloser) Open() (io.Reader, error) {
+	hd := homeDirOf(h.user)
+	path, err := hd(h.path)
+	if err != nil {
+		return nil, errors2.Wrapf(err, "cannot open '%s' in current user's home directory", h.path)
+	}
+	f, err := os.Open(path)
+	h.Closer = f
+	return f, err
+}
+
+// InHomeDir looks for a file inside a user's home directory.
+func InHomeDir(u func() (*user.User, error), path string, decoder Decode) Place {
+	return func(p *parser) {
+		s := source{}
+		s.Decode = decoder
+		s.openCloser = &homeDirOpenCloser{path: path, user: u}
+		p.sources = append(p.sources, s)
+	}
+}
+
+// InDir looks for a file inside a given directory.
+func InDir(dir string, file string, decoder Decode) Place {
+	return func(p *parser) {
+		s := source{}
+		s.Decode = decoder
+		s.openCloser = &homeDirOpenCloser{
+			path: file,
+			user: func() (*user.User, error) {
+				return &user.User{HomeDir: dir}, nil
+			},
+		}
+		p.sources = append(p.sources, s)
+	}
+}
+
+// JSON returns a Decode function that uses JSON format.
+func JSON() Decode {
+	return func(r io.Reader, v interface{}) error {
+		return json.NewDecoder(r).Decode(v)
+	}
+}
+
+// YAML returns a Decode function that uses YML format.
+func YAML() Decode {
+	return func(r io.Reader, v interface{}) error {
+		bytes, err := ioutil.ReadAll(r)
+		if err != nil {
+			return err
+		}
+		return yaml.Unmarshal(bytes, v)
+	}
+}
+
+// Parse runs though all places and tries to parse the configuration. If none of the places yields a successful config,
+// an error is returned.
+func (p *parser) Parse() (*Config, error) {
+	var errs []error
+	for _, s := range p.sources {
+		r, err := s.Open()
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		c, err := p.decode(s, r)
+		s.Close()
+		if err != nil {
+			errs = append(errs, err)
+			continue
+		}
+		return c, nil
+	}
+	err := errors.New(fmt.Sprint(errs))
+	return nil, errors2.Wrapf(err, "unable to find a usable config source")
+}
+
+func (p *parser) decode(s source, r io.Reader) (*Config, error) {
+	cfg := Config{}
+	net := Net{}
+	pki := Pki{}
+	login := Login{}
+	err := s.Decode(r, &struct {
+		*Net
+		*Login
+		*Pki
+	}{&net, &login, &pki})
+	cfg.Pki = &pki
+	cfg.Login = &login
+	cfg.Net = &net
+	return &cfg, err
+}
diff --git a/fritz/fritzclient.go b/fritz/fritzclient.go
index 76d25fb..8f08204 100644
--- a/fritz/fritzclient.go
+++ b/fritz/fritzclient.go
@@ -37,15 +37,21 @@ type Rights struct {
 }
 
 // NewClient creates a new Client with values read from a config file, given by the parameter configfile.
+// Deprecated: use NewClientFromConfig.
 func NewClient(configfile string) (*Client, error) {
-	configPtr, err := config.New(configfile)
+	cfg, err := config.New(configfile)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to read configuration")
 	}
-	tlsConfig := tlsConfigFrom(configPtr)
+	return NewClientFromConfig(cfg), nil
+}
+
+// NewClientFromConfig creates a new Client with the passed configuration.
+func NewClientFromConfig(cfg *config.Config) *Client {
+	tlsConfig := tlsConfigFrom(cfg)
 	transport := &http.Transport{TLSClientConfig: tlsConfig}
 	httpClient := &http.Client{Transport: transport}
-	return &Client{Config: configPtr, HTTPClient: httpClient}, nil
+	return &Client{Config: cfg, HTTPClient: httpClient}
 }
 
 // Login tries to login into the box and obtain the session id.


Test patch:
diff --git a/cmd/certificate_export_test.go b/cmd/certificate_export_test.go
index 0c24eea..cdbc8ce 100644
--- a/cmd/certificate_export_test.go
+++ b/cmd/certificate_export_test.go
@@ -11,8 +11,9 @@ import (
 
 // Test_certExport test the certificate export.
 func Test_certExport(t *testing.T) {
-	config.Dir = "../testdata/config"
-	config.Filename = "config_localhost_https_test.json"
+	oldPlaces := defaultConfigPlaces
+	defer func() { defaultConfigPlaces = oldPlaces }()
+	defaultConfigPlaces = append([]config.Place{config.InDir("../testdata/config", "config_localhost_https_test.json", config.JSON())}, defaultConfigPlaces...)
 	assertions := assert.New(t)
 	server := httptest.NewUnstartedServer(nil)
 	var err error
diff --git a/cmd/client_test.go b/cmd/client_test.go
index ada23b3..cf4a201 100644
--- a/cmd/client_test.go
+++ b/cmd/client_test.go
@@ -1,19 +1,17 @@
 package cmd
 
 import (
-	"errors"
 	"fmt"
 	"testing"
 
+	"github.com/bpicode/fritzctl/config"
 	"github.com/stretchr/testify/assert"
 )
 
 // TestConfigFileCannotBeDetermined asserts that default options are used if no config file can be found.
 func TestConfigFileCannotBeDetermined(t *testing.T) {
 	assertions := assert.New(t)
-	opts := findOptions(func() (string, error) {
-		return "", errors.New("no config file location could be determined")
-	})
+	opts := optsFromPlaces(config.InDir("", "asjnfasjfbq3.yml", config.YAML()))
 	assertions.Empty(opts)
 }
 
@@ -26,9 +24,7 @@ func TestConfigFiles(t *testing.T) {
 	} {
 		t.Run(fmt.Sprintf("config file %d %s", i, path), func(t *testing.T) {
 			assertions := assert.New(t)
-			opts := findOptions(func() (string, error) {
-				return path, nil
-			})
+			opts := optsFromPlaces(config.InDir("", path, config.JSON()))
 			assertions.NotEmpty(opts)
 		})
 	}
diff --git a/cmd/commands_test.go b/cmd/commands_test.go
index 501cceb..93f386c 100644
--- a/cmd/commands_test.go
+++ b/cmd/commands_test.go
@@ -14,8 +14,9 @@ import (
 
 // TestCommands is a unit test that runs most commands.
 func TestCommands(t *testing.T) {
-	config.Dir = "../testdata/config"
-	config.Filename = "config_localhost_http_test.json"
+	oldPlaces := defaultConfigPlaces
+	defer func() { defaultConfigPlaces = oldPlaces }()
+	defaultConfigPlaces = append([]config.Place{config.InDir("../testdata/config", "config_localhost_http_test.json", config.JSON())}, defaultConfigPlaces...)
 
 	testCases := []struct {
 		cmd  *cobra.Command
diff --git a/cmd/list_inetstats_test.go b/cmd/list_inetstats_test.go
index bb6efd6..b98d57e 100644
--- a/cmd/list_inetstats_test.go
+++ b/cmd/list_inetstats_test.go
@@ -11,8 +11,9 @@ import (
 
 // TestListInetStats  tests the command.
 func TestListInetStats(t *testing.T) {
-	config.Dir = "../testdata/config"
-	config.Filename = "config_localhost_http_test.json"
+	oldPlaces := defaultConfigPlaces
+	defer func() { defaultConfigPlaces = oldPlaces }()
+	defaultConfigPlaces = append([]config.Place{config.InDir("../testdata/config", "config_localhost_http_test.json", config.JSON())}, defaultConfigPlaces...)
 	srv := mock.New().UnstartedServer()
 	l, err := net.Listen("tcp", ":61666")
 	assert.NoError(t, err)
diff --git a/config/configfiles_test.go b/config/configfiles_test.go
index 10ad087..0b9ca6d 100644
--- a/config/configfiles_test.go
+++ b/config/configfiles_test.go
@@ -22,8 +22,8 @@ func TestConfigfilename(t *testing.T) {
 	assert.NotContains(t, Filename, " ")
 }
 
-// TestConfigfile asserts that FindConfigFile does not panic.
-func TestConfigfile(t *testing.T) {
+// TestConfigFile asserts that FindConfigFile does not panic.
+func TestConfigFile(t *testing.T) {
 	assert.NotPanics(t, func() {
 		FindConfigFile()
 	})
diff --git a/config/parser_test.go b/config/parser_test.go
new file mode 100644
index 0000000..cb50114
--- /dev/null
+++ b/config/parser_test.go
@@ -0,0 +1,98 @@
+package config
+
+import (
+	"errors"
+	"io/ioutil"
+	"os"
+	"os/user"
+	"path"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestNewParserJSON tests the parser using JSON backend.
+func TestNewParserJSON(t *testing.T) {
+	p := NewParser(
+		InDir("../testdata/config/", "config_test.json", JSON()),
+	)
+	c, err := p.Parse()
+	assert.NoError(t, err)
+	assert.NotNil(t, c)
+}
+
+// TestNewParserPathShifting tests the parser when the filename suggests a directory substructure.
+func TestNewParserPathShifting(t *testing.T) {
+	p := NewParser(
+		InDir("../testdata", "config/config_test.json", JSON()),
+	)
+	c, err := p.Parse()
+	assert.NoError(t, err)
+	assert.NotNil(t, c)
+	assert.Equal(t, "xxxxx", c.Password)
+}
+
+// TestNewParserYAML tests the parser using YAML backend.
+func TestNewParserYAML(t *testing.T) {
+	p := NewParser(
+		InDir("../testdata/config/", "config_test.yml", YAML()),
+	)
+	c, err := p.Parse()
+	assert.NoError(t, err)
+	assert.NotNil(t, c)
+	assert.Equal(t, "fritz.box", c.Host)
+}
+
+// TestNewParserFileNotFound tests the parser when a file does not exist.
+func TestNewParserFileNotFound(t *testing.T) {
+	p := NewParser(
+		InHomeDir(user.Current, "kjewhgjgsjdbgbjnjjub.json", JSON()),
+	)
+	_, err := p.Parse()
+	assert.Error(t, err)
+}
+
+// TestNewParserHomeDirError tests the parser a user's $HOME cannot be determined.
+func TestNewParserHomeDirError(t *testing.T) {
+	p := NewParser(
+		InHomeDir(func() (*user.User, error) {
+			return nil, errors.New("cannot determine current user")
+		}, "config.json", JSON()),
+	)
+	_, err := p.Parse()
+	assert.Error(t, err)
+}
+
+// TestNewParserFileEmpty tests the parser when a config file is empty.
+func TestNewParserFileEmpty(t *testing.T) {
+	f, err := ioutil.TempFile("", "TestNewParserFileEmpty_config.json")
+	assert.NoError(t, err)
+	defer f.Close()
+	defer os.Remove(f.Name())
+
+	tmpDir, fName := path.Split(f.Name())
+	p := NewParser(
+		InHomeDir(func() (*user.User, error) {
+			return &user.User{
+				HomeDir: tmpDir,
+			}, nil
+		}, fName, JSON()),
+	)
+	_, err = p.Parse()
+	assert.Error(t, err)
+}
+
+type errReader struct {
+}
+
+// Read always fails.
+func (e *errReader) Read(p []byte) (n int, err error) {
+	return 0, errors.New("I always fail")
+}
+
+// TestYAMLWithError test the YAML Decode when there is an IO error.
+func TestYAMLWithError(t *testing.T) {
+	y := YAML()
+	err := y(&errReader{}, &Config{})
+	assert.Error(t, err)
+}
diff --git a/testdata/config/config_test.yml b/testdata/config/config_test.yml
new file mode 100644
index 0000000..0ab61b5
--- /dev/null
+++ b/testdata/config/config_test.yml
@@ -0,0 +1,12 @@
+---
+net:
+  protocol: "https"
+  host: "fritz.box"
+  port: 443
+login:
+  url: "/login_sid.lua"
+  username:
+  password: "xxxxx"
+pki:
+  skip_tls_verify: true
+  certificate_file: