From dd851a80b08418ca47fed0e8cdf5643184e9b672 Mon Sep 17 00:00:00 2001 From: Scott Walkinshaw Date: Sun, 18 Oct 2026 12:13:33 -0400 Subject: [PATCH 1/2] Add yamledit package and preserve wordpress_sites.yml formatting Adds trellis/yamledit, a small wrapper around go.yaml.in/yaml/v3's Node API for making targeted edits to YAML files while preserving comments, key order, quoting style, and blank lines (which yaml.v3 normally drops). `trellis new` now uses it to update wordpress_sites.yml in place instead of round-tripping it through the Site struct with yaml.v2. Previously this stripped Trellis' inline comments and silently dropped any site options the struct doesn't model (such as `xmlrpc`). Co-Authored-By: Claude Opus 5.5 --- cmd/new.go | 20 +- go.mod | 1 + trellis/config.go | 50 ++++- trellis/config_test.go | 77 ++++++- trellis/yamledit/yamledit.go | 326 ++++++++++++++++++++++++++++++ trellis/yamledit/yamledit_test.go | 283 ++++++++++++++++++++++++++ 6 files changed, 740 insertions(+), 17 deletions(-) create mode 100644 trellis/yamledit/yamledit.go create mode 100644 trellis/yamledit/yamledit_test.go diff --git a/cmd/new.go b/cmd/new.go index ce799ab8..2219a2cb 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -159,13 +159,13 @@ func (c *NewCommand) Run(args []string) int { } // Update default configs - for env, config := range c.trellis.Environments { - c.trellis.UpdateDefaultConfig(config, c.name, c.host, env) - _ = c.trellis.WriteYamlFile( - config, - filepath.Join("group_vars", env, "wordpress_sites.yml"), - c.YamlHeader("https://roots.io/trellis/docs/wordpress-sites/"), - ) + for env := range c.trellis.Environments { + sitesPath := filepath.Join("group_vars", env, "wordpress_sites.yml") + if err := c.trellis.UpdateDefaultSiteFile(sitesPath, c.name, c.host, env, c.createdByComment()); err != nil { + c.UI.Error(fmt.Sprintf("Error updating %s: %s", sitesPath, err)) + c.UI.Error("Your project may not work correctly.") + c.UI.Error("This is probably a trellis-cli bug. Please open an issue at: https://github.com/roots/trellis-cli") + } stringGenerator := trellis.RandomStringGenerator{Length: 64} vault := c.trellis.GenerateVaultConfig(c.name, env, &stringGenerator) @@ -261,9 +261,11 @@ Options: } func (c *NewCommand) YamlHeader(doc string) string { - const header = "# Created by trellis-cli v%s\n# Documentation: %s\n\n" + return fmt.Sprintf("%s\n# Documentation: %s\n\n", c.createdByComment(), doc) +} - return fmt.Sprintf(header, c.CliVersion, doc) +func (c *NewCommand) createdByComment() string { + return fmt.Sprintf("# Created by trellis-cli v%s", c.CliVersion) } func askDomain(ui cli.Ui, path string) (host string, err error) { diff --git a/go.mod b/go.mod index 2e30899d..49c1834c 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/posener/complete v1.2.3 github.com/theckman/yacspin v0.13.12 github.com/weppos/publicsuffix-go v0.50.3 + go.yaml.in/yaml/v3 v3.0.5 golang.org/x/crypto v0.58.0 golang.org/x/oauth2 v0.37.0 gopkg.in/alessio/shellescape.v1 v1.0.0-20170105083845-52074bc9df61 diff --git a/trellis/config.go b/trellis/config.go index 30cf5cf8..e1a28455 100644 --- a/trellis/config.go +++ b/trellis/config.go @@ -6,8 +6,10 @@ import ( "os" "path/filepath" "sort" + "strings" "github.com/roots/trellis-cli/dns" + "github.com/roots/trellis-cli/trellis/yamledit" "github.com/weppos/publicsuffix-go/publicsuffix" "gopkg.in/yaml.v2" ) @@ -107,14 +109,52 @@ func (t *Trellis) HostsFromDomain(domain string, env string) (canonical *publics } } -func (t *Trellis) UpdateDefaultConfig(config *Config, name string, host string, env string) { - config.WordPressSites[name] = config.WordPressSites[DefaultSiteName] +// UpdateDefaultSiteFile renames the default site in a wordpress_sites.yml file +// (relative to the project) and applies the generated hosts and environment +// defaults. Comments and site options not modeled by Site are preserved. +func (t *Trellis) UpdateDefaultSiteFile(path string, name string, host string, env string, header string) error { + path = filepath.Join(t.Path, path) - if name != DefaultSiteName { - delete(config.WordPressSites, DefaultSiteName) + doc, err := yamledit.Load(path) + if err != nil { + return err + } + + if err := doc.RenameKey(name, "wordpress_sites", DefaultSiteName); err != nil { + return err + } + + site := &Site{} + if err := doc.Decode(site, "wordpress_sites", name); err != nil { + return err + } + + t.GenerateSite(site, host, env) + + if err := doc.Set(site.SiteHosts, "wordpress_sites", name, "site_hosts"); err != nil { + return err + } + + optional := []struct{ key, value string }{ + {"admin_email", site.AdminEmail}, + {"branch", site.Branch}, + {"repo", site.Repo}, + {"repo_subtree_path", site.RepoSubtreePath}, + } + + for _, option := range optional { + if option.value == "" { + doc.Delete("wordpress_sites", name, option.key) + } else if err := doc.Set(option.value, "wordpress_sites", name, option.key); err != nil { + return err + } + } + + if header != "" { + doc.SetHeadComment(strings.TrimRight(header+"\n"+doc.HeadComment(), "\n")) } - t.GenerateSite(config.WordPressSites[name], host, env) + return doc.Save(path) } func (s *Site) SslEnabled() bool { diff --git a/trellis/config_test.go b/trellis/config_test.go index 3e5590ae..91bdeba2 100644 --- a/trellis/config_test.go +++ b/trellis/config_test.go @@ -2,6 +2,8 @@ package trellis import ( "fmt" + "os" + "path/filepath" "reflect" "strings" "testing" @@ -10,7 +12,7 @@ import ( "gopkg.in/yaml.v2" ) -func TestUpdateDefaultConfig(t *testing.T) { +func TestUpdateDefaultSiteFile(t *testing.T) { trellis := &Trellis{} cases := []struct { @@ -216,9 +218,14 @@ wordpress_sites: } for _, tc := range cases { - config := trellis.ParseConfig(fmt.Sprintf("testdata/trellis/group_vars/%s/wordpress_sites.yml", tc.env)) - trellis.UpdateDefaultConfig(config, tc.siteName, tc.host, tc.env) + trellis.Path = t.TempDir() + copySitesFixture(t, trellis.Path, tc.env) + if err := trellis.UpdateDefaultSiteFile("wordpress_sites.yml", tc.siteName, tc.host, tc.env, ""); err != nil { + t.Fatalf("%s => unexpected error: %v", tc.name, err) + } + + config := trellis.ParseConfig(filepath.Join(trellis.Path, "wordpress_sites.yml")) configYaml, _ := yaml.Marshal(config) expected := strings.TrimSpace(tc.expectedYaml) output := strings.TrimSpace(string(configYaml)) @@ -229,6 +236,70 @@ wordpress_sites: } } +func TestUpdateDefaultSiteFilePreservesFormatting(t *testing.T) { + trellis := &Trellis{Path: t.TempDir()} + copySitesFixture(t, trellis.Path, "production") + + if err := trellis.UpdateDefaultSiteFile("wordpress_sites.yml", "mysite.com", "mysite.com", "production", "# Created by trellis-cli v1.0.0"); err != nil { + t.Fatal(err) + } + + output, _ := os.ReadFile(filepath.Join(trellis.Path, "wordpress_sites.yml")) + + expected := `# Created by trellis-cli v1.0.0 +# Documentation: https://roots.io/trellis/docs/remote-server-setup/ +# ` + "`wordpress_sites`" + ` options: https://roots.io/trellis/docs/wordpress-sites +# Define accompanying passwords/secrets in group_vars/production/vault.yml + +wordpress_sites: + mysite.com: + site_hosts: + - canonical: mysite.com + redirects: + - www.mysite.com + local_path: ../site # path targeting local Bedrock site directory (relative to Ansible root) + repo: git@github.com:example/example.com.git # replace with your Git repo URL + repo_subtree_path: site # relative path to your Bedrock/WP directory in your repo + branch: master + multisite: + enabled: false + ssl: + enabled: false + provider: letsencrypt + cache: + enabled: false +` + + if string(output) != expected { + t.Errorf("expected:\n%s\ngot:\n%s", expected, output) + } +} + +func TestUpdateDefaultSiteFileMissingDefaultSite(t *testing.T) { + trellis := &Trellis{Path: t.TempDir()} + + if err := os.WriteFile(filepath.Join(trellis.Path, "wordpress_sites.yml"), []byte("wordpress_sites:\n other.com: {}\n"), 0644); err != nil { + t.Fatal(err) + } + + if err := trellis.UpdateDefaultSiteFile("wordpress_sites.yml", "mysite.com", "mysite.com", "production", ""); err == nil { + t.Error("expected error when default site is missing") + } +} + +func copySitesFixture(t *testing.T, dir string, env string) { + t.Helper() + + data, err := os.ReadFile(fmt.Sprintf("testdata/trellis/group_vars/%s/wordpress_sites.yml", env)) + if err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(dir, "wordpress_sites.yml"), data, 0644); err != nil { + t.Fatal(err) + } +} + func TestSslProvider(t *testing.T) { cases := []struct { name string diff --git a/trellis/yamledit/yamledit.go b/trellis/yamledit/yamledit.go new file mode 100644 index 00000000..d9834386 --- /dev/null +++ b/trellis/yamledit/yamledit.go @@ -0,0 +1,326 @@ +// Package yamledit makes targeted edits to YAML files while preserving +// comments, key order, and blank lines that a struct round-trip would lose. +package yamledit + +import ( + "bytes" + "errors" + "fmt" + "os" + "slices" + "strings" + + "go.yaml.in/yaml/v3" +) + +const indent = 2 + +type Document struct { + root *yaml.Node +} + +func Parse(data []byte) (*Document, error) { + var root yaml.Node + + if err := yaml.Unmarshal(data, &root); err != nil { + return nil, err + } + + if root.Kind == 0 { + root.Kind = yaml.DocumentNode + } + + if len(root.Content) == 0 { + root.Content = []*yaml.Node{{Kind: yaml.MappingNode, Tag: "!!map"}} + } + + preserveBlankLines(&root, strings.Split(string(data), "\n")) + + return &Document{root: &root}, nil +} + +func Load(path string) (*Document, error) { + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + + return Parse(data) +} + +func (d *Document) Bytes() ([]byte, error) { + var buf bytes.Buffer + + encoder := yaml.NewEncoder(&buf) + encoder.SetIndent(indent) + + if err := encoder.Encode(d.root); err != nil { + return nil, err + } + + if err := encoder.Close(); err != nil { + return nil, err + } + + return stripBlankLineIndentation(buf.Bytes()), nil +} + +// Save writes the document to path, keeping the existing file's permissions. +func (d *Document) Save(path string) error { + data, err := d.Bytes() + if err != nil { + return err + } + + perm := os.FileMode(0644) + if info, err := os.Stat(path); err == nil { + perm = info.Mode().Perm() + } + + return os.WriteFile(path, data, perm) +} + +// SetHeadComment replaces the comment at the top of the document. +func (d *Document) SetHeadComment(comment string) { + d.root.HeadComment = comment +} + +func (d *Document) HeadComment() string { + return d.root.HeadComment +} + +// Lookup returns the value node at the given key path, or nil if it doesn't exist. +func (d *Document) Lookup(path ...string) *yaml.Node { + _, value, err := d.find(path) + if err != nil { + return nil + } + + return value +} + +// Decode decodes the value at the given key path into v. +func (d *Document) Decode(v any, path ...string) error { + node := d.Lookup(path...) + if node == nil { + return fmt.Errorf("key not found: %s", formatPath(path)) + } + + return node.Decode(v) +} + +// Set encodes value and stores it at the given key path, creating any missing +// mappings along the way. Comments on an existing value are kept, as is the +// quoting style when replacing a quoted scalar with another scalar. +func (d *Document) Set(value any, path ...string) error { + if len(path) == 0 { + return errors.New("path must not be empty") + } + + var newNode yaml.Node + if err := newNode.Encode(value); err != nil { + return err + } + + mapping := d.body() + for i, key := range path[:len(path)-1] { + _, next := findKey(mapping, key) + + if next == nil { + next = &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + appendKey(mapping, key, next) + } else if next.Kind != yaml.MappingNode { + return fmt.Errorf("%s is not a mapping", formatPath(path[:i+1])) + } + + mapping = next + } + + key := path[len(path)-1] + _, existing := findKey(mapping, key) + + if existing == nil { + appendKey(mapping, key, &newNode) + return nil + } + + if existing.Kind == yaml.ScalarNode && newNode.Kind == yaml.ScalarNode && isQuoted(existing.Style) { + newNode.Style = existing.Style + } + + newNode.HeadComment = existing.HeadComment + newNode.LineComment = existing.LineComment + newNode.FootComment = existing.FootComment + *existing = newNode + + return nil +} + +// Delete removes the key at the given path. It returns false if the key didn't exist. +func (d *Document) Delete(path ...string) bool { + if len(path) == 0 { + return false + } + + parent := d.body() + if len(path) > 1 { + parent = d.Lookup(path[:len(path)-1]...) + } + + if parent == nil || parent.Kind != yaml.MappingNode { + return false + } + + for i := 0; i < len(parent.Content)-1; i += 2 { + if parent.Content[i].Value == path[len(path)-1] { + parent.Content = append(parent.Content[:i], parent.Content[i+2:]...) + return true + } + } + + return false +} + +// RenameKey renames the key at the given path, keeping its position and comments. +func (d *Document) RenameKey(newKey string, path ...string) error { + key, _, err := d.find(path) + if err != nil { + return err + } + + if key.Value == newKey { + return nil + } + + newPath := slices.Concat(path[:len(path)-1], []string{newKey}) + if d.Lookup(newPath...) != nil { + return fmt.Errorf("key already exists: %s", formatPath(newPath)) + } + + key.Value = newKey + + return nil +} + +func (d *Document) body() *yaml.Node { + return d.root.Content[0] +} + +func (d *Document) find(path []string) (key *yaml.Node, value *yaml.Node, err error) { + if len(path) == 0 { + return nil, nil, errors.New("path must not be empty") + } + + value = d.body() + + for i, k := range path { + if value.Kind != yaml.MappingNode { + return nil, nil, fmt.Errorf("%s is not a mapping", formatPath(path[:i])) + } + + key, value = findKey(value, k) + if key == nil { + return nil, nil, fmt.Errorf("key not found: %s", formatPath(path[:i+1])) + } + } + + return key, value, nil +} + +func findKey(mapping *yaml.Node, key string) (*yaml.Node, *yaml.Node) { + for i := 0; i < len(mapping.Content)-1; i += 2 { + if mapping.Content[i].Value == key { + return mapping.Content[i], mapping.Content[i+1] + } + } + + return nil, nil +} + +func appendKey(mapping *yaml.Node, key string, value *yaml.Node) { + keyNode := &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: key} + mapping.Content = append(mapping.Content, keyNode, value) +} + +func isQuoted(style yaml.Style) bool { + return style&(yaml.DoubleQuotedStyle|yaml.SingleQuotedStyle) != 0 +} + +func formatPath(path []string) string { + return strings.Join(path, ".") +} + +// preserveBlankLines works around yaml.v3 dropping blank lines between +// entries: the encoder emits a blank line for each leading newline in a head +// comment, so blank lines found in the source above a mapping key or sequence +// item are encoded into its head comment. +func preserveBlankLines(root *yaml.Node, lines []string) { + handled := map[int]bool{} + + // The encoder already keeps the blank line after a document head comment. + if root.HeadComment != "" && len(root.Content) > 0 { + handled[root.Content[0].Line] = true + } + + var walk func(node *yaml.Node) + walk = func(node *yaml.Node) { + switch node.Kind { + case yaml.MappingNode: + for i := 0; i < len(node.Content)-1; i += 2 { + addBlankLines(node.Content[i], lines, handled) + walk(node.Content[i+1]) + } + case yaml.SequenceNode: + for _, item := range node.Content { + addBlankLines(item, lines, handled) + walk(item) + } + default: + for _, child := range node.Content { + walk(child) + } + } + } + + walk(root) +} + +// Blank lines restored inside nested blocks are emitted with the block's +// indentation. The encoder never uses block scalar style for content with +// trailing spaces, so whitespace-only lines are always safe to empty. +func stripBlankLineIndentation(data []byte) []byte { + lines := bytes.Split(data, []byte("\n")) + + for i, line := range lines { + if len(bytes.TrimLeft(line, " ")) == 0 { + lines[i] = nil + } + } + + return bytes.Join(lines, []byte("\n")) +} + +func addBlankLines(node *yaml.Node, lines []string, handled map[int]bool) { + if node.Line == 0 || handled[node.Line] { + return + } + + handled[node.Line] = true + + start := node.Line + if node.HeadComment != "" { + start -= strings.Count(node.HeadComment, "\n") + 1 + } + + blanks := 0 + for i := start - 2; i >= 0 && strings.TrimSpace(lines[i]) == ""; i-- { + blanks++ + } + + // Blank lines at the very start of the file aren't meaningful. + if blanks == 0 || blanks == start-1 { + return + } + + node.HeadComment = strings.Repeat("\n", blanks) + node.HeadComment +} diff --git a/trellis/yamledit/yamledit_test.go b/trellis/yamledit/yamledit_test.go new file mode 100644 index 00000000..a4a852e2 --- /dev/null +++ b/trellis/yamledit/yamledit_test.go @@ -0,0 +1,283 @@ +package yamledit + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRoundTripTrellisFiles(t *testing.T) { + files, _ := filepath.Glob("../testdata/trellis/group_vars/*/*.yml") + if len(files) == 0 { + t.Fatal("no fixture files found") + } + + for _, file := range files { + t.Run(file, func(t *testing.T) { + data, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + + if strings.HasPrefix(string(data), "$ANSIBLE_VAULT") { + t.Skip("encrypted vault file") + } + + assertRoundTrip(t, string(data)) + }) + } +} + +func TestRoundTrip(t *testing.T) { + cases := []struct { + name string + yaml string + }{ + {"blank_lines_between_keys", "a: 1\n\nb: 2\n\n\nc: 3\n"}, + {"blank_line_before_comment", "a: 1\n\n# comment\nb: 2\n"}, + {"doc_comment_without_blank_line", "# comment\na: 1\n\nb: 2\n"}, + {"doc_comment_with_blank_line", "# comment\n\na: 1\n\nb: 2\n"}, + {"nested_blank_lines", "a:\n b: 1\n\n c: 2\n"}, + {"sequence_items", "a:\n - name: x\n value: 1\n\n - name: y\n value: 2\n"}, + {"literal_block_with_blank_line", "a: |\n line 1\n\n line 2\nb: 2\n"}, + {"inline_comments", "a: 1 # one\nb: \"two\" # two\n"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assertRoundTrip(t, tc.yaml) + }) + } +} + +func TestParseEmpty(t *testing.T) { + for _, input := range []string{"", "# only a comment\n"} { + doc, err := Parse([]byte(input)) + if err != nil { + t.Fatal(err) + } + + if err := doc.Set("value", "key"); err != nil { + t.Errorf("Set on %q: %v", input, err) + } + } +} + +func TestLookupAndDecode(t *testing.T) { + doc := mustParse(t, "sites:\n example.com:\n enabled: true\n") + + if node := doc.Lookup("sites", "example.com", "enabled"); node == nil || node.Value != "true" { + t.Errorf("Lookup returned %v", node) + } + + if node := doc.Lookup("sites", "missing"); node != nil { + t.Errorf("expected nil for missing key, got %v", node) + } + + if node := doc.Lookup("sites", "example.com", "enabled", "deeper"); node != nil { + t.Errorf("expected nil when traversing a scalar, got %v", node) + } + + var site struct { + Enabled bool `yaml:"enabled"` + } + + if err := doc.Decode(&site, "sites", "example.com"); err != nil { + t.Fatal(err) + } + + if !site.Enabled { + t.Error("expected decoded site to be enabled") + } + + if err := doc.Decode(&site, "sites", "missing"); err == nil { + t.Error("expected error decoding missing key") + } +} + +func TestSet(t *testing.T) { + cases := []struct { + name string + input string + path []string + value any + expected string + }{ + { + "keeps_double_quotes_and_comments", + "# head\nkey: \"generateme\" # line\nother: 1\n", + []string{"key"}, + "secret", + "# head\nkey: \"secret\" # line\nother: 1\n", + }, + { + "keeps_single_quotes", + "key: 'old'\n", + []string{"key"}, + "new", + "key: 'new'\n", + }, + { + "plain_scalar_quotes_when_needed", + "key: old\n", + []string{"key"}, + "true", + "key: \"true\"\n", + }, + { + "replaces_sequence", + "hosts:\n - a\n - b\nnext: 1\n", + []string{"hosts"}, + []map[string]any{{"canonical": "c"}}, + "hosts:\n - canonical: c\nnext: 1\n", + }, + { + "appends_missing_key", + "a: 1\n", + []string{"b"}, + 2, + "a: 1\nb: 2\n", + }, + { + "creates_missing_mappings", + "sites:\n one:\n x: 1\n", + []string{"sites", "two", "env", "x"}, + "z", + "sites:\n one:\n x: 1\n two:\n env:\n x: z\n", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + doc := mustParse(t, tc.input) + + if err := doc.Set(tc.value, tc.path...); err != nil { + t.Fatal(err) + } + + assertOutput(t, doc, tc.expected) + }) + } +} + +func TestSetErrors(t *testing.T) { + doc := mustParse(t, "a: 1\n") + + if err := doc.Set("x"); err == nil { + t.Error("expected error for empty path") + } + + if err := doc.Set("x", "a", "b"); err == nil { + t.Error("expected error when traversing a scalar") + } +} + +func TestDelete(t *testing.T) { + doc := mustParse(t, "a: 1\nsite:\n repo: x # comment\n branch: main\n") + + if !doc.Delete("site", "repo") { + t.Error("expected nested delete to succeed") + } + + if !doc.Delete("a") { + t.Error("expected top-level delete to succeed") + } + + if doc.Delete("site", "missing") || doc.Delete("missing", "key") || doc.Delete() { + t.Error("expected deleting missing keys to return false") + } + + assertOutput(t, doc, "site:\n branch: main\n") +} + +func TestRenameKey(t *testing.T) { + doc := mustParse(t, "sites:\n # the site\n example.com: # inline\n a: 1\n other.com:\n b: 2\n") + + path := []string{"sites", "example.com"} + if err := doc.RenameKey("mysite.com", path...); err != nil { + t.Fatal(err) + } + + if path[1] != "example.com" { + t.Errorf("RenameKey modified the caller's path: %v", path) + } + + assertOutput(t, doc, "sites:\n # the site\n mysite.com: # inline\n a: 1\n other.com:\n b: 2\n") + + if err := doc.RenameKey("other.com", "sites", "mysite.com"); err == nil { + t.Error("expected error renaming to an existing key") + } + + if err := doc.RenameKey("x", "sites", "missing"); err == nil { + t.Error("expected error renaming a missing key") + } +} + +func TestHeadComment(t *testing.T) { + doc := mustParse(t, "# original\n\na: 1\n") + + doc.SetHeadComment("# Created by test\n" + doc.HeadComment()) + + assertOutput(t, doc, "# Created by test\n# original\n\na: 1\n") +} + +func TestSave(t *testing.T) { + path := filepath.Join(t.TempDir(), "file.yml") + + if err := os.WriteFile(path, []byte("a: 1\n"), 0600); err != nil { + t.Fatal(err) + } + + doc, err := Load(path) + if err != nil { + t.Fatal(err) + } + + if err := doc.Set(2, "a"); err != nil { + t.Fatal(err) + } + + if err := doc.Save(path); err != nil { + t.Fatal(err) + } + + data, _ := os.ReadFile(path) + if string(data) != "a: 2\n" { + t.Errorf("unexpected file contents: %q", data) + } + + info, _ := os.Stat(path) + if info.Mode().Perm() != 0600 { + t.Errorf("expected permissions 0600, got %o", info.Mode().Perm()) + } +} + +func mustParse(t *testing.T, input string) *Document { + t.Helper() + + doc, err := Parse([]byte(input)) + if err != nil { + t.Fatal(err) + } + + return doc +} + +func assertRoundTrip(t *testing.T, input string) { + t.Helper() + assertOutput(t, mustParse(t, input), input) +} + +func assertOutput(t *testing.T, doc *Document, expected string) { + t.Helper() + + out, err := doc.Bytes() + if err != nil { + t.Fatal(err) + } + + if strings.TrimRight(string(out), "\n") != strings.TrimRight(expected, "\n") { + t.Errorf("expected:\n%s\ngot:\n%s", expected, out) + } +} From 70ef27d8c8145fadc5ac812ab8918c62e6b23513 Mon Sep 17 00:00:00 2001 From: Scott Walkinshaw Date: Sun, 18 Oct 2026 12:55:32 -0400 Subject: [PATCH 2/2] Support sequence indexes in yamledit paths Co-Authored-By: Claude Opus 5.5 --- trellis/yamledit/yamledit.go | 69 +++++++++++++++++++++---------- trellis/yamledit/yamledit_test.go | 36 +++++++++++++++- 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/trellis/yamledit/yamledit.go b/trellis/yamledit/yamledit.go index d9834386..e7f0128d 100644 --- a/trellis/yamledit/yamledit.go +++ b/trellis/yamledit/yamledit.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "slices" + "strconv" "strings" "go.yaml.in/yaml/v3" @@ -110,7 +111,7 @@ func (d *Document) Decode(v any, path ...string) error { } // Set encodes value and stores it at the given key path, creating any missing -// mappings along the way. Comments on an existing value are kept, as is the +// mappings along the way (sequence items must already exist). Comments on an existing value are kept, as is the // quoting style when replacing a quoted scalar with another scalar. func (d *Document) Set(value any, path ...string) error { if len(path) == 0 { @@ -122,28 +123,34 @@ func (d *Document) Set(value any, path ...string) error { return err } - mapping := d.body() - for i, key := range path[:len(path)-1] { - _, next := findKey(mapping, key) + parent := d.body() + for i, key := range path { + if parent.Kind != yaml.MappingNode && parent.Kind != yaml.SequenceNode { + return fmt.Errorf("%s is not a mapping or sequence", formatPath(path[:i])) + } - if next == nil { - next = &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} - appendKey(mapping, key, next) - } else if next.Kind != yaml.MappingNode { - return fmt.Errorf("%s is not a mapping", formatPath(path[:i+1])) + _, next := child(parent, key) + if next != nil { + parent = next + continue } - mapping = next - } + if parent.Kind == yaml.SequenceNode { + return fmt.Errorf("index out of range: %s", formatPath(path[:i+1])) + } - key := path[len(path)-1] - _, existing := findKey(mapping, key) + if i == len(path)-1 { + appendKey(parent, key, &newNode) + return nil + } - if existing == nil { - appendKey(mapping, key, &newNode) - return nil + next = &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + appendKey(parent, key, next) + parent = next } + existing := parent + if existing.Kind == yaml.ScalarNode && newNode.Kind == yaml.ScalarNode && isQuoted(existing.Style) { newNode.Style = existing.Style } @@ -188,6 +195,10 @@ func (d *Document) RenameKey(newKey string, path ...string) error { return err } + if key == nil { + return fmt.Errorf("%s is a sequence item, not a key", formatPath(path)) + } + if key.Value == newKey { return nil } @@ -214,12 +225,8 @@ func (d *Document) find(path []string) (key *yaml.Node, value *yaml.Node, err er value = d.body() for i, k := range path { - if value.Kind != yaml.MappingNode { - return nil, nil, fmt.Errorf("%s is not a mapping", formatPath(path[:i])) - } - - key, value = findKey(value, k) - if key == nil { + key, value = child(value, k) + if value == nil { return nil, nil, fmt.Errorf("key not found: %s", formatPath(path[:i+1])) } } @@ -227,6 +234,24 @@ func (d *Document) find(path []string) (key *yaml.Node, value *yaml.Node, err er return key, value, nil } +// child returns the key and value nodes for a path segment. Sequence items are +// addressed by index and have no key node. +func child(node *yaml.Node, segment string) (key *yaml.Node, value *yaml.Node) { + switch node.Kind { + case yaml.MappingNode: + return findKey(node, segment) + case yaml.SequenceNode: + index, err := strconv.Atoi(segment) + if err != nil || index < 0 || index >= len(node.Content) { + return nil, nil + } + + return nil, node.Content[index] + default: + return nil, nil + } +} + func findKey(mapping *yaml.Node, key string) (*yaml.Node, *yaml.Node) { for i := 0; i < len(mapping.Content)-1; i += 2 { if mapping.Content[i].Value == key { diff --git a/trellis/yamledit/yamledit_test.go b/trellis/yamledit/yamledit_test.go index a4a852e2..726f45ef 100644 --- a/trellis/yamledit/yamledit_test.go +++ b/trellis/yamledit/yamledit_test.go @@ -162,7 +162,7 @@ func TestSet(t *testing.T) { } func TestSetErrors(t *testing.T) { - doc := mustParse(t, "a: 1\n") + doc := mustParse(t, "a: 1\nlist:\n - x\n") if err := doc.Set("x"); err == nil { t.Error("expected error for empty path") @@ -171,6 +171,40 @@ func TestSetErrors(t *testing.T) { if err := doc.Set("x", "a", "b"); err == nil { t.Error("expected error when traversing a scalar") } + + if err := doc.Set("x", "list", "1"); err == nil { + t.Error("expected error for out of range sequence index") + } + + if err := doc.Set("x", "list", "name"); err == nil { + t.Error("expected error for non-numeric sequence index") + } +} + +func TestSequenceItems(t *testing.T) { + doc := mustParse(t, "users:\n - name: a\n salt: \"generateme\"\n - name: b\n") + + if node := doc.Lookup("users", "1", "name"); node == nil || node.Value != "b" { + t.Errorf("Lookup by index returned %v", node) + } + + if node := doc.Lookup("users", "2"); node != nil { + t.Errorf("expected nil for out of range index, got %v", node) + } + + if err := doc.Set("secret", "users", "0", "salt"); err != nil { + t.Fatal(err) + } + + if err := doc.Set("pw", "users", "1", "password"); err != nil { + t.Fatal(err) + } + + if err := doc.RenameKey("x", "users", "0"); err == nil { + t.Error("expected error renaming a sequence item") + } + + assertOutput(t, doc, "users:\n - name: a\n salt: \"secret\"\n - name: b\n password: pw\n") } func TestDelete(t *testing.T) {