Merge #11456

11456: Implement YAML roundtripping, comment preserving edits r=AaronFriel a=AaronFriel

By caching a copy of the raw bytes of the original loaded document - project, policy project, or stack - on save we:

* Unmarshal the original bytes to a YAML AST
* Recursively edit nodes of the document to match the values of the value we're saving, preserving trivia
* Marshal the modified AST to []byte

Closes #423, as we don't support comments in JSON presently (not using "JSON5")

Closes #5235

Simplifies work done in #10797 and #10437 which used the previous `yamlutil` package to edit AST nodes directly.

# Automation API

Integration with automation API requires that operations either perform a "read-modify-write" in a single operation, or use a side channel to allow subsequent "read" (select stack?) and "write" operations to keep state of the original file.

However I don't expect this to be needed. We don't have a reason to believe automation API users maintain comments in particular stack files, and doing so is already unsupported in automation scenarios. The Pulumi Service does not persist raw config files, thus `pulumi config refresh` and `CreateOrSelectStack` create config files without comments.

n.b.: If you test this, due to the new behavior of this PR, `pulumi config refresh` will persist comments if there is an existing config file. I would expect automation API using local file workspaces to behave similarly. When using these tools to create a config file from the last deployment's definition, the file is created restored without comments.

# Tradeoffs

I believe that this closes #423 in spirit, but doesn't strictly implement round-tripping. There are a couple reasons for this.

## Marshaller interface and round-tripping simplification decisions

We support unmarshaling "simplified" YAML, including allowing:

```yaml
runtime: yaml # this unmarshals to a struct, not a scalar value
config:
  someKey: "someScalar" # likewise
```

These simplifications make it much more difficult, the `Marshal` operation would _also_ need context of decisions made when unmarshaling to ensure a bijection. Otherwise we may save one of these as the other:

```yaml
runtime:
  name: yaml
```

```yaml
runtime: yaml
```

This could be considered a feature, as it enforces a style guide - the decisions implemented in our custom `Marshaller` implementations become the style enforced on save.

That said, I believe the new `Edit` API while not round-trippable, does reach a fixed point after one save through the new API. Once saved with this API, subsequent loads & saves without changes are idempotent.

## Indentation

The YAML library uses a global indentation when marshaling values to YAML, as opposed to indentation being a property of AST nodes. This means we may lose other indentation.

Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>

changelog/pending/20221123--cli-config-new-package--preserve-comments.yaml

@@ -0,0 +1,4 @@
+changes:
+- type: feat
+  scope: cli/config,new,package
+  description: Preserve comments on editing of project and config files.

pkg/cmd/pulumi/new.go

@@ -27,8 +27,6 @@ import (
 	"strings"
 	"unicode"
 
-	"gopkg.in/yaml.v3"
-
 	survey "github.com/AlecAivazis/survey/v2"
 	surveycore "github.com/AlecAivazis/survey/v2/core"
 	"github.com/opentracing/opentracing-go"
@@ -38,7 +36,6 @@ import (
 	"github.com/pulumi/pulumi/pkg/v3/backend/display"
 	"github.com/pulumi/pulumi/pkg/v3/backend/state"
 	"github.com/pulumi/pulumi/pkg/v3/engine"
-	"github.com/pulumi/pulumi/pkg/v3/util/yamlutil"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/apitype"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/diag/colors"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/config"
@@ -257,63 +254,23 @@ func runNew(ctx context.Context, args newArgs) error {
 	fmt.Println()
 
 	// Load the project, update the name & description, remove the template section, and save it.
-	proj, path, err := readProjectWithPath()
-	root := filepath.Dir(path)
+	proj, root, err := readProject()
 	if err != nil {
 		return err
 	}
-
-	if filepath.Ext(path) == ".yaml" {
-		filedata, err := os.ReadFile(path)
-		if err != nil {
-			return err
-		}
-		var workspaceDocument yaml.Node
-		err = yaml.Unmarshal(filedata, &workspaceDocument)
-		if err != nil {
-			return err
-		}
-
-		proj.Name = tokens.PackageName(args.name)
-		err = yamlutil.Insert(&workspaceDocument, "name", args.name)
-		if err != nil {
-			return err
-		}
-		proj.Description = &args.description
-		err = yamlutil.Insert(&workspaceDocument, "description", args.description)
-		if err != nil {
-			return err
-		}
-		proj.Template = nil
-		err = yamlutil.Delete(&workspaceDocument, "template")
-		if err != nil {
-			return err
-		}
-		if proj.Runtime.Name() == "python" {
-			// If the template does give virtualenv use it, else default to "venv"
-			if len(proj.Runtime.Options()) == 0 {
-				proj.Runtime.SetOption("virtualenv", "venv")
-				err = yamlutil.Insert(&workspaceDocument, "runtime", strings.TrimSpace(`
-name: python
-options:
-  virtualenv: venv
-`))
-				if err != nil {
-					return err
-				}
-			}
-		}
-
-		contract.Assert(len(workspaceDocument.Content) == 1)
-		projFile, err := yaml.Marshal(workspaceDocument.Content[0])
-		if err != nil {
-			return err
+	proj.Name = tokens.PackageName(args.name)
+	proj.Description = &args.description
+	proj.Template = nil
+	// Workaround for python, most of our templates don't specify a venv but we want to use one
+	if proj.Runtime.Name() == "python" {
+		// If the template does give virtualenv use it, else default to "venv"
+		if _, has := proj.Runtime.Options()["virtualenv"]; !has {
+			proj.Runtime.SetOption("virtualenv", "venv")
 		}
+	}
 
-		err = os.WriteFile(path, projFile, 0600)
-		if err != nil {
-			return err
-		}
+	if err = workspace.SaveProject(proj); err != nil {
+		return fmt.Errorf("saving project: %w", err)
 	}
 
 	appendFileName := "Pulumi.yaml.append"

pkg/cmd/pulumi/policy_new.go

@@ -19,7 +19,6 @@ import (
 	"errors"
 	"fmt"
 	"os"
-	"path/filepath"
 	"sort"
 	"strings"
 
@@ -28,14 +27,12 @@ import (
 	"github.com/opentracing/opentracing-go"
 	"github.com/pulumi/pulumi/pkg/v3/backend/display"
 	"github.com/pulumi/pulumi/pkg/v3/engine"
-	"github.com/pulumi/pulumi/pkg/v3/util/yamlutil"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/diag/colors"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/cmdutil"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/workspace"
 	"github.com/spf13/cobra"
-	"gopkg.in/yaml.v3"
 )
 
 type newPolicyArgs struct {
@@ -178,41 +175,16 @@ func runNewPolicyPack(ctx context.Context, args newPolicyArgs) error {
 		return err
 	}
 
-	if filepath.Ext(projPath) == ".yaml" {
-		filedata, err := os.ReadFile(projPath)
-		if err != nil {
-			return err
-		}
-		var workspaceDocument yaml.Node
-		err = yaml.Unmarshal(filedata, &workspaceDocument)
-		if err != nil {
-			return err
-		}
-		if proj.Runtime.Name() == "python" {
-			// If the template does give virtualenv use it, else default to "venv"
-			if len(proj.Runtime.Options()) == 0 {
-				proj.Runtime.SetOption("virtualenv", "venv")
-				err = yamlutil.Insert(&workspaceDocument, "runtime", strings.TrimSpace(`
-name: python
-options:
-virtualenv: venv
-`))
-				if err != nil {
-					return err
-				}
-			}
-		}
-
-		contract.Assert(len(workspaceDocument.Content) == 1)
-		projFile, err := yaml.Marshal(workspaceDocument.Content[0])
-		if err != nil {
-			return err
+	// Workaround for python, most of our templates don't specify a venv but we want to use one
+	if proj.Runtime.Name() == "python" {
+		// If the template does give virtualenv use it, else default to "venv"
+		if _, has := proj.Runtime.Options()["virtualenv"]; !has {
+			proj.Runtime.SetOption("virtualenv", "venv")
 		}
+	}
 
-		err = os.WriteFile(projPath, projFile, 0600)
-		if err != nil {
-			return err
-		}
+	if err = proj.Save(projPath); err != nil {
+		return fmt.Errorf("saving project: %w", err)
 	}
 
 	// Install dependencies.