From 0e529ee8ad78b44a9a0a80378b7481e92583e60c Mon Sep 17 00:00:00 2001 From: Thibault Le Ouay Ducasse Date: Mon, 23 Mar 2026 10:43:52 +0100 Subject: [PATCH] fix: resolve 15 critical and high severity bugs across CLI Critical fixes: - Fix region mapping collision causing silent data loss on round-trip (Koyeb/Railway regions now use disambiguated codes like koyeb_fra, railway_us-west2) - Fix global koanf instance causing state pollution between ReadConfig/ReadOpenStatus calls - Fix partial failure in ApplyChanges leaving lock map in inconsistent state - Fix spinner leak when statusToSDK fails in status-report list/add-update - Fix TCP monitor creation accepting zero/invalid port values High severity fixes: - Fix strconv.Atoi error silently ignored in httpMonitorToLocal/tcpMonitorToLocal - Fix DNS monitor info returning opaque "unknown monitor type" error - Fix ResolveToken silently discarding file read errors (e.g. permission denied) - Fix confirmation prompt blocking in non-interactive environments (added TTY check) - Fix delete commands showing confirmation with empty ID before validation - Fix --component-ids "" silently clearing components on status-report update - Fix monitor trigger printing plain text in --json mode - Fix http.DefaultClient used without timeout (added api.DefaultHTTPClient with 30s) - Fix unknown jobType aborting entire result set in run command - Fix monitor import truncating output file before API call succeeds --- internal/api/client.go | 5 ++ internal/auth/auth.go | 2 + internal/cli/confirmation.go | 11 ++- internal/cli/confirmation_test.go | 3 + internal/cli/export_test.go | 7 ++ internal/config/config.go | 16 +--- internal/config/config_test.go | 37 +++++++- internal/config/monitor.go | 12 +++ internal/config/openstatus.go | 13 ++- internal/config/openstatus_test.go | 74 ++++++++++++++-- internal/login/login.go | 4 +- internal/monitors/export_test.go | 5 ++ internal/monitors/monitor_apply.go | 40 +++++---- internal/monitors/monitor_create.go | 33 ++++--- internal/monitors/monitor_delete.go | 4 + internal/monitors/monitor_import.go | 62 ++++++------- internal/monitors/monitor_info.go | 18 +++- internal/monitors/monitor_trigger.go | 3 + internal/monitors/monitor_update.go | 9 +- internal/monitors/monitors.go | 59 ++++++++++--- internal/monitors/monitors_region_test.go | 76 ++++++++++++++++ internal/monitors/monitors_tcp_test.go | 86 +++++++++++++++++++ internal/run/run.go | 3 +- internal/statusreport/statusreport.go | 2 +- .../statusreport/statusreport_add_update.go | 1 + internal/statusreport/statusreport_delete.go | 4 + internal/statusreport/statusreport_list.go | 5 +- internal/statusreport/statusreport_update.go | 2 +- internal/whoami/whoami.go | 2 +- 29 files changed, 473 insertions(+), 125 deletions(-) create mode 100644 internal/cli/export_test.go create mode 100644 internal/monitors/export_test.go create mode 100644 internal/monitors/monitors_region_test.go create mode 100644 internal/monitors/monitors_tcp_test.go diff --git a/internal/api/client.go b/internal/api/client.go index f07e4bd..5a276a4 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -3,6 +3,7 @@ package api import ( "context" "fmt" + "net/http" "os" "time" @@ -14,6 +15,10 @@ const APIBaseURL = "https://api.openstatus.dev/v1" const ConnectBaseURL = "https://api.openstatus.dev/rpc" +var DefaultHTTPClient = &http.Client{ + Timeout: 30 * time.Second, +} + func NewAuthInterceptor(apiKey string) connect.UnaryInterceptorFunc { return func(next connect.UnaryFunc) connect.UnaryFunc { return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { diff --git a/internal/auth/auth.go b/internal/auth/auth.go index ab89677..cc27009 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -27,6 +27,8 @@ func ResolveToken(flagValue string) (string, error) { if token != "" { return token, nil } + } else if !os.IsNotExist(readErr) { + return "", fmt.Errorf("failed to read token file %s: %w", tokenPath, readErr) } } diff --git a/internal/cli/confirmation.go b/internal/cli/confirmation.go index 3be45d2..0cc90c3 100644 --- a/internal/cli/confirmation.go +++ b/internal/cli/confirmation.go @@ -5,11 +5,20 @@ import ( "fmt" "os" "strings" + + "github.com/mattn/go-isatty" ) +var isInteractiveStdin = func() bool { + return isatty.IsTerminal(os.Stdin.Fd()) || isatty.IsCygwinTerminal(os.Stdin.Fd()) +} + func AskForConfirmation(s string) (bool, error) { + if !isInteractiveStdin() { + return false, fmt.Errorf("confirmation required but stdin is not a terminal (use --auto-accept / -y to skip)") + } reader := bufio.NewReader(os.Stdin) - fmt.Printf("%s [y/N]: ", s) + fmt.Fprintf(os.Stderr, "%s [y/N]: ", s) response, err := reader.ReadString('\n') if err != nil { return false, fmt.Errorf("failed to read user input: %w", err) diff --git a/internal/cli/confirmation_test.go b/internal/cli/confirmation_test.go index ae4acd5..8e1f38b 100644 --- a/internal/cli/confirmation_test.go +++ b/internal/cli/confirmation_test.go @@ -8,6 +8,9 @@ import ( ) func Test_AskForConfirmation(t *testing.T) { + restore := cli.SetInteractiveStdin(func() bool { return true }) + t.Cleanup(restore) + t.Run("Returns true for 'y' input", func(t *testing.T) { // Create a pipe to simulate stdin r, w, err := os.Pipe() diff --git a/internal/cli/export_test.go b/internal/cli/export_test.go new file mode 100644 index 0000000..3cc2f33 --- /dev/null +++ b/internal/cli/export_test.go @@ -0,0 +1,7 @@ +package cli + +func SetInteractiveStdin(f func() bool) func() { + old := isInteractiveStdin + isInteractiveStdin = f + return func() { isInteractiveStdin = old } +} diff --git a/internal/config/config.go b/internal/config/config.go index e0aa28a..2501aa7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -3,12 +3,9 @@ package config import ( "github.com/knadh/koanf/parsers/yaml" "github.com/knadh/koanf/providers/file" - "github.com/knadh/koanf/v2" ) -var k = koanf.New(".") - type TestsConfig struct { Ids []int `koanf:"ids"` } @@ -18,22 +15,17 @@ type Config struct { } func ReadConfig(path string) (*Config, error) { + k := koanf.New(".") - file := file.Provider(path) - - err := k.Load(file, yaml.Parser()) - - if err != nil { + f := file.Provider(path) + if err := k.Load(f, yaml.Parser()); err != nil { return nil, err } var out Config - - err = k.Unmarshal("", &out) - if err != nil { + if err := k.Unmarshal("", &out); err != nil { return nil, err } return &out, nil - } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 0fa8df6..d3c3708 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,6 +2,7 @@ package config_test import ( "os" + "path/filepath" "testing" "github.com/google/go-cmp/cmp" @@ -18,11 +19,10 @@ tests: func Test_ReadConfig(t *testing.T) { t.Run("Read valid config file", func(t *testing.T) { - f, err := os.CreateTemp(".", "config*.yaml") + f, err := os.CreateTemp(t.TempDir(), "config*.yaml") if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) if _, err := f.Write([]byte(configFile)); err != nil { t.Fatal(err) @@ -55,11 +55,10 @@ func Test_ReadConfig(t *testing.T) { }) t.Run("Invalid YAML content", func(t *testing.T) { - f, err := os.CreateTemp(".", "invalid*.yaml") + f, err := os.CreateTemp(t.TempDir(), "invalid*.yaml") if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) if _, err := f.Write([]byte("invalid: yaml: content: [")); err != nil { t.Fatal(err) @@ -74,3 +73,33 @@ func Test_ReadConfig(t *testing.T) { } }) } + +func Test_ReadConfig_NoStatePollution(t *testing.T) { + dir := t.TempDir() + + file1 := filepath.Join(dir, "config1.yaml") + if err := os.WriteFile(file1, []byte("tests:\n ids:\n - 1\n - 2\n - 3\n"), 0600); err != nil { + t.Fatal(err) + } + + file2 := filepath.Join(dir, "config2.yaml") + if err := os.WriteFile(file2, []byte("tests:\n ids:\n - 4\n - 5\n - 6\n"), 0600); err != nil { + t.Fatal(err) + } + + out1, err := config.ReadConfig(file1) + if err != nil { + t.Fatal(err) + } + if !cmp.Equal(out1.Tests.Ids, []int{1, 2, 3}) { + t.Errorf("First read: expected [1,2,3], got %v", out1.Tests.Ids) + } + + out2, err := config.ReadConfig(file2) + if err != nil { + t.Fatal(err) + } + if !cmp.Equal(out2.Tests.Ids, []int{4, 5, 6}) { + t.Errorf("Second read: expected [4,5,6], got %v", out2.Tests.Ids) + } +} diff --git a/internal/config/monitor.go b/internal/config/monitor.go index e614b89..184450f 100644 --- a/internal/config/monitor.go +++ b/internal/config/monitor.go @@ -142,6 +142,18 @@ const ( Waw Region = "waw" Yul Region = "yul" Yyz Region = "yyz" + // Koyeb regions (disambiguated from Fly.io regions with same airport code) + KoyebFra Region = "koyeb_fra" + KoyebPar Region = "koyeb_par" + KoyebSfo Region = "koyeb_sfo" + KoyebSin Region = "koyeb_sin" + KoyebTyo Region = "koyeb_tyo" + KoyebWas Region = "koyeb_was" + // Railway regions + RailwayUsWest2 Region = "railway_us-west2" + RailwayUsEast4 Region = "railway_us-east4-eqdc4a" + RailwayEuropeWest4 Region = "railway_europe-west4-drams3a" + RailwayAsiaSoutheast1 Region = "railway_asia-southeast1-eqsg3a" ) type Method string diff --git a/internal/config/openstatus.go b/internal/config/openstatus.go index 9895bf0..3cff6eb 100644 --- a/internal/config/openstatus.go +++ b/internal/config/openstatus.go @@ -3,24 +3,21 @@ package config import ( "github.com/knadh/koanf/parsers/yaml" "github.com/knadh/koanf/providers/file" + "github.com/knadh/koanf/v2" ) type Monitors map[string]Monitor func ReadOpenStatus(path string) (Monitors, error) { - f := file.Provider(path) - - err := k.Load(f, yaml.Parser()) + k := koanf.New(".") - if err != nil { + f := file.Provider(path) + if err := k.Load(f, yaml.Parser()); err != nil { return nil, err } var out Monitors - - err = k.Unmarshal("", &out) - - if err != nil { + if err := k.Unmarshal("", &out); err != nil { return nil, err } diff --git a/internal/config/openstatus_test.go b/internal/config/openstatus_test.go index 3264e26..808a6b1 100644 --- a/internal/config/openstatus_test.go +++ b/internal/config/openstatus_test.go @@ -2,6 +2,7 @@ package config_test import ( "os" + "path/filepath" "testing" "github.com/openstatusHQ/cli/internal/config" @@ -31,11 +32,10 @@ var openstatusConfig = ` func Test_ReadOpenStatus(t *testing.T) { t.Run("Read valid openstatus config", func(t *testing.T) { - f, err := os.CreateTemp(".", "openstatus*.yaml") + f, err := os.CreateTemp(t.TempDir(), "openstatus*.yaml") if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) if _, err := f.Write([]byte(openstatusConfig)); err != nil { t.Fatal(err) @@ -49,9 +49,6 @@ func Test_ReadOpenStatus(t *testing.T) { t.Fatal(err) } - // Check that the monitor was read correctly - // Note: We check for the specific monitor because the global koanf instance - // may have accumulated state from previous tests monitor, exists := out["test-monitor"] if !exists { t.Fatal("Expected 'test-monitor' to exist in output") @@ -112,11 +109,10 @@ func Test_ReadOpenStatus_FollowRedirects(t *testing.T) { url: https://example.com followRedirects: false ` - f, err := os.CreateTemp(".", "openstatus*.yaml") + f, err := os.CreateTemp(t.TempDir(), "openstatus*.yaml") if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) if _, err := f.Write([]byte(yaml)); err != nil { t.Fatal(err) @@ -156,11 +152,10 @@ func Test_ReadOpenStatus_FollowRedirects(t *testing.T) { method: GET url: https://example.com ` - f, err := os.CreateTemp(".", "openstatus*.yaml") + f, err := os.CreateTemp(t.TempDir(), "openstatus*.yaml") if err != nil { t.Fatal(err) } - defer os.Remove(f.Name()) if _, err := f.Write([]byte(yaml)); err != nil { t.Fatal(err) @@ -261,3 +256,64 @@ func Test_ParseConfigMonitorsToMonitor(t *testing.T) { } }) } + +func Test_ReadOpenStatus_NoStatePollution(t *testing.T) { + dir := t.TempDir() + + file1 := filepath.Join(dir, "config1.yaml") + if err := os.WriteFile(file1, []byte(` +"monitor-a": + active: true + frequency: 10m + kind: http + name: Monitor A + regions: + - iad + request: + method: GET + url: https://a.example.com +`), 0600); err != nil { + t.Fatal(err) + } + + file2 := filepath.Join(dir, "config2.yaml") + if err := os.WriteFile(file2, []byte(` +"monitor-b": + active: true + frequency: 5m + kind: http + name: Monitor B + regions: + - ams + request: + method: POST + url: https://b.example.com +`), 0600); err != nil { + t.Fatal(err) + } + + out1, err := config.ReadOpenStatus(file1) + if err != nil { + t.Fatal(err) + } + if _, exists := out1["monitor-a"]; !exists { + t.Error("First read: expected 'monitor-a' to exist") + } + if len(out1) != 1 { + t.Errorf("First read: expected 1 monitor, got %d", len(out1)) + } + + out2, err := config.ReadOpenStatus(file2) + if err != nil { + t.Fatal(err) + } + if _, exists := out2["monitor-b"]; !exists { + t.Error("Second read: expected 'monitor-b' to exist") + } + if _, exists := out2["monitor-a"]; exists { + t.Error("Second read: 'monitor-a' should not be present (state pollution)") + } + if len(out2) != 1 { + t.Errorf("Second read: expected 1 monitor, got %d", len(out2)) + } +} diff --git a/internal/login/login.go b/internal/login/login.go index 0fbbcc6..44aac72 100644 --- a/internal/login/login.go +++ b/internal/login/login.go @@ -4,10 +4,10 @@ import ( "bufio" "context" "fmt" - "net/http" "os" "strings" + "github.com/openstatusHQ/cli/internal/api" "github.com/openstatusHQ/cli/internal/auth" "github.com/openstatusHQ/cli/internal/whoami" "github.com/urfave/cli/v3" @@ -52,7 +52,7 @@ Get your API token from the OpenStatus dashboard.`, } fmt.Fprintln(os.Stderr, "Verifying token...") - err := whoami.GetWhoamiCmd(ctx, http.DefaultClient, token, nil) + err := whoami.GetWhoamiCmd(ctx, api.DefaultHTTPClient, token, nil) if err != nil { return cli.Exit("Invalid token. Could not authenticate with OpenStatus API", 1) } diff --git a/internal/monitors/export_test.go b/internal/monitors/export_test.go new file mode 100644 index 0000000..0b3da60 --- /dev/null +++ b/internal/monitors/export_test.go @@ -0,0 +1,5 @@ +package monitors + +var RegionToString = regionToString +var StringToRegion = stringToRegion +var ConfigToTCPMonitor = configToTCPMonitor diff --git a/internal/monitors/monitor_apply.go b/internal/monitors/monitor_apply.go index 6e813ea..8d4ba29 100644 --- a/internal/monitors/monitor_apply.go +++ b/internal/monitors/monitor_apply.go @@ -4,10 +4,10 @@ import ( "context" "errors" "fmt" - "net/http" "os" "github.com/google/go-cmp/cmp" + "github.com/openstatusHQ/cli/internal/api" "github.com/openstatusHQ/cli/internal/auth" output "github.com/openstatusHQ/cli/internal/cli" "github.com/openstatusHQ/cli/internal/config" @@ -34,18 +34,23 @@ func countChanges(lock config.MonitorsLock, configData config.Monitors) (created } // ApplyChanges applies the changes between the lock file and the config data, making API calls. +// It works on a copy of the lock map so that partial failures leave the caller's map untouched. func ApplyChanges(ctx context.Context, apiKey string, lock config.MonitorsLock, configData config.Monitors) (config.MonitorsLock, error) { + working := make(config.MonitorsLock, len(lock)) + for k, v := range lock { + working[k] = v + } + var created, updated, deleted int for v, configValue := range configData { - value, exist := lock[v] - + value, exist := working[v] if !exist { - result, err := CreateMonitor(ctx, http.DefaultClient, apiKey, configValue) + result, err := CreateMonitor(ctx, api.DefaultHTTPClient, apiKey, configValue) if err != nil { return nil, err } - lock[v] = config.Lock{ + working[v] = config.Lock{ ID: result.ID, Monitor: configValue, } @@ -53,28 +58,31 @@ func ApplyChanges(ctx context.Context, apiKey string, lock config.MonitorsLock, continue } if !cmp.Equal(configValue, value.Monitor) { - result, err := UpdateMonitor(ctx, http.DefaultClient, apiKey, value.ID, configValue) + result, err := UpdateMonitor(ctx, api.DefaultHTTPClient, apiKey, value.ID, configValue) if err != nil { return nil, err } - lock[v] = config.Lock{ + working[v] = config.Lock{ ID: result.ID, Monitor: configValue, } updated++ - continue } } - for v, value := range lock { + var toDelete []string + for v := range working { if _, exist := configData[v]; !exist { - err := DeleteMonitorWithHTTPClient(ctx, http.DefaultClient, apiKey, fmt.Sprintf("%d", value.ID)) - if err != nil { - return nil, fmt.Errorf("failed to delete monitor %d: %w", value.ID, err) - } - delete(lock, v) - deleted++ + toDelete = append(toDelete, v) + } + } + for _, v := range toDelete { + err := DeleteMonitorWithHTTPClient(ctx, api.DefaultHTTPClient, apiKey, fmt.Sprintf("%d", working[v].ID)) + if err != nil { + return nil, fmt.Errorf("failed to delete monitor %d: %w", working[v].ID, err) } + delete(working, v) + deleted++ } if created == 0 && updated == 0 && deleted == 0 { @@ -91,7 +99,7 @@ func ApplyChanges(ctx context.Context, apiKey string, lock config.MonitorsLock, if deleted > 0 { fmt.Println(" Deleted:", deleted) } - return lock, nil + return working, nil } func GetMonitorsApplyCmd() *cli.Command { diff --git a/internal/monitors/monitor_create.go b/internal/monitors/monitor_create.go index 9449739..1b108ec 100644 --- a/internal/monitors/monitor_create.go +++ b/internal/monitors/monitor_create.go @@ -10,6 +10,7 @@ import ( monitorv1 "buf.build/gen/go/openstatus/api/protocolbuffers/go/openstatus/monitor/v1" "buf.build/gen/go/openstatus/api/connectrpc/gosimple/openstatus/monitor/v1/monitorv1connect" + "github.com/openstatusHQ/cli/internal/api" "github.com/openstatusHQ/cli/internal/auth" output "github.com/openstatusHQ/cli/internal/cli" "github.com/openstatusHQ/cli/internal/config" @@ -41,13 +42,17 @@ func CreateHTTPMonitor(ctx context.Context, client monitorv1connect.MonitorServi return Monitor{}, fmt.Errorf("failed to create HTTP monitor: %w", err) } - return httpMonitorToLocal(resp.GetMonitor()), nil + return httpMonitorToLocal(resp.GetMonitor()) } // CreateTCPMonitor creates a TCP monitor using the SDK func CreateTCPMonitor(ctx context.Context, client monitorv1connect.MonitorServiceClient, monitor config.Monitor) (Monitor, error) { + tcpMonitor, err := configToTCPMonitor(monitor) + if err != nil { + return Monitor{}, err + } req := &monitorv1.CreateTCPMonitorRequest{ - Monitor: configToTCPMonitor(monitor), + Monitor: tcpMonitor, } resp, err := client.CreateTCPMonitor(ctx, req) @@ -55,12 +60,14 @@ func CreateTCPMonitor(ctx context.Context, client monitorv1connect.MonitorServic return Monitor{}, fmt.Errorf("failed to create TCP monitor: %w", err) } - return tcpMonitorToLocal(resp.GetMonitor()), nil + return tcpMonitorToLocal(resp.GetMonitor()) } -// httpMonitorToLocal converts SDK HTTPMonitor to local Monitor type -func httpMonitorToLocal(m *monitorv1.HTTPMonitor) Monitor { - id, _ := strconv.Atoi(m.GetId()) +func httpMonitorToLocal(m *monitorv1.HTTPMonitor) (Monitor, error) { + id, err := strconv.Atoi(m.GetId()) + if err != nil { + return Monitor{}, fmt.Errorf("invalid monitor ID %q: %w", m.GetId(), err) + } return Monitor{ ID: id, Name: m.GetName(), @@ -74,12 +81,14 @@ func httpMonitorToLocal(m *monitorv1.HTTPMonitor) Monitor { Timeout: int(m.GetTimeout()), Retry: int(m.GetRetry()), JobType: "http", - } + }, nil } -// tcpMonitorToLocal converts SDK TCPMonitor to local Monitor type -func tcpMonitorToLocal(m *monitorv1.TCPMonitor) Monitor { - id, _ := strconv.Atoi(m.GetId()) +func tcpMonitorToLocal(m *monitorv1.TCPMonitor) (Monitor, error) { + id, err := strconv.Atoi(m.GetId()) + if err != nil { + return Monitor{}, fmt.Errorf("invalid monitor ID %q: %w", m.GetId(), err) + } return Monitor{ ID: id, Name: m.GetName(), @@ -92,7 +101,7 @@ func tcpMonitorToLocal(m *monitorv1.TCPMonitor) Monitor { Timeout: int(m.GetTimeout()), Retry: int(m.GetRetry()), JobType: "tcp", - } + }, nil } func GetMonitorCreateCmd() *cli.Command { @@ -138,7 +147,7 @@ func GetMonitorCreateCmd() *cli.Command { } s := output.StartSpinner("Creating monitors...") for _, value := range monitors { - _, err = CreateMonitor(ctx, http.DefaultClient, apiKey, value) + _, err = CreateMonitor(ctx, api.DefaultHTTPClient, apiKey, value) if err != nil { output.StopSpinner(s) return cli.Exit("Unable to create monitor", 1) diff --git a/internal/monitors/monitor_delete.go b/internal/monitors/monitor_delete.go index c9b47a9..da2c01e 100644 --- a/internal/monitors/monitor_delete.go +++ b/internal/monitors/monitor_delete.go @@ -68,6 +68,10 @@ func GetMonitorDeleteCmd() *cli.Command { return cli.Exit(err.Error(), 1) } monitorId := cmd.Args().Get(0) + if monitorId == "" { + fmt.Fprintln(os.Stderr, "Usage: openstatus monitors delete ") + return cli.Exit("monitor ID is required", 1) + } if !cmd.Bool("auto-accept") { confirmed, err := output.AskForConfirmation(fmt.Sprintf("You are about to delete monitor: %s, do you want to continue", monitorId)) diff --git a/internal/monitors/monitor_import.go b/internal/monitors/monitor_import.go index ff68b71..d1a4909 100644 --- a/internal/monitors/monitor_import.go +++ b/internal/monitors/monitor_import.go @@ -25,67 +25,57 @@ func ExportMonitor(ctx context.Context, client monitorv1connect.MonitorServiceCl } t := map[string]config.Monitor{} - lock := make(map[string]config.Lock) - file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return err - } - defer file.Close() - - // Process HTTP monitors for _, monitor := range resp.GetHttpMonitors() { - configMonitor := convertHTTPMonitorToConfig(monitor) - id := monitor.GetId() - t[id] = configMonitor + t[monitor.GetId()] = convertHTTPMonitorToConfig(monitor) } - // Process TCP monitors for _, monitor := range resp.GetTcpMonitors() { - configMonitor := convertTCPMonitorToConfig(monitor) - id := monitor.GetId() - t[id] = configMonitor + t[monitor.GetId()] = convertTCPMonitorToConfig(monitor) } - // Process DNS monitors (skip for now as config.Monitor doesn't support DNS) - // DNS monitors would need config.Kind = "dns" support - - y, err := yaml.Marshal(&t) + configYAML, err := yaml.Marshal(&t) if err != nil { return err } - _, err = file.WriteString("# yaml-language-server: $schema=https://www.openstatus.dev/schema.json\n\n") - if err != nil { - return err - } - _, err = file.Write(y) - if err != nil { - return err - } - - // Build lock file + lock := make(map[string]config.Lock, len(t)) for id, monitor := range t { - i, _ := strconv.Atoi(id) + i, err := strconv.Atoi(id) + if err != nil { + return fmt.Errorf("invalid monitor ID %q: %w", id, err) + } lock[id] = config.Lock{ ID: i, Monitor: monitor, } } - lockFile, err := os.OpenFile("openstatus.lock", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) + lockYAML, err := yaml.Marshal(&lock) if err != nil { - return fmt.Errorf("failed to create lock file: %w", err) + return fmt.Errorf("failed to marshal lock file: %w", err) } - defer lockFile.Close() - y, err = yaml.Marshal(&lock) + file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) if err != nil { - return fmt.Errorf("failed to marshal lock file: %w", err) + return err + } + defer file.Close() + + if _, err := file.WriteString("# yaml-language-server: $schema=https://www.openstatus.dev/schema.json\n\n"); err != nil { + return err + } + if _, err := file.Write(configYAML); err != nil { + return err } - _, err = lockFile.Write(y) + lockFile, err := os.OpenFile("openstatus.lock", os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0600) if err != nil { + return fmt.Errorf("failed to create lock file: %w", err) + } + defer lockFile.Close() + + if _, err := lockFile.Write(lockYAML); err != nil { return fmt.Errorf("failed to write lock file: %w", err) } diff --git a/internal/monitors/monitor_info.go b/internal/monitors/monitor_info.go index 7844f32..ea42bce 100644 --- a/internal/monitors/monitor_info.go +++ b/internal/monitors/monitor_info.go @@ -8,6 +8,7 @@ import ( "strings" monitorv1 "buf.build/gen/go/openstatus/api/protocolbuffers/go/openstatus/monitor/v1" + "github.com/openstatusHQ/cli/internal/api" "github.com/openstatusHQ/cli/internal/auth" output "github.com/openstatusHQ/cli/internal/cli" "github.com/logrusorgru/aurora/v4" @@ -44,13 +45,22 @@ func GetMonitorInfo(ctx context.Context, httpClient *http.Client, apiKey string, var regions []monitorv1.Region switch { case monitorConfig.HasHttp(): - monitor = httpMonitorToLocal(monitorConfig.GetHttp()) + monitor, err = httpMonitorToLocal(monitorConfig.GetHttp()) + if err != nil { + return err + } regions = monitorConfig.GetHttp().GetRegions() case monitorConfig.HasTcp(): - monitor = tcpMonitorToLocal(monitorConfig.GetTcp()) + monitor, err = tcpMonitorToLocal(monitorConfig.GetTcp()) + if err != nil { + return err + } regions = monitorConfig.GetTcp().GetRegions() default: - return fmt.Errorf("unknown monitor type") + if monitorConfig.HasDns() { + return fmt.Errorf("DNS monitors are not yet supported in the CLI. Monitor ID: %s", monitorId) + } + return fmt.Errorf("unknown monitor type for monitor ID: %s", monitorId) } if output.IsJSONOutput() { @@ -141,7 +151,7 @@ func GetMonitorInfoCmd() *cli.Command { } monitorId := cmd.Args().Get(0) s := output.StartSpinner("Fetching monitor details...") - err = GetMonitorInfo(ctx, http.DefaultClient, apiKey, monitorId, s) + err = GetMonitorInfo(ctx, api.DefaultHTTPClient, apiKey, monitorId, s) if err != nil { return cli.Exit(err.Error(), 1) } diff --git a/internal/monitors/monitor_trigger.go b/internal/monitors/monitor_trigger.go index 85e2eee..3ee9f8d 100644 --- a/internal/monitors/monitor_trigger.go +++ b/internal/monitors/monitor_trigger.go @@ -31,6 +31,9 @@ func TriggerMonitor(ctx context.Context, client monitorv1connect.MonitorServiceC return output.FormatError(err, "monitor", monitorId) } + if output.IsJSONOutput() { + return output.PrintJSON(map[string]string{"status": "triggered", "monitor_id": monitorId}) + } fmt.Printf("Check triggered successfully\n") return nil } diff --git a/internal/monitors/monitor_update.go b/internal/monitors/monitor_update.go index d355516..f788095 100644 --- a/internal/monitors/monitor_update.go +++ b/internal/monitors/monitor_update.go @@ -40,12 +40,15 @@ func UpdateHTTPMonitor(ctx context.Context, client monitorv1connect.MonitorServi return Monitor{}, fmt.Errorf("failed to update HTTP monitor: %w", err) } - return httpMonitorToLocal(resp.GetMonitor()), nil + return httpMonitorToLocal(resp.GetMonitor()) } // UpdateTCPMonitor updates a TCP monitor using the SDK func UpdateTCPMonitor(ctx context.Context, client monitorv1connect.MonitorServiceClient, id int, monitor config.Monitor) (Monitor, error) { - tcpMonitor := configToTCPMonitor(monitor) + tcpMonitor, err := configToTCPMonitor(monitor) + if err != nil { + return Monitor{}, err + } tcpMonitor.Id = strconv.Itoa(id) req := &monitorv1.UpdateTCPMonitorRequest{ @@ -58,5 +61,5 @@ func UpdateTCPMonitor(ctx context.Context, client monitorv1connect.MonitorServic return Monitor{}, fmt.Errorf("failed to update TCP monitor: %w", err) } - return tcpMonitorToLocal(resp.GetMonitor()), nil + return tcpMonitorToLocal(resp.GetMonitor()) } diff --git a/internal/monitors/monitors.go b/internal/monitors/monitors.go index a13adee..8449f65 100644 --- a/internal/monitors/monitors.go +++ b/internal/monitors/monitors.go @@ -16,7 +16,7 @@ import ( func NewMonitorClient(apiKey string) monitorv1connect.MonitorServiceClient { return monitorv1connect.NewMonitorServiceClient( - http.DefaultClient, + api.DefaultHTTPClient, api.ConnectBaseURL, connect.WithInterceptors(api.NewAuthInterceptor(apiKey)), connect.WithProtoJSON(), @@ -121,21 +121,27 @@ func regionToString(r monitorv1.Region) string { case monitorv1.Region_REGION_FLY_YYZ: return "yyz" // Koyeb regions - case monitorv1.Region_REGION_KOYEB_SFO: - return "sfo" - case monitorv1.Region_REGION_KOYEB_WAS: - return "was" case monitorv1.Region_REGION_KOYEB_FRA: - return "fra" + return string(config.KoyebFra) case monitorv1.Region_REGION_KOYEB_PAR: - return "par" + return string(config.KoyebPar) + case monitorv1.Region_REGION_KOYEB_SFO: + return string(config.KoyebSfo) case monitorv1.Region_REGION_KOYEB_SIN: - return "sin" + return string(config.KoyebSin) case monitorv1.Region_REGION_KOYEB_TYO: - return "tyo" + return string(config.KoyebTyo) + case monitorv1.Region_REGION_KOYEB_WAS: + return string(config.KoyebWas) // Railway regions case monitorv1.Region_REGION_RAILWAY_US_WEST2: - return "us-west2" + return string(config.RailwayUsWest2) + case monitorv1.Region_REGION_RAILWAY_US_EAST4: + return string(config.RailwayUsEast4) + case monitorv1.Region_REGION_RAILWAY_EUROPE_WEST4: + return string(config.RailwayEuropeWest4) + case monitorv1.Region_REGION_RAILWAY_ASIA_SOUTHEAST1: + return string(config.RailwayAsiaSoutheast1) default: return r.String() } @@ -254,6 +260,28 @@ func stringToRegion(r config.Region) monitorv1.Region { return monitorv1.Region_REGION_FLY_SYD case config.Yyz: return monitorv1.Region_REGION_FLY_YYZ + // Koyeb regions + case config.KoyebFra: + return monitorv1.Region_REGION_KOYEB_FRA + case config.KoyebPar: + return monitorv1.Region_REGION_KOYEB_PAR + case config.KoyebSfo: + return monitorv1.Region_REGION_KOYEB_SFO + case config.KoyebSin: + return monitorv1.Region_REGION_KOYEB_SIN + case config.KoyebTyo: + return monitorv1.Region_REGION_KOYEB_TYO + case config.KoyebWas: + return monitorv1.Region_REGION_KOYEB_WAS + // Railway regions + case config.RailwayUsWest2: + return monitorv1.Region_REGION_RAILWAY_US_WEST2 + case config.RailwayUsEast4: + return monitorv1.Region_REGION_RAILWAY_US_EAST4 + case config.RailwayEuropeWest4: + return monitorv1.Region_REGION_RAILWAY_EUROPE_WEST4 + case config.RailwayAsiaSoutheast1: + return monitorv1.Region_REGION_RAILWAY_ASIA_SOUTHEAST1 default: return monitorv1.Region_REGION_UNSPECIFIED } @@ -393,7 +421,14 @@ func configToHTTPMonitor(m config.Monitor) *monitorv1.HTTPMonitor { } // configToTCPMonitor converts config.Monitor to SDK TCPMonitor -func configToTCPMonitor(m config.Monitor) *monitorv1.TCPMonitor { +func configToTCPMonitor(m config.Monitor) (*monitorv1.TCPMonitor, error) { + if m.Request.Host == "" { + return nil, fmt.Errorf("TCP monitor %q: host is required", m.Name) + } + if m.Request.Port <= 0 || m.Request.Port > 65535 { + return nil, fmt.Errorf("TCP monitor %q: port must be between 1 and 65535, got %d", m.Name, m.Request.Port) + } + monitor := &monitorv1.TCPMonitor{ Name: m.Name, Description: m.Description, @@ -410,7 +445,7 @@ func configToTCPMonitor(m config.Monitor) *monitorv1.TCPMonitor { monitor.DegradedAt = &m.DegradedAfter } - return monitor + return monitor, nil } type Monitor struct { diff --git a/internal/monitors/monitors_region_test.go b/internal/monitors/monitors_region_test.go new file mode 100644 index 0000000..763ed40 --- /dev/null +++ b/internal/monitors/monitors_region_test.go @@ -0,0 +1,76 @@ +package monitors_test + +import ( + "testing" + + monitorv1 "buf.build/gen/go/openstatus/api/protocolbuffers/go/openstatus/monitor/v1" + "github.com/openstatusHQ/cli/internal/config" + "github.com/openstatusHQ/cli/internal/monitors" +) + +func Test_RegionRoundTrip(t *testing.T) { + for i := 1; i <= 28; i++ { + r := monitorv1.Region(i) + t.Run(r.String(), func(t *testing.T) { + code := monitors.RegionToString(r) + back := monitors.StringToRegion(config.Region(code)) + if back != r { + t.Errorf("round-trip failed: %v -> %q -> %v", r, code, back) + } + }) + } +} + +func Test_RegionToString_NoCollisions(t *testing.T) { + seen := make(map[string]monitorv1.Region) + for i := 1; i <= 28; i++ { + r := monitorv1.Region(i) + code := monitors.RegionToString(r) + if prev, exists := seen[code]; exists { + t.Errorf("collision: %v and %v both map to %q", prev, r, code) + } + seen[code] = r + } +} + +func Test_RegionToString_Koyeb(t *testing.T) { + tests := []struct { + region monitorv1.Region + expected string + }{ + {monitorv1.Region_REGION_KOYEB_FRA, "koyeb_fra"}, + {monitorv1.Region_REGION_KOYEB_PAR, "koyeb_par"}, + {monitorv1.Region_REGION_KOYEB_SFO, "koyeb_sfo"}, + {monitorv1.Region_REGION_KOYEB_SIN, "koyeb_sin"}, + {monitorv1.Region_REGION_KOYEB_TYO, "koyeb_tyo"}, + {monitorv1.Region_REGION_KOYEB_WAS, "koyeb_was"}, + } + for _, tt := range tests { + t.Run(tt.region.String(), func(t *testing.T) { + got := monitors.RegionToString(tt.region) + if got != tt.expected { + t.Errorf("RegionToString(%v) = %q, want %q", tt.region, got, tt.expected) + } + }) + } +} + +func Test_RegionToString_Railway(t *testing.T) { + tests := []struct { + region monitorv1.Region + expected string + }{ + {monitorv1.Region_REGION_RAILWAY_US_WEST2, "railway_us-west2"}, + {monitorv1.Region_REGION_RAILWAY_US_EAST4, "railway_us-east4-eqdc4a"}, + {monitorv1.Region_REGION_RAILWAY_EUROPE_WEST4, "railway_europe-west4-drams3a"}, + {monitorv1.Region_REGION_RAILWAY_ASIA_SOUTHEAST1, "railway_asia-southeast1-eqsg3a"}, + } + for _, tt := range tests { + t.Run(tt.region.String(), func(t *testing.T) { + got := monitors.RegionToString(tt.region) + if got != tt.expected { + t.Errorf("RegionToString(%v) = %q, want %q", tt.region, got, tt.expected) + } + }) + } +} diff --git a/internal/monitors/monitors_tcp_test.go b/internal/monitors/monitors_tcp_test.go new file mode 100644 index 0000000..8c0b30b --- /dev/null +++ b/internal/monitors/monitors_tcp_test.go @@ -0,0 +1,86 @@ +package monitors_test + +import ( + "strings" + "testing" + + "github.com/openstatusHQ/cli/internal/config" + "github.com/openstatusHQ/cli/internal/monitors" +) + +func Test_ConfigToTCPMonitor_ZeroPort(t *testing.T) { + m := config.Monitor{ + Name: "test", + Kind: config.TCP, + Request: config.Request{Host: "example.com", Port: 0}, + } + _, err := monitors.ConfigToTCPMonitor(m) + if err == nil { + t.Error("expected error for port 0") + } +} + +func Test_ConfigToTCPMonitor_NegativePort(t *testing.T) { + m := config.Monitor{ + Name: "test", + Kind: config.TCP, + Request: config.Request{Host: "example.com", Port: -1}, + } + _, err := monitors.ConfigToTCPMonitor(m) + if err == nil { + t.Error("expected error for negative port") + } +} + +func Test_ConfigToTCPMonitor_ExceedsMax(t *testing.T) { + m := config.Monitor{ + Name: "test", + Kind: config.TCP, + Request: config.Request{Host: "example.com", Port: 65536}, + } + _, err := monitors.ConfigToTCPMonitor(m) + if err == nil { + t.Error("expected error for port > 65535") + } +} + +func Test_ConfigToTCPMonitor_EmptyHost(t *testing.T) { + m := config.Monitor{ + Name: "test", + Kind: config.TCP, + Request: config.Request{Host: "", Port: 80}, + } + _, err := monitors.ConfigToTCPMonitor(m) + if err == nil { + t.Error("expected error for empty host") + } +} + +func Test_ConfigToTCPMonitor_ValidInputs(t *testing.T) { + m := config.Monitor{ + Name: "test", + Kind: config.TCP, + Frequency: config.The10M, + Request: config.Request{Host: "example.com", Port: 80}, + } + result, err := monitors.ConfigToTCPMonitor(m) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(result.GetUri(), "example.com:80") { + t.Errorf("expected URI to contain 'example.com:80', got %q", result.GetUri()) + } +} + +func Test_ConfigToTCPMonitor_MaxPort(t *testing.T) { + m := config.Monitor{ + Name: "test", + Kind: config.TCP, + Frequency: config.The10M, + Request: config.Request{Host: "db.internal", Port: 65535}, + } + _, err := monitors.ConfigToTCPMonitor(m) + if err != nil { + t.Fatalf("unexpected error for port 65535: %v", err) + } +} diff --git a/internal/run/run.go b/internal/run/run.go index e6af663..71c3af5 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -110,7 +110,8 @@ func MonitorTrigger(ctx context.Context, httpClient *http.Client, apiKey string, entry.Error = httpResult.Error } default: - return runMonitorResult{}, fmt.Errorf("unknown job type") + entry.Status = "unknown" + entry.Error = fmt.Sprintf("unknown job type: %s", rr.JobType) } regionResults = append(regionResults, entry) diff --git a/internal/statusreport/statusreport.go b/internal/statusreport/statusreport.go index 55c309a..25b2baf 100644 --- a/internal/statusreport/statusreport.go +++ b/internal/statusreport/statusreport.go @@ -14,7 +14,7 @@ import ( func NewStatusReportClient(apiKey string) status_reportv1connect.StatusReportServiceClient { return status_reportv1connect.NewStatusReportServiceClient( - http.DefaultClient, + api.DefaultHTTPClient, api.ConnectBaseURL, connect.WithInterceptors(api.NewAuthInterceptor(apiKey)), connect.WithProtoJSON(), diff --git a/internal/statusreport/statusreport_add_update.go b/internal/statusreport/statusreport_add_update.go index fd3d2d6..6bbbf78 100644 --- a/internal/statusreport/statusreport_add_update.go +++ b/internal/statusreport/statusreport_add_update.go @@ -25,6 +25,7 @@ func AddStatusReportUpdate(ctx context.Context, client status_reportv1connect.St sdkStatus, err := statusToSDK(status) if err != nil { + output.StopSpinner(s) return err } diff --git a/internal/statusreport/statusreport_delete.go b/internal/statusreport/statusreport_delete.go index 885b28f..830a476 100644 --- a/internal/statusreport/statusreport_delete.go +++ b/internal/statusreport/statusreport_delete.go @@ -61,6 +61,10 @@ func GetStatusReportDeleteCmd() *cli.Command { return cli.Exit(err.Error(), 1) } reportId := cmd.Args().Get(0) + if reportId == "" { + fmt.Fprintln(os.Stderr, "Usage: openstatus status-report delete ") + return cli.Exit("report ID is required", 1) + } if !cmd.Bool("auto-accept") { confirmed, err := output.AskForConfirmation(fmt.Sprintf("You are about to delete status report: %s, do you want to continue", reportId)) diff --git a/internal/statusreport/statusreport_list.go b/internal/statusreport/statusreport_list.go index 7e5835d..85b5599 100644 --- a/internal/statusreport/statusreport_list.go +++ b/internal/statusreport/statusreport_list.go @@ -31,11 +31,12 @@ func ListStatusReports(ctx context.Context, client status_reportv1connect.Status } if statusFilter != "" { - s, err := statusToSDK(statusFilter) + sdkStatus, err := statusToSDK(statusFilter) if err != nil { + output.StopSpinner(s) return err } - req.SetStatuses([]status_reportv1.StatusReportStatus{s}) + req.SetStatuses([]status_reportv1.StatusReportStatus{sdkStatus}) } resp, err := client.ListStatusReports(ctx, req) diff --git a/internal/statusreport/statusreport_update.go b/internal/statusreport/statusreport_update.go index f5f3630..8385bc7 100644 --- a/internal/statusreport/statusreport_update.go +++ b/internal/statusreport/statusreport_update.go @@ -80,7 +80,7 @@ func GetStatusReportUpdateCmd() *cli.Command { reportId := cmd.Args().Get(0) hasTitle := cmd.IsSet("title") - hasComponents := cmd.IsSet("component-ids") + hasComponents := cmd.IsSet("component-ids") && cmd.String("component-ids") != "" var componentIds []string if ids := cmd.String("component-ids"); ids != "" { diff --git a/internal/whoami/whoami.go b/internal/whoami/whoami.go index ac07570..62fc001 100644 --- a/internal/whoami/whoami.go +++ b/internal/whoami/whoami.go @@ -75,7 +75,7 @@ Displays the workspace name, slug, and plan.`, fmt.Println("Your current workspace information") } s := output.StartSpinner("Fetching workspace info...") - err = GetWhoamiCmd(ctx, http.DefaultClient, apiKey, s) + err = GetWhoamiCmd(ctx, api.DefaultHTTPClient, apiKey, s) if err != nil { return cli.Exit(err.Error(), 1) }