diff --git a/alert.go b/alert.go index b7d979c..2a6c7c8 100644 --- a/alert.go +++ b/alert.go @@ -37,13 +37,32 @@ type AlertNotice struct { LastCheckOutput string } -// IsValid returns a boolean indicating if the Alert has been correctly -// configured -func (alert Alert) IsValid() bool { - hasAtLeastOneCommand := alert.Command != nil || alert.ShellCommand != "" - hasAtMostOneCommand := alert.Command == nil || alert.ShellCommand == "" +// Validate checks that the Alert is properly configured and returns errors if not +func (alert Alert) Validate() error { + hasCommand := len(alert.Command) > 0 + hasShellCommand := alert.ShellCommand != "" - return hasAtLeastOneCommand && hasAtMostOneCommand + var err error + + hasAtLeastOneCommand := hasCommand || hasShellCommand + if !hasAtLeastOneCommand { + err = errors.Join(err, fmt.Errorf( + "%w: alert %s has no command or shell_command configured", + ErrInvalidAlert, + alert.Name, + )) + } + + hasAtMostOneCommand := !(hasCommand && hasShellCommand) + if !hasAtMostOneCommand { + err = errors.Join(err, fmt.Errorf( + "%w: alert %s has both command and shell_command configured", + ErrInvalidAlert, + alert.Name, + )) + } + + return err } // BuildTemplates compiles command templates for the Alert diff --git a/alert_test.go b/alert_test.go index 9e7e4a0..71896b3 100644 --- a/alert_test.go +++ b/alert_test.go @@ -1,20 +1,24 @@ package main_test import ( + "errors" "testing" m "git.iamthefij.com/iamthefij/minitor-go" ) -func TestAlertIsValid(t *testing.T) { +func TestAlertValidate(t *testing.T) { + t.Parallel() + cases := []struct { alert m.Alert - expected bool + expected error name string }{ - {m.Alert{Command: []string{"echo", "test"}}, true, "Command only"}, - {m.Alert{ShellCommand: "echo test"}, true, "CommandShell only"}, - {m.Alert{}, false, "No commands"}, + {m.Alert{Command: []string{"echo", "test"}}, nil, "Command only"}, + {m.Alert{ShellCommand: "echo test"}, nil, "CommandShell only"}, + {m.Alert{Command: []string{"echo", "test"}, ShellCommand: "echo test"}, m.ErrInvalidAlert, "Both commands"}, + {m.Alert{}, m.ErrInvalidAlert, "No commands"}, } for _, c := range cases { @@ -23,8 +27,11 @@ func TestAlertIsValid(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() - actual := c.alert.IsValid() - if actual != c.expected { + actual := c.alert.Validate() + hasErr := (actual != nil) + expectErr := (c.expected != nil) + + if hasErr != expectErr || !errors.Is(actual, c.expected) { t.Errorf("expected=%t actual=%t", c.expected, actual) } }) diff --git a/config.go b/config.go index 0b6b22b..610ff50 100644 --- a/config.go +++ b/config.go @@ -73,9 +73,7 @@ func (config Config) IsValid() error { } for _, alert := range config.Alerts { - if !alert.IsValid() { - err = errors.Join(err, fmt.Errorf("%w: %s", ErrInvalidAlert, alert.Name)) - } + err = errors.Join(err, alert.Validate()) } // Validate monitors @@ -84,9 +82,7 @@ func (config Config) IsValid() error { } for _, monitor := range config.Monitors { - if !monitor.IsValid() { - err = errors.Join(err, fmt.Errorf("%w: %s", ErrInvalidMonitor, monitor.Name)) - } + err = errors.Join(err, monitor.Validate()) // Check that all Monitor alerts actually exist for _, isUp := range []bool{true, false} { diff --git a/monitor.go b/monitor.go index fef7472..4f72143 100644 --- a/monitor.go +++ b/monitor.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "math" "os/exec" @@ -65,21 +66,51 @@ func (monitor *Monitor) Init(defaultAlertAfter int, defaultAlertEvery *int, defa return nil } -// IsValid returns a boolean indicating if the Monitor has been correctly configured -func (monitor Monitor) IsValid() bool { - // TODO: Refactor and return an error containing more information on what was invalid +// Validate checks that the Monitor is properly configured and returns errors if not +func (monitor Monitor) Validate() error { hasCommand := len(monitor.Command) > 0 hasShellCommand := monitor.ShellCommand != "" hasValidAlertAfter := monitor.AlertAfter > 0 hasAlertDown := len(monitor.AlertDown) > 0 - hasAtLeastOneCommand := hasCommand || hasShellCommand - hasAtMostOneCommand := !(hasCommand && hasShellCommand) + var err error - return hasAtLeastOneCommand && - hasAtMostOneCommand && - hasValidAlertAfter && - hasAlertDown + hasAtLeastOneCommand := hasCommand || hasShellCommand + if !hasAtLeastOneCommand { + err = errors.Join(err, fmt.Errorf( + "%w: monitor %s has no command or shell_command configured", + ErrInvalidMonitor, + monitor.Name, + )) + } + + hasAtMostOneCommand := !(hasCommand && hasShellCommand) + if !hasAtMostOneCommand { + err = errors.Join(err, fmt.Errorf( + "%w: monitor %s has both command and shell_command configured", + ErrInvalidMonitor, + monitor.Name, + )) + } + + if !hasValidAlertAfter { + err = errors.Join(err, fmt.Errorf( + "%w: monitor %s has invalid alert_after value %d. Must be greater than 0", + ErrInvalidMonitor, + monitor.Name, + monitor.AlertAfter, + )) + } + + if !hasAlertDown { + err = errors.Join(err, fmt.Errorf( + "%w: monitor %s has no alert_down configured. Configure one here or add a default_alert_down", + ErrInvalidMonitor, + monitor.Name, + )) + } + + return err } func (monitor Monitor) LastOutput() string { diff --git a/monitor_test.go b/monitor_test.go index ec5e43e..0150170 100644 --- a/monitor_test.go +++ b/monitor_test.go @@ -1,6 +1,7 @@ package main_test import ( + "errors" "reflect" "testing" "time" @@ -8,18 +9,19 @@ import ( m "git.iamthefij.com/iamthefij/minitor-go" ) -// TestMonitorIsValid tests the Monitor.IsValid() -func TestMonitorIsValid(t *testing.T) { +func TestMonitorValidate(t *testing.T) { + t.Parallel() + cases := []struct { monitor m.Monitor - expected bool + expected error name string }{ - {m.Monitor{AlertAfter: 1, Command: []string{"echo", "test"}, AlertDown: []string{"log"}}, true, "Command only"}, - {m.Monitor{AlertAfter: 1, ShellCommand: "echo test", AlertDown: []string{"log"}}, true, "CommandShell only"}, - {m.Monitor{AlertAfter: 1, Command: []string{"echo", "test"}}, false, "No AlertDown"}, - {m.Monitor{AlertAfter: 1, AlertDown: []string{"log"}}, false, "No commands"}, - {m.Monitor{AlertAfter: -1, Command: []string{"echo", "test"}, AlertDown: []string{"log"}}, false, "Invalid alert threshold, -1"}, + {m.Monitor{AlertAfter: 1, Command: []string{"echo", "test"}, AlertDown: []string{"log"}}, nil, "Command only"}, + {m.Monitor{AlertAfter: 1, ShellCommand: "echo test", AlertDown: []string{"log"}}, nil, "CommandShell only"}, + {m.Monitor{AlertAfter: 1, Command: []string{"echo", "test"}}, m.ErrInvalidMonitor, "No AlertDown"}, + {m.Monitor{AlertAfter: 1, AlertDown: []string{"log"}}, m.ErrInvalidMonitor, "No commands"}, + {m.Monitor{AlertAfter: -1, Command: []string{"echo", "test"}, AlertDown: []string{"log"}}, m.ErrInvalidMonitor, "Invalid alert threshold, -1"}, } for _, c := range cases { @@ -28,8 +30,11 @@ func TestMonitorIsValid(t *testing.T) { t.Run(c.name, func(t *testing.T) { t.Parallel() - actual := c.monitor.IsValid() - if actual != c.expected { + actual := c.monitor.Validate() + hasErr := (actual != nil) + expectErr := (c.expected != nil) + + if hasErr != expectErr || !errors.Is(actual, c.expected) { t.Errorf("IsValid(%v), expected=%t actual=%t", c.name, c.expected, actual) } })