Skip to content

Commit

Permalink
Add vlidations for configs and panic instead of returning err
Browse files Browse the repository at this point in the history
  • Loading branch information
arunvelsriram committed Aug 12, 2020
1 parent 9655dfe commit ad2344a
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 83 deletions.
12 changes: 2 additions & 10 deletions .talismanrc
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
fileignoreconfig:
- filename: sftp/ssh/key_without_passphrase.pub
checksum: 50e2a352407045a8ae28b9b53f2412d393f5697254dd1d3f26e41050f8186f23
- filename: sftp/ssh/key_with_passphrase.pub
checksum: 39a2a36d4416dfb4df9f479d106d83fa45a12026140e945f1e279be5a983e4d7
- filename: sftp/ssh/key_without_passphrase
checksum: 46831ea158a93518214983659d2d0991d9ca6da76f9e5428b749ce0fde031bde
- filename: sftp/ssh/key_with_passphrase
checksum: 6714cc1a10ad23ebc8b65290ac93e9996664ae4e1416094c57da90f32d75cf8d
- filename: sftp/docker-compose.yml
checksum: a320313a0715a649b1bf4c8db3e4cf45c2cdc9d5cbab154a4252b1d33ed8063c
- filename: pkg/config/config.go
checksum: 3d4547bb16555a9afb9277fa6d475c97f295d69b2eb9f000c9d89501f90f8c8c
9 changes: 4 additions & 5 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"fmt"
"os"
"strings"

"github.com/arunvelsriram/sftp-exporter/pkg/config"
Expand All @@ -28,7 +29,8 @@ var rootCmd = &cobra.Command{

func Execute() {
if err := rootCmd.Execute(); err != nil {
log.WithFields(log.Fields{"event": "executing command"}).Fatal(err)
fmt.Println(err)
os.Exit(1)
}
}

Expand Down Expand Up @@ -79,10 +81,7 @@ func initConfig() {

var err error
fs := afero.NewOsFs()
cfg, err = config.LoadConfig(fs)
if err != nil {
panic(err)
}
cfg = config.MustLoadConfig(fs)

log.SetFormatter(&log.TextFormatter{
FullTimestamp: true,
Expand Down
69 changes: 45 additions & 24 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Config interface {
GetSFTPPass() string
GetSFTPKey() []byte
GetSFTPKeyFile() string
GetSFTPKeyPassphrase() string
GetSFTPKeyPassphrase() []byte
}

type sftpExporterConfig struct {
Expand All @@ -40,32 +40,53 @@ type sftpExporterConfig struct {
SFTPConfig sftpConfig
}

func resolveKey(encodedKey, keyfile string, fs afero.Fs) (sftpKey []byte, err error) {
if utils.IsNotEmpty(encodedKey) && utils.IsNotEmpty(keyfile) {
return sftpKey, fmt.Errorf("only one of key or keyfile should be specified")
}
func mustResolveKey(encodedKey, keyFile string, fs afero.Fs) []byte {
mustValidateKeySource(encodedKey, keyFile)

var sftpKey []byte
var err error
if utils.IsNotEmpty(encodedKey) {
sftpKey, err = base64.StdEncoding.DecodeString(encodedKey)
if err != nil {
return sftpKey, err
}
} else if utils.IsNotEmpty(keyfile) {
sftpKey, err = afero.ReadFile(fs, keyfile)
if err != nil {
return sftpKey, err
}
utils.PanicIfErr(err)
} else if utils.IsNotEmpty(keyFile) {
sftpKey, err = afero.ReadFile(fs, keyFile)
utils.PanicIfErr(err)
}
return sftpKey
}

func mustGetString(k string) string {
v := viper.GetString(k)
if utils.IsEmpty(v) {
errMsg := fmt.Sprintf("config %s is required", k)
panic(errMsg)
}
return v
}

func mustValidateKeySource(key, keyFile string) {
if utils.IsNotEmpty(key) && utils.IsNotEmpty(keyFile) {
errMsg := fmt.Sprintf("only one of %s, %s should be provided", c.ViperKeySFTPKey, c.ViperKeySFTPKeyFile)
panic(errMsg)
}
return sftpKey, nil
}

func LoadConfig(fs afero.Fs) (Config, error) {
func mustValidateAuthTypes(pass, key, keyFile string) {
if utils.IsEmpty(pass) && utils.IsEmpty(key) && utils.IsEmpty(keyFile) {
errMsg := fmt.Sprintf("either one of %s, %s, %s is required", c.ViperKeySFTPPass, c.ViperKeySFTPKey, c.ViperKeySFTPKeyFile)
panic(errMsg)
}
}

func MustLoadConfig(fs afero.Fs) Config {
user := mustGetString(c.ViperKeySFTPUser)

pass := viper.GetString(c.ViperKeySFTPPass)
encodedKey := viper.GetString(c.ViperKeySFTPKey)
keyFile := viper.GetString(c.ViperKeySFTPKeyFile)
key, err := resolveKey(encodedKey, keyFile, fs)
if err != nil {
return nil, err
}
mustValidateAuthTypes(pass, encodedKey, keyFile)

key := mustResolveKey(encodedKey, keyFile, fs)

return sftpExporterConfig{
BindAddress: viper.GetString(c.ViperKeyBindAddress),
Expand All @@ -74,13 +95,13 @@ func LoadConfig(fs afero.Fs) (Config, error) {
SFTPConfig: sftpConfig{
Host: viper.GetString(c.ViperKeySFTPHost),
Port: viper.GetInt(c.ViperKeySFTPPort),
User: viper.GetString(c.ViperKeySFTPUser),
Pass: viper.GetString(c.ViperKeySFTPPass),
User: user,
Pass: pass,
Key: key,
KeyFile: keyFile,
KeyPassphrase: viper.GetString(c.ViperKeySFTPKeyPassphrase),
},
}, nil
}
}

func (c sftpExporterConfig) GetBindAddress() string {
Expand Down Expand Up @@ -119,6 +140,6 @@ func (c sftpExporterConfig) GetSFTPKeyFile() string {
return c.SFTPConfig.KeyFile
}

func (c sftpExporterConfig) GetSFTPKeyPassphrase() string {
return c.SFTPConfig.KeyPassphrase
func (c sftpExporterConfig) GetSFTPKeyPassphrase() []byte {
return []byte(c.SFTPConfig.KeyPassphrase)
}
92 changes: 49 additions & 43 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package config
package config_test

import (
"testing"

"github.com/arunvelsriram/sftp-exporter/pkg/config"
c "github.com/arunvelsriram/sftp-exporter/pkg/constants"
"github.com/spf13/afero"
"github.com/spf13/viper"
Expand All @@ -22,62 +23,67 @@ func (s *ConfigTestSuite) SetupTest() {
s.fs = afero.NewMemMapFs()
}

func (s *ConfigTestSuite) TestConfigLoadConfig() {
s.Run("should return config", func() {
actual, err := LoadConfig(s.fs)
func (s *ConfigTestSuite) TearDownTest() {
viper.Reset()
}

s.NoError(err)
s.Equal(sftpExporterConfig{}, actual)
})
func (s *ConfigTestSuite) TestConfigLoadConfig() {
viper.Set(c.ViperKeySFTPUser, "sftp user")
viper.Set(c.ViperKeySFTPPass, "sftp pass")
actual := config.MustLoadConfig(s.fs)

s.Run("should return err when both key and keyfile are provided", func() {
viper.Reset()
viper.Set(c.ViperKeySFTPKey, "sftp private key")
viper.Set(c.ViperKeySFTPKeyFile, "sftp private keyfile")
s.Equal("sftp user", actual.GetSFTPUser())
s.Equal("sftp pass", actual.GetSFTPPass())
}

_, err := LoadConfig(s.fs)
func (s *ConfigTestSuite) TestConfigLoadConfigPanicsIfSFTPEmptyUser() {
s.PanicsWithValue("config sftp_user is required", func() { config.MustLoadConfig(s.fs) })
}

s.EqualError(err, "only one of key or keyfile should be specified")
})
func (s *ConfigTestSuite) TestConfigLoadConfigPanicsWhenBothAllAuthTypesAreEmpty() {
viper.Set(c.ViperKeySFTPUser, "sftp user")

s.Run("should return error if key is not encoded properly", func() {
viper.Reset()
viper.Set(c.ViperKeySFTPKey, "invalid encoding")
s.PanicsWithValue("either one of sftp_pass, sftp_key, sftp_key_file is required", func() { config.MustLoadConfig(s.fs) })
}

_, err := LoadConfig(s.fs)
func (s *ConfigTestSuite) TestConfigLoadConfigPanicsWhenBothKeyAndKeyFileAreGiven() {
viper.Set(c.ViperKeySFTPUser, "sftp user")
viper.Set(c.ViperKeySFTPKey, "sftp private key")
viper.Set(c.ViperKeySFTPKeyFile, "sftp private keyfile")

s.EqualError(err, "illegal base64 data at input byte 7")
})
s.PanicsWithValue("only one of sftp_key, sftp_key_file should be provided", func() { config.MustLoadConfig(s.fs) })
}
func (s *ConfigTestSuite) TestConfigLoadConfigPanicsForInvalidKey() {
viper.Set(c.ViperKeySFTPUser, "sftp user")
viper.Set(c.ViperKeySFTPKey, "invalid encoding")

s.Run("should store decoded key for given encoded key", func() {
viper.Reset()
viper.Set(c.ViperKeySFTPKey, "YXJ1bg==")
s.PanicsWithValue("illegal base64 data at input byte 7", func() { config.MustLoadConfig(s.fs) })
}

actual, err := LoadConfig(s.fs)
func (s *ConfigTestSuite) TestConfigLoadConfigStoresDecodedKey() {
viper.Set(c.ViperKeySFTPUser, "sftp user")
viper.Set(c.ViperKeySFTPKey, "YXJ1bg==")

s.NoError(err)
s.Equal([]byte("arun"), actual.GetSFTPKey())
})
actual := config.MustLoadConfig(s.fs)

s.Run("should return error if reading keyfile fails", func() {
viper.Reset()
viper.Set(c.ViperKeySFTPKeyFile, "invalidfile")
s.Equal([]byte("arun"), actual.GetSFTPKey())
}

_, err := LoadConfig(s.fs)
func (s *ConfigTestSuite) TestConfigLoadConfigPanicsIfReadingKeyFileFails() {
viper.Set(c.ViperKeySFTPUser, "sftp user")
viper.Set(c.ViperKeySFTPKeyFile, "invalidfile")

s.EqualError(err, "open invalidfile: file does not exist")
})
s.PanicsWithValue("open invalidfile: file does not exist", func() { config.MustLoadConfig(s.fs) })
}

s.Run("should store key and keyfile for a valid keyfile", func() {
file, _ := afero.TempFile(s.fs, "", "config-test")
_, _ = file.WriteString("private key")
viper.Reset()
viper.Set(c.ViperKeySFTPKeyFile, file.Name())
func (s *ConfigTestSuite) TestConfigLoadConfigStoresKeyForValidKeyFile() {
file, _ := afero.TempFile(s.fs, "", "config-test")
_, _ = file.WriteString("private key")
viper.Set(c.ViperKeySFTPUser, "sftp user")
viper.Set(c.ViperKeySFTPKeyFile, file.Name())

actual, err := LoadConfig(s.fs)
actual := config.MustLoadConfig(s.fs)

s.NoError(err)
s.Equal(file.Name(), actual.GetSFTPKeyFile())
s.Equal([]byte("private key"), actual.GetSFTPKey())
})
s.Equal(file.Name(), actual.GetSFTPKeyFile())
s.Equal([]byte("private key"), actual.GetSFTPKey())
}
12 changes: 11 additions & 1 deletion pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@ package utils

import "strings"

func IsEmpty(s string) bool {
return len(strings.TrimSpace(s)) == 0
}

func IsNotEmpty(s string) bool {
return strings.TrimSpace(s) != ""
return !IsEmpty(s)
}

func PanicIfErr(err error) {
if err != nil {
panic(err.Error())
}
}
72 changes: 72 additions & 0 deletions pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package utils_test

import (
"fmt"
"github.com/arunvelsriram/sftp-exporter/pkg/utils"
"github.com/stretchr/testify/assert"
"testing"
)

func TestIsEmpty(t *testing.T) {
tests := []struct {
name string
in string
expected bool
}{
{
name: "empty string",
in: "",
expected: true,
},
{
name: "non-empty string",
in: "some value",
expected: false,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := utils.IsEmpty(test.in)

assert.Equal(t, test.expected, actual)
})
}
}

func TestIsNotEmpty(t *testing.T) {
tests := []struct {
name string
in string
expected bool
}{
{
name: "empty string",
in: "",
expected: false,
},
{
name: "non-empty string",
in: "some value",
expected: true,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := utils.IsNotEmpty(test.in)

assert.Equal(t, test.expected, actual)
})
}
}

func TestPanicIfErrShouldPanicForErr(t *testing.T) {
err := fmt.Errorf("some error")

assert.PanicsWithValue(t, "some error", func() { utils.PanicIfErr(err) })
}

func TestPanicIfErrShouldNotPanicWhenErrIsNil(t *testing.T) {
assert.NotPanics(t, func() { utils.PanicIfErr(nil) })
}

0 comments on commit ad2344a

Please sign in to comment.