From dc04497d59509d6a89fb433e2ae25691fd14ea88 Mon Sep 17 00:00:00 2001 From: Ed Zavada Date: Thu, 9 Jan 2025 11:25:18 -0500 Subject: [PATCH] Addressed review feedback - fixed all lint issues - error handling for all JSON response parsing - switched to using port 8001 instead of 8081 - got rid of magic number for project name length --- cmd/world/forge/deployment.go | 141 ++++++++++++++++++++++++++-------- cmd/world/forge/forge_test.go | 2 +- cmd/world/forge/project.go | 4 +- 3 files changed, 112 insertions(+), 35 deletions(-) diff --git a/cmd/world/forge/deployment.go b/cmd/world/forge/deployment.go index 095772f..4a27cda 100644 --- a/cmd/world/forge/deployment.go +++ b/cmd/world/forge/deployment.go @@ -134,6 +134,7 @@ func destroy(ctx context.Context) error { return nil } +//nolint:funlen, gocognit, gocyclo, cyclop // this is actually a straightforward function with a lot of error handling func status(ctx context.Context) error { globalConfig, err := globalconfig.GetGlobalConfig() if err != nil { @@ -158,11 +159,18 @@ func status(ctx context.Context) error { var response map[string]any err = json.Unmarshal(result, &response) if err != nil { - return eris.Wrap(err, "Failed to unmarshal deployment status") + return eris.Wrap(err, "Failed to unmarshal deployment response") } var data map[string]any if response["data"] != nil { - data = response["data"].(map[string]any) + // data = null is returned when there are no deployments, so we have to check for that before we + // try to cast the response into a json object map, since this is not an error but the cast would + // fail + var ok bool + data, ok = response["data"].(map[string]any) + if !ok { + return eris.New("Failed to unmarshal deployment data") + } } fmt.Println("Deployment Status") fmt.Println("-----------------") @@ -179,17 +187,35 @@ func status(ctx context.Context) error { if data["type"] != "deploy" { return eris.Errorf("Deployment status does not match type %s", data["type"]) } - executorID := data["executor_id"].(string) - dt, dte := time.Parse(time.RFC3339, data["execution_time"].(string)) + executorID, ok := data["executor_id"].(string) + if !ok { + return eris.New("Failed to unmarshal deployment executor_id") + } + executionTimeStr, ok := data["execution_time"].(string) + if !ok { + return eris.New("Failed to unmarshal deployment execution_time") + } + dt, dte := time.Parse(time.RFC3339, executionTimeStr) if dte != nil { - return eris.Wrapf(dte, "Failed to parse execution time %s", dt) + return eris.Wrapf(dte, "Failed to parse deployment execution_time %s", dt) } - buildNumber := int(data["build_number"].(float64)) - bt, bte := time.Parse(time.RFC3339, data["build_start_time"].(string)) + bnf, ok := data["build_number"].(float64) + if !ok { + return eris.New("Failed to unmarshal deployment build_number") + } + buildNumber := int(bnf) + buildTimeStr, ok := data["build_time"].(string) + if !ok { + return eris.New("Failed to unmarshal deployment build_time") + } + bt, bte := time.Parse(time.RFC3339, buildTimeStr) if bte != nil { - return eris.Wrapf(bte, "Failed to parse build start time %s", bt) + return eris.Wrapf(bte, "Failed to parse deployment build_time %s", bt) + } + buildState, ok := data["build_state"].(string) + if !ok { + return eris.New("Failed to unmarshal deployment build_state") } - buildState := data["build_state"].(string) if buildState != "finished" { fmt.Printf("Build: #%d started %s by %s - %s\n", buildNumber, dt.Format(time.RFC822), executorID, buildState) return nil @@ -207,9 +233,15 @@ func status(ctx context.Context) error { } err = json.Unmarshal(result, &response) if err != nil { - return eris.Wrap(err, "Failed to unmarshal deployment status") + return eris.Wrap(err, "Failed to unmarshal health response") + } + if response["data"] == nil { + return eris.New("Failed to unmarshal health data") + } + instances, ok := response["data"].([]any) + if !ok { + return eris.New("Failed to unmarshal deployment status") } - instances := response["data"].([]any) if len(instances) == 0 { fmt.Println("** No deployed instances found **") return nil @@ -217,46 +249,89 @@ func status(ctx context.Context) error { fmt.Printf("(%d deployed instances)\n", len(instances)) currRegion := "" for _, instance := range instances { - info := instance.(map[string]any) - region := info["region"].(string) - instanceNum := int(info["instance"].(float64)) - cardinalInfo := info["cardinal"].(map[string]any) - nakamaInfo := info["nakama"].(map[string]any) - cardinalURL := cardinalInfo["url"].(string) + info, ok := instance.(map[string]any) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d info", instance) + } + region, ok := info["region"].(string) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d region", instance) + } + instancef, ok := info["instance"].(float64) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d instance number", instance) + } + instanceNum := int(instancef) + cardinalInfo, ok := info["cardinal"].(map[string]any) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d cardinal data", instance) + } + nakamaInfo, ok := info["nakama"].(map[string]any) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d nakama data", instance) + } + cardinalURL, ok := cardinalInfo["url"].(string) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d cardinal url", instance) + } cardinalHost := strings.Split(cardinalURL, "/")[2] - cardinalOK := cardinalInfo["ok"].(bool) - cardinalResultCode := int(cardinalInfo["result_code"].(float64)) - cardinalResultStr := cardinalInfo["result_str"].(string) - nakamaURL := nakamaInfo["url"].(string) + cardinalOK, ok := cardinalInfo["ok"].(bool) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d cardinal ok flag", instance) + } + cardinalResultCodef, ok := cardinalInfo["result_code"].(float64) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d cardinal result_code", instance) + } + cardinalResultCode := int(cardinalResultCodef) + cardinalResultStr, ok := cardinalInfo["result_str"].(string) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d cardinal result_str", instance) + } + nakamaURL, ok := nakamaInfo["url"].(string) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d nakama url", instance) + } nakamaHost := strings.Split(nakamaURL, "/")[2] - nakamaOK := nakamaInfo["ok"].(bool) - nakamaResultCode := int(nakamaInfo["result_code"].(float64)) - nakamaResultStr := nakamaInfo["result_str"].(string) - + nakamaOK, ok := nakamaInfo["ok"].(bool) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d nakama ok", instance) + } + nakamaResultCodef, ok := nakamaInfo["result_code"].(float64) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d result_code", instance) + } + nakamaResultCode := int(nakamaResultCodef) + nakamaResultStr, ok := nakamaInfo["result_str"].(string) + if !ok { + return eris.Errorf("Failed to unmarshal deployment instance %d result_str", instance) + } if region != currRegion { currRegion = region fmt.Printf("• %s\n", currRegion) } fmt.Printf(" %d)", instanceNum) fmt.Printf("\tCardinal: %s - ", cardinalHost) - if cardinalOK { + switch { + case cardinalOK: fmt.Print("OK\n") - } else if cardinalResultCode == 0 { + case cardinalResultCode == 0: fmt.Printf("FAIL %s\n", statusFailRegEx.ReplaceAllString(cardinalResultStr, "")) - } else { + default: fmt.Printf("FAIL %d %s\n", cardinalResultCode, statusFailRegEx.ReplaceAllString(cardinalResultStr, "")) } fmt.Printf("\tNakama: %s - ", nakamaHost) - if nakamaOK { + switch { + case nakamaOK: fmt.Print("OK\n") - } else if nakamaResultCode == 0 { + case nakamaResultCode == 0: fmt.Printf("FAIL %s\n", statusFailRegEx.ReplaceAllString(nakamaResultStr, "")) - } else { + default: fmt.Printf("FAIL %d %s\n", nakamaResultCode, statusFailRegEx.ReplaceAllString(nakamaResultStr, "")) } } - //fmt.Println() - //fmt.Println(string(result)) + // fmt.Println() + // fmt.Println(string(result)) return nil } diff --git a/cmd/world/forge/forge_test.go b/cmd/world/forge/forge_test.go index c2a41a2..442e111 100644 --- a/cmd/world/forge/forge_test.go +++ b/cmd/world/forge/forge_test.go @@ -34,7 +34,7 @@ func (s *ForgeTestSuite) SetupTest() { s.ctx = context.Background() // Create test server on port 8001 - listener, err := net.Listen("tcp", ":8081") + listener, err := net.Listen("tcp", ":8001") s.Require().NoError(err) // Create test server diff --git a/cmd/world/forge/project.go b/cmd/world/forge/project.go index a1fc1c1..6c9e146 100644 --- a/cmd/world/forge/project.go +++ b/cmd/world/forge/project.go @@ -12,6 +12,8 @@ import ( "pkg.world.dev/world-cli/common/globalconfig" ) +const MaxProjectNameLen = 50 + type project struct { ID string `json:"id"` OrgID string `json:"org_id"` @@ -187,7 +189,7 @@ func inputProjectNameAndSlug() (string, string, error) { } // Check length (arbitrary max of 50 chars) - if len(name) > 50 { + if len(name) > MaxProjectNameLen { fmt.Printf("Error: Project name cannot be longer than 50 characters\n") attempts++ continue