Ensure [local_server.pushpin] section persists during fastly.toml manifest file update (#1535)

### Change summary

This PR updates `MarshalTOML()` for `manifest/File` such that the
`[local_server.pushpin]` section and its fields persist when the file is
written.

* The `pushpin` section as well as its individual fields are now
optional, such that it will be able to persist the file back as it was
before starting.
* `[local_server.pushpin]` has been added to
`fastly-viceroy-update.toml`, which is used by the test
`TestManifestPersistsLocalServerSection`, ensuring that the section is
written out and reads back in to the same value.

This fixes #1532.

* [x] Have you followed the guidelines in our Contributing document?
* [x] Have you checked to ensure there aren't other open [Pull
Requests](https://github.com/fastly/cli/pulls) for the same
update/change?

### Changes to Core Features:

* [x] Have you successfully run tests with your changes locally?

### User Impact

* [x] What is the user impact of this change?

Users can now safely run CLI commands that modify fastly.toml without
losing their `[local_server.pushpin]` section.

### Are there any considerations that need to be addressed for release?

Since this overwrite happens right after initializing an app from the
starter kit, this would be nice to get out asap.
Would be great if this can be released as a quick patch release such as
11.5.1.

---------

Co-authored-by: Richard Carillo <77027245+rcaril@users.noreply.github.com>

Author: harmony7

CHANGELOG.md

@@ -10,6 +10,8 @@
 
 ### Bug fixes:
 
+- fix(manifest): Ensure pushpin section is persisted during manifest file update ([#1535](https://github.com/fastly/cli/pull/1535))
+
 ### Dependencies:
 - build(deps): `github.com/ulikunitz/xz` from 0.5.12 to 0.5.13 ([#1524](https://github.com/fastly/cli/pull/1524))
 - build(deps): `github.com/stretchr/testify` from 1.10.0 to 1.11.0 ([#1527](https://github.com/fastly/cli/pull/1527))

pkg/commands/compute/serve.go

@@ -215,7 +215,10 @@ func (c *ServeCommand) Exec(in io.Reader, out io.Writer) (err error) {
 		return err
 	}
 
-	enablePushpin := c.enablePushpin || c.Globals.Manifest.File.LocalServer.Pushpin.EnablePushpin
+	enablePushpin := c.enablePushpin ||
+		(c.Globals.Manifest.File.LocalServer.Pushpin != nil &&
+			c.Globals.Manifest.File.LocalServer.Pushpin.EnablePushpin != nil &&
+			*c.Globals.Manifest.File.LocalServer.Pushpin.EnablePushpin)
 	var pushpinCtx pushpinContext
 	if enablePushpin {
 		// The function checks for nil, so the semgrep warning is falsely triggered
@@ -607,12 +610,15 @@ func (c *ServeCommand) GetPushpinProxyPort(out io.Writer) (uint16, error) {
 		return pushpinProxyPort, nil
 	}
 
-	pushpinProxyPort = c.Globals.Manifest.File.LocalServer.Pushpin.PushpinProxyPort
-	if pushpinProxyPort != 0 {
-		if c.Globals.Verbose() {
-			text.Info(out, "Using Pushpin proxy port via `local_server.pushpin.proxy_port` setting: %d", pushpinProxyPort)
+	if c.Globals.Manifest.File.LocalServer.Pushpin != nil &&
+		c.Globals.Manifest.File.LocalServer.Pushpin.PushpinProxyPort != nil {
+		pushpinProxyPort = *c.Globals.Manifest.File.LocalServer.Pushpin.PushpinProxyPort
+		if pushpinProxyPort != 0 {
+			if c.Globals.Verbose() {
+				text.Info(out, "Using Pushpin proxy port via `local_server.pushpin.proxy_port` setting: %d", pushpinProxyPort)
+			}
+			return pushpinProxyPort, nil
 		}
-		return pushpinProxyPort, nil
 	}
 
 	pushpinProxyPort = 7677
@@ -647,12 +653,15 @@ func (c *ServeCommand) GetPushpinPublishPort(out io.Writer) (uint16, error) {
 		return pushpinPublishPort, nil
 	}
 
-	pushpinPublishPort = c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPublishPort
-	if pushpinPublishPort != 0 {
-		if c.Globals.Verbose() {
-			text.Info(out, "Using Pushpin publish handler port via `local_server.pushpin.publish_port` setting: %d", pushpinPublishPort)
+	if c.Globals.Manifest.File.LocalServer.Pushpin != nil &&
+		c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPublishPort != nil {
+		pushpinPublishPort = *c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPublishPort
+		if pushpinPublishPort != 0 {
+			if c.Globals.Verbose() {
+				text.Info(out, "Using Pushpin publish handler port via `local_server.pushpin.publish_port` setting: %d", pushpinPublishPort)
+			}
+			return pushpinPublishPort, nil
 		}
-		return pushpinPublishPort, nil
 	}
 
 	pushpinPublishPort = 5561
@@ -676,12 +685,15 @@ func (c *ServeCommand) GetPushpinRunner(out io.Writer) (bin string, err error) {
 		return filepath.Abs(pushpinRunnerBinPath)
 	}
 
-	pushpinRunnerBinPath = c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPath
-	if pushpinRunnerBinPath != "" {
-		if c.Globals.Verbose() {
-			text.Info(out, "Using user provided install of Pushpin runner via `local_server.pushpin.pushpin_path` setting: %s", pushpinRunnerBinPath)
+	if c.Globals.Manifest.File.LocalServer.Pushpin != nil &&
+		c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPath != nil {
+		pushpinRunnerBinPath = *c.Globals.Manifest.File.LocalServer.Pushpin.PushpinPath
+		if pushpinRunnerBinPath != "" {
+			if c.Globals.Verbose() {
+				text.Info(out, "Using user provided install of Pushpin runner via `local_server.pushpin.pushpin_path` setting: %s", pushpinRunnerBinPath)
+			}
+			return filepath.Abs(pushpinRunnerBinPath)
 		}
-		return filepath.Abs(pushpinRunnerBinPath)
 	}
 
 	if c.Globals.Verbose() {

pkg/manifest/file.go

@@ -92,6 +92,23 @@ func (f *File) MarshalTOML() ([]byte, error) {
 		localServer["kv_stores"] = kvStores
 	}
 
+	if f.LocalServer.Pushpin != nil {
+		pushpin := make(map[string]any)
+		if f.LocalServer.Pushpin.EnablePushpin != nil {
+			pushpin["enable"] = *f.LocalServer.Pushpin.EnablePushpin
+		}
+		if f.LocalServer.Pushpin.PushpinPath != nil {
+			pushpin["pushpin_path"] = *f.LocalServer.Pushpin.PushpinPath
+		}
+		if f.LocalServer.Pushpin.PushpinProxyPort != nil {
+			pushpin["proxy_port"] = *f.LocalServer.Pushpin.PushpinProxyPort
+		}
+		if f.LocalServer.Pushpin.PushpinPublishPort != nil {
+			pushpin["publish_port"] = *f.LocalServer.Pushpin.PushpinPublishPort
+		}
+		localServer["pushpin"] = pushpin
+	}
+
 	if f.LocalServer.SecretStores != nil {
 		secretStores := make(map[string]any)
 		for key, entry := range f.LocalServer.SecretStores {

pkg/manifest/local_server.go

@@ -13,7 +13,7 @@ type LocalServer struct {
 	ConfigStores   map[string]LocalConfigStore `toml:"config_stores,omitempty"`
 	KVStores       LocalKVStoreMap             `toml:"kv_stores,omitempty"`
 	SecretStores   LocalSecretStoreMap         `toml:"secret_stores,omitempty"`
-	Pushpin        LocalPushpinMap             `toml:"pushpin,omitempty"`
+	Pushpin        *LocalPushpinMap            `toml:"pushpin,omitempty"`
 	ViceroyVersion string                      `toml:"viceroy_version,omitempty"`
 }
 
@@ -196,10 +196,10 @@ func (m *LocalSecretStoreMap) UnmarshalTOML(v any) error {
 // LocalPushpinMap represents configuration of a local instance of Pushpin,
 // used for local experimentation and testing of handoff_fanout.
 type LocalPushpinMap struct {
-	EnablePushpin      bool   `toml:"enable,omitempty"`
-	PushpinPath        string `toml:"pushpin_path,omitempty"`
-	PushpinProxyPort   uint16 `toml:"proxy_port,omitempty"`
-	PushpinPublishPort uint16 `toml:"publish_port,omitempty"`
+	EnablePushpin      *bool   `toml:"enable,omitempty"`
+	PushpinPath        *string `toml:"pushpin_path,omitempty"`
+	PushpinProxyPort   *uint16 `toml:"proxy_port,omitempty"`
+	PushpinPublishPort *uint16 `toml:"publish_port,omitempty"`
 }
 
 func decodeTOMLMap(m map[string]any, out any) error {

pkg/manifest/testdata/fastly-viceroy-update.toml

@@ -51,6 +51,12 @@ metadata = "This is some metadata"
 key = "second"
 file = "strings.json"
 
+[local_server.pushpin]
+enable = true
+pushpin_path = "path/to/pushpin"
+proxy_port = 7777
+publish_port = 6666
+
 [local_server.secret_stores]
 store_one = [
   { key = "first", data = "This is some secret data" },