fix: crash when list fails and cloud flags are present. (#2029)

## What this PR does / why we need it: If you do `terramate list --status=<any status>` but list fails for any reason unrelated to the cloud feature then Terramate crashes. This is an edge case that only happens if `list` would fail due to a `git` issue but a cloud filter flag is present. ``` $ terramate list --status=ok --target=aaa --changed panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x156d4a1] goroutine 1 [running]: github.com/terramate-io/terramate/cmd/terramate/cli.(*cli).listStacks(0xc0007f5000, 0x48?, {0xc000554848, 0x3}, {0x0?, 0x0?, 0x0?}, 0x0?) /home/i4k/go/pkg/mod/github.com/terramate-io/[email protected]/cmd/terramate/cli/cli.go:1493 +0x601 github.com/terramate-io/terramate/cmd/terramate/cli.(*cli).printStacks(0xc0007f5000) /home/i4k/go/pkg/mod/github.com/terramate-io/[email protected]/cmd/terramate/cli/cli.go:1970 +0x2aa github.com/terramate-io/terramate/cmd/terramate/cli.(*cli).run(0xc0007f5000) /home/i4k/go/pkg/mod/github.com/terramate-io/[email protected]/cmd/terramate/cli/cli.go:729 +0x7f2 github.com/terramate-io/terramate/cmd/terramate/cli.Exec({0x1a08502, 0x6}, {0xc0000400b0, 0x4, 0x4}, {0x1db2ec0, 0xc00007a090}, {0x1db2ee0, 0xc00007a098}, {0x1db2ee0, ...}) /home/i4k/go/pkg/mod/github.com/terramate-io/[email protected]/cmd/terramate/cli/cli.go:389 +0xfc main.main() /home/i4k/go/pkg/mod/github.com/terramate-io/[email protected]/cmd/terramate/main.go:20 +0x89 ``` ## Which issue(s) this PR fixes: none ## Special notes for your reviewer: ## Does this PR introduce a user-facing change? ``` yes, fixes a crash. ```
terramate-io · Jan 7, 2025 · 4091916 · 4091916
2 parents 3378ebc + 34fc8bc
commit 4091916
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 58 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,12 @@ Given a version number `MAJOR.MINOR.PATCH`, we increment the:
 - Backward compatibility in versions `0.0.z` is **not guaranteed** when `z` is increased.
 - Backward compatibility in versions `0.y.z` is **not guaranteed** when `y` is increased.
 
+## Unreleased
+
+### Fixed
+
+- Fix an edge case crash in `terramate list` when using the `--status` flag.
+
 ## v0.11.6
 
 ### Fixed

diff --git a/cmd/terramate/cli/cli.go b/cmd/terramate/cli/cli.go
@@ -1455,12 +1455,14 @@ func (c *cli) listStacks(isChanged bool, target string, stackFilters cloud.Statu
 		report, err = mgr.List(checkRepo)
 	}
 
-	if report != nil {
-		// memoize the list of affected stacks so they can be retrieved later
-		// without computing the list again
-		c.affectedStacks = report.Stacks
+	if err != nil {
+		return nil, err
 	}
 
+	// memoize the list of affected stacks so they can be retrieved later
+	// without computing the list again
+	c.affectedStacks = report.Stacks
+
 	if stackFilters.HasFilter() {
 		if !c.prj.isRepo {
 			fatal(errors.E("cloud filters requires a git repository"))
@@ -1501,10 +1503,6 @@ func (c *cli) listStacks(isChanged bool, target string, stackFilters cloud.Statu
 		report.Stacks = stacks
 	}
 
-	if err != nil {
-		return nil, err
-	}
-
 	c.prj.git.repoChecks = report.Checks
 	return report, nil
 }

diff --git a/e2etests/cloud/cloud_status_test.go b/e2etests/cloud/cloud_status_test.go
@@ -13,14 +13,17 @@ import (
 
 	"github.com/madlambda/spells/assert"
 	"github.com/terramate-io/terramate/cloud"
+	"github.com/terramate-io/terramate/stack"
+
 	"github.com/terramate-io/terramate/cloud/deployment"
 	"github.com/terramate-io/terramate/cloud/drift"
-	"github.com/terramate-io/terramate/cloud/stack"
+	cloudstack "github.com/terramate-io/terramate/cloud/stack"
 	"github.com/terramate-io/terramate/cloud/testserver/cloudstore"
 	"github.com/terramate-io/terramate/cmd/terramate/cli/clitest"
 	. "github.com/terramate-io/terramate/e2etests/internal/runner"
 	"github.com/terramate-io/terramate/test"
 	"github.com/terramate-io/terramate/test/hclwrite"
+
 	. "github.com/terramate-io/terramate/test/hclwrite/hclutils"
 	"github.com/terramate-io/terramate/test/sandbox"
 )
@@ -94,7 +97,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Unknown,
 					},
@@ -105,7 +108,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -137,7 +140,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Failed,
 					},
@@ -158,7 +161,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Failed,
 					},
@@ -179,7 +182,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -203,7 +206,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -227,7 +230,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -251,7 +254,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -275,7 +278,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -299,7 +302,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "gitlab.com/unknown-io/other",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.OK,
 					},
@@ -320,7 +323,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.Drifted,
 					},
@@ -345,7 +348,7 @@ func TestCloudStatus(t *testing.T) {
 						MetaTags:   []string{"something.with.dots"},
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.Drifted,
 					},
@@ -369,7 +372,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -390,7 +393,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.Drifted,
 					},
@@ -414,7 +417,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.OK,
 					},
@@ -425,7 +428,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -446,7 +449,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.OK,
 					},
@@ -457,7 +460,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -478,7 +481,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.OK,
 					},
@@ -489,7 +492,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -513,7 +516,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.Drifted,
 					},
@@ -524,7 +527,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -548,7 +551,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.Drifted,
 					},
@@ -559,7 +562,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -584,7 +587,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Failed,
+						Status:           cloudstack.Failed,
 						DeploymentStatus: deployment.Failed,
 						DriftStatus:      drift.OK,
 					},
@@ -595,7 +598,7 @@ func TestCloudStatus(t *testing.T) {
 						Repository: "github.com/terramate-io/terramate",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -621,7 +624,7 @@ func TestCloudStatus(t *testing.T) {
 						Target:     "default",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -633,7 +636,7 @@ func TestCloudStatus(t *testing.T) {
 						Target:     "stage",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.Drifted,
+						Status:           cloudstack.Drifted,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.Drifted,
 					},
@@ -645,7 +648,7 @@ func TestCloudStatus(t *testing.T) {
 						Target:     "stage",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -670,7 +673,7 @@ func TestCloudStatus(t *testing.T) {
 						Target:     "default",
 					},
 					State: cloudstore.StackState{
-						Status:           stack.OK,
+						Status:           cloudstack.OK,
 						DeploymentStatus: deployment.OK,
 						DriftStatus:      drift.OK,
 					},
@@ -798,6 +801,43 @@ func TestCloudStatus(t *testing.T) {
 	}
 }
 
+func TestCloudStatusRegresionCrash(t *testing.T) {
+	t.Parallel()
+
+	// Terramate crashed due to an oversight in error handling.
+	// This test ensures that the crash does not happen again.
+
+	store, err := cloudstore.LoadDatastore(testserverJSONFile)
+	assert.NoError(t, err)
+	addr := startFakeTMCServer(t, store)
+
+	s := sandbox.New(t)
+	s.BuildTree([]string{
+		"f:terramate.tm:" + Doc(
+			Block("terramate",
+				Block("config",
+					Block("git",
+						Str("default_branch", "master"),
+					),
+				),
+			),
+		).String(),
+	})
+
+	s.Git().SetRemoteURL("origin", "[email protected]:terramate-io/terramate.git")
+
+	env := RemoveEnv(os.Environ(), "CI")
+	env = append(env, "TMC_API_URL=http://"+addr, "CI=")
+
+	cli := NewCLI(t, filepath.Join(s.RootDir()), env...)
+	args := []string{"list", "--status=ok", "--changed"}
+	result := cli.Run(args...)
+	AssertRunResult(t, result, RunExpected{
+		Status:      1,
+		StderrRegex: string(stack.ErrListChanged),
+	})
+}
+
 func paginationTestcase(perPage int) cloudStatusTestcase {
 	const nstacks = 100
 
@@ -814,7 +854,7 @@ func paginationTestcase(perPage int) cloudStatusTestcase {
 				Repository: "github.com/terramate-io/terramate",
 			},
 			State: cloudstore.StackState{
-				Status:           stack.Failed,
+				Status:           cloudstack.Failed,
 				DeploymentStatus: deployment.Failed,
 				DriftStatus:      drift.OK,
 			},