Add error handling and tests for saved plan bookmark load/save (#33268)

internal/cloud/cloudplan/saved_plan.go

@@ -1,13 +1,19 @@
 // Copyright (c) HashiCorp, Inc.
 // SPDX-License-Identifier: MPL-2.0
-
 package cloudplan
 
 import (
 	"encoding/json"
+	"errors"
+	"io"
 	"os"
+	"strings"
 )
 
+var ErrInvalidRemotePlanFormat = errors.New("invalid remote plan format, must be 1")
+var ErrInvalidRunID = errors.New("invalid run ID")
+var ErrInvalidHostname = errors.New("invalid hostname")
+
 type SavedPlanBookmark struct {
 	RemotePlanFormat int    `json:"remote_plan_format"`
 	RunID            string `json:"run_id"`
@@ -16,13 +22,31 @@ type SavedPlanBookmark struct {
 
 func LoadSavedPlanBookmark(filepath string) (SavedPlanBookmark, error) {
 	bookmark := SavedPlanBookmark{}
-	data, err := os.ReadFile(filepath)
 
+	file, err := os.Open(filepath)
+	if err != nil {
+		return bookmark, err
+	}
+	defer file.Close()
+
+	data, err := io.ReadAll(file)
 	if err != nil {
 		return bookmark, err
 	}
 
-	err = json.Unmarshal([]byte(data), &bookmark)
+	err = json.Unmarshal(data, &bookmark)
+	if err != nil {
+		return bookmark, err
+	}
+
+	if bookmark.RemotePlanFormat != 1 {
+		return bookmark, ErrInvalidRemotePlanFormat
+	} else if bookmark.Hostname == "" {
+		return bookmark, ErrInvalidHostname
+	} else if bookmark.RunID == "" || !strings.HasPrefix(bookmark.RunID, "run-") {
+		return bookmark, ErrInvalidRunID
+	}
+
 	return bookmark, err
 }
 

internal/cloud/cloudplan/saved_plan_test.go

@@ -4,6 +4,8 @@
 package cloudplan
 
 import (
+	"errors"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -18,7 +20,8 @@ func TestCloud_loadBasic(t *testing.T) {
 		Hostname:         "app.terraform.io",
 	}
 
-	result, err := LoadSavedPlanBookmark("./testdata/plan-bookmark/bookmark.json")
+	file := "./testdata/plan-bookmark/bookmark.json"
+	result, err := LoadSavedPlanBookmark(file)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -28,15 +31,69 @@ func TestCloud_loadBasic(t *testing.T) {
 	}
 }
 
-func TestCloud_saveBasic(t *testing.T) {
-	tmp := t.TempDir()
-	bookmarkPath := filepath.Join(tmp, "saved-bookmark.json")
+func TestCloud_loadCheckRunID(t *testing.T) {
+	// Run ID must never be empty
+	file := "./testdata/plan-bookmark/empty_run_id.json"
+	_, err := LoadSavedPlanBookmark(file)
+	if !errors.Is(err, ErrInvalidRunID) {
+		t.Fatalf("expected %s but got %s", ErrInvalidRunID, err)
+	}
+}
+
+func TestCloud_loadCheckHostname(t *testing.T) {
+	// Hostname must never be empty
+	file := "./testdata/plan-bookmark/empty_hostname.json"
+	_, err := LoadSavedPlanBookmark(file)
+	if !errors.Is(err, ErrInvalidHostname) {
+		t.Fatalf("expected %s but got %s", ErrInvalidHostname, err)
+	}
+}
+
+func TestCloud_loadCheckVersionNumberBasic(t *testing.T) {
+	// remote_plan_format must be set to 1
+	// remote_plan_format and format version number are used interchangeably
+	file := "./testdata/plan-bookmark/invalid_version.json"
+	_, err := LoadSavedPlanBookmark(file)
+	if !errors.Is(err, ErrInvalidRemotePlanFormat) {
+		t.Fatalf("expected %s but got %s", ErrInvalidRemotePlanFormat, err)
+	}
+}
+
+func TestCloud_saveWhenFileExistsBasic(t *testing.T) {
+	tmpDir := t.TempDir()
+	tmpFile, err := os.Create(filepath.Join(tmpDir, "saved-bookmark.json"))
+	if err != nil {
+		t.Fatal("File could not be created.", err)
+	}
+	defer tmpFile.Close()
+
+	// verify the created path exists
+	// os.Stat() wants path to file
+	_, error := os.Stat(tmpFile.Name())
+	if error != nil {
+		t.Fatal("Path to file does not exist.", error)
+	} else {
+		b := &SavedPlanBookmark{
+			RemotePlanFormat: 1,
+			RunID:            "run-GXfuHMkbyHccAGUg",
+			Hostname:         "app.terraform.io",
+		}
+		err := b.Save(tmpFile.Name())
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+}
 
+func TestCloud_saveWhenFileDoesNotExistBasic(t *testing.T) {
+	tmpDir := t.TempDir()
 	b := &SavedPlanBookmark{
 		RemotePlanFormat: 1,
 		RunID:            "run-GXfuHMkbyHccAGUg",
 		Hostname:         "app.terraform.io",
 	}
-
-	b.Save(bookmarkPath)
+	err := b.Save(filepath.Join(tmpDir, "create-new-file.txt"))
+	if err != nil {
+		t.Fatal(err)
+	}
 }

internal/cloud/cloudplan/testdata/plan-bookmark/empty_hostname.json

@@ -0,0 +1,5 @@
+{
+  "remote_plan_format": 1,
+  "run_id": "run-GXfuHMkbyHccAGUg",
+  "hostname": ""
+}

internal/cloud/cloudplan/testdata/plan-bookmark/empty_run_id.json

@@ -0,0 +1,5 @@
+{
+  "remote_plan_format": 1,
+  "run_id": "",
+  "hostname": "app.terraform.io"
+}

internal/cloud/cloudplan/testdata/plan-bookmark/invalid_version.json

@@ -0,0 +1,5 @@
+{
+  "remote_plan_format": 11,
+  "run_id": "run-GXfuHMkbyHccAGUg",
+  "hostname": "app.terraform.io"
+}

internal/cloud/testdata/plan-bookmark/bookmark.json

@@ -0,0 +1,5 @@
+{
+  "remote_plan_format": 1,
+  "run_id": "run-GXfuHMkbyHccAGUg",
+  "hostname": "app.terraform.io"
+}