Skip to content

Commit

Permalink
Merge pull request #45 from justenwalker/missing-fields
Browse files Browse the repository at this point in the history
Better error messages for missing config fields
  • Loading branch information
tgross committed Dec 14, 2015
2 parents f9a77ed + 61e9096 commit 7a94e50
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 42 deletions.
44 changes: 39 additions & 5 deletions src/containerbuddy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,6 @@ func loadConfig() (*Config, error) {

var configFlag string
var versionFlag bool
var discovery DiscoveryService
discoveryCount := 0
flag.StringVar(&configFlag, "config", "", "JSON config or file:// path to JSON config file.")
flag.BoolVar(&versionFlag, "version", false, "Show version identifier and quit.")
flag.Parse()
Expand All @@ -155,7 +153,12 @@ func loadConfig() (*Config, error) {
if err != nil {
return nil, err
}
return initializeConfig(config)
}

func initializeConfig(config *Config) (*Config, error) {
var discovery DiscoveryService
discoveryCount := 0
onStartCmd, err := parseCommandArgs(config.OnStart)
if err != nil {
return nil, fmt.Errorf("Could not parse `onStart`: %s", err)
Expand Down Expand Up @@ -195,23 +198,52 @@ func loadConfig() (*Config, error) {
}

for _, backend := range config.Backends {
if backend.Name == "" {
return nil, fmt.Errorf("backend must have a `name`")
}
cmd, err := parseCommandArgs(backend.OnChangeExec)
if err != nil {
return nil, fmt.Errorf("Could not parse `onChange` in backend %s: %s",
backend.Name, err)
}
if cmd == nil {
return nil, fmt.Errorf("`onChange` is required in backend %s",
backend.Name)
}
if backend.Poll < 1 {
return nil, fmt.Errorf("`poll` must be > 0 in backend %s",
backend.Name)
}
backend.onChangeCmd = cmd
backend.discoveryService = discovery
}

hostname, _ := os.Hostname()
for _, service := range config.Services {
if service.Name == "" {
return nil, fmt.Errorf("service must have a `name`")
}
service.Id = fmt.Sprintf("%s-%s", service.Name, hostname)
service.discoveryService = discovery
if service.Poll < 1 {
return nil, fmt.Errorf("`poll` must be > 0 in service %s",
service.Name)
}
if service.TTL < 1 {
return nil, fmt.Errorf("`ttl` must be > 0 in service %s",
service.Name)
}
if service.Port < 1 {
return nil, fmt.Errorf("`port` must be > 0 in service %s",
service.Name)
}

if cmd, err := parseCommandArgs(service.HealthCheckExec); err != nil {
return nil, fmt.Errorf("Could not parse `health` in service %s: %s",
service.Name, err)
} else if cmd == nil {
return nil, fmt.Errorf("`health` is required in service %s",
service.Name)
} else {
service.healthCheckCmd = cmd
}
Expand Down Expand Up @@ -245,14 +277,16 @@ func parseConfig(configFlag string) (*Config, error) {
} else {
data = []byte(configFlag)
}
return unmarshalConfig(data)
}

func unmarshalConfig(data []byte) (*Config, error) {
config := &Config{}
if err := json.Unmarshal(data, &config); err != nil {
return nil, errors.New(fmt.Sprintf(
return nil, fmt.Errorf(
"Could not parse configuration: %s",
err))
err)
}

return config, nil
}

Expand Down
139 changes: 102 additions & 37 deletions src/containerbuddy/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,49 +6,51 @@ import (
"os"
"os/exec"
"reflect"
"strings"
"testing"
)

func TestValidConfigParse(t *testing.T) {
defer argTestCleanup(argTestSetup())
var testJson = `{
"consul": "consul:8500",
"onStart": "/bin/to/onStart.sh arg1 arg2",
"preStop": ["/bin/to/preStop.sh","arg1","arg2"],
"postStop": ["/bin/to/postStop.sh"],
"services": [
{
"name": "serviceA",
"port": 8080,
"interfaces": "eth0",
"health": "/bin/to/healthcheck/for/service/A.sh",
"poll": 30,
"ttl": 19
},
{
"name": "serviceB",
"port": 5000,
"interfaces": ["ethwe","eth0"],
"health": "/bin/to/healthcheck/for/service/B.sh",
"poll": 30,
"ttl": 103
}
],
"backends": [
{
"name": "upstreamA",
"poll": 11,
"onChange": "/bin/to/onChangeEvent/for/upstream/A.sh"
},
{
"name": "upstreamB",
"poll": 79,
"onChange": "/bin/to/onChangeEvent/for/upstream/B.sh"
}
]
var testJson = `{
"consul": "consul:8500",
"onStart": "/bin/to/onStart.sh arg1 arg2",
"preStop": ["/bin/to/preStop.sh","arg1","arg2"],
"postStop": ["/bin/to/postStop.sh"],
"services": [
{
"name": "serviceA",
"port": 8080,
"interfaces": "eth0",
"health": "/bin/to/healthcheck/for/service/A.sh",
"poll": 30,
"ttl": 19
},
{
"name": "serviceB",
"port": 5000,
"interfaces": ["ethwe","eth0"],
"health": "/bin/to/healthcheck/for/service/B.sh",
"poll": 30,
"ttl": 103
}
],
"backends": [
{
"name": "upstreamA",
"poll": 11,
"onChange": "/bin/to/onChangeEvent/for/upstream/A.sh"
},
{
"name": "upstreamB",
"poll": 79,
"onChange": "/bin/to/onChangeEvent/for/upstream/B.sh"
}
]
}
`

func TestValidConfigParse(t *testing.T) {
defer argTestCleanup(argTestSetup())

os.Args = []string{"this", "-config", testJson, "/test.sh", "valid1", "--debug"}
config, _ := loadConfig()

Expand Down Expand Up @@ -219,6 +221,64 @@ func TestHealthCheck(t *testing.T) {
}
}

func TestConfigRequiredFields(t *testing.T) {
var testConfig *Config

// --------------
// Service Tests
// --------------

// Missing `name`
testConfig = unmarshalTestJson()
testConfig.Services[0].Name = ""
validateParseError(t, []string{"`name`"}, testConfig)
// Missing `poll`
testConfig = unmarshalTestJson()
testConfig.Services[0].Poll = 0
validateParseError(t, []string{"`poll`", testConfig.Services[0].Name}, testConfig)
// Missing `ttl`
testConfig = unmarshalTestJson()
testConfig.Services[0].TTL = 0
validateParseError(t, []string{"`ttl`", testConfig.Services[0].Name}, testConfig)
// Missing `health`
testConfig = unmarshalTestJson()
testConfig.Services[0].HealthCheckExec = nil
validateParseError(t, []string{"`health`", testConfig.Services[0].Name}, testConfig)
// Missing `port`
testConfig = unmarshalTestJson()
testConfig.Services[0].Port = 0
validateParseError(t, []string{"`port`", testConfig.Services[0].Name}, testConfig)

// --------------
// Backend Tests
// --------------

// Missing `name`
testConfig = unmarshalTestJson()
testConfig.Backends[0].Name = ""
validateParseError(t, []string{"`name`"}, testConfig)
// Missing `poll`
testConfig = unmarshalTestJson()
testConfig.Backends[0].Poll = 0
validateParseError(t, []string{"`poll`", testConfig.Backends[0].Name}, testConfig)
// Missing `onChange`
testConfig = unmarshalTestJson()
testConfig.Backends[0].OnChangeExec = nil
validateParseError(t, []string{"`onChange`", testConfig.Backends[0].Name}, testConfig)
}

func validateParseError(t *testing.T, matchStrings []string, config *Config) {
if _, err := initializeConfig(config); err == nil {
t.Errorf("Expected error parsing config")
} else {
for _, match := range matchStrings {
if !strings.Contains(err.Error(), match) {
t.Errorf("Expected message does not contain %s: %s", match, err)
}
}
}
}

// ----------------------------------------------------
// test helpers

Expand All @@ -238,3 +298,8 @@ func testParseExpectError(t *testing.T, testJson string, expected string) {
t.Errorf("Expected %s but got %s", expected, err)
}
}

func unmarshalTestJson() *Config {
config, _ := unmarshalConfig([]byte(testJson))
return config
}

0 comments on commit 7a94e50

Please sign in to comment.