feat(compute/deploy): avoid store conflicts (#1041)

* refactor(compute/deploy): rename test case

* fix(compute/deploy): pass user's prompt input rather than the configured value

* feat(compute/deploy): avoid kv store conflicts

* refactor(compute/deploy): use spinner.Process abstraction

* feat(compute/deploy): avoid config store conflicts

* feat(compute/deploy): avoid secret store conflicts

* refactor(compute/deploy): uppercase KV Store

* doc(release): explain manifest documentation

* tests(compute/deploy): define secret store tests

* refactor(compute/deploy): sort api fields

* fix(compute/deploy): display version number

* refactor: clean up text.Break

* fix(compute/deploy): pass through Store ID

* fix(text): adjust prompt colors

* fix(compute/deploy): avoid config store key conflicts

* refactor(cmd): make output an info message

* doc(compute/deploy): document special case for package comparison

* fix(dictionary): update test output assertion

* fix(compute/deploy): remove unnecessary line break

* feat(compute/deploy): support signal termination

* fix(compute/deploy): notify users of upsert operation

* fix(compute/deploy): use correct value for abs path

* refactor(compute/deploy): move upsert message to main warning

RELEASE.md

@@ -13,5 +13,5 @@
 
 ## Footnotes
 
-1. <a name="note1"></a>We utilize [semantic versioning](https://semver.org/) and only include relevant/significant changes within the CHANGELOG (be sure to document changes to the app config if `config_version` has changed).
+1. <a name="note1"></a>We utilize [semantic versioning](https://semver.org/) and only include relevant/significant changes within the CHANGELOG (be sure to document changes to the app config if `config_version` has changed, and if any breaking interface changes are made to the fastly.toml manifest those should be documented on developer.fastly.com).
 1. <a name="note2"></a>Triggers a [github action](https://github.com/fastly/cli/blob/main/.github/workflows/tag_release.yml) that produces a 'draft' release.

pkg/cmd/flags.go

@@ -235,7 +235,7 @@ func (ac *OptionalAutoClone) Parse(v *fastly.Version, sid string, verbose bool,
 		if verbose {
 			msg := "Service version %d is not editable, so it was automatically cloned because --autoclone is enabled. Now operating on version %d.\n\n"
 			format := fmt.Sprintf(msg, v.Number, version.Number)
-			text.Output(out, format)
+			text.Info(out, format)
 		}
 		return version, nil
 	}

pkg/commands/compute/compute_mocks_test.go

@@ -5,8 +5,9 @@ package compute_test
 // also a mocked HTTP client).
 
 import (
-	"github.com/fastly/cli/pkg/testutil"
 	"github.com/fastly/go-fastly/v8/fastly"
+
+	"github.com/fastly/cli/pkg/testutil"
 )
 
 func getServiceOK(i *fastly.GetServiceInput) (*fastly.Service, error) {
@@ -38,7 +39,7 @@ func createConfigStoreOK(i *fastly.CreateConfigStoreInput) (*fastly.ConfigStore,
 	}, nil
 }
 
-func createConfigStoreItemOK(i *fastly.CreateConfigStoreItemInput) (*fastly.ConfigStoreItem, error) {
+func updateConfigStoreItemOK(i *fastly.UpdateConfigStoreItemInput) (*fastly.ConfigStoreItem, error) {
 	return &fastly.ConfigStoreItem{
 		Key:   i.Key,
 		Value: i.Value,
@@ -103,6 +104,96 @@ func listDomainsOk(i *fastly.ListDomainsInput) ([]*fastly.Domain, error) {
 	}, nil
 }
 
+func listKVStoresOk(i *fastly.ListKVStoresInput) (*fastly.ListKVStoresResponse, error) {
+	return &fastly.ListKVStoresResponse{
+		Data: []fastly.KVStore{
+			{
+				ID:   "123",
+				Name: "store_one",
+			},
+			{
+				ID:   "456",
+				Name: "store_two",
+			},
+		},
+	}, nil
+}
+
+func listKVStoresEmpty(i *fastly.ListKVStoresInput) (*fastly.ListKVStoresResponse, error) {
+	return &fastly.ListKVStoresResponse{}, nil
+}
+
+func getKVStoreOk(i *fastly.GetKVStoreInput) (*fastly.KVStore, error) {
+	return &fastly.KVStore{
+		ID:   "123",
+		Name: "store_one",
+	}, nil
+}
+
+func listSecretStoresOk(i *fastly.ListSecretStoresInput) (*fastly.SecretStores, error) {
+	return &fastly.SecretStores{
+		Data: []fastly.SecretStore{
+			{
+				ID:   "123",
+				Name: "store_one",
+			},
+			{
+				ID:   "456",
+				Name: "store_two",
+			},
+		},
+	}, nil
+}
+
+func listSecretStoresEmpty(i *fastly.ListSecretStoresInput) (*fastly.SecretStores, error) {
+	return &fastly.SecretStores{}, nil
+}
+
+func getSecretStoreOk(i *fastly.GetSecretStoreInput) (*fastly.SecretStore, error) {
+	return &fastly.SecretStore{
+		ID:   "123",
+		Name: "store_one",
+	}, nil
+}
+
+func createSecretStoreOk(i *fastly.CreateSecretStoreInput) (*fastly.SecretStore, error) {
+	return &fastly.SecretStore{
+		ID:   "123",
+		Name: "store_one",
+	}, nil
+}
+
+func createSecretOk(i *fastly.CreateSecretInput) (*fastly.Secret, error) {
+	return &fastly.Secret{
+		Digest: []byte("123"),
+		Name:   "foo",
+	}, nil
+}
+
+func listConfigStoresOk() ([]*fastly.ConfigStore, error) {
+	return []*fastly.ConfigStore{
+		{
+			ID:   "123",
+			Name: "example",
+		},
+		{
+			ID:   "456",
+			Name: "example_two",
+		},
+	}, nil
+}
+
+func listConfigStoresEmpty() ([]*fastly.ConfigStore, error) {
+	return []*fastly.ConfigStore{}, nil
+}
+
+func getConfigStoreOk(i *fastly.GetConfigStoreInput) (*fastly.ConfigStore, error) {
+	return &fastly.ConfigStore{
+		ID:   "123",
+		Name: "example",
+	}, nil
+}
+
 func getServiceDetailsWasm(i *fastly.GetServiceInput) (*fastly.ServiceDetail, error) {
 	return &fastly.ServiceDetail{
 		Type: "wasm",

pkg/commands/compute/deploy.go

@@ -7,8 +7,10 @@ import (
 	"io/fs"
 	"net/http"
 	"os"
+	"os/signal"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/fastly/go-fastly/v8/fastly"
@@ -175,6 +177,10 @@ func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		undoStack.RunIfError(out, err)
 	}(c.Globals.ErrLog)
 
+	signalCh := make(chan os.Signal, 1)
+	signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM)
+	go monitorSignals(signalCh, noExistingService, out, undoStack, spinner)
+
 	var serviceVersion *fastly.Version
 	if noExistingService {
 		serviceID, serviceVersion, err = c.NewService(fnActivateTrial, spinner, in, out)
@@ -188,7 +194,7 @@ func (c *DeployCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		serviceVersion, err = c.ExistingServiceVersion(serviceID, out)
 		if err != nil {
 			if errors.Is(err, ErrPackageUnchanged) {
-				text.Info(out, "Skipping package deployment, local and service version are identical. (service %v, version %v) ", serviceID, serviceVersion)
+				text.Info(out, "Skipping package deployment, local and service version are identical. (service %s, version %d) ", serviceID, serviceVersion.Number)
 				return nil
 			}
 			return err
@@ -521,7 +527,7 @@ func (c *DeployCommand) NewService(fnActivateTrial Activator, spinner text.Spinn
 		}
 
 		text.Break(out)
-		answer, err := text.AskYesNo(out, text.BoldYellow("Create new service: [y/N] "), in)
+		answer, err := text.AskYesNo(out, "Create new service: [y/N] ", in)
 		if err != nil {
 			return serviceID, serviceVersion, err
 		}
@@ -543,7 +549,7 @@ func (c *DeployCommand) NewService(fnActivateTrial Activator, spinner text.Spinn
 	case c.Globals.Flags.AcceptDefaults || c.Globals.Flags.NonInteractive:
 		serviceName = defaultServiceName
 	default:
-		serviceName, err = text.Input(out, text.BoldYellow(fmt.Sprintf("Service name: [%s] ", defaultServiceName)), in)
+		serviceName, err = text.Input(out, text.Prompt(fmt.Sprintf("Service name: [%s] ", defaultServiceName)), in)
 		if err != nil || serviceName == "" {
 			serviceName = defaultServiceName
 		}
@@ -726,6 +732,15 @@ func errLogService(l fsterr.LogInterface, err error, sid string, sv int) {
 
 // CompareLocalRemotePackage compares the local package files hash against the
 // existing service package version and exits early with message if identical.
+//
+// NOTE: We can't avoid the first 'no-changes' upload after the initial deploy.
+// This is because the fastly.toml manifest does actual change after first deploy.
+// When user first deploys, there is no value for service_id.
+// That version of the manifest is inside the package we're checking against.
+// So on the second deploy, even if user has made no changes themselves, we will
+// still upload that package because technically there was a change made by the
+// CLI to add the Service ID. Any subsequent deploys will be aborted because
+// there will be no changes made by the CLI nor the user.
 func (c *DeployCommand) CompareLocalRemotePackage(serviceID string, version int) error {
 	filesHash, err := getFilesHash(c.PackagePath)
 	if err != nil {
@@ -1215,3 +1230,15 @@ func (c *DeployCommand) ExistingServiceVersion(serviceID string, out io.Writer)
 
 	return serviceVersion, nil
 }
+
+func monitorSignals(signalCh chan os.Signal, noExistingService bool, out io.Writer, undoStack *undo.Stack, spinner text.Spinner) {
+	<-signalCh
+	signal.Stop(signalCh)
+	spinner.StopFailMessage("Signal received to interrupt/terminate the Fastly CLI process")
+	spinner.StopFail()
+	text.Important(out, "\n\nThe Fastly CLI process will be terminated after any clean-up tasks have been processed")
+	if noExistingService {
+		undoStack.Unwind(out)
+	}
+	os.Exit(1)
+}